Skip to content

Commit

Permalink
Auto merge of #44215 - oli-obk:import_sugg, r=nrc
Browse files Browse the repository at this point in the history
don't suggest placing `use` statements into expanded code

r? @nrc

fixes #44210

```rust
#[derive(Debug)]
struct Foo;

type X = Path;
```

will try to place `use std::path::Path;` between `#[derive(Debug)]` and `struct Foo;`

I am not sure how to obtain a span before the first attribute, because derive attributes are removed during expansion.

It would be trivial to detect this case and place the `use` after the item, but that would be somewhat weird I think.
  • Loading branch information
bors committed Sep 21, 2017
2 parents 1b55d19 + f0e7a5b commit 17f56c5
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 30 deletions.
54 changes: 37 additions & 17 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ impl<'tcx> Visitor<'tcx> for UsePlacementFinder {
ItemKind::Use(..) => {
// don't suggest placing a use before the prelude
// import or other generated ones
if item.span == DUMMY_SP {
if item.span.ctxt().outer().expn_info().is_none() {
self.span = Some(item.span.with_hi(item.span.lo()));
self.found_use = true;
return;
Expand All @@ -615,11 +615,22 @@ impl<'tcx> Visitor<'tcx> for UsePlacementFinder {
ItemKind::ExternCrate(_) => {}
// but place them before the first other item
_ => if self.span.map_or(true, |span| item.span < span ) {
self.span = Some(item.span.with_hi(item.span.lo()));
if item.span.ctxt().outer().expn_info().is_none() {
// don't insert between attributes and an item
if item.attrs.is_empty() {
self.span = Some(item.span.with_hi(item.span.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.with_hi(attr.span.lo()));
}
}
}
}
},
}
}
assert!(self.span.is_some(), "a file can't have no items and emit suggestions");
}
}

Expand Down Expand Up @@ -3553,8 +3564,7 @@ impl<'a> Resolver<'a> {
};
visit::walk_crate(&mut finder, krate);
if !candidates.is_empty() {
let span = finder.span.expect("did not find module");
show_candidates(&mut err, span, &candidates, better, finder.found_use);
show_candidates(&mut err, finder.span, &candidates, better, finder.found_use);
}
err.emit();
}
Expand Down Expand Up @@ -3748,7 +3758,8 @@ fn import_candidate_to_paths(suggestion: &ImportSuggestion) -> (Span, String, St
/// entities with that name in all crates. This method allows outputting the
/// results of this search in a programmer-friendly way
fn show_candidates(err: &mut DiagnosticBuilder,
span: Span,
// This is `None` if all placement locations are inside expansions
span: Option<Span>,
candidates: &[ImportSuggestion],
better: bool,
found_use: bool) {
Expand All @@ -3766,18 +3777,27 @@ fn show_candidates(err: &mut DiagnosticBuilder,
};
let msg = format!("possible {}candidate{} into scope", better, msg_diff);

for candidate in &mut path_strings {
// produce an additional newline to separate the new use statement
// from the directly following item.
let additional_newline = if found_use {
""
} else {
"\n"
};
*candidate = format!("use {};\n{}", candidate, additional_newline);
}
if let Some(span) = span {
for candidate in &mut path_strings {
// produce an additional newline to separate the new use statement
// from the directly following item.
let additional_newline = if found_use {
""
} else {
"\n"
};
*candidate = format!("use {};\n{}", candidate, additional_newline);
}

err.span_suggestions(span, &msg, path_strings);
err.span_suggestions(span, &msg, path_strings);
} else {
let mut msg = msg;
msg.push(':');
for candidate in path_strings {
msg.push('\n');
msg.push_str(&candidate);
}
}
}

/// A somewhat inefficient routine to obtain the name of a module.
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/resolve/privacy-struct-ctor.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ error[E0423]: expected value, found struct `Z`
|
help: possible better candidate is found in another module, you can import it into scope
|
16 | use m::n::Z;
22 | use m::n::Z;
|

error[E0423]: expected value, found struct `S`
Expand All @@ -24,7 +24,7 @@ error[E0423]: expected value, found struct `S`
|
help: possible better candidate is found in another module, you can import it into scope
|
15 | use m::S;
32 | use m::S;
|

error[E0423]: expected value, found struct `xcrate::S`
Expand All @@ -38,7 +38,7 @@ error[E0423]: expected value, found struct `xcrate::S`
|
help: possible better candidate is found in another module, you can import it into scope
|
15 | use m::S;
32 | use m::S;
|

error[E0603]: tuple struct `Z` is private
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/resolve/use_suggestion_placement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ mod m {
pub const A: i32 = 0;
}

mod foo {
#[derive(Debug)]
pub struct Foo;

// test whether the use suggestion isn't
// placed into the expansion of `#[derive(Debug)]
type Bar = Path;
}

fn main() {
y!();
let _ = A;
Expand Down
29 changes: 20 additions & 9 deletions src/test/ui/resolve/use_suggestion_placement.stderr
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
error[E0412]: cannot find type `Path` in this scope
--> $DIR/use_suggestion_placement.rs:25:16
|
25 | type Bar = Path;
| ^^^^ not found in this scope
|
help: possible candidate is found in another module, you can import it into scope
|
21 | use std::path::Path;
|

error[E0425]: cannot find value `A` in this scope
--> $DIR/use_suggestion_placement.rs:21:13
--> $DIR/use_suggestion_placement.rs:30:13
|
21 | let _ = A;
30 | let _ = A;
| ^ not found in this scope
|
help: possible candidate is found in another module, you can import it into scope
Expand All @@ -10,9 +21,9 @@ help: possible candidate is found in another module, you can import it into scop
|

error[E0412]: cannot find type `HashMap` in this scope
--> $DIR/use_suggestion_placement.rs:26:23
--> $DIR/use_suggestion_placement.rs:35:23
|
26 | type Dict<K, V> = HashMap<K, V>;
35 | type Dict<K, V> = HashMap<K, V>;
| ^^^^^^^ not found in this scope
|
help: possible candidates are found in other modules, you can import them into scope
Expand All @@ -23,16 +34,16 @@ help: possible candidates are found in other modules, you can import them into s
|

error[E0091]: type parameter `K` is unused
--> $DIR/use_suggestion_placement.rs:26:15
--> $DIR/use_suggestion_placement.rs:35:15
|
26 | type Dict<K, V> = HashMap<K, V>;
35 | type Dict<K, V> = HashMap<K, V>;
| ^ unused type parameter

error[E0091]: type parameter `V` is unused
--> $DIR/use_suggestion_placement.rs:26:18
--> $DIR/use_suggestion_placement.rs:35:18
|
26 | type Dict<K, V> = HashMap<K, V>;
35 | type Dict<K, V> = HashMap<K, V>;
| ^ unused type parameter

error: aborting due to 4 previous errors
error: aborting due to 5 previous errors

2 changes: 1 addition & 1 deletion src/test/ui/span/issue-35987.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ error[E0404]: expected trait, found type parameter `Add`
|
help: possible better candidate is found in another module, you can import it into scope
|
11 | use std::ops::Add;
13 | use std::ops::Add;
|

error[E0601]: main function not found
Expand Down

0 comments on commit 17f56c5

Please sign in to comment.