Skip to content

Commit 9c3c938

Browse files
oli-obkoli-obk
authored
Merge pull request #1613 from Manishearth/dont_ref_operator_args
Dont ref operator args
2 parents 813daf2 + 0ae1a77 commit 9c3c938

25 files changed

+136
-68
lines changed

‎clippy_lints/src/approx_const.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ fn check_lit(cx: &LateContext, lit: &Lit, e: &Expr) {
7777
}
7878

7979
fn check_known_consts(cx: &LateContext, e: &Expr, s: &symbol::Symbol, module: &str) {
80-
let s = &*s.as_str();
80+
let s = s.as_str();
8181
if s.parse::<f64>().is_ok() {
8282
for &(constant, name, min_digits) in KNOWN_CONSTS {
83-
if is_approx_const(constant, s, min_digits) {
83+
if is_approx_const(constant, &s, min_digits) {
8484
span_lint(cx,
8585
APPROX_CONSTANT,
8686
e.span,

‎clippy_lints/src/attrs.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ fn check_attrs(cx: &LateContext, span: Span, name: &Name, attrs: &[Attribute]) {
237237

238238
fn check_semver(cx: &LateContext, span: Span, lit: &Lit) {
239239
if let LitKind::Str(ref is, _) = lit.node {
240-
if Version::parse(&*is.as_str()).is_ok() {
240+
if Version::parse(&is.as_str()).is_ok() {
241241
return;
242242
}
243243
}

‎clippy_lints/src/blacklisted_name.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ impl LintPass for BlackListedName {
4040
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlackListedName {
4141
fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat) {
4242
if let PatKind::Binding(_, _, ref ident, _) = pat.node {
43-
if self.blacklist.iter().any(|s| s == &*ident.node.as_str()) {
43+
if self.blacklist.iter().any(|s| ident.node == *s) {
4444
span_lint(cx,
4545
BLACKLISTED_NAME,
4646
pat.span,

‎clippy_lints/src/doc.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ pub fn check_attrs<'a>(cx: &EarlyContext, valid_idents: &[String], attrs: &'a [a
9090
for attr in attrs {
9191
if attr.is_sugared_doc {
9292
if let Some(ref doc) = attr.value_str() {
93-
let doc = (*doc.as_str()).to_owned();
93+
let doc = doc.to_string();
9494
docs.extend_from_slice(&strip_doc_comment_decoration((doc, attr.span)));
9595
}
9696
}

‎clippy_lints/src/entry.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ fn check_cond<'a, 'tcx, 'b>(
8585
if_let_chain! {[
8686
let ExprMethodCall(ref name, _, ref params) = check.node,
8787
params.len() >= 2,
88-
&*name.node.as_str() == "contains_key",
88+
name.node == "contains_key",
8989
let ExprAddrOf(_, ref key) = params[1].node
9090
], {
9191
let map = &params[0];
@@ -119,7 +119,7 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> {
119119
if_let_chain! {[
120120
let ExprMethodCall(ref name, _, ref params) = expr.node,
121121
params.len() == 3,
122-
&*name.node.as_str() == "insert",
122+
name.node == "insert",
123123
get_item_name(self.cx, self.map) == get_item_name(self.cx, &params[0]),
124124
SpanlessEq::new(self.cx).eq_expr(self.key, &params[1])
125125
], {

‎clippy_lints/src/enum_variants.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ impl EarlyLintPass for EnumVariantNames {
232232
if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() {
233233
// constants don't have surrounding modules
234234
if !mod_camel.is_empty() {
235-
if mod_name == &item_name {
235+
if *mod_name == item_name {
236236
if let ItemKind::Mod(..) = item.node {
237237
span_lint(cx,
238238
MODULE_INCEPTION,

‎clippy_lints/src/eq_op.rs

+76-7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use rustc::hir::*;
22
use rustc::lint::*;
3-
use utils::{SpanlessEq, span_lint};
3+
use utils::{SpanlessEq, span_lint, span_lint_and_then, multispan_sugg, snippet};
4+
use utils::sugg::Sugg;
45

56
/// **What it does:** Checks for equal operands to comparison, logical and
67
/// bitwise, difference and division binary operators (`==`, `>`, etc., `&&`,
@@ -23,23 +24,91 @@ declare_lint! {
2324
"equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`)"
2425
}
2526

27+
/// **What it does:** Checks for arguments to `==` which have their address taken to satisfy a bound
28+
/// and suggests to dereference the other argument instead
29+
///
30+
/// **Why is this bad?** It is more idiomatic to dereference the other argument.
31+
///
32+
/// **Known problems:** None
33+
///
34+
/// **Example:**
35+
/// ```rust
36+
/// &x == y
37+
/// ```
38+
declare_lint! {
39+
pub OP_REF,
40+
Warn,
41+
"taking a reference to satisfy the type constraints on `==`"
42+
}
43+
2644
#[derive(Copy,Clone)]
2745
pub struct EqOp;
2846

2947
impl LintPass for EqOp {
3048
fn get_lints(&self) -> LintArray {
31-
lint_array!(EQ_OP)
49+
lint_array!(EQ_OP, OP_REF)
3250
}
3351
}
3452

3553
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp {
3654
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
3755
if let ExprBinary(ref op, ref left, ref right) = e.node {
38-
if is_valid_operator(op) && SpanlessEq::new(cx).ignore_fn().eq_expr(left, right) {
39-
span_lint(cx,
40-
EQ_OP,
41-
e.span,
42-
&format!("equal expressions as operands to `{}`", op.node.as_str()));
56+
if is_valid_operator(op) {
57+
if SpanlessEq::new(cx).ignore_fn().eq_expr(left, right) {
58+
span_lint(cx,
59+
EQ_OP,
60+
e.span,
61+
&format!("equal expressions as operands to `{}`", op.node.as_str()));
62+
} else {
63+
match (&left.node, &right.node) {
64+
(&ExprAddrOf(_, ref l), &ExprAddrOf(_, ref r)) => {
65+
span_lint_and_then(cx,
66+
OP_REF,
67+
e.span,
68+
"taken reference of both operands, which is done automatically by the operator anyway",
69+
|db| {
70+
let lsnip = snippet(cx, l.span, "...").to_string();
71+
let rsnip = snippet(cx, r.span, "...").to_string();
72+
multispan_sugg(db,
73+
"use the values directly".to_string(),
74+
vec![(left.span, lsnip),
75+
(right.span, rsnip)]);
76+
}
77+
)
78+
}
79+
(&ExprAddrOf(_, ref l), _) => {
80+
span_lint_and_then(cx,
81+
OP_REF,
82+
e.span,
83+
"taken reference of left operand",
84+
|db| {
85+
let lsnip = snippet(cx, l.span, "...").to_string();
86+
let rsnip = Sugg::hir(cx, right, "...").deref().to_string();
87+
multispan_sugg(db,
88+
"dereference the right operand instead".to_string(),
89+
vec![(left.span, lsnip),
90+
(right.span, rsnip)]);
91+
}
92+
)
93+
}
94+
(_, &ExprAddrOf(_, ref r)) => {
95+
span_lint_and_then(cx,
96+
OP_REF,
97+
e.span,
98+
"taken reference of right operand",
99+
|db| {
100+
let lsnip = Sugg::hir(cx, left, "...").deref().to_string();
101+
let rsnip = snippet(cx, r.span, "...").to_string();
102+
multispan_sugg(db,
103+
"dereference the left operand instead".to_string(),
104+
vec![(left.span, lsnip),
105+
(right.span, rsnip)]);
106+
}
107+
)
108+
}
109+
_ => {}
110+
}
111+
}
43112
}
44113
}
45114
}

‎clippy_lints/src/format.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ pub fn get_argument_fmtstr_parts<'a, 'b>(cx: &LateContext<'a, 'b>, expr: &'a Exp
7979
let StmtDecl(ref decl, _) = block.stmts[0].node,
8080
let DeclItem(ref decl) = decl.node,
8181
let Some(NodeItem(decl)) = cx.tcx.hir.find(decl.id),
82-
&*decl.name.as_str() == "__STATIC_FMTSTR",
82+
decl.name == "__STATIC_FMTSTR",
8383
let ItemStatic(_, _, ref expr) = decl.node,
8484
let ExprAddrOf(_, ref expr) = cx.tcx.hir.body(*expr).value.node, // &["…", "…", …]
8585
let ExprArray(ref exprs) = expr.node,

‎clippy_lints/src/len_zero.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LenZero {
8989

9090
fn check_trait_items(cx: &LateContext, item: &Item, trait_items: &[TraitItemRef]) {
9191
fn is_named_self(cx: &LateContext, item: &TraitItemRef, name: &str) -> bool {
92-
&*item.name.as_str() == name &&
92+
item.name == name &&
9393
if let AssociatedItemKind::Method { has_self } = item.kind {
9494
has_self &&
9595
{
@@ -116,7 +116,7 @@ fn check_trait_items(cx: &LateContext, item: &Item, trait_items: &[TraitItemRef]
116116

117117
fn check_impl_items(cx: &LateContext, item: &Item, impl_items: &[ImplItemRef]) {
118118
fn is_named_self(cx: &LateContext, item: &ImplItemRef, name: &str) -> bool {
119-
&*item.name.as_str() == name &&
119+
item.name == name &&
120120
if let AssociatedItemKind::Method { has_self } = item.kind {
121121
has_self &&
122122
{
@@ -155,22 +155,22 @@ fn check_impl_items(cx: &LateContext, item: &Item, impl_items: &[ImplItemRef]) {
155155
fn check_cmp(cx: &LateContext, span: Span, left: &Expr, right: &Expr, op: &str) {
156156
// check if we are in an is_empty() method
157157
if let Some(name) = get_item_name(cx, left) {
158-
if &*name.as_str() == "is_empty" {
158+
if name == "is_empty" {
159159
return;
160160
}
161161
}
162162
match (&left.node, &right.node) {
163163
(&ExprLit(ref lit), &ExprMethodCall(ref method, _, ref args)) |
164164
(&ExprMethodCall(ref method, _, ref args), &ExprLit(ref lit)) => {
165-
check_len_zero(cx, span, &method.node, args, lit, op)
165+
check_len_zero(cx, span, method.node, args, lit, op)
166166
},
167167
_ => (),
168168
}
169169
}
170170

171-
fn check_len_zero(cx: &LateContext, span: Span, name: &Name, args: &[Expr], lit: &Lit, op: &str) {
171+
fn check_len_zero(cx: &LateContext, span: Span, name: Name, args: &[Expr], lit: &Lit, op: &str) {
172172
if let Spanned { node: LitKind::Int(0, _), .. } = *lit {
173-
if &*name.as_str() == "len" && args.len() == 1 && has_is_empty(cx, &args[0]) {
173+
if name == "len" && args.len() == 1 && has_is_empty(cx, &args[0]) {
174174
span_lint_and_then(cx, LEN_ZERO, span, "length comparison to zero", |db| {
175175
db.span_suggestion(span,
176176
"consider using `is_empty`",
@@ -185,7 +185,7 @@ fn has_is_empty(cx: &LateContext, expr: &Expr) -> bool {
185185
/// Get an `AssociatedItem` and return true if it matches `is_empty(self)`.
186186
fn is_is_empty(cx: &LateContext, item: &ty::AssociatedItem) -> bool {
187187
if let ty::AssociatedKind::Method = item.kind {
188-
if &*item.name.as_str() == "is_empty" {
188+
if item.name == "is_empty" {
189189
let sig = cx.tcx.item_type(item.def_id).fn_sig();
190190
let ty = sig.skip_binder();
191191
ty.inputs().len() == 1

‎clippy_lints/src/lifetimes.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ fn allowed_lts_from(named_lts: &[LifetimeDef]) -> HashSet<RefLt> {
199199

200200
fn lts_from_bounds<'a, T: Iterator<Item = &'a Lifetime>>(mut vec: Vec<RefLt>, bounds_lts: T) -> Vec<RefLt> {
201201
for lt in bounds_lts {
202-
if &*lt.name.as_str() != "'static" {
202+
if lt.name != "'static" {
203203
vec.push(RefLt::Named(lt.name));
204204
}
205205
}
@@ -228,7 +228,7 @@ impl<'v, 't> RefVisitor<'v, 't> {
228228

229229
fn record(&mut self, lifetime: &Option<Lifetime>) {
230230
if let Some(ref lt) = *lifetime {
231-
if &*lt.name.as_str() == "'static" {
231+
if lt.name == "'static" {
232232
self.lts.push(RefLt::Static);
233233
} else if lt.is_elided() {
234234
self.lts.push(RefLt::Unnamed);

‎clippy_lints/src/loops.rs

+8-8
Original file line numberDiff line numberDiff line change
@@ -389,8 +389,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
389389
&ExprMethodCall(method_name, _, ref method_args)) = (pat, &match_expr.node) {
390390
let iter_expr = &method_args[0];
391391
let lhs_constructor = last_path_segment(qpath);
392-
if &*method_name.node.as_str() == "next" && match_trait_method(cx, match_expr, &paths::ITERATOR) &&
393-
&*lhs_constructor.name.as_str() == "Some" && !is_refutable(cx, &pat_args[0]) &&
392+
if method_name.node == "next" && match_trait_method(cx, match_expr, &paths::ITERATOR) &&
393+
lhs_constructor.name == "Some" && !is_refutable(cx, &pat_args[0]) &&
394394
!is_iterator_used_after_while_let(cx, iter_expr) {
395395
let iterator = snippet(cx, method_args[0].span, "_");
396396
let loop_var = snippet(cx, pat_args[0].span, "_");
@@ -409,7 +409,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
409409
fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) {
410410
if let StmtSemi(ref expr, _) = stmt.node {
411411
if let ExprMethodCall(ref method, _, ref args) = expr.node {
412-
if args.len() == 1 && &*method.node.as_str() == "collect" &&
412+
if args.len() == 1 && method.node == "collect" &&
413413
match_trait_method(cx, expr, &paths::ITERATOR) {
414414
span_lint(cx,
415415
UNUSED_COLLECT,
@@ -579,10 +579,10 @@ fn is_len_call(expr: &Expr, var: &Name) -> bool {
579579
if_let_chain! {[
580580
let ExprMethodCall(method, _, ref len_args) = expr.node,
581581
len_args.len() == 1,
582-
&*method.node.as_str() == "len",
582+
method.node == "len",
583583
let ExprPath(QPath::Resolved(_, ref path)) = len_args[0].node,
584584
path.segments.len() == 1,
585-
&path.segments[0].name == var
585+
path.segments[0].name == *var
586586
], {
587587
return true;
588588
}}
@@ -664,11 +664,11 @@ fn check_for_loop_arg(cx: &LateContext, pat: &Pat, arg: &Expr, expr: &Expr) {
664664
if let ExprMethodCall(ref method, _, ref args) = arg.node {
665665
// just the receiver, no arguments
666666
if args.len() == 1 {
667-
let method_name = &*method.node.as_str();
667+
let method_name = method.node.as_str();
668668
// check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
669669
if method_name == "iter" || method_name == "iter_mut" {
670670
if is_ref_iterable_type(cx, &args[0]) {
671-
lint_iter_method(cx, args, arg, method_name);
671+
lint_iter_method(cx, args, arg, &method_name);
672672
}
673673
} else if method_name == "into_iter" && match_trait_method(cx, arg, &paths::INTO_ITERATOR) {
674674
let method_call = ty::MethodCall::expr(arg.id);
@@ -680,7 +680,7 @@ fn check_for_loop_arg(cx: &LateContext, pat: &Pat, arg: &Expr, expr: &Expr) {
680680
let fn_arg_tys = fn_ty.fn_args();
681681
assert_eq!(fn_arg_tys.skip_binder().len(), 1);
682682
if fn_arg_tys.skip_binder()[0].is_region_ptr() {
683-
lint_iter_method(cx, args, arg, method_name);
683+
lint_iter_method(cx, args, arg, &method_name);
684684
} else {
685685
let object = snippet(cx, args[0].span, "_");
686686
span_lint_and_sugg(cx,

‎clippy_lints/src/map_clone.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
2828
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
2929
// call to .map()
3030
if let ExprMethodCall(name, _, ref args) = expr.node {
31-
if &*name.node.as_str() == "map" && args.len() == 2 {
31+
if name.node == "map" && args.len() == 2 {
3232
match args[1].node {
3333
ExprClosure(_, ref decl, closure_eid, _) => {
3434
let body = cx.tcx.hir.body(closure_eid);
@@ -53,7 +53,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
5353
}
5454
// explicit clone() calls ( .map(|x| x.clone()) )
5555
else if let ExprMethodCall(clone_call, _, ref clone_args) = closure_expr.node {
56-
if &*clone_call.node.as_str() == "clone" &&
56+
if clone_call.node == "clone" &&
5757
clone_args.len() == 1 &&
5858
match_trait_method(cx, closure_expr, &paths::CLONE_TRAIT) &&
5959
expr_eq_name(&clone_args[0], arg_ident)

‎clippy_lints/src/matches.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ fn check_single_match_opt_like(
257257
};
258258

259259
for &(ty_path, pat_path) in candidates {
260-
if &path == pat_path && match_type(cx, ty, ty_path) {
260+
if path == *pat_path && match_type(cx, ty, ty_path) {
261261
report_single_match_single_pattern(cx, ex, arms, expr, els);
262262
}
263263
}

‎clippy_lints/src/methods.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -607,14 +607,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
607607
lint_or_fun_call(cx, expr, &name.node.as_str(), args);
608608

609609
let self_ty = cx.tables.expr_ty_adjusted(&args[0]);
610-
if args.len() == 1 && &*name.node.as_str() == "clone" {
610+
if args.len() == 1 && name.node == "clone" {
611611
lint_clone_on_copy(cx, expr, &args[0], self_ty);
612612
}
613613

614614
match self_ty.sty {
615615
ty::TyRef(_, ty) if ty.ty.sty == ty::TyStr => {
616616
for &(method, pos) in &PATTERN_METHODS {
617-
if &*name.node.as_str() == method && args.len() > pos {
617+
if name.node == method && args.len() > pos {
618618
lint_single_char_pattern(cx, expr, &args[pos]);
619619
}
620620
}
@@ -646,7 +646,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
646646
], {
647647
// check missing trait implementations
648648
for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS {
649-
if &*name.as_str() == method_name &&
649+
if name == method_name &&
650650
sig.decl.inputs.len() == n_args &&
651651
out_type.matches(&sig.decl.output) &&
652652
self_kind.matches(&first_arg_ty, &first_arg, &self_ty, false) {
@@ -683,7 +683,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
683683
}
684684

685685
let ret_ty = return_ty(cx, implitem.id);
686-
if &*name.as_str() == "new" &&
686+
if name == "new" &&
687687
!ret_ty.walk().any(|t| same_tys(cx, t, ty, implitem.id)) {
688688
span_lint(cx,
689689
NEW_RET_NO_SELF,
@@ -712,7 +712,7 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir:
712712

713713
if name == "unwrap_or" {
714714
if let hir::ExprPath(ref qpath) = fun.node {
715-
let path: &str = &*last_path_segment(qpath).name.as_str();
715+
let path = &*last_path_segment(qpath).name.as_str();
716716

717717
if ["default", "new"].contains(&path) {
718718
let arg_ty = cx.tables.expr_ty(arg);
@@ -991,7 +991,7 @@ fn derefs_to_slice(cx: &LateContext, expr: &hir::Expr, ty: ty::Ty) -> Option<sug
991991
}
992992

993993
if let hir::ExprMethodCall(name, _, ref args) = expr.node {
994-
if &*name.node.as_str() == "iter" && may_slice(cx, cx.tables.expr_ty(&args[0])) {
994+
if name.node == "iter" && may_slice(cx, cx.tables.expr_ty(&args[0])) {
995995
sugg::Sugg::hir_opt(cx, &args[0]).map(|sugg| sugg.addr())
996996
} else {
997997
None
@@ -1209,7 +1209,7 @@ fn lint_chars_next(cx: &LateContext, expr: &hir::Expr, chain: &hir::Expr, other:
12091209
arg_char.len() == 1,
12101210
let hir::ExprPath(ref qpath) = fun.node,
12111211
let Some(segment) = single_segment_path(qpath),
1212-
&*segment.name.as_str() == "Some"
1212+
segment.name == "Some"
12131213
], {
12141214
let self_ty = walk_ptrs_ty(cx.tables.expr_ty_adjusted(&args[0][0]));
12151215

0 commit comments

Comments
 (0)