]> git.lizzy.rs Git - rust.git/commitdiff
Rollup merge of #88838 - FabianWolff:issue-88472, r=estebank
authorManish Goregaokar <manishsmail@gmail.com>
Fri, 1 Oct 2021 06:41:05 +0000 (23:41 -0700)
committerGitHub <noreply@github.com>
Fri, 1 Oct 2021 06:41:05 +0000 (23:41 -0700)
Do not suggest importing inaccessible items

Fixes #88472. For this example:
```rust
mod a {
    struct Foo;
}

mod b {
    type Bar = Foo;
}
```
rustc currently emits:
```
error[E0412]: cannot find type `Foo` in this scope
 --> test.rs:6:16
  |
6 |     type Bar = Foo;
  |                ^^^ not found in this scope
  |
help: consider importing this struct
  |
6 |     use a::Foo;
  |
```
this is incorrect, as applying this suggestion leads to
```
error[E0603]: struct `Foo` is private
 --> test.rs:6:12
  |
6 |     use a::Foo;
  |            ^^^ private struct
  |
note: the struct `Foo` is defined here
 --> test.rs:2:5
  |
2 |     struct Foo;
  |     ^^^^^^^^^^^
```
With my changes, I get:
```
error[E0412]: cannot find type `Foo` in this scope
 --> test.rs:6:16
  |
6 |     type Bar = Foo;
  |                ^^^ not found in this scope
  |
  = note: this struct exists but is inaccessible:
          a::Foo
```
As for the wildcard mentioned in #88472, I would argue that the warning is actually correct, since the import _is_ unused. I think the real issue is the wrong suggestion, which I have fixed here.

compiler/rustc_resolve/src/diagnostics.rs
compiler/rustc_resolve/src/lib.rs
src/test/ui/hygiene/globs.stderr
src/test/ui/imports/glob-resolve1.stderr
src/test/ui/imports/issue-4366-2.stderr
src/test/ui/resolve/issue-42944.stderr
src/test/ui/resolve/issue-88472.rs [new file with mode: 0644]
src/test/ui/resolve/issue-88472.stderr [new file with mode: 0644]
src/test/ui/resolve/privacy-enum-ctor.stderr
src/test/ui/resolve/privacy-struct-ctor.stderr
src/test/ui/self/self_type_keyword.stderr

index 5acbe9864beb05343381f64f1b36b5905fb1416f..ab1f47c81db975129f9827786f8d0da7e9c05c62 100644 (file)
@@ -973,7 +973,15 @@ fn lookup_import_candidates_from_module<FilterFn>(
 
         let import_suggestions =
             self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, is_expected);
-        show_candidates(err, None, &import_suggestions, false, true);
+        show_candidates(
+            &self.definitions,
+            self.session,
+            err,
+            None,
+            &import_suggestions,
+            false,
+            true,
+        );
 
         if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) {
             let msg = format!("unsafe traits like `{}` should be implemented explicitly", ident);
@@ -1713,6 +1721,8 @@ fn find_span_immediately_after_crate_name(
 /// entities with that name in all crates. This method allows outputting the
 /// results of this search in a programmer-friendly way
 crate fn show_candidates(
+    definitions: &rustc_hir::definitions::Definitions,
+    session: &Session,
     err: &mut DiagnosticBuilder<'_>,
     // This is `None` if all placement locations are inside expansions
     use_placement_span: Option<Span>,
@@ -1724,43 +1734,111 @@ fn find_span_immediately_after_crate_name(
         return;
     }
 
+    let mut accessible_path_strings: Vec<(String, &str, Option<DefId>)> = Vec::new();
+    let mut inaccessible_path_strings: Vec<(String, &str, Option<DefId>)> = Vec::new();
+
+    candidates.iter().for_each(|c| {
+        (if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings })
+            .push((path_names_to_string(&c.path), c.descr, c.did))
+    });
+
     // we want consistent results across executions, but candidates are produced
     // by iterating through a hash map, so make sure they are ordered:
-    let mut path_strings: Vec<_> =
-        candidates.iter().map(|c| path_names_to_string(&c.path)).collect();
+    for path_strings in [&mut accessible_path_strings, &mut inaccessible_path_strings] {
+        path_strings.sort_by(|a, b| a.0.cmp(&b.0));
+        let core_path_strings =
+            path_strings.drain_filter(|p| p.0.starts_with("core::")).collect::<Vec<_>>();
+        path_strings.extend(core_path_strings);
+        path_strings.dedup_by(|a, b| a.0 == b.0);
+    }
+
+    if !accessible_path_strings.is_empty() {
+        let (determiner, kind) = if accessible_path_strings.len() == 1 {
+            ("this", accessible_path_strings[0].1)
+        } else {
+            ("one of these", "items")
+        };
 
-    path_strings.sort();
-    let core_path_strings =
-        path_strings.drain_filter(|p| p.starts_with("core::")).collect::<Vec<String>>();
-    path_strings.extend(core_path_strings);
-    path_strings.dedup();
+        let instead = if instead { " instead" } else { "" };
+        let mut msg = format!("consider importing {} {}{}", determiner, kind, instead);
 
-    let (determiner, kind) = if candidates.len() == 1 {
-        ("this", candidates[0].descr)
-    } else {
-        ("one of these", "items")
-    };
-
-    let instead = if instead { " instead" } else { "" };
-    let mut msg = format!("consider importing {} {}{}", determiner, kind, instead);
-
-    if let Some(span) = use_placement_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);
-        }
+        if let Some(span) = use_placement_span {
+            for candidate in &mut accessible_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.0 = format!("use {};\n{}", &candidate.0, additional_newline);
+            }
 
-        err.span_suggestions(span, &msg, path_strings.into_iter(), Applicability::Unspecified);
-    } else {
-        msg.push(':');
+            err.span_suggestions(
+                span,
+                &msg,
+                accessible_path_strings.into_iter().map(|a| a.0),
+                Applicability::Unspecified,
+            );
+        } else {
+            msg.push(':');
 
-        for candidate in path_strings {
-            msg.push('\n');
-            msg.push_str(&candidate);
+            for candidate in accessible_path_strings {
+                msg.push('\n');
+                msg.push_str(&candidate.0);
+            }
+
+            err.note(&msg);
         }
+    } else {
+        assert!(!inaccessible_path_strings.is_empty());
+
+        if inaccessible_path_strings.len() == 1 {
+            let (name, descr, def_id) = &inaccessible_path_strings[0];
+            let msg = format!("{} `{}` exists but is inaccessible", descr, name);
+
+            if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
+                let span = definitions.def_span(local_def_id);
+                let span = session.source_map().guess_head_span(span);
+                let mut multi_span = MultiSpan::from_span(span);
+                multi_span.push_span_label(span, "not accessible".to_string());
+                err.span_note(multi_span, &msg);
+            } else {
+                err.note(&msg);
+            }
+        } else {
+            let (_, descr_first, _) = &inaccessible_path_strings[0];
+            let descr = if inaccessible_path_strings
+                .iter()
+                .skip(1)
+                .all(|(_, descr, _)| descr == descr_first)
+            {
+                format!("{}", descr_first)
+            } else {
+                "item".to_string()
+            };
+
+            let mut msg = format!("these {}s exist but are inaccessible", descr);
+            let mut has_colon = false;
 
-        err.note(&msg);
+            let mut spans = Vec::new();
+            for (name, _, def_id) in &inaccessible_path_strings {
+                if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
+                    let span = definitions.def_span(local_def_id);
+                    let span = session.source_map().guess_head_span(span);
+                    spans.push((name, span));
+                } else {
+                    if !has_colon {
+                        msg.push(':');
+                        has_colon = true;
+                    }
+                    msg.push('\n');
+                    msg.push_str(name);
+                }
+            }
+
+            let mut multi_span = MultiSpan::from_spans(spans.iter().map(|(_, sp)| *sp).collect());
+            for (name, span) in spans {
+                multi_span.push_span_label(span, format!("`{}`: not accessible", name));
+            }
+
+            err.span_note(multi_span, &msg);
+        }
     }
 }
index 19b9e1dc460d658b38a7deaae142cb7c32c2438a..ff7f465dbc6d7769cde1e56e29c4686085810878 100644 (file)
@@ -2969,7 +2969,15 @@ fn report_with_use_injections(&mut self, krate: &Crate) {
                 (None, false)
             };
             if !candidates.is_empty() {
-                diagnostics::show_candidates(&mut err, span, &candidates, instead, found_use);
+                diagnostics::show_candidates(
+                    &self.definitions,
+                    self.session,
+                    &mut err,
+                    span,
+                    &candidates,
+                    instead,
+                    found_use,
+                );
             } else if let Some((span, msg, sugg, appl)) = suggestion {
                 err.span_suggestion(span, msg, sugg, appl);
             }
index c2497f8ff74d2629ef0f0a00e6a2e384a5bfa5f5..6c8b707b8e2f14d7d6db5109bf6bc3e759c36640 100644 (file)
@@ -4,7 +4,7 @@ error[E0425]: cannot find function `f` in this scope
 LL |         f();
    |         ^ not found in this scope
    |
-help: consider importing one of these items
+help: consider importing this function
    |
 LL | use foo::f;
    |
@@ -37,7 +37,7 @@ LL | n!(f);
 LL |         n!(f);
    |            ^ not found in this scope
    |
-   = note: consider importing one of these items:
+   = note: consider importing this function:
            foo::f
    = note: this error originates in the macro `n` (in Nightly builds, run with -Z macro-backtrace for more info)
 
@@ -50,7 +50,7 @@ LL | n!(f);
 LL |                 f
    |                 ^ not found in this scope
    |
-   = note: consider importing one of these items:
+   = note: consider importing this function:
            foo::f
    = note: this error originates in the macro `n` (in Nightly builds, run with -Z macro-backtrace for more info)
 
index 2c7a8ad5c1adb12a77b5e7337d2dd790046c7939..3b66a5e3150502600768bc6a95f71873983ef991 100644 (file)
@@ -4,10 +4,11 @@ error[E0425]: cannot find function `fpriv` in this scope
 LL |     fpriv();
    |     ^^^^^ not found in this scope
    |
-help: consider importing this function
-   |
-LL | use bar::fpriv;
+note: function `bar::fpriv` exists but is inaccessible
+  --> $DIR/glob-resolve1.rs:7:5
    |
+LL |     fn fpriv() {}
+   |     ^^^^^^^^^^ not accessible
 
 error[E0425]: cannot find function `epriv` in this scope
   --> $DIR/glob-resolve1.rs:27:5
@@ -15,10 +16,11 @@ error[E0425]: cannot find function `epriv` in this scope
 LL |     epriv();
    |     ^^^^^ not found in this scope
    |
-help: consider importing this function
-   |
-LL | use bar::epriv;
+note: function `bar::epriv` exists but is inaccessible
+  --> $DIR/glob-resolve1.rs:9:9
    |
+LL |         fn epriv();
+   |         ^^^^^^^^^^^ not accessible
 
 error[E0423]: expected value, found enum `B`
   --> $DIR/glob-resolve1.rs:28:5
@@ -44,10 +46,11 @@ error[E0425]: cannot find value `C` in this scope
 LL |     C;
    |     ^ not found in this scope
    |
-help: consider importing this unit struct
-   |
-LL | use bar::C;
+note: unit struct `bar::C` exists but is inaccessible
+  --> $DIR/glob-resolve1.rs:18:5
    |
+LL |     struct C;
+   |     ^^^^^^^^^ not accessible
 
 error[E0425]: cannot find function `import` in this scope
   --> $DIR/glob-resolve1.rs:30:5
@@ -67,16 +70,13 @@ LL |     pub enum B {
    |     ---------- similarly named enum `B` defined here
 ...
 LL |     foo::<A>();
-   |           ^
-   |
-help: an enum with a similar name exists
+   |           ^ help: an enum with a similar name exists: `B`
    |
-LL |     foo::<B>();
-   |           ~
-help: consider importing this enum
-   |
-LL | use bar::A;
+note: enum `bar::A` exists but is inaccessible
+  --> $DIR/glob-resolve1.rs:11:5
    |
+LL |     enum A {
+   |     ^^^^^^ not accessible
 
 error[E0412]: cannot find type `C` in this scope
   --> $DIR/glob-resolve1.rs:33:11
@@ -85,16 +85,13 @@ LL |     pub enum B {
    |     ---------- similarly named enum `B` defined here
 ...
 LL |     foo::<C>();
-   |           ^
-   |
-help: an enum with a similar name exists
-   |
-LL |     foo::<B>();
-   |           ~
-help: consider importing this struct
+   |           ^ help: an enum with a similar name exists: `B`
    |
-LL | use bar::C;
+note: struct `bar::C` exists but is inaccessible
+  --> $DIR/glob-resolve1.rs:18:5
    |
+LL |     struct C;
+   |     ^^^^^^^^^ not accessible
 
 error[E0412]: cannot find type `D` in this scope
   --> $DIR/glob-resolve1.rs:34:11
@@ -103,16 +100,13 @@ LL |     pub enum B {
    |     ---------- similarly named enum `B` defined here
 ...
 LL |     foo::<D>();
-   |           ^
-   |
-help: an enum with a similar name exists
-   |
-LL |     foo::<B>();
-   |           ~
-help: consider importing this type alias
+   |           ^ help: an enum with a similar name exists: `B`
    |
-LL | use bar::D;
+note: type alias `bar::D` exists but is inaccessible
+  --> $DIR/glob-resolve1.rs:20:5
    |
+LL |     type D = isize;
+   |     ^^^^^^^^^^^^^^^ not accessible
 
 error: aborting due to 8 previous errors
 
index a86ec7fabea4bf37690c55ff9175e20eacb00782..4c94634ee60f7d459959efe76f13adcfbfe5042c 100644 (file)
@@ -4,10 +4,11 @@ error[E0412]: cannot find type `Bar` in this scope
 LL |         fn sub() -> Bar { 1 }
    |                     ^^^ not found in this scope
    |
-help: consider importing this type alias
-   |
-LL |         use a::b::Bar;
+note: type alias `a::b::Bar` exists but is inaccessible
+  --> $DIR/issue-4366-2.rs:11:9
    |
+LL |         type Bar = isize;
+   |         ^^^^^^^^^^^^^^^^^ not accessible
 
 error[E0423]: expected function, found module `foo`
   --> $DIR/issue-4366-2.rs:25:5
index 008492529d18c9de7d6ce66ba46542402b287955..cad3ccc4a0ef836883eaa23f32a77506ab70f843 100644 (file)
@@ -16,10 +16,11 @@ error[E0425]: cannot find function, tuple struct or tuple variant `Bx` in this s
 LL |         Bx(());
    |         ^^ not found in this scope
    |
-help: consider importing this tuple struct
-   |
-LL |     use foo::Bx;
+note: tuple struct `foo::Bx` exists but is inaccessible
+  --> $DIR/issue-42944.rs:2:5
    |
+LL |     pub struct Bx(());
+   |     ^^^^^^^^^^^^^^^^^^ not accessible
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/resolve/issue-88472.rs b/src/test/ui/resolve/issue-88472.rs
new file mode 100644 (file)
index 0000000..6bf7cae
--- /dev/null
@@ -0,0 +1,38 @@
+// Regression test for #88472, where a suggestion was issued to
+// import an inaccessible struct.
+
+#![warn(unused_imports)]
+//~^ NOTE: the lint level is defined here
+
+mod a {
+    struct Foo;
+    //~^ NOTE: struct `a::Foo` exists but is inaccessible
+    //~| NOTE: not accessible
+}
+
+mod b {
+    use crate::a::*;
+    //~^ WARNING: unused import
+    type Bar = Foo;
+    //~^ ERROR: cannot find type `Foo` in this scope [E0412]
+    //~| NOTE: not found in this scope
+}
+
+mod c {
+    enum Eee {}
+    //~^ NOTE: these enums exist but are inaccessible
+    //~| NOTE: `c::Eee`: not accessible
+
+    mod d {
+        enum Eee {}
+        //~^ NOTE: `c::d::Eee`: not accessible
+    }
+}
+
+mod e {
+    type Baz = Eee;
+    //~^ ERROR: cannot find type `Eee` in this scope [E0412]
+    //~| NOTE: not found in this scope
+}
+
+fn main() {}
diff --git a/src/test/ui/resolve/issue-88472.stderr b/src/test/ui/resolve/issue-88472.stderr
new file mode 100644 (file)
index 0000000..8431fc9
--- /dev/null
@@ -0,0 +1,42 @@
+error[E0412]: cannot find type `Foo` in this scope
+  --> $DIR/issue-88472.rs:16:16
+   |
+LL |     type Bar = Foo;
+   |                ^^^ not found in this scope
+   |
+note: struct `a::Foo` exists but is inaccessible
+  --> $DIR/issue-88472.rs:8:5
+   |
+LL |     struct Foo;
+   |     ^^^^^^^^^^^ not accessible
+
+error[E0412]: cannot find type `Eee` in this scope
+  --> $DIR/issue-88472.rs:33:16
+   |
+LL |     type Baz = Eee;
+   |                ^^^ not found in this scope
+   |
+note: these enums exist but are inaccessible
+  --> $DIR/issue-88472.rs:22:5
+   |
+LL |     enum Eee {}
+   |     ^^^^^^^^ `c::Eee`: not accessible
+...
+LL |         enum Eee {}
+   |         ^^^^^^^^ `c::d::Eee`: not accessible
+
+warning: unused import: `crate::a::*`
+  --> $DIR/issue-88472.rs:14:9
+   |
+LL |     use crate::a::*;
+   |         ^^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/issue-88472.rs:4:9
+   |
+LL | #![warn(unused_imports)]
+   |         ^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors; 1 warning emitted
+
+For more information about this error, try `rustc --explain E0412`.
index 192349e0fafe31ab6b55c52dd7e11abd11ce9224..ff72b0b563ab176fc361e0b13a117f173fe00f8f 100644 (file)
@@ -169,16 +169,13 @@ LL |     pub enum E {
    |     ---------- similarly named enum `E` defined here
 ...
 LL |     let _: Z = m::n::Z;
-   |            ^
+   |            ^ help: an enum with a similar name exists: `E`
    |
-help: an enum with a similar name exists
-   |
-LL |     let _: E = m::n::Z;
-   |            ~
-help: consider importing this enum
-   |
-LL | use m::Z;
+note: enum `m::Z` exists but is inaccessible
+  --> $DIR/privacy-enum-ctor.rs:11:9
    |
+LL |         pub(in m) enum Z {
+   |         ^^^^^^^^^^^^^^^^ not accessible
 
 error[E0423]: expected value, found enum `m::n::Z`
   --> $DIR/privacy-enum-ctor.rs:57:16
@@ -215,16 +212,13 @@ LL |     pub enum E {
    |     ---------- similarly named enum `E` defined here
 ...
 LL |     let _: Z = m::n::Z::Fn;
-   |            ^
+   |            ^ help: an enum with a similar name exists: `E`
    |
-help: an enum with a similar name exists
-   |
-LL |     let _: E = m::n::Z::Fn;
-   |            ~
-help: consider importing this enum
-   |
-LL | use m::Z;
+note: enum `m::Z` exists but is inaccessible
+  --> $DIR/privacy-enum-ctor.rs:11:9
    |
+LL |         pub(in m) enum Z {
+   |         ^^^^^^^^^^^^^^^^ not accessible
 
 error[E0412]: cannot find type `Z` in this scope
   --> $DIR/privacy-enum-ctor.rs:64:12
@@ -233,16 +227,13 @@ LL |     pub enum E {
    |     ---------- similarly named enum `E` defined here
 ...
 LL |     let _: Z = m::n::Z::Struct;
-   |            ^
+   |            ^ help: an enum with a similar name exists: `E`
    |
-help: an enum with a similar name exists
-   |
-LL |     let _: E = m::n::Z::Struct;
-   |            ~
-help: consider importing this enum
-   |
-LL | use m::Z;
+note: enum `m::Z` exists but is inaccessible
+  --> $DIR/privacy-enum-ctor.rs:11:9
    |
+LL |         pub(in m) enum Z {
+   |         ^^^^^^^^^^^^^^^^ not accessible
 
 error[E0423]: expected value, found struct variant `m::n::Z::Struct`
   --> $DIR/privacy-enum-ctor.rs:64:16
@@ -262,16 +253,13 @@ LL |     pub enum E {
    |     ---------- similarly named enum `E` defined here
 ...
 LL |     let _: Z = m::n::Z::Unit {};
-   |            ^
+   |            ^ help: an enum with a similar name exists: `E`
    |
-help: an enum with a similar name exists
-   |
-LL |     let _: E = m::n::Z::Unit {};
-   |            ~
-help: consider importing this enum
-   |
-LL | use m::Z;
+note: enum `m::Z` exists but is inaccessible
+  --> $DIR/privacy-enum-ctor.rs:11:9
    |
+LL |         pub(in m) enum Z {
+   |         ^^^^^^^^^^^^^^^^ not accessible
 
 error[E0603]: enum `Z` is private
   --> $DIR/privacy-enum-ctor.rs:57:22
index e5d6f7e9e24f0627091909bfa437fd67acf17a42..ada053014ef5eb38e064fcf7c2c7c54ab94d6d09 100644 (file)
@@ -33,10 +33,11 @@ error[E0423]: expected value, found struct `xcrate::S`
 LL |     xcrate::S;
    |     ^^^^^^^^^ constructor is not visible here due to private fields
    |
-help: consider importing this tuple struct instead
-   |
-LL | use m::S;
+note: tuple struct `m::S` exists but is inaccessible
+  --> $DIR/privacy-struct-ctor.rs:6:5
    |
+LL |     pub struct S(u8);
+   |     ^^^^^^^^^^^^^^^^^ not accessible
 
 error[E0603]: tuple struct constructor `Z` is private
   --> $DIR/privacy-struct-ctor.rs:18:12
index 47c04f1eb72ec08d1e6517706705dd95c1fc3304..aca08d81163fd7428a4b590705222f26b27fd500 100644 (file)
@@ -66,10 +66,11 @@ error[E0531]: cannot find unit struct, unit variant or constant `Self` in this s
 LL |         mut Self => (),
    |             ^^^^ not found in this scope
    |
-help: consider importing this unit struct
-   |
-LL | use foo::Self;
+note: unit struct `foo::Self` exists but is inaccessible
+  --> $DIR/self_type_keyword.rs:2:3
    |
+LL |   struct Self;
+   |   ^^^^^^^^^^^^ not accessible
 
 error[E0392]: parameter `'Self` is never used
   --> $DIR/self_type_keyword.rs:6:12