Add a quickfix for accessing a private field of a struct

This commit is contained in:
Tyler Breisacher 2025-05-26 12:23:29 -07:00
parent 25808c1ba1
commit 4055436776
6 changed files with 138 additions and 182 deletions

View file

@ -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<T> = Result<InferOk<T>, TypeError>;
pub enum InferenceDiagnostic {
NoSuchField {
field: ExprOrPatId,
private: bool,
private: Option<LocalFieldId>,
variant: VariantId,
},
PrivateField {

View file

@ -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

View file

@ -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()

View file

@ -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 } => {

View file

@ -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: () }
",
);
}

View file

@ -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<Vec<Assist>> {
// 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<Vec<Assist>> {
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<ast::RecordFieldList> {
@ -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(