rust-lang/rust
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Rollup merge of #145827 - estebank:issue-51976, r=jackh726

Browse Source 
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 rust-lang/rust#51976.
This commit is contained in:
Stuart Cook
2025-09-04 10:01:54 +10:00
committed by GitHub
parent d71a9b6bcf 86085b4f65
commit 3a6ae1167f
22 changed files with 911 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

172
compiler/rustc_resolve/src/diagnostics.rs
View File

@@ -661,8 +661,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
ResolutionError::VariableNotBoundInPattern(binding_error, parent_scope) => {
let BindingError { name, target, origin, could_be_path } = binding_error;
let target_sp = target.iter().copied().collect::<Vec<_>>();
let origin_sp = origin.iter().copied().collect::<Vec<_>>();
let mut target_sp = target.iter().map(|pat| pat.span).collect::<Vec<_>>();
target_sp.sort();
target_sp.dedup();
let mut origin_sp = origin.iter().map(|(span, _)| *span).collect::<Vec<_>>();
origin_sp.sort();
origin_sp.dedup();
let msp = MultiSpan::from_spans(target_sp.clone());
let mut err = self
@@ -671,8 +675,35 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
for sp in target_sp {
err.subdiagnostic(errors::PatternDoesntBindName { span: sp, name });
}
for sp in origin_sp {
err.subdiagnostic(errors::VariableNotInAllPatterns { span: sp });
for sp in &origin_sp {
err.subdiagnostic(errors::VariableNotInAllPatterns { span: *sp });
}
let mut suggested_typo = false;
if !target.iter().all(|pat| matches!(pat.kind, ast::PatKind::Ident(..)))
&& !origin.iter().all(|(_, pat)| matches!(pat.kind, ast::PatKind::Ident(..)))
{
// The check above is so that when we encounter `match foo { (a | b) => {} }`,
// we don't suggest `(a | a) => {}`, which would never be what the user wants.
let mut target_visitor = BindingVisitor::default();
for pat in &target {
target_visitor.visit_pat(pat);
}
target_visitor.identifiers.sort();
target_visitor.identifiers.dedup();
let mut origin_visitor = BindingVisitor::default();
for (_, pat) in &origin {
origin_visitor.visit_pat(pat);
}
origin_visitor.identifiers.sort();
origin_visitor.identifiers.dedup();
// Find if the binding could have been a typo
if let Some(typo) =
find_best_match_for_name(&target_visitor.identifiers, name.name, None)
&& !origin_visitor.identifiers.contains(&typo)
{
err.subdiagnostic(errors::PatternBindingTypo { spans: origin_sp, typo });
suggested_typo = true;
}
}
if could_be_path {
let import_suggestions = self.lookup_import_candidates(
@@ -693,10 +724,86 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
},
);
if import_suggestions.is_empty() {
if import_suggestions.is_empty() && !suggested_typo {
let kinds = [
DefKind::Ctor(CtorOf::Variant, CtorKind::Const),
DefKind::Ctor(CtorOf::Struct, CtorKind::Const),
DefKind::Const,
DefKind::AssocConst,
];
let mut local_names = vec![];
self.add_module_candidates(
parent_scope.module,
&mut local_names,
&|res| matches!(res, Res::Def(_, _)),
None,
);
let local_names: FxHashSet<_> = local_names
.into_iter()
.filter_map(|s| match s.res {
Res::Def(_, def_id) => Some(def_id),
_ => None,
})
.collect();
let mut local_suggestions = vec![];
let mut suggestions = vec![];
for kind in kinds {
if let Some(suggestion) = self.early_lookup_typo_candidate(
ScopeSet::All(Namespace::ValueNS),
&parent_scope,
name,
&|res: Res| match res {
Res::Def(k, _) => k == kind,
_ => false,
},
) && let Res::Def(kind, mut def_id) = suggestion.res
{
if let DefKind::Ctor(_, _) = kind {
def_id = self.tcx.parent(def_id);
}
let kind = kind.descr(def_id);
if local_names.contains(&def_id) {
// The item is available in the current scope. Very likely to
// be a typo. Don't use the full path.
local_suggestions.push((
suggestion.candidate,
suggestion.candidate.to_string(),
kind,
));
} else {
suggestions.push((
suggestion.candidate,
self.def_path_str(def_id),
kind,
));
}
}
}
let suggestions = if !local_suggestions.is_empty() {
// There is at least one item available in the current scope that is a
// likely typo. We only show those.
local_suggestions
} else {
suggestions
};
for (name, sugg, kind) in suggestions {
err.span_suggestion_verbose(
span,
format!(
"you might have meant to use the similarly named {kind} `{name}`",
),
sugg,
Applicability::MaybeIncorrect,
);
suggested_typo = true;
}
}
if import_suggestions.is_empty() && !suggested_typo {
let help_msg = format!(
"if you meant to match on a variant or a `const` item, consider \
making the path in the pattern qualified: `path::to::ModOrType::{name}`",
"if you meant to match on a unit struct, unit variant or a `const` \
item, consider making the path in the pattern qualified: \
`path::to::ModOrType::{name}`",
);
err.span_help(span, help_msg);
}
@@ -1016,6 +1123,39 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
.emit()
}
fn def_path_str(&self, mut def_id: DefId) -> String {
// We can't use `def_path_str` in resolve.
let mut path = vec![def_id];
while let Some(parent) = self.tcx.opt_parent(def_id) {
def_id = parent;
path.push(def_id);
if def_id.is_top_level_module() {
break;
}
}
// We will only suggest importing directly if it is accessible through that path.
path.into_iter()
.rev()
.map(|def_id| {
self.tcx
.opt_item_name(def_id)
.map(|name| {
match (
def_id.is_top_level_module(),
def_id.is_local(),
self.tcx.sess.edition(),
) {
(true, true, Edition::Edition2015) => String::new(),
(true, true, _) => kw::Crate.to_string(),
(true, false, _) | (false, _, _) => name.to_string(),
}
})
.unwrap_or_else(|| "_".to_string())
})
.collect::<Vec<String>>()
.join("::")
}
pub(crate) fn add_scope_set_candidates(
&mut self,
suggestions: &mut Vec<TypoSuggestion>,
@@ -3396,7 +3536,7 @@ impl UsePlacementFinder {
}
}
impl<'tcx> visit::Visitor<'tcx> for UsePlacementFinder {
impl<'tcx> Visitor<'tcx> for UsePlacementFinder {
fn visit_crate(&mut self, c: &Crate) {
if self.target_module == CRATE_NODE_ID {
let inject = c.spans.inject_use_span;
@@ -3424,6 +3564,22 @@ impl<'tcx> visit::Visitor<'tcx> for UsePlacementFinder {
}
}
#[derive(Default)]
struct BindingVisitor {
identifiers: Vec<Symbol>,
spans: FxHashMap<Symbol, Vec<Span>>,
}
impl<'tcx> Visitor<'tcx> for BindingVisitor {
fn visit_pat(&mut self, pat: &ast::Pat) {
if let ast::PatKind::Ident(_, ident, _) = pat.kind {
self.identifiers.push(ident.name);
self.spans.entry(ident.name).or_default().push(ident.span);
}
visit::walk_pat(self, pat);
}
}
fn search_for_any_use_in_items(items: &[Box<ast::Item>]) -> Option<Span> {
for item in items {
if let ItemKind::Use(..) = item.kind

12
compiler/rustc_resolve/src/errors.rs
View File

@@ -986,6 +986,18 @@ pub(crate) struct VariableNotInAllPatterns {
pub(crate) span: Span,
}
#[derive(Subdiagnostic)]
#[multipart_suggestion(
resolve_variable_is_a_typo,
applicability = "maybe-incorrect",
style = "verbose"
)]
pub(crate) struct PatternBindingTypo {
#[suggestion_part(code = "{typo}")]
pub(crate) spans: Vec<Span>,
pub(crate) typo: Symbol,
}
#[derive(Diagnostic)]
#[diag(resolve_name_defined_multiple_time)]
#[note]

50
compiler/rustc_resolve/src/late.rs
View File

@@ -8,7 +8,6 @@
use std::assert_matches::debug_assert_matches;
use std::borrow::Cow;
use std::collections::BTreeSet;
use std::collections::hash_map::Entry;
use std::mem::{replace, swap, take};
@@ -3682,31 +3681,30 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
// 2) Record any missing bindings or binding mode inconsistencies.
for (map_outer, pat_outer) in not_never_pats.iter() {
// Check against all arms except for the same pattern which is always self-consistent.
let inners = not_never_pats
.iter()
.filter(|(_, pat)| pat.id != pat_outer.id)
.flat_map(|(map, _)| map);
let inners = not_never_pats.iter().filter(|(_, pat)| pat.id != pat_outer.id);
for (&name, binding_inner) in inners {
match map_outer.get(&name) {
None => {
// The inner binding is missing in the outer.
let binding_error =
missing_vars.entry(name).or_insert_with(|| BindingError {
name,
origin: BTreeSet::new(),
target: BTreeSet::new(),
could_be_path: name.as_str().starts_with(char::is_uppercase),
});
binding_error.origin.insert(binding_inner.span);
binding_error.target.insert(pat_outer.span);
}
Some(binding_outer) => {
if binding_outer.annotation != binding_inner.annotation {
// The binding modes in the outer and inner bindings differ.
inconsistent_vars
.entry(name)
.or_insert((binding_inner.span, binding_outer.span));
for (map, pat) in inners {
for (&name, binding_inner) in map {
match map_outer.get(&name) {
None => {
// The inner binding is missing in the outer.
let binding_error =
missing_vars.entry(name).or_insert_with(|| BindingError {
name,
origin: Default::default(),
target: Default::default(),
could_be_path: name.as_str().starts_with(char::is_uppercase),
});
binding_error.origin.push((binding_inner.span, (***pat).clone()));
binding_error.target.push((***pat_outer).clone());
}
Some(binding_outer) => {
if binding_outer.annotation != binding_inner.annotation {
// The binding modes in the outer and inner bindings differ.
inconsistent_vars
.entry(name)
.or_insert((binding_inner.span, binding_outer.span));
}
}
}
}
@@ -3719,7 +3717,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
v.could_be_path = false;
}
self.report_error(
*v.origin.iter().next().unwrap(),
v.origin.iter().next().unwrap().0,
ResolutionError::VariableNotBoundInPattern(v, self.parent_scope),
);
}

4
compiler/rustc_resolve/src/lib.rs
View File

@@ -230,8 +230,8 @@ enum Used {
#[derive(Debug)]
struct BindingError {
name: Ident,
origin: BTreeSet<Span>,
target: BTreeSet<Span>,
origin: Vec<(Span, ast::Pat)>,
target: Vec<ast::Pat>,
could_be_path: bool,
}