fix: add incorrect case diagnostics for module names

rmehri01 · rmehri01 · commit 36eac9abee9d · 2023-10-09T13:36:39.000-07:00
diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs
@@ -73,6 +73,7 @@ use hir_expand::{
     db::ExpandDatabase,
     eager::expand_eager_macro_input,
     hygiene::Hygiene,
+    name::Name,
     proc_macro::ProcMacroExpander,
     AstId, ExpandError, ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind,
     MacroDefId, MacroDefKind, UnresolvedMacro,
@@ -174,6 +175,18 @@ impl ModuleId {
         self.krate
     }
 
+    pub fn name(self, db: &dyn db::DefDatabase) -> Option<Name> {
+        let def_map = self.def_map(db);
+        let parent = def_map[self.local_id].parent?;
+        def_map[parent].children.iter().find_map(|(name, module_id)| {
+            if *module_id == self.local_id {
+                Some(name.clone())
+            } else {
+                None
+            }
+        })
+    }
+
     pub fn containing_module(self, db: &dyn db::DefDatabase) -> Option<ModuleId> {
         self.def_map(db).containing_module(self.local_id)
     }
diff --git a/crates/hir-ty/src/diagnostics/decl_check.rs b/crates/hir-ty/src/diagnostics/decl_check.rs
@@ -9,6 +9,7 @@
 //! - constants (e.g. `const FOO: u8 = 10;`)
 //! - static items (e.g. `static FOO: u8 = 10;`)
 //! - match arm bindings (e.g. `foo @ Some(_)`)
+//! - modules (e.g. `mod foo { ... }` or `mod foo;`)
 
 mod case_conv;
 
@@ -19,7 +20,7 @@ use hir_def::{
     hir::{Pat, PatId},
     src::HasSource,
     AdtId, AttrDefId, ConstId, DefWithBodyId, EnumId, EnumVariantId, FunctionId, ItemContainerId,
-    Lookup, ModuleDefId, StaticId, StructId,
+    Lookup, ModuleDefId, ModuleId, StaticId, StructId,
 };
 use hir_expand::{
     name::{AsName, Name},
@@ -83,6 +84,7 @@ pub enum IdentType {
     Structure,
     Variable,
     Variant,
+    Module,
 }
 
 impl fmt::Display for IdentType {
@@ -97,6 +99,7 @@ impl fmt::Display for IdentType {
             IdentType::Structure => "Structure",
             IdentType::Variable => "Variable",
             IdentType::Variant => "Variant",
+            IdentType::Module => "Module",
         };
 
         repr.fmt(f)
@@ -132,6 +135,7 @@ 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::FunctionId(func) => self.validate_func(func),
             ModuleDefId::AdtId(adt) => self.validate_adt(adt),
             ModuleDefId::ConstId(const_id) => self.validate_const(const_id),
@@ -230,6 +234,55 @@ impl<'a> DeclValidator<'a> {
             || parent()
     }
 
+    fn validate_module(&mut self, module_id: ModuleId) {
+        // Check whether non-snake case identifiers are allowed for this module.
+        if self.allowed(module_id.into(), allow::NON_SNAKE_CASE, false) {
+            return;
+        }
+
+        // Check the module name.
+        let Some(module_name) = module_id.name(self.db.upcast()) else { return };
+        let 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,
+            });
+
+        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_func(&mut self, func: FunctionId) {
         let data = self.db.function_data(func);
         if matches!(func.lookup(self.db.upcast()).container, ItemContainerId::ExternBlockId(_)) {
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
@@ -452,15 +452,7 @@ impl HasVisibility for ModuleDef {
 impl Module {
     /// Name of this module.
     pub fn name(self, db: &dyn HirDatabase) -> Option<Name> {
-        let def_map = self.id.def_map(db.upcast());
-        let parent = def_map[self.id.local_id].parent?;
-        def_map[parent].children.iter().find_map(|(name, module_id)| {
-            if *module_id == self.id.local_id {
-                Some(name.clone())
-            } else {
-                None
-            }
-        })
+        self.id.name(db.upcast())
     }
 
     /// Returns the crate this module is part of.
@@ -571,6 +563,7 @@ impl Module {
                     if def_map[m.id.local_id].origin.is_inline() {
                         m.diagnostics(db, acc)
                     }
+                    acc.extend(def.diagnostics(db))
                 }
                 ModuleDef::Trait(t) => {
                     for diag in db.trait_data_with_diagnostics(t.id).1.iter() {
diff --git a/crates/ide-diagnostics/src/handlers/incorrect_case.rs b/crates/ide-diagnostics/src/handlers/incorrect_case.rs
@@ -111,6 +111,31 @@ fn some_fn() {
     let what_aweird_formatting = 10;
     another_func(what_aweird_formatting);
 }
+"#,
+        );
+
+        check_fix(
+            r#"
+static S: i32 = M::A;
+
+mod $0M {
+    pub const A: i32 = 10;
+}
+
+mod other {
+    use crate::M::A;
+}
+"#,
+            r#"
+static S: i32 = m::A;
+
+mod m {
+    pub const A: i32 = 10;
+}
+
+mod other {
+    use crate::m::A;
+}
 "#,
         );
     }
@@ -518,17 +543,20 @@ fn NonSnakeCaseName(some_var: u8) -> u8 {
 
 #[deny(nonstandard_style)]
 mod CheckNonstandardStyle {
+  //^^^^^^^^^^^^^^^^^^^^^ 💡 error: Module `CheckNonstandardStyle` should have snake_case name, e.g. `check_nonstandard_style`
     fn HiImABadFnName() {}
      //^^^^^^^^^^^^^^ 💡 error: Function `HiImABadFnName` should have snake_case name, e.g. `hi_im_abad_fn_name`
 }
 
 #[deny(warnings)]
 mod CheckBadStyle {
+  //^^^^^^^^^^^^^ 💡 error: Module `CheckBadStyle` should have snake_case name, e.g. `check_bad_style`
     struct fooo;
          //^^^^ 💡 error: Structure `fooo` should have CamelCase name, e.g. `Fooo`
 }
 
 mod F {
+  //^ 💡 warn: Module `F` should have snake_case name, e.g. `f`
     #![deny(non_snake_case)]
     fn CheckItWorksWithModAttr() {}
      //^^^^^^^^^^^^^^^^^^^^^^^ 💡 error: Function `CheckItWorksWithModAttr` should have snake_case name, e.g. `check_it_works_with_mod_attr`
@@ -649,4 +677,30 @@ enum E {
 "#,
         );
     }
+
+    #[test]
+    fn module_name_inline() {
+        check_diagnostics(
+            r#"
+mod M {
+  //^ 💡 warn: Module `M` should have snake_case name, e.g. `m`
+    mod IncorrectCase {}
+      //^^^^^^^^^^^^^ 💡 warn: Module `IncorrectCase` should have snake_case name, e.g. `incorrect_case`
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn module_name_decl() {
+        check_diagnostics(
+            r#"
+//- /Foo.rs
+
+//- /main.rs
+mod Foo;
+  //^^^ 💡 warn: Module `Foo` should have snake_case name, e.g. `foo`
+"#,
+        )
+    }
 }
