Skip to content

Commit

Permalink
Auto merge of #15901 - Veykril:inner-diag, r=lnicola
Browse files Browse the repository at this point in the history
fix: Diagnose everything in nested items, not just def diagnostics

Turns out we only calculated def diagnostics for these before (was wondering why I wasn't getting any type mismatches)
  • Loading branch information
bors committed Nov 15, 2023
2 parents 535eb0d + 498f39e commit 57ef70c
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 33 deletions.
28 changes: 2 additions & 26 deletions crates/hir-ty/src/diagnostics/decl_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ use hir_def::{
data::adt::VariantData,
hir::{Pat, PatId},
src::HasSource,
AdtId, AttrDefId, ConstId, DefWithBodyId, EnumId, EnumVariantId, FunctionId, ItemContainerId,
Lookup, ModuleDefId, ModuleId, StaticId, StructId,
AdtId, AttrDefId, ConstId, EnumId, FunctionId, ItemContainerId, Lookup, ModuleDefId, ModuleId,
StaticId, StructId,
};
use hir_expand::{
name::{AsName, Name},
Expand Down Expand Up @@ -290,8 +290,6 @@ impl<'a> DeclValidator<'a> {
return;
}

self.validate_body_inner_items(func.into());

// Check whether non-snake case identifiers are allowed for this function.
if self.allowed(func.into(), allow::NON_SNAKE_CASE, false) {
return;
Expand Down Expand Up @@ -568,11 +566,6 @@ impl<'a> DeclValidator<'a> {
fn validate_enum(&mut self, enum_id: EnumId) {
let data = self.db.enum_data(enum_id);

for (local_id, _) in data.variants.iter() {
let variant_id = EnumVariantId { parent: enum_id, local_id };
self.validate_body_inner_items(variant_id.into());
}

// Check whether non-camel case names are allowed for this enum.
if self.allowed(enum_id.into(), allow::NON_CAMEL_CASE_TYPES, false) {
return;
Expand Down Expand Up @@ -697,8 +690,6 @@ impl<'a> DeclValidator<'a> {
fn validate_const(&mut self, const_id: ConstId) {
let data = self.db.const_data(const_id);

self.validate_body_inner_items(const_id.into());

if self.allowed(const_id.into(), allow::NON_UPPER_CASE_GLOBAL, false) {
return;
}
Expand Down Expand Up @@ -747,8 +738,6 @@ impl<'a> DeclValidator<'a> {
return;
}

self.validate_body_inner_items(static_id.into());

if self.allowed(static_id.into(), allow::NON_UPPER_CASE_GLOBAL, false) {
return;
}
Expand Down Expand Up @@ -786,17 +775,4 @@ impl<'a> DeclValidator<'a> {

self.sink.push(diagnostic);
}

// FIXME: We don't currently validate names within `DefWithBodyId::InTypeConstId`.
/// Recursively validates inner scope items, such as static variables and constants.
fn validate_body_inner_items(&mut self, body_id: DefWithBodyId) {
let body = self.db.body(body_id);
for (_, block_def_map) in body.blocks(self.db.upcast()) {
for (_, module) in block_def_map.modules() {
for def_id in module.scope.declarations() {
self.validate_item(def_id);
}
}
}
}
}
4 changes: 1 addition & 3 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1537,9 +1537,7 @@ impl DefWithBody {
let (body, source_map) = db.body_with_source_map(self.into());

for (_, def_map) in body.blocks(db.upcast()) {
for diag in def_map.diagnostics() {
emit_def_diagnostic(db, acc, diag);
}
Module { id: def_map.module_id(DefMap::ROOT) }.diagnostics(db, acc);
}

for diag in source_map.diagnostics() {
Expand Down
8 changes: 4 additions & 4 deletions crates/ide-diagnostics/src/handlers/incorrect_case.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,12 +599,12 @@ fn main() {
//^^^ 💡 warn: Static variable `bar` should have UPPER_SNAKE_CASE name, e.g. `BAR`
fn BAZ() {
//^^^ 💡 warn: Function `BAZ` should have snake_case name, e.g. `baz`
let INNER_INNER = 42;
//^^^^^^^^^^^ 💡 warn: Variable `INNER_INNER` should have snake_case name, e.g. `inner_inner`
let _INNER_INNER = 42;
//^^^^^^^^^^^^ 💡 warn: Variable `_INNER_INNER` should have snake_case name, e.g. `_inner_inner`
}
let INNER_LOCAL = 42;
//^^^^^^^^^^^ 💡 warn: Variable `INNER_LOCAL` should have snake_case name, e.g. `inner_local`
let _INNER_LOCAL = 42;
//^^^^^^^^^^^^ 💡 warn: Variable `_INNER_LOCAL` should have snake_case name, e.g. `_inner_local`
}
}
"#,
Expand Down
15 changes: 15 additions & 0 deletions crates/ide-diagnostics/src/handlers/type_mismatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,21 @@ fn f() -> i32 {
0
}
fn g() { return; }
"#,
);
}

#[test]
fn smoke_test_inner_items() {
check_diagnostics(
r#"
fn f() {
fn inner() -> i32 {
return;
// ^^^^^^ error: expected i32, found ()
0
}
}
"#,
);
}
Expand Down

0 comments on commit 57ef70c

Please sign in to comment.