Clean up rustc_session::output::{find,validate}_crate_name

Author: fmease

compiler/rustc_session/messages.ftl

@@ -8,7 +8,7 @@ session_cannot_mix_and_match_sanitizers = `-Zsanitizer={$first}` is incompatible
 session_cli_feature_diagnostic_help =
     add `-Zcrate-attr="feature({$feature})"` to the command-line options to enable
 
-session_crate_name_does_not_match = `--crate-name` and `#[crate_name]` are required to match, but `{$s}` != `{$name}`
+session_crate_name_does_not_match = `--crate-name` and `#[crate_name]` are required to match, but `{$crate_name}` != `{$attr_crate_name}`
 
 session_crate_name_empty = crate name must not be empty
 
@@ -52,8 +52,8 @@ session_instrumentation_not_supported = {$us} instrumentation is not supported f
 session_int_literal_too_large = integer literal is too large
     .note = value exceeds limit of `{$limit}`
 
-session_invalid_character_in_create_name = invalid character `{$character}` in crate name: `{$crate_name}`
-session_invalid_character_in_create_name_help = you can either pass `--crate-name` on the command line or add `#![crate_name="…"]` to set the crate name
+session_invalid_character_in_crate_name = invalid character {$character} in crate name: `{$crate_name}`
+    .help = you can either pass `--crate-name` on the command line or add `#![crate_name = "…"]` to set the crate name
 
 session_invalid_float_literal_suffix = invalid suffix `{$suffix}` for float literal
     .label = invalid suffix `{$suffix}`

compiler/rustc_session/src/errors.rs

@@ -217,8 +217,8 @@ pub(crate) struct FileWriteFail<'a> {
 pub(crate) struct CrateNameDoesNotMatch {
     #[primary_span]
     pub(crate) span: Span,
-    pub(crate) s: Symbol,
-    pub(crate) name: Symbol,
+    pub(crate) crate_name: Symbol,
+    pub(crate) attr_crate_name: Symbol,
 }
 
 #[derive(Diagnostic)]
@@ -235,20 +235,14 @@ pub(crate) struct CrateNameEmpty {
 }
 
 #[derive(Diagnostic)]
-#[diag(session_invalid_character_in_create_name)]
+#[diag(session_invalid_character_in_crate_name)]
 pub(crate) struct InvalidCharacterInCrateName {
     #[primary_span]
     pub(crate) span: Option<Span>,
     pub(crate) character: char,
     pub(crate) crate_name: Symbol,
-    #[subdiagnostic]
-    pub(crate) crate_name_help: Option<InvalidCrateNameHelp>,
-}
-
-#[derive(Subdiagnostic)]
-pub(crate) enum InvalidCrateNameHelp {
-    #[help(session_invalid_character_in_create_name_help)]
-    AddCrateName,
+    #[help]
+    pub(crate) help: Option<()>,
 }
 
 #[derive(Subdiagnostic)]

compiler/rustc_session/src/output.rs

@@ -9,7 +9,7 @@ use crate::Session;
 use crate::config::{self, CrateType, Input, OutFileName, OutputFilenames, OutputType};
 use crate::errors::{
     self, CrateNameDoesNotMatch, CrateNameEmpty, CrateNameInvalid, FileIsNotWriteable,
-    InvalidCharacterInCrateName, InvalidCrateNameHelp,
+    InvalidCharacterInCrateName,
 };
 
 pub fn out_filename(
@@ -49,10 +49,13 @@ fn is_writeable(p: &Path) -> bool {
     }
 }
 
+/// Find and [validate] the crate name.
+///
+/// [validate]: validate_crate_name
 pub fn find_crate_name(sess: &Session, attrs: &[ast::Attribute]) -> Symbol {
-    let validate = |s: Symbol, span: Option<Span>| {
-        validate_crate_name(sess, s, span);
-        s
+    let validate = |name, span| {
+        validate_crate_name(sess, name, span);
+        name
     };
 
     // Look in attributes 100% of the time to make sure the attribute is marked
@@ -62,56 +65,62 @@ pub fn find_crate_name(sess: &Session, attrs: &[ast::Attribute]) -> Symbol {
     let attr_crate_name =
         attr::find_by_name(attrs, sym::crate_name).and_then(|at| at.value_str().map(|s| (at, s)));
 
-    if let Some(ref s) = sess.opts.crate_name {
-        let s = Symbol::intern(s);
-        if let Some((attr, name)) = attr_crate_name {
-            if name != s {
-                sess.dcx().emit_err(CrateNameDoesNotMatch { span: attr.span, s, name });
-            }
+    if let Some(crate_name) = &sess.opts.crate_name {
+        let crate_name = Symbol::intern(crate_name);
+        if let Some((attr, attr_crate_name)) = attr_crate_name
+            && attr_crate_name != crate_name
+        {
+            sess.dcx().emit_err(CrateNameDoesNotMatch {
+                span: attr.span,
+                crate_name,
+                attr_crate_name,
+            });
         }
-        return validate(s, None);
+        return validate(crate_name, None);
     }
 
-    if let Some((attr, s)) = attr_crate_name {
-        return validate(s, Some(attr.span));
+    if let Some((attr, crate_name)) = attr_crate_name {
+        return validate(crate_name, Some(attr.span));
     }
-    if let Input::File(ref path) = sess.io.input {
-        if let Some(s) = path.file_stem().and_then(|s| s.to_str()) {
-            if s.starts_with('-') {
-                sess.dcx().emit_err(CrateNameInvalid { s });
-            } else {
-                return validate(Symbol::intern(&s.replace('-', "_")), None);
-            }
+
+    if let Input::File(ref path) = sess.io.input
+        && let Some(s) = path.file_stem().and_then(|s| s.to_str())
+    {
+        if s.starts_with('-') {
+            sess.dcx().emit_err(CrateNameInvalid { s });
+        } else {
+            return validate(Symbol::intern(&s.replace('-', "_")), None);
         }
     }
 
     sym::rust_out
 }
 
-pub fn validate_crate_name(sess: &Session, s: Symbol, sp: Option<Span>) {
+/// Validate the given crate name.
+///
+/// Note that this validation is more permissive than identifier parsing. It considers
+/// non-empty sequences of alphanumeric and underscore characters to be valid crate names.
+/// Most notably, it accepts names starting with a numeric character like `0`!
+///
+/// Furthermore, this shouldn't be taken as the canonical crate name validator.
+/// Other places may use a more restrictive grammar (e.g., identifier or ASCII identifier).
+pub fn validate_crate_name(sess: &Session, crate_name: Symbol, span: Option<Span>) {
     let mut guar = None;
-    {
-        if s.is_empty() {
-            guar = Some(sess.dcx().emit_err(CrateNameEmpty { span: sp }));
-        }
-        for c in s.as_str().chars() {
-            if c.is_alphanumeric() {
-                continue;
-            }
-            if c == '_' {
-                continue;
-            }
-            guar = Some(sess.dcx().emit_err(InvalidCharacterInCrateName {
-                span: sp,
-                character: c,
-                crate_name: s,
-                crate_name_help: if sp.is_none() {
-                    Some(InvalidCrateNameHelp::AddCrateName)
-                } else {
-                    None
-                },
-            }));
+
+    if crate_name.is_empty() {
+        guar = Some(sess.dcx().emit_err(CrateNameEmpty { span }));
+    }
+
+    for c in crate_name.as_str().chars() {
+        if c.is_alphanumeric() || c == '_' {
+            continue;
         }
+        guar = Some(sess.dcx().emit_err(InvalidCharacterInCrateName {
+            span,
+            character: c,
+            crate_name,
+            help: span.is_none().then_some(()),
+        }));
     }
 
     if let Some(guar) = guar {

tests/ui/command/need-crate-arg-ignore-tidy$x.stderr

@@ -1,6 +1,6 @@
-error: invalid character `'$'` in crate name: `need_crate_arg_ignore_tidy$x`
+error: invalid character '$' in crate name: `need_crate_arg_ignore_tidy$x`
    |
-   = help: you can either pass `--crate-name` on the command line or add `#![crate_name="…"]` to set the crate name
+   = help: you can either pass `--crate-name` on the command line or add `#![crate_name = "…"]` to set the crate name
 
 error: aborting due to 1 previous error