about summary refs log tree commit diff
path: root/compiler/rustc_lint/src/non_ascii_idents.rs
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-10-01 10:44:25 +0000
committerbors <bors@rust-lang.org>2022-10-01 10:44:25 +0000
commit744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1 (patch)
tree1721987352b5f0a8548fc46984821d974b661934 /compiler/rustc_lint/src/non_ascii_idents.rs
parent277bb6653b55475b5fbce6309e9852fa2100dabe (diff)
parentb5b3ffe3fc9cfb524a6432ec60a0fc95c514d2e1 (diff)
downloadrust-744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1.tar.gz
rust-744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1.zip
Auto merge of #101986 - WaffleLapkin:move_lint_note_to_the_bottom, r=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 <https://github.com/rust-lang/rust/issues/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 😢
Diffstat (limited to 'compiler/rustc_lint/src/non_ascii_idents.rs')
-rw-r--r--compiler/rustc_lint/src/non_ascii_idents.rs68
1 files changed, 40 insertions, 28 deletions
diff --git a/compiler/rustc_lint/src/non_ascii_idents.rs b/compiler/rustc_lint/src/non_ascii_idents.rs
index 764003e61a6..b2626efb6d7 100644
--- a/compiler/rustc_lint/src/non_ascii_idents.rs
+++ b/compiler/rustc_lint/src/non_ascii_idents.rs
@@ -180,15 +180,21 @@ impl EarlyLintPass for NonAsciiIdents {
                 continue;
             }
             has_non_ascii_idents = true;
-            cx.struct_span_lint(NON_ASCII_IDENTS, sp, |lint| {
-                lint.build(fluent::lint::identifier_non_ascii_char).emit();
-            });
+            cx.struct_span_lint(
+                NON_ASCII_IDENTS,
+                sp,
+                fluent::lint::identifier_non_ascii_char,
+                |lint| lint,
+            );
             if check_uncommon_codepoints
                 && !symbol_str.chars().all(GeneralSecurityProfile::identifier_allowed)
             {
-                cx.struct_span_lint(UNCOMMON_CODEPOINTS, sp, |lint| {
-                    lint.build(fluent::lint::identifier_uncommon_codepoints).emit();
-                })
+                cx.struct_span_lint(
+                    UNCOMMON_CODEPOINTS,
+                    sp,
+                    fluent::lint::identifier_uncommon_codepoints,
+                    |lint| lint,
+                )
             }
         }
 
@@ -216,13 +222,16 @@ impl EarlyLintPass for NonAsciiIdents {
                     .entry(skeleton_sym)
                     .and_modify(|(existing_symbol, existing_span, existing_is_ascii)| {
                         if !*existing_is_ascii || !is_ascii {
-                            cx.struct_span_lint(CONFUSABLE_IDENTS, sp, |lint| {
-                                lint.build(fluent::lint::confusable_identifier_pair)
-                                    .set_arg("existing_sym", *existing_symbol)
-                                    .set_arg("sym", symbol)
-                                    .span_label(*existing_span, fluent::lint::label)
-                                    .emit();
-                            });
+                            cx.struct_span_lint(
+                                CONFUSABLE_IDENTS,
+                                sp,
+                                fluent::lint::confusable_identifier_pair,
+                                |lint| {
+                                    lint.set_arg("existing_sym", *existing_symbol)
+                                        .set_arg("sym", symbol)
+                                        .span_label(*existing_span, fluent::lint::label)
+                                },
+                            );
                         }
                         if *existing_is_ascii && !is_ascii {
                             *existing_symbol = symbol;
@@ -322,22 +331,25 @@ impl EarlyLintPass for NonAsciiIdents {
                 }
 
                 for ((sp, ch_list), script_set) in lint_reports {
-                    cx.struct_span_lint(MIXED_SCRIPT_CONFUSABLES, sp, |lint| {
-                        let mut includes = String::new();
-                        for (idx, ch) in ch_list.into_iter().enumerate() {
-                            if idx != 0 {
-                                includes += ", ";
+                    cx.struct_span_lint(
+                        MIXED_SCRIPT_CONFUSABLES,
+                        sp,
+                        fluent::lint::mixed_script_confusables,
+                        |lint| {
+                            let mut includes = String::new();
+                            for (idx, ch) in ch_list.into_iter().enumerate() {
+                                if idx != 0 {
+                                    includes += ", ";
+                                }
+                                let char_info = format!("'{}' (U+{:04X})", ch, ch as u32);
+                                includes += &char_info;
                             }
-                            let char_info = format!("'{}' (U+{:04X})", ch, ch as u32);
-                            includes += &char_info;
-                        }
-                        lint.build(fluent::lint::mixed_script_confusables)
-                            .set_arg("set", script_set.to_string())
-                            .set_arg("includes", includes)
-                            .note(fluent::lint::includes_note)
-                            .note(fluent::lint::note)
-                            .emit();
-                    });
+                            lint.set_arg("set", script_set.to_string())
+                                .set_arg("includes", includes)
+                                .note(fluent::lint::includes_note)
+                                .note(fluent::lint::note)
+                        },
+                    );
                 }
             }
         }