mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #16303 - rosefromthedead:non-exhaustive-let, r=Veykril

Browse Source 
feat: add non-exhaustive-let diagnostic

I want this to have a quickfix to add an else branch but I couldn't figure out how to do that, so here's the diagnostic on its own. It pretends a `let` is a match with one arm, and asks the match checking whether that match would be exhaustive.

Previously the pattern was checked based on its own type, but that was causing a panic in `match_check` (while processing e.g. `crates/hir/src/lib.rs`) so I changed it to use the initialiser's type instead, to align with the checking of actual match expressions. I think the panic can still happen, but I hear that `match_check` is going to be updated to a new version from rustc, so I'm posting this now in the hopes that the panic will magically go away when that happens.
This commit is contained in:
bors 2024-02-19 11:51:06 +00:00
commit ff8fe522e8
5 changed files with 137 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
crates/hir-ty/src/diagnostics/expr.rs
View File

@ -12,6 +12,7 @@ use hir_expand::name;
use itertools::Itertools;
use rustc_hash::FxHashSet;
use rustc_pattern_analysis::usefulness::{compute_match_usefulness, ValidityConstraint};
use tracing::debug;
use triomphe::Arc;
use typed_arena::Arena;
@ -44,6 +45,10 @@ pub enum BodyValidationDiagnostic {
match_expr: ExprId,
uncovered_patterns: String,
},
NonExhaustiveLet {
pat: PatId,
uncovered_patterns: String,
},
RemoveTrailingReturn {
return_expr: ExprId,
},
@ -57,7 +62,8 @@ impl BodyValidationDiagnostic {
let _p =
tracing::span!(tracing::Level::INFO, "BodyValidationDiagnostic::collect").entered();
let infer = db.infer(owner);
let mut validator = ExprValidator::new(owner, infer);
let body = db.body(owner);
let mut validator = ExprValidator { owner, body, infer, diagnostics: Vec::new() };
validator.validate_body(db);
validator.diagnostics
}
@ -65,18 +71,16 @@ impl BodyValidationDiagnostic {
struct ExprValidator {
owner: DefWithBodyId,
body: Arc<Body>,
infer: Arc<InferenceResult>,
pub(super) diagnostics: Vec<BodyValidationDiagnostic>,
diagnostics: Vec<BodyValidationDiagnostic>,
}
impl ExprValidator {
fn new(owner: DefWithBodyId, infer: Arc<InferenceResult>) -> ExprValidator {
ExprValidator { owner, infer, diagnostics: Vec::new() }
}
fn validate_body(&mut self, db: &dyn HirDatabase) {
let body = db.body(self.owner);
let mut filter_map_next_checker = None;
// we'll pass &mut self while iterating over body.exprs, so they need to be disjoint
let body = Arc::clone(&self.body);
if matches!(self.owner, DefWithBodyId::FunctionId(_)) {
self.check_for_trailing_return(body.body_expr, &body);
@ -106,6 +110,9 @@ impl ExprValidator {
Expr::If { .. } => {
self.check_for_unnecessary_else(id, expr, &body);
}
Expr::Block { .. } => {
self.validate_block(db, expr);
}
_ => {}
}
}
@ -162,8 +169,6 @@ impl ExprValidator {
arms: &[MatchArm],
db: &dyn HirDatabase,
) {
let body = db.body(self.owner);
let scrut_ty = &self.infer[scrutinee_expr];
if scrut_ty.is_unknown() {
return;
@ -191,12 +196,12 @@ impl ExprValidator {
.as_reference()
.map(|(match_expr_ty, ..)| match_expr_ty == pat_ty)
.unwrap_or(false))
&& types_of_subpatterns_do_match(arm.pat, &body, &self.infer)
&& types_of_subpatterns_do_match(arm.pat, &self.body, &self.infer)
{
// If we had a NotUsefulMatchArm diagnostic, we could
// check the usefulness of each pattern as we added it
// to the matrix here.
let pat = self.lower_pattern(&cx, arm.pat, db, &body, &mut has_lowering_errors);
let pat = self.lower_pattern(&cx, arm.pat, db, &mut has_lowering_errors);
let m_arm = pat_analysis::MatchArm {
pat: pattern_arena.alloc(pat),
has_guard: arm.guard.is_some(),
@ -234,20 +239,63 @@ impl ExprValidator {
if !witnesses.is_empty() {
self.diagnostics.push(BodyValidationDiagnostic::MissingMatchArms {
match_expr,
uncovered_patterns: missing_match_arms(&cx, scrut_ty, witnesses, arms),
uncovered_patterns: missing_match_arms(&cx, scrut_ty, witnesses, m_arms.is_empty()),
});
}
}
fn validate_block(&mut self, db: &dyn HirDatabase, expr: &Expr) {
let Expr::Block { statements, .. } = expr else { return };
let pattern_arena = Arena::new();
let cx = MatchCheckCtx::new(self.owner.module(db.upcast()), self.owner, db);
for stmt in &**statements {
let &Statement::Let { pat, initializer, else_branch: None, .. } = stmt else {
continue;
};
let Some(initializer) = initializer else { continue };
let ty = &self.infer[initializer];
let mut have_errors = false;
let deconstructed_pat = self.lower_pattern(&cx, pat, db, &mut have_errors);
let match_arm = rustc_pattern_analysis::MatchArm {
pat: pattern_arena.alloc(deconstructed_pat),
has_guard: false,
arm_data: (),
};
if have_errors {
continue;
}
let report = match compute_match_usefulness(
&cx,
&[match_arm],
ty.clone(),
ValidityConstraint::ValidOnly,
) {
Ok(v) => v,
Err(e) => {
debug!(?e, "match usefulness error");
continue;
}
};
let witnesses = report.non_exhaustiveness_witnesses;
if !witnesses.is_empty() {
self.diagnostics.push(BodyValidationDiagnostic::NonExhaustiveLet {
pat,
uncovered_patterns: missing_match_arms(&cx, ty, witnesses, false),
});
}
}
}
fn lower_pattern<'p>(
&self,
cx: &MatchCheckCtx<'p>,
pat: PatId,
db: &dyn HirDatabase,
body: &Body,
have_errors: &mut bool,
) -> DeconstructedPat<'p> {
let mut patcx = match_check::PatCtxt::new(db, &self.infer, body);
let mut patcx = match_check::PatCtxt::new(db, &self.infer, &self.body);
let pattern = patcx.lower_pattern(pat);
let pattern = cx.lower_pat(&pattern);
if !patcx.errors.is_empty() {
@ -448,7 +496,7 @@ fn missing_match_arms<'p>(
cx: &MatchCheckCtx<'p>,
scrut_ty: &Ty,
witnesses: Vec<WitnessPat<'p>>,
arms: &[MatchArm],
arms_is_empty: bool,
) -> String {
struct DisplayWitness<'a, 'p>(&'a WitnessPat<'p>, &'a MatchCheckCtx<'p>);
impl fmt::Display for DisplayWitness<'_, '_> {
@ -463,7 +511,7 @@ fn missing_match_arms<'p>(
Some((AdtId::EnumId(e), _)) => !cx.db.enum_data(e).variants.is_empty(),
_ => false,
};
if arms.is_empty() && !non_empty_enum {
if arms_is_empty && !non_empty_enum {
format!("type `{}` is non-empty", scrut_ty.display(cx.db))
} else {
let pat_display = |witness| DisplayWitness(witness, cx);

23
crates/hir/src/diagnostics.rs
View File

@ -64,6 +64,7 @@ diagnostics![
MissingUnsafe,
MovedOutOfRef,
NeedMut,
NonExhaustiveLet,
NoSuchField,
PrivateAssocItem,
PrivateField,
@ -280,6 +281,12 @@ pub struct MissingMatchArms {
pub uncovered_patterns: String,
}
#[derive(Debug)]
pub struct NonExhaustiveLet {
pub pat: InFile<AstPtr<ast::Pat>>,
pub uncovered_patterns: String,
}
#[derive(Debug)]
pub struct TypeMismatch {
pub expr_or_pat: InFile<AstPtr<Either<ast::Expr, ast::Pat>>>,
@ -456,6 +463,22 @@ impl AnyDiagnostic {
Err(SyntheticSyntax) => (),
}
}
BodyValidationDiagnostic::NonExhaustiveLet { pat, uncovered_patterns } => {
match source_map.pat_syntax(pat) {
Ok(source_ptr) => {
if let Some(ast_pat) = source_ptr.value.cast::<ast::Pat>() {
return Some(
NonExhaustiveLet {
pat: InFile::new(source_ptr.file_id, ast_pat),
uncovered_patterns,
}
.into(),
);
}
}
Err(SyntheticSyntax) => {}
}
}
BodyValidationDiagnostic::RemoveTrailingReturn { return_expr } => {
if let Ok(source_ptr) = source_map.expr_syntax(return_expr) {
// Filters out desugared return expressions (e.g. desugared try operators).

2
crates/ide-diagnostics/src/handlers/mutability_errors.rs
View File

@ -817,7 +817,7 @@ fn f() {
//- minicore: option
fn f(_: i32) {}
fn main() {
let ((Some(mut x), None) | (_, Some(mut x))) = (None, Some(7));
let ((Some(mut x), None) | (_, Some(mut x))) = (None, Some(7)) else { return };
//^^^^^ 💡 warn: variable does not need to be mutable
f(x);
}

47
crates/ide-diagnostics/src/handlers/non_exhaustive_let.rs Normal file
View File

@ -0,0 +1,47 @@
use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext};
// Diagnostic: non-exhaustive-let
//
// This diagnostic is triggered if a `let` statement without an `else` branch has a non-exhaustive
// pattern.
pub(crate) fn non_exhaustive_let(
ctx: &DiagnosticsContext<'_>,
d: &hir::NonExhaustiveLet,
) -> Diagnostic {
Diagnostic::new_with_syntax_node_ptr(
ctx,
DiagnosticCode::RustcHardError("E0005"),
format!("non-exhaustive pattern: {}", d.uncovered_patterns),
d.pat.map(Into::into),
)
}
#[cfg(test)]
mod tests {
use crate::tests::check_diagnostics;
#[test]
fn option_nonexhaustive() {
check_diagnostics(
r#"
//- minicore: option
fn main() {
let None = Some(5);
//^^^^ error: non-exhaustive pattern: `Some(_)` not covered
}
"#,
);
}
#[test]
fn option_exhaustive() {
check_diagnostics(
r#"
//- minicore: option
fn main() {
let Some(_) | None = Some(5);
}
"#,
);
}
}

2
crates/ide-diagnostics/src/lib.rs
View File

@ -41,6 +41,7 @@ mod handlers {
pub(crate) mod moved_out_of_ref;
pub(crate) mod mutability_errors;
pub(crate) mod no_such_field;
pub(crate) mod non_exhaustive_let;
pub(crate) mod private_assoc_item;
pub(crate) mod private_field;
pub(crate) mod remove_trailing_return;
@ -359,6 +360,7 @@ pub fn diagnostics(
AnyDiagnostic::MissingUnsafe(d) => handlers::missing_unsafe::missing_unsafe(&ctx, &d),
AnyDiagnostic::MovedOutOfRef(d) => handlers::moved_out_of_ref::moved_out_of_ref(&ctx, &d),
AnyDiagnostic::NeedMut(d) => handlers::mutability_errors::need_mut(&ctx, &d),
AnyDiagnostic::NonExhaustiveLet(d) => handlers::non_exhaustive_let::non_exhaustive_let(&ctx, &d),
AnyDiagnostic::NoSuchField(d) => handlers::no_such_field::no_such_field(&ctx, &d),
AnyDiagnostic::PrivateAssocItem(d) => handlers::private_assoc_item::private_assoc_item(&ctx, &d),
AnyDiagnostic::PrivateField(d) => handlers::private_field::private_field(&ctx, &d),