Keep comments inside parens when suggesting to remove unused delims by yashaggarwal85d · Pull Request #165036 · rust-lang/rust

yashaggarwal85d · 2026-10-27T12:15:37Z

When there's a comment between a paren and the inner expression, unused_parens suggests removing
it along with the paren, and with line breaks the spans cover the whole block. This points at the
delimiters only in that case, and only removes the delimiter on a side that has a comment.

Fixes #164722

FYI, tests are written with the help of AI. I reviewed and they are working as expected. Thanks

rustbot · 2026-10-27T12:15:43Z

r? @adwinwhite

rustbot has assigned @adwinwhite.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 77 candidates
Random selection from 20 candidates

oli-obk · 2026-10-29T12:36:08Z

Reopening after a conversation with the author

yashaggarwal85d · 2026-10-29T14:16:11Z

Hi @adwinwhite can you have a look at this! Lemme know if any change is needed! Thanks

adwinwhite

Could you also fix the CI error?

View changes since this review

adwinwhite · 2026-11-02T03:43:34Z

+        let sm = cx.sess().source_map();
+        // If there is a comment or a line break between a delimiter and the inner value,
+        // point at the delimiter alone, and never suggest removing the comment.
+        let shrink = |span: Span, open: bool, keep_newlines: bool| {


Why do we need keep_newlines? It's better to keep spans in main message and help consistent?

The rule is if we have comments, we keep newlines/other whilespaces? 🤔

Yeah, dropped it. Now each side shrinks to just the delimiter only if there's a comment, same spans for the message and the help.

adwinwhite · 2026-11-02T03:46:27Z

+            let (delim, gap) = if open {
+                (snip.starts_with(['(', '{']), snip.get(1..))
+            } else {
+                (snip.ends_with([')', '}']), snip.get(..snip.len().saturating_sub(1)))
+            };
+            let Some(gap) = gap.filter(|_| delim) else { return span };


We can check whether the spans are empty and obtain gap directly?

Done, gap is taken from the span now.

adwinwhite · 2026-11-02T03:49:07Z

@rustbot author

rustbot · 2026-11-02T03:49:12Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

rustbot · 2026-11-02T04:38:49Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

yashaggarwal85d · 2026-11-02T05:13:26Z

Could you also fix the CI error?

View changes since this review

Sure, I'll re-request a review as soon as the CI passes!

adwinwhite · 2026-11-03T01:21:20Z

+LL |        if (
+   |  _________^
+LL | |          true
+   | | ________^____^
+   | ||________|
+   |  |
+LL |  |     ) {}
+   |  |_____^


Sorry, I get the newline thing now.

I guess the rules are

We display single letter span in warning, regardless of whether there's a comment.

We shrink the span to a single letter in help/autofix, if there's a comment.

View changes since the review

adwinwhite · 2026-11-03T01:22:38Z

+    let (/* d */ _x) = 1;
+    //~^ WARN unnecessary parentheses around pattern
+
+    foo::<{ /* e */ 7 }>();


We need more test cases for braces? Like one-line closure body.

View changes since the review

adwinwhite · 2026-11-04T02:03:16Z

+        let sm = cx.sess().source_map();
+        // Returns the delimiter itself, and whether there is a comment between it and the
+        // inner value.
+        let delim = |span: Span, open: bool| {


Using Option combinators below makes the code quite unreadable. And the delim naming is confusing.
How about using previous let shrink = |span: Span, open: bool, in_help: bool| -> Span?

View changes since the review

Yeah that sounds good, I'll do exactly that

adwinwhite · 2026-11-09T02:09:45Z

Thanks!

@bors rollup
@bors r+ squash

* Keep comments inside parens when suggesting to remove unused delims * Only shrink unused delim spans when there is a comment * Always point at the delimiters alone in unused delims warnings * Use a single shrink closure for unused delim spans

rust-bors · 2026-11-09T02:11:07Z

🔨 4 commits were squashed into 7c2e5a5.

rust-bors · 2026-11-09T02:11:08Z

📌 Commit 7c2e5a5 has been tentatively approved by adwinwhite

It will be put into the queue for this repository once PR CI succeeds.

…comments, r=adwinwhite Keep comments inside parens when suggesting to remove unused delims When there's a comment between a paren and the inner expression, `unused_parens` suggests removing it along with the paren, and with line breaks the spans cover the whole block. This points at the delimiters only in that case, and only removes the delimiter on a side that has a comment. Fixes rust-lang#164722 FYI, tests are written with the help of AI. I reviewed and they are working as expected. Thanks

Rollup of 7 pull requests Successful merges: - #165660 (rustdoc: suggest reference definitions for unresolved links) - #163749 (Refactor enum layout calculation) - #165036 (Keep comments inside parens when suggesting to remove unused delims) - #165658 (get rid of InterpCx::get_alloc_bytes_unchecked_raw(_mut)) - #165664 (Fix unused imports in VEXos filesystem) - #165666 (Error on `macro_use` attribute on decl macro) - #165667 (Bump deps for `citool` and main workspace)

Rollup merge of #165036 - yashaggarwal85d:fix-unused-delims-comments, r=adwinwhite Keep comments inside parens when suggesting to remove unused delims When there's a comment between a paren and the inner expression, `unused_parens` suggests removing it along with the paren, and with line breaks the spans cover the whole block. This points at the delimiters only in that case, and only removes the delimiter on a side that has a comment. Fixes #164722 FYI, tests are written with the help of AI. I reviewed and they are working as expected. Thanks

Rollup of 7 pull requests Successful merges: - rust-lang/rust#165660 (rustdoc: suggest reference definitions for unresolved links) - rust-lang/rust#163749 (Refactor enum layout calculation) - rust-lang/rust#165036 (Keep comments inside parens when suggesting to remove unused delims) - rust-lang/rust#165658 (get rid of InterpCx::get_alloc_bytes_unchecked_raw(_mut)) - rust-lang/rust#165664 (Fix unused imports in VEXos filesystem) - rust-lang/rust#165666 (Error on `macro_use` attribute on decl macro) - rust-lang/rust#165667 (Bump deps for `citool` and main workspace)

rustbot assigned adwinwhite Oct 27, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 27, 2026

oli-obk closed this Oct 28, 2026

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 28, 2026

oli-obk reopened this Oct 29, 2026

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 29, 2026

This comment has been minimized.

Sign in to view

adwinwhite reviewed Nov 2, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 2, 2026

yashaggarwal85d force-pushed the fix-unused-delims-comments branch from c5abcf1 to 7b55ab6 Compare November 2, 2026 04:38

adwinwhite reviewed Nov 3, 2026

View reviewed changes

adwinwhite reviewed Nov 4, 2026

View reviewed changes

yashaggarwal85d requested a review from adwinwhite November 7, 2026 12:51

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 7, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot force-pushed the fix-unused-delims-comments branch from c89ce74 to 7c2e5a5 Compare November 9, 2026 02:11

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2026

Zalathar mentioned this pull request Nov 9, 2026

Rollup of 3 pull requests #165671

Closed

This was referenced Nov 9, 2026

Rollup of 4 pull requests #165673

Closed

Rollup of 7 pull requests #165678

Merged

rust-bors Bot merged commit 3521ea2 into rust-lang:main Nov 9, 2026
14 checks passed

rustbot added this to the 1.102.0 milestone Nov 9, 2026

Conversation

yashaggarwal85d commented Oct 27, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 27, 2026

Uh oh!

oli-obk commented Oct 29, 2026

Uh oh!

This comment has been minimized.

yashaggarwal85d commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adwinwhite left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adwinwhite Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

yashaggarwal85d Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

adwinwhite Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

yashaggarwal85d Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

adwinwhite commented Nov 2, 2026

Uh oh!

rustbot commented Nov 2, 2026

Uh oh!

rustbot commented Nov 2, 2026

Uh oh!

yashaggarwal85d commented Nov 2, 2026

Uh oh!

adwinwhite Nov 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adwinwhite Nov 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adwinwhite Nov 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yashaggarwal85d Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

adwinwhite commented Nov 9, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Nov 9, 2026

Uh oh!

rust-bors Bot commented Nov 9, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yashaggarwal85d commented Oct 27, 2026 •

edited by rustbot

Loading

yashaggarwal85d commented Oct 29, 2026 •

edited

Loading

adwinwhite left a comment •

edited by rustbot

Loading

adwinwhite Nov 3, 2026 •

edited by rustbot

Loading

adwinwhite Nov 3, 2026 •

edited by rustbot

Loading

adwinwhite Nov 4, 2026 •

edited by rustbot

Loading