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/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, ); } } 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"); 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 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