feat: Add incorrect case diagnostics for enum variant fields and all variables

This commit is contained in:
Campbell 2024-07-13 21:54:22 +12:00
parent ffbc5ad993
commit af30111b1c
3 changed files with 194 additions and 31 deletions

View file

@ -17,8 +17,8 @@ use std::fmt;
use hir_def::{ use hir_def::{
data::adt::VariantData, db::DefDatabase, hir::Pat, src::HasSource, AdtId, AttrDefId, ConstId, data::adt::VariantData, db::DefDatabase, hir::Pat, src::HasSource, AdtId, AttrDefId, ConstId,
EnumId, FunctionId, ItemContainerId, Lookup, ModuleDefId, ModuleId, StaticId, StructId, EnumId, EnumVariantId, FunctionId, ItemContainerId, Lookup, ModuleDefId, ModuleId, StaticId,
TraitId, TypeAliasId, StructId, TraitId, TypeAliasId,
}; };
use hir_expand::{ use hir_expand::{
name::{AsName, Name}, name::{AsName, Name},
@ -353,17 +353,16 @@ impl<'a> DeclValidator<'a> {
continue; continue;
}; };
let is_param = ast::Param::can_cast(parent.kind()); let is_shorthand = ast::RecordPatField::cast(parent.clone())
// We have to check that it's either `let var = ...` or `var @ Variant(_)` statement, .map(|parent| parent.name_ref().is_none())
// because e.g. match arms are patterns as well. .unwrap_or_default();
// In other words, we check that it's a named variable binding. if is_shorthand {
let is_binding = ast::LetStmt::can_cast(parent.kind()) // We don't check shorthand field patterns, such as 'field' in `Thing { field }`,
|| (ast::MatchArm::can_cast(parent.kind()) && ident_pat.at_token().is_some()); // since the shorthand isn't the declaration.
if !(is_param || is_binding) {
// This pattern is not an actual variable declaration, e.g. `Some(val) => {..}` match arm.
continue; continue;
} }
let is_param = ast::Param::can_cast(parent.kind());
let ident_type = if is_param { IdentType::Parameter } else { IdentType::Variable }; let ident_type = if is_param { IdentType::Parameter } else { IdentType::Variable };
self.create_incorrect_case_diagnostic_for_ast_node( self.create_incorrect_case_diagnostic_for_ast_node(
@ -489,6 +488,11 @@ impl<'a> DeclValidator<'a> {
/// Check incorrect names for enum variants. /// Check incorrect names for enum variants.
fn validate_enum_variants(&mut self, enum_id: EnumId) { fn validate_enum_variants(&mut self, enum_id: EnumId) {
let data = self.db.enum_data(enum_id); let data = self.db.enum_data(enum_id);
for (variant_id, _) in data.variants.iter() {
self.validate_enum_variant_fields(*variant_id);
}
let mut enum_variants_replacements = data let mut enum_variants_replacements = data
.variants .variants
.iter() .iter()
@ -551,6 +555,75 @@ impl<'a> DeclValidator<'a> {
} }
} }
/// Check incorrect names for fields of enum variant.
fn validate_enum_variant_fields(&mut self, variant_id: EnumVariantId) {
let variant_data = self.db.enum_variant_data(variant_id);
let VariantData::Record(fields) = variant_data.variant_data.as_ref() else {
return;
};
let mut variant_field_replacements = fields
.iter()
.filter_map(|(_, field)| {
to_lower_snake_case(&field.name.to_smol_str()).map(|new_name| Replacement {
current_name: field.name.clone(),
suggested_text: new_name,
expected_case: CaseType::LowerSnakeCase,
})
})
.peekable();
// XXX: only look at sources if we do have incorrect names
if variant_field_replacements.peek().is_none() {
return;
}
let variant_loc = variant_id.lookup(self.db.upcast());
let variant_src = variant_loc.source(self.db.upcast());
let Some(ast::FieldList::RecordFieldList(variant_fields_list)) =
variant_src.value.field_list()
else {
always!(
variant_field_replacements.peek().is_none(),
"Replacements ({:?}) were generated for an enum variant \
which had no fields list: {:?}",
variant_field_replacements.collect::<Vec<_>>(),
variant_src
);
return;
};
let mut variant_variants_iter = variant_fields_list.fields();
for field_replacement in variant_field_replacements {
// We assume that parameters in replacement are in the same order as in the
// actual params list, but just some of them (ones that named correctly) are skipped.
let field = loop {
if let Some(field) = variant_variants_iter.next() {
let Some(field_name) = field.name() else {
continue;
};
if field_name.as_name() == field_replacement.current_name {
break field;
}
} else {
never!(
"Replacement ({:?}) was generated for an enum variant field \
which was not found: {:?}",
field_replacement,
variant_src
);
return;
}
};
self.create_incorrect_case_diagnostic_for_ast_node(
field_replacement,
variant_src.file_id,
&field,
IdentType::Field,
);
}
}
fn validate_const(&mut self, const_id: ConstId) { fn validate_const(&mut self, const_id: ConstId) {
let container = const_id.lookup(self.db.upcast()).container; let container = const_id.lookup(self.db.upcast()).container;
if self.is_trait_impl_container(container) { if self.is_trait_impl_container(container) {

View file

@ -332,6 +332,7 @@ impl someStruct {
check_diagnostics( check_diagnostics(
r#" r#"
enum Option { Some, None } enum Option { Some, None }
use Option::{Some, None};
#[allow(unused)] #[allow(unused)]
fn main() { fn main() {
@ -344,24 +345,6 @@ fn main() {
); );
} }
#[test]
fn non_let_bind() {
check_diagnostics(
r#"
enum Option { Some, None }
#[allow(unused)]
fn main() {
match Option::None {
SOME_VAR @ None => (),
// ^^^^^^^^ 💡 warn: Variable `SOME_VAR` should have snake_case name, e.g. `some_var`
Some => (),
}
}
"#,
);
}
#[test] #[test]
fn allow_attributes_crate_attr() { fn allow_attributes_crate_attr() {
check_diagnostics( check_diagnostics(
@ -427,7 +410,12 @@ fn qualify() {
#[test] // Issue #8809. #[test] // Issue #8809.
fn parenthesized_parameter() { fn parenthesized_parameter() {
check_diagnostics(r#"fn f((O): _) { _ = O; }"#) check_diagnostics(
r#"
fn f((_O): u8) {}
// ^^ 💡 warn: Variable `_O` should have snake_case name, e.g. `_o`
"#,
)
} }
#[test] #[test]
@ -766,4 +754,106 @@ mod Foo;
"#, "#,
) )
} }
#[test]
fn test_field_shorthand() {
check_diagnostics(
r#"
struct Foo { _nonSnake: u8 }
// ^^^^^^^^^ 💡 warn: Field `_nonSnake` should have snake_case name, e.g. `_non_snake`
fn func(Foo { _nonSnake }: Foo) {}
"#,
);
}
#[test]
fn test_match() {
check_diagnostics(
r#"
enum Foo { Variant { nonSnake1: u8 } }
// ^^^^^^^^^ 💡 warn: Field `nonSnake1` should have snake_case name, e.g. `non_snake1`
fn func() {
match (Foo::Variant { nonSnake1: 1 }) {
Foo::Variant { nonSnake1: _nonSnake2 } => {},
// ^^^^^^^^^^ 💡 warn: Variable `_nonSnake2` should have snake_case name, e.g. `_non_snake2`
}
}
"#,
);
check_diagnostics(
r#"
struct Foo(u8);
fn func() {
match Foo(1) {
Foo(_nonSnake) => {},
// ^^^^^^^^^ 💡 warn: Variable `_nonSnake` should have snake_case name, e.g. `_non_snake`
}
}
"#,
);
check_diagnostics(
r#"
fn main() {
match 1 {
_Bad1 @ _Bad2 => {}
// ^^^^^ 💡 warn: Variable `_Bad1` should have snake_case name, e.g. `_bad1`
// ^^^^^ 💡 warn: Variable `_Bad2` should have snake_case name, e.g. `_bad2`
}
}
"#,
);
check_diagnostics(
r#"
fn main() {
match 1 { _Bad1 => () }
// ^^^^^ 💡 warn: Variable `_Bad1` should have snake_case name, e.g. `_bad1`
}
"#,
);
check_diagnostics(
r#"
enum Foo { V1, V2 }
use Foo::V1;
fn main() {
match V1 {
_Bad1 @ V1 => {},
// ^^^^^ 💡 warn: Variable `_Bad1` should have snake_case name, e.g. `_bad1`
Foo::V2 => {}
}
}
"#,
);
}
#[test]
fn test_for_loop() {
check_diagnostics(
r#"
//- minicore: iterators
fn func() {
for _nonSnake in [] {}
// ^^^^^^^^^ 💡 warn: Variable `_nonSnake` should have snake_case name, e.g. `_non_snake`
}
"#,
);
check_fix(
r#"
//- minicore: iterators
fn func() {
for nonSnake$0 in [] { nonSnake; }
}
"#,
r#"
fn func() {
for non_snake in [] { non_snake; }
}
"#,
);
}
} }

View file

@ -608,8 +608,8 @@ fn main() {
// `Never` is deliberately not defined so that it's an uninferred type. // `Never` is deliberately not defined so that it's an uninferred type.
// We ignore these to avoid triggering bugs in the analysis. // We ignore these to avoid triggering bugs in the analysis.
match Option::<Never>::None { match Option::<Never>::None {
None => (), Option::None => (),
Some(never) => match never {}, Option::Some(never) => match never {},
} }
match Option::<Never>::None { match Option::<Never>::None {
Option::Some(_never) => {}, Option::Some(_never) => {},