mirror of
https://github.com/rust-lang/rust-analyzer
synced 2024-12-26 13:03:31 +00:00
Auto merge of #16477 - davidsemakula:trait-incorrect-case-diagnostic, r=Veykril
feat: Add incorrect case diagnostics for traits and their associated items Updates incorrect case diagnostic to: - Check traits and their associated items - Ignore trait implementations except for patterns in associated function bodies Also cleans up `hir-ty::diagnostics::decl_check` a bit (mostly to make it a bit more DRY and easier to maintain) Also fixes: #8675 and fixes: #8225
This commit is contained in:
commit
0e5766b0d8
5 changed files with 411 additions and 421 deletions
|
@ -16,11 +16,9 @@ mod case_conv;
|
|||
use std::fmt;
|
||||
|
||||
use hir_def::{
|
||||
data::adt::VariantData,
|
||||
hir::{Pat, PatId},
|
||||
src::HasSource,
|
||||
AdtId, AttrDefId, ConstId, EnumId, FunctionId, ItemContainerId, Lookup, ModuleDefId, ModuleId,
|
||||
StaticId, StructId,
|
||||
data::adt::VariantData, db::DefDatabase, hir::Pat, src::HasSource, AdtId, AttrDefId, ConstId,
|
||||
EnumId, FunctionId, ItemContainerId, Lookup, ModuleDefId, ModuleId, StaticId, StructId,
|
||||
TraitId, TypeAliasId,
|
||||
};
|
||||
use hir_expand::{
|
||||
name::{AsName, Name},
|
||||
|
@ -79,12 +77,14 @@ pub enum IdentType {
|
|||
Enum,
|
||||
Field,
|
||||
Function,
|
||||
Module,
|
||||
Parameter,
|
||||
StaticVariable,
|
||||
Structure,
|
||||
Trait,
|
||||
TypeAlias,
|
||||
Variable,
|
||||
Variant,
|
||||
Module,
|
||||
}
|
||||
|
||||
impl fmt::Display for IdentType {
|
||||
|
@ -94,12 +94,14 @@ impl fmt::Display for IdentType {
|
|||
IdentType::Enum => "Enum",
|
||||
IdentType::Field => "Field",
|
||||
IdentType::Function => "Function",
|
||||
IdentType::Module => "Module",
|
||||
IdentType::Parameter => "Parameter",
|
||||
IdentType::StaticVariable => "Static variable",
|
||||
IdentType::Structure => "Structure",
|
||||
IdentType::Trait => "Trait",
|
||||
IdentType::TypeAlias => "Type alias",
|
||||
IdentType::Variable => "Variable",
|
||||
IdentType::Variant => "Variant",
|
||||
IdentType::Module => "Module",
|
||||
};
|
||||
|
||||
repr.fmt(f)
|
||||
|
@ -136,10 +138,12 @@ impl<'a> DeclValidator<'a> {
|
|||
pub(super) fn validate_item(&mut self, item: ModuleDefId) {
|
||||
match item {
|
||||
ModuleDefId::ModuleId(module_id) => self.validate_module(module_id),
|
||||
ModuleDefId::TraitId(trait_id) => self.validate_trait(trait_id),
|
||||
ModuleDefId::FunctionId(func) => self.validate_func(func),
|
||||
ModuleDefId::AdtId(adt) => self.validate_adt(adt),
|
||||
ModuleDefId::ConstId(const_id) => self.validate_const(const_id),
|
||||
ModuleDefId::StaticId(static_id) => self.validate_static(static_id),
|
||||
ModuleDefId::TypeAliasId(type_alias_id) => self.validate_type_alias(type_alias_id),
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
|
@ -242,50 +246,46 @@ impl<'a> DeclValidator<'a> {
|
|||
|
||||
// Check the module name.
|
||||
let Some(module_name) = module_id.name(self.db.upcast()) else { return };
|
||||
let module_name_replacement =
|
||||
let Some(module_name_replacement) =
|
||||
module_name.as_str().and_then(to_lower_snake_case).map(|new_name| Replacement {
|
||||
current_name: module_name,
|
||||
suggested_text: new_name,
|
||||
expected_case: CaseType::LowerSnakeCase,
|
||||
});
|
||||
})
|
||||
else {
|
||||
return;
|
||||
};
|
||||
let module_data = &module_id.def_map(self.db.upcast())[module_id.local_id];
|
||||
let Some(module_src) = module_data.declaration_source(self.db.upcast()) else {
|
||||
return;
|
||||
};
|
||||
self.create_incorrect_case_diagnostic_for_ast_node(
|
||||
module_name_replacement,
|
||||
module_src.file_id,
|
||||
&module_src.value,
|
||||
IdentType::Module,
|
||||
);
|
||||
}
|
||||
|
||||
if let Some(module_name_replacement) = module_name_replacement {
|
||||
let module_data = &module_id.def_map(self.db.upcast())[module_id.local_id];
|
||||
let module_src = module_data.declaration_source(self.db.upcast());
|
||||
|
||||
if let Some(module_src) = module_src {
|
||||
let ast_ptr = match module_src.value.name() {
|
||||
Some(name) => name,
|
||||
None => {
|
||||
never!(
|
||||
"Replacement ({:?}) was generated for a module without a name: {:?}",
|
||||
module_name_replacement,
|
||||
module_src
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let diagnostic = IncorrectCase {
|
||||
file: module_src.file_id,
|
||||
ident_type: IdentType::Module,
|
||||
ident: AstPtr::new(&ast_ptr),
|
||||
expected_case: module_name_replacement.expected_case,
|
||||
ident_text: module_name_replacement
|
||||
.current_name
|
||||
.display(self.db.upcast())
|
||||
.to_string(),
|
||||
suggested_text: module_name_replacement.suggested_text,
|
||||
};
|
||||
|
||||
self.sink.push(diagnostic);
|
||||
}
|
||||
fn validate_trait(&mut self, trait_id: TraitId) {
|
||||
// Check whether non-snake case identifiers are allowed for this trait.
|
||||
if self.allowed(trait_id.into(), allow::NON_CAMEL_CASE_TYPES, false) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check the trait name.
|
||||
let data = self.db.trait_data(trait_id);
|
||||
self.create_incorrect_case_diagnostic_for_item_name(
|
||||
trait_id,
|
||||
&data.name,
|
||||
CaseType::UpperCamelCase,
|
||||
IdentType::Trait,
|
||||
);
|
||||
}
|
||||
|
||||
fn validate_func(&mut self, func: FunctionId) {
|
||||
let data = self.db.function_data(func);
|
||||
if matches!(func.lookup(self.db.upcast()).container, ItemContainerId::ExternBlockId(_)) {
|
||||
let container = func.lookup(self.db.upcast()).container;
|
||||
if matches!(container, ItemContainerId::ExternBlockId(_)) {
|
||||
cov_mark::hit!(extern_func_incorrect_case_ignored);
|
||||
return;
|
||||
}
|
||||
|
@ -296,270 +296,173 @@ impl<'a> DeclValidator<'a> {
|
|||
}
|
||||
|
||||
// Check the function name.
|
||||
let function_name = data.name.display(self.db.upcast()).to_string();
|
||||
let fn_name_replacement = to_lower_snake_case(&function_name).map(|new_name| Replacement {
|
||||
current_name: data.name.clone(),
|
||||
suggested_text: new_name,
|
||||
expected_case: CaseType::LowerSnakeCase,
|
||||
});
|
||||
|
||||
let body = self.db.body(func.into());
|
||||
// Skipped if function is an associated item of a trait implementation.
|
||||
if !self.is_trait_impl_container(container) {
|
||||
let data = self.db.function_data(func);
|
||||
self.create_incorrect_case_diagnostic_for_item_name(
|
||||
func,
|
||||
&data.name,
|
||||
CaseType::LowerSnakeCase,
|
||||
IdentType::Function,
|
||||
);
|
||||
} else {
|
||||
cov_mark::hit!(trait_impl_assoc_func_name_incorrect_case_ignored);
|
||||
}
|
||||
|
||||
// Check the patterns inside the function body.
|
||||
// This includes function parameters.
|
||||
let pats_replacements = body
|
||||
self.validate_func_body(func);
|
||||
}
|
||||
|
||||
/// Check incorrect names for patterns inside the function body.
|
||||
/// This includes function parameters except for trait implementation associated functions.
|
||||
fn validate_func_body(&mut self, func: FunctionId) {
|
||||
let body = self.db.body(func.into());
|
||||
let mut pats_replacements = body
|
||||
.pats
|
||||
.iter()
|
||||
.filter_map(|(pat_id, pat)| match pat {
|
||||
Pat::Bind { id, .. } => Some((pat_id, &body.bindings[*id].name)),
|
||||
Pat::Bind { id, .. } => {
|
||||
let bind_name = &body.bindings[*id].name;
|
||||
let replacement = Replacement {
|
||||
current_name: bind_name.clone(),
|
||||
suggested_text: to_lower_snake_case(&bind_name.to_smol_str())?,
|
||||
expected_case: CaseType::LowerSnakeCase,
|
||||
};
|
||||
Some((pat_id, replacement))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.filter_map(|(id, bind_name)| {
|
||||
Some((
|
||||
id,
|
||||
Replacement {
|
||||
current_name: bind_name.clone(),
|
||||
suggested_text: to_lower_snake_case(
|
||||
&bind_name.display(self.db.upcast()).to_string(),
|
||||
)?,
|
||||
expected_case: CaseType::LowerSnakeCase,
|
||||
},
|
||||
))
|
||||
})
|
||||
.collect();
|
||||
.peekable();
|
||||
|
||||
// If there is at least one element to spawn a warning on, go to the source map and generate a warning.
|
||||
if let Some(fn_name_replacement) = fn_name_replacement {
|
||||
self.create_incorrect_case_diagnostic_for_func(func, fn_name_replacement);
|
||||
}
|
||||
|
||||
self.create_incorrect_case_diagnostic_for_variables(func, pats_replacements);
|
||||
}
|
||||
|
||||
/// Given the information about incorrect names in the function declaration, looks up into the source code
|
||||
/// for exact locations and adds diagnostics into the sink.
|
||||
fn create_incorrect_case_diagnostic_for_func(
|
||||
&mut self,
|
||||
func: FunctionId,
|
||||
fn_name_replacement: Replacement,
|
||||
) {
|
||||
let fn_loc = func.lookup(self.db.upcast());
|
||||
let fn_src = fn_loc.source(self.db.upcast());
|
||||
|
||||
// Diagnostic for function name.
|
||||
let ast_ptr = match fn_src.value.name() {
|
||||
Some(name) => name,
|
||||
None => {
|
||||
never!(
|
||||
"Replacement ({:?}) was generated for a function without a name: {:?}",
|
||||
fn_name_replacement,
|
||||
fn_src
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let diagnostic = IncorrectCase {
|
||||
file: fn_src.file_id,
|
||||
ident_type: IdentType::Function,
|
||||
ident: AstPtr::new(&ast_ptr),
|
||||
expected_case: fn_name_replacement.expected_case,
|
||||
ident_text: fn_name_replacement.current_name.display(self.db.upcast()).to_string(),
|
||||
suggested_text: fn_name_replacement.suggested_text,
|
||||
};
|
||||
|
||||
self.sink.push(diagnostic);
|
||||
}
|
||||
|
||||
/// Given the information about incorrect variable names, looks up into the source code
|
||||
/// for exact locations and adds diagnostics into the sink.
|
||||
fn create_incorrect_case_diagnostic_for_variables(
|
||||
&mut self,
|
||||
func: FunctionId,
|
||||
pats_replacements: Vec<(PatId, Replacement)>,
|
||||
) {
|
||||
// XXX: only look at source_map if we do have missing fields
|
||||
if pats_replacements.is_empty() {
|
||||
if pats_replacements.peek().is_none() {
|
||||
return;
|
||||
}
|
||||
|
||||
let (_, source_map) = self.db.body_with_source_map(func.into());
|
||||
|
||||
for (id, replacement) in pats_replacements {
|
||||
if let Ok(source_ptr) = source_map.pat_syntax(id) {
|
||||
if let Some(ptr) = source_ptr.value.cast::<ast::IdentPat>() {
|
||||
let root = source_ptr.file_syntax(self.db.upcast());
|
||||
let ident_pat = ptr.to_node(&root);
|
||||
let parent = match ident_pat.syntax().parent() {
|
||||
Some(parent) => parent,
|
||||
None => continue,
|
||||
};
|
||||
let name_ast = match ident_pat.name() {
|
||||
Some(name_ast) => name_ast,
|
||||
None => continue,
|
||||
};
|
||||
let Ok(source_ptr) = source_map.pat_syntax(id) else {
|
||||
continue;
|
||||
};
|
||||
let Some(ptr) = source_ptr.value.cast::<ast::IdentPat>() else {
|
||||
continue;
|
||||
};
|
||||
let root = source_ptr.file_syntax(self.db.upcast());
|
||||
let ident_pat = ptr.to_node(&root);
|
||||
let Some(parent) = ident_pat.syntax().parent() else {
|
||||
continue;
|
||||
};
|
||||
|
||||
let is_param = ast::Param::can_cast(parent.kind());
|
||||
|
||||
// We have to check that it's either `let var = ...` or `var @ Variant(_)` statement,
|
||||
// because e.g. match arms are patterns as well.
|
||||
// In other words, we check that it's a named variable binding.
|
||||
let is_binding = ast::LetStmt::can_cast(parent.kind())
|
||||
|| (ast::MatchArm::can_cast(parent.kind())
|
||||
&& ident_pat.at_token().is_some());
|
||||
if !(is_param || is_binding) {
|
||||
// This pattern is not an actual variable declaration, e.g. `Some(val) => {..}` match arm.
|
||||
continue;
|
||||
}
|
||||
|
||||
let ident_type =
|
||||
if is_param { IdentType::Parameter } else { IdentType::Variable };
|
||||
|
||||
let diagnostic = IncorrectCase {
|
||||
file: source_ptr.file_id,
|
||||
ident_type,
|
||||
ident: AstPtr::new(&name_ast),
|
||||
expected_case: replacement.expected_case,
|
||||
ident_text: replacement.current_name.display(self.db.upcast()).to_string(),
|
||||
suggested_text: replacement.suggested_text,
|
||||
};
|
||||
|
||||
self.sink.push(diagnostic);
|
||||
}
|
||||
let is_param = ast::Param::can_cast(parent.kind());
|
||||
// We have to check that it's either `let var = ...` or `var @ Variant(_)` statement,
|
||||
// because e.g. match arms are patterns as well.
|
||||
// In other words, we check that it's a named variable binding.
|
||||
let is_binding = ast::LetStmt::can_cast(parent.kind())
|
||||
|| (ast::MatchArm::can_cast(parent.kind()) && ident_pat.at_token().is_some());
|
||||
if !(is_param || is_binding) {
|
||||
// This pattern is not an actual variable declaration, e.g. `Some(val) => {..}` match arm.
|
||||
continue;
|
||||
}
|
||||
|
||||
let ident_type = if is_param { IdentType::Parameter } else { IdentType::Variable };
|
||||
|
||||
self.create_incorrect_case_diagnostic_for_ast_node(
|
||||
replacement,
|
||||
source_ptr.file_id,
|
||||
&ident_pat,
|
||||
ident_type,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_struct(&mut self, struct_id: StructId) {
|
||||
let data = self.db.struct_data(struct_id);
|
||||
|
||||
// Check the structure name.
|
||||
let non_camel_case_allowed =
|
||||
self.allowed(struct_id.into(), allow::NON_CAMEL_CASE_TYPES, false);
|
||||
let non_snake_case_allowed = self.allowed(struct_id.into(), allow::NON_SNAKE_CASE, false);
|
||||
|
||||
// Check the structure name.
|
||||
let struct_name = data.name.display(self.db.upcast()).to_string();
|
||||
let struct_name_replacement = if !non_camel_case_allowed {
|
||||
to_camel_case(&struct_name).map(|new_name| Replacement {
|
||||
current_name: data.name.clone(),
|
||||
suggested_text: new_name,
|
||||
expected_case: CaseType::UpperCamelCase,
|
||||
})
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
// Check the field names.
|
||||
let mut struct_fields_replacements = Vec::new();
|
||||
|
||||
if !non_snake_case_allowed {
|
||||
if let VariantData::Record(fields) = data.variant_data.as_ref() {
|
||||
for (_, field) in fields.iter() {
|
||||
let field_name = field.name.display(self.db.upcast()).to_string();
|
||||
if let Some(new_name) = to_lower_snake_case(&field_name) {
|
||||
let replacement = Replacement {
|
||||
current_name: field.name.clone(),
|
||||
suggested_text: new_name,
|
||||
expected_case: CaseType::LowerSnakeCase,
|
||||
};
|
||||
struct_fields_replacements.push(replacement);
|
||||
}
|
||||
}
|
||||
}
|
||||
if !non_camel_case_allowed {
|
||||
let data = self.db.struct_data(struct_id);
|
||||
self.create_incorrect_case_diagnostic_for_item_name(
|
||||
struct_id,
|
||||
&data.name,
|
||||
CaseType::UpperCamelCase,
|
||||
IdentType::Structure,
|
||||
);
|
||||
}
|
||||
|
||||
// If there is at least one element to spawn a warning on, go to the source map and generate a warning.
|
||||
self.create_incorrect_case_diagnostic_for_struct(
|
||||
struct_id,
|
||||
struct_name_replacement,
|
||||
struct_fields_replacements,
|
||||
);
|
||||
// Check the field names.
|
||||
self.validate_struct_fields(struct_id);
|
||||
}
|
||||
|
||||
/// Given the information about incorrect names in the struct declaration, looks up into the source code
|
||||
/// for exact locations and adds diagnostics into the sink.
|
||||
fn create_incorrect_case_diagnostic_for_struct(
|
||||
&mut self,
|
||||
struct_id: StructId,
|
||||
struct_name_replacement: Option<Replacement>,
|
||||
struct_fields_replacements: Vec<Replacement>,
|
||||
) {
|
||||
/// Check incorrect names for struct fields.
|
||||
fn validate_struct_fields(&mut self, struct_id: StructId) {
|
||||
if self.allowed(struct_id.into(), allow::NON_SNAKE_CASE, false) {
|
||||
return;
|
||||
}
|
||||
|
||||
let data = self.db.struct_data(struct_id);
|
||||
let VariantData::Record(fields) = data.variant_data.as_ref() else {
|
||||
return;
|
||||
};
|
||||
let mut struct_fields_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 struct_name_replacement.is_none() && struct_fields_replacements.is_empty() {
|
||||
if struct_fields_replacements.peek().is_none() {
|
||||
return;
|
||||
}
|
||||
|
||||
let struct_loc = struct_id.lookup(self.db.upcast());
|
||||
let struct_src = struct_loc.source(self.db.upcast());
|
||||
|
||||
if let Some(replacement) = struct_name_replacement {
|
||||
let ast_ptr = match struct_src.value.name() {
|
||||
Some(name) => name,
|
||||
None => {
|
||||
let Some(ast::FieldList::RecordFieldList(struct_fields_list)) =
|
||||
struct_src.value.field_list()
|
||||
else {
|
||||
always!(
|
||||
struct_fields_replacements.peek().is_none(),
|
||||
"Replacements ({:?}) were generated for a structure fields \
|
||||
which had no fields list: {:?}",
|
||||
struct_fields_replacements.collect::<Vec<_>>(),
|
||||
struct_src
|
||||
);
|
||||
return;
|
||||
};
|
||||
let mut struct_fields_iter = struct_fields_list.fields();
|
||||
for field_replacement in struct_fields_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) = struct_fields_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 a structure without a name: {:?}",
|
||||
replacement,
|
||||
"Replacement ({:?}) was generated for a structure field \
|
||||
which was not found: {:?}",
|
||||
field_replacement,
|
||||
struct_src
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let diagnostic = IncorrectCase {
|
||||
file: struct_src.file_id,
|
||||
ident_type: IdentType::Structure,
|
||||
ident: AstPtr::new(&ast_ptr),
|
||||
expected_case: replacement.expected_case,
|
||||
ident_text: replacement.current_name.display(self.db.upcast()).to_string(),
|
||||
suggested_text: replacement.suggested_text,
|
||||
};
|
||||
|
||||
self.sink.push(diagnostic);
|
||||
}
|
||||
|
||||
let struct_fields_list = match struct_src.value.field_list() {
|
||||
Some(ast::FieldList::RecordFieldList(fields)) => fields,
|
||||
_ => {
|
||||
always!(
|
||||
struct_fields_replacements.is_empty(),
|
||||
"Replacements ({:?}) were generated for a structure fields which had no fields list: {:?}",
|
||||
struct_fields_replacements,
|
||||
struct_src
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let mut struct_fields_iter = struct_fields_list.fields();
|
||||
for field_to_rename in struct_fields_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 ast_ptr = loop {
|
||||
match struct_fields_iter.next().and_then(|field| field.name()) {
|
||||
Some(field_name) => {
|
||||
if field_name.as_name() == field_to_rename.current_name {
|
||||
break field_name;
|
||||
}
|
||||
}
|
||||
None => {
|
||||
never!(
|
||||
"Replacement ({:?}) was generated for a structure field which was not found: {:?}",
|
||||
field_to_rename, struct_src
|
||||
);
|
||||
return;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let diagnostic = IncorrectCase {
|
||||
file: struct_src.file_id,
|
||||
ident_type: IdentType::Field,
|
||||
ident: AstPtr::new(&ast_ptr),
|
||||
expected_case: field_to_rename.expected_case,
|
||||
ident_text: field_to_rename.current_name.display(self.db.upcast()).to_string(),
|
||||
suggested_text: field_to_rename.suggested_text,
|
||||
};
|
||||
|
||||
self.sink.push(diagnostic);
|
||||
self.create_incorrect_case_diagnostic_for_ast_node(
|
||||
field_replacement,
|
||||
struct_src.file_id,
|
||||
&field,
|
||||
IdentType::Field,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -572,163 +475,103 @@ impl<'a> DeclValidator<'a> {
|
|||
}
|
||||
|
||||
// Check the enum name.
|
||||
let enum_name = data.name.display(self.db.upcast()).to_string();
|
||||
let enum_name_replacement = to_camel_case(&enum_name).map(|new_name| Replacement {
|
||||
current_name: data.name.clone(),
|
||||
suggested_text: new_name,
|
||||
expected_case: CaseType::UpperCamelCase,
|
||||
});
|
||||
self.create_incorrect_case_diagnostic_for_item_name(
|
||||
enum_id,
|
||||
&data.name,
|
||||
CaseType::UpperCamelCase,
|
||||
IdentType::Enum,
|
||||
);
|
||||
|
||||
// Check the field names.
|
||||
let enum_fields_replacements = data
|
||||
// Check the variant names.
|
||||
self.validate_enum_variants(enum_id)
|
||||
}
|
||||
|
||||
/// Check incorrect names for enum variants.
|
||||
fn validate_enum_variants(&mut self, enum_id: EnumId) {
|
||||
let data = self.db.enum_data(enum_id);
|
||||
let mut enum_variants_replacements = data
|
||||
.variants
|
||||
.iter()
|
||||
.filter_map(|(_, name)| {
|
||||
Some(Replacement {
|
||||
to_camel_case(&name.to_smol_str()).map(|new_name| Replacement {
|
||||
current_name: name.clone(),
|
||||
suggested_text: to_camel_case(&name.to_smol_str())?,
|
||||
suggested_text: new_name,
|
||||
expected_case: CaseType::UpperCamelCase,
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
.peekable();
|
||||
|
||||
// If there is at least one element to spawn a warning on, go to the source map and generate a warning.
|
||||
self.create_incorrect_case_diagnostic_for_enum(
|
||||
enum_id,
|
||||
enum_name_replacement,
|
||||
enum_fields_replacements,
|
||||
)
|
||||
}
|
||||
|
||||
/// Given the information about incorrect names in the struct declaration, looks up into the source code
|
||||
/// for exact locations and adds diagnostics into the sink.
|
||||
fn create_incorrect_case_diagnostic_for_enum(
|
||||
&mut self,
|
||||
enum_id: EnumId,
|
||||
enum_name_replacement: Option<Replacement>,
|
||||
enum_variants_replacements: Vec<Replacement>,
|
||||
) {
|
||||
// XXX: only look at sources if we do have incorrect names
|
||||
if enum_name_replacement.is_none() && enum_variants_replacements.is_empty() {
|
||||
if enum_variants_replacements.peek().is_none() {
|
||||
return;
|
||||
}
|
||||
|
||||
let enum_loc = enum_id.lookup(self.db.upcast());
|
||||
let enum_src = enum_loc.source(self.db.upcast());
|
||||
|
||||
if let Some(replacement) = enum_name_replacement {
|
||||
let ast_ptr = match enum_src.value.name() {
|
||||
Some(name) => name,
|
||||
None => {
|
||||
let Some(enum_variants_list) = enum_src.value.variant_list() else {
|
||||
always!(
|
||||
enum_variants_replacements.peek().is_none(),
|
||||
"Replacements ({:?}) were generated for enum variants \
|
||||
which had no fields list: {:?}",
|
||||
enum_variants_replacements,
|
||||
enum_src
|
||||
);
|
||||
return;
|
||||
};
|
||||
let mut enum_variants_iter = enum_variants_list.variants();
|
||||
for variant_replacement in enum_variants_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 variant = loop {
|
||||
if let Some(variant) = enum_variants_iter.next() {
|
||||
let Some(variant_name) = variant.name() else {
|
||||
continue;
|
||||
};
|
||||
if variant_name.as_name() == variant_replacement.current_name {
|
||||
break variant;
|
||||
}
|
||||
} else {
|
||||
never!(
|
||||
"Replacement ({:?}) was generated for a enum without a name: {:?}",
|
||||
replacement,
|
||||
"Replacement ({:?}) was generated for an enum variant \
|
||||
which was not found: {:?}",
|
||||
variant_replacement,
|
||||
enum_src
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let diagnostic = IncorrectCase {
|
||||
file: enum_src.file_id,
|
||||
ident_type: IdentType::Enum,
|
||||
ident: AstPtr::new(&ast_ptr),
|
||||
expected_case: replacement.expected_case,
|
||||
ident_text: replacement.current_name.display(self.db.upcast()).to_string(),
|
||||
suggested_text: replacement.suggested_text,
|
||||
};
|
||||
|
||||
self.sink.push(diagnostic);
|
||||
}
|
||||
|
||||
let enum_variants_list = match enum_src.value.variant_list() {
|
||||
Some(variants) => variants,
|
||||
_ => {
|
||||
always!(
|
||||
enum_variants_replacements.is_empty(),
|
||||
"Replacements ({:?}) were generated for a enum variants which had no fields list: {:?}",
|
||||
enum_variants_replacements,
|
||||
enum_src
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let mut enum_variants_iter = enum_variants_list.variants();
|
||||
for variant_to_rename in enum_variants_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 ast_ptr = loop {
|
||||
match enum_variants_iter.next().and_then(|v| v.name()) {
|
||||
Some(variant_name) => {
|
||||
if variant_name.as_name() == variant_to_rename.current_name {
|
||||
break variant_name;
|
||||
}
|
||||
}
|
||||
None => {
|
||||
never!(
|
||||
"Replacement ({:?}) was generated for a enum variant which was not found: {:?}",
|
||||
variant_to_rename, enum_src
|
||||
);
|
||||
return;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let diagnostic = IncorrectCase {
|
||||
file: enum_src.file_id,
|
||||
ident_type: IdentType::Variant,
|
||||
ident: AstPtr::new(&ast_ptr),
|
||||
expected_case: variant_to_rename.expected_case,
|
||||
ident_text: variant_to_rename.current_name.display(self.db.upcast()).to_string(),
|
||||
suggested_text: variant_to_rename.suggested_text,
|
||||
};
|
||||
|
||||
self.sink.push(diagnostic);
|
||||
self.create_incorrect_case_diagnostic_for_ast_node(
|
||||
variant_replacement,
|
||||
enum_src.file_id,
|
||||
&variant,
|
||||
IdentType::Variant,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_const(&mut self, const_id: ConstId) {
|
||||
let data = self.db.const_data(const_id);
|
||||
let container = const_id.lookup(self.db.upcast()).container;
|
||||
if self.is_trait_impl_container(container) {
|
||||
cov_mark::hit!(trait_impl_assoc_const_incorrect_case_ignored);
|
||||
return;
|
||||
}
|
||||
|
||||
if self.allowed(const_id.into(), allow::NON_UPPER_CASE_GLOBAL, false) {
|
||||
return;
|
||||
}
|
||||
|
||||
let name = match &data.name {
|
||||
Some(name) => name,
|
||||
None => return,
|
||||
};
|
||||
|
||||
let const_name = name.to_smol_str();
|
||||
let replacement = if let Some(new_name) = to_upper_snake_case(&const_name) {
|
||||
Replacement {
|
||||
current_name: name.clone(),
|
||||
suggested_text: new_name,
|
||||
expected_case: CaseType::UpperSnakeCase,
|
||||
}
|
||||
} else {
|
||||
// Nothing to do here.
|
||||
let data = self.db.const_data(const_id);
|
||||
let Some(name) = &data.name else {
|
||||
return;
|
||||
};
|
||||
|
||||
let const_loc = const_id.lookup(self.db.upcast());
|
||||
let const_src = const_loc.source(self.db.upcast());
|
||||
|
||||
let ast_ptr = match const_src.value.name() {
|
||||
Some(name) => name,
|
||||
None => return,
|
||||
};
|
||||
|
||||
let diagnostic = IncorrectCase {
|
||||
file: const_src.file_id,
|
||||
ident_type: IdentType::Constant,
|
||||
ident: AstPtr::new(&ast_ptr),
|
||||
expected_case: replacement.expected_case,
|
||||
ident_text: replacement.current_name.display(self.db.upcast()).to_string(),
|
||||
suggested_text: replacement.suggested_text,
|
||||
};
|
||||
|
||||
self.sink.push(diagnostic);
|
||||
self.create_incorrect_case_diagnostic_for_item_name(
|
||||
const_id,
|
||||
name,
|
||||
CaseType::UpperSnakeCase,
|
||||
IdentType::Constant,
|
||||
);
|
||||
}
|
||||
|
||||
fn validate_static(&mut self, static_id: StaticId) {
|
||||
|
@ -742,32 +585,91 @@ impl<'a> DeclValidator<'a> {
|
|||
return;
|
||||
}
|
||||
|
||||
let name = &data.name;
|
||||
self.create_incorrect_case_diagnostic_for_item_name(
|
||||
static_id,
|
||||
&data.name,
|
||||
CaseType::UpperSnakeCase,
|
||||
IdentType::StaticVariable,
|
||||
);
|
||||
}
|
||||
|
||||
let static_name = name.to_smol_str();
|
||||
let replacement = if let Some(new_name) = to_upper_snake_case(&static_name) {
|
||||
Replacement {
|
||||
current_name: name.clone(),
|
||||
suggested_text: new_name,
|
||||
expected_case: CaseType::UpperSnakeCase,
|
||||
}
|
||||
} else {
|
||||
// Nothing to do here.
|
||||
fn validate_type_alias(&mut self, type_alias_id: TypeAliasId) {
|
||||
let container = type_alias_id.lookup(self.db.upcast()).container;
|
||||
if self.is_trait_impl_container(container) {
|
||||
cov_mark::hit!(trait_impl_assoc_type_incorrect_case_ignored);
|
||||
return;
|
||||
}
|
||||
|
||||
// Check whether non-snake case identifiers are allowed for this type alias.
|
||||
if self.allowed(type_alias_id.into(), allow::NON_CAMEL_CASE_TYPES, false) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check the type alias name.
|
||||
let data = self.db.type_alias_data(type_alias_id);
|
||||
self.create_incorrect_case_diagnostic_for_item_name(
|
||||
type_alias_id,
|
||||
&data.name,
|
||||
CaseType::UpperCamelCase,
|
||||
IdentType::TypeAlias,
|
||||
);
|
||||
}
|
||||
|
||||
fn create_incorrect_case_diagnostic_for_item_name<N, S, L>(
|
||||
&mut self,
|
||||
item_id: L,
|
||||
name: &Name,
|
||||
expected_case: CaseType,
|
||||
ident_type: IdentType,
|
||||
) where
|
||||
N: AstNode + HasName + fmt::Debug,
|
||||
S: HasSource<Value = N>,
|
||||
L: Lookup<Data = S, Database<'a> = dyn DefDatabase + 'a>,
|
||||
{
|
||||
let to_expected_case_type = match expected_case {
|
||||
CaseType::LowerSnakeCase => to_lower_snake_case,
|
||||
CaseType::UpperSnakeCase => to_upper_snake_case,
|
||||
CaseType::UpperCamelCase => to_camel_case,
|
||||
};
|
||||
let Some(replacement) = to_expected_case_type(&name.to_smol_str()).map(|new_name| {
|
||||
Replacement { current_name: name.clone(), suggested_text: new_name, expected_case }
|
||||
}) else {
|
||||
return;
|
||||
};
|
||||
|
||||
let static_loc = static_id.lookup(self.db.upcast());
|
||||
let static_src = static_loc.source(self.db.upcast());
|
||||
let item_loc = item_id.lookup(self.db.upcast());
|
||||
let item_src = item_loc.source(self.db.upcast());
|
||||
self.create_incorrect_case_diagnostic_for_ast_node(
|
||||
replacement,
|
||||
item_src.file_id,
|
||||
&item_src.value,
|
||||
ident_type,
|
||||
);
|
||||
}
|
||||
|
||||
let ast_ptr = match static_src.value.name() {
|
||||
Some(name) => name,
|
||||
None => return,
|
||||
fn create_incorrect_case_diagnostic_for_ast_node<T>(
|
||||
&mut self,
|
||||
replacement: Replacement,
|
||||
file_id: HirFileId,
|
||||
node: &T,
|
||||
ident_type: IdentType,
|
||||
) where
|
||||
T: AstNode + HasName + fmt::Debug,
|
||||
{
|
||||
let Some(name_ast) = node.name() else {
|
||||
never!(
|
||||
"Replacement ({:?}) was generated for a {:?} without a name: {:?}",
|
||||
replacement,
|
||||
ident_type,
|
||||
node
|
||||
);
|
||||
return;
|
||||
};
|
||||
|
||||
let diagnostic = IncorrectCase {
|
||||
file: static_src.file_id,
|
||||
ident_type: IdentType::StaticVariable,
|
||||
ident: AstPtr::new(&ast_ptr),
|
||||
file: file_id,
|
||||
ident_type,
|
||||
ident: AstPtr::new(&name_ast),
|
||||
expected_case: replacement.expected_case,
|
||||
ident_text: replacement.current_name.display(self.db.upcast()).to_string(),
|
||||
suggested_text: replacement.suggested_text,
|
||||
|
@ -775,4 +677,13 @@ impl<'a> DeclValidator<'a> {
|
|||
|
||||
self.sink.push(diagnostic);
|
||||
}
|
||||
|
||||
fn is_trait_impl_container(&self, container_id: ItemContainerId) -> bool {
|
||||
if let ItemContainerId::ImplId(impl_id) = container_id {
|
||||
if self.db.impl_trait(impl_id).is_some() {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
}
|
||||
|
|
|
@ -563,6 +563,11 @@ impl Module {
|
|||
for diag in db.trait_data_with_diagnostics(t.id).1.iter() {
|
||||
emit_def_diagnostic(db, acc, diag);
|
||||
}
|
||||
|
||||
for item in t.items(db) {
|
||||
item.diagnostics(db, acc);
|
||||
}
|
||||
|
||||
acc.extend(def.diagnostics(db))
|
||||
}
|
||||
ModuleDef::Adt(adt) => {
|
||||
|
@ -730,13 +735,7 @@ impl Module {
|
|||
}
|
||||
|
||||
for &item in &db.impl_data(impl_def.id).items {
|
||||
let def: DefWithBody = match AssocItem::from(item) {
|
||||
AssocItem::Function(it) => it.into(),
|
||||
AssocItem::Const(it) => it.into(),
|
||||
AssocItem::TypeAlias(_) => continue,
|
||||
};
|
||||
|
||||
def.diagnostics(db, acc);
|
||||
AssocItem::from(item).diagnostics(db, acc);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -2651,6 +2650,22 @@ impl AssocItem {
|
|||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) {
|
||||
match self {
|
||||
AssocItem::Function(func) => {
|
||||
DefWithBody::from(func).diagnostics(db, acc);
|
||||
}
|
||||
AssocItem::Const(const_) => {
|
||||
DefWithBody::from(const_).diagnostics(db, acc);
|
||||
}
|
||||
AssocItem::TypeAlias(type_alias) => {
|
||||
for diag in hir_ty::diagnostics::incorrect_case(db, type_alias.id.into()) {
|
||||
acc.push(diag.into());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl HasVisibility for AssocItem {
|
||||
|
|
|
@ -52,7 +52,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::IncorrectCase) -> Option<Vec<Ass
|
|||
|
||||
#[cfg(test)]
|
||||
mod change_case {
|
||||
use crate::tests::{check_diagnostics, check_fix};
|
||||
use crate::tests::{check_diagnostics, check_diagnostics_with_disabled, check_fix};
|
||||
|
||||
#[test]
|
||||
fn test_rename_incorrect_case() {
|
||||
|
@ -388,14 +388,13 @@ mod F {
|
|||
|
||||
#[test]
|
||||
fn complex_ignore() {
|
||||
// FIXME: this should trigger errors for the second case.
|
||||
check_diagnostics(
|
||||
r#"
|
||||
trait T { fn a(); }
|
||||
struct U {}
|
||||
impl T for U {
|
||||
fn a() {
|
||||
#[allow(non_snake_case)]
|
||||
#[allow(non_snake_case, non_upper_case_globals)]
|
||||
trait __BitFlagsOk {
|
||||
const HiImAlsoBad: u8 = 2;
|
||||
fn Dirty(&self) -> bool { false }
|
||||
|
@ -403,7 +402,9 @@ impl T for U {
|
|||
|
||||
trait __BitFlagsBad {
|
||||
const HiImAlsoBad: u8 = 2;
|
||||
// ^^^^^^^^^^^ 💡 warn: Constant `HiImAlsoBad` should have UPPER_SNAKE_CASE name, e.g. `HI_IM_ALSO_BAD`
|
||||
fn Dirty(&self) -> bool { false }
|
||||
// ^^^^^💡 warn: Function `Dirty` should have snake_case name, e.g. `dirty`
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -462,19 +463,59 @@ extern {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn bug_traits_arent_checked() {
|
||||
// FIXME: Traits and functions in traits aren't currently checked by
|
||||
// r-a, even though rustc will complain about them.
|
||||
fn incorrect_trait_and_assoc_item_names() {
|
||||
check_diagnostics(
|
||||
r#"
|
||||
trait BAD_TRAIT {
|
||||
// ^^^^^^^^^ 💡 warn: Trait `BAD_TRAIT` should have CamelCase name, e.g. `BadTrait`
|
||||
const bad_const: u8;
|
||||
// ^^^^^^^^^ 💡 warn: Constant `bad_const` should have UPPER_SNAKE_CASE name, e.g. `BAD_CONST`
|
||||
type BAD_TYPE;
|
||||
// ^^^^^^^^ 💡 warn: Type alias `BAD_TYPE` should have CamelCase name, e.g. `BadType`
|
||||
fn BAD_FUNCTION();
|
||||
// ^^^^^^^^^^^^ 💡 warn: Function `BAD_FUNCTION` should have snake_case name, e.g. `bad_function`
|
||||
fn BadFunction();
|
||||
// ^^^^^^^^^^^ 💡 warn: Function `BadFunction` should have snake_case name, e.g. `bad_function`
|
||||
}
|
||||
"#,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_diagnostics_for_trait_impl_assoc_items_except_pats_in_body() {
|
||||
cov_mark::check!(trait_impl_assoc_const_incorrect_case_ignored);
|
||||
cov_mark::check!(trait_impl_assoc_type_incorrect_case_ignored);
|
||||
cov_mark::check_count!(trait_impl_assoc_func_name_incorrect_case_ignored, 2);
|
||||
check_diagnostics_with_disabled(
|
||||
r#"
|
||||
trait BAD_TRAIT {
|
||||
// ^^^^^^^^^ 💡 warn: Trait `BAD_TRAIT` should have CamelCase name, e.g. `BadTrait`
|
||||
const bad_const: u8;
|
||||
// ^^^^^^^^^ 💡 warn: Constant `bad_const` should have UPPER_SNAKE_CASE name, e.g. `BAD_CONST`
|
||||
type BAD_TYPE;
|
||||
// ^^^^^^^^ 💡 warn: Type alias `BAD_TYPE` should have CamelCase name, e.g. `BadType`
|
||||
fn BAD_FUNCTION(BAD_PARAM: u8);
|
||||
// ^^^^^^^^^^^^ 💡 warn: Function `BAD_FUNCTION` should have snake_case name, e.g. `bad_function`
|
||||
// ^^^^^^^^^ 💡 warn: Parameter `BAD_PARAM` should have snake_case name, e.g. `bad_param`
|
||||
fn BadFunction();
|
||||
// ^^^^^^^^^^^ 💡 warn: Function `BadFunction` should have snake_case name, e.g. `bad_function`
|
||||
}
|
||||
|
||||
impl BAD_TRAIT for () {
|
||||
const bad_const: u8 = 0;
|
||||
type BAD_TYPE = ();
|
||||
fn BAD_FUNCTION(BAD_PARAM: u8) {
|
||||
// ^^^^^^^^^ 💡 warn: Parameter `BAD_PARAM` should have snake_case name, e.g. `bad_param`
|
||||
let BAD_VAR = 0;
|
||||
// ^^^^^^^ 💡 warn: Variable `BAD_VAR` should have snake_case name, e.g. `bad_var`
|
||||
}
|
||||
fn BadFunction() {}
|
||||
}
|
||||
"#,
|
||||
std::iter::once("unused_variables".to_string()),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allow_attributes() {
|
||||
check_diagnostics(
|
||||
|
@ -519,6 +560,14 @@ pub const some_const: u8 = 10;
|
|||
|
||||
#[allow(non_upper_case_globals)]
|
||||
pub static SomeStatic: u8 = 10;
|
||||
|
||||
#[allow(non_snake_case, non_camel_case_types, non_upper_case_globals)]
|
||||
trait BAD_TRAIT {
|
||||
const bad_const: u8;
|
||||
type BAD_TYPE;
|
||||
fn BAD_FUNCTION(BAD_PARAM: u8);
|
||||
fn BadFunction();
|
||||
}
|
||||
"#,
|
||||
);
|
||||
}
|
||||
|
@ -578,6 +627,20 @@ pub const some_const: u8 = 10;
|
|||
#[deny(non_upper_case_globals)]
|
||||
pub static SomeStatic: u8 = 10;
|
||||
//^^^^^^^^^^ 💡 error: Static variable `SomeStatic` should have UPPER_SNAKE_CASE name, e.g. `SOME_STATIC`
|
||||
|
||||
#[deny(non_snake_case, non_camel_case_types, non_upper_case_globals)]
|
||||
trait BAD_TRAIT {
|
||||
// ^^^^^^^^^ 💡 error: Trait `BAD_TRAIT` should have CamelCase name, e.g. `BadTrait`
|
||||
const bad_const: u8;
|
||||
// ^^^^^^^^^ 💡 error: Constant `bad_const` should have UPPER_SNAKE_CASE name, e.g. `BAD_CONST`
|
||||
type BAD_TYPE;
|
||||
// ^^^^^^^^ 💡 error: Type alias `BAD_TYPE` should have CamelCase name, e.g. `BadType`
|
||||
fn BAD_FUNCTION(BAD_PARAM: u8);
|
||||
// ^^^^^^^^^^^^ 💡 error: Function `BAD_FUNCTION` should have snake_case name, e.g. `bad_function`
|
||||
// ^^^^^^^^^ 💡 error: Parameter `BAD_PARAM` should have snake_case name, e.g. `bad_param`
|
||||
fn BadFunction();
|
||||
// ^^^^^^^^^^^ 💡 error: Function `BadFunction` should have snake_case name, e.g. `bad_function`
|
||||
}
|
||||
"#,
|
||||
);
|
||||
}
|
||||
|
|
|
@ -199,7 +199,7 @@ fn f() {
|
|||
// future, but we shouldn't emit an argument count diagnostic here
|
||||
check_diagnostics(
|
||||
r#"
|
||||
trait Foo { fn method(&self, arg: usize) {} }
|
||||
trait Foo { fn method(&self, _arg: usize) {} }
|
||||
|
||||
fn f() {
|
||||
let x;
|
||||
|
|
|
@ -186,6 +186,7 @@ fn minicore_smoke_test() {
|
|||
let mut config = DiagnosticsConfig::test_sample();
|
||||
// This should be ignored since we conditionally remove code which creates single item use with braces
|
||||
config.disabled.insert("unused_braces".to_string());
|
||||
config.disabled.insert("unused_variables".to_string());
|
||||
check_diagnostics_with_config(config, &source);
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue