From f9c1acbc452e75123b0a481aa955d508972aa5f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= <matthias.krueger@famsik.de>
Date: Fri, 24 Apr 2020 11:57:34 +0200
Subject: [PATCH 1/4] rustup https://github.com/rust-lang/rust/pull/71215/
---
clippy_lints/src/cognitive_complexity.rs | 2 +-
clippy_lints/src/derive.rs | 20 ++++++++++++--------
clippy_lints/src/doc.rs | 2 +-
clippy_lints/src/escape.rs | 2 +-
clippy_lints/src/exit.rs | 2 +-
clippy_lints/src/len_zero.rs | 2 +-
clippy_lints/src/missing_const_for_fn.rs | 8 ++++----
clippy_lints/src/missing_inline.rs | 2 +-
clippy_lints/src/needless_pass_by_value.rs | 3 ++-
clippy_lints/src/new_without_default.rs | 4 ++--
clippy_lints/src/utils/inspector.rs | 2 +-
11 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs
index e842388ac98..3ba72e84fa8 100644
--- a/clippy_lints/src/cognitive_complexity.rs
+++ b/clippy_lints/src/cognitive_complexity.rs
@@ -123,7 +123,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CognitiveComplexity {
hir_id: HirId,
) {
let def_id = cx.tcx.hir().local_def_id(hir_id);
- if !cx.tcx.has_attr(def_id, sym!(test)) {
+ if !cx.tcx.has_attr(def_id.to_def_id(), sym!(test)) {
self.check(cx, kind, decl, body, span);
}
}
diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs
index 16300db0974..3cbb8fa72f7 100644
--- a/clippy_lints/src/derive.rs
+++ b/clippy_lints/src/derive.rs
@@ -160,16 +160,20 @@ fn check_hash_peq<'a, 'tcx>(
};
span_lint_and_then(
- cx, DERIVE_HASH_XOR_EQ, span,
+ cx,
+ DERIVE_HASH_XOR_EQ,
+ span,
mess,
|diag| {
- if let Some(hir_id) = cx.tcx.hir().as_local_hir_id(impl_id) {
- diag.span_note(
- cx.tcx.hir().span(hir_id),
- "`PartialEq` implemented here"
- );
+ if let Some(local_def_id) = impl_id.as_local() {
+ let hir_id = cx.tcx.hir().as_local_hir_id(local_def_id);
+ diag.span_note(
+ cx.tcx.hir().span(hir_id),
+ "`PartialEq` implemented here"
+ );
+ }
}
- });
+ );
}
});
}
@@ -225,7 +229,7 @@ fn check_unsafe_derive_deserialize<'a, 'tcx>(
ty: Ty<'tcx>,
) {
fn item_from_def_id<'tcx>(cx: &LateContext<'_, 'tcx>, def_id: DefId) -> &'tcx Item<'tcx> {
- let hir_id = cx.tcx.hir().as_local_hir_id(def_id).unwrap();
+ let hir_id = cx.tcx.hir().as_local_hir_id(def_id.expect_local());
cx.tcx.hir().expect_item(hir_id)
}
diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs
index 926bd8ed001..8d1e91f9adb 100644
--- a/clippy_lints/src/doc.rs
+++ b/clippy_lints/src/doc.rs
@@ -155,7 +155,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
match item.kind {
hir::ItemKind::Fn(ref sig, _, body_id) => {
- if !(is_entrypoint_fn(cx, cx.tcx.hir().local_def_id(item.hir_id))
+ if !(is_entrypoint_fn(cx, cx.tcx.hir().local_def_id(item.hir_id).to_def_id())
|| in_external_macro(cx.tcx.sess, item.span))
{
lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers, Some(body_id));
diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs
index 1ec60a0e6e6..6907e021a00 100644
--- a/clippy_lints/src/escape.rs
+++ b/clippy_lints/src/escape.rs
@@ -77,7 +77,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoxedLocal {
let fn_def_id = cx.tcx.hir().local_def_id(hir_id);
cx.tcx.infer_ctxt().enter(|infcx| {
- ExprUseVisitor::new(&mut v, &infcx, fn_def_id, cx.param_env, cx.tables).consume_body(body);
+ ExprUseVisitor::new(&mut v, &infcx, fn_def_id.to_def_id(), cx.param_env, cx.tables).consume_body(body);
});
for node in v.set {
diff --git a/clippy_lints/src/exit.rs b/clippy_lints/src/exit.rs
index dc1126d751d..621d56185a9 100644
--- a/clippy_lints/src/exit.rs
+++ b/clippy_lints/src/exit.rs
@@ -37,7 +37,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Exit {
// If the next item up is a function we check if it is an entry point
// and only then emit a linter warning
let def_id = cx.tcx.hir().local_def_id(parent);
- if !is_entrypoint_fn(cx, def_id) {
+ if !is_entrypoint_fn(cx, def_id.to_def_id()) {
span_lint(cx, EXIT, e.span, "usage of `process::exit`");
}
}
diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs
index 5d94013cb65..1d86ca9696f 100644
--- a/clippy_lints/src/len_zero.rs
+++ b/clippy_lints/src/len_zero.rs
@@ -143,7 +143,7 @@ fn check_trait_items(cx: &LateContext<'_, '_>, visited_trait: &Item<'_>, trait_i
if cx.access_levels.is_exported(visited_trait.hir_id) && trait_items.iter().any(|i| is_named_self(cx, i, "len")) {
let mut current_and_super_traits = FxHashSet::default();
let visited_trait_def_id = cx.tcx.hir().local_def_id(visited_trait.hir_id);
- fill_trait_set(visited_trait_def_id, &mut current_and_super_traits, cx);
+ fill_trait_set(visited_trait_def_id.to_def_id(), &mut current_and_super_traits, cx);
let is_empty_method_found = current_and_super_traits
.iter()
diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs
index 0b235bdfe3c..4301157e164 100644
--- a/clippy_lints/src/missing_const_for_fn.rs
+++ b/clippy_lints/src/missing_const_for_fn.rs
@@ -83,12 +83,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingConstForFn {
) {
let def_id = cx.tcx.hir().local_def_id(hir_id);
- if in_external_macro(cx.tcx.sess, span) || is_entrypoint_fn(cx, def_id) {
+ if in_external_macro(cx.tcx.sess, span) || is_entrypoint_fn(cx, def_id.to_def_id()) {
return;
}
// Building MIR for `fn`s with unsatisfiable preds results in ICE.
- if fn_has_unsatisfiable_preds(cx, def_id) {
+ if fn_has_unsatisfiable_preds(cx, def_id.to_def_id()) {
return;
}
@@ -118,8 +118,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingConstForFn {
let mir = cx.tcx.optimized_mir(def_id);
- if let Err((span, err)) = is_min_const_fn(cx.tcx, def_id, &mir) {
- if rustc_mir::const_eval::is_min_const_fn(cx.tcx, def_id) {
+ if let Err((span, err)) = is_min_const_fn(cx.tcx, def_id.to_def_id(), &mir) {
+ if rustc_mir::const_eval::is_min_const_fn(cx.tcx, def_id.to_def_id()) {
cx.tcx.sess.span_err(span, &err);
}
} else {
diff --git a/clippy_lints/src/missing_inline.rs b/clippy_lints/src/missing_inline.rs
index 61d164292d2..5300fd2215b 100644
--- a/clippy_lints/src/missing_inline.rs
+++ b/clippy_lints/src/missing_inline.rs
@@ -152,7 +152,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingInline {
};
if let Some(trait_def_id) = trait_def_id {
- if cx.tcx.hir().as_local_hir_id(trait_def_id).is_some() && !cx.access_levels.is_exported(impl_item.hir_id) {
+ if trait_def_id.is_local() && !cx.access_levels.is_exported(impl_item.hir_id) {
// If a trait is being implemented for an item, and the
// trait is not exported, we don't need #[inline]
return;
diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs
index 32e8f37062a..28650c88b48 100644
--- a/clippy_lints/src/needless_pass_by_value.rs
+++ b/clippy_lints/src/needless_pass_by_value.rs
@@ -135,7 +135,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
} = {
let mut ctx = MovedVariablesCtxt::default();
cx.tcx.infer_ctxt().enter(|infcx| {
- euv::ExprUseVisitor::new(&mut ctx, &infcx, fn_def_id, cx.param_env, cx.tables).consume_body(body);
+ euv::ExprUseVisitor::new(&mut ctx, &infcx, fn_def_id.to_def_id(), cx.param_env, cx.tables)
+ .consume_body(body);
});
ctx
};
diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs
index 19e06ab66c4..a599667b8d8 100644
--- a/clippy_lints/src/new_without_default.rs
+++ b/clippy_lints/src/new_without_default.rs
@@ -136,8 +136,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault {
let mut impls = HirIdSet::default();
cx.tcx.for_each_impl(default_trait_id, |d| {
if let Some(ty_def) = cx.tcx.type_of(d).ty_adt_def() {
- if let Some(hir_id) = cx.tcx.hir().as_local_hir_id(ty_def.did) {
- impls.insert(hir_id);
+ if let Some(local_def_id) = ty_def.did.as_local() {
+ impls.insert(cx.tcx.hir().as_local_hir_id(local_def_id));
}
}
});
diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs
index b97fc9547e5..7e8c61ba24a 100644
--- a/clippy_lints/src/utils/inspector.rs
+++ b/clippy_lints/src/utils/inspector.rs
@@ -378,7 +378,7 @@ fn print_item(cx: &LateContext<'_, '_>, item: &hir::Item<'_>) {
},
hir::ItemKind::Trait(..) => {
println!("trait decl");
- if cx.tcx.trait_is_auto(did) {
+ if cx.tcx.trait_is_auto(did.to_def_id()) {
println!("trait is auto");
} else {
println!("trait is not auto");
From fe25dbe549aecd409897b00fe13c32072d498336 Mon Sep 17 00:00:00 2001
From: flip1995 <hello@philkrones.com>
Date: Sat, 25 Apr 2020 20:00:00 +0200
Subject: [PATCH 2/4] Fix while_let_on_iterator suggestion and make it
MachineApplicable
---
clippy_lints/src/loops.rs | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index d76df908f09..56dd2795c60 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -576,20 +576,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops {
&& !is_iterator_used_after_while_let(cx, iter_expr)
&& !is_nested(cx, expr, &method_args[0]))
{
- let iterator = snippet(cx, method_args[0].span, "_");
+ let mut applicability = Applicability::MachineApplicable;
+ let iterator = snippet_with_applicability(cx, method_args[0].span, "_", &mut applicability);
let loop_var = if pat_args.is_empty() {
"_".to_string()
} else {
- snippet(cx, pat_args[0].span, "_").into_owned()
+ snippet_with_applicability(cx, pat_args[0].span, "_", &mut applicability).into_owned()
};
span_lint_and_sugg(
cx,
WHILE_LET_ON_ITERATOR,
- expr.span,
+ expr.span.with_hi(match_expr.span.hi()),
"this loop could be written as a `for` loop",
"try",
- format!("for {} in {} {{ .. }}", loop_var, iterator),
- Applicability::HasPlaceholders,
+ format!("for {} in {}", loop_var, iterator),
+ applicability,
);
}
}
From 44511d5ee6c2e1ec6b6c9b3f82e7d1027a1f2b29 Mon Sep 17 00:00:00 2001
From: flip1995 <hello@philkrones.com>
Date: Sat, 25 Apr 2020 20:01:22 +0200
Subject: [PATCH 3/4] Update while_let_on_iterator tests
---
tests/ui/while_let_on_iterator.fixed | 144 ++++++++++++++++++++++++++
tests/ui/while_let_on_iterator.rs | 34 ++++--
tests/ui/while_let_on_iterator.stderr | 22 ++--
3 files changed, 178 insertions(+), 22 deletions(-)
create mode 100644 tests/ui/while_let_on_iterator.fixed
diff --git a/tests/ui/while_let_on_iterator.fixed b/tests/ui/while_let_on_iterator.fixed
new file mode 100644
index 00000000000..447b36048ee
--- /dev/null
+++ b/tests/ui/while_let_on_iterator.fixed
@@ -0,0 +1,144 @@
+// run-rustfix
+
+#![warn(clippy::while_let_on_iterator)]
+#![allow(clippy::never_loop, unreachable_code, unused_mut)]
+
+fn base() {
+ let mut iter = 1..20;
+ for x in iter {
+ println!("{}", x);
+ }
+
+ let mut iter = 1..20;
+ for x in iter {
+ println!("{}", x);
+ }
+
+ let mut iter = 1..20;
+ for _ in iter {}
+
+ let mut iter = 1..20;
+ while let None = iter.next() {} // this is fine (if nonsensical)
+
+ let mut iter = 1..20;
+ if let Some(x) = iter.next() {
+ // also fine
+ println!("{}", x)
+ }
+
+ // the following shouldn't warn because it can't be written with a for loop
+ let mut iter = 1u32..20;
+ while let Some(_) = iter.next() {
+ println!("next: {:?}", iter.next())
+ }
+
+ // neither can this
+ let mut iter = 1u32..20;
+ while let Some(_) = iter.next() {
+ println!("next: {:?}", iter.next());
+ }
+
+ // or this
+ let mut iter = 1u32..20;
+ while let Some(_) = iter.next() {
+ break;
+ }
+ println!("Remaining iter {:?}", iter);
+
+ // or this
+ let mut iter = 1u32..20;
+ while let Some(_) = iter.next() {
+ iter = 1..20;
+ }
+}
+
+// Issue #1188
+fn refutable() {
+ let a = [42, 1337];
+ let mut b = a.iter();
+
+ // consume all the 42s
+ while let Some(&42) = b.next() {}
+
+ let a = [(1, 2, 3)];
+ let mut b = a.iter();
+
+ while let Some(&(1, 2, 3)) = b.next() {}
+
+ let a = [Some(42)];
+ let mut b = a.iter();
+
+ while let Some(&None) = b.next() {}
+
+ /* This gives “refutable pattern in `for` loop binding: `&_` not covered”
+ for &42 in b {}
+ for &(1, 2, 3) in b {}
+ for &Option::None in b.next() {}
+ // */
+}
+
+fn nested_loops() {
+ let a = [42, 1337];
+ let mut y = a.iter();
+ loop {
+ // x is reused, so don't lint here
+ while let Some(_) = y.next() {}
+ }
+
+ let mut y = a.iter();
+ for _ in 0..2 {
+ while let Some(_) = y.next() {
+ // y is reused, don't lint
+ }
+ }
+
+ loop {
+ let mut y = a.iter();
+ for _ in y {
+ // use a for loop here
+ }
+ }
+}
+
+fn issue1121() {
+ use std::collections::HashSet;
+ let mut values = HashSet::new();
+ values.insert(1);
+
+ while let Some(&value) = values.iter().next() {
+ values.remove(&value);
+ }
+}
+
+fn issue2965() {
+ // This should not cause an ICE and suggest:
+ //
+ // for _ in values.iter() {}
+ //
+ use std::collections::HashSet;
+ let mut values = HashSet::new();
+ values.insert(1);
+
+ for _ in values.iter() {
+ // FIXME(flip1995): Linting this with the following line uncommented is a FP, see #1654
+ // values.remove(&1);
+ }
+}
+
+fn issue3670() {
+ let array = [Some(0), None, Some(1)];
+ let mut iter = array.iter();
+
+ while let Some(elem) = iter.next() {
+ let _ = elem.or_else(|| *iter.next()?);
+ }
+}
+
+fn main() {
+ base();
+ refutable();
+ nested_loops();
+ issue1121();
+ issue2965();
+ issue3670();
+}
diff --git a/tests/ui/while_let_on_iterator.rs b/tests/ui/while_let_on_iterator.rs
index 84dfc34db15..56a245aa8c7 100644
--- a/tests/ui/while_let_on_iterator.rs
+++ b/tests/ui/while_let_on_iterator.rs
@@ -1,7 +1,9 @@
-#![warn(clippy::while_let_on_iterator)]
-#![allow(clippy::never_loop)]
+// run-rustfix
-fn main() {
+#![warn(clippy::while_let_on_iterator)]
+#![allow(clippy::never_loop, unreachable_code, unused_mut)]
+
+fn base() {
let mut iter = 1..20;
while let Option::Some(x) = iter.next() {
println!("{}", x);
@@ -26,26 +28,26 @@ fn main() {
// the following shouldn't warn because it can't be written with a for loop
let mut iter = 1u32..20;
- while let Some(x) = iter.next() {
+ while let Some(_) = iter.next() {
println!("next: {:?}", iter.next())
}
// neither can this
let mut iter = 1u32..20;
- while let Some(x) = iter.next() {
+ while let Some(_) = iter.next() {
println!("next: {:?}", iter.next());
}
// or this
let mut iter = 1u32..20;
- while let Some(x) = iter.next() {
+ while let Some(_) = iter.next() {
break;
}
println!("Remaining iter {:?}", iter);
// or this
let mut iter = 1u32..20;
- while let Some(x) = iter.next() {
+ while let Some(_) = iter.next() {
iter = 1..20;
}
}
@@ -80,19 +82,19 @@ fn nested_loops() {
let mut y = a.iter();
loop {
// x is reused, so don't lint here
- while let Some(v) = y.next() {}
+ while let Some(_) = y.next() {}
}
let mut y = a.iter();
for _ in 0..2 {
- while let Some(v) = y.next() {
+ while let Some(_) = y.next() {
// y is reused, don't lint
}
}
loop {
let mut y = a.iter();
- while let Some(v) = y.next() {
+ while let Some(_) = y.next() {
// use a for loop here
}
}
@@ -118,7 +120,8 @@ fn issue2965() {
values.insert(1);
while let Some(..) = values.iter().next() {
- values.remove(&1);
+ // FIXME(flip1995): Linting this with the following line uncommented is a FP, see #1654
+ // values.remove(&1);
}
}
@@ -130,3 +133,12 @@ fn issue3670() {
let _ = elem.or_else(|| *iter.next()?);
}
}
+
+fn main() {
+ base();
+ refutable();
+ nested_loops();
+ issue1121();
+ issue2965();
+ issue3670();
+}
diff --git a/tests/ui/while_let_on_iterator.stderr b/tests/ui/while_let_on_iterator.stderr
index 03d2ef55066..94caedb43a5 100644
--- a/tests/ui/while_let_on_iterator.stderr
+++ b/tests/ui/while_let_on_iterator.stderr
@@ -1,34 +1,34 @@
error: this loop could be written as a `for` loop
- --> $DIR/while_let_on_iterator.rs:6:33
+ --> $DIR/while_let_on_iterator.rs:8:5
|
LL | while let Option::Some(x) = iter.next() {
- | ^^^^^^^^^^^ help: try: `for x in iter { .. }`
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in iter`
|
= note: `-D clippy::while-let-on-iterator` implied by `-D warnings`
error: this loop could be written as a `for` loop
- --> $DIR/while_let_on_iterator.rs:11:25
+ --> $DIR/while_let_on_iterator.rs:13:5
|
LL | while let Some(x) = iter.next() {
- | ^^^^^^^^^^^ help: try: `for x in iter { .. }`
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in iter`
error: this loop could be written as a `for` loop
- --> $DIR/while_let_on_iterator.rs:16:25
+ --> $DIR/while_let_on_iterator.rs:18:5
|
LL | while let Some(_) = iter.next() {}
- | ^^^^^^^^^^^ help: try: `for _ in iter { .. }`
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in iter`
error: this loop could be written as a `for` loop
- --> $DIR/while_let_on_iterator.rs:95:29
+ --> $DIR/while_let_on_iterator.rs:97:9
|
-LL | while let Some(v) = y.next() {
- | ^^^^^^^^ help: try: `for v in y { .. }`
+LL | while let Some(_) = y.next() {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in y`
error: this loop could be written as a `for` loop
- --> $DIR/while_let_on_iterator.rs:120:26
+ --> $DIR/while_let_on_iterator.rs:122:5
|
LL | while let Some(..) = values.iter().next() {
- | ^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in values.iter() { .. }`
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in values.iter()`
error: aborting due to 5 previous errors
From dda1c8d8af92237a9db9f67e2c210ffca2f3f8e0 Mon Sep 17 00:00:00 2001
From: flip1995 <hello@philkrones.com>
Date: Sat, 25 Apr 2020 20:07:55 +0200
Subject: [PATCH 4/4] Update issue_2356.stderr reference file
---
tests/ui/issue_2356.stderr | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/ui/issue_2356.stderr b/tests/ui/issue_2356.stderr
index c8c00f78035..51b872e21c0 100644
--- a/tests/ui/issue_2356.stderr
+++ b/tests/ui/issue_2356.stderr
@@ -1,8 +1,8 @@
error: this loop could be written as a `for` loop
- --> $DIR/issue_2356.rs:15:29
+ --> $DIR/issue_2356.rs:15:9
|
LL | while let Some(e) = it.next() {
- | ^^^^^^^^^ help: try: `for e in it { .. }`
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for e in it`
|
note: the lint level is defined here
--> $DIR/issue_2356.rs:1:9