Merge pull request #21794 from Albab-Hasan/fix/union-naming-validation

fix: add naming convention validation for `union` types

Author: ChayimFriedman2

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);

crates/ide-diagnostics/src/handlers/incorrect_case.rs

@@ -262,6 +262,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(