Skip to content

Commit

Permalink
Handle proc-macro spans pointing at attribute in suggestions
Browse files Browse the repository at this point in the history 
Hide invalid proc-macro suggestions and track spans
coming from proc-macros pointing at attribute.

Effectively, unless the proc-macro keeps user spans,
suggestions will not be produced for the code they
produce.

Account for desugaring when checking if a span can be used for suggestions

Fix rust-lang#106720.
  • Loading branch information
@estebank
estebank committed Apr 26, 2023
1 parent ae3ab14 commit ba7d52a
Show file tree
Hide file tree
Showing 68 changed files with 2,898 additions and 2,659 deletions.
7 changes: 5 additions & 2 deletions compiler/rustc_ast_lowering/src/path.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
segment_ident_span.find_ancestor_inside(path_span).unwrap_or(path_span)
} else if generic_args.is_empty() {
// If there are brackets, but not generic arguments, then use the opening bracket
generic_args.span.with_hi(generic_args.span.lo() + BytePos(1))
self.tcx.adjust_span(generic_args.span).with_hi(generic_args.span.lo() + BytePos(1))
} else {
// Else use an empty span right after the opening bracket.
generic_args.span.with_lo(generic_args.span.lo() + BytePos(1)).shrink_to_lo()
self.tcx
.adjust_span(generic_args.span)
.with_lo(generic_args.span.lo() + BytePos(1))
.shrink_to_lo()
};

generic_args.args.insert_many(
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2250,12 +2250,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
Ok(string) => {
if string.starts_with("async ") {
let pos = args_span.lo() + BytePos(6);
(args_span.with_lo(pos).with_hi(pos), "move ")
(self.infcx.tcx.adjust_span(args_span).with_lo(pos).with_hi(pos), "move ")
} else if string.starts_with("async|") {
let pos = args_span.lo() + BytePos(5);
(args_span.with_lo(pos).with_hi(pos), " move")
(self.infcx.tcx.adjust_span(args_span).with_lo(pos).with_hi(pos), " move")
} else {
(args_span.shrink_to_lo(), "move ")
(self.infcx.tcx.adjust_span(args_span).shrink_to_lo(), "move ")
}
}
Err(_) => (args_span, "move |<args>| <body>"),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/move_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,15 +470,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
match self.infcx.tcx.sess.source_map().span_to_snippet(span) {
Ok(snippet) if snippet.starts_with('*') => {
err.span_suggestion_verbose(
span.with_hi(span.lo() + BytePos(1)),
self.infcx.tcx.adjust_span(span).with_hi(span.lo() + BytePos(1)),
"consider removing the dereference here",
String::new(),
Applicability::MaybeIncorrect,
);
}
_ => {
err.span_suggestion_verbose(
span.shrink_to_lo(),
self.infcx.tcx.adjust_span(span).shrink_to_lo(),
"consider borrowing here",
'&',
Applicability::MaybeIncorrect,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_builtin_macros/src/alloc_error_handler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use thin_vec::{thin_vec, ThinVec};

pub fn expand(
ecx: &mut ExtCtxt<'_>,
_span: Span,
span: Span,
meta_item: &ast::MetaItem,
item: Annotatable,
) -> Vec<Annotatable> {
Expand All @@ -37,7 +37,7 @@ pub fn expand(
};

// Generate a bunch of new items using the AllocFnFactory
let span = ecx.with_def_site_ctxt(item.span);
let span = ecx.with_def_site_ctxt(span);

// Generate item statements for the allocator methods.
let stmts = thin_vec![generate_handler(ecx, item.ident, span, sig_span)];
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_const_eval/src/transform/check_consts/ops.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,15 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
{
let rhs_pos =
span.lo() + BytePos::from_usize(eq_idx + 2 + rhs_idx);
let rhs_span = span.with_lo(rhs_pos).with_hi(rhs_pos);
let rhs_span =
tcx.adjust_span(span).with_lo(rhs_pos).with_hi(rhs_pos);
err.multipart_suggestion(
"consider dereferencing here",
vec![
(span.shrink_to_lo(), deref.clone()),
(
tcx.adjust_span(span).shrink_to_lo(),
deref.clone(),
),
(rhs_span, deref),
],
Applicability::MachineApplicable,
Expand Down
18 changes: 16 additions & 2 deletions compiler/rustc_errors/src/diagnostic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,10 @@ impl Diagnostic {
.map(|(span, snippet)| SubstitutionPart { snippet, span })
.collect::<Vec<_>>();

if !parts.iter().all(|sub| sub.span.can_be_used_for_suggestions()) {
return self;
}

parts.sort_unstable_by_key(|part| part.span);

assert!(!parts.is_empty());
Expand Down Expand Up @@ -711,6 +715,9 @@ impl Diagnostic {
!(sp.is_empty() && suggestion.to_string().is_empty()),
"Span must not be empty and have no suggestion"
);
if !sp.can_be_used_for_suggestions() {
return self;
}
self.push_suggestion(CodeSuggestion {
substitutions: vec![Substitution {
parts: vec![SubstitutionPart { snippet: suggestion.to_string(), span: sp }],
Expand Down Expand Up @@ -774,6 +781,9 @@ impl Diagnostic {
!(sp.is_empty() && suggestions.iter().any(|suggestion| suggestion.is_empty())),
"Span must not be empty and have no suggestion"
);
if !sp.can_be_used_for_suggestions() {
return self;
}

let substitutions = suggestions
.into_iter()
Expand All @@ -799,12 +809,16 @@ impl Diagnostic {
) -> &mut Self {
let substitutions = suggestions
.into_iter()
.map(|sugg| {
.filter_map(|sugg| {
let mut parts = sugg
.into_iter()
.map(|(span, snippet)| SubstitutionPart { snippet, span })
.collect::<Vec<_>>();

if !parts.iter().all(|sub| sub.span.can_be_used_for_suggestions()) {
return None;
}

parts.sort_unstable_by_key(|part| part.span);

assert!(!parts.is_empty());
Expand All @@ -819,7 +833,7 @@ impl Diagnostic {
"suggestion must not have overlapping parts",
);

Substitution { parts }
Some(Substitution { parts })
})
.collect();

Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1355,7 +1355,7 @@ fn compare_number_of_method_arguments<'tcx>(
if pos == 0 {
arg.span
} else {
arg.span.with_lo(trait_m_sig.decl.inputs[0].span.lo())
tcx.adjust_span(arg.span).with_lo(trait_m_sig.decl.inputs[0].span.lo())
}
})
})
Expand All @@ -1371,7 +1371,7 @@ fn compare_number_of_method_arguments<'tcx>(
if pos == 0 {
arg.span
} else {
arg.span.with_lo(impl_m_sig.decl.inputs[0].span.lo())
tcx.adjust_span(arg.span).with_lo(impl_m_sig.decl.inputs[0].span.lo())
}
})
.unwrap_or_else(|| tcx.def_span(impl_m.def_id));
Expand Down Expand Up @@ -1468,7 +1468,8 @@ fn compare_synthetic_generics<'tcx>(

// in case there are no generics, take the spot between the function name
// and the opening paren of the argument list
let new_generics_span = tcx.def_ident_span(impl_def_id)?.shrink_to_hi();
let new_generics_span =
tcx.adjust_span(tcx.def_ident_span(impl_def_id)?).shrink_to_hi();
// in case there are generics, just replace them
let generics_span =
impl_m.generics.span.substitute_dummy(new_generics_span);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ fn missing_items_err(
let hi = full_impl_span.hi() - BytePos(1);
// Point at the place right before the closing brace of the relevant `impl` to suggest
// adding the associated item at the end of its body.
let sugg_sp = full_impl_span.with_lo(hi).with_hi(hi);
let sugg_sp = tcx.adjust_span(full_impl_span).with_lo(hi).with_hi(hi);
// Obtain the level of indentation ending in `sugg_sp`.
let padding =
tcx.sess.source_map().indentation_before(sugg_sp).unwrap_or_else(|| String::new());
Expand Down
19 changes: 8 additions & 11 deletions compiler/rustc_hir_analysis/src/structured_errors/wrong_number_of_generic_args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -610,10 +610,10 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {

AngleBrackets::Available => {
let (sugg_span, is_first) = if self.num_provided_lifetime_args() == 0 {
(self.gen_args.span().unwrap().shrink_to_lo(), true)
(self.tcx.adjust_span(self.gen_args.span().unwrap()).shrink_to_lo(), true)
} else {
let last_lt = &self.gen_args.args[self.num_provided_lifetime_args() - 1];
(last_lt.span().shrink_to_hi(), false)
(self.tcx.adjust_span(last_lt.span()).shrink_to_hi(), false)
};
let has_non_lt_args = self.num_provided_type_or_const_args() != 0;
let has_bindings = !self.gen_args.bindings.is_empty();
Expand Down Expand Up @@ -658,14 +658,14 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {
);
}
AngleBrackets::Available => {
let gen_args_span = self.gen_args.span().unwrap();
let gen_args_span = self.tcx.adjust_span(self.gen_args.span().unwrap());
let sugg_offset =
self.get_lifetime_args_offset() + self.num_provided_type_or_const_args();

let (sugg_span, is_first) = if sugg_offset == 0 {
(gen_args_span.shrink_to_lo(), true)
(self.tcx.adjust_span(gen_args_span).shrink_to_lo(), true)
} else {
let arg_span = self.gen_args.args[sugg_offset - 1].span();
let arg_span = self.tcx.adjust_span(self.gen_args.args[sugg_offset - 1].span());
// If we came here then inferred lifetime's spans can only point
// to either the opening bracket or to the space right after.
// Both of these spans have an `hi` lower than or equal to the span
Expand Down Expand Up @@ -770,7 +770,7 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {
&& let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
let sugg = vec![
(self.path_segment.ident.span, format!("{}::{}", snippet, self.path_segment.ident)),
(span.with_lo(self.path_segment.ident.span.hi()), "".to_owned())
(self.tcx.adjust_span(span).with_lo(self.path_segment.ident.span.hi()), "".to_owned())
];

err.multipart_suggestion(
Expand Down Expand Up @@ -953,11 +953,8 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {
}
} else if remove_entire_generics {
let span = self
.path_segment
.args
.unwrap()
.span_ext()
.unwrap()
.tcx
.adjust_span(self.path_segment.args.unwrap().span_ext().unwrap())
.with_lo(self.path_segment.ident.span.hi());

let msg = format!(
Expand Down
61 changes: 41 additions & 20 deletions compiler/rustc_hir_typeck/src/demand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
),
),
match &args[..] {
[] => (base.span.shrink_to_hi().with_hi(deref.span.hi()), ")".to_string()),
[] => (
self.tcx.adjust_span(base.span).shrink_to_hi().with_hi(deref.span.hi()),
")".to_string(),
),
[first, ..] => (base.span.between(first.span), ", ".to_string()),
},
]
Expand Down Expand Up @@ -771,8 +774,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"use `?` to coerce and return an appropriate `Err`, and wrap the resulting value \
in `Ok` so the expression remains of type `Result`",
vec![
(expr.span.shrink_to_lo(), "Ok(".to_string()),
(expr.span.shrink_to_hi(), "?)".to_string()),
(self.tcx.adjust_span(expr.span).shrink_to_lo(), "Ok(".to_string()),
(self.tcx.adjust_span(expr.span).shrink_to_hi(), "?)".to_string()),
],
Applicability::MaybeIncorrect,
);
Expand Down Expand Up @@ -843,8 +846,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} else {
return false;
};
if let Some(indent) =
self.tcx.sess.source_map().indentation_before(span.shrink_to_lo())
if let Some(indent) = self
.tcx
.sess
.source_map()
.indentation_before(self.tcx.adjust_span(span).shrink_to_lo())
{
// Add a semicolon, except after `}`.
let semicolon =
Expand All @@ -853,7 +859,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
_ => ";",
};
err.span_suggestions(
span.shrink_to_hi(),
self.tcx.adjust_span(span).shrink_to_hi(),
"try adding an expression at the end of the block",
return_suggestions
.into_iter()
Expand Down Expand Up @@ -931,8 +937,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

vec![
(expr.span.shrink_to_lo(), format!("{prefix}{variant}{open}")),
(expr.span.shrink_to_hi(), close.to_owned()),
(
self.tcx.adjust_span(expr.span).shrink_to_lo(),
format!("{prefix}{variant}{open}"),
),
(self.tcx.adjust_span(expr.span).shrink_to_hi(), close.to_owned()),
]
};

Expand Down Expand Up @@ -1016,8 +1025,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err.multipart_suggestion(
format!("consider calling `{s}::new`"),
vec![
(expr.span.shrink_to_lo(), format!("{path}::new(")),
(expr.span.shrink_to_hi(), format!("){unwrap}")),
(self.tcx.adjust_span(expr.span).shrink_to_lo(), format!("{path}::new(")),
(self.tcx.adjust_span(expr.span).shrink_to_hi(), format!("){unwrap}")),
],
Applicability::MaybeIncorrect,
);
Expand Down Expand Up @@ -1271,7 +1280,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&& replace_prefix(&src, "\"", "b\"").is_some()
{
return Some((
sp.shrink_to_lo(),
self.tcx.adjust_span(sp).shrink_to_lo(),
"consider adding a leading `b`".to_string(),
"b".to_string(),
Applicability::MachineApplicable,
Expand Down Expand Up @@ -1468,7 +1477,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
_ if sp == expr.span => {
if let Some(mut steps) = self.deref_steps(checked_ty, expected) {
let mut expr = expr.peel_blocks();
let mut prefix_span = expr.span.shrink_to_lo();
let mut prefix_span = self.tcx.adjust_span(expr.span).shrink_to_lo();
let mut remove = String::new();

// Try peeling off any existing `&` and `&mut` to reach our target type
Expand Down Expand Up @@ -1539,7 +1548,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return None;
} else if let Some(expr) = self.maybe_get_block_expr(expr) {
// prefix should be empty here..
(expr.span.shrink_to_lo(), "*".to_string())
(self.tcx.adjust_span(expr.span).shrink_to_lo(), "*".to_string())
} else {
(prefix_span, format!("{}{}", prefix, "*".repeat(steps)))
};
Expand Down Expand Up @@ -1596,7 +1605,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// `expr` is a literal field for a struct, only suggest if appropriate
if field.is_shorthand {
// This is a field literal
sugg.push((field.ident.span.shrink_to_lo(), format!("{}: ", field.ident)));
sugg.push((
self.tcx.adjust_span(field.ident.span).shrink_to_lo(),
format!("{}: ", field.ident),
));
} else {
// Likely a field was meant, but this field wasn't found. Do not suggest anything.
return false;
Expand Down Expand Up @@ -1652,16 +1664,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);

let close_paren = if expr.precedence().order() < PREC_POSTFIX {
sugg.push((expr.span.shrink_to_lo(), "(".to_string()));
sugg.push((self.tcx.adjust_span(expr.span).shrink_to_lo(), "(".to_string()));
")"
} else {
""
};

let mut cast_suggestion = sugg.clone();
cast_suggestion.push((expr.span.shrink_to_hi(), format!("{close_paren} as {expected_ty}")));
cast_suggestion.push((
self.tcx.adjust_span(expr.span).shrink_to_hi(),
format!("{close_paren} as {expected_ty}"),
));
let mut into_suggestion = sugg.clone();
into_suggestion.push((expr.span.shrink_to_hi(), format!("{close_paren}.into()")));
into_suggestion.push((
self.tcx.adjust_span(expr.span).shrink_to_hi(),
format!("{close_paren}.into()"),
));
let mut suffix_suggestion = sugg.clone();
suffix_suggestion.push((
if matches!(
Expand Down Expand Up @@ -1715,15 +1733,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"you can convert `{lhs_src}` from `{expected_ty}` to `{checked_ty}`, matching the type of `{src}`",
);
let suggestion = vec![
(lhs_expr.span.shrink_to_lo(), format!("{checked_ty}::from(")),
(lhs_expr.span.shrink_to_hi(), ")".to_string()),
(
self.tcx.adjust_span(lhs_expr.span).shrink_to_lo(),
format!("{checked_ty}::from("),
),
(self.tcx.adjust_span(lhs_expr.span).shrink_to_hi(), ")".to_string()),
];
(msg, suggestion)
} else {
let msg = format!("{msg} and panic if the converted value doesn't fit");
let mut suggestion = sugg.clone();
suggestion.push((
expr.span.shrink_to_hi(),
self.tcx.adjust_span(expr.span).shrink_to_hi(),
format!("{close_paren}.try_into().unwrap()"),
));
(msg, suggestion)
Expand Down
Loading

0 comments on commit ba7d52a

Please sign in to comment.