diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index e698fb201c..7d8e0f7dd2 100644 --- a/crates/hir-ty/src/infer.rs +++ b/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}, @@ -203,7 +204,7 @@ pub(crate) type InferResult = Result, TypeError>; pub enum InferenceDiagnostic { NoSuchField { field: ExprOrPatId, - private: bool, + private: Option, variant: VariantId, }, PrivateField { diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 87b7f3406f..6403127929 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/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 diff --git a/crates/hir-ty/src/infer/pat.rs b/crates/hir-ty/src/infer/pat.rs index a9a3265858..4bc3e167eb 100644 --- a/crates/hir-ty/src/infer/pat.rs +++ b/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() diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index b6e3002ed5..f7b140e03d 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -224,7 +224,7 @@ pub struct MalformedDerive { #[derive(Debug)] pub struct NoSuchField { pub field: InFile>>, - pub private: bool, + pub private: Option, 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 } => { diff --git a/crates/ide-assists/src/handlers/fix_visibility.rs b/crates/ide-assists/src/handlers/fix_visibility.rs index 19e0a73f33..3badc17d01 100644 --- a/crates/ide-assists/src/handlers/fix_visibility.rs +++ b/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: () } ", ); } diff --git a/crates/ide-diagnostics/src/handlers/no_such_field.rs b/crates/ide-diagnostics/src/handlers/no_such_field.rs index 84fb467a5c..ef42f2dc74 100644 --- a/crates/ide-diagnostics/src/handlers/no_such_field.rs +++ b/crates/ide-diagnostics/src/handlers/no_such_field.rs @@ -1,4 +1,5 @@ use either::Either; +use hir::{Field, HasCrate}; use hir::{HasSource, HirDisplay, Semantics, VariantId, db::ExpandDatabase}; use ide_db::text_edit::TextEdit; use ide_db::{EditionedFileId, RootDatabase, source_change::SourceChange}; @@ -13,44 +14,69 @@ use crate::{Assist, Diagnostic, DiagnosticCode, DiagnosticsContext, fix}; // // This diagnostic is triggered if created structure does not have field provided in record. pub(crate) fn no_such_field(ctx: &DiagnosticsContext<'_>, d: &hir::NoSuchField) -> Diagnostic { - let node = d.field.map(Into::into); - if d.private { - // FIXME: quickfix to add required visibility - Diagnostic::new_with_syntax_node_ptr( - ctx, - DiagnosticCode::RustcHardError("E0451"), - "field is private", - node, - ) - .stable() + let (code, message) = if d.private.is_some() { + ("E0451", "field is private") + } else if let VariantId::EnumVariantId(_) = d.variant { + ("E0559", "no such field") } else { - Diagnostic::new_with_syntax_node_ptr( - ctx, - match d.variant { - VariantId::EnumVariantId(_) => DiagnosticCode::RustcHardError("E0559"), - _ => DiagnosticCode::RustcHardError("E0560"), - }, - "no such field", - node, - ) + ("E0560", "no such field") + }; + + let node = d.field.map(Into::into); + Diagnostic::new_with_syntax_node_ptr(ctx, DiagnosticCode::RustcHardError(code), message, node) .stable() .with_fixes(fixes(ctx, d)) - } } fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::NoSuchField) -> Option> { // FIXME: quickfix for pattern let root = ctx.sema.db.parse_or_expand(d.field.file_id); match &d.field.value.to_node(&root) { - Either::Left(node) => missing_record_expr_field_fixes( - &ctx.sema, - d.field.file_id.original_file(ctx.sema.db), - node, - ), + Either::Left(node) => { + if let Some(private_field) = d.private { + field_is_private_fixes( + &ctx.sema, + d.field.file_id.original_file(ctx.sema.db), + node, + private_field, + ) + } else { + missing_record_expr_field_fixes( + &ctx.sema, + d.field.file_id.original_file(ctx.sema.db), + node, + ) + } + } _ => None, } } +fn field_is_private_fixes( + sema: &Semantics<'_, RootDatabase>, + usage_file_id: EditionedFileId, + record_expr_field: &ast::RecordExprField, + private_field: Field, +) -> Option> { + let def_crate = private_field.krate(sema.db); + let usage_crate = sema.file_to_module_def(usage_file_id.file_id(sema.db))?.krate(); + let visibility = if usage_crate == def_crate { "pub(crate) " } else { "pub " }; + + let source = private_field.source(sema.db)?; + let (range, _) = source.syntax().original_file_range_opt(sema.db)?; + let source_change = SourceChange::from_text_edit( + range.file_id.file_id(sema.db), + TextEdit::insert(range.range.start(), visibility.into()), + ); + + Some(vec![fix( + "increase_field_visibility", + "Increase field visibility", + source_change, + sema.original_range(record_expr_field.syntax()).range, + )]) +} + fn missing_record_expr_field_fixes( sema: &Semantics<'_, RootDatabase>, usage_file_id: EditionedFileId, @@ -118,7 +144,7 @@ fn missing_record_expr_field_fixes( "create_field", "Create field", source_change, - record_expr_field.syntax().text_range(), + sema.original_range(record_expr_field.syntax()).range, )]); fn record_field_list(field_def_list: ast::FieldList) -> Option { @@ -387,21 +413,92 @@ fn f(s@m::Struct { // assignee expression m::Struct { field: 0, - //^^^^^^^^ error: field is private + //^^^^^^^^ 💡 error: field is private field2 - //^^^^^^ error: field is private + //^^^^^^ 💡 error: field is private } = s; m::Struct { field: 0, - //^^^^^^^^ error: field is private + //^^^^^^^^ 💡 error: field is private field2 - //^^^^^^ error: field is private + //^^^^^^ 💡 error: field is private }; } "#, ) } + #[test] + fn test_struct_field_private_same_crate_fix() { + check_diagnostics( + r#" +mod m { + pub struct Struct { + field: u32, + } +} +fn f() { + let _ = m::Struct { + field: 0, + //^^^^^^^^ 💡 error: field is private + }; +} +"#, + ); + + check_fix( + r#" +mod m { + pub struct Struct { + field: u32, + } +} +fn f() { + let _ = m::Struct { + field$0: 0, + }; +} +"#, + r#" +mod m { + pub struct Struct { + pub(crate) field: u32, + } +} +fn f() { + let _ = m::Struct { + field: 0, + }; +} +"#, + ); + } + + #[test] + fn test_struct_field_private_other_crate_fix() { + check_fix( + r#" +//- /lib.rs crate:another_crate +pub struct Struct { + field: u32, +} +//- /lib.rs crate:this_crate deps:another_crate +use another_crate; + +fn f() { + let _ = another_crate::Struct { + field$0: 0, + }; +} +"#, + r#" +pub struct Struct { + pub field: u32, +} +"#, + ); + } + #[test] fn editions_between_macros() { check_diagnostics(