Merge pull request #19869 from MatrixFrog/publicize_field

Add a quickfix for accessing a private field of a struct

Author: Veykril

crates/hir-ty/src/infer.rs

@@ -35,7 +35,8 @@ use chalk_ir::{
 use either::Either;
 use hir_def::{
     AdtId, AssocItemId, ConstId, DefWithBodyId, FieldId, FunctionId, GenericDefId, GenericParamId,
-    ImplId, ItemContainerId, Lookup, TraitId, TupleFieldId, TupleId, TypeAliasId, VariantId,
+    ImplId, ItemContainerId, LocalFieldId, Lookup, TraitId, TupleFieldId, TupleId, TypeAliasId,
+    VariantId,
     builtin_type::{BuiltinInt, BuiltinType, BuiltinUint},
     expr_store::{Body, ExpressionStore, HygieneId, path::Path},
     hir::{BindingAnnotation, BindingId, ExprId, ExprOrPatId, LabelId, PatId},
@@ -207,7 +208,7 @@ pub(crate) type InferResult<T> = Result<InferOk<T>, TypeError>;
 pub enum InferenceDiagnostic {
     NoSuchField {
         field: ExprOrPatId,
-        private: bool,
+        private: Option<LocalFieldId>,
         variant: VariantId,
     },
     PrivateField {

crates/hir-ty/src/infer/expr.rs

@@ -554,7 +554,7 @@ impl InferenceContext<'_> {
                                             self.push_diagnostic(
                                                 InferenceDiagnostic::NoSuchField {
                                                     field: field.expr.into(),
-                                                    private: true,
+                                                    private: Some(local_id),
                                                     variant: def,
                                                 },
                                             );
@@ -564,7 +564,7 @@ impl InferenceContext<'_> {
                                     None => {
                                         self.push_diagnostic(InferenceDiagnostic::NoSuchField {
                                             field: field.expr.into(),
-                                            private: false,
+                                            private: None,
                                             variant: def,
                                         });
                                         None

crates/hir-ty/src/infer/pat.rs

@@ -143,7 +143,7 @@ impl InferenceContext<'_> {
                                 {
                                     self.push_diagnostic(InferenceDiagnostic::NoSuchField {
                                         field: inner.into(),
-                                        private: true,
+                                        private: Some(local_id),
                                         variant: def,
                                     });
                                 }
@@ -157,7 +157,7 @@ impl InferenceContext<'_> {
                             None => {
                                 self.push_diagnostic(InferenceDiagnostic::NoSuchField {
                                     field: inner.into(),
-                                    private: false,
+                                    private: None,
                                     variant: def,
                                 });
                                 self.err_ty()

crates/hir/src/diagnostics.rs

@@ -224,7 +224,7 @@ pub struct MalformedDerive {
 #[derive(Debug)]
 pub struct NoSuchField {
     pub field: InFile<AstPtr<Either<ast::RecordExprField, ast::RecordPatField>>>,
-    pub private: bool,
+    pub private: Option<Field>,
     pub variant: VariantId,
 }
 
@@ -648,6 +648,7 @@ impl AnyDiagnostic {
                     }
                     ExprOrPatId::PatId(pat) => source_map.pat_field_syntax(pat),
                 };
+                let private = private.map(|id| Field { id, parent: variant.into() });
                 NoSuchField { field: expr_or_pat, private, variant }.into()
             }
             &InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => {

crates/ide-assists/src/handlers/fix_visibility.rs

@@ -7,10 +7,10 @@ use syntax::{
 
 use crate::{AssistContext, AssistId, Assists};
 
-// FIXME: this really should be a fix for diagnostic, rather than an assist.
-
 // Assist: fix_visibility
 //
+// Note that there is some duplication between this and the no_such_field diagnostic.
+//
 // Makes inaccessible item public.
 //
 // ```
@@ -32,7 +32,6 @@ use crate::{AssistContext, AssistId, Assists};
 // ```
 pub(crate) fn fix_visibility(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
     add_vis_to_referenced_module_def(acc, ctx)
-        .or_else(|| add_vis_to_referenced_record_field(acc, ctx))
 }
 
 fn add_vis_to_referenced_module_def(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
@@ -88,59 +87,6 @@ fn add_vis_to_referenced_module_def(acc: &mut Assists, ctx: &AssistContext<'_>)
     })
 }
 
-fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
-    let record_field: ast::RecordExprField = ctx.find_node_at_offset()?;
-    let (record_field_def, _, _) = ctx.sema.resolve_record_field(&record_field)?;
-
-    let current_module = ctx.sema.scope(record_field.syntax())?.module();
-    let current_edition = current_module.krate().edition(ctx.db());
-    let visibility = record_field_def.visibility(ctx.db());
-    if visibility.is_visible_from(ctx.db(), current_module.into()) {
-        return None;
-    }
-
-    let parent = record_field_def.parent_def(ctx.db());
-    let parent_name = parent.name(ctx.db());
-    let target_module = parent.module(ctx.db());
-
-    let in_file_source = record_field_def.source(ctx.db())?;
-    let (vis_owner, target) = match in_file_source.value {
-        hir::FieldSource::Named(it) => {
-            let range = it.syntax().text_range();
-            (ast::AnyHasVisibility::new(it), range)
-        }
-        hir::FieldSource::Pos(it) => {
-            let range = it.syntax().text_range();
-            (ast::AnyHasVisibility::new(it), range)
-        }
-    };
-
-    let missing_visibility = if current_module.krate() == target_module.krate() {
-        make::visibility_pub_crate()
-    } else {
-        make::visibility_pub()
-    };
-    let target_file = in_file_source.file_id.original_file(ctx.db());
-
-    let target_name = record_field_def.name(ctx.db());
-    let assist_label = format!(
-        "Change visibility of {}.{} to {missing_visibility}",
-        parent_name.display(ctx.db(), current_edition),
-        target_name.display(ctx.db(), current_edition)
-    );
-
-    acc.add(AssistId::quick_fix("fix_visibility"), assist_label, target, |edit| {
-        edit.edit_file(target_file.file_id(ctx.db()));
-
-        let vis_owner = edit.make_mut(vis_owner);
-        vis_owner.set_visibility(Some(missing_visibility.clone_for_update()));
-
-        if let Some((cap, vis)) = ctx.config.snippet_cap.zip(vis_owner.visibility()) {
-            edit.add_tabstop_before(cap, vis);
-        }
-    })
-}
-
 fn target_data_for_def(
     db: &dyn HirDatabase,
     def: hir::ModuleDef,
@@ -293,44 +239,6 @@ struct Foo;
         );
     }
 
-    #[test]
-    fn fix_visibility_of_struct_field() {
-        check_assist(
-            fix_visibility,
-            r"mod foo { pub struct Foo { bar: (), } }
-              fn main() { foo::Foo { $0bar: () }; } ",
-            r"mod foo { pub struct Foo { $0pub(crate) bar: (), } }
-              fn main() { foo::Foo { bar: () }; } ",
-        );
-        check_assist(
-            fix_visibility,
-            r"
-//- /lib.rs
-mod foo;
-fn main() { foo::Foo { $0bar: () }; }
-//- /foo.rs
-pub struct Foo { bar: () }
-",
-            r"pub struct Foo { $0pub(crate) bar: () }
-",
-        );
-        check_assist_not_applicable(
-            fix_visibility,
-            r"mod foo { pub struct Foo { pub bar: (), } }
-              fn main() { foo::Foo { $0bar: () }; } ",
-        );
-        check_assist_not_applicable(
-            fix_visibility,
-            r"
-//- /lib.rs
-mod foo;
-fn main() { foo::Foo { $0bar: () }; }
-//- /foo.rs
-pub struct Foo { pub bar: () }
-",
-        );
-    }
-
     #[test]
     fn fix_visibility_of_enum_variant_field() {
         // Enum variants, as well as their fields, always get the enum's visibility. In fact, rustc
@@ -367,44 +275,6 @@ pub struct Foo { pub bar: () }
         );
     }
 
-    #[test]
-    fn fix_visibility_of_union_field() {
-        check_assist(
-            fix_visibility,
-            r"mod foo { pub union Foo { bar: (), } }
-              fn main() { foo::Foo { $0bar: () }; } ",
-            r"mod foo { pub union Foo { $0pub(crate) bar: (), } }
-              fn main() { foo::Foo { bar: () }; } ",
-        );
-        check_assist(
-            fix_visibility,
-            r"
-//- /lib.rs
-mod foo;
-fn main() { foo::Foo { $0bar: () }; }
-//- /foo.rs
-pub union Foo { bar: () }
-",
-            r"pub union Foo { $0pub(crate) bar: () }
-",
-        );
-        check_assist_not_applicable(
-            fix_visibility,
-            r"mod foo { pub union Foo { pub bar: (), } }
-              fn main() { foo::Foo { $0bar: () }; } ",
-        );
-        check_assist_not_applicable(
-            fix_visibility,
-            r"
-//- /lib.rs
-mod foo;
-fn main() { foo::Foo { $0bar: () }; }
-//- /foo.rs
-pub union Foo { pub bar: () }
-",
-        );
-    }
-
     #[test]
     fn fix_visibility_of_const() {
         check_assist(
@@ -570,19 +440,6 @@ foo::Bar$0
 pub(crate) struct Bar;
 ",
             r"$0pub struct Bar;
-",
-        );
-        check_assist(
-            fix_visibility,
-            r"
-//- /main.rs crate:a deps:foo
-fn main() {
-    foo::Foo { $0bar: () };
-}
-//- /lib.rs crate:foo
-pub struct Foo { pub(crate) bar: () }
-",
-            r"pub struct Foo { $0pub bar: () }
 ",
         );
     }