Skip to content

Commit bd38871

Browse files
committed
Auto merge of rust-lang#15736 - rmehri01:15678_module_incorrect_case_diagnostics, r=HKalbasi
fix: add incorrect case diagnostics for module names Adds diagnostics for checking both inline and file module names are snake case. Closes rust-lang#15678
2 parents 0cae1ca + 36eac9a commit bd38871

File tree

4 files changed

+123
-10
lines changed

4 files changed

+123
-10
lines changed

crates/hir-def/src/lib.rs

+13
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ use hir_expand::{
7373
db::ExpandDatabase,
7474
eager::expand_eager_macro_input,
7575
hygiene::Hygiene,
76+
name::Name,
7677
proc_macro::ProcMacroExpander,
7778
AstId, ExpandError, ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind,
7879
MacroDefId, MacroDefKind, UnresolvedMacro,
@@ -174,6 +175,18 @@ impl ModuleId {
174175
self.krate
175176
}
176177

178+
pub fn name(self, db: &dyn db::DefDatabase) -> Option<Name> {
179+
let def_map = self.def_map(db);
180+
let parent = def_map[self.local_id].parent?;
181+
def_map[parent].children.iter().find_map(|(name, module_id)| {
182+
if *module_id == self.local_id {
183+
Some(name.clone())
184+
} else {
185+
None
186+
}
187+
})
188+
}
189+
177190
pub fn containing_module(self, db: &dyn db::DefDatabase) -> Option<ModuleId> {
178191
self.def_map(db).containing_module(self.local_id)
179192
}

crates/hir-ty/src/diagnostics/decl_check.rs

+54-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
//! - constants (e.g. `const FOO: u8 = 10;`)
1010
//! - static items (e.g. `static FOO: u8 = 10;`)
1111
//! - match arm bindings (e.g. `foo @ Some(_)`)
12+
//! - modules (e.g. `mod foo { ... }` or `mod foo;`)
1213
1314
mod case_conv;
1415

@@ -19,7 +20,7 @@ use hir_def::{
1920
hir::{Pat, PatId},
2021
src::HasSource,
2122
AdtId, AttrDefId, ConstId, DefWithBodyId, EnumId, EnumVariantId, FunctionId, ItemContainerId,
22-
Lookup, ModuleDefId, StaticId, StructId,
23+
Lookup, ModuleDefId, ModuleId, StaticId, StructId,
2324
};
2425
use hir_expand::{
2526
name::{AsName, Name},
@@ -83,6 +84,7 @@ pub enum IdentType {
8384
Structure,
8485
Variable,
8586
Variant,
87+
Module,
8688
}
8789

8890
impl fmt::Display for IdentType {
@@ -97,6 +99,7 @@ impl fmt::Display for IdentType {
9799
IdentType::Structure => "Structure",
98100
IdentType::Variable => "Variable",
99101
IdentType::Variant => "Variant",
102+
IdentType::Module => "Module",
100103
};
101104

102105
repr.fmt(f)
@@ -132,6 +135,7 @@ impl<'a> DeclValidator<'a> {
132135

133136
pub(super) fn validate_item(&mut self, item: ModuleDefId) {
134137
match item {
138+
ModuleDefId::ModuleId(module_id) => self.validate_module(module_id),
135139
ModuleDefId::FunctionId(func) => self.validate_func(func),
136140
ModuleDefId::AdtId(adt) => self.validate_adt(adt),
137141
ModuleDefId::ConstId(const_id) => self.validate_const(const_id),
@@ -230,6 +234,55 @@ impl<'a> DeclValidator<'a> {
230234
|| parent()
231235
}
232236

237+
fn validate_module(&mut self, module_id: ModuleId) {
238+
// Check whether non-snake case identifiers are allowed for this module.
239+
if self.allowed(module_id.into(), allow::NON_SNAKE_CASE, false) {
240+
return;
241+
}
242+
243+
// Check the module name.
244+
let Some(module_name) = module_id.name(self.db.upcast()) else { return };
245+
let module_name_replacement =
246+
module_name.as_str().and_then(to_lower_snake_case).map(|new_name| Replacement {
247+
current_name: module_name,
248+
suggested_text: new_name,
249+
expected_case: CaseType::LowerSnakeCase,
250+
});
251+
252+
if let Some(module_name_replacement) = module_name_replacement {
253+
let module_data = &module_id.def_map(self.db.upcast())[module_id.local_id];
254+
let module_src = module_data.declaration_source(self.db.upcast());
255+
256+
if let Some(module_src) = module_src {
257+
let ast_ptr = match module_src.value.name() {
258+
Some(name) => name,
259+
None => {
260+
never!(
261+
"Replacement ({:?}) was generated for a module without a name: {:?}",
262+
module_name_replacement,
263+
module_src
264+
);
265+
return;
266+
}
267+
};
268+
269+
let diagnostic = IncorrectCase {
270+
file: module_src.file_id,
271+
ident_type: IdentType::Module,
272+
ident: AstPtr::new(&ast_ptr),
273+
expected_case: module_name_replacement.expected_case,
274+
ident_text: module_name_replacement
275+
.current_name
276+
.display(self.db.upcast())
277+
.to_string(),
278+
suggested_text: module_name_replacement.suggested_text,
279+
};
280+
281+
self.sink.push(diagnostic);
282+
}
283+
}
284+
}
285+
233286
fn validate_func(&mut self, func: FunctionId) {
234287
let data = self.db.function_data(func);
235288
if matches!(func.lookup(self.db.upcast()).container, ItemContainerId::ExternBlockId(_)) {

crates/hir/src/lib.rs

+2-9
Original file line numberDiff line numberDiff line change
@@ -452,15 +452,7 @@ impl HasVisibility for ModuleDef {
452452
impl Module {
453453
/// Name of this module.
454454
pub fn name(self, db: &dyn HirDatabase) -> Option<Name> {
455-
let def_map = self.id.def_map(db.upcast());
456-
let parent = def_map[self.id.local_id].parent?;
457-
def_map[parent].children.iter().find_map(|(name, module_id)| {
458-
if *module_id == self.id.local_id {
459-
Some(name.clone())
460-
} else {
461-
None
462-
}
463-
})
455+
self.id.name(db.upcast())
464456
}
465457

466458
/// Returns the crate this module is part of.
@@ -571,6 +563,7 @@ impl Module {
571563
if def_map[m.id.local_id].origin.is_inline() {
572564
m.diagnostics(db, acc)
573565
}
566+
acc.extend(def.diagnostics(db))
574567
}
575568
ModuleDef::Trait(t) => {
576569
for diag in db.trait_data_with_diagnostics(t.id).1.iter() {

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

+54
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,31 @@ fn some_fn() {
111111
let what_aweird_formatting = 10;
112112
another_func(what_aweird_formatting);
113113
}
114+
"#,
115+
);
116+
117+
check_fix(
118+
r#"
119+
static S: i32 = M::A;
120+
121+
mod $0M {
122+
pub const A: i32 = 10;
123+
}
124+
125+
mod other {
126+
use crate::M::A;
127+
}
128+
"#,
129+
r#"
130+
static S: i32 = m::A;
131+
132+
mod m {
133+
pub const A: i32 = 10;
134+
}
135+
136+
mod other {
137+
use crate::m::A;
138+
}
114139
"#,
115140
);
116141
}
@@ -518,17 +543,20 @@ fn NonSnakeCaseName(some_var: u8) -> u8 {
518543
519544
#[deny(nonstandard_style)]
520545
mod CheckNonstandardStyle {
546+
//^^^^^^^^^^^^^^^^^^^^^ 💡 error: Module `CheckNonstandardStyle` should have snake_case name, e.g. `check_nonstandard_style`
521547
fn HiImABadFnName() {}
522548
//^^^^^^^^^^^^^^ 💡 error: Function `HiImABadFnName` should have snake_case name, e.g. `hi_im_abad_fn_name`
523549
}
524550
525551
#[deny(warnings)]
526552
mod CheckBadStyle {
553+
//^^^^^^^^^^^^^ 💡 error: Module `CheckBadStyle` should have snake_case name, e.g. `check_bad_style`
527554
struct fooo;
528555
//^^^^ 💡 error: Structure `fooo` should have CamelCase name, e.g. `Fooo`
529556
}
530557
531558
mod F {
559+
//^ 💡 warn: Module `F` should have snake_case name, e.g. `f`
532560
#![deny(non_snake_case)]
533561
fn CheckItWorksWithModAttr() {}
534562
//^^^^^^^^^^^^^^^^^^^^^^^ 💡 error: Function `CheckItWorksWithModAttr` should have snake_case name, e.g. `check_it_works_with_mod_attr`
@@ -649,4 +677,30 @@ enum E {
649677
"#,
650678
);
651679
}
680+
681+
#[test]
682+
fn module_name_inline() {
683+
check_diagnostics(
684+
r#"
685+
mod M {
686+
//^ 💡 warn: Module `M` should have snake_case name, e.g. `m`
687+
mod IncorrectCase {}
688+
//^^^^^^^^^^^^^ 💡 warn: Module `IncorrectCase` should have snake_case name, e.g. `incorrect_case`
689+
}
690+
"#,
691+
);
692+
}
693+
694+
#[test]
695+
fn module_name_decl() {
696+
check_diagnostics(
697+
r#"
698+
//- /Foo.rs
699+
700+
//- /main.rs
701+
mod Foo;
702+
//^^^ 💡 warn: Module `Foo` should have snake_case name, e.g. `foo`
703+
"#,
704+
)
705+
}
652706
}

0 commit comments

Comments
 (0)