Skip to content

Commit db50499

Browse files
authored
Unrolled build for #145827
Rollup merge of #145827 - estebank:issue-51976, r=jackh726 On unused binding or binding not present in all patterns, suggest potential typo of unit struct/variant or const When encountering an or-pattern with a binding not available in all patterns, look for consts and unit struct/variants that have similar names as the binding to detect typos. ``` error[E0408]: variable `Ban` is not bound in all patterns --> $DIR/binding-typo.rs:22:9 | LL | (Foo, _) | (Ban, Foo) => {} | ^^^^^^^^ --- variable not in all patterns | | | pattern doesn't bind `Ban` | help: you might have meant to use the similarly named unit variant `Bar` | LL - (Foo, _) | (Ban, Foo) => {} LL + (Foo, _) | (Bar, Foo) => {} | ``` For items that are not in the immedate scope, suggest the full path for them: ``` error[E0408]: variable `Non` is not bound in all patterns --> $DIR/binding-typo-2.rs:51:16 | LL | (Non | Some(_))=> {} | --- ^^^^^^^ pattern doesn't bind `Non` | | | variable not in all patterns | help: you might have meant to use the similarly named unit variant `None` | LL - (Non | Some(_))=> {} LL + (core::option::Option::None | Some(_))=> {} | ``` When encountering a typo in a pattern that gets interpreted as an unused binding, look for unit struct/variant of the same type as the binding: ``` error: unused variable: `Non` --> $DIR/binding-typo-2.rs:36:9 | LL | Non => {} | ^^^ | help: if this is intentional, prefix it with an underscore | LL | _Non => {} | + help: you might have meant to pattern match on the similarly named variant `None` | LL - Non => {} LL + std::prelude::v1::None => {} | ``` Suggest constant on unused binding in a pattern ``` error: unused variable: `Batery` --> $DIR/binding-typo-2.rs:110:9 | LL | Batery => {} | ^^^^^^ | help: if this is intentional, prefix it with an underscore | LL | _Batery => {} | + help: you might have meant to pattern match on the similarly named constant `Battery` | LL | Battery => {} | + ``` Fix #51976.
2 parents 9385c64 + 86085b4 commit db50499

22 files changed

+912
-84
lines changed

compiler/rustc_passes/messages.ftl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,7 @@ passes_unused_var_maybe_capture_ref = unused variable: `{$name}`
687687
688688
passes_unused_var_remove_field = unused variable: `{$name}`
689689
passes_unused_var_remove_field_suggestion = try removing the field
690+
passes_unused_var_typo = you might have meant to pattern match on the similarly named {$kind} `{$item_name}`
690691
691692
passes_unused_variable_args_in_macro = `{$name}` is captured in macro and introduced a unused variable
692693

compiler/rustc_passes/src/errors.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,6 +1367,22 @@ pub(crate) struct UnusedVarRemoveFieldSugg {
13671367
#[note]
13681368
pub(crate) struct UnusedVarAssignedOnly {
13691369
pub name: String,
1370+
#[subdiagnostic]
1371+
pub typo: Option<PatternTypo>,
1372+
}
1373+
1374+
#[derive(Subdiagnostic)]
1375+
#[multipart_suggestion(
1376+
passes_unused_var_typo,
1377+
style = "verbose",
1378+
applicability = "machine-applicable"
1379+
)]
1380+
pub(crate) struct PatternTypo {
1381+
#[suggestion_part(code = "{code}")]
1382+
pub span: Span,
1383+
pub code: String,
1384+
pub item_name: String,
1385+
pub kind: String,
13701386
}
13711387

13721388
#[derive(LintDiagnostic)]
@@ -1434,6 +1450,8 @@ pub(crate) struct UnusedVariableTryPrefix {
14341450
#[subdiagnostic]
14351451
pub sugg: UnusedVariableSugg,
14361452
pub name: String,
1453+
#[subdiagnostic]
1454+
pub typo: Option<PatternTypo>,
14371455
}
14381456

14391457
#[derive(Subdiagnostic)]

compiler/rustc_passes/src/liveness.rs

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,10 @@ use rustc_hir::{Expr, HirId, HirIdMap, HirIdSet, find_attr};
9595
use rustc_index::IndexVec;
9696
use rustc_middle::query::Providers;
9797
use rustc_middle::span_bug;
98+
use rustc_middle::ty::print::with_no_trimmed_paths;
9899
use rustc_middle::ty::{self, RootVariableMinCaptureList, Ty, TyCtxt};
99100
use rustc_session::lint;
101+
use rustc_span::edit_distance::find_best_match_for_name;
100102
use rustc_span::{BytePos, Span, Symbol};
101103
use tracing::{debug, instrument};
102104

@@ -1688,6 +1690,51 @@ impl<'tcx> Liveness<'_, 'tcx> {
16881690
let is_assigned =
16891691
if ln == self.exit_ln { false } else { self.assigned_on_exit(ln, var) };
16901692

1693+
let mut typo = None;
1694+
for (hir_id, _, span) in &hir_ids_and_spans {
1695+
let ty = self.typeck_results.node_type(*hir_id);
1696+
if let ty::Adt(adt, _) = ty.peel_refs().kind() {
1697+
let name = Symbol::intern(&name);
1698+
let adt_def = self.ir.tcx.adt_def(adt.did());
1699+
let variant_names: Vec<_> = adt_def
1700+
.variants()
1701+
.iter()
1702+
.filter(|v| matches!(v.ctor, Some((CtorKind::Const, _))))
1703+
.map(|v| v.name)
1704+
.collect();
1705+
if let Some(name) = find_best_match_for_name(&variant_names, name, None)
1706+
&& let Some(variant) = adt_def.variants().iter().find(|v| {
1707+
v.name == name && matches!(v.ctor, Some((CtorKind::Const, _)))
1708+
})
1709+
{
1710+
typo = Some(errors::PatternTypo {
1711+
span: *span,
1712+
code: with_no_trimmed_paths!(self.ir.tcx.def_path_str(variant.def_id)),
1713+
kind: self.ir.tcx.def_descr(variant.def_id).to_string(),
1714+
item_name: variant.name.to_string(),
1715+
});
1716+
}
1717+
}
1718+
}
1719+
if typo.is_none() {
1720+
for (hir_id, _, span) in &hir_ids_and_spans {
1721+
let ty = self.typeck_results.node_type(*hir_id);
1722+
// Look for consts of the same type with similar names as well, not just unit
1723+
// structs and variants.
1724+
for def_id in self.ir.tcx.hir_body_owners() {
1725+
if let DefKind::Const = self.ir.tcx.def_kind(def_id)
1726+
&& self.ir.tcx.type_of(def_id).instantiate_identity() == ty
1727+
{
1728+
typo = Some(errors::PatternTypo {
1729+
span: *span,
1730+
code: with_no_trimmed_paths!(self.ir.tcx.def_path_str(def_id)),
1731+
kind: "constant".to_string(),
1732+
item_name: self.ir.tcx.item_name(def_id).to_string(),
1733+
});
1734+
}
1735+
}
1736+
}
1737+
}
16911738
if is_assigned {
16921739
self.ir.tcx.emit_node_span_lint(
16931740
lint::builtin::UNUSED_VARIABLES,
@@ -1696,7 +1743,7 @@ impl<'tcx> Liveness<'_, 'tcx> {
16961743
.into_iter()
16971744
.map(|(_, _, ident_span)| ident_span)
16981745
.collect::<Vec<_>>(),
1699-
errors::UnusedVarAssignedOnly { name },
1746+
errors::UnusedVarAssignedOnly { name, typo },
17001747
)
17011748
} else if can_remove {
17021749
let spans = hir_ids_and_spans
@@ -1788,6 +1835,7 @@ impl<'tcx> Liveness<'_, 'tcx> {
17881835
name,
17891836
sugg,
17901837
string_interp: suggestions,
1838+
typo,
17911839
},
17921840
);
17931841
}

compiler/rustc_resolve/messages.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,8 @@ resolve_variable_bound_with_different_mode =
470470
.label = bound in different ways
471471
.first_binding_span = first binding
472472
473+
resolve_variable_is_a_typo = you might have meant to use the similarly named previously used binding `{$typo}`
474+
473475
resolve_variable_is_not_bound_in_all_patterns =
474476
variable `{$name}` is not bound in all patterns
475477

compiler/rustc_resolve/src/diagnostics.rs

Lines changed: 164 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -661,8 +661,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
661661
ResolutionError::VariableNotBoundInPattern(binding_error, parent_scope) => {
662662
let BindingError { name, target, origin, could_be_path } = binding_error;
663663

664-
let target_sp = target.iter().copied().collect::<Vec<_>>();
665-
let origin_sp = origin.iter().copied().collect::<Vec<_>>();
664+
let mut target_sp = target.iter().map(|pat| pat.span).collect::<Vec<_>>();
665+
target_sp.sort();
666+
target_sp.dedup();
667+
let mut origin_sp = origin.iter().map(|(span, _)| *span).collect::<Vec<_>>();
668+
origin_sp.sort();
669+
origin_sp.dedup();
666670

667671
let msp = MultiSpan::from_spans(target_sp.clone());
668672
let mut err = self
@@ -671,8 +675,35 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
671675
for sp in target_sp {
672676
err.subdiagnostic(errors::PatternDoesntBindName { span: sp, name });
673677
}
674-
for sp in origin_sp {
675-
err.subdiagnostic(errors::VariableNotInAllPatterns { span: sp });
678+
for sp in &origin_sp {
679+
err.subdiagnostic(errors::VariableNotInAllPatterns { span: *sp });
680+
}
681+
let mut suggested_typo = false;
682+
if !target.iter().all(|pat| matches!(pat.kind, ast::PatKind::Ident(..)))
683+
&& !origin.iter().all(|(_, pat)| matches!(pat.kind, ast::PatKind::Ident(..)))
684+
{
685+
// The check above is so that when we encounter `match foo { (a | b) => {} }`,
686+
// we don't suggest `(a | a) => {}`, which would never be what the user wants.
687+
let mut target_visitor = BindingVisitor::default();
688+
for pat in &target {
689+
target_visitor.visit_pat(pat);
690+
}
691+
target_visitor.identifiers.sort();
692+
target_visitor.identifiers.dedup();
693+
let mut origin_visitor = BindingVisitor::default();
694+
for (_, pat) in &origin {
695+
origin_visitor.visit_pat(pat);
696+
}
697+
origin_visitor.identifiers.sort();
698+
origin_visitor.identifiers.dedup();
699+
// Find if the binding could have been a typo
700+
if let Some(typo) =
701+
find_best_match_for_name(&target_visitor.identifiers, name.name, None)
702+
&& !origin_visitor.identifiers.contains(&typo)
703+
{
704+
err.subdiagnostic(errors::PatternBindingTypo { spans: origin_sp, typo });
705+
suggested_typo = true;
706+
}
676707
}
677708
if could_be_path {
678709
let import_suggestions = self.lookup_import_candidates(
@@ -693,10 +724,86 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
693724
},
694725
);
695726

696-
if import_suggestions.is_empty() {
727+
if import_suggestions.is_empty() && !suggested_typo {
728+
let kinds = [
729+
DefKind::Ctor(CtorOf::Variant, CtorKind::Const),
730+
DefKind::Ctor(CtorOf::Struct, CtorKind::Const),
731+
DefKind::Const,
732+
DefKind::AssocConst,
733+
];
734+
let mut local_names = vec![];
735+
self.add_module_candidates(
736+
parent_scope.module,
737+
&mut local_names,
738+
&|res| matches!(res, Res::Def(_, _)),
739+
None,
740+
);
741+
let local_names: FxHashSet<_> = local_names
742+
.into_iter()
743+
.filter_map(|s| match s.res {
744+
Res::Def(_, def_id) => Some(def_id),
745+
_ => None,
746+
})
747+
.collect();
748+
749+
let mut local_suggestions = vec![];
750+
let mut suggestions = vec![];
751+
for kind in kinds {
752+
if let Some(suggestion) = self.early_lookup_typo_candidate(
753+
ScopeSet::All(Namespace::ValueNS),
754+
&parent_scope,
755+
name,
756+
&|res: Res| match res {
757+
Res::Def(k, _) => k == kind,
758+
_ => false,
759+
},
760+
) && let Res::Def(kind, mut def_id) = suggestion.res
761+
{
762+
if let DefKind::Ctor(_, _) = kind {
763+
def_id = self.tcx.parent(def_id);
764+
}
765+
let kind = kind.descr(def_id);
766+
if local_names.contains(&def_id) {
767+
// The item is available in the current scope. Very likely to
768+
// be a typo. Don't use the full path.
769+
local_suggestions.push((
770+
suggestion.candidate,
771+
suggestion.candidate.to_string(),
772+
kind,
773+
));
774+
} else {
775+
suggestions.push((
776+
suggestion.candidate,
777+
self.def_path_str(def_id),
778+
kind,
779+
));
780+
}
781+
}
782+
}
783+
let suggestions = if !local_suggestions.is_empty() {
784+
// There is at least one item available in the current scope that is a
785+
// likely typo. We only show those.
786+
local_suggestions
787+
} else {
788+
suggestions
789+
};
790+
for (name, sugg, kind) in suggestions {
791+
err.span_suggestion_verbose(
792+
span,
793+
format!(
794+
"you might have meant to use the similarly named {kind} `{name}`",
795+
),
796+
sugg,
797+
Applicability::MaybeIncorrect,
798+
);
799+
suggested_typo = true;
800+
}
801+
}
802+
if import_suggestions.is_empty() && !suggested_typo {
697803
let help_msg = format!(
698-
"if you meant to match on a variant or a `const` item, consider \
699-
making the path in the pattern qualified: `path::to::ModOrType::{name}`",
804+
"if you meant to match on a unit struct, unit variant or a `const` \
805+
item, consider making the path in the pattern qualified: \
806+
`path::to::ModOrType::{name}`",
700807
);
701808
err.span_help(span, help_msg);
702809
}
@@ -1016,6 +1123,39 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
10161123
.emit()
10171124
}
10181125

1126+
fn def_path_str(&self, mut def_id: DefId) -> String {
1127+
// We can't use `def_path_str` in resolve.
1128+
let mut path = vec![def_id];
1129+
while let Some(parent) = self.tcx.opt_parent(def_id) {
1130+
def_id = parent;
1131+
path.push(def_id);
1132+
if def_id.is_top_level_module() {
1133+
break;
1134+
}
1135+
}
1136+
// We will only suggest importing directly if it is accessible through that path.
1137+
path.into_iter()
1138+
.rev()
1139+
.map(|def_id| {
1140+
self.tcx
1141+
.opt_item_name(def_id)
1142+
.map(|name| {
1143+
match (
1144+
def_id.is_top_level_module(),
1145+
def_id.is_local(),
1146+
self.tcx.sess.edition(),
1147+
) {
1148+
(true, true, Edition::Edition2015) => String::new(),
1149+
(true, true, _) => kw::Crate.to_string(),
1150+
(true, false, _) | (false, _, _) => name.to_string(),
1151+
}
1152+
})
1153+
.unwrap_or_else(|| "_".to_string())
1154+
})
1155+
.collect::<Vec<String>>()
1156+
.join("::")
1157+
}
1158+
10191159
pub(crate) fn add_scope_set_candidates(
10201160
&mut self,
10211161
suggestions: &mut Vec<TypoSuggestion>,
@@ -3396,7 +3536,7 @@ impl UsePlacementFinder {
33963536
}
33973537
}
33983538

3399-
impl<'tcx> visit::Visitor<'tcx> for UsePlacementFinder {
3539+
impl<'tcx> Visitor<'tcx> for UsePlacementFinder {
34003540
fn visit_crate(&mut self, c: &Crate) {
34013541
if self.target_module == CRATE_NODE_ID {
34023542
let inject = c.spans.inject_use_span;
@@ -3424,6 +3564,22 @@ impl<'tcx> visit::Visitor<'tcx> for UsePlacementFinder {
34243564
}
34253565
}
34263566

3567+
#[derive(Default)]
3568+
struct BindingVisitor {
3569+
identifiers: Vec<Symbol>,
3570+
spans: FxHashMap<Symbol, Vec<Span>>,
3571+
}
3572+
3573+
impl<'tcx> Visitor<'tcx> for BindingVisitor {
3574+
fn visit_pat(&mut self, pat: &ast::Pat) {
3575+
if let ast::PatKind::Ident(_, ident, _) = pat.kind {
3576+
self.identifiers.push(ident.name);
3577+
self.spans.entry(ident.name).or_default().push(ident.span);
3578+
}
3579+
visit::walk_pat(self, pat);
3580+
}
3581+
}
3582+
34273583
fn search_for_any_use_in_items(items: &[Box<ast::Item>]) -> Option<Span> {
34283584
for item in items {
34293585
if let ItemKind::Use(..) = item.kind

compiler/rustc_resolve/src/errors.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,18 @@ pub(crate) struct VariableNotInAllPatterns {
986986
pub(crate) span: Span,
987987
}
988988

989+
#[derive(Subdiagnostic)]
990+
#[multipart_suggestion(
991+
resolve_variable_is_a_typo,
992+
applicability = "maybe-incorrect",
993+
style = "verbose"
994+
)]
995+
pub(crate) struct PatternBindingTypo {
996+
#[suggestion_part(code = "{typo}")]
997+
pub(crate) spans: Vec<Span>,
998+
pub(crate) typo: Symbol,
999+
}
1000+
9891001
#[derive(Diagnostic)]
9901002
#[diag(resolve_name_defined_multiple_time)]
9911003
#[note]

0 commit comments

Comments
 (0)