Remove braces when fixing a nested use tree into a single item#123344

emilyalbini · 2024-04-01T23:15:27Z

Back in 2019 I added rustfix support for the unused_imports lint, to automatically remove them when running cargo fix. For the most part this worked great, but when removing all but one childs of a nested use tree it turned use foo::{Unused, Used} into use foo::{Used}. This is slightly annoying, because it then requires you to run rustfmt to get use foo::Used.

This PR automatically removes braces and the surrouding whitespace when all but one child of a nested use tree are unused. To get it done I had to add the span of the nested use tree to the AST, and refactor a bit the code I wrote back then.

A thing I noticed is, there doesn't seem to be any //@ run-rustfix test for fixing the unused_imports lint. I created a test in tests/suggestions (is that the right directory?) that for now tests just what I added in the PR. I can followup in a separate PR to add more tests for fixing unused_lints.

This PR is best reviewed commit-by-commit.

rustbot · 2024-04-01T23:15:35Z

r? @wesleywiser

rustbot has assigned @wesleywiser.
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

rustbot · 2024-04-01T23:29:08Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

rustbot · 2024-04-02T18:17:43Z

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

tests/ui/lint/unnecessary-qualification/lint-unnecessary-qualification-issue-121331.fixed

emilyalbini · 2024-04-14T18:20:40Z

All tests pass, this is ready for review.

@rustbot ready

Noratrieb · 2024-05-08T16:33:10Z

compiler/rustc_ast/src/ast.rs

    Simple(Option<Ident>),
    /// `use prefix::{...}`
-    Nested(ThinVec<(UseTree, NodeId)>),
+    Nested { items: ThinVec<(UseTree, NodeId)>, span: Span },


this could use a comment explaining what the span is for (the parentheses, I was told)

Noratrieb · 2024-05-08T16:40:26Z

@pietroalbini told me he does feel like adding the comment later so I will approve this now. if adding the comment never happens, that's sad but ok too.

@bors r+

bors · 2024-05-08T16:40:30Z

📌 Commit 2d3a9a5 has been approved by Nilstrieb

It is now in the queue for this repository.

Noratrieb · 2024-05-08T16:42:12Z

compiler/rustc_resolve/src/check_unused.rs

            let mut unused_spans = Vec::new();
            let mut to_remove = Vec::new();
-            let mut all_nested_unused = true;
+            let mut used_childs = 0;


we agreed to not talk about this one

…Nilstrieb Remove braces when fixing a nested use tree into a single item [Back in 2019](rust-lang#56645) I added rustfix support for the `unused_imports` lint, to automatically remove them when running `cargo fix`. For the most part this worked great, but when removing all but one childs of a nested use tree it turned `use foo::{Unused, Used}` into `use foo::{Used}`. This is slightly annoying, because it then requires you to run `rustfmt` to get `use foo::Used`. This PR automatically removes braces and the surrouding whitespace when all but one child of a nested use tree are unused. To get it done I had to add the span of the nested use tree to the AST, and refactor a bit the code I wrote back then. A thing I noticed is, there doesn't seem to be any `//@ run-rustfix` test for fixing the `unused_imports` lint. I created a test in `tests/suggestions` (is that the right directory?) that for now tests just what I added in the PR. I can followup in a separate PR to add more tests for fixing `unused_lints`. This PR is best reviewed commit-by-commit.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#123344 (Remove braces when fixing a nested use tree into a single item) - rust-lang#124587 (Generic `NonZero` post-stabilization changes.) - rust-lang#124775 (crashes: add lastest batch of crash tests) - rust-lang#124869 (Make sure we don't deny macro vars w keyword names) - rust-lang#124876 (Simplify `use crate::rustc_foo::bar` occurrences.) - rust-lang#124892 (Update cc crate to v1.0.97) - rust-lang#124903 (Ignore empty RUSTC_WRAPPER in bootstrap) - rust-lang#124909 (Reapply the part of rust-lang#124548 that bors forgot) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#123344 - pietroalbini:pa-unused-imports, r=Nilstrieb Remove braces when fixing a nested use tree into a single item [Back in 2019](rust-lang#56645) I added rustfix support for the `unused_imports` lint, to automatically remove them when running `cargo fix`. For the most part this worked great, but when removing all but one childs of a nested use tree it turned `use foo::{Unused, Used}` into `use foo::{Used}`. This is slightly annoying, because it then requires you to run `rustfmt` to get `use foo::Used`. This PR automatically removes braces and the surrouding whitespace when all but one child of a nested use tree are unused. To get it done I had to add the span of the nested use tree to the AST, and refactor a bit the code I wrote back then. A thing I noticed is, there doesn't seem to be any `//@ run-rustfix` test for fixing the `unused_imports` lint. I created a test in `tests/suggestions` (is that the right directory?) that for now tests just what I added in the PR. I can followup in a separate PR to add more tests for fixing `unused_lints`. This PR is best reviewed commit-by-commit.

…Nilstrieb Remove braces when fixing a nested use tree into a single item [Back in 2019](rust-lang#56645) I added rustfix support for the `unused_imports` lint, to automatically remove them when running `cargo fix`. For the most part this worked great, but when removing all but one childs of a nested use tree it turned `use foo::{Unused, Used}` into `use foo::{Used}`. This is slightly annoying, because it then requires you to run `rustfmt` to get `use foo::Used`. This PR automatically removes braces and the surrouding whitespace when all but one child of a nested use tree are unused. To get it done I had to add the span of the nested use tree to the AST, and refactor a bit the code I wrote back then. A thing I noticed is, there doesn't seem to be any `//@ run-rustfix` test for fixing the `unused_imports` lint. I created a test in `tests/suggestions` (is that the right directory?) that for now tests just what I added in the PR. I can followup in a separate PR to add more tests for fixing `unused_lints`. This PR is best reviewed commit-by-commit.

…s, r=Nilstrieb Followup fixes from rust-lang#123344 `@Nilstrieb` doesn't deserve [to be sad](rust-lang#123344 (comment)), so this PR addresses the two pieces of feedback from that PR. r? `@Nilstrieb`

…llaumeGomez Rollup of 3 pull requests Successful merges: - rust-lang#125261 (crashes: add more) - rust-lang#125270 (Followup fixes from rust-lang#123344) - rust-lang#125275 (Migrate `run-make/rustdoc-scrape-examples-test` to new `rmake.rs`) r? `@ghost` `@rustbot` modify labels: rollup

…s, r=Nilstrieb Followup fixes from rust-lang#123344 ``@Nilstrieb`` doesn't deserve [to be sad](rust-lang#123344 (comment)), so this PR addresses the two pieces of feedback from that PR. r? ``@Nilstrieb``

…mpiler-errors Rollup of 7 pull requests Successful merges: - rust-lang#123709 (Update documentation related to the recent cmd.exe fix) - rust-lang#124304 (revise the interpretation of ReadDir for HermitOS) - rust-lang#124708 (Actually use the `#[do_not_recommend]` attribute if present) - rust-lang#125252 (Add `#[inline]` to float `Debug` fallback used by `cfg(no_fp_fmt_parse)`) - rust-lang#125261 (crashes: add more) - rust-lang#125270 (Followup fixes from rust-lang#123344) - rust-lang#125275 (Migrate `run-make/rustdoc-scrape-examples-test` to new `rmake.rs`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#125270 - pietroalbini:pa-no-sad-contributors, r=Nilstrieb Followup fixes from rust-lang#123344 ``@Nilstrieb`` doesn't deserve [to be sad](rust-lang#123344 (comment)), so this PR addresses the two pieces of feedback from that PR. r? ``@Nilstrieb``

…llaumeGomez Rollup of 3 pull requests Successful merges: - rust-lang#125261 (crashes: add more) - rust-lang#125270 (Followup fixes from rust-lang#123344) - rust-lang#125275 (Migrate `run-make/rustdoc-scrape-examples-test` to new `rmake.rs`) r? `@ghost` `@rustbot` modify labels: rollup

…Nilstrieb Remove braces when fixing a nested use tree into a single item [Back in 2019](rust-lang#56645) I added rustfix support for the `unused_imports` lint, to automatically remove them when running `cargo fix`. For the most part this worked great, but when removing all but one childs of a nested use tree it turned `use foo::{Unused, Used}` into `use foo::{Used}`. This is slightly annoying, because it then requires you to run `rustfmt` to get `use foo::Used`. This PR automatically removes braces and the surrouding whitespace when all but one child of a nested use tree are unused. To get it done I had to add the span of the nested use tree to the AST, and refactor a bit the code I wrote back then. A thing I noticed is, there doesn't seem to be any `//@ run-rustfix` test for fixing the `unused_imports` lint. I created a test in `tests/suggestions` (is that the right directory?) that for now tests just what I added in the PR. I can followup in a separate PR to add more tests for fixing `unused_lints`. This PR is best reviewed commit-by-commit.

rustbot assigned wesleywiser Apr 1, 2024

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 Apr 1, 2024

This comment has been minimized.

Sign in to view

emilyalbini force-pushed the pa-unused-imports branch from a58b963 to 47788ff Compare April 2, 2024 20:11

This comment has been minimized.

Sign in to view

emilyalbini 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 Apr 2, 2024

calebcartwright reviewed Apr 9, 2024

View reviewed changes

tests/ui/lint/unnecessary-qualification/lint-unnecessary-qualification-issue-121331.fixed Outdated Show resolved Hide resolved

emilyalbini added 5 commits April 14, 2024 18:45

add enum variant field names to make the code clearer

829338a

remove redundant flat vs nested distinction to simplify enum

2c26072

turn all_nested_unused into used_childs

2ec337c

store the span of the nested part of the use tree in the ast

13f7623

remove braces when fixing a nested use tree into a single use

2d3a9a5

emilyalbini force-pushed the pa-unused-imports branch from 47788ff to 2d3a9a5 Compare April 14, 2024 16:45

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 Apr 14, 2024

Noratrieb reviewed May 8, 2024

View reviewed changes

bors 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 May 8, 2024

Noratrieb reviewed May 8, 2024

View reviewed changes

This was referenced May 8, 2024

Rollup of 7 pull requests #124906

Closed

Rollup of 8 pull requests #124910

Merged

bors merged commit d30af5e into rust-lang:master May 8, 2024

rustbot added this to the 1.80.0 milestone May 8, 2024

emilyalbini deleted the pa-unused-imports branch May 9, 2024 12:00

weihanglo mentioned this pull request May 9, 2024

test(rustfix): run some tests only on nightly rust-lang/cargo#13890

Merged

emilyalbini mentioned this pull request May 19, 2024

Followup fixes from #123344 #125270

Merged

GuillaumeGomez mentioned this pull request May 19, 2024

Rollup of 3 pull requests #125278

Closed

compiler-errors mentioned this pull request May 19, 2024

Rollup of 7 pull requests #125280

Merged

Pyr0de mentioned this pull request Apr 13, 2025

test(rustfix): Use snapbox for snapshot testing rust-lang/cargo#15429

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove braces when fixing a nested use tree into a single item#123344