Skip to content

Commit

Permalink
Rollup merge of #127301 - estebank:fix-suggestions, r=Urgau
Browse files Browse the repository at this point in the history 
Tweak some structured suggestions to be more verbose and accurate

Addressing some issues I found while working on #127282.
```
error: this URL is not a hyperlink
  --> $DIR/auxiliary/include-str-bare-urls.md:1:11
   |
LL | HEADS UP! https://example.com MUST SHOW UP IN THE STDERR FILE!
   |           ^^^^^^^^^^^^^^^^^^^
   |
   = note: bare URLs are not automatically turned into clickable links
note: the lint level is defined here
  --> $DIR/include-str-bare-urls.rs:14:9
   |
LL | #![deny(rustdoc::bare_urls)]
   |         ^^^^^^^^^^^^^^^^^^
help: use an automatic link instead
   |
LL | HEADS UP! <https://example.com> MUST SHOW UP IN THE STDERR FILE!
   |           +                   +
```
```
error[E0384]: cannot assign twice to immutable variable `v`
  --> $DIR/assign-imm-local-twice.rs:7:5
   |
LL |     v = 1;
   |     ----- first assignment to `v`
LL |     println!("v={}", v);
LL |     v = 2;
   |     ^^^^^ cannot assign twice to immutable variable
   |
help: consider making this binding mutable
   |
LL |     let mut v: isize;
   |         +++
```
```
error[E0393]: the type parameter `Rhs` must be explicitly specified
  --> $DIR/issue-22560.rs:9:23
   |
LL | trait Sub<Rhs=Self> {
   | ------------------- type parameter `Rhs` must be specified for this
...
LL | type Test = dyn Add + Sub;
   |                       ^^^
   |
   = note: because of the default `Self` reference, type parameters must be specified on object types
help: set the type parameter to the desired type
   |
LL | type Test = dyn Add + Sub<Rhs>;
   |                          +++++
```
```
error[E0596]: cannot borrow `v` as mutable, as it is not declared as mutable
  --> $DIR/issue-33819.rs:4:34
   |
LL |         Some(ref v) => { let a = &mut v; },
   |                                  ^^^^^^ cannot borrow as mutable
   |
help: try removing `&mut` here
   |
LL -         Some(ref v) => { let a = &mut v; },
LL +         Some(ref v) => { let a = v; },
   |
```
```
help: remove the invocation before committing it to a version control system
   |
LL -     dbg!();
   |
```
```
error[E0308]: mismatched types
  --> $DIR/issue-39974.rs:1:21
   |
LL | const LENGTH: f64 = 2;
   |                     ^ expected `f64`, found integer
   |
help: use a float literal
   |
LL | const LENGTH: f64 = 2.0;
   |                      ++
```
```
error[E0529]: expected an array or slice, found `Vec<i32>`
  --> $DIR/match-ergonomics.rs:8:9
   |
LL |         [&v] => {},
   |         ^^^^ pattern cannot match with input type `Vec<i32>`
   |
help: consider slicing here
   |
LL |     match x[..] {
   |            ++++
```
```
error[E0609]: no field `0` on type `[u32; 1]`
  --> $DIR/parenthesized-deref-suggestion.rs:10:21
   |
LL |     (x as [u32; 1]).0;
   |                     ^ unknown field
   |
help: instead of using tuple indexing, use array indexing
   |
LL |     (x as [u32; 1])[0];
   |                    ~ +
```
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Jul 4, 2024
2 parents d27136f + a5e7da0 commit 54bd3a7
Show file tree
Hide file tree
Showing 82 changed files with 808 additions and 468 deletions.
25 changes: 11 additions & 14 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3757,13 +3757,11 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
assigned_span: Span,
err_place: Place<'tcx>,
) {
let (from_arg, local_decl, local_name) = match err_place.as_local() {
Some(local) => (
self.body.local_kind(local) == LocalKind::Arg,
Some(&self.body.local_decls[local]),
self.local_names[local],
),
None => (false, None, None),
let (from_arg, local_decl) = match err_place.as_local() {
Some(local) => {
(self.body.local_kind(local) == LocalKind::Arg, Some(&self.body.local_decls[local]))
}
None => (false, None),
};

// If root local is initialized immediately (everything apart from let
Expand Down Expand Up @@ -3795,13 +3793,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
err.span_label(assigned_span, format!("first assignment to {place_description}"));
}
if let Some(decl) = local_decl
&& let Some(name) = local_name
&& decl.can_be_made_mutable()
{
err.span_suggestion(
decl.source_info.span,
err.span_suggestion_verbose(
decl.source_info.span.shrink_to_lo(),
"consider making this binding mutable",
format!("mut {name}"),
"mut ".to_string(),
Applicability::MachineApplicable,
);
if !from_arg
Expand All @@ -3813,10 +3810,10 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
}))
)
{
err.span_suggestion(
decl.source_info.span,
err.span_suggestion_verbose(
decl.source_info.span.shrink_to_lo(),
"to modify the original value, take a borrow instead",
format!("ref mut {name}"),
"ref mut ".to_string(),
Applicability::MaybeIncorrect,
);
}
Expand Down
29 changes: 16 additions & 13 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,21 +408,21 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
fn_decl.implicit_self,
hir::ImplicitSelfKind::RefImm | hir::ImplicitSelfKind::RefMut
) {
err.span_suggestion(
upvar_ident.span,
err.span_suggestion_verbose(
upvar_ident.span.shrink_to_lo(),
"consider changing this to be mutable",
format!("mut {}", upvar_ident.name),
"mut ",
Applicability::MachineApplicable,
);
break;
}
}
}
} else {
err.span_suggestion(
upvar_ident.span,
err.span_suggestion_verbose(
upvar_ident.span.shrink_to_lo(),
"consider changing this to be mutable",
format!("mut {}", upvar_ident.name),
"mut ",
Applicability::MachineApplicable,
);
}
Expand All @@ -449,8 +449,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
.is_ok_and(|snippet| snippet.starts_with("&mut ")) =>
{
err.span_label(span, format!("cannot {act}"));
err.span_suggestion(
span,
err.span_suggestion_verbose(
span.with_hi(span.lo() + BytePos(5)),
"try removing `&mut` here",
"",
Applicability::MaybeIncorrect,
Expand Down Expand Up @@ -755,13 +755,16 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
pat: hir::Pat { kind: hir::PatKind::Ref(_, _), .. },
..
}) = node
&& let Ok(name) =
self.infcx.tcx.sess.source_map().span_to_snippet(local_decl.source_info.span)
{
err.span_suggestion(
pat_span,
err.multipart_suggestion(
"consider changing this to be mutable",
format!("&(mut {name})"),
vec![
(pat_span.until(local_decl.source_info.span), "&(mut ".to_string()),
(
local_decl.source_info.span.shrink_to_hi().with_hi(pat_span.hi()),
")".to_string(),
),
],
Applicability::MachineApplicable,
);
return;
Expand Down
23 changes: 20 additions & 3 deletions compiler/rustc_errors/src/emitter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2273,9 +2273,26 @@ impl HumanEmitter {
&normalize_whitespace(last_line),
Style::NoStyle,
);
buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber);
buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
buffer.append(*row_num, &normalize_whitespace(line_to_add), Style::NoStyle);
if !line_to_add.trim().is_empty() {
// Check if after the removal, the line is left with only whitespace. If so, we
// will not show an "addition" line, as removing the whole line is what the user
// would really want.
// For example, for the following:
// |
// 2 - .await
// 2 + (note the left over whitepsace)
// |
// We really want
// |
// 2 - .await
// |
// *row_num -= 1;
buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber);
buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
buffer.append(*row_num, &normalize_whitespace(line_to_add), Style::NoStyle);
} else {
*row_num -= 1;
}
} else {
*row_num -= 2;
}
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_hir_analysis/src/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -453,12 +453,11 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for MissingTypeParams {
} else {
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
// least we can clue them to the correct syntax `Iterator<Type>`.
err.span_suggestion(
self.span,
err.span_suggestion_verbose(
self.span.shrink_to_hi(),
fluent::hir_analysis_suggestion,
format!(
"{}<{}>",
snippet,
"<{}>",
self.missing_type_params
.iter()
.map(|n| n.to_string())
Expand Down
44 changes: 26 additions & 18 deletions compiler/rustc_hir_typeck/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2551,10 +2551,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

match *base_ty.peel_refs().kind() {
ty::Array(_, len) => {
self.maybe_suggest_array_indexing(&mut err, expr, base, ident, len);
self.maybe_suggest_array_indexing(&mut err, base, ident, len);
}
ty::RawPtr(..) => {
self.suggest_first_deref_field(&mut err, expr, base, ident);
self.suggest_first_deref_field(&mut err, base, ident);
}
ty::Param(param_ty) => {
err.span_label(ident.span, "unknown field");
Expand Down Expand Up @@ -2721,40 +2721,48 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn maybe_suggest_array_indexing(
&self,
err: &mut Diag<'_>,
expr: &hir::Expr<'_>,
base: &hir::Expr<'_>,
field: Ident,
len: ty::Const<'tcx>,
) {
err.span_label(field.span, "unknown field");
if let (Some(len), Ok(user_index)) =
(len.try_eval_target_usize(self.tcx, self.param_env), field.as_str().parse::<u64>())
&& let Ok(base) = self.tcx.sess.source_map().span_to_snippet(base.span)
{
let help = "instead of using tuple indexing, use array indexing";
let suggestion = format!("{base}[{field}]");
let applicability = if len < user_index {
Applicability::MachineApplicable
} else {
Applicability::MaybeIncorrect
};
err.span_suggestion(expr.span, help, suggestion, applicability);
err.multipart_suggestion(
help,
vec![
(base.span.between(field.span), "[".to_string()),
(field.span.shrink_to_hi(), "]".to_string()),
],
applicability,
);
}
}

fn suggest_first_deref_field(
&self,
err: &mut Diag<'_>,
expr: &hir::Expr<'_>,
base: &hir::Expr<'_>,
field: Ident,
) {
fn suggest_first_deref_field(&self, err: &mut Diag<'_>, base: &hir::Expr<'_>, field: Ident) {
err.span_label(field.span, "unknown field");
if let Ok(base) = self.tcx.sess.source_map().span_to_snippet(base.span) {
let msg = format!("`{base}` is a raw pointer; try dereferencing it");
let suggestion = format!("(*{base}).{field}");
err.span_suggestion(expr.span, msg, suggestion, Applicability::MaybeIncorrect);
}
let val = if let Ok(base) = self.tcx.sess.source_map().span_to_snippet(base.span)
&& base.len() < 20
{
format!("`{base}`")
} else {
"the value".to_string()
};
err.multipart_suggestion(
format!("{val} is a raw pointer; try dereferencing it"),
vec![
(base.span.shrink_to_lo(), "(*".to_string()),
(base.span.shrink_to_hi(), ")".to_string()),
],
Applicability::MaybeIncorrect,
);
}

fn no_such_field_err(&self, field: Ident, expr_t: Ty<'tcx>, id: HirId) -> Diag<'_> {
Expand Down
13 changes: 6 additions & 7 deletions compiler/rustc_hir_typeck/src/pat.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2499,7 +2499,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.any(|(ty, _)| matches!(ty.kind(), ty::Slice(..) | ty::Array(..)))
&& let Some(span) = ti.span
&& let Some(_) = ti.origin_expr
&& let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span)
{
let resolved_ty = self.resolve_vars_if_possible(ti.expected);
let (is_slice_or_array_or_vector, resolved_ty) =
Expand All @@ -2510,10 +2509,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|| self.tcx.is_diagnostic_item(sym::Result, adt_def.did()) =>
{
// Slicing won't work here, but `.as_deref()` might (issue #91328).
err.span_suggestion(
span,
err.span_suggestion_verbose(
span.shrink_to_hi(),
"consider using `as_deref` here",
format!("{snippet}.as_deref()"),
".as_deref()",
Applicability::MaybeIncorrect,
);
}
Expand All @@ -2522,10 +2521,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let is_top_level = current_depth <= 1;
if is_slice_or_array_or_vector && is_top_level {
err.span_suggestion(
span,
err.span_suggestion_verbose(
span.shrink_to_hi(),
"consider slicing here",
format!("{snippet}[..]"),
"[..]",
Applicability::MachineApplicable,
);
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
) = tcx.sess.source_map().span_to_snippet(sp) =>
{
if snippet.chars().all(|c| c.is_digit(10) || c == '-' || c == '_') {
diag.span_suggestion(
sp,
diag.span_suggestion_verbose(
sp.shrink_to_hi(),
"use a float literal",
format!("{snippet}.0"),
".0",
MachineApplicable,
);
}
Expand Down
36 changes: 17 additions & 19 deletions src/librustdoc/passes/lint/bare_urls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,22 @@ pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item) {
};
let dox = item.doc_value();
if !dox.is_empty() {
let report_diag =
|cx: &DocContext<'_>, msg: &'static str, url: &str, range: Range<usize>| {
let sp =
source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs.doc_strings)
.unwrap_or_else(|| item.attr_span(cx.tcx));
cx.tcx.node_span_lint(crate::lint::BARE_URLS, hir_id, sp, |lint| {
lint.primary_message(msg)
.note("bare URLs are not automatically turned into clickable links")
.span_suggestion(
sp,
"use an automatic link instead",
format!("<{url}>"),
Applicability::MachineApplicable,
);
});
};
let report_diag = |cx: &DocContext<'_>, msg: &'static str, range: Range<usize>| {
let sp = source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs.doc_strings)
.unwrap_or_else(|| item.attr_span(cx.tcx));
cx.tcx.node_span_lint(crate::lint::BARE_URLS, hir_id, sp, |lint| {
lint.primary_message(msg)
.note("bare URLs are not automatically turned into clickable links")
.multipart_suggestion(
"use an automatic link instead",
vec![
(sp.shrink_to_lo(), "<".to_string()),
(sp.shrink_to_hi(), ">".to_string()),
],
Applicability::MachineApplicable,
);
});
};

let mut p = Parser::new_ext(&dox, main_body_opts()).into_offset_iter();

Expand Down Expand Up @@ -74,17 +74,15 @@ fn find_raw_urls(
cx: &DocContext<'_>,
text: &str,
range: Range<usize>,
f: &impl Fn(&DocContext<'_>, &'static str, &str, Range<usize>),
f: &impl Fn(&DocContext<'_>, &'static str, Range<usize>),
) {
trace!("looking for raw urls in {text}");
// For now, we only check "full" URLs (meaning, starting with "http://" or "https://").
for match_ in URL_REGEX.find_iter(text) {
let url = match_.as_str();
let url_range = match_.range();
f(
cx,
"this URL is not a hyperlink",
url,
Range { start: range.start + url_range.start, end: range.start + url_range.end },
);
}
Expand Down
2 changes: 0 additions & 2 deletions src/tools/clippy/tests/ui/dbg_macro/dbg_macro.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ LL | dbg!();
help: remove the invocation before committing it to a version control system
|
LL - dbg!();
LL +
|

error: the `dbg!` macro is intended as a debugging tool
Expand Down Expand Up @@ -146,7 +145,6 @@ LL | expand_to_dbg!();
help: remove the invocation before committing it to a version control system
|
LL - dbg!();
LL +
|

error: the `dbg!` macro is intended as a debugging tool
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/tests/ui/dbg_macro/dbg_macro_unfixable.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ LL | dbg!();
help: remove the invocation before committing it to a version control system
|
LL - dbg!();
LL +
|

error: the `dbg!` macro is intended as a debugging tool
Expand Down
Loading

0 comments on commit 54bd3a7

Please sign in to comment.