New internal lint: interning_defined_symbol

Author: Michael Wright

clippy_lints/src/lib.rs

@@ -513,6 +513,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         #[cfg(feature = "internal-lints")]
         &utils::internal_lints::INVALID_PATHS,
         #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::INTERNING_DEFINED_SYMBOL,
+        #[cfg(feature = "internal-lints")]
         &utils::internal_lints::LINT_WITHOUT_LINT_PASS,
         #[cfg(feature = "internal-lints")]
         &utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
@@ -958,6 +960,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls);
         store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new());
         store.register_late_pass(|| box utils::internal_lints::InvalidPaths);
+        store.register_late_pass(|| box utils::internal_lints::InterningDefinedSymbol::default());
         store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default());
         store.register_late_pass(|| box utils::internal_lints::MatchTypeOnDiagItem);
         store.register_late_pass(|| box utils::internal_lints::OuterExpnDataPass);
@@ -1350,6 +1353,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS),
         LintId::of(&utils::internal_lints::DEFAULT_LINT),
         LintId::of(&utils::internal_lints::INVALID_PATHS),
+        LintId::of(&utils::internal_lints::INTERNING_DEFINED_SYMBOL),
         LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS),
         LintId::of(&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM),
         LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA),

clippy_lints/src/utils/internal_lints.rs

@@ -15,6 +15,7 @@ use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
 use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Node, Path, StmtKind, Ty, TyKind};
 use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
 use rustc_middle::hir::map::Map;
+use rustc_middle::mir::interpret::ConstValue;
 use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::{Span, Spanned};
@@ -247,6 +248,30 @@ declare_clippy_lint! {
     "invalid path"
 }
 
+declare_clippy_lint! {
+    /// **What it does:**
+    /// Checks for interning symbols that have already been pre-interned and defined as constants.
+    ///
+    /// **Why is this bad?**
+    /// It's faster and easier to use the symbol constant.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// Bad:
+    /// ```rust,ignore
+    /// let _ = sym!(f32);
+    /// ```
+    ///
+    /// Good:
+    /// ```rust,ignore
+    /// let _ = sym::f32;
+    /// ```
+    pub INTERNING_DEFINED_SYMBOL,
+    internal,
+    "interning a symbol that is pre-interned and defined as a constant"
+}
+
 declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]);
 
 impl EarlyLintPass for ClippyLintsInternal {
@@ -840,3 +865,58 @@ impl<'tcx> LateLintPass<'tcx> for InvalidPaths {
         }
     }
 }
+
+#[derive(Default)]
+pub struct InterningDefinedSymbol {
+    // Maps the symbol to the constant name.
+    symbol_map: FxHashMap<String, String>,
+}
+
+impl_lint_pass!(InterningDefinedSymbol => [INTERNING_DEFINED_SYMBOL]);
+
+impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol {
+    fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) {
+        if !self.symbol_map.is_empty() {
+            return;
+        }
+
+        if let Some(Res::Def(_, def_id)) = path_to_res(cx, &paths::SYM_MODULE) {
+            for item in cx.tcx.item_children(def_id).iter() {
+                if_chain! {
+                    if let Res::Def(DefKind::Const, item_def_id) = item.res;
+                    let ty = cx.tcx.type_of(item_def_id);
+                    if match_type(cx, ty, &paths::SYMBOL);
+                    if let Ok(ConstValue::Scalar(value)) = cx.tcx.const_eval_poly(item_def_id);
+                    if let Ok(value) = value.to_u32();
+                    then {
+                        // SAFETY: We're converting the raw bytes of the symbol value back
+                        // into a Symbol instance.
+                        let symbol = unsafe { std::mem::transmute::<u32, Symbol>(value) };
+                        self.symbol_map.insert(symbol.to_string(), item.ident.to_string());
+                    }
+                }
+            }
+        }
+    }
+
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if_chain! {
+            if let ExprKind::Call(func, [arg]) = &expr.kind;
+            if let ty::FnDef(def_id, _) = cx.typeck_results().expr_ty(func).kind();
+            if match_def_path(cx, *def_id, &paths::SYMBOL_INTERN);
+            if let Some(Constant::Str(arg)) = constant_simple(cx, cx.typeck_results(), arg);
+            if let Some(symbol_const) = self.symbol_map.get(&arg);
+            then {
+                span_lint_and_sugg(
+                    cx,
+                    INTERNING_DEFINED_SYMBOL,
+                    is_expn_of(expr.span, "sym").unwrap_or(expr.span),
+                    "interning a defined symbol",
+                    "try",
+                    format!("rustc_span::symbol::sym::{}", symbol_const),
+                    Applicability::MachineApplicable,
+                );
+            }
+        }
+    }
+}

clippy_lints/src/utils/paths.rs

@@ -146,6 +146,12 @@ pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"];
 pub const STR_LEN: [&str; 4] = ["core", "str", "<impl str>", "len"];
 pub const STR_STARTS_WITH: [&str; 4] = ["core", "str", "<impl str>", "starts_with"];
 #[cfg(feature = "internal-lints")]
+pub const SYMBOL: [&str; 3] = ["rustc_span", "symbol", "Symbol"];
+#[cfg(feature = "internal-lints")]
+pub const SYMBOL_INTERN: [&str; 4] = ["rustc_span", "symbol", "Symbol", "intern"];
+#[cfg(feature = "internal-lints")]
+pub const SYM_MODULE: [&str; 3] = ["rustc_span", "symbol", "sym"];
+#[cfg(feature = "internal-lints")]
 pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"];
 pub const TO_OWNED: [&str; 3] = ["alloc", "borrow", "ToOwned"];
 pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"];

tests/ui-internal/interning_defined_symbol.fixed

@@ -0,0 +1,33 @@
+// run-rustfix
+#![deny(clippy::internal)]
+#![feature(rustc_private)]
+
+extern crate rustc_span;
+
+use rustc_span::symbol::Symbol;
+
+macro_rules! sym {
+    ($tt:tt) => {
+        rustc_span::symbol::Symbol::intern(stringify!($tt))
+    };
+}
+
+fn main() {
+    // Direct use of Symbol::intern
+    let _ = rustc_span::symbol::sym::f32;
+
+    // Using a sym macro
+    let _ = rustc_span::symbol::sym::f32;
+
+    // Correct suggestion when symbol isn't stringified constant name
+    let _ = rustc_span::symbol::sym::proc_dash_macro;
+
+    // Interning a symbol that is not defined
+    let _ = Symbol::intern("xyz123");
+    let _ = sym!(xyz123);
+
+    // Using a different `intern` function
+    let _ = intern("f32");
+}
+
+fn intern(_: &str) {}

tests/ui-internal/interning_defined_symbol.rs

@@ -0,0 +1,33 @@
+// run-rustfix
+#![deny(clippy::internal)]
+#![feature(rustc_private)]
+
+extern crate rustc_span;
+
+use rustc_span::symbol::Symbol;
+
+macro_rules! sym {
+    ($tt:tt) => {
+        rustc_span::symbol::Symbol::intern(stringify!($tt))
+    };
+}
+
+fn main() {
+    // Direct use of Symbol::intern
+    let _ = Symbol::intern("f32");
+
+    // Using a sym macro
+    let _ = sym!(f32);
+
+    // Correct suggestion when symbol isn't stringified constant name
+    let _ = Symbol::intern("proc-macro");
+
+    // Interning a symbol that is not defined
+    let _ = Symbol::intern("xyz123");
+    let _ = sym!(xyz123);
+
+    // Using a different `intern` function
+    let _ = intern("f32");
+}
+
+fn intern(_: &str) {}

tests/ui-internal/interning_defined_symbol.stderr

@@ -0,0 +1,27 @@
+error: interning a defined symbol
+  --> $DIR/interning_defined_symbol.rs:17:13
+   |
+LL |     let _ = Symbol::intern("f32");
+   |             ^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::symbol::sym::f32`
+   |
+note: the lint level is defined here
+  --> $DIR/interning_defined_symbol.rs:2:9
+   |
+LL | #![deny(clippy::internal)]
+   |         ^^^^^^^^^^^^^^^^
+   = note: `#[deny(clippy::interning_defined_symbol)]` implied by `#[deny(clippy::internal)]`
+
+error: interning a defined symbol
+  --> $DIR/interning_defined_symbol.rs:20:13
+   |
+LL |     let _ = sym!(f32);
+   |             ^^^^^^^^^ help: try: `rustc_span::symbol::sym::f32`
+
+error: interning a defined symbol
+  --> $DIR/interning_defined_symbol.rs:23:13
+   |
+LL |     let _ = Symbol::intern("proc-macro");
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::symbol::sym::proc_dash_macro`
+
+error: aborting due to 3 previous errors
+