diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f06926fa9..0498d469c 100644 --- a/clippy_lints/src/lib.rs +++ b/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), diff --git a/clippy_lints/src/manual_ok_or.rs b/clippy_lints/src/manual_ok_or.rs index c99d2e35b..b97d97ea1 100644 --- a/clippy_lints/src/manual_ok_or.rs +++ b/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; diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 6a141d777..37691f27b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/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 diff --git a/clippy_lints/src/ref_option_ref.rs b/clippy_lints/src/ref_option_ref.rs index a914a77d4..803ebada5 100644 --- a/clippy_lints/src/ref_option_ref.rs +++ b/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 { diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 77e790733..31dd59654 100644 --- a/clippy_lints/src/strings.rs +++ b/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, diff --git a/clippy_lints/src/unnecessary_wraps.rs b/clippy_lints/src/unnecessary_wraps.rs index e763da593..5b9a80f92 100644 --- a/clippy_lints/src/unnecessary_wraps.rs +++ b/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; diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 8b59a9541..9ba39f73e 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/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, +} + +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, + ); + } + } + } +} diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 6fdc7b458..2080a49a1 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/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", "", "len"]; pub const STR_STARTS_WITH: [&str; 4] = ["core", "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"]; diff --git a/tests/ui-internal/interning_defined_symbol.fixed b/tests/ui-internal/interning_defined_symbol.fixed new file mode 100644 index 000000000..c6b84d2ef --- /dev/null +++ b/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) {} diff --git a/tests/ui-internal/interning_defined_symbol.rs b/tests/ui-internal/interning_defined_symbol.rs new file mode 100644 index 000000000..9ec82d4ad --- /dev/null +++ b/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) {} diff --git a/tests/ui-internal/interning_defined_symbol.stderr b/tests/ui-internal/interning_defined_symbol.stderr new file mode 100644 index 000000000..74b906c8a --- /dev/null +++ b/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 +