-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Tweak suggestion for this
-> self
#75509
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ use crate::{PathResult, PathSource, Segment}; | |
|
||
use rustc_ast::ast::{self, Expr, ExprKind, Item, ItemKind, NodeId, Path, Ty, TyKind}; | ||
use rustc_ast::util::lev_distance::find_best_match_for_name; | ||
use rustc_ast::visit::FnKind; | ||
use rustc_data_structures::fx::FxHashSet; | ||
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder}; | ||
use rustc_hir as hir; | ||
|
@@ -174,16 +175,39 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { | |
let code = source.error_code(res.is_some()); | ||
let mut err = self.r.session.struct_span_err_with_code(base_span, &base_msg, code); | ||
|
||
let is_assoc_fn = self.self_type_is_available(span); | ||
// Emit help message for fake-self from other languages (e.g., `this` in Javascript). | ||
if ["this", "my"].contains(&&*item_str.as_str()) | ||
&& self.self_value_is_available(path[0].ident.span, span) | ||
{ | ||
if ["this", "my"].contains(&&*item_str.as_str()) && is_assoc_fn { | ||
err.span_suggestion_short( | ||
span, | ||
"you might have meant to use `self` here instead", | ||
"self".to_string(), | ||
Applicability::MaybeIncorrect, | ||
); | ||
if !self.self_value_is_available(path[0].ident.span, span) { | ||
if let Some((FnKind::Fn(_, _, sig, ..), fn_span)) = | ||
&self.diagnostic_metadata.current_function | ||
{ | ||
let (span, sugg) = if let Some(param) = sig.decl.inputs.get(0) { | ||
(param.span.shrink_to_lo(), "&self, ") | ||
} else { | ||
( | ||
self.r | ||
.session | ||
.source_map() | ||
.span_through_char(*fn_span, '(') | ||
.shrink_to_hi(), | ||
"&self", | ||
) | ||
}; | ||
err.span_suggestion_verbose( | ||
span, | ||
"you are also missing a `self` receiver argument", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this is a slightly too strong suggestion as there are often cases where I instead have to rename Maybe "you might be missing a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This suggestion is predicated on the previous one also being "correct". I can change the wording to something like "if you meant to use |
||
sugg.to_string(), | ||
Applicability::MaybeIncorrect, | ||
); | ||
} | ||
} | ||
} | ||
|
||
// Emit special messages for unresolved `Self` and `self`. | ||
|
@@ -212,7 +236,38 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { | |
if fn_kind.decl().inputs.get(0).map(|p| p.is_self()).unwrap_or(false) { | ||
err.span_label(*span, "this function has a `self` parameter, but a macro invocation can only access identifiers it receives from parameters"); | ||
} else { | ||
err.span_label(*span, "this function doesn't have a `self` parameter"); | ||
let doesnt = if is_assoc_fn { | ||
let (span, sugg) = fn_kind | ||
.decl() | ||
.inputs | ||
.get(0) | ||
.map(|p| (p.span.shrink_to_lo(), "&self, ")) | ||
.unwrap_or_else(|| { | ||
( | ||
self.r | ||
.session | ||
.source_map() | ||
.span_through_char(*span, '(') | ||
.shrink_to_hi(), | ||
"&self", | ||
) | ||
}); | ||
err.span_suggestion_verbose( | ||
span, | ||
"add a `self` receiver parameter to make the associated `fn` a method", | ||
sugg.to_string(), | ||
Applicability::MaybeIncorrect, | ||
); | ||
"doesn't" | ||
} else { | ||
"can't" | ||
}; | ||
if let Some(ident) = fn_kind.ident() { | ||
err.span_label( | ||
ident.span, | ||
&format!("this function {} have a `self` parameter", doesnt), | ||
); | ||
} | ||
} | ||
} | ||
return (err, Vec::new()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,21 +1,37 @@ | ||
error[E0424]: expected value, found module `self` | ||
--> $DIR/E0424.rs:7:9 | ||
| | ||
LL | / fn foo() { | ||
LL | | self.bar(); | ||
| | ^^^^ `self` value is a keyword only available in methods with a `self` parameter | ||
LL | | } | ||
| |_____- this function doesn't have a `self` parameter | ||
LL | fn foo() { | ||
| --- this function doesn't have a `self` parameter | ||
LL | self.bar(); | ||
| ^^^^ `self` value is a keyword only available in methods with a `self` parameter | ||
| | ||
help: add a `self` receiver parameter to make the associated `fn` a method | ||
| | ||
LL | fn foo(&self) { | ||
| ^^^^^ | ||
|
||
error[E0424]: expected value, found module `self` | ||
--> $DIR/E0424.rs:11:9 | ||
| | ||
LL | fn baz(_: i32) { | ||
| --- this function doesn't have a `self` parameter | ||
LL | self.bar(); | ||
| ^^^^ `self` value is a keyword only available in methods with a `self` parameter | ||
| | ||
help: add a `self` receiver parameter to make the associated `fn` a method | ||
| | ||
LL | fn baz(&self, _: i32) { | ||
| ^^^^^^ | ||
|
||
error[E0424]: expected unit struct, unit variant or constant, found module `self` | ||
--> $DIR/E0424.rs:12:9 | ||
--> $DIR/E0424.rs:16:9 | ||
| | ||
LL | / fn main () { | ||
LL | | let self = "self"; | ||
| | ^^^^ `self` value is a keyword and may not be bound to variables or shadowed | ||
LL | | } | ||
| |_- this function doesn't have a `self` parameter | ||
LL | fn main () { | ||
| ---- this function can't have a `self` parameter | ||
LL | let self = "self"; | ||
| ^^^^ `self` value is a keyword and may not be bound to variables or shadowed | ||
|
||
error: aborting due to 2 previous errors | ||
error: aborting due to 3 previous errors | ||
|
||
For more information about this error, try `rustc --explain E0424`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,10 @@ | ||
trait B < A > { fn a() -> A { this.a } } //~ ERROR cannot find value `this` in this scope | ||
trait B <A> { | ||
fn a() -> A { | ||
this.a //~ ERROR cannot find value `this` in this scope | ||
} | ||
fn b(x: i32) { | ||
this.b(x); //~ ERROR cannot find value `this` in this scope | ||
} | ||
} | ||
|
||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,33 @@ | ||
error[E0425]: cannot find value `this` in this scope | ||
--> $DIR/issue-5099.rs:1:31 | ||
--> $DIR/issue-5099.rs:3:9 | ||
| | ||
LL | trait B < A > { fn a() -> A { this.a } } | ||
| ^^^^ not found in this scope | ||
LL | this.a | ||
| ^^^^ not found in this scope | ||
| | ||
help: you might have meant to use `self` here instead | ||
| | ||
LL | self.a | ||
| ^^^^ | ||
help: you are also missing a `self` receiver argument | ||
| | ||
LL | fn a(&self) -> A { | ||
| ^^^^^ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
^ |
||
|
||
error[E0425]: cannot find value `this` in this scope | ||
--> $DIR/issue-5099.rs:6:9 | ||
| | ||
LL | this.b(x); | ||
| ^^^^ not found in this scope | ||
| | ||
help: you might have meant to use `self` here instead | ||
| | ||
LL | self.b(x); | ||
| ^^^^ | ||
help: you are also missing a `self` receiver argument | ||
| | ||
LL | fn b(&self, x: i32) { | ||
| ^^^^^^ | ||
|
||
error: aborting due to previous error | ||
error: aborting due to 2 previous errors | ||
|
||
For more information about this error, try `rustc --explain E0425`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this be incorrect for
fn test<const N: [u8; 3 / (7 + 2)]>()
. Not sure how easy this is to fix but it seems like getting an empty span after the opening paren of the argument list might be something we can put into a method after getting it right once.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct. This is not the only place where that problem exists though, and wanted to keep this PR small by not threading a
Span
from the AST all the way to resolve for this corner case. It seems to me that the cases where the suggestion is needed are unlikely to be using const generics, but even then they are not yet stable :)I'll follow up in a bigger PR to refactor all the
Span
wrangling cases at a later date, including this one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't even be a problem with
min_const_generics
because arrays won't be allowed as const params at the start, so this isn't really a pressing issue, though it is something I should keep in mind before trying to stabilize it 😅