Auto merge of #6569 - camsteffen:symbol-comparison, r=Manishearth
Internal lint symbol comparisons changelog: none * Added awareness of `rustc_span::symbol::kw::*` symbols. * Compare with const symbols: `symbol.as_str() == "self"` => `symbol == kw::SelfLower` * Don't compare symbols by string: `a.as_str() == b.as_str()` => `a == b` * Lint comparing with `to_ident_string` or `to_string` instead of `Symbol::as_str`.
This commit is contained in:
commit
68bcd202fe
@ -399,7 +399,7 @@ fn extract_clippy_lint(lint: &NestedMetaItem) -> Option<SymbolStr> {
|
||||
if let Some(meta_item) = lint.meta_item();
|
||||
if meta_item.path.segments.len() > 1;
|
||||
if let tool_name = meta_item.path.segments[0].ident;
|
||||
if tool_name.as_str() == "clippy";
|
||||
if tool_name.name == sym::clippy;
|
||||
let lint_name = meta_item.path.segments.last().unwrap().ident.name;
|
||||
then {
|
||||
return Some(lint_name.as_str());
|
||||
|
@ -145,7 +145,7 @@ impl<'tcx, 'l> ArmVisitor<'tcx, 'l> {
|
||||
fn is_mutex_lock_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
|
||||
if_chain! {
|
||||
if let ExprKind::MethodCall(path, _span, args, _) = &expr.kind;
|
||||
if path.ident.to_string() == "lock";
|
||||
if path.ident.as_str() == "lock";
|
||||
let ty = cx.typeck_results().expr_ty(&args[0]);
|
||||
if is_type_diagnostic_item(cx, ty, sym!(mutex_type));
|
||||
then {
|
||||
|
@ -526,6 +526,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
&utils::internal_lints::OUTER_EXPN_EXPN_DATA,
|
||||
#[cfg(feature = "internal-lints")]
|
||||
&utils::internal_lints::PRODUCE_ICE,
|
||||
#[cfg(feature = "internal-lints")]
|
||||
&utils::internal_lints::UNNECESSARY_SYMBOL_STR,
|
||||
&approx_const::APPROX_CONSTANT,
|
||||
&arithmetic::FLOAT_ARITHMETIC,
|
||||
&arithmetic::INTEGER_ARITHMETIC,
|
||||
@ -1372,6 +1374,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
LintId::of(&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM),
|
||||
LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA),
|
||||
LintId::of(&utils::internal_lints::PRODUCE_ICE),
|
||||
LintId::of(&utils::internal_lints::UNNECESSARY_SYMBOL_STR),
|
||||
]);
|
||||
|
||||
store.register_group(true, "clippy::all", Some("clippy"), vec![
|
||||
|
@ -9,7 +9,7 @@ use rustc_hir::{
|
||||
};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use rustc_span::Span;
|
||||
use rustc_span::{sym, Span};
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** It checks for manual implementations of `async` functions.
|
||||
@ -137,7 +137,7 @@ fn future_output_ty<'tcx>(trait_ref: &'tcx TraitRef<'tcx>) -> Option<&'tcx Ty<'t
|
||||
if let Some(args) = segment.args;
|
||||
if args.bindings.len() == 1;
|
||||
let binding = &args.bindings[0];
|
||||
if binding.ident.as_str() == "Output";
|
||||
if binding.ident.name == sym::Output;
|
||||
if let TypeBindingKind::Equality{ty: output} = binding.kind;
|
||||
then {
|
||||
return Some(output)
|
||||
|
@ -53,7 +53,7 @@ impl<'tcx> LateLintPass<'tcx> for MapClone {
|
||||
if_chain! {
|
||||
if let hir::ExprKind::MethodCall(ref method, _, ref args, _) = e.kind;
|
||||
if args.len() == 2;
|
||||
if method.ident.as_str() == "map";
|
||||
if method.ident.name == sym::map;
|
||||
let ty = cx.typeck_results().expr_ty(&args[0]);
|
||||
if is_type_diagnostic_item(cx, ty, sym::option_type) || match_trait_method(cx, e, &paths::ITERATOR);
|
||||
if let hir::ExprKind::Closure(_, _, body_id, _, _) = args[1].kind;
|
||||
|
@ -63,7 +63,7 @@ impl<'tcx> LateLintPass<'tcx> for MapIdentity {
|
||||
fn get_map_argument<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<&'a [Expr<'a>]> {
|
||||
if_chain! {
|
||||
if let ExprKind::MethodCall(ref method, _, ref args, _) = expr.kind;
|
||||
if args.len() == 2 && method.ident.as_str() == "map";
|
||||
if args.len() == 2 && method.ident.name == sym::map;
|
||||
let caller_ty = cx.typeck_results().expr_ty(&args[0]);
|
||||
if match_trait_method(cx, expr, &paths::ITERATOR)
|
||||
|| is_type_diagnostic_item(cx, caller_ty, sym::result_type)
|
||||
|
@ -3095,7 +3095,7 @@ fn lint_flat_map_identity<'tcx>(
|
||||
if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = body.value.kind;
|
||||
|
||||
if path.segments.len() == 1;
|
||||
if path.segments[0].ident.as_str() == binding_ident.as_str();
|
||||
if path.segments[0].ident.name == binding_ident.name;
|
||||
|
||||
then {
|
||||
apply_lint("called `flat_map(|x| x)` on an `Iterator`");
|
||||
|
@ -89,9 +89,9 @@ fn min_max<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(MinMax, Cons
|
||||
if let [obj, _] = args;
|
||||
if cx.typeck_results().expr_ty(obj).is_floating_point() || match_trait_method(cx, expr, &paths::ORD);
|
||||
then {
|
||||
if path.ident.as_str() == sym!(max).as_str() {
|
||||
if path.ident.name == sym!(max) {
|
||||
fetch_const(cx, args, MinMax::Max)
|
||||
} else if path.ident.as_str() == sym!(min).as_str() {
|
||||
} else if path.ident.name == sym!(min) {
|
||||
fetch_const(cx, args, MinMax::Min)
|
||||
} else {
|
||||
None
|
||||
|
@ -63,7 +63,7 @@ impl MissingDoc {
|
||||
if let Some(meta) = list.get(0);
|
||||
if let Some(name) = meta.ident();
|
||||
then {
|
||||
name.as_str() == "include"
|
||||
name.name == sym::include
|
||||
} else {
|
||||
false
|
||||
}
|
||||
|
@ -13,6 +13,7 @@ use rustc_infer::infer::TyCtxtInferExt;
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty::{self, TypeFoldable};
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use rustc_span::symbol::kw;
|
||||
use rustc_span::{sym, Span};
|
||||
use rustc_target::spec::abi::Abi;
|
||||
use rustc_trait_selection::traits;
|
||||
@ -153,7 +154,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue {
|
||||
// Ignore `self`s.
|
||||
if idx == 0 {
|
||||
if let PatKind::Binding(.., ident, _) = arg.pat.kind {
|
||||
if ident.as_str() == "self" {
|
||||
if ident.name == kw::SelfLower {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
@ -66,7 +66,7 @@ declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]);
|
||||
/// Returns true iff the given expression is the result of calling `Result::ok`
|
||||
fn is_result_ok(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
|
||||
if let ExprKind::MethodCall(ref path, _, &[ref receiver], _) = &expr.kind {
|
||||
path.ident.name.to_ident_string() == "ok"
|
||||
path.ident.name.as_str() == "ok"
|
||||
&& is_type_diagnostic_item(cx, &cx.typeck_results().expr_ty(&receiver), sym::result_type)
|
||||
} else {
|
||||
false
|
||||
|
@ -389,5 +389,5 @@ fn is_self_shadow(name: Symbol, expr: &Expr<'_>) -> bool {
|
||||
}
|
||||
|
||||
fn path_eq_name(name: Symbol, path: &Path<'_>) -> bool {
|
||||
!path.is_global() && path.segments.len() == 1 && path.segments[0].ident.as_str() == name.as_str()
|
||||
!path.is_global() && path.segments.len() == 1 && path.segments[0].ident.name == name
|
||||
}
|
||||
|
@ -91,7 +91,7 @@ fn check_manual_swap(cx: &LateContext<'_>, block: &Block<'_>) {
|
||||
if let ExprKind::Path(QPath::Resolved(None, ref rhs2)) = rhs2.kind;
|
||||
if rhs2.segments.len() == 1;
|
||||
|
||||
if ident.as_str() == rhs2.segments[0].ident.as_str();
|
||||
if ident.name == rhs2.segments[0].ident.name;
|
||||
if eq_expr_value(cx, tmp_init, lhs1);
|
||||
if eq_expr_value(cx, rhs1, lhs2);
|
||||
then {
|
||||
|
@ -183,7 +183,7 @@ fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<LintTrigger> {
|
||||
Param { pat: Pat { kind: PatKind::Binding(_, _, right_ident, _), .. }, .. }
|
||||
] = &closure_body.params;
|
||||
if let ExprKind::MethodCall(method_path, _, [ref left_expr, ref right_expr], _) = &closure_body.value.kind;
|
||||
if method_path.ident.name.to_ident_string() == "cmp";
|
||||
if method_path.ident.name == sym::cmp;
|
||||
then {
|
||||
let (closure_body, closure_arg, reverse) = if mirrored_exprs(
|
||||
&cx,
|
||||
|
@ -80,10 +80,10 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
|
||||
);
|
||||
}
|
||||
}
|
||||
if match_trait_method(cx, e, &paths::INTO_ITERATOR) && &*name.ident.as_str() == "into_iter" {
|
||||
if match_trait_method(cx, e, &paths::INTO_ITERATOR) && name.ident.name == sym::into_iter {
|
||||
if let Some(parent_expr) = get_parent_expr(cx, e) {
|
||||
if let ExprKind::MethodCall(ref parent_name, ..) = parent_expr.kind {
|
||||
if &*parent_name.ident.as_str() != "into_iter" {
|
||||
if parent_name.ident.name != sym::into_iter {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
@ -1,6 +1,7 @@
|
||||
use rustc_ast::ast;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_session::Session;
|
||||
use rustc_span::sym;
|
||||
use std::str::FromStr;
|
||||
|
||||
/// Deprecation status of attributes known by Clippy.
|
||||
@ -64,11 +65,11 @@ pub fn get_attr<'a>(
|
||||
return false;
|
||||
};
|
||||
let attr_segments = &attr.path.segments;
|
||||
if attr_segments.len() == 2 && attr_segments[0].ident.to_string() == "clippy" {
|
||||
if attr_segments.len() == 2 && attr_segments[0].ident.name == sym::clippy {
|
||||
BUILTIN_ATTRIBUTES
|
||||
.iter()
|
||||
.find_map(|(builtin_name, deprecation_status)| {
|
||||
if *builtin_name == attr_segments[1].ident.to_string() {
|
||||
.find_map(|&(builtin_name, ref deprecation_status)| {
|
||||
if attr_segments[1].ident.name.as_str() == builtin_name {
|
||||
Some(deprecation_status)
|
||||
} else {
|
||||
None
|
||||
@ -99,7 +100,7 @@ pub fn get_attr<'a>(
|
||||
},
|
||||
DeprecationStatus::None => {
|
||||
diag.cancel();
|
||||
attr_segments[1].ident.to_string() == name
|
||||
attr_segments[1].ident.name.as_str() == name
|
||||
},
|
||||
}
|
||||
},
|
||||
|
@ -86,7 +86,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
|
||||
lb == rb && l_mut == r_mut && self.eq_expr(le, re)
|
||||
},
|
||||
(&ExprKind::Continue(li), &ExprKind::Continue(ri)) => {
|
||||
both(&li.label, &ri.label, |l, r| l.ident.as_str() == r.ident.as_str())
|
||||
both(&li.label, &ri.label, |l, r| l.ident.name == r.ident.name)
|
||||
},
|
||||
(&ExprKind::Assign(ref ll, ref lr, _), &ExprKind::Assign(ref rl, ref rr, _)) => {
|
||||
self.allow_side_effects && self.eq_expr(ll, rl) && self.eq_expr(lr, rr)
|
||||
@ -102,7 +102,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
|
||||
})
|
||||
},
|
||||
(&ExprKind::Break(li, ref le), &ExprKind::Break(ri, ref re)) => {
|
||||
both(&li.label, &ri.label, |l, r| l.ident.as_str() == r.ident.as_str())
|
||||
both(&li.label, &ri.label, |l, r| l.ident.name == r.ident.name)
|
||||
&& both(le, re, |l, r| self.eq_expr(l, r))
|
||||
},
|
||||
(&ExprKind::Box(ref l), &ExprKind::Box(ref r)) => self.eq_expr(l, r),
|
||||
@ -121,7 +121,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
|
||||
},
|
||||
(&ExprKind::Lit(ref l), &ExprKind::Lit(ref r)) => l.node == r.node,
|
||||
(&ExprKind::Loop(ref lb, ref ll, ref lls), &ExprKind::Loop(ref rb, ref rl, ref rls)) => {
|
||||
lls == rls && self.eq_block(lb, rb) && both(ll, rl, |l, r| l.ident.as_str() == r.ident.as_str())
|
||||
lls == rls && self.eq_block(lb, rb) && both(ll, rl, |l, r| l.ident.name == r.ident.name)
|
||||
},
|
||||
(&ExprKind::Match(ref le, ref la, ref ls), &ExprKind::Match(ref re, ref ra, ref rs)) => {
|
||||
ls == rs
|
||||
@ -188,7 +188,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
|
||||
|
||||
pub fn eq_fieldpat(&mut self, left: &FieldPat<'_>, right: &FieldPat<'_>) -> bool {
|
||||
let (FieldPat { ident: li, pat: lp, .. }, FieldPat { ident: ri, pat: rp, .. }) = (&left, &right);
|
||||
li.name.as_str() == ri.name.as_str() && self.eq_pat(lp, rp)
|
||||
li.name == ri.name && self.eq_pat(lp, rp)
|
||||
}
|
||||
|
||||
/// Checks whether two patterns are the same.
|
||||
@ -202,7 +202,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
|
||||
self.eq_qpath(lp, rp) && over(la, ra, |l, r| self.eq_pat(l, r)) && ls == rs
|
||||
},
|
||||
(&PatKind::Binding(ref lb, .., ref li, ref lp), &PatKind::Binding(ref rb, .., ref ri, ref rp)) => {
|
||||
lb == rb && li.name.as_str() == ri.name.as_str() && both(lp, rp, |l, r| self.eq_pat(l, r))
|
||||
lb == rb && li.name == ri.name && both(lp, rp, |l, r| self.eq_pat(l, r))
|
||||
},
|
||||
(&PatKind::Path(ref l), &PatKind::Path(ref r)) => self.eq_qpath(l, r),
|
||||
(&PatKind::Lit(ref l), &PatKind::Lit(ref r)) => self.eq_expr(l, r),
|
||||
@ -263,8 +263,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
|
||||
pub fn eq_path_segment(&mut self, left: &PathSegment<'_>, right: &PathSegment<'_>) -> bool {
|
||||
// The == of idents doesn't work with different contexts,
|
||||
// we have to be explicit about hygiene
|
||||
left.ident.as_str() == right.ident.as_str()
|
||||
&& both(&left.args, &right.args, |l, r| self.eq_path_parameters(l, r))
|
||||
left.ident.name == right.ident.name && both(&left.args, &right.args, |l, r| self.eq_path_parameters(l, r))
|
||||
}
|
||||
|
||||
pub fn eq_ty(&mut self, left: &Ty<'_>, right: &Ty<'_>) -> bool {
|
||||
|
@ -10,9 +10,12 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir as hir;
|
||||
use rustc_hir::def::{DefKind, Res};
|
||||
use rustc_hir::def_id::DefId;
|
||||
use rustc_hir::hir_id::CRATE_HIR_ID;
|
||||
use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
|
||||
use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Node, Path, StmtKind, Ty, TyKind};
|
||||
use rustc_hir::{
|
||||
BinOpKind, Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Node, Path, StmtKind, Ty, TyKind, UnOp,
|
||||
};
|
||||
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
|
||||
use rustc_middle::hir::map::Map;
|
||||
use rustc_middle::mir::interpret::ConstValue;
|
||||
@ -272,6 +275,28 @@ declare_clippy_lint! {
|
||||
"interning a symbol that is pre-interned and defined as a constant"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for unnecessary conversion from Symbol to a string.
|
||||
///
|
||||
/// **Why is this bad?** It's faster use symbols directly intead of strings.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// Bad:
|
||||
/// ```rust,ignore
|
||||
/// symbol.as_str() == "clippy";
|
||||
/// ```
|
||||
///
|
||||
/// Good:
|
||||
/// ```rust,ignore
|
||||
/// symbol == sym::clippy;
|
||||
/// ```
|
||||
pub UNNECESSARY_SYMBOL_STR,
|
||||
internal,
|
||||
"unnecessary conversion between Symbol and string"
|
||||
}
|
||||
|
||||
declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]);
|
||||
|
||||
impl EarlyLintPass for ClippyLintsInternal {
|
||||
@ -868,11 +893,11 @@ impl<'tcx> LateLintPass<'tcx> for InvalidPaths {
|
||||
|
||||
#[derive(Default)]
|
||||
pub struct InterningDefinedSymbol {
|
||||
// Maps the symbol value to the constant name.
|
||||
symbol_map: FxHashMap<u32, String>,
|
||||
// Maps the symbol value to the constant DefId.
|
||||
symbol_map: FxHashMap<u32, DefId>,
|
||||
}
|
||||
|
||||
impl_lint_pass!(InterningDefinedSymbol => [INTERNING_DEFINED_SYMBOL]);
|
||||
impl_lint_pass!(InterningDefinedSymbol => [INTERNING_DEFINED_SYMBOL, UNNECESSARY_SYMBOL_STR]);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol {
|
||||
fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) {
|
||||
@ -880,16 +905,18 @@ impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol {
|
||||
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());
|
||||
for &module in &[&paths::KW_MODULE, &paths::SYM_MODULE] {
|
||||
if let Some(Res::Def(_, def_id)) = path_to_res(cx, 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_def_id);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -903,7 +930,7 @@ impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol {
|
||||
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);
|
||||
if let Some(&def_id) = self.symbol_map.get(&value);
|
||||
then {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
@ -911,10 +938,135 @@ impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol {
|
||||
is_expn_of(expr.span, "sym").unwrap_or(expr.span),
|
||||
"interning a defined symbol",
|
||||
"try",
|
||||
format!("rustc_span::symbol::sym::{}", symbol_const),
|
||||
cx.tcx.def_path_str(def_id),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
}
|
||||
if let ExprKind::Binary(op, left, right) = expr.kind {
|
||||
if matches!(op.node, BinOpKind::Eq | BinOpKind::Ne) {
|
||||
let data = [
|
||||
(left, self.symbol_str_expr(left, cx)),
|
||||
(right, self.symbol_str_expr(right, cx)),
|
||||
];
|
||||
match data {
|
||||
// both operands are a symbol string
|
||||
[(_, Some(left)), (_, Some(right))] => {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
UNNECESSARY_SYMBOL_STR,
|
||||
expr.span,
|
||||
"unnecessary `Symbol` to string conversion",
|
||||
"try",
|
||||
format!(
|
||||
"{} {} {}",
|
||||
left.as_symbol_snippet(cx),
|
||||
op.node.as_str(),
|
||||
right.as_symbol_snippet(cx),
|
||||
),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
},
|
||||
// one of the operands is a symbol string
|
||||
[(expr, Some(symbol)), _] | [_, (expr, Some(symbol))] => {
|
||||
// creating an owned string for comparison
|
||||
if matches!(symbol, SymbolStrExpr::Expr { is_to_owned: true, .. }) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
UNNECESSARY_SYMBOL_STR,
|
||||
expr.span,
|
||||
"unnecessary string allocation",
|
||||
"try",
|
||||
format!("{}.as_str()", symbol.as_symbol_snippet(cx)),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
},
|
||||
// nothing found
|
||||
[(_, None), (_, None)] => {},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl InterningDefinedSymbol {
|
||||
fn symbol_str_expr<'tcx>(&self, expr: &'tcx Expr<'tcx>, cx: &LateContext<'tcx>) -> Option<SymbolStrExpr<'tcx>> {
|
||||
static IDENT_STR_PATHS: &[&[&str]] = &[&paths::IDENT_AS_STR, &paths::TO_STRING_METHOD];
|
||||
static SYMBOL_STR_PATHS: &[&[&str]] = &[
|
||||
&paths::SYMBOL_AS_STR,
|
||||
&paths::SYMBOL_TO_IDENT_STRING,
|
||||
&paths::TO_STRING_METHOD,
|
||||
];
|
||||
// SymbolStr might be de-referenced: `&*symbol.as_str()`
|
||||
let call = if_chain! {
|
||||
if let ExprKind::AddrOf(_, _, e) = expr.kind;
|
||||
if let ExprKind::Unary(UnOp::UnDeref, e) = e.kind;
|
||||
then { e } else { expr }
|
||||
};
|
||||
if_chain! {
|
||||
// is a method call
|
||||
if let ExprKind::MethodCall(_, _, [item], _) = call.kind;
|
||||
if let Some(did) = cx.typeck_results().type_dependent_def_id(call.hir_id);
|
||||
let ty = cx.typeck_results().expr_ty(item);
|
||||
// ...on either an Ident or a Symbol
|
||||
if let Some(is_ident) = if match_type(cx, ty, &paths::SYMBOL) {
|
||||
Some(false)
|
||||
} else if match_type(cx, ty, &paths::IDENT) {
|
||||
Some(true)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
// ...which converts it to a string
|
||||
let paths = if is_ident { IDENT_STR_PATHS } else { SYMBOL_STR_PATHS };
|
||||
if let Some(path) = paths.iter().find(|path| match_def_path(cx, did, path));
|
||||
then {
|
||||
let is_to_owned = path.last().unwrap().ends_with("string");
|
||||
return Some(SymbolStrExpr::Expr {
|
||||
item,
|
||||
is_ident,
|
||||
is_to_owned,
|
||||
});
|
||||
}
|
||||
}
|
||||
// is a string constant
|
||||
if let Some(Constant::Str(s)) = constant_simple(cx, cx.typeck_results(), expr) {
|
||||
let value = Symbol::intern(&s).as_u32();
|
||||
// ...which matches a symbol constant
|
||||
if let Some(&def_id) = self.symbol_map.get(&value) {
|
||||
return Some(SymbolStrExpr::Const(def_id));
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
enum SymbolStrExpr<'tcx> {
|
||||
/// a string constant with a corresponding symbol constant
|
||||
Const(DefId),
|
||||
/// a "symbol to string" expression like `symbol.as_str()`
|
||||
Expr {
|
||||
/// part that evaluates to `Symbol` or `Ident`
|
||||
item: &'tcx Expr<'tcx>,
|
||||
is_ident: bool,
|
||||
/// whether an owned `String` is created like `to_ident_string()`
|
||||
is_to_owned: bool,
|
||||
},
|
||||
}
|
||||
|
||||
impl<'tcx> SymbolStrExpr<'tcx> {
|
||||
/// Returns a snippet that evaluates to a `Symbol` and is const if possible
|
||||
fn as_symbol_snippet(&self, cx: &LateContext<'_>) -> Cow<'tcx, str> {
|
||||
match *self {
|
||||
Self::Const(def_id) => cx.tcx.def_path_str(def_id).into(),
|
||||
Self::Expr { item, is_ident, .. } => {
|
||||
let mut snip = snippet(cx, item.span.source_callsite(), "..");
|
||||
if is_ident {
|
||||
// get `Ident.name`
|
||||
snip.to_mut().push_str(".name");
|
||||
}
|
||||
snip
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -54,6 +54,10 @@ pub const HASH: [&str; 3] = ["core", "hash", "Hash"];
|
||||
pub const HASHMAP: [&str; 5] = ["std", "collections", "hash", "map", "HashMap"];
|
||||
pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"];
|
||||
pub const HASHSET: [&str; 5] = ["std", "collections", "hash", "set", "HashSet"];
|
||||
#[cfg(feature = "internal-lints")]
|
||||
pub const IDENT: [&str; 3] = ["rustc_span", "symbol", "Ident"];
|
||||
#[cfg(feature = "internal-lints")]
|
||||
pub const IDENT_AS_STR: [&str; 4] = ["rustc_span", "symbol", "Ident", "as_str"];
|
||||
pub const INDEX: [&str; 3] = ["core", "ops", "Index"];
|
||||
pub const INDEX_MUT: [&str; 3] = ["core", "ops", "IndexMut"];
|
||||
pub const INSERT_STR: [&str; 4] = ["alloc", "string", "String", "insert_str"];
|
||||
@ -65,6 +69,8 @@ pub const IPADDR_V4: [&str; 4] = ["std", "net", "IpAddr", "V4"];
|
||||
pub const IPADDR_V6: [&str; 4] = ["std", "net", "IpAddr", "V6"];
|
||||
pub const ITERATOR: [&str; 5] = ["core", "iter", "traits", "iterator", "Iterator"];
|
||||
#[cfg(feature = "internal-lints")]
|
||||
pub const KW_MODULE: [&str; 3] = ["rustc_span", "symbol", "kw"];
|
||||
#[cfg(feature = "internal-lints")]
|
||||
pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"];
|
||||
pub const LINKED_LIST: [&str; 4] = ["alloc", "collections", "linked_list", "LinkedList"];
|
||||
#[cfg(feature = "internal-lints")]
|
||||
@ -148,8 +154,12 @@ pub const STR_STARTS_WITH: [&str; 4] = ["core", "str", "<impl str>", "starts_wit
|
||||
#[cfg(feature = "internal-lints")]
|
||||
pub const SYMBOL: [&str; 3] = ["rustc_span", "symbol", "Symbol"];
|
||||
#[cfg(feature = "internal-lints")]
|
||||
pub const SYMBOL_AS_STR: [&str; 4] = ["rustc_span", "symbol", "Symbol", "as_str"];
|
||||
#[cfg(feature = "internal-lints")]
|
||||
pub const SYMBOL_INTERN: [&str; 4] = ["rustc_span", "symbol", "Symbol", "intern"];
|
||||
#[cfg(feature = "internal-lints")]
|
||||
pub const SYMBOL_TO_IDENT_STRING: [&str; 4] = ["rustc_span", "symbol", "Symbol", "to_ident_string"];
|
||||
#[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"];
|
||||
|
@ -158,7 +158,7 @@ fn get_vec_init_kind<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Op
|
||||
ExprKind::Path(QPath::TypeRelative(ty, name))
|
||||
if is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::vec_type) =>
|
||||
{
|
||||
if name.ident.name.as_str() == "new" {
|
||||
if name.ident.name == sym::new {
|
||||
return Some(VecInitKind::New);
|
||||
} else if name.ident.name.as_str() == "with_capacity" {
|
||||
return args.get(0).and_then(|arg| {
|
||||
|
@ -7,7 +7,8 @@ use rustc_hir::{
|
||||
};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||
use rustc_span::BytePos;
|
||||
use rustc_span::symbol::kw;
|
||||
use rustc_span::{sym, BytePos};
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for `use Enum::*`.
|
||||
@ -198,12 +199,12 @@ impl WildcardImports {
|
||||
// Allow "...prelude::..::*" imports.
|
||||
// Many crates have a prelude, and it is imported as a glob by design.
|
||||
fn is_prelude_import(segments: &[PathSegment<'_>]) -> bool {
|
||||
segments.iter().any(|ps| ps.ident.as_str() == "prelude")
|
||||
segments.iter().any(|ps| ps.ident.name == sym::prelude)
|
||||
}
|
||||
|
||||
// Allow "super::*" imports in tests.
|
||||
fn is_super_only_import(segments: &[PathSegment<'_>]) -> bool {
|
||||
segments.len() == 1 && segments[0].ident.as_str() == "super"
|
||||
segments.len() == 1 && segments[0].ident.name == kw::Super
|
||||
}
|
||||
|
||||
fn is_test_module_or_function(item: &Item<'_>) -> bool {
|
||||
|
@ -10,7 +10,8 @@ use rustc_lexer::unescape::{self, EscapeError};
|
||||
use rustc_lint::{EarlyContext, EarlyLintPass};
|
||||
use rustc_parse::parser;
|
||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||
use rustc_span::{sym, BytePos, Span, Symbol};
|
||||
use rustc_span::symbol::kw;
|
||||
use rustc_span::{sym, BytePos, Span};
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** This lint warns when you use `println!("")` to
|
||||
@ -301,7 +302,7 @@ impl EarlyLintPass for Write {
|
||||
}
|
||||
} else if mac.path == sym!(writeln) {
|
||||
if let (Some(fmt_str), expr) = self.check_tts(cx, mac.args.inner_tokens(), true) {
|
||||
if fmt_str.symbol == Symbol::intern("") {
|
||||
if fmt_str.symbol == kw::Empty {
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
// FIXME: remove this `#[allow(...)]` once the issue #5822 gets fixed
|
||||
#[allow(clippy::option_if_let_else)]
|
||||
@ -484,7 +485,7 @@ impl Write {
|
||||
|
||||
fn lint_println_empty_string(&self, cx: &EarlyContext<'_>, mac: &MacCall) {
|
||||
if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) {
|
||||
if fmt_str.symbol == Symbol::intern("") {
|
||||
if fmt_str.symbol == kw::Empty {
|
||||
let name = mac.path.segments[0].ident.name;
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
|
@ -14,13 +14,16 @@ macro_rules! sym {
|
||||
|
||||
fn main() {
|
||||
// Direct use of Symbol::intern
|
||||
let _ = rustc_span::symbol::sym::f32;
|
||||
let _ = rustc_span::sym::f32;
|
||||
|
||||
// Using a sym macro
|
||||
let _ = rustc_span::symbol::sym::f32;
|
||||
let _ = rustc_span::sym::f32;
|
||||
|
||||
// Correct suggestion when symbol isn't stringified constant name
|
||||
let _ = rustc_span::symbol::sym::proc_dash_macro;
|
||||
let _ = rustc_span::sym::proc_dash_macro;
|
||||
|
||||
// interning a keyword
|
||||
let _ = rustc_span::symbol::kw::SelfLower;
|
||||
|
||||
// Interning a symbol that is not defined
|
||||
let _ = Symbol::intern("xyz123");
|
||||
|
@ -22,6 +22,9 @@ fn main() {
|
||||
// Correct suggestion when symbol isn't stringified constant name
|
||||
let _ = Symbol::intern("proc-macro");
|
||||
|
||||
// interning a keyword
|
||||
let _ = Symbol::intern("self");
|
||||
|
||||
// Interning a symbol that is not defined
|
||||
let _ = Symbol::intern("xyz123");
|
||||
let _ = sym!(xyz123);
|
||||
|
@ -2,7 +2,7 @@ error: interning a defined symbol
|
||||
--> $DIR/interning_defined_symbol.rs:17:13
|
||||
|
|
||||
LL | let _ = Symbol::intern("f32");
|
||||
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::symbol::sym::f32`
|
||||
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::sym::f32`
|
||||
|
|
||||
note: the lint level is defined here
|
||||
--> $DIR/interning_defined_symbol.rs:2:9
|
||||
@ -15,13 +15,19 @@ error: interning a defined symbol
|
||||
--> $DIR/interning_defined_symbol.rs:20:13
|
||||
|
|
||||
LL | let _ = sym!(f32);
|
||||
| ^^^^^^^^^ help: try: `rustc_span::symbol::sym::f32`
|
||||
| ^^^^^^^^^ help: try: `rustc_span::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`
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::sym::proc_dash_macro`
|
||||
|
||||
error: aborting due to 3 previous errors
|
||||
error: interning a defined symbol
|
||||
--> $DIR/interning_defined_symbol.rs:26:13
|
||||
|
|
||||
LL | let _ = Symbol::intern("self");
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::symbol::kw::SelfLower`
|
||||
|
||||
error: aborting due to 4 previous errors
|
||||
|
||||
|
16
tests/ui-internal/unnecessary_symbol_str.fixed
Normal file
16
tests/ui-internal/unnecessary_symbol_str.fixed
Normal file
@ -0,0 +1,16 @@
|
||||
// run-rustfix
|
||||
#![feature(rustc_private)]
|
||||
#![deny(clippy::internal)]
|
||||
#![allow(clippy::unnecessary_operation, unused_must_use)]
|
||||
|
||||
extern crate rustc_span;
|
||||
|
||||
use rustc_span::symbol::{Ident, Symbol};
|
||||
|
||||
fn main() {
|
||||
Symbol::intern("foo") == rustc_span::sym::clippy;
|
||||
Symbol::intern("foo") == rustc_span::symbol::kw::SelfLower;
|
||||
Symbol::intern("foo") != rustc_span::symbol::kw::SelfUpper;
|
||||
Ident::invalid().name == rustc_span::sym::clippy;
|
||||
rustc_span::sym::clippy == Ident::invalid().name;
|
||||
}
|
16
tests/ui-internal/unnecessary_symbol_str.rs
Normal file
16
tests/ui-internal/unnecessary_symbol_str.rs
Normal file
@ -0,0 +1,16 @@
|
||||
// run-rustfix
|
||||
#![feature(rustc_private)]
|
||||
#![deny(clippy::internal)]
|
||||
#![allow(clippy::unnecessary_operation, unused_must_use)]
|
||||
|
||||
extern crate rustc_span;
|
||||
|
||||
use rustc_span::symbol::{Ident, Symbol};
|
||||
|
||||
fn main() {
|
||||
Symbol::intern("foo").as_str() == "clippy";
|
||||
Symbol::intern("foo").to_string() == "self";
|
||||
Symbol::intern("foo").to_ident_string() != "Self";
|
||||
&*Ident::invalid().as_str() == "clippy";
|
||||
"clippy" == Ident::invalid().to_string();
|
||||
}
|
39
tests/ui-internal/unnecessary_symbol_str.stderr
Normal file
39
tests/ui-internal/unnecessary_symbol_str.stderr
Normal file
@ -0,0 +1,39 @@
|
||||
error: unnecessary `Symbol` to string conversion
|
||||
--> $DIR/unnecessary_symbol_str.rs:11:5
|
||||
|
|
||||
LL | Symbol::intern("foo").as_str() == "clippy";
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Symbol::intern("foo") == rustc_span::sym::clippy`
|
||||
|
|
||||
note: the lint level is defined here
|
||||
--> $DIR/unnecessary_symbol_str.rs:3:9
|
||||
|
|
||||
LL | #![deny(clippy::internal)]
|
||||
| ^^^^^^^^^^^^^^^^
|
||||
= note: `#[deny(clippy::unnecessary_symbol_str)]` implied by `#[deny(clippy::internal)]`
|
||||
|
||||
error: unnecessary `Symbol` to string conversion
|
||||
--> $DIR/unnecessary_symbol_str.rs:12:5
|
||||
|
|
||||
LL | Symbol::intern("foo").to_string() == "self";
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Symbol::intern("foo") == rustc_span::symbol::kw::SelfLower`
|
||||
|
||||
error: unnecessary `Symbol` to string conversion
|
||||
--> $DIR/unnecessary_symbol_str.rs:13:5
|
||||
|
|
||||
LL | Symbol::intern("foo").to_ident_string() != "Self";
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Symbol::intern("foo") != rustc_span::symbol::kw::SelfUpper`
|
||||
|
||||
error: unnecessary `Symbol` to string conversion
|
||||
--> $DIR/unnecessary_symbol_str.rs:14:5
|
||||
|
|
||||
LL | &*Ident::invalid().as_str() == "clippy";
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Ident::invalid().name == rustc_span::sym::clippy`
|
||||
|
||||
error: unnecessary `Symbol` to string conversion
|
||||
--> $DIR/unnecessary_symbol_str.rs:15:5
|
||||
|
|
||||
LL | "clippy" == Ident::invalid().to_string();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::sym::clippy == Ident::invalid().name`
|
||||
|
||||
error: aborting due to 5 previous errors
|
||||
|
Loading…
Reference in New Issue
Block a user