Auto merge of rust-lang#100675 - Xiretza:fluent-mandate-crate-prefix, r=davidtwco

fluent: mandate slug names to be prefixed by crate name

This is currently only convention, but not actively checked for.

Additionally, improve error messages to highlight the path of the offending fluent file rather than the identifier preceding it.

This will conflict with rust-lang#100671, so I'll leave it as draft until that's merged.

Author: bors

compiler/rustc_error_messages/locales/en-US/privacy.ftl

@@ -14,7 +14,7 @@ privacy_in_public_interface = {$vis_descr} {$kind} `{$descr}` in public interfac
 privacy_from_private_dep_in_public_interface =
     {$kind} `{$descr}` from private dependency '{$krate}' in public interface
 
-private_in_public_lint =
+privacy_private_in_public_lint =
     {$vis_descr} {$kind} `{$descr}` in public interface (error {$kind ->
         [trait] E0445
         *[other] E0446

compiler/rustc_macros/src/diagnostics/fluent.rs

@@ -187,11 +187,11 @@ pub(crate) fn fluent_messages(input: proc_macro::TokenStream) -> proc_macro::Tok
         for entry in resource.entries() {
             let span = res.ident.span();
             if let Entry::Message(Message { id: Identifier { name }, attributes, .. }) = entry {
-                let _ = previous_defns.entry(name.to_string()).or_insert(ident_span);
+                let _ = previous_defns.entry(name.to_string()).or_insert(path_span);
 
                 if name.contains('-') {
                     Diagnostic::spanned(
-                        ident_span,
+                        path_span,
                         Level::Error,
                         format!("name `{name}` contains a '-' character"),
                     )
@@ -205,11 +205,23 @@ pub(crate) fn fluent_messages(input: proc_macro::TokenStream) -> proc_macro::Tok
                 // 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('-', "_").replace(&format!("{}_", res.ident), ""),
-                    span,
-                );
+                let crate_prefix = format!("{}_", res.ident);
+
+                let snake_name = name.replace('-', "_");
+                let snake_name = match snake_name.strip_prefix(&crate_prefix) {
+                    Some(rest) => Ident::new(rest, span),
+                    None => {
+                        Diagnostic::spanned(
+                            path_span,
+                            Level::Error,
+                            format!("name `{name}` does not start with the crate name"),
+                        )
+                        .help(format!("prepend `{crate_prefix}` to the slug name: `{crate_prefix}{snake_name}`"))
+                        .emit();
+                        Ident::new(&snake_name, span)
+                    }
+                };
+
                 constants.extend(quote! {
                     pub const #snake_name: crate::DiagnosticMessage =
                         crate::DiagnosticMessage::FluentIdentifier(
@@ -226,7 +238,7 @@ pub(crate) fn fluent_messages(input: proc_macro::TokenStream) -> proc_macro::Tok
 
                     if attr_name.contains('-') {
                         Diagnostic::spanned(
-                            ident_span,
+                            path_span,
                             Level::Error,
                             format!("attribute `{attr_name}` contains a '-' character"),
                         )
@@ -249,7 +261,7 @@ pub(crate) fn fluent_messages(input: proc_macro::TokenStream) -> proc_macro::Tok
                 match e {
                     FluentError::Overriding { kind, id } => {
                         Diagnostic::spanned(
-                            ident_span,
+                            path_span,
                             Level::Error,
                             format!("overrides existing {}: `{}`", kind, id),
                         )

src/test/ui-fulldeps/fluent-messages/duplicate-a-b.ftl

@@ -0,0 +1 @@
+a_b_key = Value

src/test/ui-fulldeps/fluent-messages/duplicate-a.ftl

@@ -1 +1 @@
-key = Value
+a_b_key = Value

src/test/ui-fulldeps/fluent-messages/duplicate-b.ftl

@@ -1,2 +1,2 @@
-some_slug = hi
+label_with_hyphens_some_slug = hi
     .label-has-hyphens = test

src/test/ui-fulldeps/fluent-messages/label-with-hyphens.ftl

@@ -0,0 +1,2 @@
+with-hyphens = 1234
+test-crate_foo = abcd

src/test/ui-fulldeps/fluent-messages/missing-crate-name.ftl

@@ -1 +1 @@
-this-slug-has-hyphens = hi
+slug_with_hyphens_this-slug-has-hyphens = hi

src/test/ui-fulldeps/fluent-messages/slug-with-hyphens.ftl

@@ -50,8 +50,8 @@ mod duplicate {
 
     fluent_messages! {
         a => "./duplicate-a.ftl",
-        b => "./duplicate-b.ftl",
-//~^ ERROR overrides existing message: `key`
+        a_b => "./duplicate-a-b.ftl",
+//~^ ERROR overrides existing message: `a_b_key`
     }
 }
 
@@ -60,7 +60,7 @@ mod slug_with_hyphens {
 
     fluent_messages! {
         slug_with_hyphens => "./slug-with-hyphens.ftl",
-//~^ ERROR name `this-slug-has-hyphens` contains a '-' character
+//~^ ERROR name `slug_with_hyphens_this-slug-has-hyphens` contains a '-' character
     }
 }
 
@@ -80,5 +80,18 @@ mod valid {
         valid => "./valid.ftl",
     }
 
-    use self::fluent_generated::{DEFAULT_LOCALE_RESOURCES, valid::valid};
+    use self::fluent_generated::{DEFAULT_LOCALE_RESOURCES, valid::key};
+}
+
+mod missing_crate_name {
+    use super::fluent_messages;
+
+    fluent_messages! {
+        test_crate => "./missing-crate-name.ftl",
+//~^ ERROR name `test-crate_foo` contains a '-' character
+//~| ERROR name `with-hyphens` contains a '-' character
+//~| ERROR name `with-hyphens` does not start with the crate name
+    }
+
+    use self::fluent_generated::{DEFAULT_LOCALE_RESOURCES, test_crate::{foo, with_hyphens}};
 }

src/test/ui-fulldeps/fluent-messages/test.rs

@@ -29,33 +29,57 @@ error: expected a message field for "missing_message"
   | ^^^^^^^^^^^^^^^^^
   |
 
-error: overrides existing message: `key`
-  --> $DIR/test.rs:53:9
+error: overrides existing message: `a_b_key`
+  --> $DIR/test.rs:53:16
    |
-LL |         b => "./duplicate-b.ftl",
-   |         ^
+LL |         a_b => "./duplicate-a-b.ftl",
+   |                ^^^^^^^^^^^^^^^^^^^^^
    |
 help: previously defined in this resource
-  --> $DIR/test.rs:52:9
+  --> $DIR/test.rs:52:14
    |
 LL |         a => "./duplicate-a.ftl",
-   |         ^
+   |              ^^^^^^^^^^^^^^^^^^^
 
-error: name `this-slug-has-hyphens` contains a '-' character
-  --> $DIR/test.rs:62:9
+error: name `slug_with_hyphens_this-slug-has-hyphens` contains a '-' character
+  --> $DIR/test.rs:62:30
    |
 LL |         slug_with_hyphens => "./slug-with-hyphens.ftl",
-   |         ^^^^^^^^^^^^^^^^^
+   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: replace any '-'s with '_'s
 
 error: attribute `label-has-hyphens` contains a '-' character
-  --> $DIR/test.rs:71:9
+  --> $DIR/test.rs:71:31
    |
 LL |         label_with_hyphens => "./label-with-hyphens.ftl",
-   |         ^^^^^^^^^^^^^^^^^^
+   |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: replace any '-'s with '_'s
 
-error: aborting due to 6 previous errors
+error: name `with-hyphens` contains a '-' character
+  --> $DIR/test.rs:90:23
+   |
+LL |         test_crate => "./missing-crate-name.ftl",
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: replace any '-'s with '_'s
+
+error: name `with-hyphens` does not start with the crate name
+  --> $DIR/test.rs:90:23
+   |
+LL |         test_crate => "./missing-crate-name.ftl",
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: prepend `test_crate_` to the slug name: `test_crate_with_hyphens`
+
+error: name `test-crate_foo` contains a '-' character
+  --> $DIR/test.rs:90:23
+   |
+LL |         test_crate => "./missing-crate-name.ftl",
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: replace any '-'s with '_'s
+
+error: aborting due to 9 previous errors