Skip to content

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

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.

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

Merged
merged 1 commit into from
Jul 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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)
Comment on lines +1045 to +1047
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think while it makes sense to have #[rustc_confusables] because #[doc(alias)] pollutes the search, it still makes sense to actually check both for diagnostics, so that we don't have to write 2 annotations when #[doc(alias)] is already present

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe #107108 already makes the #[doc(alias)] check to make suggestions. #[rustc_confusables] shouldn't trigger if any similar_candidate was present to make the #[doc(alias)] "there is a similar method named ..." suggestion.

{
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`.