Skip to content

Commit

Permalink
Auto merge of #16589 - rosefromthedead:unresolved-ident, r=Veykril
Browse files Browse the repository at this point in the history
feat: add unresolved-ident diagnostic

This should cover missing local variables and missing unit structs and the like. It's conservatively marked experimental
  • Loading branch information
bors committed Feb 19, 2024
2 parents e6b96db + 1e448f8 commit bbb781e
Show file tree
Hide file tree
Showing 17 changed files with 130 additions and 43 deletions.
3 changes: 3 additions & 0 deletions crates/hir-ty/src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@ pub enum InferenceDiagnostic {
UnresolvedAssocItem {
id: ExprOrPatId,
},
UnresolvedIdent {
expr: ExprId,
},
// FIXME: This should be emitted in body lowering
BreakOutsideOfLoop {
expr: ExprId,
Expand Down
14 changes: 12 additions & 2 deletions crates/hir-ty/src/infer/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use hir_def::{
ArithOp, Array, BinaryOp, ClosureKind, Expr, ExprId, LabelId, Literal, Statement, UnaryOp,
},
lang_item::{LangItem, LangItemTarget},
path::{GenericArg, GenericArgs},
path::{GenericArg, GenericArgs, Path},
BlockId, ConstParamId, FieldId, ItemContainerId, Lookup, TupleFieldId, TupleId,
};
use hir_expand::name::{name, Name};
Expand Down Expand Up @@ -439,7 +439,17 @@ impl InferenceContext<'_> {
}
Expr::Path(p) => {
let g = self.resolver.update_to_inner_scope(self.db.upcast(), self.owner, tgt_expr);
let ty = self.infer_path(p, tgt_expr.into()).unwrap_or_else(|| self.err_ty());
let ty = match self.infer_path(p, tgt_expr.into()) {
Some(ty) => ty,
None => {
if matches!(p, Path::Normal { mod_path, .. } if mod_path.is_ident()) {
self.push_diagnostic(InferenceDiagnostic::UnresolvedIdent {
expr: tgt_expr,
});
}
self.err_ty()
}
};
self.resolver.reset_to_guard(g);
ty
}
Expand Down
10 changes: 10 additions & 0 deletions crates/hir/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ diagnostics![
UnresolvedMacroCall,
UnresolvedMethodCall,
UnresolvedModule,
UnresolvedIdent,
UnresolvedProcMacro,
UnusedMut,
UnusedVariable,
Expand Down Expand Up @@ -242,6 +243,11 @@ pub struct UnresolvedAssocItem {
pub expr_or_pat: InFile<AstPtr<Either<ast::Expr, Either<ast::Pat, ast::SelfParam>>>>,
}

#[derive(Debug)]
pub struct UnresolvedIdent {
pub expr: InFile<AstPtr<ast::Expr>>,
}

#[derive(Debug)]
pub struct PrivateField {
pub expr: InFile<AstPtr<ast::Expr>>,
Expand Down Expand Up @@ -588,6 +594,10 @@ impl AnyDiagnostic {
};
UnresolvedAssocItem { expr_or_pat }.into()
}
&InferenceDiagnostic::UnresolvedIdent { expr } => {
let expr = expr_syntax(expr);
UnresolvedIdent { expr }.into()
}
&InferenceDiagnostic::BreakOutsideOfLoop { expr, is_break, bad_value_break } => {
let expr = expr_syntax(expr);
BreakOutsideOfLoop { expr, is_break, bad_value_break }.into()
Expand Down
1 change: 1 addition & 0 deletions crates/ide-diagnostics/src/handlers/inactive_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ fn f() {
#[cfg(a)] let x = 0; // let statement
//^^^^^^^^^^^^^^^^^^^^ weak: code is inactive due to #[cfg] directives: a is disabled
fn abc() {}
abc(#[cfg(a)] 0);
//^^^^^^^^^^^ weak: code is inactive due to #[cfg] directives: a is disabled
let x = Struct {
Expand Down
2 changes: 1 addition & 1 deletion crates/ide-diagnostics/src/handlers/incorrect_case.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ impl BAD_TRAIT for () {
fn BadFunction() {}
}
"#,
std::iter::once("unused_variables".to_owned()),
&["unused_variables"],
);
}

Expand Down
3 changes: 2 additions & 1 deletion crates/ide-diagnostics/src/handlers/missing_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,8 @@ struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let one = 1;
let s = TestStruct{ ..a };
let a = TestStruct{ one, two: 2 };
let _ = TestStruct{ ..a };
}
"#,
);
Expand Down
16 changes: 10 additions & 6 deletions crates/ide-diagnostics/src/handlers/missing_match_arms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ pub(crate) fn missing_match_arms(
#[cfg(test)]
mod tests {
use crate::{
tests::{check_diagnostics, check_diagnostics_with_config},
tests::{
check_diagnostics, check_diagnostics_with_config, check_diagnostics_with_disabled,
},
DiagnosticsConfig,
};

Expand Down Expand Up @@ -282,7 +284,7 @@ fn main() {
cov_mark::check_count!(validate_match_bailed_out, 4);
// Match statements with arms that don't match the
// expression pattern do not fire this diagnostic.
check_diagnostics(
check_diagnostics_with_disabled(
r#"
enum Either { A, B }
enum Either2 { C, D }
Expand All @@ -307,6 +309,7 @@ fn main() {
match Unresolved::Bar { Unresolved::Baz => () }
}
"#,
&["E0425"],
);
}

Expand Down Expand Up @@ -397,11 +400,11 @@ fn main() {
match loop {} {
Either::A => (),
}
match loop { break Foo::A } {
//^^^^^^^^^^^^^^^^^^^^^ error: missing match arm: `B` not covered
match loop { break Either::A } {
//^^^^^^^^^^^^^^^^^^^^^^^^ error: missing match arm: `B` not covered
Either::A => (),
}
match loop { break Foo::A } {
match loop { break Either::A } {
Either::A => (),
Either::B => (),
}
Expand Down Expand Up @@ -977,7 +980,7 @@ fn f(ty: Enum) {
#[test]
fn unexpected_ty_fndef() {
cov_mark::check!(validate_match_bailed_out);
check_diagnostics(
check_diagnostics_with_disabled(
r"
enum Exp {
Tuple(()),
Expand All @@ -987,6 +990,7 @@ fn f() {
Exp::Tuple => {}
}
}",
&["E0425"],
);
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ide-diagnostics/src/handlers/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ fn main(b: bool) {
&mut x;
}
"#,
std::iter::once("remove-unnecessary-else".to_owned()),
&["remove-unnecessary-else"],
);
check_diagnostics_with_disabled(
r#"
Expand All @@ -463,7 +463,7 @@ fn main(b: bool) {
&mut x;
}
"#,
std::iter::once("remove-unnecessary-else".to_owned()),
&["remove-unnecessary-else"],
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ fn foo(x: usize) -> u8 {
} //^^^^^^^^^ 💡 weak: replace return <expr>; with <expr>
}
"#,
std::iter::once("remove-unnecessary-else".to_owned()),
&["remove-unnecessary-else"],
);
}

Expand Down
42 changes: 25 additions & 17 deletions crates/ide-diagnostics/src/handlers/remove_unnecessary_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,11 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &RemoveUnnecessaryElse) -> Option<Vec<

#[cfg(test)]
mod tests {
use crate::tests::{check_diagnostics, check_diagnostics_with_disabled, check_fix};

fn check_diagnostics_with_needless_return_disabled(ra_fixture: &str) {
check_diagnostics_with_disabled(ra_fixture, std::iter::once("needless_return".to_owned()));
}
use crate::tests::{check_diagnostics_with_disabled, check_fix};

#[test]
fn remove_unnecessary_else_for_return() {
check_diagnostics_with_needless_return_disabled(
check_diagnostics_with_disabled(
r#"
fn test() {
if foo {
Expand All @@ -114,6 +110,7 @@ fn test() {
}
}
"#,
&["needless_return", "E0425"],
);
check_fix(
r#"
Expand All @@ -138,7 +135,7 @@ fn test() {

#[test]
fn remove_unnecessary_else_for_return2() {
check_diagnostics_with_needless_return_disabled(
check_diagnostics_with_disabled(
r#"
fn test() {
if foo {
Expand All @@ -151,6 +148,7 @@ fn test() {
}
}
"#,
&["needless_return", "E0425"],
);
check_fix(
r#"
Expand Down Expand Up @@ -181,7 +179,7 @@ fn test() {

#[test]
fn remove_unnecessary_else_for_return3() {
check_diagnostics_with_needless_return_disabled(
check_diagnostics_with_disabled(
r#"
fn test(a: bool) -> i32 {
if a {
Expand All @@ -192,6 +190,7 @@ fn test(a: bool) -> i32 {
}
}
"#,
&["needless_return", "E0425"],
);
check_fix(
r#"
Expand All @@ -216,7 +215,7 @@ fn test(a: bool) -> i32 {

#[test]
fn remove_unnecessary_else_for_return_in_child_if_expr() {
check_diagnostics_with_needless_return_disabled(
check_diagnostics_with_disabled(
r#"
fn test() {
if foo {
Expand All @@ -229,6 +228,7 @@ fn test() {
}
}
"#,
&["needless_return", "E0425"],
);
check_fix(
r#"
Expand Down Expand Up @@ -294,7 +294,7 @@ fn test() {

#[test]
fn remove_unnecessary_else_for_break() {
check_diagnostics(
check_diagnostics_with_disabled(
r#"
fn test() {
loop {
Expand All @@ -307,6 +307,7 @@ fn test() {
}
}
"#,
&["E0425"],
);
check_fix(
r#"
Expand Down Expand Up @@ -335,7 +336,7 @@ fn test() {

#[test]
fn remove_unnecessary_else_for_continue() {
check_diagnostics(
check_diagnostics_with_disabled(
r#"
fn test() {
loop {
Expand All @@ -348,6 +349,7 @@ fn test() {
}
}
"#,
&["E0425"],
);
check_fix(
r#"
Expand Down Expand Up @@ -376,7 +378,7 @@ fn test() {

#[test]
fn remove_unnecessary_else_for_never() {
check_diagnostics(
check_diagnostics_with_disabled(
r#"
fn test() {
if foo {
Expand All @@ -391,6 +393,7 @@ fn never() -> ! {
loop {}
}
"#,
&["E0425"],
);
check_fix(
r#"
Expand Down Expand Up @@ -423,7 +426,7 @@ fn never() -> ! {

#[test]
fn no_diagnostic_if_no_else_branch() {
check_diagnostics(
check_diagnostics_with_disabled(
r#"
fn test() {
if foo {
Expand All @@ -433,12 +436,13 @@ fn test() {
do_something_else();
}
"#,
&["E0425"],
);
}

#[test]
fn no_diagnostic_if_no_divergence() {
check_diagnostics(
check_diagnostics_with_disabled(
r#"
fn test() {
if foo {
Expand All @@ -448,12 +452,13 @@ fn test() {
}
}
"#,
&["E0425"],
);
}

#[test]
fn no_diagnostic_if_no_divergence_in_else_branch() {
check_diagnostics_with_needless_return_disabled(
check_diagnostics_with_disabled(
r#"
fn test() {
if foo {
Expand All @@ -463,12 +468,13 @@ fn test() {
}
}
"#,
&["needless_return", "E0425"],
);
}

#[test]
fn no_diagnostic_if_not_expr_stmt() {
check_diagnostics_with_needless_return_disabled(
check_diagnostics_with_disabled(
r#"
fn test1() {
let _x = if a {
Expand All @@ -490,13 +496,15 @@ fn test2() {
};
}
"#,
&["needless_return", "E0425"],
);
check_diagnostics(
check_diagnostics_with_disabled(
r#"
fn test3() -> u8 {
foo(if a { return 1 } else { 0 })
}
"#,
&["E0425"],
);
}
}
2 changes: 1 addition & 1 deletion crates/ide-diagnostics/src/handlers/type_mismatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ fn f() -> i32 {
}
fn g() { return; }
"#,
std::iter::once("needless_return".to_owned()),
&["needless_return"],
);
}

Expand Down
Loading

0 comments on commit bbb781e

Please sign in to comment.