Auto merge of #76499 - guswynn:priv_des, r=petrochenkov

Give better diagnostic when using a private tuple struct constructor

Fixes #75907

Some notes:
1. This required some deep changes, including removing a Copy impl for PatKind. If some tests fail, I would still appreciate review on the overall approach
2. this only works with basic patterns (no wildcards for example), and fails if there is any problems getting the visibility of the fields (i am not sure what the failure that can happen in resolve_visibility_speculative, but we check the length of our fields in both cases against each other, so if anything goes wrong, we fall back to the worse error. This could be extended to more patterns
3. this does not yet deal with #75906, but I believe it will be similar
4. let me know if you want more tests
5. doesn't yet at the suggestion that `@yoshuawuyts` suggested at the end of their issue, but that could be added relatively easily (i believe)

9 files changed, 152 insertions, 29 deletions

diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
index 03c2915f848..565313902a4 100644
--- a/compiler/rustc_resolve/src/build_reduced_graph.rs
+++ b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -796,23 +796,26 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                         vis
                     };
 
+                    let mut ret_fields = Vec::with_capacity(vdata.fields().len());
+
                     for field in vdata.fields() {
                         // NOTE: The field may be an expansion placeholder, but expansion sets
                         // correct visibilities for unnamed field placeholders specifically, so the
                         // constructor visibility should still be determined correctly.
-                        if let Ok(field_vis) = self.resolve_visibility_speculative(&field.vis, true)
-                        {
-                            if ctor_vis.is_at_least(field_vis, &*self.r) {
-                                ctor_vis = field_vis;
-                            }
+                        let field_vis = self
+                            .resolve_visibility_speculative(&field.vis, true)
+                            .unwrap_or(ty::Visibility::Public);
+                        if ctor_vis.is_at_least(field_vis, &*self.r) {
+                            ctor_vis = field_vis;
                         }
+                        ret_fields.push(field_vis);
                     }
                     let ctor_res = Res::Def(
                         DefKind::Ctor(CtorOf::Struct, CtorKind::from_ast(vdata)),
                         self.r.local_def_id(ctor_node_id).to_def_id(),
                     );
                     self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, sp, expansion));
-                    self.r.struct_constructors.insert(def_id, (ctor_res, ctor_vis));
+                    self.r.struct_constructors.insert(def_id, (ctor_res, ctor_vis, ret_fields));
                 }
             }
 
@@ -964,7 +967,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
             Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => {
                 let parent = cstore.def_key(def_id).parent;
                 if let Some(struct_def_id) = parent.map(|index| DefId { index, ..def_id }) {
-                    self.r.struct_constructors.insert(struct_def_id, (res, vis));
+                    self.r.struct_constructors.insert(struct_def_id, (res, vis, vec![]));
                 }
             }
             _ => {}
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
index dda084214b8..07f36c7b7ad 100644
--- a/compiler/rustc_resolve/src/late.rs
+++ b/compiler/rustc_resolve/src/late.rs
@@ -188,7 +188,7 @@ crate enum PathSource<'a> {
     // Paths in struct expressions and patterns `Path { .. }`.
     Struct,
     // Paths in tuple struct patterns `Path(..)`.
-    TupleStruct(Span),
+    TupleStruct(Span, &'a [Span]),
     // `m::A::B` in `<T as m::A>::B::C`.
     TraitItem(Namespace),
 }
@@ -197,7 +197,7 @@ impl<'a> PathSource<'a> {
     fn namespace(self) -> Namespace {
         match self {
             PathSource::Type | PathSource::Trait(_) | PathSource::Struct => TypeNS,
-            PathSource::Expr(..) | PathSource::Pat | PathSource::TupleStruct(_) => ValueNS,
+            PathSource::Expr(..) | PathSource::Pat | PathSource::TupleStruct(..) => ValueNS,
             PathSource::TraitItem(ns) => ns,
         }
     }
@@ -208,7 +208,7 @@ impl<'a> PathSource<'a> {
             | PathSource::Expr(..)
             | PathSource::Pat
             | PathSource::Struct
-            | PathSource::TupleStruct(_) => true,
+            | PathSource::TupleStruct(..) => true,
             PathSource::Trait(_) | PathSource::TraitItem(..) => false,
         }
     }
@@ -219,7 +219,7 @@ impl<'a> PathSource<'a> {
             PathSource::Trait(_) => "trait",
             PathSource::Pat => "unit struct, unit variant or constant",
             PathSource::Struct => "struct, variant or union type",
-            PathSource::TupleStruct(_) => "tuple struct or tuple variant",
+            PathSource::TupleStruct(..) => "tuple struct or tuple variant",
             PathSource::TraitItem(ns) => match ns {
                 TypeNS => "associated type",
                 ValueNS => "method or associated constant",
@@ -305,7 +305,7 @@ impl<'a> PathSource<'a> {
                 | Res::SelfCtor(..) => true,
                 _ => false,
             },
-            PathSource::TupleStruct(_) => match res {
+            PathSource::TupleStruct(..) => match res {
                 Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) | Res::SelfCtor(..) => true,
                 _ => false,
             },
@@ -340,8 +340,8 @@ impl<'a> PathSource<'a> {
             (PathSource::Struct, false) => error_code!(E0422),
             (PathSource::Expr(..), true) => error_code!(E0423),
             (PathSource::Expr(..), false) => error_code!(E0425),
-            (PathSource::Pat | PathSource::TupleStruct(_), true) => error_code!(E0532),
-            (PathSource::Pat | PathSource::TupleStruct(_), false) => error_code!(E0531),
+            (PathSource::Pat | PathSource::TupleStruct(..), true) => error_code!(E0532),
+            (PathSource::Pat | PathSource::TupleStruct(..), false) => error_code!(E0531),
             (PathSource::TraitItem(..), true) => error_code!(E0575),
             (PathSource::TraitItem(..), false) => error_code!(E0576),
         }
@@ -411,7 +411,7 @@ struct LateResolutionVisitor<'a, 'b, 'ast> {
 }
 
 /// Walks the whole crate in DFS order, visiting each item, resolving names as it goes.
-impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
+impl<'a: 'ast, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
     fn visit_item(&mut self, item: &'ast Item) {
         let prev = replace(&mut self.diagnostic_metadata.current_item, Some(item));
         // Always report errors in items we just entered.
@@ -659,7 +659,7 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
     }
 }
 
-impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
+impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
     fn new(resolver: &'b mut Resolver<'a>) -> LateResolutionVisitor<'a, 'b, 'ast> {
         // During late resolution we only track the module component of the parent scope,
         // although it may be useful to track other components as well for diagnostics.
@@ -1539,8 +1539,16 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                         .unwrap_or_else(|| self.fresh_binding(ident, pat.id, pat_src, bindings));
                     self.r.record_partial_res(pat.id, PartialRes::new(res));
                 }
-                PatKind::TupleStruct(ref path, ..) => {
-                    self.smart_resolve_path(pat.id, None, path, PathSource::TupleStruct(pat.span));
+                PatKind::TupleStruct(ref path, ref sub_patterns) => {
+                    self.smart_resolve_path(
+                        pat.id,
+                        None,
+                        path,
+                        PathSource::TupleStruct(
+                            pat.span,
+                            self.r.arenas.alloc_pattern_spans(sub_patterns.iter().map(|p| p.span)),
+                        ),
+                    );
                 }
                 PatKind::Path(ref qself, ref path) => {
                     self.smart_resolve_path(pat.id, qself.as_ref(), path, PathSource::Pat);
diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs
index 9f631705b2b..9b5650c260c 100644
--- a/compiler/rustc_resolve/src/late/diagnostics.rs
+++ b/compiler/rustc_resolve/src/late/diagnostics.rs
@@ -90,7 +90,7 @@ fn import_candidate_to_enum_paths(suggestion: &ImportSuggestion) -> (String, Str
     (variant_path_string, enum_path_string)
 }
 
-impl<'a> LateResolutionVisitor<'a, '_, '_> {
+impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
     fn def_span(&self, def_id: DefId) -> Option<Span> {
         match def_id.krate {
             LOCAL_CRATE => self.r.opt_span(def_id),
@@ -623,12 +623,12 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
                         );
                     }
                 }
-                PathSource::Expr(_) | PathSource::TupleStruct(_) | PathSource::Pat => {
+                PathSource::Expr(_) | PathSource::TupleStruct(..) | PathSource::Pat => {
                     let span = match &source {
                         PathSource::Expr(Some(Expr {
                             span, kind: ExprKind::Call(_, _), ..
                         }))
-                        | PathSource::TupleStruct(span) => {
+                        | PathSource::TupleStruct(span, _) => {
                             // We want the main underline to cover the suggested code as well for
                             // cleaner output.
                             err.set_span(*span);
@@ -640,7 +640,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
                         err.span_label(span, &format!("`{}` defined here", path_str));
                     }
                     let (tail, descr, applicability) = match source {
-                        PathSource::Pat | PathSource::TupleStruct(_) => {
+                        PathSource::Pat | PathSource::TupleStruct(..) => {
                             ("", "pattern", Applicability::MachineApplicable)
                         }
                         _ => (": val", "literal", Applicability::HasPlaceholders),
@@ -705,7 +705,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
             }
             (
                 Res::Def(DefKind::Enum, def_id),
-                PathSource::TupleStruct(_) | PathSource::Expr(..),
+                PathSource::TupleStruct(..) | PathSource::Expr(..),
             ) => {
                 if self
                     .diagnostic_metadata
@@ -745,15 +745,50 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
                 }
             }
             (Res::Def(DefKind::Struct, def_id), _) if ns == ValueNS => {
-                if let Some((ctor_def, ctor_vis)) = self.r.struct_constructors.get(&def_id).cloned()
+                if let Some((ctor_def, ctor_vis, fields)) =
+                    self.r.struct_constructors.get(&def_id).cloned()
                 {
                     let accessible_ctor =
                         self.r.is_accessible_from(ctor_vis, self.parent_scope.module);
                     if is_expected(ctor_def) && !accessible_ctor {
-                        err.span_label(
-                            span,
-                            "constructor is not visible here due to private fields".to_string(),
-                        );
+                        let mut better_diag = false;
+                        if let PathSource::TupleStruct(_, pattern_spans) = source {
+                            if pattern_spans.len() > 0 && fields.len() == pattern_spans.len() {
+                                let non_visible_spans: Vec<Span> = fields
+                                    .iter()
+                                    .zip(pattern_spans.iter())
+                                    .filter_map(|(vis, span)| {
+                                        match self
+                                            .r
+                                            .is_accessible_from(*vis, self.parent_scope.module)
+                                        {
+                                            true => None,
+                                            false => Some(*span),
+                                        }
+                                    })
+                                    .collect();
+                                // Extra check to be sure
+                                if non_visible_spans.len() > 0 {
+                                    let mut m: rustc_span::MultiSpan =
+                                        non_visible_spans.clone().into();
+                                    non_visible_spans.into_iter().for_each(|s| {
+                                        m.push_span_label(s, "private field".to_string())
+                                    });
+                                    err.span_note(
+                                        m,
+                                        "constructor is not visible here due to private fields",
+                                    );
+                                    better_diag = true;
+                                }
+                            }
+                        }
+
+                        if !better_diag {
+                            err.span_label(
+                                span,
+                                "constructor is not visible here due to private fields".to_string(),
+                            );
+                        }
                     }
                 } else {
                     bad_struct_syntax_suggestion(def_id);
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
index c7913e84455..0f5b9b51816 100644
--- a/compiler/rustc_resolve/src/lib.rs
+++ b/compiler/rustc_resolve/src/lib.rs
@@ -1005,7 +1005,8 @@ pub struct Resolver<'a> {
 
     /// Table for mapping struct IDs into struct constructor IDs,
     /// it's not used during normal resolution, only for better error reporting.
-    struct_constructors: DefIdMap<(Res, ty::Visibility)>,
+    /// Also includes of list of each fields visibility
+    struct_constructors: DefIdMap<(Res, ty::Visibility, Vec<ty::Visibility>)>,
 
     /// Features enabled for this crate.
     active_features: FxHashSet<Symbol>,
@@ -1042,6 +1043,7 @@ pub struct ResolverArenas<'a> {
     name_resolutions: TypedArena<RefCell<NameResolution<'a>>>,
     macro_rules_bindings: TypedArena<MacroRulesBinding<'a>>,
     ast_paths: TypedArena<ast::Path>,
+    pattern_spans: TypedArena<Span>,
 }
 
 impl<'a> ResolverArenas<'a> {
@@ -1073,6 +1075,9 @@ impl<'a> ResolverArenas<'a> {
     fn alloc_ast_paths(&'a self, paths: &[ast::Path]) -> &'a [ast::Path] {
         self.ast_paths.alloc_from_iter(paths.iter().cloned())
     }
+    fn alloc_pattern_spans(&'a self, spans: impl Iterator<Item = Span>) -> &'a [Span] {
+        self.pattern_spans.alloc_from_iter(spans)
+    }
 }
 
 impl<'a> AsMut<Resolver<'a>> for Resolver<'a> {
diff --git a/src/test/ui/issues/auxiliary/issue-75907.rs b/src/test/ui/issues/auxiliary/issue-75907.rs
new file mode 100644
index 00000000000..0b70452a24d
--- /dev/null
+++ b/src/test/ui/issues/auxiliary/issue-75907.rs
@@ -0,0 +1,5 @@
+pub struct Bar(pub u8, u8, u8);
+
+pub fn make_bar() -> Bar {
+    Bar(1, 12, 10)
+}
diff --git a/src/test/ui/issues/issue-75907.rs b/src/test/ui/issues/issue-75907.rs
new file mode 100644
index 00000000000..8c155d9be35
--- /dev/null
+++ b/src/test/ui/issues/issue-75907.rs
@@ -0,0 +1,18 @@
+// Test for for diagnostic improvement issue #75907
+
+mod foo {
+    pub(crate) struct Foo(u8);
+    pub(crate) struct Bar(pub u8, u8, Foo);
+
+    pub(crate) fn make_bar() -> Bar {
+        Bar(1, 12, Foo(10))
+    }
+}
+
+use foo::{make_bar, Bar, Foo};
+
+fn main() {
+    let Bar(x, y, Foo(z)) = make_bar();
+    //~^ ERROR expected tuple struct
+    //~| ERROR expected tuple struct
+}
diff --git a/src/test/ui/issues/issue-75907.stderr b/src/test/ui/issues/issue-75907.stderr
new file mode 100644
index 00000000000..65b9a51e01d
--- /dev/null
+++ b/src/test/ui/issues/issue-75907.stderr
@@ -0,0 +1,29 @@
+error[E0532]: expected tuple struct or tuple variant, found struct `Bar`
+  --> $DIR/issue-75907.rs:15:9
+   |
+LL |     let Bar(x, y, Foo(z)) = make_bar();
+   |         ^^^
+   |
+note: constructor is not visible here due to private fields
+  --> $DIR/issue-75907.rs:15:16
+   |
+LL |     let Bar(x, y, Foo(z)) = make_bar();
+   |                ^  ^^^^^^ private field
+   |                |
+   |                private field
+
+error[E0532]: expected tuple struct or tuple variant, found struct `Foo`
+  --> $DIR/issue-75907.rs:15:19
+   |
+LL |     let Bar(x, y, Foo(z)) = make_bar();
+   |                   ^^^
+   |
+note: constructor is not visible here due to private fields
+  --> $DIR/issue-75907.rs:15:23
+   |
+LL |     let Bar(x, y, Foo(z)) = make_bar();
+   |                       ^ private field
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0532`.
diff --git a/src/test/ui/issues/issue-75907_b.rs b/src/test/ui/issues/issue-75907_b.rs
new file mode 100644
index 00000000000..fdd3bc6d724
--- /dev/null
+++ b/src/test/ui/issues/issue-75907_b.rs
@@ -0,0 +1,11 @@
+// Test for for diagnostic improvement issue #75907, extern crate
+// aux-build:issue-75907.rs
+
+extern crate issue_75907 as a;
+
+use a::{make_bar, Bar};
+
+fn main() {
+    let Bar(x, y, z) = make_bar();
+    //~^ ERROR expected tuple struct
+}
diff --git a/src/test/ui/issues/issue-75907_b.stderr b/src/test/ui/issues/issue-75907_b.stderr
new file mode 100644
index 00000000000..cdd21de6c33
--- /dev/null
+++ b/src/test/ui/issues/issue-75907_b.stderr
@@ -0,0 +1,9 @@
+error[E0532]: expected tuple struct or tuple variant, found struct `Bar`
+  --> $DIR/issue-75907_b.rs:9:9
+   |
+LL |     let Bar(x, y, z) = make_bar();
+   |         ^^^ constructor is not visible here due to private fields
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0532`.