From 21c580e4c4c9e6f2772fddf8d5ad7824696f7fc8 Mon Sep 17 00:00:00 2001 From: Maxim Zholobak Date: Sat, 30 Sep 2017 11:27:02 +0300 Subject: [PATCH 1/4] Add hints for the case of confusing enum with its variants --- src/librustc_resolve/lib.rs | 79 +++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 8207057fd0a..502dcd1b7e8 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2599,6 +2599,21 @@ impl<'a> Resolver<'a> { } _ => {} }, + (Def::Enum(..), PathSource::TupleStruct) => { + if let Some(variants) = this.collect_enum_variants(def) { + err.note(&format!("did you mean to use one \ + of the following variants?\n{}", + variants.iter() + .map(|suggestion| format!("- `{}`", + path_names_to_string(suggestion))) + .collect::>() + .join("\n"))); + + } else { + err.note("did you mean to use one of the enum's variants?"); + } + return (err, candidates); + }, _ if ns == ValueNS && is_struct_like(def) => { if let Def::Struct(def_id) = def { if let Some((ctor_def, ctor_vis)) @@ -3495,6 +3510,70 @@ impl<'a> Resolver<'a> { candidates } + fn find_module(&mut self, + module_def: Def) + -> Option<(Module<'a>, ImportSuggestion)> + { + let mut result = None; + let mut worklist = Vec::new(); + let mut seen_modules = FxHashSet(); + worklist.push((self.graph_root, Vec::new())); + + while let Some((in_module, path_segments)) = worklist.pop() { + // abort if the module is already found + if let Some(_) = result { break; } + + self.populate_module_if_necessary(in_module); + + in_module.for_each_child(|ident, _, name_binding| { + // abort if the module is already found + if let Some(_) = result { + return (); + } + if let Some(module) = name_binding.module() { + // form the path + let mut path_segments = path_segments.clone(); + path_segments.push(ast::PathSegment::from_ident(ident, name_binding.span)); + if module.def() == Some(module_def) { + let path = Path { + span: name_binding.span, + segments: path_segments, + }; + result = Some((module, ImportSuggestion { path: path })); + } else { + // add the module to the lookup + if seen_modules.insert(module.def_id().unwrap()) { + worklist.push((module, path_segments)); + } + } + } + }); + } + + result + } + + fn collect_enum_variants(&mut self, enum_def: Def) -> Option> { + if let Def::Enum(..) = enum_def {} else { + panic!("Non-enum def passed to collect_enum_variants: {:?}", enum_def) + } + + self.find_module(enum_def).map(|(enum_module, enum_import_suggestion)| { + let mut variants = Vec::new(); + enum_module.for_each_child_stable(|ident, _, name_binding| { + if let Def::Variant(..) = name_binding.def() { + let mut segms = enum_import_suggestion.path.segments.clone(); + segms.push(ast::PathSegment::from_ident(ident, name_binding.span)); + variants.push(Path { + span: name_binding.span, + segments: segms, + }); + } + }); + variants + }) + } + fn record_def(&mut self, node_id: NodeId, resolution: PathResolution) { debug!("(recording def) recording {:?} for {}", resolution, node_id); if let Some(prev_res) = self.def_map.insert(node_id, resolution) { From 08c81c1a797322c68479263f5600eed8c6c2aabe Mon Sep 17 00:00:00 2001 From: Maxim Zholobak Date: Wed, 15 Nov 2017 09:20:09 +0200 Subject: [PATCH 2/4] Add failing testcases --- .../issue-43871-enum-instead-of-variant.rs | 19 +++++++++++++++++ ...issue-43871-enum-instead-of-variant.stderr | 21 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.rs create mode 100644 src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr diff --git a/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.rs b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.rs new file mode 100644 index 00000000000..22c5e04419a --- /dev/null +++ b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.rs @@ -0,0 +1,19 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn result_test() { + let x = Option(1); + + if let Option(_) = x { + println!("It is OK."); + } +} + +fn main() {} diff --git a/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr new file mode 100644 index 00000000000..4882cb10df6 --- /dev/null +++ b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr @@ -0,0 +1,21 @@ +error[E0423]: expected function, found enum `Option` + --> $DIR/issue-43871-enum-instead-of-variant.rs:12:13 + | +12 | let x = Option(1); + | ^^^^^^ not a function + = note: did you mean to use one of the following variants? + - `std::prelude::v1::Option::None` + - `std::prelude::v1::Option::Some` + +error[E0532]: expected tuple struct/variant, found enum `Option` + --> $DIR/issue-43871-enum-instead-of-variant.rs:14:12 + | +14 | if let Option(_) = x { + | ^^^^^^ + | + = note: did you mean to use one of the following variants? + - `std::prelude::v1::Option::None` + - `std::prelude::v1::Option::Some` + +error: aborting due to previous error + From 39f848efb010447621278f90a5241c8657fdccc8 Mon Sep 17 00:00:00 2001 From: Maxim Zholobak Date: Thu, 23 Nov 2017 15:10:23 +0200 Subject: [PATCH 3/4] Add module population and case of enum in place of expression --- src/librustc_resolve/lib.rs | 9 +++++--- .../issue-43871-enum-instead-of-variant.rs | 8 +++++++ ...issue-43871-enum-instead-of-variant.stderr | 23 ++++++++++++++----- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 502dcd1b7e8..fb9ba9a09bb 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2599,13 +2599,14 @@ impl<'a> Resolver<'a> { } _ => {} }, - (Def::Enum(..), PathSource::TupleStruct) => { + (Def::Enum(..), PathSource::TupleStruct) + | (Def::Enum(..), PathSource::Expr(..)) => { if let Some(variants) = this.collect_enum_variants(def) { err.note(&format!("did you mean to use one \ of the following variants?\n{}", variants.iter() - .map(|suggestion| format!("- `{}`", - path_names_to_string(suggestion))) + .map(|suggestion| path_names_to_string(suggestion)) + .map(|suggestion| format!("- `{}`", suggestion)) .collect::>() .join("\n"))); @@ -3559,6 +3560,8 @@ impl<'a> Resolver<'a> { } self.find_module(enum_def).map(|(enum_module, enum_import_suggestion)| { + self.populate_module_if_necessary(enum_module); + let mut variants = Vec::new(); enum_module.for_each_child_stable(|ident, _, name_binding| { if let Def::Variant(..) = name_binding.def() { diff --git a/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.rs b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.rs index 22c5e04419a..923b0984d06 100644 --- a/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.rs +++ b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.rs @@ -8,12 +8,20 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +enum Example { Ex(String), NotEx } + fn result_test() { let x = Option(1); if let Option(_) = x { println!("It is OK."); } + + let y = Example::Ex(String::from("test")); + + if let Example(_) = y { + println!("It is OK."); + } } fn main() {} diff --git a/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr index 4882cb10df6..8371413c5a2 100644 --- a/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr +++ b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr @@ -1,21 +1,32 @@ error[E0423]: expected function, found enum `Option` - --> $DIR/issue-43871-enum-instead-of-variant.rs:12:13 + --> $DIR/issue-43871-enum-instead-of-variant.rs:14:13 + | +14 | let x = Option(1); + | ^^^^^^ | -12 | let x = Option(1); - | ^^^^^^ not a function = note: did you mean to use one of the following variants? - `std::prelude::v1::Option::None` - `std::prelude::v1::Option::Some` error[E0532]: expected tuple struct/variant, found enum `Option` - --> $DIR/issue-43871-enum-instead-of-variant.rs:14:12 + --> $DIR/issue-43871-enum-instead-of-variant.rs:16:12 | -14 | if let Option(_) = x { +16 | if let Option(_) = x { | ^^^^^^ | = note: did you mean to use one of the following variants? - `std::prelude::v1::Option::None` - `std::prelude::v1::Option::Some` -error: aborting due to previous error +error[E0532]: expected tuple struct/variant, found enum `Example` + --> $DIR/issue-43871-enum-instead-of-variant.rs:22:12 + | +22 | if let Example(_) = y { + | ^^^^^^^ + | + = note: did you mean to use one of the following variants? + - `Example::Ex` + - `Example::NotEx` + +error: aborting due to 3 previous errors From a3686c685a10de7b56cacf998a9e7eabc622787b Mon Sep 17 00:00:00 2001 From: Maxim Zholobak Date: Thu, 23 Nov 2017 21:09:27 +0200 Subject: [PATCH 4/4] Use for_each_child_stable in find_module --- src/librustc_resolve/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index fb9ba9a09bb..ec0c0e04a91 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3526,7 +3526,7 @@ impl<'a> Resolver<'a> { self.populate_module_if_necessary(in_module); - in_module.for_each_child(|ident, _, name_binding| { + in_module.for_each_child_stable(|ident, _, name_binding| { // abort if the module is already found if let Some(_) = result { return ();