Auto merge of rust-lang#6569 - camsteffen:symbol-comparison, r=Manish…

…earth 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`.
flip1995 · Jan 8, 2021 · 68bcd20 · 68bcd20
2 parents 2950c8e + 7871eba
commit 68bcd20
Show file tree

Hide file tree

Showing 28 changed files with 309 additions and 58 deletions.
diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs
@@ -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());

diff --git a/clippy_lints/src/if_let_mutex.rs b/clippy_lints/src/if_let_mutex.rs
@@ -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 {

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -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![

diff --git a/clippy_lints/src/manual_async_fn.rs b/clippy_lints/src/manual_async_fn.rs
@@ -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)

diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs
@@ -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;

diff --git a/clippy_lints/src/map_identity.rs b/clippy_lints/src/map_identity.rs
@@ -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)

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
@@ -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`");

diff --git a/clippy_lints/src/minmax.rs b/clippy_lints/src/minmax.rs
@@ -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

diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs
@@ -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
             }

diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs
@@ -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;
                     }
                 }

diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs
@@ -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

diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs
@@ -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
 }
diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs
@@ -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 {

diff --git a/clippy_lints/src/unnecessary_sort_by.rs b/clippy_lints/src/unnecessary_sort_by.rs
@@ -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,

diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs
@@ -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;
                             }
                         }

diff --git a/clippy_lints/src/utils/attrs.rs b/clippy_lints/src/utils/attrs.rs
@@ -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
                             },
                         }
                     },

diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs
@@ -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 {