From 4606168dd508007fb1014b6ab12b27e320e07038 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Sat, 13 Jun 2020 11:12:29 -0700
Subject: [PATCH 1/3] Make new type param suggestion more targetted
Do not suggest new type param when encountering a missing type in an ADT
field with generic parameters.
Fix #72640.
---
src/librustc_resolve/build_reduced_graph.rs | 1 +
src/librustc_resolve/late/diagnostics.rs | 2 +-
src/librustc_resolve/lib.rs | 14 ++++++++------
.../ui/suggestions/type-not-found-in-adt-field.rs | 5 +++++
.../suggestions/type-not-found-in-adt-field.stderr | 9 +++++++++
5 files changed, 24 insertions(+), 7 deletions(-)
create mode 100644 src/test/ui/suggestions/type-not-found-in-adt-field.rs
create mode 100644 src/test/ui/suggestions/type-not-found-in-adt-field.stderr
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 2ae063660e38d..8661af6d7a1de 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -485,6 +485,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
module_path.push(Segment {
ident: Ident { name: kw::PathRoot, span: source.ident.span },
id: Some(self.r.next_node_id()),
+ has_args: false,
});
source.ident.name = crate_name;
}
diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs
index b1a1f8725a180..28ff89f66925e 100644
--- a/src/librustc_resolve/late/diagnostics.rs
+++ b/src/librustc_resolve/late/diagnostics.rs
@@ -920,7 +920,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
path: &[Segment],
) -> Option<(Span, &'static str, String, Applicability)> {
let ident = match path {
- [segment] => segment.ident,
+ [segment] if !segment.has_args => segment.ident,
_ => return None,
};
match (
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 6bd73877fab75..f7ec919fa04e3 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -225,13 +225,15 @@ enum VisResolutionError<'a> {
ModuleOnly(Span),
}
-// A minimal representation of a path segment. We use this in resolve because
-// we synthesize 'path segments' which don't have the rest of an AST or HIR
-// `PathSegment`.
+/// A minimal representation of a path segment. We use this in resolve because we synthesize 'path
+/// segments' which don't have the rest of an AST or HIR `PathSegment`.
#[derive(Clone, Copy, Debug)]
pub struct Segment {
ident: Ident,
id: Option<NodeId>,
+ /// Signals whether this `PathSegment` has generic arguments. Used to avoid providing
+ /// nonsensical suggestions.
+ has_args: bool,
}
impl Segment {
@@ -240,7 +242,7 @@ impl Segment {
}
fn from_ident(ident: Ident) -> Segment {
- Segment { ident, id: None }
+ Segment { ident, id: None, has_args: false }
}
fn names_to_string(segments: &[Segment]) -> String {
@@ -250,7 +252,7 @@ impl Segment {
impl<'a> From<&'a ast::PathSegment> for Segment {
fn from(seg: &'a ast::PathSegment) -> Segment {
- Segment { ident: seg.ident, id: Some(seg.id) }
+ Segment { ident: seg.ident, id: Some(seg.id), has_args: seg.args.is_some() }
}
}
@@ -2017,7 +2019,7 @@ impl<'a> Resolver<'a> {
path, opt_ns, record_used, path_span, crate_lint,
);
- for (i, &Segment { ident, id }) in path.iter().enumerate() {
+ for (i, &Segment { ident, id, has_args: _ }) in path.iter().enumerate() {
debug!("resolve_path ident {} {:?} {:?}", i, ident, id);
let record_segment_res = |this: &mut Self, res| {
if record_used {
diff --git a/src/test/ui/suggestions/type-not-found-in-adt-field.rs b/src/test/ui/suggestions/type-not-found-in-adt-field.rs
new file mode 100644
index 0000000000000..6bd42472f5a55
--- /dev/null
+++ b/src/test/ui/suggestions/type-not-found-in-adt-field.rs
@@ -0,0 +1,5 @@
+struct S {
+ m: Vec<Hashmap<String, ()>>, //~ ERROR cannot find type `Hashmap` in this scope
+ //~^ NOTE not found in this scope
+}
+fn main() {}
diff --git a/src/test/ui/suggestions/type-not-found-in-adt-field.stderr b/src/test/ui/suggestions/type-not-found-in-adt-field.stderr
new file mode 100644
index 0000000000000..cfad8c689d038
--- /dev/null
+++ b/src/test/ui/suggestions/type-not-found-in-adt-field.stderr
@@ -0,0 +1,9 @@
+error[E0412]: cannot find type `Hashmap` in this scope
+ --> $DIR/type-not-found-in-adt-field.rs:2:12
+ |
+LL | m: Vec<Hashmap<String, ()>>,
+ | ^^^^^^^ not found in this scope
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0412`.
From af45d8a5bb4909e6908c4d7bba80eedcdcd7f961 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Sat, 13 Jun 2020 12:08:32 -0700
Subject: [PATCH 2/3] Suggest new type param on single char ident
Suggest new type parameter on single char uppercase ident even if it
doesn't appear in a field's type parameter.
Address comment in #72641.
---
src/librustc_resolve/late/diagnostics.rs | 41 +++++++++++++++----
.../associated-types-eq-1.stderr | 11 ++++-
.../type-not-found-in-adt-field.rs | 8 +++-
.../type-not-found-in-adt-field.stderr | 16 ++++++--
4 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs
index 28ff89f66925e..694e2ac2a36a9 100644
--- a/src/librustc_resolve/late/diagnostics.rs
+++ b/src/librustc_resolve/late/diagnostics.rs
@@ -919,20 +919,45 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
&self,
path: &[Segment],
) -> Option<(Span, &'static str, String, Applicability)> {
- let ident = match path {
- [segment] if !segment.has_args => segment.ident,
+ let (ident, span) = match path {
+ [segment] if !segment.has_args => (segment.ident.to_string(), segment.ident.span),
_ => return None,
};
- match (
- self.diagnostic_metadata.current_item,
- self.diagnostic_metadata.currently_processing_generics,
- ) {
- (Some(Item { kind: ItemKind::Fn(..), ident, .. }), true) if ident.name == sym::main => {
+ let mut iter = ident.chars().map(|c| c.is_uppercase());
+ let single_uppercase_char =
+ matches!(iter.next(), Some(true)) && matches!(iter.next(), None);
+ if !self.diagnostic_metadata.currently_processing_generics && !single_uppercase_char {
+ return None;
+ }
+ match (self.diagnostic_metadata.current_item, single_uppercase_char) {
+ (Some(Item { kind: ItemKind::Fn(..), ident, .. }), _) if ident.name == sym::main => {
// Ignore `fn main()` as we don't want to suggest `fn main<T>()`
}
- (Some(Item { kind, .. }), true) => {
+ (
+ Some(Item {
+ kind:
+ kind @ ItemKind::Fn(..)
+ | kind @ ItemKind::Enum(..)
+ | kind @ ItemKind::Struct(..)
+ | kind @ ItemKind::Union(..),
+ ..
+ }),
+ true,
+ )
+ | (Some(Item { kind, .. }), false) => {
// Likely missing type parameter.
if let Some(generics) = kind.generics() {
+ if span.overlaps(generics.span) {
+ // Avoid the following:
+ // error[E0405]: cannot find trait `A` in this scope
+ // --> $DIR/typo-suggestion-named-underscore.rs:CC:LL
+ // |
+ // L | fn foo<T: A>(x: T) {} // Shouldn't suggest underscore
+ // | ^- help: you might be missing a type parameter: `, A`
+ // | |
+ // | not found in this scope
+ return None;
+ }
let msg = "you might be missing a type parameter";
let (span, sugg) = if let [.., param] = &generics.params[..] {
let span = if let [.., bound] = ¶m.bounds[..] {
diff --git a/src/test/ui/associated-types/associated-types-eq-1.stderr b/src/test/ui/associated-types/associated-types-eq-1.stderr
index 66c5f34644c01..53a45cf4e4f4d 100644
--- a/src/test/ui/associated-types/associated-types-eq-1.stderr
+++ b/src/test/ui/associated-types/associated-types-eq-1.stderr
@@ -4,7 +4,16 @@ error[E0412]: cannot find type `A` in this scope
LL | fn foo2<I: Foo>(x: I) {
| - similarly named type parameter `I` defined here
LL | let _: A = x.boo();
- | ^ help: a type parameter with a similar name exists: `I`
+ | ^
+ |
+help: a type parameter with a similar name exists
+ |
+LL | let _: I = x.boo();
+ | ^
+help: you might be missing a type parameter
+ |
+LL | fn foo2<I: Foo, A>(x: I) {
+ | ^^^
error: aborting due to previous error
diff --git a/src/test/ui/suggestions/type-not-found-in-adt-field.rs b/src/test/ui/suggestions/type-not-found-in-adt-field.rs
index 6bd42472f5a55..4cbfe58d35703 100644
--- a/src/test/ui/suggestions/type-not-found-in-adt-field.rs
+++ b/src/test/ui/suggestions/type-not-found-in-adt-field.rs
@@ -1,5 +1,9 @@
-struct S {
- m: Vec<Hashmap<String, ()>>, //~ ERROR cannot find type `Hashmap` in this scope
+struct Struct {
+ m: Vec<Someunknownname<String, ()>>, //~ ERROR cannot find type `Someunknownname` in this scope
+ //~^ NOTE not found in this scope
+}
+struct OtherStruct { //~ HELP you might be missing a type parameter
+ m: K, //~ ERROR cannot find type `K` in this scope
//~^ NOTE not found in this scope
}
fn main() {}
diff --git a/src/test/ui/suggestions/type-not-found-in-adt-field.stderr b/src/test/ui/suggestions/type-not-found-in-adt-field.stderr
index cfad8c689d038..e990fb5ba1210 100644
--- a/src/test/ui/suggestions/type-not-found-in-adt-field.stderr
+++ b/src/test/ui/suggestions/type-not-found-in-adt-field.stderr
@@ -1,9 +1,17 @@
-error[E0412]: cannot find type `Hashmap` in this scope
+error[E0412]: cannot find type `Someunknownname` in this scope
--> $DIR/type-not-found-in-adt-field.rs:2:12
|
-LL | m: Vec<Hashmap<String, ()>>,
- | ^^^^^^^ not found in this scope
+LL | m: Vec<Someunknownname<String, ()>>,
+ | ^^^^^^^^^^^^^^^ not found in this scope
-error: aborting due to previous error
+error[E0412]: cannot find type `K` in this scope
+ --> $DIR/type-not-found-in-adt-field.rs:6:8
+ |
+LL | struct OtherStruct {
+ | - help: you might be missing a type parameter: `<K>`
+LL | m: K,
+ | ^ not found in this scope
+
+error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0412`.
From 8d1a3801faa199acb2a86580247afd672bd838f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Wed, 17 Jun 2020 16:29:03 -0700
Subject: [PATCH 3/3] review comments
---
src/librustc_resolve/build_reduced_graph.rs | 2 +-
src/librustc_resolve/late/diagnostics.rs | 4 +++-
src/librustc_resolve/lib.rs | 8 ++++----
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 8661af6d7a1de..ef2389fcefafc 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -485,7 +485,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
module_path.push(Segment {
ident: Ident { name: kw::PathRoot, span: source.ident.span },
id: Some(self.r.next_node_id()),
- has_args: false,
+ has_generic_args: false,
});
source.ident.name = crate_name;
}
diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs
index 694e2ac2a36a9..a6e016e9e6a22 100644
--- a/src/librustc_resolve/late/diagnostics.rs
+++ b/src/librustc_resolve/late/diagnostics.rs
@@ -920,7 +920,9 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
path: &[Segment],
) -> Option<(Span, &'static str, String, Applicability)> {
let (ident, span) = match path {
- [segment] if !segment.has_args => (segment.ident.to_string(), segment.ident.span),
+ [segment] if !segment.has_generic_args => {
+ (segment.ident.to_string(), segment.ident.span)
+ }
_ => return None,
};
let mut iter = ident.chars().map(|c| c.is_uppercase());
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index f7ec919fa04e3..a30e5cc1ab601 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -233,7 +233,7 @@ pub struct Segment {
id: Option<NodeId>,
/// Signals whether this `PathSegment` has generic arguments. Used to avoid providing
/// nonsensical suggestions.
- has_args: bool,
+ has_generic_args: bool,
}
impl Segment {
@@ -242,7 +242,7 @@ impl Segment {
}
fn from_ident(ident: Ident) -> Segment {
- Segment { ident, id: None, has_args: false }
+ Segment { ident, id: None, has_generic_args: false }
}
fn names_to_string(segments: &[Segment]) -> String {
@@ -252,7 +252,7 @@ impl Segment {
impl<'a> From<&'a ast::PathSegment> for Segment {
fn from(seg: &'a ast::PathSegment) -> Segment {
- Segment { ident: seg.ident, id: Some(seg.id), has_args: seg.args.is_some() }
+ Segment { ident: seg.ident, id: Some(seg.id), has_generic_args: seg.args.is_some() }
}
}
@@ -2019,7 +2019,7 @@ impl<'a> Resolver<'a> {
path, opt_ns, record_used, path_span, crate_lint,
);
- for (i, &Segment { ident, id, has_args: _ }) in path.iter().enumerate() {
+ for (i, &Segment { ident, id, has_generic_args: _ }) in path.iter().enumerate() {
debug!("resolve_path ident {} {:?} {:?}", i, ident, id);
let record_segment_res = |this: &mut Self, res| {
if record_used {