Unnamed repository; edit this file 'description' to name the repository.
fix: implement naming convention validation for `union` types.
generated with ai assistance (claude sonnet 4.6).
Albab-Hasan 2 months ago
parent a1b86d6 · commit d5fc68f
-rw-r--r--crates/hir-ty/src/diagnostics/decl_check.rs96
-rw-r--r--crates/ide-diagnostics/src/handlers/incorrect_case.rs42
2 files changed, 134 insertions, 4 deletions
diff --git a/crates/hir-ty/src/diagnostics/decl_check.rs b/crates/hir-ty/src/diagnostics/decl_check.rs
index 29da1b0c51..0931b85965 100644
--- a/crates/hir-ty/src/diagnostics/decl_check.rs
+++ b/crates/hir-ty/src/diagnostics/decl_check.rs
@@ -17,7 +17,7 @@ use std::fmt;
use hir_def::{
AdtId, ConstId, EnumId, EnumVariantId, FunctionId, HasModule, ItemContainerId, Lookup,
- ModuleDefId, ModuleId, StaticId, StructId, TraitId, TypeAliasId, attrs::AttrFlags,
+ ModuleDefId, ModuleId, StaticId, StructId, TraitId, TypeAliasId, UnionId, attrs::AttrFlags,
db::DefDatabase, hir::Pat, item_tree::FieldsShape, signatures::StaticFlags, src::HasSource,
};
use hir_expand::{
@@ -77,6 +77,7 @@ pub enum IdentType {
Structure,
Trait,
TypeAlias,
+ Union,
Variable,
Variant,
}
@@ -94,6 +95,7 @@ impl fmt::Display for IdentType {
IdentType::Structure => "Structure",
IdentType::Trait => "Trait",
IdentType::TypeAlias => "Type alias",
+ IdentType::Union => "Union",
IdentType::Variable => "Variable",
IdentType::Variant => "Variant",
};
@@ -146,9 +148,7 @@ impl<'a> DeclValidator<'a> {
match adt {
AdtId::StructId(struct_id) => self.validate_struct(struct_id),
AdtId::EnumId(enum_id) => self.validate_enum(enum_id),
- AdtId::UnionId(_) => {
- // FIXME: Unions aren't yet supported by this validator.
- }
+ AdtId::UnionId(union_id) => self.validate_union(union_id),
}
}
@@ -383,6 +383,94 @@ impl<'a> DeclValidator<'a> {
}
}
+ fn validate_union(&mut self, union_id: UnionId) {
+ // Check the union name.
+ let data = self.db.union_signature(union_id);
+
+ // rustc implementation excuses repr(C) since C unions predominantly don't
+ // use camel case.
+ let has_repr_c = AttrFlags::repr(self.db, union_id.into()).is_some_and(|repr| repr.c());
+ if !has_repr_c {
+ self.create_incorrect_case_diagnostic_for_item_name(
+ union_id,
+ &data.name,
+ CaseType::UpperCamelCase,
+ IdentType::Union,
+ );
+ }
+
+ // Check the field names.
+ self.validate_union_fields(union_id);
+ }
+
+ /// Check incorrect names for union fields.
+ fn validate_union_fields(&mut self, union_id: UnionId) {
+ let data = union_id.fields(self.db);
+ let edition = self.edition(union_id);
+ let mut union_fields_replacements = data
+ .fields()
+ .iter()
+ .filter_map(|(_, field)| {
+ to_lower_snake_case(&field.name.display_no_db(edition).to_smolstr()).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 union_fields_replacements.peek().is_none() {
+ return;
+ }
+
+ let union_loc = union_id.lookup(self.db);
+ let union_src = union_loc.source(self.db);
+
+ let Some(union_fields_list) = union_src.value.record_field_list() else {
+ always!(
+ union_fields_replacements.peek().is_none(),
+ "Replacements ({:?}) were generated for a union fields \
+ which had no fields list: {:?}",
+ union_fields_replacements.collect::<Vec<_>>(),
+ union_src
+ );
+ return;
+ };
+ let mut union_fields_iter = union_fields_list.fields();
+ for field_replacement in union_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) = union_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 union field \
+ which was not found: {:?}",
+ field_replacement,
+ union_src
+ );
+ return;
+ }
+ };
+
+ self.create_incorrect_case_diagnostic_for_ast_node(
+ field_replacement,
+ union_src.file_id,
+ &field,
+ IdentType::Field,
+ );
+ }
+ }
+
fn validate_enum(&mut self, enum_id: EnumId) {
// Check the enum name.
let data = self.db.enum_signature(enum_id);
diff --git a/crates/ide-diagnostics/src/handlers/incorrect_case.rs b/crates/ide-diagnostics/src/handlers/incorrect_case.rs
index c47449f259..5410f8b58a 100644
--- a/crates/ide-diagnostics/src/handlers/incorrect_case.rs
+++ b/crates/ide-diagnostics/src/handlers/incorrect_case.rs
@@ -263,6 +263,48 @@ struct SomeStruct { SomeField: u8 }
}
#[test]
+ fn incorrect_union_names() {
+ check_diagnostics(
+ r#"
+union non_camel_case_name { field: u8 }
+ // ^^^^^^^^^^^^^^^^^^^ 💡 warn: Union `non_camel_case_name` should have UpperCamelCase name, e.g. `NonCamelCaseName`
+
+union SCREAMING_CASE { field: u8 }
+ // ^^^^^^^^^^^^^^ 💡 warn: Union `SCREAMING_CASE` should have UpperCamelCase name, e.g. `ScreamingCase`
+"#,
+ );
+ }
+
+ #[test]
+ fn no_diagnostic_for_camel_cased_acronyms_in_union_name() {
+ check_diagnostics(
+ r#"
+union AABB { field: u8 }
+"#,
+ );
+ }
+
+ #[test]
+ fn no_diagnostic_for_repr_c_union() {
+ check_diagnostics(
+ r#"
+#[repr(C)]
+union my_union { field: u8 }
+"#,
+ );
+ }
+
+ #[test]
+ fn incorrect_union_field() {
+ check_diagnostics(
+ r#"
+union SomeUnion { SomeField: u8 }
+ // ^^^^^^^^^ 💡 warn: Field `SomeField` should have snake_case name, e.g. `some_field`
+"#,
+ );
+ }
+
+ #[test]
fn incorrect_enum_names() {
check_diagnostics(
r#"