diff options
| author | bors <bors@rust-lang.org> | 2021-06-24 14:56:28 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-06-24 14:56:28 +0000 |
| commit | d95745e5fa39680d97a52d0e07ed7842b163bacc (patch) | |
| tree | 5121952dd0824b51d5e5a998d93fb53ce7f88a58 | |
| parent | 456a03227e3c81a51631f87ec80cac301e5fa6d7 (diff) | |
| parent | 1400cb0295210bc500aa27f6cca3ccb546b38814 (diff) | |
| download | rust-d95745e5fa39680d97a52d0e07ed7842b163bacc.tar.gz rust-d95745e5fa39680d97a52d0e07ed7842b163bacc.zip | |
Auto merge of #85427 - ehuss:fix-use-placement, r=jackh726
Fix use placement for suggestions near main. This fixes an edge case for the suggestion to add a `use`. When running with `--test`, the `main` function will be annotated with an `#[allow(dead_code)]` attribute. The `UsePlacementFinder` would end up using the dummy span of that synthetic attribute. If there are top-level inner attributes, this would place the `use` in the wrong position. The solution here is to ignore attributes with dummy spans. In the process of working on this, I discovered that the `use_suggestion_placement` test was broken. `UsePlacementFinder` is unaware of active attributes. Attributes like `#[derive]` don't exist in the AST since they are removed. Fixing that is difficult, since the AST does not retain enough information. I considered trying to place the `use` towards the top of the module after any `extern crate` items, but I couldn't find a way to get a span for the start of a module block (the `mod` span starts at the `mod` keyword, and it seems tricky to find the spot just after the opening bracket and past inner attributes). For now, I just put some comments about the issue. This appears to have been a known issue in #44215 where the test for it was introduced, and the fix seemed to be deferred to later.
| -rw-r--r-- | compiler/rustc_resolve/src/lib.rs | 16 | ||||
| -rw-r--r-- | compiler/rustc_typeck/src/check/method/suggest.rs | 16 | ||||
| -rw-r--r-- | src/test/ui/resolve/use_suggestion_placement.fixed | 39 | ||||
| -rw-r--r-- | src/test/ui/resolve/use_suggestion_placement.rs | 9 | ||||
| -rw-r--r-- | src/test/ui/resolve/use_suggestion_placement.stderr | 6 | ||||
| -rw-r--r-- | src/test/ui/suggestions/use-placement-resolve.fixed | 13 | ||||
| -rw-r--r-- | src/test/ui/suggestions/use-placement-resolve.rs | 11 | ||||
| -rw-r--r-- | src/test/ui/suggestions/use-placement-resolve.stderr | 14 | ||||
| -rw-r--r-- | src/test/ui/suggestions/use-placement-typeck.fixed | 22 | ||||
| -rw-r--r-- | src/test/ui/suggestions/use-placement-typeck.rs | 20 | ||||
| -rw-r--r-- | src/test/ui/suggestions/use-placement-typeck.stderr | 21 |
11 files changed, 167 insertions, 20 deletions
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 00f0fe4a288..83e904eb16b 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -335,15 +335,15 @@ impl UsePlacementFinder { if self.span.map_or(true, |span| item.span < span) && !item.span.from_expansion() { + self.span = Some(item.span.shrink_to_lo()); // don't insert between attributes and an item - if item.attrs.is_empty() { - self.span = Some(item.span.shrink_to_lo()); - } else { - // find the first attribute on the item - for attr in &item.attrs { - if self.span.map_or(true, |span| attr.span < span) { - self.span = Some(attr.span.shrink_to_lo()); - } + // find the first attribute on the item + // FIXME: This is broken for active attributes. + for attr in &item.attrs { + if !attr.span.is_dummy() + && self.span.map_or(true, |span| attr.span < span) + { + self.span = Some(attr.span.shrink_to_lo()); } } } diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index 16382c7e7a4..3cb4ac8e182 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -1650,16 +1650,16 @@ impl intravisit::Visitor<'tcx> for UsePlacementFinder<'tcx> { _ => { if self.span.map_or(true, |span| item.span < span) { if !item.span.from_expansion() { + self.span = Some(item.span.shrink_to_lo()); // Don't insert between attributes and an item. let attrs = self.tcx.hir().attrs(item.hir_id()); - if attrs.is_empty() { - self.span = Some(item.span.shrink_to_lo()); - } else { - // Find the first attribute on the item. - for attr in attrs { - if self.span.map_or(true, |span| attr.span < span) { - self.span = Some(attr.span.shrink_to_lo()); - } + // Find the first attribute on the item. + // FIXME: This is broken for active attributes. + for attr in attrs { + if !attr.span.is_dummy() + && self.span.map_or(true, |span| attr.span < span) + { + self.span = Some(attr.span.shrink_to_lo()); } } } diff --git a/src/test/ui/resolve/use_suggestion_placement.fixed b/src/test/ui/resolve/use_suggestion_placement.fixed new file mode 100644 index 00000000000..63676327aa0 --- /dev/null +++ b/src/test/ui/resolve/use_suggestion_placement.fixed @@ -0,0 +1,39 @@ +// run-rustfix +#![allow(dead_code)] + +use m::A; + +use std::collections::HashMap; + +macro_rules! y { + () => {} +} + +mod m { + pub const A: i32 = 0; +} + +mod foo { + // FIXME: UsePlacementFinder is broken because active attributes are + // removed, and thus the `derive` attribute here is not in the AST. + // An inert attribute should work, though. + // #[derive(Debug)] + use std::path::Path; + +#[allow(warnings)] + pub struct Foo; + + // test whether the use suggestion isn't + // placed into the expansion of `#[derive(Debug)] + type Bar = Path; //~ ERROR cannot find +} + +fn main() { + y!(); + let _ = A; //~ ERROR cannot find + foo(); +} + +fn foo() { + type Dict<K, V> = HashMap<K, V>; //~ ERROR cannot find +} diff --git a/src/test/ui/resolve/use_suggestion_placement.rs b/src/test/ui/resolve/use_suggestion_placement.rs index 56d4b8d6d11..ecc74d78167 100644 --- a/src/test/ui/resolve/use_suggestion_placement.rs +++ b/src/test/ui/resolve/use_suggestion_placement.rs @@ -1,3 +1,6 @@ +// run-rustfix +#![allow(dead_code)] + macro_rules! y { () => {} } @@ -7,7 +10,11 @@ mod m { } mod foo { - #[derive(Debug)] + // FIXME: UsePlacementFinder is broken because active attributes are + // removed, and thus the `derive` attribute here is not in the AST. + // An inert attribute should work, though. + // #[derive(Debug)] + #[allow(warnings)] pub struct Foo; // test whether the use suggestion isn't diff --git a/src/test/ui/resolve/use_suggestion_placement.stderr b/src/test/ui/resolve/use_suggestion_placement.stderr index af0495a57a1..217c08a560b 100644 --- a/src/test/ui/resolve/use_suggestion_placement.stderr +++ b/src/test/ui/resolve/use_suggestion_placement.stderr @@ -1,5 +1,5 @@ error[E0412]: cannot find type `Path` in this scope - --> $DIR/use_suggestion_placement.rs:15:16 + --> $DIR/use_suggestion_placement.rs:22:16 | LL | type Bar = Path; | ^^^^ not found in this scope @@ -10,7 +10,7 @@ LL | use std::path::Path; | error[E0425]: cannot find value `A` in this scope - --> $DIR/use_suggestion_placement.rs:20:13 + --> $DIR/use_suggestion_placement.rs:27:13 | LL | let _ = A; | ^ not found in this scope @@ -21,7 +21,7 @@ LL | use m::A; | error[E0412]: cannot find type `HashMap` in this scope - --> $DIR/use_suggestion_placement.rs:25:23 + --> $DIR/use_suggestion_placement.rs:32:23 | LL | type Dict<K, V> = HashMap<K, V>; | ^^^^^^^ not found in this scope diff --git a/src/test/ui/suggestions/use-placement-resolve.fixed b/src/test/ui/suggestions/use-placement-resolve.fixed new file mode 100644 index 00000000000..afe74cff2e9 --- /dev/null +++ b/src/test/ui/suggestions/use-placement-resolve.fixed @@ -0,0 +1,13 @@ +// compile-flags: --test +// run-rustfix +// Checks that the `use` suggestion appears *below* this inner attribute. +// There was an issue where the test synthetic #[allow(dead)] attribute on +// main which has a dummy span caused the suggestion to be placed at the top +// of the file. +#![allow(unused)] + +use std::fmt::Debug; + +fn main() {} + +fn foobar<T: Debug>(x: T) {} //~ ERROR expected trait, found derive macro diff --git a/src/test/ui/suggestions/use-placement-resolve.rs b/src/test/ui/suggestions/use-placement-resolve.rs new file mode 100644 index 00000000000..b30ddb3af07 --- /dev/null +++ b/src/test/ui/suggestions/use-placement-resolve.rs @@ -0,0 +1,11 @@ +// compile-flags: --test +// run-rustfix +// Checks that the `use` suggestion appears *below* this inner attribute. +// There was an issue where the test synthetic #[allow(dead)] attribute on +// main which has a dummy span caused the suggestion to be placed at the top +// of the file. +#![allow(unused)] + +fn main() {} + +fn foobar<T: Debug>(x: T) {} //~ ERROR expected trait, found derive macro diff --git a/src/test/ui/suggestions/use-placement-resolve.stderr b/src/test/ui/suggestions/use-placement-resolve.stderr new file mode 100644 index 00000000000..9da9e8e2702 --- /dev/null +++ b/src/test/ui/suggestions/use-placement-resolve.stderr @@ -0,0 +1,14 @@ +error[E0404]: expected trait, found derive macro `Debug` + --> $DIR/use-placement-resolve.rs:11:14 + | +LL | fn foobar<T: Debug>(x: T) {} + | ^^^^^ not a trait + | +help: consider importing this trait instead + | +LL | use std::fmt::Debug; + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0404`. diff --git a/src/test/ui/suggestions/use-placement-typeck.fixed b/src/test/ui/suggestions/use-placement-typeck.fixed new file mode 100644 index 00000000000..40c55d1dd06 --- /dev/null +++ b/src/test/ui/suggestions/use-placement-typeck.fixed @@ -0,0 +1,22 @@ +// compile-flags: --test +// run-rustfix +// Checks that the `use` suggestion appears *below* this inner attribute. +// There was an issue where the test synthetic #[allow(dead)] attribute on +// main which has a dummy span caused the suggestion to be placed at the top +// of the file. +#![allow(unused)] + +use m::Foo; + +fn main() { + let s = m::S; + s.abc(); //~ ERROR no method named `abc` +} + +mod m { + pub trait Foo { + fn abc(&self) {} + } + pub struct S; + impl Foo for S{} +} diff --git a/src/test/ui/suggestions/use-placement-typeck.rs b/src/test/ui/suggestions/use-placement-typeck.rs new file mode 100644 index 00000000000..aab20d2e90a --- /dev/null +++ b/src/test/ui/suggestions/use-placement-typeck.rs @@ -0,0 +1,20 @@ +// compile-flags: --test +// run-rustfix +// Checks that the `use` suggestion appears *below* this inner attribute. +// There was an issue where the test synthetic #[allow(dead)] attribute on +// main which has a dummy span caused the suggestion to be placed at the top +// of the file. +#![allow(unused)] + +fn main() { + let s = m::S; + s.abc(); //~ ERROR no method named `abc` +} + +mod m { + pub trait Foo { + fn abc(&self) {} + } + pub struct S; + impl Foo for S{} +} diff --git a/src/test/ui/suggestions/use-placement-typeck.stderr b/src/test/ui/suggestions/use-placement-typeck.stderr new file mode 100644 index 00000000000..21f22dade2c --- /dev/null +++ b/src/test/ui/suggestions/use-placement-typeck.stderr @@ -0,0 +1,21 @@ +error[E0599]: no method named `abc` found for struct `S` in the current scope + --> $DIR/use-placement-typeck.rs:11:7 + | +LL | s.abc(); + | ^^^ method not found in `S` +... +LL | fn abc(&self) {} + | --- the method is available for `S` here +LL | } +LL | pub struct S; + | ------------- method `abc` not found for this + | + = help: items from traits can only be used if the trait is in scope +help: the following trait is implemented but not in scope; perhaps add a `use` for it: + | +LL | use m::Foo; + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. |
