Skip to content

Commit

Permalink
Auto merge of #101986 - WaffleLapkin:move_lint_note_to_the_bottom, r=…
Browse files Browse the repository at this point in the history
…estebank

Move lint level source explanation to the bottom

So, uhhhhh

r? `@estebank`

## User-facing change

"note: `#[warn(...)]` on by default" and such are moved to the bottom of the diagnostic:
```diff
-   = note: `#[warn(unsupported_calling_conventions)]` on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #87678 <rust-lang/rust#87678>
+   = note: `#[warn(unsupported_calling_conventions)]` on by default
```

Why warning is enabled is the least important thing, so it shouldn't be the first note the user reads, IMO.

## Developer-facing change

`struct_span_lint` and similar methods have a different signature.

Before: `..., impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>)`
After: `..., impl Into<DiagnosticMessage>, impl for<'a, 'b> FnOnce(&'b mut DiagnosticBuilder<'a, ()>) -> &'b mut DiagnosticBuilder<'a, ()>`

The reason for this is that `struct_span_lint` needs to edit the diagnostic _after_ `decorate` closure is called. This also makes lint code a little bit nicer in my opinion.

Another option is to use `impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>) -> DiagnosticBuilder<'a, ()>` altough I don't _really_ see reasons to do `let lint = lint.build(message)` everywhere.

## Subtle problem

By moving the message outside of the closure (that may not be called if the lint is disabled) `format!(...)` is executed earlier, possibly formatting `Ty` which may call a query that trims paths that crashes the compiler if there were no warnings...

I don't think it's that big of a deal, considering that we move from `format!(...)` to `fluent` (which is lazy by-default) anyway, however this required adding a workaround which is unfortunate.

## P.S.

I'm sorry, I do not how to make this PR smaller/easier to review. Changes to the lint API affect SO MUCH 😢
  • Loading branch information
bors committed Oct 1, 2022
2 parents 8b59fe4 + 8dfbad9 commit 9e8f53d
Show file tree
Hide file tree
Showing 207 changed files with 260 additions and 267 deletions.
19 changes: 9 additions & 10 deletions clippy_lints/src/module_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,13 @@ impl EarlyLintPass for ModStyle {
cx.struct_span_lint(
SELF_NAMED_MODULE_FILES,
Span::new(file.start_pos, file.start_pos, SyntaxContext::root(), None),
|build| {
let mut lint =
build.build(&format!("`mod.rs` files are required, found `{}`", path.display()));
lint.help(&format!("move `{}` to `{}`", path.display(), correct.display(),));
lint.emit();
format!("`mod.rs` files are required, found `{}`", path.display()),
|lint| {
lint.help(format!(
"move `{}` to `{}`",
path.display(),
correct.display(),
))
},
);
}
Expand Down Expand Up @@ -156,11 +158,8 @@ fn check_self_named_mod_exists(cx: &EarlyContext<'_>, path: &Path, file: &Source
cx.struct_span_lint(
MOD_MODULE_FILES,
Span::new(file.start_pos, file.start_pos, SyntaxContext::root(), None),
|build| {
let mut lint = build.build(&format!("`mod.rs` files are not allowed, found `{}`", path.display()));
lint.help(&format!("move `{}` to `{}`", path.display(), mod_file.display(),));
lint.emit();
},
format!("`mod.rs` files are not allowed, found `{}`", path.display()),
|lint| lint.help(format!("move `{}` to `{}`", path.display(), mod_file.display())),
);
}
}
46 changes: 20 additions & 26 deletions clippy_utils/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,9 @@ fn docs_link(diag: &mut Diagnostic, lint: &'static Lint) {
/// | ^^^^^^^^^^^^^^^^^^^^^^^
/// ```
pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: impl Into<MultiSpan>, msg: &str) {
cx.struct_span_lint(lint, sp, |diag| {
let mut diag = diag.build(msg);
docs_link(&mut diag, lint);
diag.emit();
cx.struct_span_lint(lint, sp, msg, |diag| {
docs_link(diag, lint);
diag
});
}

Expand Down Expand Up @@ -82,15 +81,14 @@ pub fn span_lint_and_help<'a, T: LintContext>(
help_span: Option<Span>,
help: &str,
) {
cx.struct_span_lint(lint, span, |diag| {
let mut diag = diag.build(msg);
cx.struct_span_lint(lint, span, msg, |diag| {
if let Some(help_span) = help_span {
diag.span_help(help_span, help);
} else {
diag.help(help);
}
docs_link(&mut diag, lint);
diag.emit();
docs_link(diag, lint);
diag
});
}

Expand Down Expand Up @@ -125,15 +123,14 @@ pub fn span_lint_and_note<'a, T: LintContext>(
note_span: Option<Span>,
note: &str,
) {
cx.struct_span_lint(lint, span, |diag| {
let mut diag = diag.build(msg);
cx.struct_span_lint(lint, span, msg, |diag| {
if let Some(note_span) = note_span {
diag.span_note(note_span, note);
} else {
diag.note(note);
}
docs_link(&mut diag, lint);
diag.emit();
docs_link(diag, lint);
diag
});
}

Expand All @@ -147,19 +144,17 @@ where
S: Into<MultiSpan>,
F: FnOnce(&mut Diagnostic),
{
cx.struct_span_lint(lint, sp, |diag| {
let mut diag = diag.build(msg);
f(&mut diag);
docs_link(&mut diag, lint);
diag.emit();
cx.struct_span_lint(lint, sp, msg, |diag| {
f(diag);
docs_link(diag, lint);
diag
});
}

pub fn span_lint_hir(cx: &LateContext<'_>, lint: &'static Lint, hir_id: HirId, sp: Span, msg: &str) {
cx.tcx.struct_span_lint_hir(lint, hir_id, sp, |diag| {
let mut diag = diag.build(msg);
docs_link(&mut diag, lint);
diag.emit();
cx.tcx.struct_span_lint_hir(lint, hir_id, sp, msg, |diag| {
docs_link(diag, lint);
diag
});
}

Expand All @@ -171,11 +166,10 @@ pub fn span_lint_hir_and_then(
msg: &str,
f: impl FnOnce(&mut Diagnostic),
) {
cx.tcx.struct_span_lint_hir(lint, hir_id, sp, |diag| {
let mut diag = diag.build(msg);
f(&mut diag);
docs_link(&mut diag, lint);
diag.emit();
cx.tcx.struct_span_lint_hir(lint, hir_id, sp, msg, |diag| {
f(diag);
docs_link(diag, lint);
diag
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: `std::string::String` may not be held across an `await` point per `clippy
LL | let _x = String::from("hello");
| ^^
|
= note: `-D clippy::await-holding-invalid-type` implied by `-D warnings`
= note: strings are bad
= note: `-D clippy::await-holding-invalid-type` implied by `-D warnings`

error: `std::net::Ipv4Addr` may not be held across an `await` point per `clippy.toml`
--> $DIR/await_holding_invalid_type.rs:10:9
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ error: the function has a cognitive complexity of (3/2)
LL | fn cognitive_complexity() {
| ^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::cognitive-complexity` implied by `-D warnings`
= help: you could split it up into multiple smaller functions
= note: `-D clippy::cognitive-complexity` implied by `-D warnings`

error: aborting due to previous error; 2 warnings emitted

2 changes: 1 addition & 1 deletion tests/ui-toml/expect_used/expect_used.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: used `expect()` on `an Option` value
LL | let _ = opt.expect("");
| ^^^^^^^^^^^^^^
|
= note: `-D clippy::expect-used` implied by `-D warnings`
= help: if this value is `None`, it will panic
= note: `-D clippy::expect-used` implied by `-D warnings`

error: used `expect()` on `a Result` value
--> $DIR/expect_used.rs:11:13
Expand Down
2 changes: 1 addition & 1 deletion tests/ui-toml/fn_params_excessive_bools/test.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: more than 1 bools in function parameters
LL | fn g(_: bool, _: bool) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::fn-params-excessive-bools` implied by `-D warnings`
= help: consider refactoring bools into two-variant enums
= note: `-D clippy::fn-params-excessive-bools` implied by `-D warnings`

error: aborting due to previous error

2 changes: 1 addition & 1 deletion tests/ui-toml/large_include_file/large_include_file.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: attempted to include a large file
LL | const TOO_BIG_INCLUDE_BYTES: &[u8; 654] = include_bytes!("too_big.txt");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::large-include-file` implied by `-D warnings`
= note: the configuration allows a maximum size of 600 bytes
= note: `-D clippy::large-include-file` implied by `-D warnings`
= note: this error originates in the macro `include_bytes` (in Nightly builds, run with -Z macro-backtrace for more info)

error: attempted to include a large file
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ error: use of irregular braces for `vec!` macro
LL | let _ = vec! {1, 2, 3};
| ^^^^^^^^^^^^^^
|
= note: `-D clippy::nonstandard-macro-braces` implied by `-D warnings`
help: consider writing `vec![1, 2, 3]`
--> $DIR/conf_nonstandard_macro_braces.rs:43:13
|
LL | let _ = vec! {1, 2, 3};
| ^^^^^^^^^^^^^^
= note: `-D clippy::nonstandard-macro-braces` implied by `-D warnings`

error: use of irregular braces for `format!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:44:13
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ error: some fields in `NoGeneric` are not safe to be sent to another thread
LL | unsafe impl Send for NoGeneric {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::non-send-fields-in-send-ty` implied by `-D warnings`
note: it is not safe to send field `rc_is_not_send` to another thread
--> $DIR/test.rs:8:5
|
LL | rc_is_not_send: Rc<String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: use a thread-safe type that implements `Send`
= note: `-D clippy::non-send-fields-in-send-ty` implied by `-D warnings`

error: some fields in `MultiField<T>` are not safe to be sent to another thread
--> $DIR/test.rs:19:1
Expand Down
2 changes: 1 addition & 1 deletion tests/ui-toml/struct_excessive_bools/test.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ LL | | a: bool,
LL | | }
| |_^
|
= note: `-D clippy::struct-excessive-bools` implied by `-D warnings`
= help: consider using a state machine or refactoring bools into two-variant enums
= note: `-D clippy::struct-excessive-bools` implied by `-D warnings`

error: aborting due to previous error

2 changes: 1 addition & 1 deletion tests/ui-toml/unwrap_used/unwrap_used.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ error: used `unwrap()` on `an Option` value
LL | let _ = boxed_slice.get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::unwrap-used` implied by `-D warnings`
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message
= note: `-D clippy::unwrap-used` implied by `-D warnings`

error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/unwrap_used.rs:36:17
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/absurd-extreme-comparisons.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: this comparison involving the minimum or maximum element for this type co
LL | u <= 0;
| ^^^^^^
|
= note: `-D clippy::absurd-extreme-comparisons` implied by `-D warnings`
= help: because `0` is the minimum value for this type, the case where the two sides are not equal never occurs, consider using `u == 0` instead
= note: `-D clippy::absurd-extreme-comparisons` implied by `-D warnings`

error: this comparison involving the minimum or maximum element for this type contains a case that is always true or always false
--> $DIR/absurd-extreme-comparisons.rs:15:5
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/allow_attributes_without_reason.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ error: `allow` attribute without specifying a reason
LL | #[allow(dead_code)]
| ^^^^^^^^^^^^^^^^^^^
|
= help: try adding a reason at the end with `, reason = ".."`
note: the lint level is defined here
--> $DIR/allow_attributes_without_reason.rs:2:9
|
LL | #![deny(clippy::allow_attributes_without_reason)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: try adding a reason at the end with `, reason = ".."`

error: `allow` attribute without specifying a reason
--> $DIR/allow_attributes_without_reason.rs:6:1
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/approx_const.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: approximate value of `f{32, 64}::consts::E` found
LL | let my_e = 2.7182;
| ^^^^^^
|
= note: `-D clippy::approx-constant` implied by `-D warnings`
= help: consider using the constant directly
= note: `-D clippy::approx-constant` implied by `-D warnings`

error: approximate value of `f{32, 64}::consts::E` found
--> $DIR/approx_const.rs:5:20
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/as_conversions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: using a potentially dangerous silent `as` conversion
LL | let i = 0u32 as u64;
| ^^^^^^^^^^^
|
= note: `-D clippy::as-conversions` implied by `-D warnings`
= help: consider using a safe wrapper for this conversion
= note: `-D clippy::as-conversions` implied by `-D warnings`

error: using a potentially dangerous silent `as` conversion
--> $DIR/as_conversions.rs:17:13
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/asm_syntax.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: Intel x86 assembly syntax used
LL | asm!("");
| ^^^^^^^^
|
= note: `-D clippy::inline-asm-x86-intel-syntax` implied by `-D warnings`
= help: use AT&T x86 assembly syntax
= note: `-D clippy::inline-asm-x86-intel-syntax` implied by `-D warnings`

error: Intel x86 assembly syntax used
--> $DIR/asm_syntax.rs:9:9
Expand All @@ -29,8 +29,8 @@ error: AT&T x86 assembly syntax used
LL | asm!("", options(att_syntax));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::inline-asm-x86-att-syntax` implied by `-D warnings`
= help: use Intel x86 assembly syntax
= note: `-D clippy::inline-asm-x86-att-syntax` implied by `-D warnings`

error: AT&T x86 assembly syntax used
--> $DIR/asm_syntax.rs:24:9
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/assertions_on_constants.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: `assert!(true)` will be optimized out by the compiler
LL | assert!(true);
| ^^^^^^^^^^^^^
|
= note: `-D clippy::assertions-on-constants` implied by `-D warnings`
= help: remove it
= note: `-D clippy::assertions-on-constants` implied by `-D warnings`

error: `assert!(false)` should probably be replaced
--> $DIR/assertions_on_constants.rs:11:5
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/await_holding_lock.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ error: this `MutexGuard` is held across an `await` point
LL | let guard = x.lock().unwrap();
| ^^^^^
|
= note: `-D clippy::await-holding-lock` implied by `-D warnings`
= help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await
note: these are all the `await` points this lock is held through
--> $DIR/await_holding_lock.rs:9:9
Expand All @@ -13,6 +12,7 @@ LL | / let guard = x.lock().unwrap();
LL | | baz().await
LL | | }
| |_____^
= note: `-D clippy::await-holding-lock` implied by `-D warnings`

error: this `MutexGuard` is held across an `await` point
--> $DIR/await_holding_lock.rs:24:13
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/await_holding_refcell_ref.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ error: this `RefCell` reference is held across an `await` point
LL | let b = x.borrow();
| ^
|
= note: `-D clippy::await-holding-refcell-ref` implied by `-D warnings`
= help: ensure the reference is dropped before calling `await`
note: these are all the `await` points this reference is held through
--> $DIR/await_holding_refcell_ref.rs:6:5
Expand All @@ -13,6 +12,7 @@ LL | / let b = x.borrow();
LL | | baz().await
LL | | }
| |_^
= note: `-D clippy::await-holding-refcell-ref` implied by `-D warnings`

error: this `RefCell` reference is held across an `await` point
--> $DIR/await_holding_refcell_ref.rs:11:9
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/blanket_clippy_restriction_lints.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: restriction lints are not meant to be all enabled
LL | #![warn(clippy::restriction)]
| ^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::blanket-clippy-restriction-lints` implied by `-D warnings`
= help: try enabling only the lints you really need
= note: `-D clippy::blanket-clippy-restriction-lints` implied by `-D warnings`

error: restriction lints are not meant to be all enabled
--> $DIR/blanket_clippy_restriction_lints.rs:5:9
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/bool_to_int_with_if.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ LL | | 0
LL | | };
| |_____^ help: replace with from: `i32::from(a)`
|
= note: `-D clippy::bool-to-int-with-if` implied by `-D warnings`
= note: `a as i32` or `a.into()` can also be valid options
= note: `-D clippy::bool-to-int-with-if` implied by `-D warnings`

error: boolean to int conversion using if
--> $DIR/bool_to_int_with_if.rs:20:5
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/borrow_interior_mutable_const/enums.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: a `const` item with interior mutability should not be borrowed
LL | let _ = &UNFROZEN_VARIANT; //~ ERROR interior mutability
| ^^^^^^^^^^^^^^^^
|
= note: `-D clippy::borrow-interior-mutable-const` implied by `-D warnings`
= help: assign this const to a local or static variable, and use the variable here
= note: `-D clippy::borrow-interior-mutable-const` implied by `-D warnings`

error: a `const` item with interior mutability should not be borrowed
--> $DIR/enums.rs:37:18
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/borrow_interior_mutable_const/others.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: a `const` item with interior mutability should not be borrowed
LL | ATOMIC.store(1, Ordering::SeqCst); //~ ERROR interior mutability
| ^^^^^^
|
= note: `-D clippy::borrow-interior-mutable-const` implied by `-D warnings`
= help: assign this const to a local or static variable, and use the variable here
= note: `-D clippy::borrow-interior-mutable-const` implied by `-D warnings`

error: a `const` item with interior mutability should not be borrowed
--> $DIR/others.rs:55:16
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/borrow_interior_mutable_const/traits.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: a `const` item with interior mutability should not be borrowed
LL | let _ = &Self::ATOMIC; //~ ERROR interior mutable
| ^^^^^^^^^^^^
|
= note: `-D clippy::borrow-interior-mutable-const` implied by `-D warnings`
= help: assign this const to a local or static variable, and use the variable here
= note: `-D clippy::borrow-interior-mutable-const` implied by `-D warnings`

error: a `const` item with interior mutability should not be borrowed
--> $DIR/traits.rs:26:18
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/box_collection.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error: you seem to be trying to use `Box<Vec<..>>`. Consider using just `Vec<..>
LL | fn test1(foo: Box<Vec<bool>>) {}
| ^^^^^^^^^^^^^^
|
= note: `-D clippy::box-collection` implied by `-D warnings`
= help: `Vec<..>` is already on the heap, `Box<Vec<..>>` makes an extra allocation
= note: `-D clippy::box-collection` implied by `-D warnings`

error: you seem to be trying to use `Box<String>`. Consider using just `String`
--> $DIR/box_collection.rs:28:15
Expand Down
Loading

0 comments on commit 9e8f53d

Please sign in to comment.