Skip to content

Respect file-lines range per predicate in where clauses #6874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Souradip121 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Respect file-lines range per predicate in where clauses #6874

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bfc783f
Respect file-lines range per predicate in where clauses
Souradip121 Apr 21, 2026
8f10037
Move file-lines check into ListItems::next()
Souradip121 Apr 21, 2026
528e7ee
Skip reformatting when entire where clause is out of file-lines range
Souradip121 Apr 21, 2026
638f61c
Make file-lines-where-clause test diff more obvious
Souradip121 Apr 21, 2026
be33c5b
Test where-clause preservation when no predicate lines selected
Souradip121 Apr 21, 2026
130d6df
Drop redundant is_all() check in where-clause skip guard
Souradip121 Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/items.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3136,7 +3136,13 @@ fn rewrite_bounds_on_where_clause(
",",
|pred| pred.span().lo(),
|pred| pred.span().hi(),
|pred| pred.rewrite_result(context, shape),
|pred| {
if out_of_file_lines_range!(context, pred.span()) {
Ok(context.snippet(pred.span()).to_owned())
} else {
pred.rewrite_result(context, shape)
}
},
Copy link
Copy Markdown
Contributor

@ytmimi ytmimi Apr 21, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing this check here, I'm wondering if we could move this logic to the Iterator impl for ListItems. It will likely require us to pass the whole RewriteContext to itemize_list instead of just the snippet_provider, but that should be fine.

Moving this check there means that we wouldn't need to duplicateif out_of_file_lines_range! every time we use itemize_list, and it means that you'd actually fix things for other code paths that use this pattern.

View changes since the review

Copy link
Copy Markdown
Author

@Souradip121 Souradip121 Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, made a commit, kindly check.

span_start,
span_end,
false,
Expand Down Expand Up @@ -3222,7 +3228,13 @@ fn rewrite_where_clause(
",",
|pred| pred.span().lo(),
|pred| pred.span().hi(),
|pred| pred.rewrite_result(context, Shape::legacy(budget, offset)),
|pred| {
if out_of_file_lines_range!(context, pred.span()) {
Ok(context.snippet(pred.span()).to_owned())
} else {
pred.rewrite_result(context, Shape::legacy(budget, offset))
}
},
span_start,
span_end,
false,
Expand Down
9 changes: 9 additions & 0 deletions tests/source/file-lines-where-clause.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// rustfmt-file_lines: [{"file":"tests/source/file-lines-where-clause.rs","range":[5,5]}]

fn foo<T, U, V>()
where
T: Clone + Debug,
U: Copy,
V: Default,
Copy link
Copy Markdown
Contributor

@randomPoison randomPoison Apr 21, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor note: The way this test is written it's a bit hard to see what exactly is changing between the source file and target file, since only a single space is changing. I'd suggest changing the input file so that the bad formatting is more obvious, such that it's easier to tell at a glance what the test covers:

fn foo<T, U, V>()
where
    T     :     Clone      +     Debug,
    U    :       Copy,
    V   :        Default,

Would then become

fn foo<T, U, V>()
where
    T: Clone + Debug,
    U    :       Copy,
    V   :        Default,

View changes since the review

{
}
9 changes: 9 additions & 0 deletions tests/target/file-lines-where-clause.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// rustfmt-file_lines: [{"file":"tests/source/file-lines-where-clause.rs","range":[5,5]}]

fn foo<T, U, V>()
where
T: Clone + Debug,
U: Copy,
V: Default,
{
}
Loading