Auto merge of #6448 - mikerite:interning_defined_symbol, r=Manishearth

New internal lint: Interning defined symbol

New internal lint: interning_defined_symbol

changelog: none

clippy_lints/src/lib.rs

@@ -511,6 +511,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         #[cfg(feature = "internal-lints")]
         &utils::internal_lints::DEFAULT_LINT,
         #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::INTERNING_DEFINED_SYMBOL,
+        #[cfg(feature = "internal-lints")]
         &utils::internal_lints::INVALID_PATHS,
         #[cfg(feature = "internal-lints")]
         &utils::internal_lints::LINT_WITHOUT_LINT_PASS,
@@ -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);
@@ -1349,6 +1352,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS),
         LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS),
         LintId::of(&utils::internal_lints::DEFAULT_LINT),
+        LintId::of(&utils::internal_lints::INTERNING_DEFINED_SYMBOL),
         LintId::of(&utils::internal_lints::INVALID_PATHS),
         LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS),
         LintId::of(&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM),

clippy_lints/src/manual_ok_or.rs

@@ -8,6 +8,7 @@ use rustc_lint::LintContext;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::lint::in_external_macro;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::symbol::sym;
 
 declare_clippy_lint! {
     /// **What it does:**
@@ -51,7 +52,7 @@ impl LateLintPass<'_> for ManualOkOr {
             if args.len() == 3;
             let method_receiver = &args[0];
             let ty = cx.typeck_results().expr_ty(method_receiver);
-            if is_type_diagnostic_item(cx, ty, sym!(option_type));
+            if is_type_diagnostic_item(cx, ty, sym::option_type);
             let or_expr = &args[1];
             if is_ok_wrapping(cx, &args[2]);
             if let ExprKind::Call(Expr { kind: ExprKind::Path(err_path), .. }, &[ref err_arg]) = or_expr.kind;

clippy_lints/src/methods/mod.rs

@@ -1568,7 +1568,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
                 lint_expect_fun_call(cx, expr, *method_span, &method_call.ident.as_str(), args);
 
                 let self_ty = cx.typeck_results().expr_ty_adjusted(&args[0]);
-                if args.len() == 1 && method_call.ident.name == sym!(clone) {
+                if args.len() == 1 && method_call.ident.name == sym::clone {
                     lint_clone_on_copy(cx, expr, &args[0], self_ty);
                     lint_clone_on_ref_ptr(cx, expr, &args[0]);
                 }
@@ -1592,7 +1592,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
                             }
                         }
                     },
-                    ty::Ref(..) if method_call.ident.name == sym!(into_iter) => {
+                    ty::Ref(..) if method_call.ident.name == sym::into_iter => {
                         lint_into_iter(cx, expr, self_ty, *method_span);
                     },
                     _ => (),
@@ -2647,9 +2647,9 @@ fn lint_unwrap(cx: &LateContext<'_>, expr: &hir::Expr<'_>, unwrap_args: &[hir::E
 fn lint_expect(cx: &LateContext<'_>, expr: &hir::Expr<'_>, expect_args: &[hir::Expr<'_>]) {
     let obj_ty = cx.typeck_results().expr_ty(&expect_args[0]).peel_refs();
 
-    let mess = if is_type_diagnostic_item(cx, obj_ty, sym!(option_type)) {
+    let mess = if is_type_diagnostic_item(cx, obj_ty, sym::option_type) {
         Some((EXPECT_USED, "an Option", "None"))
-    } else if is_type_diagnostic_item(cx, obj_ty, sym!(result_type)) {
+    } else if is_type_diagnostic_item(cx, obj_ty, sym::result_type) {
         Some((EXPECT_USED, "a Result", "Err"))
     } else {
         None
@@ -3142,7 +3142,7 @@ fn lint_search_is_some<'tcx>(
     else if search_method == "find" {
         let is_string_or_str_slice = |e| {
             let self_ty = cx.typeck_results().expr_ty(e).peel_refs();
-            if is_type_diagnostic_item(cx, self_ty, sym!(string_type)) {
+            if is_type_diagnostic_item(cx, self_ty, sym::string_type) {
                 true
             } else {
                 *self_ty.kind() == ty::Str

clippy_lints/src/ref_option_ref.rs

@@ -2,6 +2,7 @@ use crate::utils::{last_path_segment, snippet, span_lint_and_sugg};
 use rustc_hir::{GenericArg, Mutability, Ty, TyKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::symbol::sym;
 
 use if_chain::if_chain;
 use rustc_errors::Applicability;
@@ -41,7 +42,7 @@ impl<'tcx> LateLintPass<'tcx> for RefOptionRef {
             if let Some(res) = last.res;
             if let Some(def_id) = res.opt_def_id();
 
-            if cx.tcx.is_diagnostic_item(sym!(option_type), def_id);
+            if cx.tcx.is_diagnostic_item(sym::option_type, def_id);
             if let Some(ref params) = last_path_segment(qpath).args ;
             if !params.parenthesized;
             if let Some(inner_ty) = params.args.iter().find_map(|arg| match arg {

clippy_lints/src/strings.rs

@@ -372,7 +372,7 @@ impl LateLintPass<'_> for StringToString {
             if let ExprKind::MethodCall(path, _, args, _) = &expr.kind;
             if path.ident.name == sym!(to_string);
             let ty = cx.typeck_results().expr_ty(&args[0]);
-            if is_type_diagnostic_item(cx, ty, sym!(string_type));
+            if is_type_diagnostic_item(cx, ty, sym::string_type);
             then {
                 span_lint_and_help(
                     cx,

clippy_lints/src/unnecessary_wraps.rs

@@ -9,6 +9,7 @@ use rustc_hir::{Body, ExprKind, FnDecl, HirId, ItemKind, Node};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::subst::GenericArgKind;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::symbol::sym;
 use rustc_span::Span;
 
 declare_clippy_lint! {
@@ -82,9 +83,9 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWraps {
             }
         }
 
-        let (return_type, path) = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(option_type)) {
+        let (return_type, path) = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym::option_type) {
             ("Option", &paths::OPTION_SOME)
-        } else if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(result_type)) {
+        } else if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym::result_type) {
             ("Result", &paths::RESULT_OK)
         } else {
             return;

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,56 @@ impl<'tcx> LateLintPass<'tcx> for InvalidPaths {
         }
     }
 }
+
+#[derive(Default)]
+pub struct InterningDefinedSymbol {
+    // Maps the symbol value to the constant name.
+    symbol_map: FxHashMap<u32, 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 {
+                        self.symbol_map.insert(value, 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);
+            let value = Symbol::intern(&arg).as_u32();
+            if let Some(symbol_const) = self.symbol_map.get(&value);
+            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
+