Skip to content
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

Do not provide suggestions when the spans come from expanded code that doesn't point at user code #109082

Closed
wants to merge 14 commits into from
Closed
Prev Previous commit
Next Next commit
wip
estebank committed Apr 26, 2023
commit d42105b3e1ac1b84219e8dc2dac310eee6536ca0
Original file line number Diff line number Diff line change
@@ -618,10 +618,10 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {
let last_lt = &self.gen_args.args[self.num_provided_lifetime_args() - 1];
(self.tcx.mark_span_for_resize(last_lt.span()).shrink_to_hi(), false)
};
let path_sp = self.path_segment.ident.span.peel_ctxt();
let path_sp = self.path_segment.ident.span;
if !self.gen_args.args.iter().all(|arg| {
arg.span().can_be_used_for_suggestions()
&& arg.span().peel_ctxt().ctxt() == path_sp.ctxt()
&& arg.span().peel_ctxt() == path_sp.peel_ctxt()
}) || !path_sp.can_be_used_for_suggestions()
{
// Do not suggest syntax when macros are involved. (#90557)
@@ -670,10 +670,10 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {
);
}
AngleBrackets::Available => {
let path_sp = self.path_segment.ident.span.peel_ctxt();
let path_sp = self.path_segment.ident.span;
if !self.gen_args.args.iter().all(|arg| {
arg.span().can_be_used_for_suggestions()
&& arg.span().peel_ctxt().ctxt() == path_sp.ctxt()
&& arg.span().peel_ctxt() == path_sp.peel_ctxt()
}) || !path_sp.can_be_used_for_suggestions()
{
// Do not suggest syntax when macros are involved. (#90557)
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/lint.rs
Original file line number Diff line number Diff line change
@@ -466,7 +466,7 @@ pub fn struct_lint_level(
/// This is used to test whether a lint should not even begin to figure out whether it should
/// be reported on the current node.
pub fn in_external_macro(sess: &Session, span: Span) -> bool {
let expn_data = span.peel_ctxt().ctxt().outer_expn_data();
let expn_data = span.peel_ctxt().outer_expn_data();
match expn_data.kind {
ExpnKind::Inlined
| ExpnKind::Root
11 changes: 3 additions & 8 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
@@ -2400,15 +2400,10 @@ impl<'tcx> TyCtxt<'tcx> {
/// `Span` before it was changed. This is used today only to be able to identify `Span`s coming
/// from a proc-macro even when it was modified, to avoid giving spurious suggestions when the
/// `Span` points at an attribute and not user code.
#[inline(always)]
pub fn mark_span_for_resize(self, span: Span) -> Span {
span
// if true {
// return span;
// }
// self.with_stable_hashing_context(|hcx| {
// span.mark_with_reason(None, rustc_span::DesugaringKind::Resize, span.edition(), hcx)
// })
self.with_stable_hashing_context(|hcx| {
span.mark_with_reason(None, rustc_span::DesugaringKind::Resize, span.edition(), hcx)
})
}

pub fn adjust_ident(self, mut ident: Ident, scope: DefId) -> Ident {
36 changes: 16 additions & 20 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -211,9 +211,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
suggestion: None,
}
} else {
let item_span = path.last().unwrap().ident.span;
let sp = item_span.peel_ctxt();
let ctxt_kind = sp.ctxt().outer_expn_data().kind;
let sp = path.last().unwrap().ident.span;
let ctxt_kind = sp.peel_ctxt().outer_expn_data().kind;
let (mod_prefix, mod_str, name, mod_label, suggestion) =
if let ExpnKind::Macro(MacroKind::Attr | MacroKind::Bang, name) = ctxt_kind
&& sp.parent_callsite().map(|p| (p.lo(), p.hi())) == Some((sp.lo(), sp.hi()))
@@ -260,7 +259,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
};

Some((
item_span.shrink_to_lo(),
sp.shrink_to_lo(),
match &item.kind {
AssocItemKind::Fn(..) => "consider using the associated function",
AssocItemKind::Const(..) => "consider using the associated constant",
@@ -304,28 +303,25 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
("`async` blocks are only allowed in Rust 2018 or later".to_string(), suggestion)
} else {
// check if we are in situation of typo like `True` instead of `true`.
let override_suggestion =
if ["true", "false"].contains(&item_str.to_string().to_lowercase().as_str()) {
let item_typo = item_str.to_string().to_lowercase();
Some((item_span, "you may want to use a bool value instead", item_typo))
// FIXME(vincenzopalazzo): make the check smarter,
// and maybe expand with levenshtein distance checks
} else if item_str.as_str() == "printf" {
Some((
item_span,
"you may have meant to use the `print` macro",
"print!".to_owned(),
))
} else {
suggestion
};
let override_suggestion = if ["true", "false"]
.contains(&item_str.to_string().to_lowercase().as_str())
{
let item_typo = item_str.to_string().to_lowercase();
Some((sp, "you may want to use a bool value instead", item_typo))
// FIXME(vincenzopalazzo): make the check smarter,
// and maybe expand with levenshtein distance checks
} else if item_str.as_str() == "printf" {
Some((sp, "you may have meant to use the `print` macro", "print!".to_owned()))
} else {
suggestion
};
(format!("{name}not found in {mod_label}"), override_suggestion)
};

BaseError {
msg: format!("cannot find {expected} `{item_str}` in {mod_prefix}{mod_str}"),
fallback_label,
span: item_span,
span: sp,
span_label: None,
could_be_expr: false,
suggestion,
55 changes: 22 additions & 33 deletions compiler/rustc_span/src/lib.rs
Original file line number Diff line number Diff line change
@@ -584,7 +584,7 @@ impl Span {
/// Returns `true` if this span comes from any kind of macro, desugaring or inlining.
#[inline]
pub fn from_expansion(self) -> bool {
self.peel_ctxt().ctxt() != SyntaxContext::root()
self.peel_ctxt() != SyntaxContext::root()
}

/// Returns `true` if `span` originates in a macro's expansion where debuginfo should be
@@ -769,7 +769,7 @@ impl Span {

/// Checks if this span arises from a compiler desugaring of kind `kind`.
pub fn is_desugaring(self, kind: DesugaringKind) -> bool {
match self.peel_ctxt().ctxt().outer_expn_data().kind {
match self.peel_ctxt().outer_expn_data().kind {
ExpnKind::Desugaring(k) => k == kind,
_ => false,
}
@@ -778,7 +778,7 @@ impl Span {
/// Returns the compiler desugaring that created this span, or `None`
/// if this span is not from a desugaring.
pub fn desugaring_kind(self) -> Option<DesugaringKind> {
match self.peel_ctxt().ctxt().outer_expn_data().kind {
match self.peel_ctxt().outer_expn_data().kind {
ExpnKind::Desugaring(k) => Some(k),
_ => None,
}
@@ -841,10 +841,10 @@ impl Span {
// FIXME(jseyfried): `self.ctxt` should always equal `end.ctxt` here (cf. issue #23480).
// Return the macro span on its own to avoid weird diagnostic output. It is preferable to
// have an incomplete span than a completely nonsensical one.
if self.peel_ctxt().ctxt() != end.peel_ctxt().ctxt() {
if self.peel_ctxt().ctxt() == SyntaxContext::root() {
if self.peel_ctxt() != end.peel_ctxt() {
if self.peel_ctxt() == SyntaxContext::root() {
return end;
} else if end.peel_ctxt().ctxt() == SyntaxContext::root() {
} else if end.peel_ctxt() == SyntaxContext::root() {
return self;
}
// Both spans fall within a macro.
@@ -853,11 +853,7 @@ impl Span {
Span::new(
cmp::min(span_data.lo, end_data.lo),
cmp::max(span_data.hi, end_data.hi),
if self.peel_ctxt().ctxt() == SyntaxContext::root() {
end_data.ctxt
} else {
span_data.ctxt
},
if self.peel_ctxt() == SyntaxContext::root() { end_data.ctxt } else { span_data.ctxt },
if span_data.parent == end_data.parent { span_data.parent } else { None },
)
}
@@ -875,25 +871,22 @@ impl Span {
Span::new(
span_data.hi,
end_data.lo,
if end.peel_ctxt().ctxt() == SyntaxContext::root() {
end_data.ctxt
} else {
span_data.ctxt
},
if end.peel_ctxt() == SyntaxContext::root() { end_data.ctxt } else { span_data.ctxt },
if span_data.parent == end_data.parent { span_data.parent } else { None },
)
}

pub fn peel_ctxt(self) -> Span {
// loop {
// let data = self.data().ctxt.outer_expn_data();
// if let ExpnKind::Desugaring(DesugaringKind::Resize) = data.kind {
// self = data.call_site;
// } else {
// break;
// }
// }
self
pub fn peel_ctxt(self) -> SyntaxContext {
let mut ctxt = self.ctxt();
loop {
let data = ctxt.outer_expn_data();
if let ExpnKind::Desugaring(DesugaringKind::Resize) = data.kind {
ctxt = data.call_site.ctxt();
} else {
break;
}
}
ctxt
}

/// Returns a `Span` from the beginning of `self` until the beginning of `end`.
@@ -915,9 +908,9 @@ impl Span {
// Return the macro span on its own to avoid weird diagnostic output. It is preferable to
// have an incomplete span than a completely nonsensical one.
if span_data.ctxt != end_data.ctxt {
if self.peel_ctxt().ctxt() == SyntaxContext::root() {
if self.peel_ctxt() == SyntaxContext::root() {
return end;
} else if end.peel_ctxt().ctxt() == SyntaxContext::root() {
} else if end.peel_ctxt() == SyntaxContext::root() {
return self;
}
// Both spans fall within a macro.
@@ -926,11 +919,7 @@ impl Span {
Span::new(
span_data.lo,
end_data.lo,
if end.peel_ctxt().ctxt() == SyntaxContext::root() {
end_data.ctxt
} else {
span_data.ctxt
},
if end.peel_ctxt() == SyntaxContext::root() { end_data.ctxt } else { span_data.ctxt },
if span_data.parent == end_data.parent { span_data.parent } else { None },
)
}
Original file line number Diff line number Diff line change
@@ -956,7 +956,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
.join(", ");

if matches!(obligation.cause.code(), ObligationCauseCode::FunctionArgumentObligation { .. })
&& match obligation.cause.span.peel_ctxt().ctxt().outer_expn_data().kind {
&& match obligation.cause.span.peel_ctxt().outer_expn_data().kind {
ExpnKind::Root
| ExpnKind::AstPass(_)
| ExpnKind::Desugaring(_)
@@ -1248,7 +1248,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
{
obligation.cause.code()
} else if let ExpnKind::Desugaring(DesugaringKind::ForLoop) =
span.peel_ctxt().ctxt().outer_expn_data().kind
span.peel_ctxt().outer_expn_data().kind
{
obligation.cause.code()
} else {
@@ -1323,7 +1323,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
// }
// ```
if !matches!(
span.peel_ctxt().ctxt().outer_expn_data().kind,
span.peel_ctxt().outer_expn_data().kind,
ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop)
) {
return false;
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/ranges.rs
Original file line number Diff line number Diff line change
@@ -511,11 +511,11 @@ fn y_plus_one<'t>(cx: &LateContext<'_>, expr: &'t Expr<'_>) -> Option<Span> {
rhs,
) => {
if is_integer_const(cx, lhs, 1)
&& lhs.span.peel_ctxt().ctxt() == span.peel_ctxt().ctxt()
&& lhs.span.peel_ctxt() == span.peel_ctxt()
{
Some(lhs.span.to(span))
} else if is_integer_const(cx, rhs, 1)
&& rhs.span.peel_ctxt().ctxt() == span.peel_ctxt().ctxt()
&& rhs.span.peel_ctxt() == span.peel_ctxt()
{
Some(span.to(rhs.span))
} else {
@@ -536,7 +536,7 @@ fn y_minus_one<'t>(cx: &LateContext<'_>, expr: &'t Expr<'_>) -> Option<Span> {
_,
rhs,
) if is_integer_const(cx, rhs, 1)
&& rhs.span.peel_ctxt().ctxt() == span.peel_ctxt().ctxt()
&& rhs.span.peel_ctxt() == span.peel_ctxt()
=> Some(span.to(rhs.span)),
_ => None,
}