Implement lint against Symbol::intern on a string literal

Author: clubby789

compiler/rustc_lint/messages.ftl

@@ -772,6 +772,9 @@ lint_suspicious_double_ref_clone =
 lint_suspicious_double_ref_deref =
     using `.deref()` on a double reference, which returns `{$ty}` instead of dereferencing the inner type
 
+lint_symbol_intern_string_literal = using `Symbol::intern` on a string literal
+    .help = consider adding the symbol to `compiler/rustc_span/src/symbol.rs`
+
 lint_trailing_semi_macro = trailing semicolon in macro used in expression position
     .note1 = macro invocations at the end of a block are treated as expressions
     .note2 = to ignore the value produced by the macro, add a semicolon after the invocation of `{$name}`

compiler/rustc_lint/src/internal.rs

@@ -17,8 +17,9 @@ use tracing::debug;
 
 use crate::lints::{
     BadOptAccessDiag, DefaultHashTypesDiag, DiagOutOfImpl, LintPassByHand, NonExistentDocKeyword,
-    NonGlobImportTypeIrInherent, QueryInstability, QueryUntracked, SpanUseEqCtxtDiag, TyQualified,
-    TykindDiag, TykindKind, TypeIrInherentUsage, UntranslatableDiag,
+    NonGlobImportTypeIrInherent, QueryInstability, QueryUntracked, SpanUseEqCtxtDiag,
+    SymbolInternStringLiteralDiag, TyQualified, TykindDiag, TykindKind, TypeIrInherentUsage,
+    UntranslatableDiag,
 };
 use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
 
@@ -657,3 +658,33 @@ fn is_span_ctxt_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
         _ => false,
     }
 }
+
+declare_tool_lint! {
+    /// The `symbol_intern_string_literal` detects `Symbol::intern` being called on a string literal
+    pub rustc::SYMBOL_INTERN_STRING_LITERAL,
+    // rustc_driver crates out of the compiler can't/shouldn't add preinterned symbols;
+    // bootstrap will deny this manually
+    Allow,
+    "Forbid uses of string literals in `Symbol::intern`, suggesting preinterning instead",
+    report_in_external_macro: true
+}
+
+declare_lint_pass!(SymbolInternStringLiteral => [SYMBOL_INTERN_STRING_LITERAL]);
+
+impl<'tcx> LateLintPass<'tcx> for SymbolInternStringLiteral {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
+        if let ExprKind::Call(path, [arg]) = expr.kind
+            && let ExprKind::Path(ref qpath) = path.kind
+            && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
+            && cx.tcx.is_diagnostic_item(sym::SymbolIntern, def_id)
+            && let ExprKind::Lit(kind) = arg.kind
+            && let rustc_ast::LitKind::Str(_, _) = kind.node
+        {
+            cx.emit_span_lint(
+                SYMBOL_INTERN_STRING_LITERAL,
+                kind.span,
+                SymbolInternStringLiteralDiag,
+            );
+        }
+    }
+}

compiler/rustc_lint/src/lib.rs

@@ -614,6 +614,8 @@ fn register_internals(store: &mut LintStore) {
     store.register_late_mod_pass(|_| Box::new(PassByValue));
     store.register_lints(&SpanUseEqCtxt::lint_vec());
     store.register_late_mod_pass(|_| Box::new(SpanUseEqCtxt));
+    store.register_lints(&SymbolInternStringLiteral::lint_vec());
+    store.register_late_mod_pass(|_| Box::new(SymbolInternStringLiteral));
     // FIXME(davidtwco): deliberately do not include `UNTRANSLATABLE_DIAGNOSTIC` and
     // `DIAGNOSTIC_OUTSIDE_OF_IMPL` here because `-Wrustc::internal` is provided to every crate and
     // these lints will trigger all of the time - change this once migration to diagnostic structs

compiler/rustc_lint/src/lints.rs

@@ -907,6 +907,11 @@ pub(crate) struct QueryUntracked {
 #[diag(lint_span_use_eq_ctxt)]
 pub(crate) struct SpanUseEqCtxtDiag;
 
+#[derive(LintDiagnostic)]
+#[diag(lint_symbol_intern_string_literal)]
+#[help]
+pub(crate) struct SymbolInternStringLiteralDiag;
+
 #[derive(LintDiagnostic)]
 #[diag(lint_tykind_kind)]
 pub(crate) struct TykindKind {

compiler/rustc_span/src/symbol.rs

@@ -315,6 +315,7 @@ symbols! {
         StructuralPartialEq,
         SubdiagMessage,
         Subdiagnostic,
+        SymbolIntern,
         Sync,
         SyncUnsafeCell,
         T,
@@ -2401,6 +2402,7 @@ impl Symbol {
     }
 
     /// Maps a string to its interned representation.
+    #[rustc_diagnostic_item = "SymbolIntern"]
     pub fn intern(string: &str) -> Self {
         with_session_globals(|session_globals| session_globals.symbol_interner.intern(string))
     }

src/bootstrap/src/core/builder/cargo.rs

@@ -1030,6 +1030,15 @@ impl Builder<'_> {
 
         if mode == Mode::Rustc {
             rustflags.arg("-Wrustc::internal");
+            // cfg(bootstrap) - remove this check when lint is in bootstrap compiler
+            if stage != 0 {
+                // Lint is allow by default so downstream tools don't get a lit
+                // they can do nothing about
+                // We shouldn't be preinterning symbols used by tests
+                if cmd_kind != Kind::Test {
+                    rustflags.arg("-Drustc::symbol_intern_string_literal");
+                }
+            }
             // FIXME(edition_2024): Change this to `-Wrust_2024_idioms` when all
             // of the individual lints are satisfied.
             rustflags.arg("-Wkeyword_idents_2024");