Rollup merge of #100377 - est31:fluent_grepability, r=davidtwco

Replace - with _ in fluent slugs to improve developer workflows

This is a proposal to smoothen the compiler contribution experience in the face of the move to fluent.

## Context

The fluent project has introduced a layer of abstraction to compiler errors. Previously, people would write down error messages directly in the same file the code was located to emit them. Now, there is a slug that connects the code in the compiler to the error message in the ftl file.

You can look at 7ef610c003f8072ec4ca4ecf195922a9a44e48dd to see an example of the changes:

Old:
```Rust
let msg = format!(
    "bounds on `{}` are most likely incorrect, consider instead \
        using `{}` to detect whether a type can be trivially dropped",
    predicate,
    cx.tcx.def_path_str(needs_drop)
);
lint.build(&msg).emit();
```
New (Rust side):
```Rust
lint.build(fluent::lint::drop_trait_constraints)
    .set_arg("predicate", predicate)
    .set_arg("needs_drop", cx.tcx.def_path_str(needs_drop))
    .emit();
```
New (Fluent side):
```fluent
lint-drop-trait-constraints =
    bounds on `{$predicate}` are most likely incorrect, consider instead using `{$needs_drop}` to detect whether a type can be trivially dropped
```

You will note that in the ftl file, the slug is slightly different from the slug in the Rust file: The ftl slug uses `-` (e.g. `lint-drop-trait-constraints`) while the rust slug uses `::` and `_` (e.g. `lint::drop_trait_constraints`). This choice was probably done due to:

* Rust not accepting `-` in identifiers (as it is an operator)
* fluent not supporting the `:` character in slug names (parse error upon attempts)
* all official fluent documentation using `-` instead of `_`

## The problem

The two different types of slugs, one with `-`, and one with `_`, cause difficulties for contributors. Imagine you don't have perfect knowledge of where stuff is in the compiler (i would say this is most people), and you encounter an error for which you think there is something you could improve that is not just a rewording.

So you want to find out where in the compiler's code that error is being emitted. The best way is via grepping.

1. you grep for the message in the compiler's source code. You discover the ftl file and find out the slug for that error.
2. That slug however contains `-` instead of `_`, so you have to manually translate the `-`'s into `_`s, and furthermore either remove the leading module name, or replace the first `-` with a `::`.
3. you do a second grep to get to the emitting location in the compiler's code.

This translation difficulty in step 2 appears also in the other direction when you want to figure out what some code in the compiler is doing and use error messages to help your understanding. Comments and variable names are way less exposed to users so [are more likely going to lie](https://github.com/rust-lang/rust/commit/cc3c5d2700481bae497d6cde825c1d48e79c776a) than error messages.

I think that at least the `-`→`_` translation which makes up most of step 2 can be removed at low cost.

## The solution

If you look closely, the practice of fluent to use `-` is only a stylistic choice and it is not enforced by fluent implementations, neither the playground nor the one the rust compiler uses, that slugs may not contain `_`. Thus, we can in fact migrate the ftl side to `_`. So now we'll have slugs like  `lint_drop_trait_constraints` on the ftl side. You only have to do one replacement now to get to the Rust slug: remove the first `_` and place a `::` in its stead. I would argue that this change is in fact useful as it allows you to control whether you want to look at the rust side of things or the ftl side of things via changing the query string only: with an increased number of translations checked into the repository, grepping for raw slugs will return the slug in many ftl files, so an explicit step to look for the source code is always useful. In the other direction (rust to fluent), you don't need a translation at all any more, as you can just take the final piece of the slug (e.g. `drop_trait_constraints`) and grep for that. The PR also adds enforcement to forbid usage of `_` in slug names. Internal slug names (those leading with a `-`) are exempt from that enforcement.

As another workflow that benefits from this change, people who add new errors don't have to do that `-` conversion either.

| Before/After | Fluent slug | Rust slug (no change) |
|--|--|--|
| Before | `lint-drop-trait-constraints` | `lint::drop_trait_constraints`|
| After | `lint_drop_trait_constraints` | `lint::drop_trait_constraints`|

Note that I've suggested this previously in the translation thread on zulip. I think it's important to think about non-translator contribution impact of fluent. I have certainly plans for more improvements, but this is a good first step.

``@rustbot`` label A-diagnostics

3 files changed, 40 insertions, 18 deletions

diff --git a/compiler/rustc_macros/src/diagnostics/fluent.rs b/compiler/rustc_macros/src/diagnostics/fluent.rs
index 562d5e9f4d2..2e7652ad333 100644
--- a/compiler/rustc_macros/src/diagnostics/fluent.rs
+++ b/compiler/rustc_macros/src/diagnostics/fluent.rs
@@ -189,13 +189,25 @@ pub(crate) fn fluent_messages(input: proc_macro::TokenStream) -> proc_macro::Tok
             if let Entry::Message(Message { id: Identifier { name }, attributes, .. }) = entry {
                 let _ = previous_defns.entry(name.to_string()).or_insert(ident_span);
 
-                // `typeck-foo-bar` => `foo_bar` (in `typeck.ftl`)
-                // `const-eval-baz` => `baz` (in `const_eval.ftl`)
+                if name.contains('-') {
+                    Diagnostic::spanned(
+                        ident_span,
+                        Level::Error,
+                        format!("name `{name}` contains a '-' character"),
+                    )
+                    .help("replace any '-'s with '_'s")
+                    .emit();
+                }
+
+                // `typeck_foo_bar` => `foo_bar` (in `typeck.ftl`)
+                // `const_eval_baz` => `baz` (in `const_eval.ftl`)
+                // `const-eval-hyphen-having` => `hyphen_having` (in `const_eval.ftl`)
+                // The last case we error about above, but we want to fall back gracefully
+                // so that only the error is being emitted and not also one about the macro
+                // failing.
                 let snake_name = Ident::new(
                     // FIXME: should probably trim prefix, not replace all occurrences
-                    &name
-                        .replace(&format!("{}-", res.ident).replace('_', "-"), "")
-                        .replace('-', "_"),
+                    &name.replace('-', "_").replace(&format!("{}_", res.ident), ""),
                     span,
                 );
                 constants.extend(quote! {
@@ -212,6 +224,16 @@ pub(crate) fn fluent_messages(input: proc_macro::TokenStream) -> proc_macro::Tok
                         continue;
                     }
 
+                    if attr_name.contains('-') {
+                        Diagnostic::spanned(
+                            ident_span,
+                            Level::Error,
+                            format!("attribute `{attr_name}` contains a '-' character"),
+                        )
+                        .help("replace any '-'s with '_'s")
+                        .emit();
+                    }
+
                     constants.extend(quote! {
                         pub const #snake_name: crate::SubdiagnosticMessage =
                             crate::SubdiagnosticMessage::FluentAttr(
diff --git a/compiler/rustc_macros/src/diagnostics/mod.rs b/compiler/rustc_macros/src/diagnostics/mod.rs
index 39979002666..733454cb2a7 100644
--- a/compiler/rustc_macros/src/diagnostics/mod.rs
+++ b/compiler/rustc_macros/src/diagnostics/mod.rs
@@ -38,9 +38,9 @@ use synstructure::Structure;
 /// ```
 ///
 /// ```fluent
-/// move-out-of-borrow = cannot move out of {$name} because it is borrowed
+/// move_out_of_borrow = cannot move out of {$name} because it is borrowed
 ///     .label = cannot move out of borrow
-///     .first-borrow-label = `{$ty}` first borrowed here
+///     .first_borrow_label = `{$ty}` first borrowed here
 ///     .suggestion = consider cloning here
 /// ```
 ///
@@ -84,9 +84,9 @@ pub fn session_diagnostic_derive(s: Structure<'_>) -> TokenStream {
 /// ```
 ///
 /// ```fluent
-/// lint-atomic-ordering-invalid-fail-success = `{$method}`'s success ordering must be at least as strong as its failure ordering
-///     .fail-label = `{$fail_ordering}` failure ordering
-///     .success-label = `{$success_ordering}` success ordering
+/// lint_atomic_ordering_invalid_fail_success = `{$method}`'s success ordering must be at least as strong as its failure ordering
+///     .fail_label = `{$fail_ordering}` failure ordering
+///     .success_label = `{$success_ordering}` success ordering
 ///     .suggestion = consider using `{$success_suggestion}` success ordering instead
 /// ```
 ///
@@ -140,11 +140,11 @@ pub fn lint_diagnostic_derive(s: Structure<'_>) -> TokenStream {
 /// ```
 ///
 /// ```fluent
-/// parser-expected-identifier = expected identifier
+/// parser_expected_identifier = expected identifier
 ///
-/// parser-expected-identifier-found = expected identifier, found {$found}
+/// parser_expected_identifier-found = expected identifier, found {$found}
 ///
-/// parser-raw-identifier = escape `{$ident}` to use it as an identifier
+/// parser_raw_identifier = escape `{$ident}` to use it as an identifier
 /// ```
 ///
 /// Then, later, to add the subdiagnostic:
diff --git a/compiler/rustc_macros/src/lib.rs b/compiler/rustc_macros/src/lib.rs
index ab509b26f1c..2f9c13cf817 100644
--- a/compiler/rustc_macros/src/lib.rs
+++ b/compiler/rustc_macros/src/lib.rs
@@ -65,10 +65,10 @@ pub fn newtype_index(input: TokenStream) -> TokenStream {
 /// ..where `typeck.ftl` has the following contents..
 ///
 /// ```fluent
-/// typeck-field-multiply-specified-in-initializer =
+/// typeck_field_multiply_specified_in_initializer =
 ///     field `{$ident}` specified more than once
 ///     .label = used more than once
-///     .label-previous-use = first use of `{$ident}`
+///     .label_previous_use = first use of `{$ident}`
 /// ```
 /// ...then the macro parse the Fluent resource, emitting a diagnostic if it fails to do so, and
 /// will generate the following code:
@@ -81,11 +81,11 @@ pub fn newtype_index(input: TokenStream) -> TokenStream {
 /// mod fluent_generated {
 ///     mod typeck {
 ///         pub const field_multiply_specified_in_initializer: DiagnosticMessage =
-///             DiagnosticMessage::fluent("typeck-field-multiply-specified-in-initializer");
+///             DiagnosticMessage::fluent("typeck_field_multiply_specified_in_initializer");
 ///         pub const field_multiply_specified_in_initializer_label_previous_use: DiagnosticMessage =
 ///             DiagnosticMessage::fluent_attr(
-///                 "typeck-field-multiply-specified-in-initializer",
-///                 "previous-use-label"
+///                 "typeck_field_multiply_specified_in_initializer",
+///                 "previous_use_label"
 ///             );
 ///     }
 /// }