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

pietroalbini · 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

pietroalbini · 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

@@ -2703,7 +2703,7 @@ pub enum UseTreeKind {
    /// `use prefix` or `use prefix as rename`
    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.

# to view

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

This comment has been minimized.

# to view

pietroalbini 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

pietroalbini 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

pietroalbini 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
11 checks passed

rustbot added this to the 1.80.0 milestone May 8, 2024

pietroalbini 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

pietroalbini 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

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

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

pietroalbini commented Apr 1, 2024 •

edited

Loading

rustbot commented Apr 1, 2024

This comment has been minimized.

rustbot commented Apr 1, 2024

This comment has been minimized.

rustbot commented Apr 2, 2024

This comment has been minimized.

This comment has been minimized.

pietroalbini commented Apr 14, 2024

Noratrieb May 8, 2024

Noratrieb commented May 8, 2024

bors commented May 8, 2024

Noratrieb May 8, 2024

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

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

Conversation

pietroalbini commented Apr 1, 2024 • edited Loading

rustbot commented Apr 1, 2024

This comment has been minimized.

rustbot commented Apr 1, 2024

This comment has been minimized.

rustbot commented Apr 2, 2024

This comment has been minimized.

This comment has been minimized.

pietroalbini commented Apr 14, 2024

Noratrieb May 8, 2024

Choose a reason for hiding this comment

Noratrieb commented May 8, 2024

bors commented May 8, 2024

Noratrieb May 8, 2024

Choose a reason for hiding this comment

pietroalbini commented Apr 1, 2024 •

edited

Loading