Skip to content

Commit

Permalink
Auto merge of #112239 - jieyouxu:targeted-no-method-suggestions, r=cj…
Browse files Browse the repository at this point in the history
…gillot

Add `#[rustc_confusables]` attribute to allow targeted "no method" error suggestions on standard library types

After this PR, the standard library developer can annotate methods on e.g. `BTreeSet::push` with `#[rustc_confusables("insert")]`. When the user mistypes `btreeset.push()`, `BTreeSet::insert` will be suggested if there are no other candidates to suggest. This PR lays the foundations for contributors to add `rustc_confusables` annotations to standard library types for targeted suggestions, as specified in #59450, or to address cases such as #108437.

### Example

Assume `BTreeSet` is the standard library type:

```
// Standard library definition
#![feature(rustc_attrs)]

struct BTreeSet;

impl BTreeSet {
    #[rustc_confusables("push")]
    fn insert(&self) {}
}

// User code
fn main() {
    let x = BTreeSet {};
    x.push();
}
```

A new suggestion (which has lower precedence than suggestions for misspellings and only is shown when there are no misspellings suggestions) will be added to hint the user maybe they intended to write `x.insert()` instead:

```
error[E0599]: no method named `push` found for struct `BTreeSet` in the current scope
  --> test.rs:12:7
   |
3  | struct BTreeSet;
   | --------------- method `push` not found for this struct
...
12 |     x.push();
   |       ^^^^ method not found in `BTreeSet`
   |
help: you might have meant to use `insert`
   |
12 |     x.insert();
   |       ~~~~~~

error: aborting due to previous error
```
  • Loading branch information
bors committed Jul 16, 2023
2 parents 4a07b2b + 08c77a6 commit 11da267
Show file tree
Hide file tree
Showing 12 changed files with 259 additions and 4 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3665,6 +3665,7 @@ name = "rustc_hir_typeck"
version = "0.1.0"
dependencies = [
"rustc_ast",
"rustc_attr",
"rustc_data_structures",
"rustc_errors",
"rustc_fluent_macro",
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1217,3 +1217,20 @@ pub fn parse_alignment(node: &ast::LitKind) -> Result<u32, &'static str> {
Err("not an unsuffixed integer")
}
}

/// Read the content of a `rustc_confusables` attribute, and return the list of candidate names.
pub fn parse_confusables(attr: &Attribute) -> Option<Vec<Symbol>> {
let meta = attr.meta()?;
let MetaItem { kind: MetaItemKind::List(ref metas), .. } = meta else { return None };

let mut candidates = Vec::new();

for meta in metas {
let NestedMetaItem::Lit(meta_lit) = meta else {
return None;
};
candidates.push(meta_lit.symbol);
}

return Some(candidates);
}
6 changes: 6 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
ErrorFollowing,
INTERNAL_UNSTABLE
),
rustc_attr!(
rustc_confusables, Normal,
template!(List: r#""name1", "name2", ..."#),
ErrorFollowing,
INTERNAL_UNSTABLE,
),
// Enumerates "identity-like" conversion methods to suggest on type mismatch.
rustc_attr!(
rustc_conversion_suggestion, Normal, template!(Word), WarnFollowing, INTERNAL_UNSTABLE
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ edition = "2021"
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
tracing = "0.1"
rustc_ast = { path = "../rustc_ast" }
rustc_attr = { path = "../rustc_attr" }
rustc_data_structures = { path = "../rustc_data_structures" }
rustc_errors = { path = "../rustc_errors" }
rustc_graphviz = { path = "../rustc_graphviz" }
Expand Down
29 changes: 25 additions & 4 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@
//! found or is otherwise invalid.

use crate::errors;
use crate::errors::CandidateTraitNote;
use crate::errors::NoAssociatedItem;
use crate::errors::{CandidateTraitNote, NoAssociatedItem};
use crate::Expectation;
use crate::FnCtxt;
use rustc_ast::ast::Mutability;
use rustc_data_structures::fx::FxIndexMap;
use rustc_data_structures::fx::FxIndexSet;
use rustc_attr::parse_confusables;
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_data_structures::unord::UnordSet;
use rustc_errors::StashKey;
use rustc_errors::{
Expand Down Expand Up @@ -1038,6 +1037,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"the {item_kind} was found for\n{}{}",
type_candidates, additional_types
));
} else {
'outer: for inherent_impl_did in self.tcx.inherent_impls(adt.did()) {
for inherent_method in
self.tcx.associated_items(inherent_impl_did).in_definition_order()
{
if let Some(attr) = self.tcx.get_attr(inherent_method.def_id, sym::rustc_confusables)
&& let Some(candidates) = parse_confusables(attr)
&& candidates.contains(&item_name.name)
{
err.span_suggestion_verbose(
item_name.span,
format!(
"you might have meant to use `{}`",
inherent_method.name.as_str()
),
inherent_method.name.as_str(),
Applicability::MaybeIncorrect,
);
break 'outer;
}
}
}
}
}
} else {
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ passes_collapse_debuginfo =
`collapse_debuginfo` attribute should be applied to macro definitions
.label = not a macro definition
passes_confusables = attribute should be applied to an inherent method
.label = not an inherent method
passes_const_impl_const_trait =
const `impl`s must be for traits marked with `#[const_trait]`
.note = this trait must be annotated with `#[const_trait]`
Expand Down Expand Up @@ -266,6 +269,9 @@ passes_duplicate_lang_item_crate_depends =
.first_definition_path = first definition in `{$orig_crate_name}` loaded from {$orig_path}
.second_definition_path = second definition in `{$crate_name}` loaded from {$path}
passes_empty_confusables =
expected at least one confusable name
passes_export_name =
attribute should be applied to a free function, impl method or static
.label = not a free function, impl method or static
Expand Down Expand Up @@ -326,6 +332,9 @@ passes_implied_feature_not_exist =
passes_incorrect_do_not_recommend_location =
`#[do_not_recommend]` can only be placed on trait implementations
passes_incorrect_meta_item = expected a quoted string literal
passes_incorrect_meta_item_suggestion = consider surrounding this with quotes
passes_incorrect_target =
`{$name}` language item must be applied to a {$kind} with {$at_least ->
[true] at least {$num}
Expand Down
41 changes: 41 additions & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ impl CheckAttrVisitor<'_> {
| sym::rustc_allowed_through_unstable_modules
| sym::rustc_promotable => self.check_stability_promotable(&attr, span, target),
sym::link_ordinal => self.check_link_ordinal(&attr, span, target),
sym::rustc_confusables => self.check_confusables(&attr, target),
_ => true,
};

Expand Down Expand Up @@ -1985,6 +1986,46 @@ impl CheckAttrVisitor<'_> {
}
}

fn check_confusables(&self, attr: &Attribute, target: Target) -> bool {
match target {
Target::Method(MethodKind::Inherent) => {
let Some(meta) = attr.meta() else {
return false;
};
let ast::MetaItem { kind: MetaItemKind::List(ref metas), .. } = meta else {
return false;
};

let mut candidates = Vec::new();

for meta in metas {
let NestedMetaItem::Lit(meta_lit) = meta else {
self.tcx.sess.emit_err(errors::IncorrectMetaItem {
span: meta.span(),
suggestion: errors::IncorrectMetaItemSuggestion {
lo: meta.span().shrink_to_lo(),
hi: meta.span().shrink_to_hi(),
},
});
return false;
};
candidates.push(meta_lit.symbol);
}

if candidates.is_empty() {
self.tcx.sess.emit_err(errors::EmptyConfusables { span: attr.span });
return false;
}

true
}
_ => {
self.tcx.sess.emit_err(errors::Confusables { attr_span: attr.span });
false
}
}
}

fn check_deprecated(&self, hir_id: HirId, attr: &Attribute, _span: Span, target: Target) {
match target {
Target::Closure | Target::Expression | Target::Statement | Target::Arm => {
Expand Down
32 changes: 32 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,38 @@ pub struct LinkOrdinal {
pub attr_span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_confusables)]
pub struct Confusables {
#[primary_span]
pub attr_span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_empty_confusables)]
pub(crate) struct EmptyConfusables {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_incorrect_meta_item, code = "E0539")]
pub(crate) struct IncorrectMetaItem {
#[primary_span]
pub span: Span,
#[subdiagnostic]
pub suggestion: IncorrectMetaItemSuggestion,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(passes_incorrect_meta_item_suggestion, applicability = "maybe-incorrect")]
pub(crate) struct IncorrectMetaItemSuggestion {
#[suggestion_part(code = "\"")]
pub lo: Span,
#[suggestion_part(code = "\"")]
pub hi: Span,
}

#[derive(Diagnostic)]
#[diag(passes_stability_promotable)]
pub struct StabilityPromotable {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1265,6 +1265,7 @@ symbols! {
rustc_clean,
rustc_coherence_is_core,
rustc_coinductive,
rustc_confusables,
rustc_const_stable,
rustc_const_unstable,
rustc_conversion_suggestion,
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/attributes/auxiliary/rustc_confusables_across_crate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![feature(rustc_attrs)]

pub struct BTreeSet;

impl BTreeSet {
#[rustc_confusables("push", "test_b")]
pub fn insert(&self) {}

#[rustc_confusables("pulled")]
pub fn pull(&self) {}
}
47 changes: 47 additions & 0 deletions tests/ui/attributes/rustc_confusables.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// aux-build: rustc_confusables_across_crate.rs

#![feature(rustc_attrs)]

extern crate rustc_confusables_across_crate;

use rustc_confusables_across_crate::BTreeSet;

fn main() {
// Misspellings (similarly named methods) take precedence over `rustc_confusables`.
let x = BTreeSet {};
x.inser();
//~^ ERROR no method named
//~| HELP there is a method with a similar name
x.foo();
//~^ ERROR no method named
x.push();
//~^ ERROR no method named
//~| HELP you might have meant to use `insert`
x.test();
//~^ ERROR no method named
x.pulled();
//~^ ERROR no method named
//~| HELP there is a method with a similar name
}

struct Bar;

impl Bar {
#[rustc_confusables()]
//~^ ERROR expected at least one confusable name
fn baz() {}

#[rustc_confusables]
//~^ ERROR malformed `rustc_confusables` attribute input
//~| HELP must be of the form
fn qux() {}

#[rustc_confusables(invalid_meta_item)]
//~^ ERROR expected a quoted string literal
//~| HELP consider surrounding this with quotes
fn quux() {}
}

#[rustc_confusables("blah")]
//~^ ERROR attribute should be applied to an inherent method
fn not_inherent_impl_method() {}
68 changes: 68 additions & 0 deletions tests/ui/attributes/rustc_confusables.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
error: malformed `rustc_confusables` attribute input
--> $DIR/rustc_confusables.rs:34:5
|
LL | #[rustc_confusables]
| ^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[rustc_confusables("name1", "name2", ...)]`

error: attribute should be applied to an inherent method
--> $DIR/rustc_confusables.rs:45:1
|
LL | #[rustc_confusables("blah")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: expected at least one confusable name
--> $DIR/rustc_confusables.rs:30:5
|
LL | #[rustc_confusables()]
| ^^^^^^^^^^^^^^^^^^^^^^

error[E0539]: expected a quoted string literal
--> $DIR/rustc_confusables.rs:39:25
|
LL | #[rustc_confusables(invalid_meta_item)]
| ^^^^^^^^^^^^^^^^^
|
help: consider surrounding this with quotes
|
LL | #[rustc_confusables("invalid_meta_item")]
| + +

error[E0599]: no method named `inser` found for struct `rustc_confusables_across_crate::BTreeSet` in the current scope
--> $DIR/rustc_confusables.rs:12:7
|
LL | x.inser();
| ^^^^^ help: there is a method with a similar name: `insert`

error[E0599]: no method named `foo` found for struct `rustc_confusables_across_crate::BTreeSet` in the current scope
--> $DIR/rustc_confusables.rs:15:7
|
LL | x.foo();
| ^^^ method not found in `BTreeSet`

error[E0599]: no method named `push` found for struct `rustc_confusables_across_crate::BTreeSet` in the current scope
--> $DIR/rustc_confusables.rs:17:7
|
LL | x.push();
| ^^^^ method not found in `BTreeSet`
|
help: you might have meant to use `insert`
|
LL | x.insert();
| ~~~~~~

error[E0599]: no method named `test` found for struct `rustc_confusables_across_crate::BTreeSet` in the current scope
--> $DIR/rustc_confusables.rs:20:7
|
LL | x.test();
| ^^^^ method not found in `BTreeSet`

error[E0599]: no method named `pulled` found for struct `rustc_confusables_across_crate::BTreeSet` in the current scope
--> $DIR/rustc_confusables.rs:22:7
|
LL | x.pulled();
| ^^^^^^ help: there is a method with a similar name: `pull`

error: aborting due to 9 previous errors

Some errors have detailed explanations: E0539, E0599.
For more information about an error, try `rustc --explain E0539`.

0 comments on commit 11da267

Please sign in to comment.