Auto merge of #13870 - Veykril:private-field-diag, r=Veykril

Diagnose private field accesses
This commit is contained in:
bors 2022-12-31 13:21:53 +00:00
commit da15d92a32
6 changed files with 86 additions and 11 deletions

View file

@ -208,6 +208,7 @@ pub(crate) type InferResult<T> = Result<InferOk<T>, TypeError>;
#[derive(Debug, PartialEq, Eq, Clone)] #[derive(Debug, PartialEq, Eq, Clone)]
pub enum InferenceDiagnostic { pub enum InferenceDiagnostic {
NoSuchField { expr: ExprId }, NoSuchField { expr: ExprId },
PrivateField { expr: ExprId, field: FieldId },
BreakOutsideOfLoop { expr: ExprId, is_break: bool }, BreakOutsideOfLoop { expr: ExprId, is_break: bool },
MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize }, MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize },
} }

View file

@ -1,7 +1,6 @@
//! Type inference for expressions. //! Type inference for expressions.
use std::{ use std::{
collections::hash_map::Entry,
iter::{repeat, repeat_with}, iter::{repeat, repeat_with},
mem, mem,
}; };
@ -521,6 +520,7 @@ impl<'a> InferenceContext<'a> {
let receiver_ty = self.infer_expr_inner(*expr, &Expectation::none()); let receiver_ty = self.infer_expr_inner(*expr, &Expectation::none());
let mut autoderef = Autoderef::new(&mut self.table, receiver_ty); 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 ty = autoderef.by_ref().find_map(|(derefed_ty, _)| {
let (field_id, parameters) = match derefed_ty.kind(Interner) { let (field_id, parameters) = match derefed_ty.kind(Interner) {
TyKind::Tuple(_, substs) => { TyKind::Tuple(_, substs) => {
@ -547,13 +547,8 @@ impl<'a> InferenceContext<'a> {
let is_visible = self.db.field_visibilities(field_id.parent)[field_id.local_id] let is_visible = self.db.field_visibilities(field_id.parent)[field_id.local_id]
.is_visible_from(self.db.upcast(), self.resolver.module()); .is_visible_from(self.db.upcast(), self.resolver.module());
if !is_visible { if !is_visible {
// Write down the first field resolution even if it is not visible if private_field.is_none() {
// This aids IDE features for private fields like goto def and in private_field = Some(field_id);
// 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);
} }
return None; return None;
} }
@ -572,7 +567,17 @@ impl<'a> InferenceContext<'a> {
let ty = self.normalize_associated_types_in(ty); let ty = self.normalize_associated_types_in(ty);
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 ty
} }

View file

@ -10,7 +10,7 @@ use hir_def::path::ModPath;
use hir_expand::{name::Name, HirFileId, InFile}; use hir_expand::{name::Name, HirFileId, InFile};
use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange}; use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange};
use crate::{MacroKind, Type}; use crate::{Field, MacroKind, Type};
macro_rules! diagnostics { macro_rules! diagnostics {
($($diag:ident,)*) => { ($($diag:ident,)*) => {
@ -41,6 +41,7 @@ diagnostics![
MissingMatchArms, MissingMatchArms,
MissingUnsafe, MissingUnsafe,
NoSuchField, NoSuchField,
PrivateField,
ReplaceFilterMapNextWithFindMap, ReplaceFilterMapNextWithFindMap,
TypeMismatch, TypeMismatch,
UnimplementedBuiltinMacro, UnimplementedBuiltinMacro,
@ -121,6 +122,12 @@ pub struct NoSuchField {
pub field: InFile<AstPtr<ast::RecordExprField>>, pub field: InFile<AstPtr<ast::RecordExprField>>,
} }
#[derive(Debug)]
pub struct PrivateField {
pub expr: InFile<AstPtr<ast::Expr>>,
pub field: Field,
}
#[derive(Debug)] #[derive(Debug)]
pub struct BreakOutsideOfLoop { pub struct BreakOutsideOfLoop {
pub expr: InFile<AstPtr<ast::Expr>>, pub expr: InFile<AstPtr<ast::Expr>>,

View file

@ -85,7 +85,7 @@ pub use crate::{
diagnostics::{ diagnostics::{
AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase, InvalidDeriveTarget, AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase, InvalidDeriveTarget,
MacroError, MalformedDerive, MismatchedArgCount, MissingFields, MissingMatchArms, MacroError, MalformedDerive, MismatchedArgCount, MissingFields, MissingMatchArms,
MissingUnsafe, NoSuchField, ReplaceFilterMapNextWithFindMap, TypeMismatch, MissingUnsafe, NoSuchField, PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch,
UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall,
UnresolvedModule, UnresolvedProcMacro, UnresolvedModule, UnresolvedProcMacro,
}, },
@ -1353,6 +1353,11 @@ impl DefWithBody {
Err(SyntheticSyntax) => (), 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() { for (expr, mismatch) in infer.expr_type_mismatches() {

View file

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

View file

@ -37,6 +37,7 @@ mod handlers {
pub(crate) mod missing_match_arms; pub(crate) mod missing_match_arms;
pub(crate) mod missing_unsafe; pub(crate) mod missing_unsafe;
pub(crate) mod no_such_field; pub(crate) mod no_such_field;
pub(crate) mod private_field;
pub(crate) mod replace_filter_map_next_with_find_map; pub(crate) mod replace_filter_map_next_with_find_map;
pub(crate) mod type_mismatch; pub(crate) mod type_mismatch;
pub(crate) mod unimplemented_builtin_macro; 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::MissingMatchArms(d) => handlers::missing_match_arms::missing_match_arms(&ctx, &d),
AnyDiagnostic::MissingUnsafe(d) => handlers::missing_unsafe::missing_unsafe(&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::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::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::TypeMismatch(d) => handlers::type_mismatch::type_mismatch(&ctx, &d),
AnyDiagnostic::UnimplementedBuiltinMacro(d) => handlers::unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), AnyDiagnostic::UnimplementedBuiltinMacro(d) => handlers::unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d),