Skip to content

Commit 1e9f076

Browse files
committed
Ignore spans when comparing expressions
1 parent 06d6710 commit 1e9f076

13 files changed

+86
-53
lines changed

clippy_lints/src/enum_glob_use.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EnumGlobUse {
4444

4545
impl EnumGlobUse {
4646
fn lint_item(&self, cx: &LateContext, item: &Item) {
47-
if item.vis.node == VisibilityKind::Public {
47+
if item.vis.node.is_pub() {
4848
return; // re-exports are fine
4949
}
5050
if let ItemUse(ref path, UseKind::Glob) = item.node {

clippy_lints/src/enum_variants.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ impl EarlyLintPass for EnumVariantNames {
262262
);
263263
}
264264
}
265-
if item.vis.node == VisibilityKind::Public {
265+
if item.vis.node.is_pub() {
266266
let matching = partial_match(mod_camel, &item_camel);
267267
let rmatching = partial_rmatch(mod_camel, &item_camel);
268268
let nchars = mod_camel.chars().count();

clippy_lints/src/if_let_redundant_pattern_matching.rs

+11-7
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
4848
if let ExprMatch(ref op, ref arms, MatchSource::IfLetDesugar { .. }) = expr.node {
4949
if arms[0].pats.len() == 1 {
5050
let good_method = match arms[0].pats[0].node {
51-
PatKind::TupleStruct(ref path, ref pats, _) if pats.len() == 1 && pats[0].node == PatKind::Wild => {
52-
if match_qpath(path, &paths::RESULT_OK) {
53-
"is_ok()"
54-
} else if match_qpath(path, &paths::RESULT_ERR) {
55-
"is_err()"
56-
} else if match_qpath(path, &paths::OPTION_SOME) {
57-
"is_some()"
51+
PatKind::TupleStruct(ref path, ref pats, _) if pats.len() == 1 => {
52+
if let PatKind::Wild = pats[0].node {
53+
if match_qpath(path, &paths::RESULT_OK) {
54+
"is_ok()"
55+
} else if match_qpath(path, &paths::RESULT_ERR) {
56+
"is_err()"
57+
} else if match_qpath(path, &paths::OPTION_SOME) {
58+
"is_some()"
59+
} else {
60+
return;
61+
}
5862
} else {
5963
return;
6064
}

clippy_lints/src/loops.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::consts::{constant, Constant};
2323

2424
use crate::utils::{get_enclosing_block, get_parent_expr, higher, in_external_macro, is_integer_literal, is_refutable,
2525
last_path_segment, match_trait_method, match_type, match_var, multispan_sugg, snippet, snippet_opt,
26-
span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then};
26+
span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, SpanlessEq};
2727
use crate::utils::paths;
2828

2929
/// **What it does:** Checks for for-loops that manually copy items between
@@ -1955,7 +1955,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
19551955
if self.state == VarState::DontWarn {
19561956
return;
19571957
}
1958-
if expr == self.end_expr {
1958+
if SpanlessEq::new(self.cx).eq_expr(&expr, self.end_expr) {
19591959
self.past_loop = true;
19601960
return;
19611961
}

clippy_lints/src/map_clone.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use rustc::hir::*;
33
use rustc::ty;
44
use syntax::ast;
55
use crate::utils::{get_arg_ident, is_adjusted, iter_input_pats, match_qpath, match_trait_method, match_type,
6-
paths, remove_blocks, snippet, span_help_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth};
6+
paths, remove_blocks, snippet, span_help_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq};
77

88
/// **What it does:** Checks for mapping `clone()` over an iterator.
99
///
@@ -66,7 +66,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
6666
if clone_call.ident.name == "clone" &&
6767
clone_args.len() == 1 &&
6868
match_trait_method(cx, closure_expr, &paths::CLONE_TRAIT) &&
69-
expr_eq_name(&clone_args[0], arg_ident)
69+
expr_eq_name(cx, &clone_args[0], arg_ident)
7070
{
7171
span_help_and_lint(cx, MAP_CLONE, expr.span, &format!(
7272
"you seem to be using .map() to clone the contents of an {}, consider \
@@ -98,7 +98,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
9898
}
9999
}
100100

101-
fn expr_eq_name(expr: &Expr, id: ast::Ident) -> bool {
101+
fn expr_eq_name(cx: &LateContext, expr: &Expr, id: ast::Ident) -> bool {
102102
match expr.node {
103103
ExprPath(QPath::Resolved(None, ref path)) => {
104104
let arg_segment = [
@@ -108,7 +108,7 @@ fn expr_eq_name(expr: &Expr, id: ast::Ident) -> bool {
108108
infer_types: true,
109109
},
110110
];
111-
!path.is_global() && path.segments[..] == arg_segment
111+
!path.is_global() && SpanlessEq::new(cx).eq_path_segments(&path.segments[..], &arg_segment)
112112
},
113113
_ => false,
114114
}
@@ -127,7 +127,7 @@ fn get_type_name(cx: &LateContext, expr: &Expr, arg: &Expr) -> Option<&'static s
127127
fn only_derefs(cx: &LateContext, expr: &Expr, id: ast::Ident) -> bool {
128128
match expr.node {
129129
ExprUnary(UnDeref, ref subexpr) if !is_adjusted(cx, subexpr) => only_derefs(cx, subexpr, id),
130-
_ => expr_eq_name(expr, id),
130+
_ => expr_eq_name(cx, expr, id),
131131
}
132132
}
133133

clippy_lints/src/matches.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ fn check_single_match(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
221221
}
222222

223223
fn check_single_match_single_pattern(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr, els: Option<&Expr>) {
224-
if arms[1].pats[0].node == PatKind::Wild {
224+
if is_wild(&arms[1].pats[0]) {
225225
report_single_match_single_pattern(cx, ex, arms, expr, els);
226226
}
227227
}
@@ -265,7 +265,7 @@ fn check_single_match_opt_like(cx: &LateContext, ex: &Expr, arms: &[Arm], expr:
265265
let path = match arms[1].pats[0].node {
266266
PatKind::TupleStruct(ref path, ref inner, _) => {
267267
// contains any non wildcard patterns? e.g. Err(err)
268-
if inner.iter().any(|pat| pat.node != PatKind::Wild) {
268+
if !inner.iter().all(is_wild) {
269269
return;
270270
}
271271
print::to_string(print::NO_ANN, |s| s.print_qpath(path, false))
@@ -356,6 +356,13 @@ fn check_overlapping_arms<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ex: &'tcx Expr,
356356
}
357357
}
358358

359+
fn is_wild(pat: &impl std::ops::Deref<Target = Pat>) -> bool {
360+
match pat.node {
361+
PatKind::Wild => true,
362+
_ => false,
363+
}
364+
}
365+
359366
fn check_wild_err_arm(cx: &LateContext, ex: &Expr, arms: &[Arm]) {
360367
let ex_ty = walk_ptrs_ty(cx.tables.expr_ty(ex));
361368
if match_type(cx, ex_ty, &paths::RESULT) {
@@ -364,7 +371,7 @@ fn check_wild_err_arm(cx: &LateContext, ex: &Expr, arms: &[Arm]) {
364371
let path_str = print::to_string(print::NO_ANN, |s| s.print_qpath(path, false));
365372
if_chain! {
366373
if path_str == "Err";
367-
if inner.iter().any(|pat| pat.node == PatKind::Wild);
374+
if inner.iter().any(is_wild);
368375
if let ExprBlock(ref block, _) = arm.body.node;
369376
if is_panic_block(block);
370377
then {

clippy_lints/src/methods.rs

+11-9
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use syntax::codemap::{Span, BytePos};
1010
use crate::utils::{get_arg_name, get_trait_def_id, implements_trait, in_external_macro, in_macro, is_copy, is_expn_of, is_self,
1111
is_self_ty, iter_input_pats, last_path_segment, match_def_path, match_path, match_qpath, match_trait_method,
1212
match_type, method_chain_args, match_var, return_ty, remove_blocks, same_tys, single_segment_path, snippet,
13-
span_lint, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth};
13+
span_lint, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq};
1414
use crate::utils::paths;
1515
use crate::utils::sugg;
1616
use crate::consts::{constant, Constant};
@@ -820,8 +820,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
820820
for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS {
821821
if name == method_name &&
822822
sig.decl.inputs.len() == n_args &&
823-
out_type.matches(&sig.decl.output) &&
824-
self_kind.matches(first_arg_ty, first_arg, self_ty, false, &implitem.generics) {
823+
out_type.matches(cx, &sig.decl.output) &&
824+
self_kind.matches(cx, first_arg_ty, first_arg, self_ty, false, &implitem.generics) {
825825
span_lint(cx, SHOULD_IMPLEMENT_TRAIT, implitem.span, &format!(
826826
"defining a method called `{}` on this type; consider implementing \
827827
the `{}` trait or choosing a less ambiguous name", name, trait_name));
@@ -838,9 +838,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
838838
if conv.check(&name.as_str());
839839
if !self_kinds
840840
.iter()
841-
.any(|k| k.matches(first_arg_ty, first_arg, self_ty, is_copy, &implitem.generics));
841+
.any(|k| k.matches(cx, first_arg_ty, first_arg, self_ty, is_copy, &implitem.generics));
842842
then {
843-
let lint = if item.vis.node == hir::VisibilityKind::Public {
843+
let lint = if item.vis.node.is_pub() {
844844
WRONG_PUB_SELF_CONVENTION
845845
} else {
846846
WRONG_SELF_CONVENTION
@@ -2030,6 +2030,7 @@ enum SelfKind {
20302030
impl SelfKind {
20312031
fn matches(
20322032
self,
2033+
cx: &LateContext,
20332034
ty: &hir::Ty,
20342035
arg: &hir::Arg,
20352036
self_ty: &hir::Ty,
@@ -2047,7 +2048,7 @@ impl SelfKind {
20472048
// `Self`, `&mut Self`,
20482049
// and `Box<Self>`, including the equivalent types with `Foo`.
20492050

2050-
let is_actually_self = |ty| is_self_ty(ty) || ty == self_ty;
2051+
let is_actually_self = |ty| is_self_ty(ty) || SpanlessEq::new(cx).eq_ty(ty, self_ty);
20512052
if is_self(arg) {
20522053
match self {
20532054
SelfKind::Value => is_actually_self(ty),
@@ -2173,12 +2174,13 @@ enum OutType {
21732174
}
21742175

21752176
impl OutType {
2176-
fn matches(self, ty: &hir::FunctionRetTy) -> bool {
2177+
fn matches(self, cx: &LateContext, ty: &hir::FunctionRetTy) -> bool {
2178+
let is_unit = |ty: &hir::Ty| SpanlessEq::new(cx).eq_ty_kind(&ty.node, &hir::TyTup(vec![].into()));
21772179
match (self, ty) {
21782180
(OutType::Unit, &hir::DefaultReturn(_)) => true,
2179-
(OutType::Unit, &hir::Return(ref ty)) if ty.node == hir::TyTup(vec![].into()) => true,
2181+
(OutType::Unit, &hir::Return(ref ty)) if is_unit(ty) => true,
21802182
(OutType::Bool, &hir::Return(ref ty)) if is_bool(ty) => true,
2181-
(OutType::Any, &hir::Return(ref ty)) if ty.node != hir::TyTup(vec![].into()) => true,
2183+
(OutType::Any, &hir::Return(ref ty)) if !is_unit(ty) => true,
21822184
(OutType::Ref, &hir::Return(ref ty)) => matches!(ty.node, hir::TyRptr(_, _)),
21832185
_ => false,
21842186
}

clippy_lints/src/misc.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rustc::ty;
66
use syntax::codemap::{ExpnFormat, Span};
77
use crate::utils::{get_item_name, get_parent_expr, implements_trait, in_constant, in_macro, is_integer_literal,
88
iter_input_pats, last_path_segment, match_qpath, match_trait_method, paths, snippet, span_lint,
9-
span_lint_and_then, walk_ptrs_ty};
9+
span_lint_and_then, walk_ptrs_ty, SpanlessEq};
1010
use crate::utils::sugg::Sugg;
1111
use syntax::ast::{LitKind, CRATE_NODE_ID};
1212
use crate::consts::{constant, Constant};
@@ -418,7 +418,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
418418

419419
fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat) {
420420
if let PatKind::Binding(_, _, ident, Some(ref right)) = pat.node {
421-
if right.node == PatKind::Wild {
421+
if let PatKind::Wild = right.node {
422422
span_lint(
423423
cx,
424424
REDUNDANT_PATTERN,
@@ -542,7 +542,7 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr) {
542542
fn is_used(cx: &LateContext, expr: &Expr) -> bool {
543543
if let Some(parent) = get_parent_expr(cx, expr) {
544544
match parent.node {
545-
ExprAssign(_, ref rhs) | ExprAssignOp(_, _, ref rhs) => **rhs == *expr,
545+
ExprAssign(_, ref rhs) | ExprAssignOp(_, _, ref rhs) => SpanlessEq::new(cx).eq_expr(rhs, expr),
546546
_ => is_used(cx, parent),
547547
}
548548
} else {

clippy_lints/src/misc_early.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ impl EarlyLintPass for MiscEarly {
213213
.name;
214214

215215
for field in pfields {
216-
if field.node.pat.node == PatKind::Wild {
216+
if let PatKind::Wild = field.node.pat.node {
217217
wilds += 1;
218218
}
219219
}
@@ -231,14 +231,15 @@ impl EarlyLintPass for MiscEarly {
231231
let mut normal = vec![];
232232

233233
for field in pfields {
234-
if field.node.pat.node != PatKind::Wild {
235-
if let Ok(n) = cx.sess().codemap().span_to_snippet(field.span) {
234+
match field.node.pat.node {
235+
PatKind::Wild => {},
236+
_ => if let Ok(n) = cx.sess().codemap().span_to_snippet(field.span) {
236237
normal.push(n);
237-
}
238+
},
238239
}
239240
}
240241
for field in pfields {
241-
if field.node.pat.node == PatKind::Wild {
242+
if let PatKind::Wild = field.node.pat.node {
242243
wilds -= 1;
243244
if wilds > 0 {
244245
span_lint(

clippy_lints/src/overflow_check_conditional.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use rustc::lint::*;
22
use rustc::hir::*;
3-
use crate::utils::span_lint;
3+
use crate::utils::{span_lint, SpanlessEq};
44

55
/// **What it does:** Detects classic underflow/overflow checks.
66
///
@@ -31,13 +31,14 @@ impl LintPass for OverflowCheckConditional {
3131
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OverflowCheckConditional {
3232
// a + b < a, a > a + b, a < a - b, a - b > a
3333
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
34+
let eq = |l, r| SpanlessEq::new(cx).eq_path_segment(l, r);
3435
if_chain! {
3536
if let Expr_::ExprBinary(ref op, ref first, ref second) = expr.node;
3637
if let Expr_::ExprBinary(ref op2, ref ident1, ref ident2) = first.node;
3738
if let Expr_::ExprPath(QPath::Resolved(_, ref path1)) = ident1.node;
3839
if let Expr_::ExprPath(QPath::Resolved(_, ref path2)) = ident2.node;
3940
if let Expr_::ExprPath(QPath::Resolved(_, ref path3)) = second.node;
40-
if path1.segments[0] == path3.segments[0] || path2.segments[0] == path3.segments[0];
41+
if eq(&path1.segments[0], &path3.segments[0]) || eq(&path2.segments[0], &path3.segments[0]);
4142
if cx.tables.expr_ty(ident1).is_integral();
4243
if cx.tables.expr_ty(ident2).is_integral();
4344
then {
@@ -62,7 +63,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OverflowCheckConditional {
6263
if let Expr_::ExprPath(QPath::Resolved(_, ref path1)) = ident1.node;
6364
if let Expr_::ExprPath(QPath::Resolved(_, ref path2)) = ident2.node;
6465
if let Expr_::ExprPath(QPath::Resolved(_, ref path3)) = first.node;
65-
if path1.segments[0] == path3.segments[0] || path2.segments[0] == path3.segments[0];
66+
if eq(&path1.segments[0], &path3.segments[0]) || eq(&path2.segments[0], &path3.segments[0]);
6667
if cx.tables.expr_ty(ident1).is_integral();
6768
if cx.tables.expr_ty(ident2).is_integral();
6869
then {

clippy_lints/src/ranges.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use rustc::hir::*;
33
use syntax::ast::RangeLimits;
44
use syntax::codemap::Spanned;
55
use crate::utils::{is_integer_literal, paths, snippet, span_lint, span_lint_and_then};
6-
use crate::utils::{get_trait_def_id, higher, implements_trait};
6+
use crate::utils::{get_trait_def_id, higher, implements_trait, SpanlessEq};
77
use crate::utils::sugg::Sugg;
88

99
/// **What it does:** Checks for calling `.step_by(0)` on iterators,
@@ -118,7 +118,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
118118
// .iter() and .len() called on same Path
119119
if let ExprPath(QPath::Resolved(_, ref iter_path)) = iter_args[0].node;
120120
if let ExprPath(QPath::Resolved(_, ref len_path)) = len_args[0].node;
121-
if iter_path.segments == len_path.segments;
121+
if SpanlessEq::new(cx).eq_path_segments(&iter_path.segments, &len_path.segments);
122122
then {
123123
span_lint(cx,
124124
RANGE_ZIP_WITH_LEN,

clippy_lints/src/utils/hir_utils.rs

+22-6
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
118118
})
119119
},
120120
(&ExprMethodCall(ref l_path, _, ref l_args), &ExprMethodCall(ref r_path, _, ref r_args)) => {
121-
!self.ignore_fn && l_path == r_path && self.eq_exprs(l_args, r_args)
121+
!self.ignore_fn && self.eq_path_segment(l_path, r_path) && self.eq_exprs(l_args, r_args)
122122
},
123123
(&ExprRepeat(ref le, ref ll_id), &ExprRepeat(ref re, ref rl_id)) => {
124124
let mut celcx = constant_context(self.cx, self.cx.tcx.body_tables(ll_id.body));
@@ -225,7 +225,11 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
225225
}
226226
}
227227

228-
fn eq_path_segment(&mut self, left: &PathSegment, right: &PathSegment) -> bool {
228+
pub fn eq_path_segments(&mut self, left: &[PathSegment], right: &[PathSegment]) -> bool {
229+
left.len() == right.len() && left.iter().zip(right).all(|(l, r)| self.eq_path_segment(l, r))
230+
}
231+
232+
pub fn eq_path_segment(&mut self, left: &PathSegment, right: &PathSegment) -> bool {
229233
// The == of idents doesn't work with different contexts,
230234
// we have to be explicit about hygiene
231235
if left.ident.as_str() != right.ident.as_str() {
@@ -238,8 +242,12 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
238242
}
239243
}
240244

241-
fn eq_ty(&mut self, left: &Ty, right: &Ty) -> bool {
242-
match (&left.node, &right.node) {
245+
pub fn eq_ty(&mut self, left: &Ty, right: &Ty) -> bool {
246+
self.eq_ty_kind(&left.node, &right.node)
247+
}
248+
249+
pub fn eq_ty_kind(&mut self, left: &Ty_, right: &Ty_) -> bool {
250+
match (left, right) {
243251
(&TySlice(ref l_vec), &TySlice(ref r_vec)) => self.eq_ty(l_vec, r_vec),
244252
(&TyArray(ref lt, ref ll_id), &TyArray(ref rt, ref rl_id)) => {
245253
let full_table = self.tables;
@@ -336,7 +344,12 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
336344
self.hash_expr(e);
337345
}
338346

339-
b.rules.hash(&mut self.s);
347+
match b.rules {
348+
BlockCheckMode::DefaultBlock => 0,
349+
BlockCheckMode::UnsafeBlock(_) => 1,
350+
BlockCheckMode::PushUnsafeBlock(_) => 2,
351+
BlockCheckMode::PopUnsafeBlock(_) => 3,
352+
}.hash(&mut self.s);
340353
}
341354

342355
#[allow(many_single_char_names)]
@@ -419,7 +432,10 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
419432
ExprClosure(cap, _, eid, _, _) => {
420433
let c: fn(_, _, _, _, _) -> _ = ExprClosure;
421434
c.hash(&mut self.s);
422-
cap.hash(&mut self.s);
435+
match cap {
436+
CaptureClause::CaptureByValue => 0,
437+
CaptureClause::CaptureByRef => 1,
438+
}.hash(&mut self.s);
423439
self.hash_expr(&self.cx.tcx.hir.body(eid).value);
424440
},
425441
ExprField(ref e, ref f) => {

0 commit comments

Comments
 (0)