From e3d144d17fa518d471ebc0c9b6262e62ba2a702c Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 31 Dec 2022 14:20:59 +0100 Subject: [PATCH] Diagnose private field accesses --- crates/hir-ty/src/infer.rs | 1 + crates/hir-ty/src/infer/expr.rs | 23 +++++--- crates/hir/src/diagnostics.rs | 9 ++- crates/hir/src/lib.rs | 7 ++- .../src/handlers/private_field.rs | 55 +++++++++++++++++++ crates/ide-diagnostics/src/lib.rs | 2 + 6 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 crates/ide-diagnostics/src/handlers/private_field.rs diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index 18e45511a4b..e38c2154413 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -208,6 +208,7 @@ fn map(self, f: impl FnOnce(T) -> U) -> InferOk { #[derive(Debug, PartialEq, Eq, Clone)] pub enum InferenceDiagnostic { NoSuchField { expr: ExprId }, + PrivateField { expr: ExprId, field: FieldId }, BreakOutsideOfLoop { expr: ExprId, is_break: bool }, MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize }, } diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 53fc7a5949d..4881e7c8fc7 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -1,7 +1,6 @@ //! Type inference for expressions. use std::{ - collections::hash_map::Entry, iter::{repeat, repeat_with}, mem, }; @@ -521,6 +520,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { let receiver_ty = self.infer_expr_inner(*expr, &Expectation::none()); let mut autoderef = Autoderef::new(&mut self.table, receiver_ty); + let mut private_field = None; let ty = autoderef.by_ref().find_map(|(derefed_ty, _)| { let (field_id, parameters) = match derefed_ty.kind(Interner) { TyKind::Tuple(_, substs) => { @@ -547,13 +547,8 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { let is_visible = self.db.field_visibilities(field_id.parent)[field_id.local_id] .is_visible_from(self.db.upcast(), self.resolver.module()); if !is_visible { - // Write down the first field resolution even if it is not visible - // This aids IDE features for private fields like goto def and in - // case of autoderef finding an applicable field, this will be - // overwritten in a following cycle - if let Entry::Vacant(entry) = self.result.field_resolutions.entry(tgt_expr) - { - entry.insert(field_id); + if private_field.is_none() { + private_field = Some(field_id); } return None; } @@ -572,7 +567,17 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { let ty = self.normalize_associated_types_in(ty); ty } - _ => self.err_ty(), + _ => { + // Write down the first private field resolution if we found no field + // This aids IDE features for private fields like goto def + if let Some(field) = private_field { + self.result.field_resolutions.insert(tgt_expr, field); + self.result + .diagnostics + .push(InferenceDiagnostic::PrivateField { expr: tgt_expr, field }); + } + self.err_ty() + } }; ty } diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index c5dc60f1ec5..baeb525effe 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -10,7 +10,7 @@ use hir_expand::{name::Name, HirFileId, InFile}; use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange}; -use crate::{MacroKind, Type}; +use crate::{Field, MacroKind, Type}; macro_rules! diagnostics { ($($diag:ident,)*) => { @@ -41,6 +41,7 @@ fn from(d: $diag) -> AnyDiagnostic { MissingMatchArms, MissingUnsafe, NoSuchField, + PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch, UnimplementedBuiltinMacro, @@ -121,6 +122,12 @@ pub struct NoSuchField { pub field: InFile>, } +#[derive(Debug)] +pub struct PrivateField { + pub expr: InFile>, + pub field: Field, +} + #[derive(Debug)] pub struct BreakOutsideOfLoop { pub expr: InFile>, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index ede0c0c9886..e8e623e7d61 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -85,7 +85,7 @@ diagnostics::{ AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase, InvalidDeriveTarget, MacroError, MalformedDerive, MismatchedArgCount, MissingFields, MissingMatchArms, - MissingUnsafe, NoSuchField, ReplaceFilterMapNextWithFindMap, TypeMismatch, + MissingUnsafe, NoSuchField, PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch, UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, UnresolvedProcMacro, }, @@ -1353,6 +1353,11 @@ pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec) { Err(SyntheticSyntax) => (), } } + &hir_ty::InferenceDiagnostic::PrivateField { expr, field } => { + let expr = source_map.expr_syntax(expr).expect("unexpected synthetic"); + let field = field.into(); + acc.push(PrivateField { expr, field }.into()) + } } } for (expr, mismatch) in infer.expr_type_mismatches() { diff --git a/crates/ide-diagnostics/src/handlers/private_field.rs b/crates/ide-diagnostics/src/handlers/private_field.rs new file mode 100644 index 00000000000..3db5eca07bf --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/private_field.rs @@ -0,0 +1,55 @@ +use crate::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: private-field +// +// This diagnostic is triggered if created structure does not have field provided in record. +pub(crate) fn private_field(ctx: &DiagnosticsContext<'_>, d: &hir::PrivateField) -> Diagnostic { + // FIXME: add quickfix + Diagnostic::new( + "private-field", + format!( + "field `{}` of `{}` is private", + d.field.name(ctx.sema.db), + d.field.parent_def(ctx.sema.db).name(ctx.sema.db) + ), + ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::check_diagnostics; + + #[test] + fn private_field() { + check_diagnostics( + r#" +mod module { pub struct Struct { field: u32 } } +fn main(s: module::Struct) { + s.field; + //^^^^^^^ error: field `field` of `Struct` is private +} +"#, + ); + } + + #[test] + fn private_but_shadowed_in_deref() { + check_diagnostics( + r#" +//- minicore: deref +mod module { + pub struct Struct { field: Inner } + pub struct Inner { pub field: u32 } + impl core::ops::Deref for Struct { + type Target = Inner; + fn deref(&self) -> &Inner { &self.field } + } +} +fn main(s: module::Struct) { + s.field; +} +"#, + ); + } +} diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index b244666d1c0..91555e01b9c 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -37,6 +37,7 @@ mod handlers { pub(crate) mod missing_match_arms; pub(crate) mod missing_unsafe; pub(crate) mod no_such_field; + pub(crate) mod private_field; pub(crate) mod replace_filter_map_next_with_find_map; pub(crate) mod type_mismatch; pub(crate) mod unimplemented_builtin_macro; @@ -254,6 +255,7 @@ pub fn diagnostics( AnyDiagnostic::MissingMatchArms(d) => handlers::missing_match_arms::missing_match_arms(&ctx, &d), AnyDiagnostic::MissingUnsafe(d) => handlers::missing_unsafe::missing_unsafe(&ctx, &d), AnyDiagnostic::NoSuchField(d) => handlers::no_such_field::no_such_field(&ctx, &d), + AnyDiagnostic::PrivateField(d) => handlers::private_field::private_field(&ctx, &d), AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => handlers::replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d), AnyDiagnostic::TypeMismatch(d) => handlers::type_mismatch::type_mismatch(&ctx, &d), AnyDiagnostic::UnimplementedBuiltinMacro(d) => handlers::unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d),