Skip to content

Fix unused_variables typo suggestions for const patterns #156904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
onehr wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix unused_variables typo suggestions for const patterns #156904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions compiler/rustc_mir_transform/src/liveness.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc_middle::mir::visit::{
MutatingUseContext, NonMutatingUseContext, NonUseContext, PlaceContext, Visitor,
};
use rustc_middle::mir::*;
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::print::with_crate_prefix;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_mir_dataflow::fmt::DebugWithContext;
use rustc_mir_dataflow::{Analysis, Backward, ResultsCursor};
Expand Down Expand Up @@ -168,13 +168,15 @@ fn is_capture(place: PlaceRef<'_>) -> bool {
}
}

/// Give a diagnostic when an unused variable may be a typo of a unit variant or a struct.
/// Give a diagnostic when an unused variable may be a typo of a unit variant,
/// unit struct, or const.
fn maybe_suggest_unit_pattern_typo<'tcx>(
tcx: TyCtxt<'tcx>,
body_def_id: DefId,
name: Symbol,
span: Span,
ty: Ty<'tcx>,
allow_consts: bool,
) -> Option<errors::PatternTypo> {
if let ty::Adt(adt_def, _) = ty.peel_refs().kind() {
let variant_names: Vec<_> = adt_def
Expand All @@ -191,19 +193,26 @@ fn maybe_suggest_unit_pattern_typo<'tcx>(
{
return Some(errors::PatternTypo {
span,
code: with_no_trimmed_paths!(tcx.def_path_str(variant.def_id)),
code: pattern_item_path(tcx, body_def_id, variant.def_id),
kind: tcx.def_descr(variant.def_id),
item_name: variant.name,
});
}
}

if !allow_consts {
return None;
}

// Look for consts of the same type with similar names as well,
// not just unit structs and variants.
let constants = tcx
.hir_body_owners()
.filter(|&def_id| {
// `const _: T = ...` items have no name reachable from a pattern: writing
// `path::_` is not valid syntax, so they must never appear in a suggestion.
matches!(tcx.def_kind(def_id), DefKind::Const { .. })
&& tcx.item_name(def_id.to_def_id()) != kw::Underscore
&& tcx.type_of(def_id).instantiate_identity().skip_norm_wip() == ty
&& tcx.visibility(def_id).is_accessible_from(body_def_id, tcx)
})
Expand All @@ -215,7 +224,7 @@ fn maybe_suggest_unit_pattern_typo<'tcx>(
{
return Some(errors::PatternTypo {
span,
code: with_no_trimmed_paths!(tcx.def_path_str(def_id)),
code: pattern_item_path(tcx, body_def_id, def_id.to_def_id()),
kind: "constant",
item_name,
});
Expand All @@ -224,6 +233,16 @@ fn maybe_suggest_unit_pattern_typo<'tcx>(
None
}

fn pattern_item_path(tcx: TyCtxt<'_>, body_def_id: DefId, item_def_id: DefId) -> String {
// A body-local const is in scope only by its bare name; everything else is a
// module-level item that `with_crate_prefix!` prints as a valid `crate::` path.
if tcx.is_descendant_of(item_def_id, body_def_id) {
return tcx.item_name(item_def_id).to_string();
}

with_crate_prefix!(tcx.def_path_str(item_def_id))
}

/// Return whether we should consider the current place as a drop guard and skip reporting.
fn maybe_drop_guard<'tcx>(
tcx: TyCtxt<'tcx>,
Expand Down Expand Up @@ -977,15 +996,19 @@ impl<'a, 'tcx> AssignmentResult<'a, 'tcx> {
&& introductions.iter().any(|intro| intro.span.eq_ctxt(def_span));

let maybe_suggest_typo = || {
if let LocalKind::Arg = self.body.local_kind(local) {
if matches!(self.body.local_kind(local), LocalKind::Arg) {
None
} else {
// In a bare `let x = ...`, replacing `x` with a const path would make the
// pattern refutable. Keep the existing unit ADT typo behavior unchanged.
let allow_consts = !matches!(binding.opt_match_place, Some((None, _)));
maybe_suggest_unit_pattern_typo(
tcx,
self.body.source.def_id(),
name,
def_span,
decl.ty,
allow_consts,
)
}
};
Expand Down
95 changes: 95 additions & 0 deletions tests/ui/lint/unused/unused-variables-const-pattern-typo.e2015.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
error: unused variable: `x`
--> $DIR/unused-variables-const-pattern-typo.rs:17:18
|
LL | let Some(x) = x else { return };
| ^
|
note: the lint level is defined here
--> $DIR/unused-variables-const-pattern-typo.rs:9:9
|
LL | #![deny(unused_variables)]
| ^^^^^^^^^^^^^^^^
help: you might have meant to pattern match on the similarly named constant `X`
|
LL - let Some(x) = x else { return };
LL + let Some(enclosed::X) = x else { return };
|
help: if this is intentional, prefix it with an underscore
|
LL | let Some(_x) = x else { return };
| +

error: unused variable: `x`
--> $DIR/unused-variables-const-pattern-typo.rs:27:13
|
LL | let x = system::Y;
| ^ help: if this is intentional, prefix it with an underscore: `_x`

error: unused variable: `good`
--> $DIR/unused-variables-const-pattern-typo.rs:37:18
|
LL | let Some(good) = x else { return };
| ^^^^
|
help: you might have meant to pattern match on the similarly named constant `GOOD`
|
LL - let Some(good) = x else { return };
LL + let Some(same_module::GOOD) = x else { return };
|
help: if this is intentional, prefix it with an underscore
|
LL | let Some(_good) = x else { return };
| +

error: unused variable: `local`
--> $DIR/unused-variables-const-pattern-typo.rs:46:14
|
LL | let Some(local) = x else { return };
| ^^^^^
|
help: you might have meant to pattern match on the similarly named constant `LOCAL`
|
LL - let Some(local) = x else { return };
LL + let Some(LOCAL) = x else { return };
|
help: if this is intentional, prefix it with an underscore
|
LL | let Some(_local) = x else { return };
| +

error: unused variable: `ready`
--> $DIR/unused-variables-const-pattern-typo.rs:60:18
|
LL | let Some(ready) = x else { return };
| ^^^^^
|
help: you might have meant to pattern match on the similarly named variant `Ready`
|
LL - let Some(ready) = x else { return };
LL + let Some(variants::State::Ready) = x else { return };
|
help: if this is intentional, prefix it with an underscore
|
LL | let Some(_ready) = x else { return };
| +

error: unused variable: `x`
--> $DIR/unused-variables-const-pattern-typo.rs:80:14
|
LL | let Some(x) = x else { return };
| ^ help: if this is intentional, prefix it with an underscore: `_x`

error: unused variable: `value`
--> $DIR/unused-variables-const-pattern-typo.rs:96:14
|
LL | let Some(value) = x else { return };
| ^^^^^ help: if this is intentional, prefix it with an underscore: `_value`

error: unused variable: `xyzzy`
--> $DIR/unused-variables-const-pattern-typo.rs:111:14
|
LL | let Some(xyzzy) = x else { return };
| ^^^^^ help: if this is intentional, prefix it with an underscore: `_xyzzy`

error: aborting due to 8 previous errors

95 changes: 95 additions & 0 deletions tests/ui/lint/unused/unused-variables-const-pattern-typo.e2021.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
error: unused variable: `x`
--> $DIR/unused-variables-const-pattern-typo.rs:17:18
|
LL | let Some(x) = x else { return };
| ^
|
note: the lint level is defined here
--> $DIR/unused-variables-const-pattern-typo.rs:9:9
|
LL | #![deny(unused_variables)]
| ^^^^^^^^^^^^^^^^
help: you might have meant to pattern match on the similarly named constant `X`
|
LL - let Some(x) = x else { return };
LL + let Some(crate::enclosed::X) = x else { return };
|
help: if this is intentional, prefix it with an underscore
|
LL | let Some(_x) = x else { return };
| +

error: unused variable: `x`
--> $DIR/unused-variables-const-pattern-typo.rs:27:13
|
LL | let x = system::Y;
| ^ help: if this is intentional, prefix it with an underscore: `_x`

error: unused variable: `good`
--> $DIR/unused-variables-const-pattern-typo.rs:37:18
|
LL | let Some(good) = x else { return };
| ^^^^
|
help: you might have meant to pattern match on the similarly named constant `GOOD`
|
LL - let Some(good) = x else { return };
LL + let Some(crate::same_module::GOOD) = x else { return };
|
help: if this is intentional, prefix it with an underscore
|
LL | let Some(_good) = x else { return };
| +

error: unused variable: `local`
--> $DIR/unused-variables-const-pattern-typo.rs:46:14
|
LL | let Some(local) = x else { return };
| ^^^^^
|
help: you might have meant to pattern match on the similarly named constant `LOCAL`
|
LL - let Some(local) = x else { return };
LL + let Some(LOCAL) = x else { return };
|
help: if this is intentional, prefix it with an underscore
|
LL | let Some(_local) = x else { return };
| +

error: unused variable: `ready`
--> $DIR/unused-variables-const-pattern-typo.rs:60:18
|
LL | let Some(ready) = x else { return };
| ^^^^^
|
help: you might have meant to pattern match on the similarly named variant `Ready`
|
LL - let Some(ready) = x else { return };
LL + let Some(crate::variants::State::Ready) = x else { return };
|
help: if this is intentional, prefix it with an underscore
|
LL | let Some(_ready) = x else { return };
| +

error: unused variable: `x`
--> $DIR/unused-variables-const-pattern-typo.rs:80:14
|
LL | let Some(x) = x else { return };
| ^ help: if this is intentional, prefix it with an underscore: `_x`

error: unused variable: `value`
--> $DIR/unused-variables-const-pattern-typo.rs:96:14
|
LL | let Some(value) = x else { return };
| ^^^^^ help: if this is intentional, prefix it with an underscore: `_value`

error: unused variable: `xyzzy`
--> $DIR/unused-variables-const-pattern-typo.rs:111:14
|
LL | let Some(xyzzy) = x else { return };
| ^^^^^ help: if this is intentional, prefix it with an underscore: `_xyzzy`

error: aborting due to 8 previous errors

Loading
Loading