From 7b337f6e259ce3ae19e1c0c22c8ff493ecb4a8a3 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Wed, 23 Apr 2025 13:24:01 +0000 Subject: Replace some `Symbol::as_str` usage --- clippy_lints_internal/src/interning_literals.rs | 102 -------------- clippy_lints_internal/src/lib.rs | 8 +- clippy_lints_internal/src/symbols.rs | 169 ++++++++++++++++++++++++ 3 files changed, 174 insertions(+), 105 deletions(-) delete mode 100644 clippy_lints_internal/src/interning_literals.rs create mode 100644 clippy_lints_internal/src/symbols.rs (limited to 'clippy_lints_internal') diff --git a/clippy_lints_internal/src/interning_literals.rs b/clippy_lints_internal/src/interning_literals.rs deleted file mode 100644 index 6cee3744234..00000000000 --- a/clippy_lints_internal/src/interning_literals.rs +++ /dev/null @@ -1,102 +0,0 @@ -use clippy_utils::consts::{ConstEvalCtxt, Constant}; -use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::ty::match_type; -use clippy_utils::{def_path_def_ids, paths}; -use rustc_data_structures::fx::FxHashMap; -use rustc_errors::Applicability; -use rustc_hir::def::{DefKind, Res}; -use rustc_hir::{Expr, ExprKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_lint_defs::declare_tool_lint; -use rustc_middle::mir::ConstValue; -use rustc_middle::ty; -use rustc_session::impl_lint_pass; -use rustc_span::sym; -use rustc_span::symbol::Symbol; - -declare_tool_lint! { - /// ### What it does - /// Checks for interning string literals as symbols - /// - /// ### Why is this bad? - /// It's faster and easier to use the symbol constant. If one doesn't exist it can be added to `clippy_utils/src/sym.rs` - /// - /// ### Example - /// ```rust,ignore - /// let _ = Symbol::intern("f32"); - /// ``` - /// - /// Use instead: - /// ```rust,ignore - /// let _ = sym::f32; - /// ``` - pub clippy::INTERNING_LITERALS, - Warn, - "interning a symbol that is a literal", - report_in_external_macro: true -} - -#[derive(Default)] -pub struct InterningDefinedSymbol { - // Maps the symbol to the import path - symbol_map: FxHashMap, -} - -impl_lint_pass!(InterningDefinedSymbol => [INTERNING_LITERALS]); - -impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol { - fn check_crate(&mut self, cx: &LateContext<'_>) { - let modules = [ - ("kw", &paths::KW_MODULE[..]), - ("sym", &paths::SYM_MODULE), - ("sym", &paths::CLIPPY_SYM_MODULE), - ]; - for (prefix, module) in modules { - for def_id in def_path_def_ids(cx.tcx, module) { - // When linting `clippy_utils` itself we can't use `module_children` as it's a local def id. It will - // still lint but the suggestion will say to add it to `sym.rs` even if it's already there - if def_id.is_local() { - continue; - } - - for item in cx.tcx.module_children(def_id) { - if let Res::Def(DefKind::Const, item_def_id) = item.res - && let ty = cx.tcx.type_of(item_def_id).instantiate_identity() - && match_type(cx, ty, &paths::SYMBOL) - && let Ok(ConstValue::Scalar(value)) = cx.tcx.const_eval_poly(item_def_id) - && let Some(value) = value.to_u32().discard_err() - { - self.symbol_map.insert(value, (prefix, item.ident.name)); - } - } - } - } - } - - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Call(func, [arg]) = &expr.kind - && let ty::FnDef(def_id, _) = cx.typeck_results().expr_ty(func).kind() - && cx.tcx.is_diagnostic_item(sym::SymbolIntern, *def_id) - && let Some(Constant::Str(arg)) = ConstEvalCtxt::new(cx).eval_simple(arg) - { - span_lint_and_then( - cx, - INTERNING_LITERALS, - expr.span, - "interning a string literal", - |diag| { - let value = Symbol::intern(&arg).as_u32(); - let (message, path) = if let Some((prefix, name)) = self.symbol_map.get(&value) { - ("use the preinterned symbol", format!("{prefix}::{name}")) - } else { - ( - "add the symbol to `clippy_utils/src/sym.rs` and use it", - format!("sym::{}", arg.replace(|ch: char| !ch.is_alphanumeric(), "_")), - ) - }; - diag.span_suggestion_verbose(expr.span, message, path, Applicability::MaybeIncorrect); - }, - ); - } - } -} diff --git a/clippy_lints_internal/src/lib.rs b/clippy_lints_internal/src/lib.rs index 1c42f4112f9..b02d378619c 100644 --- a/clippy_lints_internal/src/lib.rs +++ b/clippy_lints_internal/src/lib.rs @@ -2,6 +2,7 @@ #![allow( clippy::missing_docs_in_private_items, clippy::must_use_candidate, + clippy::symbol_as_str, rustc::diagnostic_outside_of_impl, rustc::untranslatable_diagnostic )] @@ -31,12 +32,12 @@ extern crate rustc_span; mod almost_standard_lint_formulation; mod collapsible_calls; -mod interning_literals; mod invalid_paths; mod lint_without_lint_pass; mod msrv_attr_impl; mod outer_expn_data_pass; mod produce_ice; +mod symbols; mod unnecessary_def_path; mod unsorted_clippy_utils_paths; @@ -45,7 +46,6 @@ use rustc_lint::{Lint, LintStore}; static LINTS: &[&Lint] = &[ almost_standard_lint_formulation::ALMOST_STANDARD_LINT_FORMULATION, collapsible_calls::COLLAPSIBLE_SPAN_LINT_CALLS, - interning_literals::INTERNING_LITERALS, invalid_paths::INVALID_PATHS, lint_without_lint_pass::DEFAULT_LINT, lint_without_lint_pass::INVALID_CLIPPY_VERSION_ATTRIBUTE, @@ -54,6 +54,8 @@ static LINTS: &[&Lint] = &[ msrv_attr_impl::MISSING_MSRV_ATTR_IMPL, outer_expn_data_pass::OUTER_EXPN_EXPN_DATA, produce_ice::PRODUCE_ICE, + symbols::INTERNING_LITERALS, + symbols::SYMBOL_AS_STR, unnecessary_def_path::UNNECESSARY_DEF_PATH, unsorted_clippy_utils_paths::UNSORTED_CLIPPY_UTILS_PATHS, ]; @@ -65,7 +67,7 @@ pub fn register_lints(store: &mut LintStore) { store.register_early_pass(|| Box::new(produce_ice::ProduceIce)); store.register_late_pass(|_| Box::new(collapsible_calls::CollapsibleCalls)); store.register_late_pass(|_| Box::new(invalid_paths::InvalidPaths)); - store.register_late_pass(|_| Box::::default()); + store.register_late_pass(|_| Box::::default()); store.register_late_pass(|_| Box::::default()); store.register_late_pass(|_| Box::::default()); store.register_late_pass(|_| Box::new(outer_expn_data_pass::OuterExpnDataPass)); diff --git a/clippy_lints_internal/src/symbols.rs b/clippy_lints_internal/src/symbols.rs new file mode 100644 index 00000000000..c64e5821916 --- /dev/null +++ b/clippy_lints_internal/src/symbols.rs @@ -0,0 +1,169 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::ty::match_type; +use clippy_utils::{def_path_def_ids, match_def_path, paths}; +use rustc_ast::LitKind; +use rustc_data_structures::fx::FxHashMap; +use rustc_errors::Applicability; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint_defs::declare_tool_lint; +use rustc_middle::mir::ConstValue; +use rustc_middle::ty; +use rustc_session::impl_lint_pass; +use rustc_span::symbol::Symbol; +use rustc_span::{Span, sym}; + +declare_tool_lint! { + /// ### What it does + /// Checks for interning string literals as symbols + /// + /// ### Why is this bad? + /// It's faster and easier to use the symbol constant. If one doesn't exist it can be added to `clippy_utils/src/sym.rs` + /// + /// ### Example + /// ```rust,ignore + /// let _ = Symbol::intern("f32"); + /// ``` + /// + /// Use instead: + /// ```rust,ignore + /// let _ = sym::f32; + /// ``` + pub clippy::INTERNING_LITERALS, + Warn, + "interning a symbol that is a literal", + report_in_external_macro: true +} + +declare_tool_lint! { + /// ### What it does + /// Checks for calls to `Symbol::as_str` + /// + /// ### Why is this bad? + /// It's faster and easier to use the symbol constant. If one doesn't exist it can be added to `clippy_utils/src/sym.rs` + /// + /// ### Example + /// ```rust,ignore + /// symbol.as_str() == "foo" + /// ``` + /// + /// Use instead: + /// ```rust,ignore + /// symbol == sym::foo + /// ``` + pub clippy::SYMBOL_AS_STR, + Warn, + "calls to `Symbol::as_str`", + report_in_external_macro: true +} + +#[derive(Default)] +pub struct Symbols { + // Maps the symbol to the import path + symbol_map: FxHashMap, +} + +impl_lint_pass!(Symbols => [INTERNING_LITERALS, SYMBOL_AS_STR]); + +impl<'tcx> LateLintPass<'tcx> for Symbols { + fn check_crate(&mut self, cx: &LateContext<'_>) { + let modules = [ + ("kw", &paths::KW_MODULE[..]), + ("sym", &paths::SYM_MODULE), + ("sym", &paths::CLIPPY_SYM_MODULE), + ]; + for (prefix, module) in modules { + for def_id in def_path_def_ids(cx.tcx, module) { + // When linting `clippy_utils` itself we can't use `module_children` as it's a local def id. It will + // still lint but the suggestion will say to add it to `sym.rs` even if it's already there + if def_id.is_local() { + continue; + } + + for item in cx.tcx.module_children(def_id) { + if let Res::Def(DefKind::Const, item_def_id) = item.res + && let ty = cx.tcx.type_of(item_def_id).instantiate_identity() + && match_type(cx, ty, &paths::SYMBOL) + && let Ok(ConstValue::Scalar(value)) = cx.tcx.const_eval_poly(item_def_id) + && let Some(value) = value.to_u32().discard_err() + { + self.symbol_map.insert(value, (prefix, item.ident.name)); + } + } + } + } + } + + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if let ExprKind::Call(func, [arg]) = &expr.kind + && let ty::FnDef(def_id, _) = cx.typeck_results().expr_ty(func).kind() + && cx.tcx.is_diagnostic_item(sym::SymbolIntern, *def_id) + && let ExprKind::Lit(lit) = arg.kind + && let LitKind::Str(name, _) = lit.node + { + span_lint_and_then( + cx, + INTERNING_LITERALS, + expr.span, + "interning a string literal", + |diag| { + let (message, path) = suggestion(&mut self.symbol_map, name); + diag.span_suggestion_verbose(expr.span, message, path, Applicability::MaybeIncorrect); + }, + ); + } + + if let ExprKind::Binary(_, lhs, rhs) = expr.kind { + check_binary(cx, lhs, rhs, &mut self.symbol_map); + check_binary(cx, rhs, lhs, &mut self.symbol_map); + } + } +} + +fn check_binary( + cx: &LateContext<'_>, + lhs: &Expr<'_>, + rhs: &Expr<'_>, + symbols: &mut FxHashMap, +) { + if let Some(removal_span) = as_str_span(cx, lhs) + && let ExprKind::Lit(lit) = rhs.kind + && let LitKind::Str(name, _) = lit.node + { + span_lint_and_then(cx, SYMBOL_AS_STR, lhs.span, "converting a Symbol to a string", |diag| { + let (message, path) = suggestion(symbols, name); + diag.multipart_suggestion_verbose( + message, + vec![(removal_span, String::new()), (rhs.span, path)], + Applicability::MachineApplicable, + ); + }); + } +} + +fn suggestion(symbols: &mut FxHashMap, name: Symbol) -> (&'static str, String) { + if let Some((prefix, name)) = symbols.get(&name.as_u32()) { + ("use the preinterned symbol", format!("{prefix}::{name}")) + } else { + ( + "add the symbol to `clippy_utils/src/sym.rs` and use it", + format!("sym::{}", name.as_str().replace(|ch: char| !ch.is_alphanumeric(), "_")), + ) + } +} + +/// ```ignore +/// symbol.as_str() +/// // ^^^^^^^^ +/// ``` +fn as_str_span(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { + if let ExprKind::MethodCall(_, recv, [], _) = expr.kind + && let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) + && match_def_path(cx, method_def_id, &paths::SYMBOL_AS_STR) + { + Some(recv.span.shrink_to_hi().to(expr.span.shrink_to_hi())) + } else { + None + } +} -- cgit 1.4.1-3-g733a5