From 6567459fc1e54eaefabffe387c0907f7b8a63611 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Sun, 26 Apr 2020 14:33:18 +0200 Subject: [PATCH] submodules: update clippy from 891e1a85 to b7c802b5 Changes: ```` rustup to https://github.com/rust-lang/rust/pull/70043 map_clone: avoid suggesting `copied()` for &mut fix redundant_pattern_matching lint Add tests for #1654 Don't trigger while_let_on_iterator when the iterator is recreated every iteration Update issue_2356.stderr reference file Update while_let_on_iterator tests Fix while_let_on_iterator suggestion and make it MachineApplicable Add lifetime test case for `new_ret_no_self` rustup https://github.com/rust-lang/rust/pull/71215/ Downgrade match_bool to pedantic Run fetch before testing if master contains beta The beta branch update should not require a force push Add a note to the beta sections of release.md Remove apt-get upgrade again Always use the deploy script and templates of the master branch README: fix lit count line clippy_dev: make it fatal when the regex for updating lint count does not match `predecessors_for` will be removed soon Rustup "Remove `BodyAndCache`" Only run (late) internal lints, when they are warn/deny/forbid Only run cargo lints, when they are warn/deny/forbid span_lint_and_note now takes an Option for the note_span instead of just a span Make lint also capture blocks and closures, adjust language to mention other mutex types don't test the code in the lint docs Switch to matching against full paths instead of just the last element of the path Lint for holding locks across await points Also mention `--fix` for nightly users fix crash on issue-69020-assoc-const-arith-overflow.rs Address review comments remark fixes Update CHANGELOG.md for Rust 1.43 and 1.44 update stderr file util/fetch_prs_between.sh: Add Markdown formatted Link factor ifs into function, add differing mutex test Update the changelog update documentation Apply suggestions from PR review update span_lint_and_help call to six args test for mutex eq, add another test case use if chain cargo dev fmt fix map import to rustc_middle dev update_lints fix internal clippy warnings change visitor name to OppVisitor use Visitor api to find Mutex::lock calls add note about update-all-refs script, revert redundant pat to master move closures to seperate fns, remove known problems use span_lint_and_help, cargo dev fmt creating suggestion progress work on suggestion for auto fix Implement unsafe_derive_deserialize lint Update empty_enum.stderr Formatting and naming Formatting and naming Cleanup: `node_id` -> `hir_id` Fix issue #2907. Don't trigger toplevel_ref_arg for `for` loops Cleanup: future_not_send: use `return_ty` method Remove badge FIXME from Cargo.toml Change note_span argument for span_lint_and_note. Add an Option argument to span_lint_and_help. Fixes internal lint warning in code base. Implement collapsible_span_lint_calls lint. ```` Fixes #71453 --- .github/workflows/clippy_bors.yml | 2 - .github/workflows/deploy.yml | 6 + CHANGELOG.md | 96 +++++- Cargo.toml | 3 - clippy_dev/src/lib.rs | 5 +- clippy_lints/src/as_conversions.rs | 1 + clippy_lints/src/assertions_on_constants.rs | 3 + clippy_lints/src/atomic_ordering.rs | 3 + clippy_lints/src/await_holding_lock.rs | 97 ++++++ clippy_lints/src/cargo_common_metadata.rs | 18 +- clippy_lints/src/cognitive_complexity.rs | 3 +- clippy_lints/src/comparison_chain.rs | 1 + clippy_lints/src/consts.rs | 6 +- clippy_lints/src/copies.rs | 6 +- clippy_lints/src/copy_iterator.rs | 2 +- clippy_lints/src/dbg_macro.rs | 1 + clippy_lints/src/derive.rs | 163 +++++++++- clippy_lints/src/doc.rs | 2 +- clippy_lints/src/drop_forget_ref.rs | 4 +- clippy_lints/src/else_if_without_else.rs | 1 + clippy_lints/src/empty_enum.rs | 18 +- clippy_lints/src/enum_variants.rs | 1 + clippy_lints/src/escape.rs | 2 +- clippy_lints/src/eta_reduction.rs | 20 +- clippy_lints/src/eval_order_dependence.rs | 2 +- clippy_lints/src/excessive_bools.rs | 2 + clippy_lints/src/exit.rs | 2 +- clippy_lints/src/formatting.rs | 9 +- clippy_lints/src/functions.rs | 1 + clippy_lints/src/future_not_send.rs | 5 +- clippy_lints/src/identity_conversion.rs | 53 ++-- clippy_lints/src/if_let_mutex.rs | 160 ++++++++++ clippy_lints/src/if_not_else.rs | 2 + clippy_lints/src/indexing_slicing.rs | 3 +- clippy_lints/src/inherent_to_string.rs | 2 + clippy_lints/src/int_plus_one.rs | 15 +- clippy_lints/src/integer_division.rs | 1 + clippy_lints/src/large_stack_arrays.rs | 1 + clippy_lints/src/len_zero.rs | 2 +- clippy_lints/src/let_underscore.rs | 3 + clippy_lints/src/lib.rs | 23 +- clippy_lints/src/loops.rs | 110 ++++--- clippy_lints/src/main_recursion.rs | 1 + clippy_lints/src/map_clone.rs | 5 +- clippy_lints/src/matches.rs | 8 +- clippy_lints/src/mem_replace.rs | 2 + clippy_lints/src/methods/mod.rs | 25 +- clippy_lints/src/misc.rs | 3 +- clippy_lints/src/misc_early.rs | 2 + clippy_lints/src/missing_const_for_fn.rs | 8 +- clippy_lints/src/missing_inline.rs | 3 +- clippy_lints/src/multiple_crate_versions.rs | 14 +- clippy_lints/src/needless_continue.rs | 1 + clippy_lints/src/needless_pass_by_value.rs | 3 +- clippy_lints/src/new_without_default.rs | 4 +- clippy_lints/src/option_env_unwrap.rs | 1 + clippy_lints/src/ptr.rs | 17 +- clippy_lints/src/redundant_clone.rs | 7 +- .../src/redundant_pattern_matching.rs | 17 +- clippy_lints/src/redundant_pub_crate.rs | 4 +- clippy_lints/src/regex.rs | 4 +- clippy_lints/src/returns.rs | 87 +++--- clippy_lints/src/trait_bounds.rs | 1 + clippy_lints/src/transmute.rs | 26 +- clippy_lints/src/types.rs | 18 +- clippy_lints/src/unnamed_address.rs | 2 + clippy_lints/src/unused_self.rs | 1 + clippy_lints/src/utils/diagnostics.rs | 26 +- clippy_lints/src/utils/inspector.rs | 2 +- clippy_lints/src/utils/internal_lints.rs | 287 +++++++++++++++++- clippy_lints/src/utils/mod.rs | 11 +- clippy_lints/src/utils/paths.rs | 4 + clippy_lints/src/verbose_file_reads.rs | 2 + clippy_lints/src/wildcard_dependencies.rs | 14 +- clippy_lints/src/zero_div_zero.rs | 1 + doc/adding_lints.md | 6 +- doc/changelog_update.md | 31 +- doc/release.md | 12 +- src/lintlist/mod.rs | 23 +- tests/ui/await_holding_lock.rs | 64 ++++ tests/ui/await_holding_lock.stderr | 63 ++++ tests/ui/borrow_box.rs | 18 ++ tests/ui/borrow_box.stderr | 18 +- tests/ui/collapsible_span_lint_calls.fixed | 91 ++++++ tests/ui/collapsible_span_lint_calls.rs | 101 ++++++ tests/ui/collapsible_span_lint_calls.stderr | 49 +++ tests/ui/crashes/ice-5497.rs | 11 + tests/ui/empty_enum.stderr | 6 +- tests/ui/if_let_mutex.rs | 42 +++ tests/ui/if_let_mutex.stderr | 29 ++ tests/ui/implicit_return.fixed | 3 +- tests/ui/implicit_return.rs | 3 +- tests/ui/implicit_return.stderr | 16 +- tests/ui/issue_2356.stderr | 4 +- tests/ui/map_clone.fixed | 11 + tests/ui/map_clone.rs | 11 + tests/ui/map_clone.stderr | 12 +- tests/ui/match_bool.rs | 2 + tests/ui/match_bool.stderr | 22 +- tests/ui/needless_return.fixed | 2 +- tests/ui/needless_return.rs | 2 +- tests/ui/new_ret_no_self.rs | 11 + tests/ui/redundant_pattern_matching.fixed | 58 +++- tests/ui/redundant_pattern_matching.rs | 58 +++- tests/ui/redundant_pattern_matching.stderr | 54 +++- tests/ui/toplevel_ref_arg.fixed | 3 + tests/ui/toplevel_ref_arg.rs | 3 + tests/ui/unsafe_derive_deserialize.rs | 60 ++++ tests/ui/unsafe_derive_deserialize.stderr | 39 +++ tests/ui/while_let_on_iterator.fixed | 160 ++++++++++ tests/ui/while_let_on_iterator.rs | 52 +++- tests/ui/while_let_on_iterator.stderr | 26 +- util/fetch_prs_between.sh | 1 + 113 files changed, 2216 insertions(+), 432 deletions(-) create mode 100644 clippy_lints/src/await_holding_lock.rs create mode 100644 clippy_lints/src/if_let_mutex.rs create mode 100644 tests/ui/await_holding_lock.rs create mode 100644 tests/ui/await_holding_lock.stderr create mode 100644 tests/ui/collapsible_span_lint_calls.fixed create mode 100644 tests/ui/collapsible_span_lint_calls.rs create mode 100644 tests/ui/collapsible_span_lint_calls.stderr create mode 100644 tests/ui/crashes/ice-5497.rs create mode 100644 tests/ui/if_let_mutex.rs create mode 100644 tests/ui/if_let_mutex.stderr create mode 100644 tests/ui/unsafe_derive_deserialize.rs create mode 100644 tests/ui/unsafe_derive_deserialize.stderr create mode 100644 tests/ui/while_let_on_iterator.fixed diff --git a/.github/workflows/clippy_bors.yml b/.github/workflows/clippy_bors.yml index 4429a7c1e5c5..6675a1029bbc 100644 --- a/.github/workflows/clippy_bors.yml +++ b/.github/workflows/clippy_bors.yml @@ -77,8 +77,6 @@ jobs: run: | sudo dpkg --add-architecture i386 sudo apt-get update - # perform system upgrade to work around https://github.com/rust-lang/rust-clippy/issues/5477 , revert as soon as that is fixed - sudo apt-get -y upgrade sudo apt-get install gcc-multilib libssl-dev:i386 libgit2-dev:i386 if: matrix.host == 'i686-unknown-linux-gnu' diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 10033daf0aed..f542f9b02c17 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -38,6 +38,12 @@ jobs: - name: Set beta to true if: github.ref == 'refs/heads/beta' run: echo "::set-env name=BETA::true" + + - name: Use scripts and templates from master branch + run: | + git fetch --no-tags --prune --depth=1 origin master + git checkout origin/master -- .github/deploy.sh util/gh-pages/ util/*.py + - name: Deploy run: | eval "$(ssh-agent -s)" diff --git a/CHANGELOG.md b/CHANGELOG.md index e513787a53a6..9aab249621ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,15 +4,98 @@ All notable changes to this project will be documented in this file. See [Changelog Update](doc/changelog_update.md) if you want to update this document. -## Unreleased / In Rust Beta or Nightly +## Unreleased / In Rust Nightly -[329923e...master](https://github.com/rust-lang/rust-clippy/compare/329923e...master) +[891e1a8...master](https://github.com/rust-lang/rust-clippy/compare/891e1a8...master) + +## Rust 1.44 + +Current beta, release 2020-06-04 + +[204bb9b...891e1a8](https://github.com/rust-lang/rust-clippy/compare/204bb9b...891e1a8) + +### New lints + +* [`explicit_deref_methods`] [#5226](https://github.com/rust-lang/rust-clippy/pull/5226) +* [`implicit_saturating_sub`] [#5427](https://github.com/rust-lang/rust-clippy/pull/5427) +* [`macro_use_imports`] [#5230](https://github.com/rust-lang/rust-clippy/pull/5230) +* [`verbose_file_reads`] [#5272](https://github.com/rust-lang/rust-clippy/pull/5272) +* [`future_not_send`] [#5423](https://github.com/rust-lang/rust-clippy/pull/5423) +* [`redundant_pub_crate`] [#5319](https://github.com/rust-lang/rust-clippy/pull/5319) +* [`large_const_arrays`] [#5248](https://github.com/rust-lang/rust-clippy/pull/5248) +* [`result_map_or_into_option`] [#5415](https://github.com/rust-lang/rust-clippy/pull/5415) +* [`redundant_allocation`] [#5349](https://github.com/rust-lang/rust-clippy/pull/5349) +* [`fn_address_comparisons`] [#5294](https://github.com/rust-lang/rust-clippy/pull/5294) +* [`vtable_address_comparisons`] [#5294](https://github.com/rust-lang/rust-clippy/pull/5294) + + +### Moves and Deprecations + +* Deprecate [`replace_consts`] lint [#5380](https://github.com/rust-lang/rust-clippy/pull/5380) +* Move [`cognitive_complexity`] to nursery [#5428](https://github.com/rust-lang/rust-clippy/pull/5428) +* Move [`useless_transmute`] to nursery [#5364](https://github.com/rust-lang/rust-clippy/pull/5364) +* Downgrade [`inefficient_to_string`] to pedantic [#5412](https://github.com/rust-lang/rust-clippy/pull/5412) +* Downgrade [`option_option`] to pedantic [#5401](https://github.com/rust-lang/rust-clippy/pull/5401) +* Downgrade [`unreadable_literal`] to pedantic [#5419](https://github.com/rust-lang/rust-clippy/pull/5419) +* Downgrade [`let_unit_value`] to pedantic [#5409](https://github.com/rust-lang/rust-clippy/pull/5409) +* Downgrade [`trivially_copy_pass_by_ref`] to pedantic [#5410](https://github.com/rust-lang/rust-clippy/pull/5410) +* Downgrade [`implicit_hasher`] to pedantic [#5411](https://github.com/rust-lang/rust-clippy/pull/5411) + +### Enhancements + +* On _nightly_ you can now use `cargo clippy --fix -Z unstable-options` to + auto-fix lints that support this [#5363](https://github.com/rust-lang/rust-clippy/pull/5363) +* Make [`redundant_clone`] also trigger on cases where the cloned value is not + consumed. [#5304](https://github.com/rust-lang/rust-clippy/pull/5304) +* Expand [`integer_arithmetic`] to also disallow bit-shifting [#5430](https://github.com/rust-lang/rust-clippy/pull/5430) +* [`option_as_ref_deref`] now detects more deref cases [#5425](https://github.com/rust-lang/rust-clippy/pull/5425) +* [`large_enum_variant`] now report the sizes of the largest and second-largest variants [#5466](https://github.com/rust-lang/rust-clippy/pull/5466) +* [`bool_comparison`] now also checks for inequality comparisons that can be + written more concisely [#5365](https://github.com/rust-lang/rust-clippy/pull/5365) +* Expand [`clone_on_copy`] to work in method call arguments as well [#5441](https://github.com/rust-lang/rust-clippy/pull/5441) +* [`redundant_pattern_matching`] now also handles `while let` [#5483](https://github.com/rust-lang/rust-clippy/pull/5483) +* [`integer_arithmetic`] now also lints references of integers [#5329](https://github.com/rust-lang/rust-clippy/pull/5329) +* Expand [`float_cmp_const`] to also work on arrays [#5345](https://github.com/rust-lang/rust-clippy/pull/5345) +* Trigger [`map_flatten`] when map is called on an `Option` [#5473](https://github.com/rust-lang/rust-clippy/pull/5473) + +### False Positive Fixes + +* [`many_single_char_names`] [#5468](https://github.com/rust-lang/rust-clippy/pull/5468) +* [`should_implement_trait`] [#5437](https://github.com/rust-lang/rust-clippy/pull/5437) +* [`unused_self`] [#5387](https://github.com/rust-lang/rust-clippy/pull/5387) +* [`redundant_clone`] [#5453](https://github.com/rust-lang/rust-clippy/pull/5453) +* [`precedence`] [#5445](https://github.com/rust-lang/rust-clippy/pull/5445) +* [`suspicious_op_assign_impl`] [#5424](https://github.com/rust-lang/rust-clippy/pull/5424) +* [`needless_lifetimes`] [#5293](https://github.com/rust-lang/rust-clippy/pull/5293) +* [`redundant_pattern`] [#5287](https://github.com/rust-lang/rust-clippy/pull/5287) +* [`inconsistent_digit_grouping`] [#5451](https://github.com/rust-lang/rust-clippy/pull/5451) + + +### Suggestion Improvements + +* Improved [`question_mark`] lint suggestion so that it doesn't add redundant `as_ref()` [#5481](https://github.com/rust-lang/rust-clippy/pull/5481) +* Improve the suggested placeholder in [`option_map_unit_fn`] [#5292](https://github.com/rust-lang/rust-clippy/pull/5292) +* Improve suggestion for [`match_single_binding`] when triggered inside a closure [#5350](https://github.com/rust-lang/rust-clippy/pull/5350) + +### ICE Fixes + +* Handle the unstable `trivial_bounds` feature [#5296](https://github.com/rust-lang/rust-clippy/pull/5296) +* `shadow_*` lints [#5297](https://github.com/rust-lang/rust-clippy/pull/5297) + +### Documentation + +* Fix documentation generation for configurable lints [#5353](https://github.com/rust-lang/rust-clippy/pull/5353) +* Update documentation for [`new_ret_no_self`] [#5448](https://github.com/rust-lang/rust-clippy/pull/5448) +* The documentation for [`option_option`] now suggest using a tri-state enum [#5403](https://github.com/rust-lang/rust-clippy/pull/5403) +* Fix bit mask example in [`verbose_bit_mask`] documentation [#5454](https://github.com/rust-lang/rust-clippy/pull/5454) +* [`wildcard_imports`] documentation now mentions that `use ...::prelude::*` is + not linted [#5312](https://github.com/rust-lang/rust-clippy/pull/5312) ## Rust 1.43 -Current beta, release 2020-04-23 +Current stable, released 2020-04-23 -[4ee1206...329923e](https://github.com/rust-lang/rust-clippy/compare/4ee1206...329923e) +[4ee1206...204bb9b](https://github.com/rust-lang/rust-clippy/compare/4ee1206...204bb9b) ### New lints @@ -68,7 +151,7 @@ Current beta, release 2020-04-23 ## Rust 1.42 -Current stable, released 2020-03-12 +Released 2020-03-12 [69f99e7...4ee1206](https://github.com/rust-lang/rust-clippy/compare/69f99e7...4ee1206) @@ -1188,6 +1271,7 @@ Released 2018-09-13 [`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants [`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern [`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops +[`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock [`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask [`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name [`block_in_if_condition_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_expr @@ -1286,6 +1370,7 @@ Released 2018-09-13 [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap [`identity_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion [`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op +[`if_let_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_mutex [`if_let_redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_redundant_pattern_matching [`if_let_some_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_some_result [`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else @@ -1524,6 +1609,7 @@ Released 2018-09-13 [`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern [`unreachable`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreachable [`unreadable_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreadable_literal +[`unsafe_derive_deserialize`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_derive_deserialize [`unsafe_removed_from_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_removed_from_name [`unsafe_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_vector_initialization [`unseparated_literal_suffix`]: https://rust-lang.github.io/rust-clippy/master/index.html#unseparated_literal_suffix diff --git a/Cargo.toml b/Cargo.toml index 7b07fde43992..63ce2cd8cad7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,9 +18,6 @@ build = "build.rs" edition = "2018" publish = false -# [badges] -# FIXME(flip1995): Add GHA badge once rust-lang/crates.io#1838 is merged - [[bin]] name = "cargo-clippy" test = false diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 1f8510f43a61..6fdd282c6849 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -291,10 +291,11 @@ where } if !found { - // This happens if the provided regex in `clippy_dev/src/main.rs` is not found in the + // This happens if the provided regex in `clippy_dev/src/main.rs` does not match in the // given text or file. Most likely this is an error on the programmer's side and the Regex // is incorrect. - eprintln!("error: regex `{:?}` not found. You may have to update it.", start); + eprintln!("error: regex \n{:?}\ndoesn't match. You may have to update it.", start); + std::process::exit(1); } let mut new_lines = new_lines.join("\n"); diff --git a/clippy_lints/src/as_conversions.rs b/clippy_lints/src/as_conversions.rs index 4d8bbcd31024..0c8efd755146 100644 --- a/clippy_lints/src/as_conversions.rs +++ b/clippy_lints/src/as_conversions.rs @@ -50,6 +50,7 @@ impl EarlyLintPass for AsConversions { AS_CONVERSIONS, expr.span, "using a potentially dangerous silent `as` conversion", + None, "consider using a safe wrapper for this conversion", ); } diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index e2fe5f2400f4..f8a8fdcd3aa3 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -41,6 +41,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants { } else { "`assert!(true)` will be optimized out by the compiler" }, + None, "remove it", ); }; @@ -50,6 +51,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants { ASSERTIONS_ON_CONSTANTS, e.span, "`assert!(false)` should probably be replaced", + None, "use `panic!()` or `unreachable!()`", ); }; @@ -59,6 +61,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants { ASSERTIONS_ON_CONSTANTS, e.span, &format!("`assert!(false, {})` should probably be replaced", panic_message), + None, &format!("use `panic!({})` or `unreachable!({})`", panic_message, panic_message), ) }; diff --git a/clippy_lints/src/atomic_ordering.rs b/clippy_lints/src/atomic_ordering.rs index d9ff1fe0a1d0..73b4cef47250 100644 --- a/clippy_lints/src/atomic_ordering.rs +++ b/clippy_lints/src/atomic_ordering.rs @@ -85,6 +85,7 @@ fn check_atomic_load_store(cx: &LateContext<'_, '_>, expr: &Expr<'_>) { INVALID_ATOMIC_ORDERING, ordering_arg.span, "atomic loads cannot have `Release` and `AcqRel` ordering", + None, "consider using ordering modes `Acquire`, `SeqCst` or `Relaxed`" ); } else if method == "store" && @@ -94,6 +95,7 @@ fn check_atomic_load_store(cx: &LateContext<'_, '_>, expr: &Expr<'_>) { INVALID_ATOMIC_ORDERING, ordering_arg.span, "atomic stores cannot have `Acquire` and `AcqRel` ordering", + None, "consider using ordering modes `Release`, `SeqCst` or `Relaxed`" ); } @@ -118,6 +120,7 @@ fn check_memory_fence(cx: &LateContext<'_, '_>, expr: &Expr<'_>) { INVALID_ATOMIC_ORDERING, args[0].span, "memory fences cannot have `Relaxed` ordering", + None, "consider using ordering modes `Acquire`, `Release`, `AcqRel` or `SeqCst`" ); } diff --git a/clippy_lints/src/await_holding_lock.rs b/clippy_lints/src/await_holding_lock.rs new file mode 100644 index 000000000000..832910763e60 --- /dev/null +++ b/clippy_lints/src/await_holding_lock.rs @@ -0,0 +1,97 @@ +use crate::utils::{match_def_path, paths, span_lint_and_note}; +use rustc_hir::def_id::DefId; +use rustc_hir::{AsyncGeneratorKind, Body, BodyId, GeneratorKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::GeneratorInteriorTypeCause; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; + +declare_clippy_lint! { + /// **What it does:** Checks for calls to await while holding a + /// non-async-aware MutexGuard. + /// + /// **Why is this bad?** The Mutex types found in syd::sync and parking_lot + /// are not designed to operator in an async context across await points. + /// + /// There are two potential solutions. One is to use an asynx-aware Mutex + /// type. Many asynchronous foundation crates provide such a Mutex type. The + /// other solution is to ensure the mutex is unlocked before calling await, + /// either by introducing a scope or an explicit call to Drop::drop. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust,ignore + /// use std::sync::Mutex; + /// + /// async fn foo(x: &Mutex) { + /// let guard = x.lock().unwrap(); + /// *guard += 1; + /// bar.await; + /// } + /// ``` + /// + /// Use instead: + /// ```rust,ignore + /// use std::sync::Mutex; + /// + /// async fn foo(x: &Mutex) { + /// { + /// let guard = x.lock().unwrap(); + /// *guard += 1; + /// } + /// bar.await; + /// } + /// ``` + pub AWAIT_HOLDING_LOCK, + pedantic, + "Inside an async function, holding a MutexGuard while calling await" +} + +declare_lint_pass!(AwaitHoldingLock => [AWAIT_HOLDING_LOCK]); + +impl LateLintPass<'_, '_> for AwaitHoldingLock { + fn check_body(&mut self, cx: &LateContext<'_, '_>, body: &'_ Body<'_>) { + use AsyncGeneratorKind::{Block, Closure, Fn}; + match body.generator_kind { + Some(GeneratorKind::Async(Block)) + | Some(GeneratorKind::Async(Closure)) + | Some(GeneratorKind::Async(Fn)) => { + let body_id = BodyId { + hir_id: body.value.hir_id, + }; + let def_id = cx.tcx.hir().body_owner_def_id(body_id); + let tables = cx.tcx.typeck_tables_of(def_id); + check_interior_types(cx, &tables.generator_interior_types, body.value.span); + }, + _ => {}, + } + } +} + +fn check_interior_types(cx: &LateContext<'_, '_>, ty_causes: &[GeneratorInteriorTypeCause<'_>], span: Span) { + for ty_cause in ty_causes { + if let rustc_middle::ty::Adt(adt, _) = ty_cause.ty.kind { + if is_mutex_guard(cx, adt.did) { + span_lint_and_note( + cx, + AWAIT_HOLDING_LOCK, + ty_cause.span, + "this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await.", + ty_cause.scope_span.or(Some(span)), + "these are all the await points this lock is held through", + ); + } + } + } +} + +fn is_mutex_guard(cx: &LateContext<'_, '_>, def_id: DefId) -> bool { + match_def_path(cx, def_id, &paths::MUTEX_GUARD) + || match_def_path(cx, def_id, &paths::RWLOCK_READ_GUARD) + || match_def_path(cx, def_id, &paths::RWLOCK_WRITE_GUARD) + || match_def_path(cx, def_id, &paths::PARKING_LOT_MUTEX_GUARD) + || match_def_path(cx, def_id, &paths::PARKING_LOT_RWLOCK_READ_GUARD) + || match_def_path(cx, def_id, &paths::PARKING_LOT_RWLOCK_WRITE_GUARD) +} diff --git a/clippy_lints/src/cargo_common_metadata.rs b/clippy_lints/src/cargo_common_metadata.rs index 06e866dd72a0..782da249808d 100644 --- a/clippy_lints/src/cargo_common_metadata.rs +++ b/clippy_lints/src/cargo_common_metadata.rs @@ -2,9 +2,9 @@ use std::path::PathBuf; -use crate::utils::span_lint; -use rustc_ast::ast::Crate; -use rustc_lint::{EarlyContext, EarlyLintPass}; +use crate::utils::{run_lints, span_lint}; +use rustc_hir::{hir_id::CRATE_HIR_ID, Crate}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::DUMMY_SP; @@ -35,11 +35,11 @@ declare_clippy_lint! { "common metadata is defined in `Cargo.toml`" } -fn warning(cx: &EarlyContext<'_>, message: &str) { +fn warning(cx: &LateContext<'_, '_>, message: &str) { span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, message); } -fn missing_warning(cx: &EarlyContext<'_>, package: &cargo_metadata::Package, field: &str) { +fn missing_warning(cx: &LateContext<'_, '_>, package: &cargo_metadata::Package, field: &str) { let message = format!("package `{}` is missing `{}` metadata", package.name, field); warning(cx, &message); } @@ -59,8 +59,12 @@ fn is_empty_vec(value: &[String]) -> bool { declare_lint_pass!(CargoCommonMetadata => [CARGO_COMMON_METADATA]); -impl EarlyLintPass for CargoCommonMetadata { - fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &Crate) { +impl LateLintPass<'_, '_> for CargoCommonMetadata { + fn check_crate(&mut self, cx: &LateContext<'_, '_>, _: &Crate<'_>) { + if !run_lints(cx, &[CARGO_COMMON_METADATA], CRATE_HIR_ID) { + return; + } + let metadata = if let Ok(metadata) = cargo_metadata::MetadataCommand::new().no_deps().exec() { metadata } else { diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs index 93a394b79e55..3ba72e84fa82 100644 --- a/clippy_lints/src/cognitive_complexity.rs +++ b/clippy_lints/src/cognitive_complexity.rs @@ -105,6 +105,7 @@ impl CognitiveComplexity { rust_cc, self.limit.limit() ), + None, "you could split it up into multiple smaller functions", ); } @@ -122,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/comparison_chain.rs b/clippy_lints/src/comparison_chain.rs index e0c381ddbfc4..96df3ffe3ce6 100644 --- a/clippy_lints/src/comparison_chain.rs +++ b/clippy_lints/src/comparison_chain.rs @@ -104,6 +104,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ComparisonChain { COMPARISON_CHAIN, expr.span, "`if` chain can be rewritten with `match`", + None, "Consider rewriting the `if` chain to use `cmp` and `match`.", ) } diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index b91607129158..7916996e990d 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -351,9 +351,9 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { let index = self.expr(index); match (lhs, index) { - (Some(Constant::Vec(vec)), Some(Constant::Int(index))) => match vec[index as usize] { - Constant::F32(x) => Some(Constant::F32(x)), - Constant::F64(x) => Some(Constant::F64(x)), + (Some(Constant::Vec(vec)), Some(Constant::Int(index))) => match vec.get(index as usize) { + Some(Constant::F32(x)) => Some(Constant::F32(*x)), + Some(Constant::F64(x)) => Some(Constant::F64(*x)), _ => None, }, (Some(Constant::Vec(vec)), _) => { diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 1afd401ca681..66722786eab4 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -183,7 +183,7 @@ fn lint_same_then_else(cx: &LateContext<'_, '_>, blocks: &[&Block<'_>]) { IF_SAME_THEN_ELSE, j.span, "this `if` has identical blocks", - i.span, + Some(i.span), "same as this", ); } @@ -206,7 +206,7 @@ fn lint_same_cond(cx: &LateContext<'_, '_>, conds: &[&Expr<'_>]) { IFS_SAME_COND, j.span, "this `if` has the same condition as a previous `if`", - i.span, + Some(i.span), "same as this", ); } @@ -234,7 +234,7 @@ fn lint_same_fns_in_if_cond(cx: &LateContext<'_, '_>, conds: &[&Expr<'_>]) { SAME_FUNCTIONS_IN_IF_CONDITION, j.span, "this `if` has the same function call as a previous `if`", - i.span, + Some(i.span), "same as this", ); } diff --git a/clippy_lints/src/copy_iterator.rs b/clippy_lints/src/copy_iterator.rs index 3e2d5b88e7b7..d79aa2ef0209 100644 --- a/clippy_lints/src/copy_iterator.rs +++ b/clippy_lints/src/copy_iterator.rs @@ -46,7 +46,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyIterator { COPY_ITERATOR, item.span, "you are implementing `Iterator` on a `Copy` type", - item.span, + None, "consider implementing `IntoIterator` instead", ); } diff --git a/clippy_lints/src/dbg_macro.rs b/clippy_lints/src/dbg_macro.rs index f9bf1141a8b2..e513dcce64e5 100644 --- a/clippy_lints/src/dbg_macro.rs +++ b/clippy_lints/src/dbg_macro.rs @@ -48,6 +48,7 @@ impl EarlyLintPass for DbgMacro { DBG_MACRO, mac.span(), "`dbg!` macro is intended as a debugging tool", + None, "ensure to avoid having uses of it in version control", ); } diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index 015fd9ed59f4..3cbb8fa72f74 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -1,8 +1,15 @@ use crate::utils::paths; -use crate::utils::{is_automatically_derived, is_copy, match_path, span_lint_and_then}; +use crate::utils::{ + is_automatically_derived, is_copy, match_path, span_lint_and_help, span_lint_and_note, span_lint_and_then, +}; use if_chain::if_chain; -use rustc_hir::{Item, ItemKind, TraitRef}; +use rustc_hir::def_id::DefId; +use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, NestedVisitorMap, Visitor}; +use rustc_hir::{ + BlockCheckMode, BodyId, Expr, ExprKind, FnDecl, HirId, Item, ItemKind, TraitRef, UnsafeSource, Unsafety, +}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::map::Map; use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; @@ -62,7 +69,41 @@ declare_clippy_lint! { "implementing `Clone` explicitly on `Copy` types" } -declare_lint_pass!(Derive => [EXPL_IMPL_CLONE_ON_COPY, DERIVE_HASH_XOR_EQ]); +declare_clippy_lint! { + /// **What it does:** Checks for deriving `serde::Deserialize` on a type that + /// has methods using `unsafe`. + /// + /// **Why is this bad?** Deriving `serde::Deserialize` will create a constructor + /// that may violate invariants hold by another constructor. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust,ignore + /// use serde::Deserialize; + /// + /// #[derive(Deserialize)] + /// pub struct Foo { + /// // .. + /// } + /// + /// impl Foo { + /// pub fn new() -> Self { + /// // setup here .. + /// } + /// + /// pub unsafe fn parts() -> (&str, &str) { + /// // assumes invariants hold + /// } + /// } + /// ``` + pub UNSAFE_DERIVE_DESERIALIZE, + pedantic, + "deriving `serde::Deserialize` on a type that has methods using `unsafe`" +} + +declare_lint_pass!(Derive => [EXPL_IMPL_CLONE_ON_COPY, DERIVE_HASH_XOR_EQ, UNSAFE_DERIVE_DESERIALIZE]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Derive { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) { @@ -76,7 +117,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Derive { check_hash_peq(cx, item.span, trait_ref, ty, is_automatically_derived); - if !is_automatically_derived { + if is_automatically_derived { + check_unsafe_derive_deserialize(cx, item, trait_ref, ty); + } else { check_copy_clone(cx, item, trait_ref, ty); } } @@ -117,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(node_id) = cx.tcx.hir().as_local_hir_id(impl_id) { - diag.span_note( - cx.tcx.hir().span(node_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" + ); + } } - }); + ); } }); } @@ -163,14 +210,100 @@ fn check_copy_clone<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, item: &Item<'_>, trait _ => (), } - span_lint_and_then( + span_lint_and_note( cx, EXPL_IMPL_CLONE_ON_COPY, item.span, "you are implementing `Clone` explicitly on a `Copy` type", - |diag| { - diag.span_note(item.span, "consider deriving `Clone` or removing `Copy`"); - }, + Some(item.span), + "consider deriving `Clone` or removing `Copy`", ); } } + +/// Implementation of the `UNSAFE_DERIVE_DESERIALIZE` lint. +fn check_unsafe_derive_deserialize<'a, 'tcx>( + cx: &LateContext<'a, 'tcx>, + item: &Item<'_>, + trait_ref: &TraitRef<'_>, + 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.expect_local()); + cx.tcx.hir().expect_item(hir_id) + } + + fn has_unsafe<'tcx>(cx: &LateContext<'_, 'tcx>, item: &'tcx Item<'_>) -> bool { + let mut visitor = UnsafeVisitor { cx, has_unsafe: false }; + walk_item(&mut visitor, item); + visitor.has_unsafe + } + + if_chain! { + if match_path(&trait_ref.path, &paths::SERDE_DESERIALIZE); + if let ty::Adt(def, _) = ty.kind; + if def.did.is_local(); + if cx.tcx.inherent_impls(def.did) + .iter() + .map(|imp_did| item_from_def_id(cx, *imp_did)) + .any(|imp| has_unsafe(cx, imp)); + then { + span_lint_and_help( + cx, + UNSAFE_DERIVE_DESERIALIZE, + item.span, + "you are deriving `serde::Deserialize` on a type that has methods using `unsafe`", + None, + "consider implementing `serde::Deserialize` manually. See https://serde.rs/impl-deserialize.html" + ); + } + } +} + +struct UnsafeVisitor<'a, 'tcx> { + cx: &'a LateContext<'a, 'tcx>, + has_unsafe: bool, +} + +impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> { + type Map = Map<'tcx>; + + fn visit_fn(&mut self, kind: FnKind<'tcx>, decl: &'tcx FnDecl<'_>, body_id: BodyId, span: Span, id: HirId) { + if self.has_unsafe { + return; + } + + if_chain! { + if let Some(header) = kind.header(); + if let Unsafety::Unsafe = header.unsafety; + then { + self.has_unsafe = true; + } + } + + walk_fn(self, kind, decl, body_id, span, id); + } + + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + if self.has_unsafe { + return; + } + + if let ExprKind::Block(block, _) = expr.kind { + match block.rules { + BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) + | BlockCheckMode::PushUnsafeBlock(UnsafeSource::UserProvided) + | BlockCheckMode::PopUnsafeBlock(UnsafeSource::UserProvided) => { + self.has_unsafe = true; + }, + _ => {}, + } + } + + walk_expr(self, expr); + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::All(self.cx.tcx.hir()) + } +} diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 926bd8ed001f..8d1e91f9adbd 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/drop_forget_ref.rs b/clippy_lints/src/drop_forget_ref.rs index 9a60b2f027ab..9de9056c1402 100644 --- a/clippy_lints/src/drop_forget_ref.rs +++ b/clippy_lints/src/drop_forget_ref.rs @@ -135,7 +135,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DropForgetRef { lint, expr.span, &msg, - arg.span, + Some(arg.span), &format!("argument has type `{}`", arg_ty)); } else if is_copy(cx, arg_ty) { if match_def_path(cx, def_id, &paths::DROP) { @@ -151,7 +151,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DropForgetRef { lint, expr.span, &msg, - arg.span, + Some(arg.span), &format!("argument has type {}", arg_ty)); } } diff --git a/clippy_lints/src/else_if_without_else.rs b/clippy_lints/src/else_if_without_else.rs index fb10ca48074e..95123e6ff6fe 100644 --- a/clippy_lints/src/else_if_without_else.rs +++ b/clippy_lints/src/else_if_without_else.rs @@ -61,6 +61,7 @@ impl EarlyLintPass for ElseIfWithoutElse { ELSE_IF_WITHOUT_ELSE, els.span, "`if` expression with an `else if`, but without a final `else`", + None, "add an `else` block here", ); } diff --git a/clippy_lints/src/empty_enum.rs b/clippy_lints/src/empty_enum.rs index 77ae6dbde72b..3bfef6f4bed1 100644 --- a/clippy_lints/src/empty_enum.rs +++ b/clippy_lints/src/empty_enum.rs @@ -1,6 +1,6 @@ //! lint when there is an enum with no variants -use crate::utils::span_lint_and_then; +use crate::utils::span_lint_and_help; use rustc_hir::{Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -45,13 +45,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EmptyEnum { let ty = cx.tcx.type_of(did); let adt = ty.ty_adt_def().expect("already checked whether this is an enum"); if adt.variants.is_empty() { - span_lint_and_then(cx, EMPTY_ENUM, item.span, "enum with no variants", |diag| { - diag.span_help( - item.span, - "consider using the uninhabited type `!` (never type) or a wrapper \ - around it to introduce a type which can't be instantiated", - ); - }); + span_lint_and_help( + cx, + EMPTY_ENUM, + item.span, + "enum with no variants", + None, + "consider using the uninhabited type `!` (never type) or a wrapper \ + around it to introduce a type which can't be instantiated", + ); } } } diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/enum_variants.rs index 882020a7adc7..a5871cf0cd4d 100644 --- a/clippy_lints/src/enum_variants.rs +++ b/clippy_lints/src/enum_variants.rs @@ -206,6 +206,7 @@ fn check_variant( lint, span, &format!("All variants have the same {}fix: `{}`", what, value), + None, &format!( "remove the {}fixes and use full paths to \ the variants instead of glob imports", diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index 1ec60a0e6e67..6907e021a00b 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/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index 3d27d8d5c8ae..e3e1136b6769 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -7,7 +7,8 @@ use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use crate::utils::{ - implements_trait, is_adjusted, iter_input_pats, snippet_opt, span_lint_and_then, type_is_unsafe_function, + implements_trait, is_adjusted, iter_input_pats, snippet_opt, span_lint_and_sugg, span_lint_and_then, + type_is_unsafe_function, }; declare_clippy_lint! { @@ -131,14 +132,15 @@ fn check_closure(cx: &LateContext<'_, '_>, expr: &Expr<'_>) { if let Some(name) = get_ufcs_type_name(cx, method_def_id, &args[0]); then { - span_lint_and_then(cx, REDUNDANT_CLOSURE_FOR_METHOD_CALLS, expr.span, "redundant closure found", |diag| { - diag.span_suggestion( - expr.span, - "remove closure as shown", - format!("{}::{}", name, path.ident.name), - Applicability::MachineApplicable, - ); - }); + span_lint_and_sugg( + cx, + REDUNDANT_CLOSURE_FOR_METHOD_CALLS, + expr.span, + "redundant closure found", + "remove closure as shown", + format!("{}::{}", name, path.ident.name), + Applicability::MachineApplicable, + ); } ); } diff --git a/clippy_lints/src/eval_order_dependence.rs b/clippy_lints/src/eval_order_dependence.rs index 48b761260a55..5206266ccf2a 100644 --- a/clippy_lints/src/eval_order_dependence.rs +++ b/clippy_lints/src/eval_order_dependence.rs @@ -310,7 +310,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ReadVisitor<'a, 'tcx> { EVAL_ORDER_DEPENDENCE, expr.span, "unsequenced read of a variable", - self.write_expr.span, + Some(self.write_expr.span), "whether read occurs before this write depends on evaluation order" ); } diff --git a/clippy_lints/src/excessive_bools.rs b/clippy_lints/src/excessive_bools.rs index ddbc3c377a27..82ca4baacb7a 100644 --- a/clippy_lints/src/excessive_bools.rs +++ b/clippy_lints/src/excessive_bools.rs @@ -114,6 +114,7 @@ impl ExcessiveBools { FN_PARAMS_EXCESSIVE_BOOLS, span, &format!("more than {} bools in function parameters", self.max_fn_params_bools), + None, "consider refactoring bools into two-variant enums", ); } @@ -153,6 +154,7 @@ impl EarlyLintPass for ExcessiveBools { STRUCT_EXCESSIVE_BOOLS, item.span, &format!("more than {} bools in a struct", self.max_struct_bools), + None, "consider using a state machine or refactoring bools into two-variant enums", ); } diff --git a/clippy_lints/src/exit.rs b/clippy_lints/src/exit.rs index dc1126d751da..621d56185a9d 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/formatting.rs b/clippy_lints/src/formatting.rs index 8f5f82b0a2ce..eb4b7a826f2c 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -149,7 +149,7 @@ fn check_assign(cx: &EarlyContext<'_>, expr: &Expr) { really are doing `.. = ({op} ..)`", op = op ), - eqop_span, + None, &format!("to remove this lint, use either `{op}=` or `= {op}`", op = op), ); } @@ -188,6 +188,7 @@ fn check_unop(cx: &EarlyContext<'_>, expr: &Expr) { binop = binop_str, unop = unop_str ), + None, &format!( "put a space between `{binop}` and `{unop}` and remove the space after `{unop}`", binop = binop_str, @@ -226,7 +227,7 @@ fn check_else(cx: &EarlyContext<'_>, expr: &Expr) { SUSPICIOUS_ELSE_FORMATTING, else_span, &format!("this is an `else {}` but the formatting might hide it", else_desc), - else_span, + None, &format!( "to remove this lint, remove the `else` or remove the new line between \ `else` and `{}`", @@ -265,7 +266,7 @@ fn check_array(cx: &EarlyContext<'_>, expr: &Expr) { POSSIBLE_MISSING_COMMA, lint_span, "possibly missing a comma here", - lint_span, + None, "to remove this lint, add a comma or write the expr in a single line", ); } @@ -296,7 +297,7 @@ fn check_missing_else(cx: &EarlyContext<'_>, first: &Expr, second: &Expr) { SUSPICIOUS_ELSE_FORMATTING, else_span, &format!("this looks like {} but the `else` is missing", looks_like), - else_span, + None, &format!( "to remove this lint, add the missing `else` or add a new line before {}", next_thing, diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index 7100bad996cd..c8c562fe29f5 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -431,6 +431,7 @@ fn check_needless_must_use( DOUBLE_MUST_USE, fn_header_span, "this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]`", + None, "either add some descriptive text or remove the attribute", ); } diff --git a/clippy_lints/src/future_not_send.rs b/clippy_lints/src/future_not_send.rs index 57f47bc9bc93..704a95ec0a09 100644 --- a/clippy_lints/src/future_not_send.rs +++ b/clippy_lints/src/future_not_send.rs @@ -60,10 +60,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FutureNotSend { if let FnKind::Closure(_) = kind { return; } - let def_id = cx.tcx.hir().local_def_id(hir_id); - let fn_sig = cx.tcx.fn_sig(def_id); - let fn_sig = cx.tcx.erase_late_bound_regions(&fn_sig); - let ret_ty = fn_sig.output(); + let ret_ty = utils::return_ty(cx, hir_id); if let Opaque(id, subst) = ret_ty.kind { let preds = cx.tcx.predicates_of(id).instantiate(cx.tcx, subst); let mut is_future = false; diff --git a/clippy_lints/src/identity_conversion.rs b/clippy_lints/src/identity_conversion.rs index 4b7c2c4156ec..33a9478f0588 100644 --- a/clippy_lints/src/identity_conversion.rs +++ b/clippy_lints/src/identity_conversion.rs @@ -1,5 +1,5 @@ use crate::utils::{ - match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_then, + match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_sugg, }; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, HirId, MatchSource}; @@ -58,14 +58,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { if same_tys(cx, a, b) { let sugg = snippet_with_macro_callsite(cx, args[0].span, "").to_string(); - span_lint_and_then(cx, IDENTITY_CONVERSION, e.span, "identical conversion", |diag| { - diag.span_suggestion( - e.span, - "consider removing `.into()`", - sugg, - Applicability::MachineApplicable, // snippet - ); - }); + span_lint_and_sugg( + cx, + IDENTITY_CONVERSION, + e.span, + "identical conversion", + "consider removing `.into()`", + sugg, + Applicability::MachineApplicable, // snippet + ); } } if match_trait_method(cx, e, &paths::INTO_ITERATOR) && &*name.ident.as_str() == "into_iter" { @@ -73,14 +74,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { let b = cx.tables.expr_ty(&args[0]); if same_tys(cx, a, b) { let sugg = snippet(cx, args[0].span, "").into_owned(); - span_lint_and_then(cx, IDENTITY_CONVERSION, e.span, "identical conversion", |diag| { - diag.span_suggestion( - e.span, - "consider removing `.into_iter()`", - sugg, - Applicability::MachineApplicable, // snippet - ); - }); + span_lint_and_sugg( + cx, + IDENTITY_CONVERSION, + e.span, + "identical conversion", + "consider removing `.into_iter()`", + sugg, + Applicability::MachineApplicable, // snippet + ); } } }, @@ -95,14 +97,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { let sugg = snippet(cx, args[0].span.source_callsite(), "").into_owned(); let sugg_msg = format!("consider removing `{}()`", snippet(cx, path.span, "From::from")); - span_lint_and_then(cx, IDENTITY_CONVERSION, e.span, "identical conversion", |diag| { - diag.span_suggestion( - e.span, - &sugg_msg, - sugg, - Applicability::MachineApplicable, // snippet - ); - }); + span_lint_and_sugg( + cx, + IDENTITY_CONVERSION, + e.span, + "identical conversion", + &sugg_msg, + sugg, + Applicability::MachineApplicable, // snippet + ); } } } diff --git a/clippy_lints/src/if_let_mutex.rs b/clippy_lints/src/if_let_mutex.rs new file mode 100644 index 000000000000..b2ece37fdb0a --- /dev/null +++ b/clippy_lints/src/if_let_mutex.rs @@ -0,0 +1,160 @@ +use crate::utils::{match_type, paths, span_lint_and_help, SpanlessEq}; +use if_chain::if_chain; +use rustc_hir::intravisit::{self as visit, NestedVisitorMap, Visitor}; +use rustc_hir::{Expr, ExprKind, MatchSource}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::map::Map; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** Checks for `Mutex::lock` calls in `if let` expression + /// with lock calls in any of the else blocks. + /// + /// **Why is this bad?** The Mutex lock remains held for the whole + /// `if let ... else` block and deadlocks. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust,ignore + /// if let Ok(thing) = mutex.lock() { + /// do_thing(); + /// } else { + /// mutex.lock(); + /// } + /// ``` + /// Should be written + /// ```rust,ignore + /// let locked = mutex.lock(); + /// if let Ok(thing) = locked { + /// do_thing(thing); + /// } else { + /// use_locked(locked); + /// } + /// ``` + pub IF_LET_MUTEX, + correctness, + "locking a `Mutex` in an `if let` block can cause deadlocks" +} + +declare_lint_pass!(IfLetMutex => [IF_LET_MUTEX]); + +impl LateLintPass<'_, '_> for IfLetMutex { + fn check_expr(&mut self, cx: &LateContext<'_, '_>, ex: &'_ Expr<'_>) { + let mut arm_visit = ArmVisitor { + mutex_lock_called: false, + found_mutex: None, + cx, + }; + let mut op_visit = OppVisitor { + mutex_lock_called: false, + found_mutex: None, + cx, + }; + if let ExprKind::Match( + ref op, + ref arms, + MatchSource::IfLetDesugar { + contains_else_clause: true, + }, + ) = ex.kind + { + op_visit.visit_expr(op); + if op_visit.mutex_lock_called { + for arm in *arms { + arm_visit.visit_arm(arm); + } + + if arm_visit.mutex_lock_called && arm_visit.same_mutex(cx, op_visit.found_mutex.unwrap()) { + span_lint_and_help( + cx, + IF_LET_MUTEX, + ex.span, + "calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock", + None, + "move the lock call outside of the `if let ...` expression", + ); + } + } + } + } +} + +/// Checks if `Mutex::lock` is called in the `if let _ = expr. +pub struct OppVisitor<'tcx, 'l> { + mutex_lock_called: bool, + found_mutex: Option<&'tcx Expr<'tcx>>, + cx: &'tcx LateContext<'tcx, 'l>, +} + +impl<'tcx, 'l> Visitor<'tcx> for OppVisitor<'tcx, 'l> { + type Map = Map<'tcx>; + + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + if_chain! { + if let Some(mutex) = is_mutex_lock_call(self.cx, expr); + then { + self.found_mutex = Some(mutex); + self.mutex_lock_called = true; + return; + } + } + visit::walk_expr(self, expr); + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } +} + +/// Checks if `Mutex::lock` is called in any of the branches. +pub struct ArmVisitor<'tcx, 'l> { + mutex_lock_called: bool, + found_mutex: Option<&'tcx Expr<'tcx>>, + cx: &'tcx LateContext<'tcx, 'l>, +} + +impl<'tcx, 'l> Visitor<'tcx> for ArmVisitor<'tcx, 'l> { + type Map = Map<'tcx>; + + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + if_chain! { + if let Some(mutex) = is_mutex_lock_call(self.cx, expr); + then { + self.found_mutex = Some(mutex); + self.mutex_lock_called = true; + return; + } + } + visit::walk_expr(self, expr); + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } +} + +impl<'tcx, 'l> ArmVisitor<'tcx, 'l> { + fn same_mutex(&self, cx: &LateContext<'_, '_>, op_mutex: &Expr<'_>) -> bool { + if let Some(arm_mutex) = self.found_mutex { + SpanlessEq::new(cx).eq_expr(op_mutex, arm_mutex) + } else { + false + } + } +} + +fn is_mutex_lock_call<'a>(cx: &LateContext<'a, '_>, expr: &'a Expr<'_>) -> Option<&'a Expr<'a>> { + if_chain! { + if let ExprKind::MethodCall(path, _span, args) = &expr.kind; + if path.ident.to_string() == "lock"; + let ty = cx.tables.expr_ty(&args[0]); + if match_type(cx, ty, &paths::MUTEX); + then { + Some(&args[0]) + } else { + None + } + } +} diff --git a/clippy_lints/src/if_not_else.rs b/clippy_lints/src/if_not_else.rs index 271df5b03e38..c11e291f98e4 100644 --- a/clippy_lints/src/if_not_else.rs +++ b/clippy_lints/src/if_not_else.rs @@ -61,6 +61,7 @@ impl EarlyLintPass for IfNotElse { IF_NOT_ELSE, item.span, "Unnecessary boolean `not` operation", + None, "remove the `!` and swap the blocks of the `if`/`else`", ); }, @@ -70,6 +71,7 @@ impl EarlyLintPass for IfNotElse { IF_NOT_ELSE, item.span, "Unnecessary `!=` operation", + None, "change to `==` and swap the blocks of the `if`/`else`", ); }, diff --git a/clippy_lints/src/indexing_slicing.rs b/clippy_lints/src/indexing_slicing.rs index a2b1085a36e6..c5808dd540b6 100644 --- a/clippy_lints/src/indexing_slicing.rs +++ b/clippy_lints/src/indexing_slicing.rs @@ -138,7 +138,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing { (None, None) => return, // [..] is ok. }; - span_lint_and_help(cx, INDEXING_SLICING, expr.span, "slicing may panic.", help_msg); + span_lint_and_help(cx, INDEXING_SLICING, expr.span, "slicing may panic.", None, help_msg); } else { // Catchall non-range index, i.e., [n] or [n << m] if let ty::Array(..) = ty.kind { @@ -154,6 +154,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing { INDEXING_SLICING, expr.span, "indexing may panic.", + None, "Consider using `.get(n)` or `.get_mut(n)` instead", ); } diff --git a/clippy_lints/src/inherent_to_string.rs b/clippy_lints/src/inherent_to_string.rs index ca8e834e347c..e343d690f6cd 100644 --- a/clippy_lints/src/inherent_to_string.rs +++ b/clippy_lints/src/inherent_to_string.rs @@ -137,6 +137,7 @@ fn show_lint(cx: &LateContext<'_, '_>, item: &ImplItem<'_>) { "type `{}` implements inherent method `to_string(&self) -> String` which shadows the implementation of `Display`", self_type.to_string() ), + None, &format!("remove the inherent method from type `{}`", self_type.to_string()) ); } else { @@ -148,6 +149,7 @@ fn show_lint(cx: &LateContext<'_, '_>, item: &ImplItem<'_>) { "implementation of inherent method `to_string(&self) -> String` for type `{}`", self_type.to_string() ), + None, &format!("implement trait `Display` for type `{}` instead", self_type.to_string()), ); } diff --git a/clippy_lints/src/int_plus_one.rs b/clippy_lints/src/int_plus_one.rs index 2392ad4d7a12..d5dbd495680b 100644 --- a/clippy_lints/src/int_plus_one.rs +++ b/clippy_lints/src/int_plus_one.rs @@ -5,7 +5,7 @@ use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use crate::utils::{snippet_opt, span_lint_and_then}; +use crate::utils::{snippet_opt, span_lint_and_sugg}; declare_clippy_lint! { /// **What it does:** Checks for usage of `x >= y + 1` or `x - 1 >= y` (and `<=`) in a block @@ -149,19 +149,14 @@ impl IntPlusOne { } fn emit_warning(cx: &EarlyContext<'_>, block: &Expr, recommendation: String) { - span_lint_and_then( + span_lint_and_sugg( cx, INT_PLUS_ONE, block.span, "Unnecessary `>= y + 1` or `x - 1 >=`", - |diag| { - diag.span_suggestion( - block.span, - "change it to", - recommendation, - Applicability::MachineApplicable, // snippet - ); - }, + "change it to", + recommendation, + Applicability::MachineApplicable, // snippet ); } } diff --git a/clippy_lints/src/integer_division.rs b/clippy_lints/src/integer_division.rs index 053d66e6af74..fe34d33fe652 100644 --- a/clippy_lints/src/integer_division.rs +++ b/clippy_lints/src/integer_division.rs @@ -35,6 +35,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IntegerDivision { INTEGER_DIVISION, expr.span, "integer division", + None, "division of integers may cause loss of precision. consider using floats.", ); } diff --git a/clippy_lints/src/large_stack_arrays.rs b/clippy_lints/src/large_stack_arrays.rs index f67fce9697af..deb57db16789 100644 --- a/clippy_lints/src/large_stack_arrays.rs +++ b/clippy_lints/src/large_stack_arrays.rs @@ -57,6 +57,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeStackArrays { "allocating a local array larger than {} bytes", self.maximum_allowed_size ), + None, &format!( "consider allocating on the heap with `vec!{}.into_boxed_slice()`", snippet(cx, expr.span, "[...]") diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 5d94013cb65d..1d86ca9696f2 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/let_underscore.rs b/clippy_lints/src/let_underscore.rs index f8f84f3d42d0..710dec8d33fc 100644 --- a/clippy_lints/src/let_underscore.rs +++ b/clippy_lints/src/let_underscore.rs @@ -90,6 +90,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore { LET_UNDERSCORE_LOCK, local.span, "non-binding let on a synchronization lock", + None, "consider using an underscore-prefixed named \ binding or dropping explicitly with `std::mem::drop`" ) @@ -99,6 +100,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore { LET_UNDERSCORE_MUST_USE, local.span, "non-binding let on an expression with `#[must_use]` type", + None, "consider explicitly using expression value" ) } else if is_must_use_func_call(cx, init) { @@ -107,6 +109,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore { LET_UNDERSCORE_MUST_USE, local.span, "non-binding let on a result of a `#[must_use]` function", + None, "consider explicitly using function result" ) } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b8415fa3af12..ac867cc4e4af 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -177,6 +177,7 @@ mod assertions_on_constants; mod assign_ops; mod atomic_ordering; mod attrs; +mod await_holding_lock; mod bit_mask; mod blacklisted_name; mod block_in_if_condition; @@ -222,6 +223,7 @@ mod future_not_send; mod get_last_with_len; mod identity_conversion; mod identity_op; +mod if_let_mutex; mod if_let_some_result; mod if_not_else; mod implicit_return; @@ -496,6 +498,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &attrs::INLINE_ALWAYS, &attrs::UNKNOWN_CLIPPY_LINTS, &attrs::USELESS_ATTRIBUTE, + &await_holding_lock::AWAIT_HOLDING_LOCK, &bit_mask::BAD_BIT_MASK, &bit_mask::INEFFECTIVE_BIT_MASK, &bit_mask::VERBOSE_BIT_MASK, @@ -520,6 +523,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &dereference::EXPLICIT_DEREF_METHODS, &derive::DERIVE_HASH_XOR_EQ, &derive::EXPL_IMPL_CLONE_ON_COPY, + &derive::UNSAFE_DERIVE_DESERIALIZE, &doc::DOC_MARKDOWN, &doc::MISSING_ERRORS_DOC, &doc::MISSING_SAFETY_DOC, @@ -572,6 +576,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &get_last_with_len::GET_LAST_WITH_LEN, &identity_conversion::IDENTITY_CONVERSION, &identity_op::IDENTITY_OP, + &if_let_mutex::IF_LET_MUTEX, &if_let_some_result::IF_LET_SOME_RESULT, &if_not_else::IF_NOT_ELSE, &implicit_return::IMPLICIT_RETURN, @@ -838,6 +843,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &unwrap::UNNECESSARY_UNWRAP, &use_self::USE_SELF, &utils::internal_lints::CLIPPY_LINTS_INTERNAL, + &utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS, &utils::internal_lints::COMPILER_LINT_FUNCTIONS, &utils::internal_lints::DEFAULT_LINT, &utils::internal_lints::LINT_WITHOUT_LINT_PASS, @@ -860,6 +866,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ]); // end register lints, do not remove this comment, it’s used in `update_lints` + store.register_late_pass(|| box await_holding_lock::AwaitHoldingLock); store.register_late_pass(|| box serde_api::SerdeAPI); store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new()); store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default()); @@ -1017,9 +1024,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| box precedence::Precedence); store.register_early_pass(|| box needless_continue::NeedlessContinue); store.register_early_pass(|| box redundant_static_lifetimes::RedundantStaticLifetimes); - store.register_early_pass(|| box cargo_common_metadata::CargoCommonMetadata); - store.register_early_pass(|| box multiple_crate_versions::MultipleCrateVersions); - store.register_early_pass(|| box wildcard_dependencies::WildcardDependencies); + store.register_late_pass(|| box cargo_common_metadata::CargoCommonMetadata); + store.register_late_pass(|| box multiple_crate_versions::MultipleCrateVersions); + store.register_late_pass(|| box wildcard_dependencies::WildcardDependencies); store.register_early_pass(|| box literal_representation::LiteralDigitGrouping); let literal_representation_threshold = conf.literal_representation_threshold; store.register_early_pass(move || box literal_representation::DecimalLiteralRepresentation::new(literal_representation_threshold)); @@ -1051,6 +1058,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box unnamed_address::UnnamedAddress); store.register_late_pass(|| box dereference::Dereferencing); store.register_late_pass(|| box future_not_send::FutureNotSend); + store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls); + store.register_late_pass(|| box if_let_mutex::IfLetMutex); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1096,6 +1105,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![ LintId::of(&attrs::INLINE_ALWAYS), + LintId::of(&await_holding_lock::AWAIT_HOLDING_LOCK), LintId::of(&checked_conversions::CHECKED_CONVERSIONS), LintId::of(&copies::MATCH_SAME_ARMS), LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION), @@ -1103,6 +1113,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS), LintId::of(&dereference::EXPLICIT_DEREF_METHODS), LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY), + LintId::of(&derive::UNSAFE_DERIVE_DESERIALIZE), LintId::of(&doc::DOC_MARKDOWN), LintId::of(&doc::MISSING_ERRORS_DOC), LintId::of(&empty_enum::EMPTY_ENUM), @@ -1123,6 +1134,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP), LintId::of(&loops::EXPLICIT_ITER_LOOP), LintId::of(¯o_use::MACRO_USE_IMPORTS), + LintId::of(&matches::MATCH_BOOL), LintId::of(&matches::SINGLE_MATCH_ELSE), LintId::of(&methods::FILTER_MAP), LintId::of(&methods::FILTER_MAP_NEXT), @@ -1162,6 +1174,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_group(true, "clippy::internal", Some("clippy_internal"), vec![ LintId::of(&utils::internal_lints::CLIPPY_LINTS_INTERNAL), + LintId::of(&utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS), LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS), LintId::of(&utils::internal_lints::DEFAULT_LINT), LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS), @@ -1228,6 +1241,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&get_last_with_len::GET_LAST_WITH_LEN), LintId::of(&identity_conversion::IDENTITY_CONVERSION), LintId::of(&identity_op::IDENTITY_OP), + LintId::of(&if_let_mutex::IF_LET_MUTEX), LintId::of(&if_let_some_result::IF_LET_SOME_RESULT), LintId::of(&indexing_slicing::OUT_OF_BOUNDS_INDEXING), LintId::of(&infinite_iter::INFINITE_ITER), @@ -1266,7 +1280,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN), LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(&matches::MATCH_AS_REF), - LintId::of(&matches::MATCH_BOOL), LintId::of(&matches::MATCH_OVERLAPPING_ARM), LintId::of(&matches::MATCH_REF_PATS), LintId::of(&matches::MATCH_SINGLE_BINDING), @@ -1457,7 +1470,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&main_recursion::MAIN_RECURSION), LintId::of(&map_clone::MAP_CLONE), LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH), - LintId::of(&matches::MATCH_BOOL), LintId::of(&matches::MATCH_OVERLAPPING_ARM), LintId::of(&matches::MATCH_REF_PATS), LintId::of(&matches::MATCH_WILD_ERR_ARM), @@ -1615,6 +1627,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&erasing_op::ERASING_OP), LintId::of(&formatting::POSSIBLE_MISSING_COMMA), LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF), + LintId::of(&if_let_mutex::IF_LET_MUTEX), LintId::of(&indexing_slicing::OUT_OF_BOUNDS_INDEXING), LintId::of(&infinite_iter::INFINITE_ITER), LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY), diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 345ae53f845a..6e9b7f685eb1 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -566,6 +566,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops { ) = (pat, &match_expr.kind) { let iter_expr = &method_args[0]; + + // Don't lint when the iterator is recreated on every iteration + if_chain! { + if let ExprKind::MethodCall(..) | ExprKind::Call(..) = iter_expr.kind; + if let Some(iter_def_id) = get_trait_def_id(cx, &paths::ITERATOR); + if implements_trait(cx, cx.tables.expr_ty(iter_expr), iter_def_id, &[]); + then { + return; + } + } + let lhs_constructor = last_path_segment(qpath); if method_path.ident.name == sym!(next) && match_trait_method(cx, match_expr, &paths::ITERATOR) @@ -576,20 +587,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, ); } } @@ -1402,6 +1414,7 @@ fn check_arg_type(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>) { `if let` statement.", snippet(cx, arg.span, "_") ), + None, &format!( "consider replacing `for {0} in {1}` with `if let Some({0}) = {1}`", snippet(cx, pat.span, "_"), @@ -1418,6 +1431,7 @@ fn check_arg_type(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>) { `if let` statement.", snippet(cx, arg.span, "_") ), + None, &format!( "consider replacing `for {0} in {1}` with `if let Ok({0}) = {1}`", snippet(cx, pat.span, "_"), @@ -1652,14 +1666,14 @@ fn check_for_mutability(cx: &LateContext<'_, '_>, bound: &Expr<'_>) -> Option Visitor<'tcx> for InitializeVisitor<'a, 'tcx> { fn var_def_id(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> Option { if let ExprKind::Path(ref qpath) = expr.kind { let path_res = qpath_res(cx, qpath, expr.hir_id); - if let Res::Local(node_id) = path_res { - return Some(node_id); + if let Res::Local(hir_id) = path_res { + return Some(hir_id); } } None @@ -2422,8 +2436,8 @@ impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> { let res = qpath_res(self.cx, qpath, ex.hir_id); then { match res { - Res::Local(node_id) => { - self.ids.insert(node_id); + Res::Local(hir_id) => { + self.ids.insert(hir_id); }, Res::Def(DefKind::Static, def_id) => { let mutable = self.cx.tcx.is_mutable_static(def_id); @@ -2471,45 +2485,53 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, ' match_type(cx, ty, &paths::HASHMAP) { if method.ident.name == sym!(len) { let span = shorten_needless_collect_span(expr); - span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |diag| { - diag.span_suggestion( - span, - "replace with", - ".count()".to_string(), - Applicability::MachineApplicable, - ); - }); + span_lint_and_sugg( + cx, + NEEDLESS_COLLECT, + span, + NEEDLESS_COLLECT_MSG, + "replace with", + ".count()".to_string(), + Applicability::MachineApplicable, + ); } if method.ident.name == sym!(is_empty) { let span = shorten_needless_collect_span(expr); - span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |diag| { - diag.span_suggestion( - span, - "replace with", - ".next().is_none()".to_string(), - Applicability::MachineApplicable, - ); - }); + span_lint_and_sugg( + cx, + NEEDLESS_COLLECT, + span, + NEEDLESS_COLLECT_MSG, + "replace with", + ".next().is_none()".to_string(), + Applicability::MachineApplicable, + ); } if method.ident.name == sym!(contains) { let contains_arg = snippet(cx, args[1].span, "??"); let span = shorten_needless_collect_span(expr); - span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |diag| { - let (arg, pred) = if contains_arg.starts_with('&') { - ("x", &contains_arg[1..]) - } else { - ("&x", &*contains_arg) - }; - diag.span_suggestion( - span, - "replace with", - format!( - ".any(|{}| x == {})", - arg, pred - ), - Applicability::MachineApplicable, - ); - }); + span_lint_and_then( + cx, + NEEDLESS_COLLECT, + span, + NEEDLESS_COLLECT_MSG, + |diag| { + let (arg, pred) = if contains_arg.starts_with('&') { + ("x", &contains_arg[1..]) + } else { + ("&x", &*contains_arg) + }; + diag.span_suggestion( + span, + "replace with", + format!( + ".any(|{}| x == {})", + arg, pred + ), + Applicability::MachineApplicable, + ); + } + ); } } } diff --git a/clippy_lints/src/main_recursion.rs b/clippy_lints/src/main_recursion.rs index 7854873509ea..8a0e47a3d31c 100644 --- a/clippy_lints/src/main_recursion.rs +++ b/clippy_lints/src/main_recursion.rs @@ -53,6 +53,7 @@ impl LateLintPass<'_, '_> for MainRecursion { MAIN_RECURSION, func.span, &format!("recursing into entrypoint `{}`", snippet(cx, func.span, "main")), + None, "consider using another function for this recursion" ) } diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index 5c5cf8015f40..0b346393ac38 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -7,6 +7,7 @@ use rustc_ast::ast::Ident; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::mir::Mutability; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; @@ -58,7 +59,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MapClone { let closure_expr = remove_blocks(&closure_body.value); then { match closure_body.params[0].pat.kind { - hir::PatKind::Ref(ref inner, _) => if let hir::PatKind::Binding( + hir::PatKind::Ref(ref inner, hir::Mutability::Not) => if let hir::PatKind::Binding( hir::BindingAnnotation::Unannotated, .., name, None ) = inner.kind { if ident_eq(name, closure_expr) { @@ -69,7 +70,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MapClone { match closure_expr.kind { hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner) => { if ident_eq(name, inner) { - if let ty::Ref(..) = cx.tables.expr_ty(inner).kind { + if let ty::Ref(.., Mutability::Not) = cx.tables.expr_ty(inner).kind { lint(cx, e.span, args[0].span, true); } } diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index a3a05fd1caa3..8f86535ef1e0 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -138,7 +138,7 @@ declare_clippy_lint! { /// } /// ``` pub MATCH_BOOL, - style, + pedantic, "a `match` on a boolean expression instead of an `if..else` block" } @@ -441,6 +441,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { REST_PAT_IN_FULLY_BOUND_STRUCTS, pat.span, "unnecessary use of `..` pattern in struct binding. All fields were already bound", + None, "consider removing `..` from this binding", ); } @@ -636,7 +637,7 @@ fn check_overlapping_arms<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ex: &'tcx Expr<' MATCH_OVERLAPPING_ARM, start.span, "some ranges overlap", - end.span, + Some(end.span), "overlaps with this", ); } @@ -674,7 +675,7 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) MATCH_WILD_ERR_ARM, arm.pat.span, &format!("`Err({})` matches all errors", &ident_bind_name), - arm.pat.span, + None, "match each error separately or use the error output", ); } @@ -887,6 +888,7 @@ fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, arms: &[Arm<'_>]) { WILDCARD_IN_OR_PATTERNS, arm.pat.span, "wildcard pattern covers any other pattern as it will match anyway.", + None, "Consider handling `_` separately.", ); } diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs index 69639f453233..ab6865bf0f3b 100644 --- a/clippy_lints/src/mem_replace.rs +++ b/clippy_lints/src/mem_replace.rs @@ -148,6 +148,7 @@ fn check_replace_with_uninit(cx: &LateContext<'_, '_>, src: &Expr<'_>, expr_span MEM_REPLACE_WITH_UNINIT, expr_span, "replacing with `mem::uninitialized()`", + None, "consider using the `take_mut` crate instead", ); } else if cx.tcx.is_diagnostic_item(sym::mem_zeroed, repl_def_id) && @@ -157,6 +158,7 @@ fn check_replace_with_uninit(cx: &LateContext<'_, '_>, src: &Expr<'_>, expr_span MEM_REPLACE_WITH_UNINIT, expr_span, "replacing with `mem::zeroed()`", + None, "consider using a default value or the `take_mut` crate instead", ); } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 2337380c7dd1..7c652229d33d 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2255,6 +2255,7 @@ fn lint_iter_nth<'a, 'tcx>( ITER_NTH, expr.span, &format!("called `.iter{0}().nth()` on a {1}", mut_str, caller_type), + None, &format!("calling `.get{}()` is both faster and more readable", mut_str), ); } @@ -2364,6 +2365,7 @@ fn lint_iter_skip_next(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>) { ITER_SKIP_NEXT, expr.span, "called `skip(x).next()` on an iterator", + None, "this is more succinctly expressed by calling `nth(x)`", ); } @@ -2431,6 +2433,7 @@ fn lint_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, unwrap_args: &[hi lint, expr.span, &format!("used `unwrap()` on `{}` value", kind,), + None, &format!( "if you don't want to handle the `{}` case gracefully, consider \ using `expect()` to provide a better panic message", @@ -2458,6 +2461,7 @@ fn lint_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, expect_args: &[hi lint, expr.span, &format!("used `expect()` on `{}` value", kind,), + None, &format!("if this value is an `{}`, it will panic", none_value,), ); } @@ -2478,6 +2482,7 @@ fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, ok_args: &[hir OK_EXPECT, expr.span, "called `ok().expect()` on a `Result` value", + None, "you can call `expect()` directly on the `Result`", ); } @@ -2572,7 +2577,7 @@ fn lint_map_unwrap_or_else<'a, 'tcx>( }, expr.span, msg, - expr.span, + None, &format!( "replace `map({0}).unwrap_or_else({1})` with `map_or_else({1}, {0})`", map_snippet, unwrap_snippet, @@ -2752,7 +2757,7 @@ fn lint_filter_next<'a, 'tcx>( FILTER_NEXT, expr.span, msg, - expr.span, + None, &format!("replace `filter({0}).next()` with `find({0})`", filter_snippet), ); } else { @@ -2774,6 +2779,7 @@ fn lint_skip_while_next<'a, 'tcx>( SKIP_WHILE_NEXT, expr.span, "called `skip_while(p).next()` on an `Iterator`", + None, "this is more succinctly expressed by calling `.find(!p)` instead", ); } @@ -2790,7 +2796,7 @@ fn lint_filter_map<'a, 'tcx>( if match_trait_method(cx, expr, &paths::ITERATOR) { let msg = "called `filter(p).map(q)` on an `Iterator`"; let hint = "this is more succinctly expressed by calling `.filter_map(..)` instead"; - span_lint_and_help(cx, FILTER_MAP, expr.span, msg, hint); + span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint); } } @@ -2810,7 +2816,7 @@ fn lint_filter_map_next<'a, 'tcx>( FILTER_MAP_NEXT, expr.span, msg, - expr.span, + None, &format!("replace `filter_map({0}).next()` with `find_map({0})`", filter_snippet), ); } else { @@ -2830,7 +2836,7 @@ fn lint_find_map<'a, 'tcx>( if match_trait_method(cx, &map_args[0], &paths::ITERATOR) { let msg = "called `find(p).map(q)` on an `Iterator`"; let hint = "this is more succinctly expressed by calling `.find_map(..)` instead"; - span_lint_and_help(cx, FIND_MAP, expr.span, msg, hint); + span_lint_and_help(cx, FIND_MAP, expr.span, msg, None, hint); } } @@ -2845,7 +2851,7 @@ fn lint_filter_map_map<'a, 'tcx>( if match_trait_method(cx, expr, &paths::ITERATOR) { let msg = "called `filter_map(p).map(q)` on an `Iterator`"; let hint = "this is more succinctly expressed by only calling `.filter_map(..)` instead"; - span_lint_and_help(cx, FILTER_MAP, expr.span, msg, hint); + span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint); } } @@ -2861,7 +2867,7 @@ fn lint_filter_flat_map<'a, 'tcx>( let msg = "called `filter(p).flat_map(q)` on an `Iterator`"; let hint = "this is more succinctly expressed by calling `.flat_map(..)` \ and filtering by returning `iter::empty()`"; - span_lint_and_help(cx, FILTER_MAP, expr.span, msg, hint); + span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint); } } @@ -2877,7 +2883,7 @@ fn lint_filter_map_flat_map<'a, 'tcx>( let msg = "called `filter_map(p).flat_map(q)` on an `Iterator`"; let hint = "this is more succinctly expressed by calling `.flat_map(..)` \ and filtering by returning `iter::empty()`"; - span_lint_and_help(cx, FILTER_MAP, expr.span, msg, hint); + span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint); } } @@ -3260,6 +3266,7 @@ fn lint_suspicious_map(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>) { SUSPICIOUS_MAP, expr.span, "this call to `map()` won't have an effect on the call to `count()`", + None, "make sure you did not confuse `map` with `filter` or `for_each`", ); } @@ -3640,7 +3647,7 @@ fn lint_filetype_is_file(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, args: & } let lint_msg = format!("`{}FileType::is_file()` only {} regular files", lint_unary, verb); let help_msg = format!("use `{}FileType::is_dir()` instead", help_unary); - span_lint_and_help(cx, FILETYPE_IS_FILE, span, &lint_msg, &help_msg); + span_lint_and_help(cx, FILETYPE_IS_FILE, span, &lint_msg, None, &help_msg); } fn fn_header_equals(expected: hir::FnHeader, actual: hir::FnHeader) -> bool { diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index e6f4be333e7f..c49d3ba5dece 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -14,7 +14,7 @@ use rustc_span::source_map::{ExpnKind, Span}; use crate::consts::{constant, Constant}; use crate::utils::sugg::Sugg; use crate::utils::{ - get_item_name, get_parent_expr, implements_trait, in_constant, is_integer_const, iter_input_pats, + get_item_name, get_parent_expr, higher, implements_trait, in_constant, is_integer_const, iter_input_pats, last_path_segment, match_qpath, match_trait_method, paths, snippet, snippet_opt, span_lint, span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then, walk_ptrs_ty, SpanlessEq, }; @@ -267,6 +267,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { if let StmtKind::Local(ref local) = stmt.kind; if let PatKind::Binding(an, .., name, None) = local.pat.kind; if let Some(ref init) = local.init; + if !higher::is_from_for_desugar(local); then { if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut { let sugg_init = if init.span.from_expansion() { diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index 75bbf0514c2d..adfd8dfb1c18 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -313,6 +313,7 @@ impl EarlyLintPass for MiscEarlyLints { UNNEEDED_FIELD_PATTERN, pat.span, "All the struct fields are matched to a wildcard pattern, consider using `..`.", + None, &format!("Try with `{} {{ .. }}` instead", type_name), ); return; @@ -348,6 +349,7 @@ impl EarlyLintPass for MiscEarlyLints { field.span, "You matched a field with a wildcard pattern. Consider using `..` \ instead", + None, &format!("Try with `{} {{ {}, .. }}`", type_name, normal[..].join(", ")), ); } diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs index 0b235bdfe3ca..4301157e1644 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 9fc26047d888..5300fd2215b3 100644 --- a/clippy_lints/src/missing_inline.rs +++ b/clippy_lints/src/missing_inline.rs @@ -152,8 +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_node_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/multiple_crate_versions.rs b/clippy_lints/src/multiple_crate_versions.rs index 88605c52f2e2..ed85d0315bd2 100644 --- a/clippy_lints/src/multiple_crate_versions.rs +++ b/clippy_lints/src/multiple_crate_versions.rs @@ -1,8 +1,8 @@ //! lint on multiple versions of a crate being used -use crate::utils::span_lint; -use rustc_ast::ast::Crate; -use rustc_lint::{EarlyContext, EarlyLintPass}; +use crate::utils::{run_lints, span_lint}; +use rustc_hir::{Crate, CRATE_HIR_ID}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::DUMMY_SP; @@ -33,8 +33,12 @@ declare_clippy_lint! { declare_lint_pass!(MultipleCrateVersions => [MULTIPLE_CRATE_VERSIONS]); -impl EarlyLintPass for MultipleCrateVersions { - fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &Crate) { +impl LateLintPass<'_, '_> for MultipleCrateVersions { + fn check_crate(&mut self, cx: &LateContext<'_, '_>, _: &Crate<'_>) { + if !run_lints(cx, &[MULTIPLE_CRATE_VERSIONS], CRATE_HIR_ID) { + return; + } + let metadata = if let Ok(metadata) = cargo_metadata::MetadataCommand::new().exec() { metadata } else { diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs index 6be4b1effeae..28183810df48 100644 --- a/clippy_lints/src/needless_continue.rs +++ b/clippy_lints/src/needless_continue.rs @@ -304,6 +304,7 @@ fn emit_warning<'a>(cx: &EarlyContext<'_>, data: &'a LintData<'_>, header: &str, NEEDLESS_CONTINUE, expr.span, message, + None, &format!("{}\n{}", header, snip), ); } diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index 32e8f37062af..28650c88b480 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 19e06ab66c42..a599667b8d8a 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/option_env_unwrap.rs b/clippy_lints/src/option_env_unwrap.rs index 96ab4238008d..66dfa20edb5e 100644 --- a/clippy_lints/src/option_env_unwrap.rs +++ b/clippy_lints/src/option_env_unwrap.rs @@ -46,6 +46,7 @@ impl EarlyLintPass for OptionEnvUnwrap { OPTION_ENV_UNWRAP, expr.span, "this will panic at run-time if the environment variable doesn't exist at compile-time", + None, "consider using the `env!` macro instead" ); } diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index c190ed42e3f0..1e2afb7a6740 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -2,8 +2,8 @@ use crate::utils::ptr::get_spans; use crate::utils::{ - is_type_diagnostic_item, match_qpath, match_type, paths, snippet_opt, span_lint, span_lint_and_then, - walk_ptrs_hir_ty, + is_type_diagnostic_item, match_qpath, match_type, paths, snippet_opt, span_lint, span_lint_and_sugg, + span_lint_and_then, walk_ptrs_hir_ty, }; use if_chain::if_chain; use rustc_errors::Applicability; @@ -234,19 +234,14 @@ fn check_fn(cx: &LateContext<'_, '_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_ then { let replacement = snippet_opt(cx, inner.span); if let Some(r) = replacement { - span_lint_and_then( + span_lint_and_sugg( cx, PTR_ARG, arg.span, "using a reference to `Cow` is not recommended.", - |diag| { - diag.span_suggestion( - arg.span, - "change this to", - "&".to_owned() + &r, - Applicability::Unspecified, - ); - }, + "change this to", + "&".to_owned() + &r, + Applicability::Unspecified, ); } } diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index 898bd2aef8bf..a0e2c3c486a7 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -85,7 +85,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { } let mir = cx.tcx.optimized_mir(def_id.to_def_id()); - let mir_read_only = mir.unwrap_read_only(); let maybe_storage_live_result = MaybeStorageLive .into_engine(cx.tcx, mir, def_id.to_def_id()) @@ -93,7 +92,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { .into_results_cursor(mir); let mut possible_borrower = { let mut vis = PossibleBorrowerVisitor::new(cx, mir); - vis.visit_body(&mir_read_only); + vis.visit_body(&mir); vis.into_map(cx, maybe_storage_live_result) }; @@ -146,7 +145,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { // `arg` is a reference as it is `.deref()`ed in the previous block. // Look into the predecessor block and find out the source of deref. - let ps = mir_read_only.predecessors_for(bb); + let ps = &mir.predecessors()[bb]; if ps.len() != 1 { continue; } @@ -191,7 +190,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { (local, deref_clone_ret) }; - let is_temp = mir_read_only.local_kind(ret_local) == mir::LocalKind::Temp; + let is_temp = mir.local_kind(ret_local) == mir::LocalKind::Temp; // 1. `local` can be moved out if it is not used later. // 2. If `ret_local` is a temporary and is neither consumed nor mutated, we can remove this `clone` diff --git a/clippy_lints/src/redundant_pattern_matching.rs b/clippy_lints/src/redundant_pattern_matching.rs index 334ceed64c23..7ee298e9833f 100644 --- a/clippy_lints/src/redundant_pattern_matching.rs +++ b/clippy_lints/src/redundant_pattern_matching.rs @@ -103,14 +103,21 @@ fn find_sugg_for_if_let<'a, 'tcx>( arms[0].pat.span, &format!("redundant pattern matching, consider using `{}`", good_method), |diag| { - // in the case of WhileLetDesugar expr.span == op.span incorrectly. - // this is a workaround to restore true value of expr.span - let expr_span = expr.span.to(arms[1].span); - let span = expr_span.until(op.span.shrink_to_hi()); + // while let ... = ... { ... } + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + let expr_span = expr.span; + + // while let ... = ... { ... } + // ^^^ + let op_span = op.span.source_callsite(); + + // while let ... = ... { ... } + // ^^^^^^^^^^^^^^^^^^^ + let span = expr_span.until(op_span.shrink_to_hi()); diag.span_suggestion( span, "try this", - format!("{} {}.{}", keyword, snippet(cx, op.span, "_"), good_method), + format!("{} {}.{}", keyword, snippet(cx, op_span, "_"), good_method), Applicability::MachineApplicable, // snippet ); }, diff --git a/clippy_lints/src/redundant_pub_crate.rs b/clippy_lints/src/redundant_pub_crate.rs index 4479c560465a..6fc07f91660e 100644 --- a/clippy_lints/src/redundant_pub_crate.rs +++ b/clippy_lints/src/redundant_pub_crate.rs @@ -45,11 +45,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPubCrate { if !cx.access_levels.is_exported(item.hir_id) { if let Some(false) = self.is_exported.last() { let span = item.span.with_hi(item.ident.span.hi()); + let def_id = cx.tcx.hir().local_def_id(item.hir_id); + let descr = cx.tcx.def_kind(def_id).descr(def_id.to_def_id()); span_lint_and_then( cx, REDUNDANT_PUB_CRATE, span, - &format!("pub(crate) {} inside private module", item.kind.descr()), + &format!("pub(crate) {} inside private module", descr), |diag| { diag.span_suggestion( item.vis.span, diff --git a/clippy_lints/src/regex.rs b/clippy_lints/src/regex.rs index 4bcb9198792d..30084e3e1ffc 100644 --- a/clippy_lints/src/regex.rs +++ b/clippy_lints/src/regex.rs @@ -208,7 +208,7 @@ fn check_regex<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>, utf8: match parser.parse(r) { Ok(r) => { if let Some(repl) = is_trivial_regex(&r) { - span_lint_and_help(cx, TRIVIAL_REGEX, expr.span, "trivial regex", repl); + span_lint_and_help(cx, TRIVIAL_REGEX, expr.span, "trivial regex", None, repl); } }, Err(regex_syntax::Error::Parse(e)) => { @@ -236,7 +236,7 @@ fn check_regex<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>, utf8: match parser.parse(&r) { Ok(r) => { if let Some(repl) = is_trivial_regex(&r) { - span_lint_and_help(cx, TRIVIAL_REGEX, expr.span, "trivial regex", repl); + span_lint_and_help(cx, TRIVIAL_REGEX, expr.span, "trivial regex", None, repl); } }, Err(regex_syntax::Error::Parse(e)) => { diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index f7ab00b73047..5c9117d5b81c 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -8,7 +8,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use rustc_span::BytePos; -use crate::utils::{in_macro, match_path_ast, snippet_opt, span_lint_and_then}; +use crate::utils::{in_macro, match_path_ast, snippet_opt, span_lint_and_sugg, span_lint_and_then}; declare_clippy_lint! { /// **What it does:** Checks for return statements at the end of a block. @@ -162,24 +162,26 @@ impl Return { }, None => match replacement { RetReplacement::Empty => { - span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| { - diag.span_suggestion( - ret_span, - "remove `return`", - String::new(), - Applicability::MachineApplicable, - ); - }); + span_lint_and_sugg( + cx, + NEEDLESS_RETURN, + ret_span, + "unneeded `return` statement", + "remove `return`", + String::new(), + Applicability::MachineApplicable, + ); }, RetReplacement::Block => { - span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| { - diag.span_suggestion( - ret_span, - "replace `return` with an empty block", - "{}".to_string(), - Applicability::MachineApplicable, - ); - }); + span_lint_and_sugg( + cx, + NEEDLESS_RETURN, + ret_span, + "unneeded `return` statement", + "replace `return` with an empty block", + "{}".to_string(), + Applicability::MachineApplicable, + ); }, }, } @@ -259,14 +261,15 @@ impl EarlyLintPass for Return { } else { (ty.span, Applicability::MaybeIncorrect) }; - span_lint_and_then(cx, UNUSED_UNIT, rspan, "unneeded unit return type", |diag| { - diag.span_suggestion( - rspan, - "remove the `-> ()`", - String::new(), - appl, - ); - }); + span_lint_and_sugg( + cx, + UNUSED_UNIT, + rspan, + "unneeded unit return type", + "remove the `-> ()`", + String::new(), + appl, + ); } } } @@ -279,14 +282,15 @@ impl EarlyLintPass for Return { if is_unit_expr(expr) && !stmt.span.from_expansion(); then { let sp = expr.span; - span_lint_and_then(cx, UNUSED_UNIT, sp, "unneeded unit expression", |diag| { - diag.span_suggestion( - sp, - "remove the final `()`", - String::new(), - Applicability::MachineApplicable, - ); - }); + span_lint_and_sugg( + cx, + UNUSED_UNIT, + sp, + "unneeded unit expression", + "remove the final `()`", + String::new(), + Applicability::MachineApplicable, + ); } } } @@ -295,14 +299,15 @@ impl EarlyLintPass for Return { match e.kind { ast::ExprKind::Ret(Some(ref expr)) | ast::ExprKind::Break(_, Some(ref expr)) => { if is_unit_expr(expr) && !expr.span.from_expansion() { - span_lint_and_then(cx, UNUSED_UNIT, expr.span, "unneeded `()`", |diag| { - diag.span_suggestion( - expr.span, - "remove the `()`", - String::new(), - Applicability::MachineApplicable, - ); - }); + span_lint_and_sugg( + cx, + UNUSED_UNIT, + expr.span, + "unneeded `()`", + "remove the `()`", + String::new(), + Applicability::MachineApplicable, + ); } }, _ => (), diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 075df19a71e2..67121729663c 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -76,6 +76,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds { TYPE_REPETITION_IN_BOUNDS, p.span, "this type has already been used as a bound predicate", + None, &hint_string, ); } diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs index b99d583c4bec..e24d2c4f495d 100644 --- a/clippy_lints/src/transmute.rs +++ b/clippy_lints/src/transmute.rs @@ -1,5 +1,6 @@ use crate::utils::{ - is_normalizable, last_path_segment, match_def_path, paths, snippet, span_lint, span_lint_and_then, sugg, + is_normalizable, last_path_segment, match_def_path, paths, snippet, span_lint, span_lint_and_sugg, + span_lint_and_then, sugg, }; use if_chain::if_chain; use rustc_ast::ast; @@ -441,24 +442,19 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute { "" }; - span_lint_and_then( + span_lint_and_sugg( cx, TRANSMUTE_BYTES_TO_STR, e.span, &format!("transmute from a `{}` to a `{}`", from_ty, to_ty), - |diag| { - diag.span_suggestion( - e.span, - "consider using", - format!( - "std::str::from_utf8{}({}).unwrap()", - postfix, - snippet(cx, args[0].span, ".."), - ), - Applicability::Unspecified, - ); - } - ) + "consider using", + format!( + "std::str::from_utf8{}({}).unwrap()", + postfix, + snippet(cx, args[0].span, ".."), + ), + Applicability::Unspecified, + ); } else { if cx.tcx.erase_regions(&from_ty) != cx.tcx.erase_regions(&to_ty) { span_lint_and_then( diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 31d8daa2d977..4d853b99bafa 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -343,6 +343,7 @@ impl Types { BOX_VEC, hir_ty.span, "you seem to be trying to use `Box>`. Consider using just `Vec`", + None, "`Vec` is already on the heap, `Box>` makes an extra allocation.", ); return; // don't recurse into the type @@ -437,6 +438,7 @@ impl Types { LINKEDLIST, hir_ty.span, "I see you're using a LinkedList! Perhaps you meant some other data structure?", + None, "a `VecDeque` might work", ); return; // don't recurse into the type @@ -531,11 +533,12 @@ impl Types { } else { format!("{} ", lt.name.ident().as_str()) }; - let mutopt = if mut_ty.mutbl == Mutability::Mut { - "mut " - } else { - "" - }; + + if mut_ty.mutbl == Mutability::Mut { + // Ignore `&mut Box` types; see issue #2907 for + // details. + return; + } let mut applicability = Applicability::MachineApplicable; span_lint_and_sugg( cx, @@ -544,9 +547,8 @@ impl Types { "you seem to be trying to use `&Box`. Consider using just `&T`", "try", format!( - "&{}{}{}", + "&{}{}", ltopt, - mutopt, &snippet_with_applicability(cx, inner.span, "..", &mut applicability) ), Applicability::Unspecified, @@ -1900,7 +1902,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AbsurdExtremeComparisons { conclusion ); - span_lint_and_help(cx, ABSURD_EXTREME_COMPARISONS, expr.span, msg, &help); + span_lint_and_help(cx, ABSURD_EXTREME_COMPARISONS, expr.span, msg, None, &help); } } } diff --git a/clippy_lints/src/unnamed_address.rs b/clippy_lints/src/unnamed_address.rs index b6473fc594ee..4e077b95b5c6 100644 --- a/clippy_lints/src/unnamed_address.rs +++ b/clippy_lints/src/unnamed_address.rs @@ -89,6 +89,7 @@ impl LateLintPass<'_, '_> for UnnamedAddress { VTABLE_ADDRESS_COMPARISONS, expr.span, "comparing trait object pointers compares a non-unique vtable address", + None, "consider extracting and comparing data pointers only", ); } @@ -109,6 +110,7 @@ impl LateLintPass<'_, '_> for UnnamedAddress { VTABLE_ADDRESS_COMPARISONS, expr.span, "comparing trait object pointers compares a non-unique vtable address", + None, "consider extracting and comparing data pointers only", ); } diff --git a/clippy_lints/src/unused_self.rs b/clippy_lints/src/unused_self.rs index 4483059e9eca..3d5e2f9fd215 100644 --- a/clippy_lints/src/unused_self.rs +++ b/clippy_lints/src/unused_self.rs @@ -69,6 +69,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedSelf { UNUSED_SELF, self_param.span, "unused `self` argument", + None, "consider refactoring to a associated function", ); return; diff --git a/clippy_lints/src/utils/diagnostics.rs b/clippy_lints/src/utils/diagnostics.rs index 4ad6689f3e06..24a1bdf1883f 100644 --- a/clippy_lints/src/utils/diagnostics.rs +++ b/clippy_lints/src/utils/diagnostics.rs @@ -49,7 +49,7 @@ pub fn span_lint(cx: &T, lint: &'static Lint, sp: impl Into(cx: &T, lint: &'static Lint, sp: impl Into(cx: &'a T, lint: &'static Lint, span: Span, msg: &str, help: &str) { +pub fn span_lint_and_help<'a, T: LintContext>( + cx: &'a T, + lint: &'static Lint, + span: Span, + msg: &str, + help_span: Option, + help: &str, +) { cx.struct_span_lint(lint, span, |diag| { let mut diag = diag.build(msg); - diag.help(help); + if let Some(help_span) = help_span { + diag.span_help(help_span, help); + } else { + diag.help(help); + } docs_link(&mut diag, lint); diag.emit(); }); @@ -97,15 +108,15 @@ pub fn span_lint_and_note<'a, T: LintContext>( lint: &'static Lint, span: Span, msg: &str, - note_span: Span, + note_span: Option, note: &str, ) { cx.struct_span_lint(lint, span, |diag| { let mut diag = diag.build(msg); - if note_span == span { - diag.note(note); - } else { + if let Some(note_span) = note_span { diag.span_note(note_span, note); + } else { + diag.note(note); } docs_link(&mut diag, lint); diag.emit(); @@ -166,6 +177,7 @@ pub fn span_lint_hir_and_then( /// | /// = note: `-D fold-any` implied by `-D warnings` /// ``` +#[allow(clippy::collapsible_span_lint_calls)] pub fn span_lint_and_sugg<'a, T: LintContext>( cx: &'a T, lint: &'static Lint, diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs index b97fc9547e55..7e8c61ba24a2 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/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index bc2200800de3..5bf9acdc5f7c 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -1,6 +1,7 @@ +use crate::utils::SpanlessEq; use crate::utils::{ - is_expn_of, match_def_path, match_type, method_calls, paths, span_lint, span_lint_and_help, span_lint_and_sugg, - walk_ptrs_ty, + is_expn_of, match_def_path, match_qpath, match_type, method_calls, paths, run_lints, snippet, span_lint, + span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty, }; use if_chain::if_chain; use rustc_ast::ast::{Crate as AstCrate, ItemKind, LitKind, Name, NodeId}; @@ -9,14 +10,17 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; -use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Path, Ty, TyKind}; +use rustc_hir::hir_id::CRATE_HIR_ID; +use rustc_hir::intravisit::{NestedVisitorMap, Visitor}; +use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Path, StmtKind, Ty, TyKind}; use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass}; use rustc_middle::hir::map::Map; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::{Span, Spanned}; use rustc_span::symbol::SymbolStr; +use std::borrow::{Borrow, Cow}; + declare_clippy_lint! { /// **What it does:** Checks for various things we like to keep tidy in clippy. /// @@ -142,6 +146,67 @@ declare_clippy_lint! { "found 'default lint description' in a lint declaration" } +declare_clippy_lint! { + /// **What it does:** Lints `span_lint_and_then` function calls, where the + /// closure argument has only one statement and that statement is a method + /// call to `span_suggestion`, `span_help`, `span_note` (using the same + /// span), `help` or `note`. + /// + /// These usages of `span_lint_and_then` should be replaced with one of the + /// wrapper functions `span_lint_and_sugg`, span_lint_and_help`, or + /// `span_lint_and_note`. + /// + /// **Why is this bad?** Using the wrapper `span_lint_and_*` functions, is more + /// convenient, readable and less error prone. + /// + /// **Known problems:** None + /// + /// *Example:** + /// Bad: + /// ```rust,ignore + /// span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |diag| { + /// diag.span_suggestion( + /// expr.span, + /// help_msg, + /// sugg.to_string(), + /// Applicability::MachineApplicable, + /// ); + /// }); + /// span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |diag| { + /// diag.span_help(expr.span, help_msg); + /// }); + /// span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |diag| { + /// diag.help(help_msg); + /// }); + /// span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |diag| { + /// diag.span_note(expr.span, note_msg); + /// }); + /// span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |diag| { + /// diag.note(note_msg); + /// }); + /// ``` + /// + /// Good: + /// ```rust,ignore + /// span_lint_and_sugg( + /// cx, + /// TEST_LINT, + /// expr.span, + /// lint_msg, + /// help_msg, + /// sugg.to_string(), + /// Applicability::MachineApplicable, + /// ); + /// span_lint_and_help(cx, TEST_LINT, expr.span, lint_msg, Some(expr.span), help_msg); + /// span_lint_and_help(cx, TEST_LINT, expr.span, lint_msg, None, help_msg); + /// span_lint_and_note(cx, TEST_LINT, expr.span, lint_msg, Some(expr.span), note_msg); + /// span_lint_and_note(cx, TEST_LINT, expr.span, lint_msg, None, note_msg); + /// ``` + pub COLLAPSIBLE_SPAN_LINT_CALLS, + internal, + "found collapsible `span_lint_and_then` calls" +} + declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]); impl EarlyLintPass for ClippyLintsInternal { @@ -188,15 +253,20 @@ impl_lint_pass!(LintWithoutLintPass => [DEFAULT_LINT, LINT_WITHOUT_LINT_PASS]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LintWithoutLintPass { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) { + if !run_lints(cx, &[DEFAULT_LINT], item.hir_id) { + return; + } + if let hir::ItemKind::Static(ref ty, Mutability::Not, body_id) = item.kind { if is_lint_ref_type(cx, ty) { let expr = &cx.tcx.hir().body(body_id).value; if_chain! { if let ExprKind::AddrOf(_, _, ref inner_exp) = expr.kind; if let ExprKind::Struct(_, ref fields, _) = inner_exp.kind; - let field = fields.iter() - .find(|f| f.ident.as_str() == "desc") - .expect("lints must have a description field"); + let field = fields + .iter() + .find(|f| f.ident.as_str() == "desc") + .expect("lints must have a description field"); if let ExprKind::Lit(Spanned { node: LitKind::Str(ref sym, _), .. @@ -241,6 +311,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LintWithoutLintPass { } fn check_crate_post(&mut self, cx: &LateContext<'a, 'tcx>, _: &'tcx Crate<'_>) { + if !run_lints(cx, &[LINT_WITHOUT_LINT_PASS], CRATE_HIR_ID) { + return; + } + for (lint_name, &lint_span) in &self.declared_lints { // When using the `declare_tool_lint!` macro, the original `lint_span`'s // file points to "". @@ -290,15 +364,12 @@ struct LintCollector<'a, 'tcx> { impl<'a, 'tcx> Visitor<'tcx> for LintCollector<'a, 'tcx> { type Map = Map<'tcx>; - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - walk_expr(self, expr); - } - fn visit_path(&mut self, path: &'tcx Path<'_>, _: HirId) { if path.segments.len() == 1 { self.output.insert(path.segments[0].ident.name); } } + fn nested_visit_map(&mut self) -> NestedVisitorMap { NestedVisitorMap::All(self.cx.tcx.hir()) } @@ -326,6 +397,10 @@ impl_lint_pass!(CompilerLintFunctions => [COMPILER_LINT_FUNCTIONS]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CompilerLintFunctions { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { + if !run_lints(cx, &[COMPILER_LINT_FUNCTIONS], expr.hir_id) { + return; + } + if_chain! { if let ExprKind::MethodCall(ref path, _, ref args) = expr.kind; let fn_name = path.ident; @@ -339,6 +414,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CompilerLintFunctions { COMPILER_LINT_FUNCTIONS, path.ident.span, "usage of a compiler lint function", + None, &format!("please use the Clippy variant of this function: `{}`", sugg), ); } @@ -350,6 +426,10 @@ declare_lint_pass!(OuterExpnDataPass => [OUTER_EXPN_EXPN_DATA]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OuterExpnDataPass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>) { + if !run_lints(cx, &[OUTER_EXPN_EXPN_DATA], expr.hir_id) { + return; + } + let (method_names, arg_lists, spans) = method_calls(expr, 2); let method_names: Vec = method_names.iter().map(|s| s.as_str()).collect(); let method_names: Vec<&str> = method_names.iter().map(|s| &**s).collect(); @@ -391,3 +471,188 @@ fn is_trigger_fn(fn_kind: FnKind<'_>) -> bool { FnKind::Closure(..) => false, } } + +declare_lint_pass!(CollapsibleCalls => [COLLAPSIBLE_SPAN_LINT_CALLS]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CollapsibleCalls { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>) { + if !run_lints(cx, &[COLLAPSIBLE_SPAN_LINT_CALLS], expr.hir_id) { + return; + } + + if_chain! { + if let ExprKind::Call(ref func, ref and_then_args) = expr.kind; + if let ExprKind::Path(ref path) = func.kind; + if match_qpath(path, &["span_lint_and_then"]); + if and_then_args.len() == 5; + if let ExprKind::Closure(_, _, body_id, _, _) = &and_then_args[4].kind; + let body = cx.tcx.hir().body(*body_id); + if let ExprKind::Block(block, _) = &body.value.kind; + let stmts = &block.stmts; + if stmts.len() == 1 && block.expr.is_none(); + if let StmtKind::Semi(only_expr) = &stmts[0].kind; + if let ExprKind::MethodCall(ref ps, _, ref span_call_args) = &only_expr.kind; + let and_then_snippets = get_and_then_snippets(cx, and_then_args); + let mut sle = SpanlessEq::new(cx).ignore_fn(); + then { + match &*ps.ident.as_str() { + "span_suggestion" if sle.eq_expr(&and_then_args[2], &span_call_args[1]) => { + suggest_suggestion(cx, expr, &and_then_snippets, &span_suggestion_snippets(cx, span_call_args)); + }, + "span_help" if sle.eq_expr(&and_then_args[2], &span_call_args[1]) => { + let help_snippet = snippet(cx, span_call_args[2].span, r#""...""#); + suggest_help(cx, expr, &and_then_snippets, help_snippet.borrow(), true); + }, + "span_note" if sle.eq_expr(&and_then_args[2], &span_call_args[1]) => { + let note_snippet = snippet(cx, span_call_args[2].span, r#""...""#); + suggest_note(cx, expr, &and_then_snippets, note_snippet.borrow(), true); + }, + "help" => { + let help_snippet = snippet(cx, span_call_args[1].span, r#""...""#); + suggest_help(cx, expr, &and_then_snippets, help_snippet.borrow(), false); + } + "note" => { + let note_snippet = snippet(cx, span_call_args[1].span, r#""...""#); + suggest_note(cx, expr, &and_then_snippets, note_snippet.borrow(), false); + } + _ => (), + } + } + } + } +} + +struct AndThenSnippets<'a> { + cx: Cow<'a, str>, + lint: Cow<'a, str>, + span: Cow<'a, str>, + msg: Cow<'a, str>, +} + +fn get_and_then_snippets<'a, 'hir>( + cx: &LateContext<'_, '_>, + and_then_snippets: &'hir [Expr<'hir>], +) -> AndThenSnippets<'a> { + let cx_snippet = snippet(cx, and_then_snippets[0].span, "cx"); + let lint_snippet = snippet(cx, and_then_snippets[1].span, ".."); + let span_snippet = snippet(cx, and_then_snippets[2].span, "span"); + let msg_snippet = snippet(cx, and_then_snippets[3].span, r#""...""#); + + AndThenSnippets { + cx: cx_snippet, + lint: lint_snippet, + span: span_snippet, + msg: msg_snippet, + } +} + +struct SpanSuggestionSnippets<'a> { + help: Cow<'a, str>, + sugg: Cow<'a, str>, + applicability: Cow<'a, str>, +} + +fn span_suggestion_snippets<'a, 'hir>( + cx: &LateContext<'_, '_>, + span_call_args: &'hir [Expr<'hir>], +) -> SpanSuggestionSnippets<'a> { + let help_snippet = snippet(cx, span_call_args[2].span, r#""...""#); + let sugg_snippet = snippet(cx, span_call_args[3].span, ".."); + let applicability_snippet = snippet(cx, span_call_args[4].span, "Applicability::MachineApplicable"); + + SpanSuggestionSnippets { + help: help_snippet, + sugg: sugg_snippet, + applicability: applicability_snippet, + } +} + +fn suggest_suggestion( + cx: &LateContext<'_, '_>, + expr: &Expr<'_>, + and_then_snippets: &AndThenSnippets<'_>, + span_suggestion_snippets: &SpanSuggestionSnippets<'_>, +) { + span_lint_and_sugg( + cx, + COLLAPSIBLE_SPAN_LINT_CALLS, + expr.span, + "this call is collapsible", + "collapse into", + format!( + "span_lint_and_sugg({}, {}, {}, {}, {}, {}, {})", + and_then_snippets.cx, + and_then_snippets.lint, + and_then_snippets.span, + and_then_snippets.msg, + span_suggestion_snippets.help, + span_suggestion_snippets.sugg, + span_suggestion_snippets.applicability + ), + Applicability::MachineApplicable, + ); +} + +fn suggest_help( + cx: &LateContext<'_, '_>, + expr: &Expr<'_>, + and_then_snippets: &AndThenSnippets<'_>, + help: &str, + with_span: bool, +) { + let option_span = if with_span { + format!("Some({})", and_then_snippets.span) + } else { + "None".to_string() + }; + + span_lint_and_sugg( + cx, + COLLAPSIBLE_SPAN_LINT_CALLS, + expr.span, + "this call is collapsible", + "collapse into", + format!( + "span_lint_and_help({}, {}, {}, {}, {}, {})", + and_then_snippets.cx, + and_then_snippets.lint, + and_then_snippets.span, + and_then_snippets.msg, + &option_span, + help + ), + Applicability::MachineApplicable, + ); +} + +fn suggest_note( + cx: &LateContext<'_, '_>, + expr: &Expr<'_>, + and_then_snippets: &AndThenSnippets<'_>, + note: &str, + with_span: bool, +) { + let note_span = if with_span { + format!("Some({})", and_then_snippets.span) + } else { + "None".to_string() + }; + + span_lint_and_sugg( + cx, + COLLAPSIBLE_SPAN_LINT_CALLS, + expr.span, + "this call is collspible", + "collapse into", + format!( + "span_lint_and_note({}, {}, {}, {}, {}, {})", + and_then_snippets.cx, + and_then_snippets.lint, + and_then_snippets.span, + and_then_snippets.msg, + note_span, + note + ), + Applicability::MachineApplicable, + ); +} diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index f7a91fcdd213..0d37932ddab5 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -241,7 +241,7 @@ pub fn match_path(path: &Path<'_>, segments: &[&str]) -> bool { /// /// # Examples /// ```rust,ignore -/// match_qpath(path, &["std", "rt", "begin_unwind"]) +/// match_path_ast(path, &["std", "rt", "begin_unwind"]) /// ``` pub fn match_path_ast(path: &ast::Path, segments: &[&str]) -> bool { path.segments @@ -1399,6 +1399,15 @@ pub fn fn_has_unsatisfiable_preds(cx: &LateContext<'_, '_>, did: DefId) -> bool ) } +pub fn run_lints(cx: &LateContext<'_, '_>, lints: &[&'static Lint], id: HirId) -> bool { + lints.iter().any(|lint| { + matches!( + cx.tcx.lint_level_at_node(lint, id), + (Level::Forbid | Level::Deny | Level::Warn, _) + ) + }) +} + #[cfg(test)] mod test { use super::{trim_multiline, without_block_comments}; diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index d93f8a1e5609..7ad09eabec15 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -72,6 +72,9 @@ pub const ORD: [&str; 3] = ["core", "cmp", "Ord"]; pub const OS_STRING: [&str; 4] = ["std", "ffi", "os_str", "OsString"]; pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", "as_os_str"]; pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"]; +pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"]; +pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 2] = ["parking_lot", "RwLockReadGuard"]; +pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWriteGuard"]; pub const PATH: [&str; 3] = ["std", "path", "Path"]; pub const PATH_BUF: [&str; 3] = ["std", "path", "PathBuf"]; pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"]; @@ -108,6 +111,7 @@ pub const RESULT_ERR: [&str; 4] = ["core", "result", "Result", "Err"]; pub const RESULT_OK: [&str; 4] = ["core", "result", "Result", "Ok"]; pub const RWLOCK_READ_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockReadGuard"]; pub const RWLOCK_WRITE_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockWriteGuard"]; +pub const SERDE_DESERIALIZE: [&str; 2] = ["_serde", "Deserialize"]; pub const SERDE_DE_VISITOR: [&str; 3] = ["serde", "de", "Visitor"]; pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "", "into_vec"]; pub const SLICE_ITER: [&str; 3] = ["core", "slice", "Iter"]; diff --git a/clippy_lints/src/verbose_file_reads.rs b/clippy_lints/src/verbose_file_reads.rs index 55d7983249ae..4d8d4438d881 100644 --- a/clippy_lints/src/verbose_file_reads.rs +++ b/clippy_lints/src/verbose_file_reads.rs @@ -40,6 +40,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VerboseFileReads { VERBOSE_FILE_READS, expr.span, "use of `File::read_to_end`", + None, "consider using `fs::read` instead", ); } else if is_file_read_to_string(cx, expr) { @@ -48,6 +49,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VerboseFileReads { VERBOSE_FILE_READS, expr.span, "use of `File::read_to_string`", + None, "consider using `fs::read_to_string` instead", ) } diff --git a/clippy_lints/src/wildcard_dependencies.rs b/clippy_lints/src/wildcard_dependencies.rs index 035a10b1a247..d8d48eb15358 100644 --- a/clippy_lints/src/wildcard_dependencies.rs +++ b/clippy_lints/src/wildcard_dependencies.rs @@ -1,6 +1,6 @@ -use crate::utils::span_lint; -use rustc_ast::ast::Crate; -use rustc_lint::{EarlyContext, EarlyLintPass}; +use crate::utils::{run_lints, span_lint}; +use rustc_hir::{hir_id::CRATE_HIR_ID, Crate}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::DUMMY_SP; @@ -28,8 +28,12 @@ declare_clippy_lint! { declare_lint_pass!(WildcardDependencies => [WILDCARD_DEPENDENCIES]); -impl EarlyLintPass for WildcardDependencies { - fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &Crate) { +impl LateLintPass<'_, '_> for WildcardDependencies { + fn check_crate(&mut self, cx: &LateContext<'_, '_>, _: &Crate<'_>) { + if !run_lints(cx, &[WILDCARD_DEPENDENCIES], CRATE_HIR_ID) { + return; + } + let metadata = if let Ok(metadata) = cargo_metadata::MetadataCommand::new().no_deps().exec() { metadata } else { diff --git a/clippy_lints/src/zero_div_zero.rs b/clippy_lints/src/zero_div_zero.rs index afd10d9ed53f..fb4700d8743f 100644 --- a/clippy_lints/src/zero_div_zero.rs +++ b/clippy_lints/src/zero_div_zero.rs @@ -49,6 +49,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ZeroDiv { ZERO_DIVIDED_BY_ZERO, expr.span, "constant division of `0.0` with `0.0` will always result in NaN", + None, &format!( "Consider using `{}::NAN` if you would like a constant representing NaN", float_type, diff --git a/doc/adding_lints.md b/doc/adding_lints.md index a66d4e66add2..9ad1315c1752 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -101,7 +101,9 @@ Once we are satisfied with the output, we need to run Please note that, we should run `TESTNAME=foo_functions cargo uitest` every time before running `tests/ui/update-all-references.sh`. Running `TESTNAME=foo_functions cargo uitest` should pass then. When we commit -our lint, we need to commit the generated `.stderr` files, too. +our lint, we need to commit the generated `.stderr` files, too. In general, you +should only commit files changed by `tests/ui/update-all-references.sh` for the +specific lint you are creating/editing. ## Rustfix tests @@ -265,6 +267,7 @@ impl EarlyLintPass for FooFunctions { FOO_FUNCTIONS, span, "function named `foo`", + None, "consider using a more meaningful name" ); } @@ -296,6 +299,7 @@ impl EarlyLintPass for FooFunctions { FOO_FUNCTIONS, span, "function named `foo`", + None, "consider using a more meaningful name" ); } diff --git a/doc/changelog_update.md b/doc/changelog_update.md index 5c7898fc465f..0b80cce6d23e 100644 --- a/doc/changelog_update.md +++ b/doc/changelog_update.md @@ -14,28 +14,34 @@ Forge][forge]. Most of the time we only need to update the changelog for minor Rust releases. It's been very rare that Clippy changes were included in a patch release. -## How to update +## Changelog update walkthrough ### 1. Finding the relevant Clippy commits Each Rust release ships with its own version of Clippy. The Clippy submodule can -be found in the [tools][tools] directory of the Rust repository. +be found in the `tools` directory of the Rust repository. -To find the Clippy commit hash for a specific Rust release you select the Rust -release tag from the dropdown and then check the commit of the Clippy directory: +Depending on the current time and what exactly you want to update, the following +bullet points might be helpful: -![Explanation of how to find the commit hash](https://user-images.githubusercontent.com/2042399/62846160-1f8b0480-bcce-11e9-9da8-7964ca034e7a.png) +* When writing the release notes for the **upcoming stable release** you need to check + out the Clippy commit of the current Rust `beta` branch. [Link][rust_beta_tools] +* When writing the release notes for the **upcoming beta release**, you need to check + out the Clippy commit of the current Rust `master`. [Link][rust_master_tools] +* When writing the (forgotten) release notes for a **past stable release**, you + need to select the Rust release tag from the dropdown and then check the + commit of the Clippy directory: + + ![Explanation of how to find the commit hash](https://user-images.githubusercontent.com/2042399/62846160-1f8b0480-bcce-11e9-9da8-7964ca034e7a.png) -When writing the release notes for the upcoming stable release you want to check -out the commit of the current Rust `beta` tag. ### 2. Fetching the PRs between those commits -You'll want to run `util/fetch_prs_between.sh commit1 commit2 > changes.txt` -and open that file in your editor of choice. +Once you've got the correct commit range, run -* `commit1` is the Clippy commit hash of the previous stable release -* `commit2` is the Clippy commit hash of the release you want to write the changelog for. + util/fetch_prs_between.sh commit1 commit2 > changes.txt + +and open that file in your editor of choice. When updating the changelog it's also a good idea to make sure that `commit1` is already correct in the current changelog. @@ -68,4 +74,5 @@ relevant commit ranges. [changelog]: https://github.com/rust-lang/rust-clippy/blob/master/CHANGELOG.md [forge]: https://forge.rust-lang.org/ -[tools]: https://github.com/rust-lang/rust/tree/master/src/tools +[rust_master_tools]: https://github.com/rust-lang/rust/tree/master/src/tools +[rust_beta_tools]: https://github.com/rust-lang/rust/tree/beta/src/tools diff --git a/doc/release.md b/doc/release.md index 25ddff4c48ca..9d69fa8a7f69 100644 --- a/doc/release.md +++ b/doc/release.md @@ -63,6 +63,16 @@ to the beta Rust release. The remerge is then necessary, to make sure that the Clippy commit, that was used by the now stable Rust release, persists in the tree of the Clippy repository. +To find out if this step is necessary run + +```bash +# Assumes that the local master branch is up-to-date +$ git fetch upstream +$ git branch master --contains upstream/beta +``` + +If this command outputs `master`, this step is **not** necessary. + ```bash # Assuming `HEAD` is the current `master` branch of rust-lang/rust-clippy $ git checkout -b backport_remerge @@ -97,5 +107,5 @@ be updated. # Assuming the current directory corresponds to the Clippy repository $ git checkout beta $ git rebase $BETA_SHA -$ git push upstream beta [-f] # This requires a force push, if a remerge was done +$ git push upstream beta ``` diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 213d054e403d..9b67bacc35d7 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -52,6 +52,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "assign_ops", }, + Lint { + name: "await_holding_lock", + group: "pedantic", + desc: "Inside an async function, holding a MutexGuard while calling await", + deprecation: None, + module: "await_holding_lock", + }, Lint { name: "bad_bit_mask", group: "correctness", @@ -731,6 +738,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "identity_op", }, + Lint { + name: "if_let_mutex", + group: "correctness", + desc: "locking a `Mutex` in an `if let` block can cause deadlocks", + deprecation: None, + module: "if_let_mutex", + }, Lint { name: "if_let_some_result", group: "style", @@ -1125,7 +1139,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "match_bool", - group: "style", + group: "pedantic", desc: "a `match` on a boolean expression instead of an `if..else` block", deprecation: None, module: "matches", @@ -2334,6 +2348,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "literal_representation", }, + Lint { + name: "unsafe_derive_deserialize", + group: "pedantic", + desc: "deriving `serde::Deserialize` on a type that has methods using `unsafe`", + deprecation: None, + module: "derive", + }, Lint { name: "unsafe_removed_from_name", group: "style", diff --git a/tests/ui/await_holding_lock.rs b/tests/ui/await_holding_lock.rs new file mode 100644 index 000000000000..5c1fdd83efb0 --- /dev/null +++ b/tests/ui/await_holding_lock.rs @@ -0,0 +1,64 @@ +// edition:2018 +#![warn(clippy::await_holding_lock)] + +use std::sync::Mutex; + +async fn bad(x: &Mutex) -> u32 { + let guard = x.lock().unwrap(); + baz().await +} + +async fn good(x: &Mutex) -> u32 { + { + let guard = x.lock().unwrap(); + let y = *guard + 1; + } + baz().await; + let guard = x.lock().unwrap(); + 47 +} + +async fn baz() -> u32 { + 42 +} + +async fn also_bad(x: &Mutex) -> u32 { + let first = baz().await; + + let guard = x.lock().unwrap(); + + let second = baz().await; + + let third = baz().await; + + first + second + third +} + +async fn not_good(x: &Mutex) -> u32 { + let first = baz().await; + + let second = { + let guard = x.lock().unwrap(); + baz().await + }; + + let third = baz().await; + + first + second + third +} + +fn block_bad(x: &Mutex) -> impl std::future::Future + '_ { + async move { + let guard = x.lock().unwrap(); + baz().await + } +} + +fn main() { + let m = Mutex::new(100); + good(&m); + bad(&m); + also_bad(&m); + not_good(&m); + block_bad(&m); +} diff --git a/tests/ui/await_holding_lock.stderr b/tests/ui/await_holding_lock.stderr new file mode 100644 index 000000000000..8c47cb37d8c9 --- /dev/null +++ b/tests/ui/await_holding_lock.stderr @@ -0,0 +1,63 @@ +error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await. + --> $DIR/await_holding_lock.rs:7:9 + | +LL | let guard = x.lock().unwrap(); + | ^^^^^ + | + = note: `-D clippy::await-holding-lock` implied by `-D warnings` +note: these are all the await points this lock is held through + --> $DIR/await_holding_lock.rs:7:5 + | +LL | / let guard = x.lock().unwrap(); +LL | | baz().await +LL | | } + | |_^ + +error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await. + --> $DIR/await_holding_lock.rs:28:9 + | +LL | let guard = x.lock().unwrap(); + | ^^^^^ + | +note: these are all the await points this lock is held through + --> $DIR/await_holding_lock.rs:28:5 + | +LL | / let guard = x.lock().unwrap(); +LL | | +LL | | let second = baz().await; +LL | | +... | +LL | | first + second + third +LL | | } + | |_^ + +error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await. + --> $DIR/await_holding_lock.rs:41:13 + | +LL | let guard = x.lock().unwrap(); + | ^^^^^ + | +note: these are all the await points this lock is held through + --> $DIR/await_holding_lock.rs:41:9 + | +LL | / let guard = x.lock().unwrap(); +LL | | baz().await +LL | | }; + | |_____^ + +error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await. + --> $DIR/await_holding_lock.rs:52:13 + | +LL | let guard = x.lock().unwrap(); + | ^^^^^ + | +note: these are all the await points this lock is held through + --> $DIR/await_holding_lock.rs:52:9 + | +LL | / let guard = x.lock().unwrap(); +LL | | baz().await +LL | | } + | |_____^ + +error: aborting due to 4 previous errors + diff --git a/tests/ui/borrow_box.rs b/tests/ui/borrow_box.rs index b30f7290ffea..1901de46ca89 100644 --- a/tests/ui/borrow_box.rs +++ b/tests/ui/borrow_box.rs @@ -4,6 +4,14 @@ #![allow(dead_code)] pub fn test1(foo: &mut Box) { + // Although this function could be changed to "&mut bool", + // avoiding the Box, mutable references to boxes are not + // flagged by this lint. + // + // This omission is intentional: By passing a mutable Box, + // the memory location of the pointed-to object could be + // modified. By passing a mutable reference, the contents + // could change, but not the location. println!("{:?}", foo) } @@ -71,6 +79,16 @@ impl<'a> Test12 for Test11<'a> { } } +pub fn test13(boxed_slice: &mut Box<[i32]>) { + // Unconditionally replaces the box pointer. + // + // This cannot be accomplished if "&mut [i32]" is passed, + // and provides a test case where passing a reference to + // a Box is valid. + let mut data = vec![12]; + *boxed_slice = data.into_boxed_slice(); +} + fn main() { test1(&mut Box::new(false)); test2(); diff --git a/tests/ui/borrow_box.stderr b/tests/ui/borrow_box.stderr index 89171ddd7c6a..b5db691f89f3 100644 --- a/tests/ui/borrow_box.stderr +++ b/tests/ui/borrow_box.stderr @@ -1,8 +1,8 @@ error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:6:19 + --> $DIR/borrow_box.rs:19:14 | -LL | pub fn test1(foo: &mut Box) { - | ^^^^^^^^^^^^^^ help: try: `&mut bool` +LL | let foo: &Box; + | ^^^^^^^^^^ help: try: `&bool` | note: the lint level is defined here --> $DIR/borrow_box.rs:1:9 @@ -11,22 +11,16 @@ LL | #![deny(clippy::borrowed_box)] | ^^^^^^^^^^^^^^^^^^^^ error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:11:14 - | -LL | let foo: &Box; - | ^^^^^^^^^^ help: try: `&bool` - -error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:15:10 + --> $DIR/borrow_box.rs:23:10 | LL | foo: &'a Box, | ^^^^^^^^^^^^^ help: try: `&'a bool` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:19:17 + --> $DIR/borrow_box.rs:27:17 | LL | fn test4(a: &Box); | ^^^^^^^^^^ help: try: `&bool` -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors diff --git a/tests/ui/collapsible_span_lint_calls.fixed b/tests/ui/collapsible_span_lint_calls.fixed new file mode 100644 index 000000000000..e588c23345e2 --- /dev/null +++ b/tests/ui/collapsible_span_lint_calls.fixed @@ -0,0 +1,91 @@ +// run-rustfix +#![deny(clippy::internal)] +#![feature(rustc_private)] + +extern crate rustc_ast; +extern crate rustc_errors; +extern crate rustc_lint; +extern crate rustc_session; +extern crate rustc_span; + +use rustc_ast::ast::Expr; +use rustc_errors::{Applicability, DiagnosticBuilder}; +use rustc_lint::{EarlyContext, EarlyLintPass, Lint, LintContext}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::source_map::Span; + +#[allow(unused_variables)] +pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F) +where + F: for<'b> FnOnce(&mut DiagnosticBuilder<'b>), +{ +} + +#[allow(unused_variables)] +fn span_lint_and_help<'a, T: LintContext>( + cx: &'a T, + lint: &'static Lint, + span: Span, + msg: &str, + option_span: Option, + help: &str, +) { +} + +#[allow(unused_variables)] +fn span_lint_and_note<'a, T: LintContext>( + cx: &'a T, + lint: &'static Lint, + span: Span, + msg: &str, + note_span: Option, + note: &str, +) { +} + +#[allow(unused_variables)] +fn span_lint_and_sugg<'a, T: LintContext>( + cx: &'a T, + lint: &'static Lint, + sp: Span, + msg: &str, + help: &str, + sugg: String, + applicability: Applicability, +) { +} + +declare_tool_lint! { + pub clippy::TEST_LINT, + Warn, + "", + report_in_external_macro: true +} + +declare_lint_pass!(Pass => [TEST_LINT]); + +impl EarlyLintPass for Pass { + fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) { + let lint_msg = "lint message"; + let help_msg = "help message"; + let note_msg = "note message"; + let sugg = "new_call()"; + let predicate = true; + + span_lint_and_sugg(cx, TEST_LINT, expr.span, lint_msg, help_msg, sugg.to_string(), Applicability::MachineApplicable); + span_lint_and_help(cx, TEST_LINT, expr.span, lint_msg, Some(expr.span), help_msg); + span_lint_and_help(cx, TEST_LINT, expr.span, lint_msg, None, help_msg); + span_lint_and_note(cx, TEST_LINT, expr.span, lint_msg, Some(expr.span), note_msg); + span_lint_and_note(cx, TEST_LINT, expr.span, lint_msg, None, note_msg); + + // This expr shouldn't trigger this lint. + span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { + db.note(note_msg); + if predicate { + db.note(note_msg); + } + }) + } +} + +fn main() {} diff --git a/tests/ui/collapsible_span_lint_calls.rs b/tests/ui/collapsible_span_lint_calls.rs new file mode 100644 index 000000000000..d5dd3bb562b4 --- /dev/null +++ b/tests/ui/collapsible_span_lint_calls.rs @@ -0,0 +1,101 @@ +// run-rustfix +#![deny(clippy::internal)] +#![feature(rustc_private)] + +extern crate rustc_ast; +extern crate rustc_errors; +extern crate rustc_lint; +extern crate rustc_session; +extern crate rustc_span; + +use rustc_ast::ast::Expr; +use rustc_errors::{Applicability, DiagnosticBuilder}; +use rustc_lint::{EarlyContext, EarlyLintPass, Lint, LintContext}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::source_map::Span; + +#[allow(unused_variables)] +pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F) +where + F: for<'b> FnOnce(&mut DiagnosticBuilder<'b>), +{ +} + +#[allow(unused_variables)] +fn span_lint_and_help<'a, T: LintContext>( + cx: &'a T, + lint: &'static Lint, + span: Span, + msg: &str, + option_span: Option, + help: &str, +) { +} + +#[allow(unused_variables)] +fn span_lint_and_note<'a, T: LintContext>( + cx: &'a T, + lint: &'static Lint, + span: Span, + msg: &str, + note_span: Option, + note: &str, +) { +} + +#[allow(unused_variables)] +fn span_lint_and_sugg<'a, T: LintContext>( + cx: &'a T, + lint: &'static Lint, + sp: Span, + msg: &str, + help: &str, + sugg: String, + applicability: Applicability, +) { +} + +declare_tool_lint! { + pub clippy::TEST_LINT, + Warn, + "", + report_in_external_macro: true +} + +declare_lint_pass!(Pass => [TEST_LINT]); + +impl EarlyLintPass for Pass { + fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) { + let lint_msg = "lint message"; + let help_msg = "help message"; + let note_msg = "note message"; + let sugg = "new_call()"; + let predicate = true; + + span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { + db.span_suggestion(expr.span, help_msg, sugg.to_string(), Applicability::MachineApplicable); + }); + span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { + db.span_help(expr.span, help_msg); + }); + span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { + db.help(help_msg); + }); + span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { + db.span_note(expr.span, note_msg); + }); + span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { + db.note(note_msg); + }); + + // This expr shouldn't trigger this lint. + span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { + db.note(note_msg); + if predicate { + db.note(note_msg); + } + }) + } +} + +fn main() {} diff --git a/tests/ui/collapsible_span_lint_calls.stderr b/tests/ui/collapsible_span_lint_calls.stderr new file mode 100644 index 000000000000..874d4a9f255c --- /dev/null +++ b/tests/ui/collapsible_span_lint_calls.stderr @@ -0,0 +1,49 @@ +error: this call is collapsible + --> $DIR/collapsible_span_lint_calls.rs:75:9 + | +LL | / span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { +LL | | db.span_suggestion(expr.span, help_msg, sugg.to_string(), Applicability::MachineApplicable); +LL | | }); + | |__________^ help: collapse into: `span_lint_and_sugg(cx, TEST_LINT, expr.span, lint_msg, help_msg, sugg.to_string(), Applicability::MachineApplicable)` + | +note: the lint level is defined here + --> $DIR/collapsible_span_lint_calls.rs:2:9 + | +LL | #![deny(clippy::internal)] + | ^^^^^^^^^^^^^^^^ + = note: `#[deny(clippy::collapsible_span_lint_calls)]` implied by `#[deny(clippy::internal)]` + +error: this call is collapsible + --> $DIR/collapsible_span_lint_calls.rs:78:9 + | +LL | / span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { +LL | | db.span_help(expr.span, help_msg); +LL | | }); + | |__________^ help: collapse into: `span_lint_and_help(cx, TEST_LINT, expr.span, lint_msg, Some(expr.span), help_msg)` + +error: this call is collapsible + --> $DIR/collapsible_span_lint_calls.rs:81:9 + | +LL | / span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { +LL | | db.help(help_msg); +LL | | }); + | |__________^ help: collapse into: `span_lint_and_help(cx, TEST_LINT, expr.span, lint_msg, None, help_msg)` + +error: this call is collspible + --> $DIR/collapsible_span_lint_calls.rs:84:9 + | +LL | / span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { +LL | | db.span_note(expr.span, note_msg); +LL | | }); + | |__________^ help: collapse into: `span_lint_and_note(cx, TEST_LINT, expr.span, lint_msg, Some(expr.span), note_msg)` + +error: this call is collspible + --> $DIR/collapsible_span_lint_calls.rs:87:9 + | +LL | / span_lint_and_then(cx, TEST_LINT, expr.span, lint_msg, |db| { +LL | | db.note(note_msg); +LL | | }); + | |__________^ help: collapse into: `span_lint_and_note(cx, TEST_LINT, expr.span, lint_msg, None, note_msg)` + +error: aborting due to 5 previous errors + diff --git a/tests/ui/crashes/ice-5497.rs b/tests/ui/crashes/ice-5497.rs new file mode 100644 index 000000000000..0769bce5fc80 --- /dev/null +++ b/tests/ui/crashes/ice-5497.rs @@ -0,0 +1,11 @@ +// reduced from rustc issue-69020-assoc-const-arith-overflow.rs +pub fn main() {} + +pub trait Foo { + const OOB: i32; +} + +impl Foo for Vec { + const OOB: i32 = [1][1] + T::OOB; + //~^ ERROR operation will panic +} diff --git a/tests/ui/empty_enum.stderr b/tests/ui/empty_enum.stderr index b1e4eb277550..466dfbe7cee7 100644 --- a/tests/ui/empty_enum.stderr +++ b/tests/ui/empty_enum.stderr @@ -5,11 +5,7 @@ LL | enum Empty {} | ^^^^^^^^^^^^^ | = note: `-D clippy::empty-enum` implied by `-D warnings` -help: consider using the uninhabited type `!` (never type) or a wrapper around it to introduce a type which can't be instantiated - --> $DIR/empty_enum.rs:4:1 - | -LL | enum Empty {} - | ^^^^^^^^^^^^^ + = help: consider using the uninhabited type `!` (never type) or a wrapper around it to introduce a type which can't be instantiated error: aborting due to previous error diff --git a/tests/ui/if_let_mutex.rs b/tests/ui/if_let_mutex.rs new file mode 100644 index 000000000000..58feae422a3c --- /dev/null +++ b/tests/ui/if_let_mutex.rs @@ -0,0 +1,42 @@ +#![warn(clippy::if_let_mutex)] + +use std::ops::Deref; +use std::sync::Mutex; + +fn do_stuff(_: T) {} + +fn if_let() { + let m = Mutex::new(1_u8); + if let Err(locked) = m.lock() { + do_stuff(locked); + } else { + let lock = m.lock().unwrap(); + do_stuff(lock); + }; +} + +// This is the most common case as the above case is pretty +// contrived. +fn if_let_option() { + let m = Mutex::new(Some(0_u8)); + if let Some(locked) = m.lock().unwrap().deref() { + do_stuff(locked); + } else { + let lock = m.lock().unwrap(); + do_stuff(lock); + }; +} + +// When mutexs are different don't warn +fn if_let_different_mutex() { + let m = Mutex::new(Some(0_u8)); + let other = Mutex::new(None::); + if let Some(locked) = m.lock().unwrap().deref() { + do_stuff(locked); + } else { + let lock = other.lock().unwrap(); + do_stuff(lock); + }; +} + +fn main() {} diff --git a/tests/ui/if_let_mutex.stderr b/tests/ui/if_let_mutex.stderr new file mode 100644 index 000000000000..e9c4d9163328 --- /dev/null +++ b/tests/ui/if_let_mutex.stderr @@ -0,0 +1,29 @@ +error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock + --> $DIR/if_let_mutex.rs:10:5 + | +LL | / if let Err(locked) = m.lock() { +LL | | do_stuff(locked); +LL | | } else { +LL | | let lock = m.lock().unwrap(); +LL | | do_stuff(lock); +LL | | }; + | |_____^ + | + = note: `-D clippy::if-let-mutex` implied by `-D warnings` + = help: move the lock call outside of the `if let ...` expression + +error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock + --> $DIR/if_let_mutex.rs:22:5 + | +LL | / if let Some(locked) = m.lock().unwrap().deref() { +LL | | do_stuff(locked); +LL | | } else { +LL | | let lock = m.lock().unwrap(); +LL | | do_stuff(lock); +LL | | }; + | |_____^ + | + = help: move the lock call outside of the `if let ...` expression + +error: aborting due to 2 previous errors + diff --git a/tests/ui/implicit_return.fixed b/tests/ui/implicit_return.fixed index dd42f06664e1..9066dc3fedfd 100644 --- a/tests/ui/implicit_return.fixed +++ b/tests/ui/implicit_return.fixed @@ -21,7 +21,6 @@ fn test_if_block() -> bool { } } -#[allow(clippy::match_bool)] #[rustfmt::skip] fn test_match(x: bool) -> bool { match x { @@ -30,7 +29,7 @@ fn test_match(x: bool) -> bool { } } -#[allow(clippy::match_bool, clippy::needless_return)] +#[allow(clippy::needless_return)] fn test_match_with_unreachable(x: bool) -> bool { match x { true => return false, diff --git a/tests/ui/implicit_return.rs b/tests/ui/implicit_return.rs index 5abbf6a5583e..c0d70ecf502e 100644 --- a/tests/ui/implicit_return.rs +++ b/tests/ui/implicit_return.rs @@ -21,7 +21,6 @@ fn test_if_block() -> bool { } } -#[allow(clippy::match_bool)] #[rustfmt::skip] fn test_match(x: bool) -> bool { match x { @@ -30,7 +29,7 @@ fn test_match(x: bool) -> bool { } } -#[allow(clippy::match_bool, clippy::needless_return)] +#[allow(clippy::needless_return)] fn test_match_with_unreachable(x: bool) -> bool { match x { true => return false, diff --git a/tests/ui/implicit_return.stderr b/tests/ui/implicit_return.stderr index 411b98067d0e..fb2ec9027645 100644 --- a/tests/ui/implicit_return.stderr +++ b/tests/ui/implicit_return.stderr @@ -19,49 +19,49 @@ LL | false | ^^^^^ help: add `return` as shown: `return false` error: missing `return` statement - --> $DIR/implicit_return.rs:28:17 + --> $DIR/implicit_return.rs:27:17 | LL | true => false, | ^^^^^ help: add `return` as shown: `return false` error: missing `return` statement - --> $DIR/implicit_return.rs:29:20 + --> $DIR/implicit_return.rs:28:20 | LL | false => { true }, | ^^^^ help: add `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:44:9 + --> $DIR/implicit_return.rs:43:9 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:52:13 + --> $DIR/implicit_return.rs:51:13 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:61:13 + --> $DIR/implicit_return.rs:60:13 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:79:18 + --> $DIR/implicit_return.rs:78:18 | LL | let _ = || { true }; | ^^^^ help: add `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:80:16 + --> $DIR/implicit_return.rs:79:16 | LL | let _ = || true; | ^^^^ help: add `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:88:5 + --> $DIR/implicit_return.rs:87:5 | LL | format!("test {}", "test") | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `return` as shown: `return format!("test {}", "test")` diff --git a/tests/ui/issue_2356.stderr b/tests/ui/issue_2356.stderr index c8c00f78035c..51b872e21c08 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/map_clone.fixed b/tests/ui/map_clone.fixed index 5e6b3fad41ce..81c7f659efb1 100644 --- a/tests/ui/map_clone.fixed +++ b/tests/ui/map_clone.fixed @@ -4,6 +4,7 @@ #![allow(clippy::clone_on_copy, clippy::redundant_clone)] #![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::redundant_closure_for_method_calls)] +#![allow(clippy::many_single_char_names)] fn main() { let _: Vec = vec![5_i8; 6].iter().copied().collect(); @@ -33,4 +34,14 @@ fn main() { let v: Vec> = vec![Rc::new(0_u32)]; let _: Vec = v.into_iter().map(|x| *x).collect(); } + + // Issue #5524 mutable references + { + let mut c = 42; + let v = vec![&mut c]; + let _: Vec = v.into_iter().map(|x| *x).collect(); + let mut d = 21; + let v = vec![&mut d]; + let _: Vec = v.into_iter().map(|&mut x| x).collect(); + } } diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs index 2fe078b27526..8ed164f0ed51 100644 --- a/tests/ui/map_clone.rs +++ b/tests/ui/map_clone.rs @@ -4,6 +4,7 @@ #![allow(clippy::clone_on_copy, clippy::redundant_clone)] #![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::redundant_closure_for_method_calls)] +#![allow(clippy::many_single_char_names)] fn main() { let _: Vec = vec![5_i8; 6].iter().map(|x| *x).collect(); @@ -33,4 +34,14 @@ fn main() { let v: Vec> = vec![Rc::new(0_u32)]; let _: Vec = v.into_iter().map(|x| *x).collect(); } + + // Issue #5524 mutable references + { + let mut c = 42; + let v = vec![&mut c]; + let _: Vec = v.into_iter().map(|x| *x).collect(); + let mut d = 21; + let v = vec![&mut d]; + let _: Vec = v.into_iter().map(|&mut x| x).collect(); + } } diff --git a/tests/ui/map_clone.stderr b/tests/ui/map_clone.stderr index f67679c7b967..9eec6928e8ce 100644 --- a/tests/ui/map_clone.stderr +++ b/tests/ui/map_clone.stderr @@ -1,5 +1,5 @@ error: You are using an explicit closure for copying elements - --> $DIR/map_clone.rs:9:22 + --> $DIR/map_clone.rs:10:22 | LL | let _: Vec = vec![5_i8; 6].iter().map(|x| *x).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `copied` method: `vec![5_i8; 6].iter().copied()` @@ -7,31 +7,31 @@ LL | let _: Vec = vec![5_i8; 6].iter().map(|x| *x).collect(); = note: `-D clippy::map-clone` implied by `-D warnings` error: You are using an explicit closure for cloning elements - --> $DIR/map_clone.rs:10:26 + --> $DIR/map_clone.rs:11:26 | LL | let _: Vec = vec![String::new()].iter().map(|x| x.clone()).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `cloned` method: `vec![String::new()].iter().cloned()` error: You are using an explicit closure for copying elements - --> $DIR/map_clone.rs:11:23 + --> $DIR/map_clone.rs:12:23 | LL | let _: Vec = vec![42, 43].iter().map(|&x| x).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `copied` method: `vec![42, 43].iter().copied()` error: You are using an explicit closure for copying elements - --> $DIR/map_clone.rs:13:26 + --> $DIR/map_clone.rs:14:26 | LL | let _: Option = Some(&16).map(|b| *b); | ^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `copied` method: `Some(&16).copied()` error: You are using an explicit closure for copying elements - --> $DIR/map_clone.rs:14:25 + --> $DIR/map_clone.rs:15:25 | LL | let _: Option = Some(&1).map(|x| x.clone()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `copied` method: `Some(&1).copied()` error: You are needlessly cloning iterator elements - --> $DIR/map_clone.rs:25:29 + --> $DIR/map_clone.rs:26:29 | LL | let _ = std::env::args().map(|v| v.clone()); | ^^^^^^^^^^^^^^^^^^^ help: Remove the `map` call diff --git a/tests/ui/match_bool.rs b/tests/ui/match_bool.rs index 811aff2a8d40..9ed55ca7ae7f 100644 --- a/tests/ui/match_bool.rs +++ b/tests/ui/match_bool.rs @@ -1,3 +1,5 @@ +#![deny(clippy::match_bool)] + fn match_bool() { let test: bool = true; diff --git a/tests/ui/match_bool.stderr b/tests/ui/match_bool.stderr index d0c20eb2696b..1ad78c740c68 100644 --- a/tests/ui/match_bool.stderr +++ b/tests/ui/match_bool.stderr @@ -1,5 +1,5 @@ error: this boolean expression can be simplified - --> $DIR/match_bool.rs:29:11 + --> $DIR/match_bool.rs:31:11 | LL | match test && test { | ^^^^^^^^^^^^ help: try: `test` @@ -7,7 +7,7 @@ LL | match test && test { = note: `-D clippy::nonminimal-bool` implied by `-D warnings` error: you seem to be trying to match on a boolean expression - --> $DIR/match_bool.rs:4:5 + --> $DIR/match_bool.rs:6:5 | LL | / match test { LL | | true => 0, @@ -15,10 +15,14 @@ LL | | false => 42, LL | | }; | |_____^ help: consider using an `if`/`else` expression: `if test { 0 } else { 42 }` | - = note: `-D clippy::match-bool` implied by `-D warnings` +note: the lint level is defined here + --> $DIR/match_bool.rs:1:9 + | +LL | #![deny(clippy::match_bool)] + | ^^^^^^^^^^^^^^^^^^ error: you seem to be trying to match on a boolean expression - --> $DIR/match_bool.rs:10:5 + --> $DIR/match_bool.rs:12:5 | LL | / match option == 1 { LL | | true => 1, @@ -27,7 +31,7 @@ LL | | }; | |_____^ help: consider using an `if`/`else` expression: `if option == 1 { 1 } else { 0 }` error: you seem to be trying to match on a boolean expression - --> $DIR/match_bool.rs:15:5 + --> $DIR/match_bool.rs:17:5 | LL | / match test { LL | | true => (), @@ -45,7 +49,7 @@ LL | }; | error: you seem to be trying to match on a boolean expression - --> $DIR/match_bool.rs:22:5 + --> $DIR/match_bool.rs:24:5 | LL | / match test { LL | | false => { @@ -63,7 +67,7 @@ LL | }; | error: you seem to be trying to match on a boolean expression - --> $DIR/match_bool.rs:29:5 + --> $DIR/match_bool.rs:31:5 | LL | / match test && test { LL | | false => { @@ -81,7 +85,7 @@ LL | }; | error: equal expressions as operands to `&&` - --> $DIR/match_bool.rs:29:11 + --> $DIR/match_bool.rs:31:11 | LL | match test && test { | ^^^^^^^^^^^^ @@ -89,7 +93,7 @@ LL | match test && test { = note: `#[deny(clippy::eq_op)]` on by default error: you seem to be trying to match on a boolean expression - --> $DIR/match_bool.rs:36:5 + --> $DIR/match_bool.rs:38:5 | LL | / match test { LL | | false => { diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index 70af5c196141..ad20e2381073 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused, clippy::needless_bool, clippy::match_bool)] +#![allow(unused, clippy::needless_bool)] #![allow(clippy::if_same_then_else, clippy::single_match)] #![warn(clippy::needless_return)] diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index a1f8321ac6e7..af0cdfb207ff 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused, clippy::needless_bool, clippy::match_bool)] +#![allow(unused, clippy::needless_bool)] #![allow(clippy::if_same_then_else, clippy::single_match)] #![warn(clippy::needless_return)] diff --git a/tests/ui/new_ret_no_self.rs b/tests/ui/new_ret_no_self.rs index 35aaecc9ac42..2c2d1e275893 100644 --- a/tests/ui/new_ret_no_self.rs +++ b/tests/ui/new_ret_no_self.rs @@ -199,3 +199,14 @@ impl NestedReturnerOk3 { unimplemented!(); } } + +struct WithLifetime<'a> { + cat: &'a str, +} + +impl<'a> WithLifetime<'a> { + // should not trigger the lint, because the lifetimes are different + pub fn new<'b: 'a>(s: &'b str) -> WithLifetime<'b> { + unimplemented!(); + } +} diff --git a/tests/ui/redundant_pattern_matching.fixed b/tests/ui/redundant_pattern_matching.fixed index c58db5493ad0..fc8cb0e747c7 100644 --- a/tests/ui/redundant_pattern_matching.fixed +++ b/tests/ui/redundant_pattern_matching.fixed @@ -2,7 +2,7 @@ #![warn(clippy::all)] #![warn(clippy::redundant_pattern_matching)] -#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool)] +#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)] fn main() { if Ok::(42).is_ok() {} @@ -62,12 +62,31 @@ fn main() { let _ = if Ok::(4).is_ok() { true } else { false }; - let _ = does_something(); - let _ = returns_unit(); - let opt = Some(false); let x = if opt.is_some() { true } else { false }; takes_bool(x); + + issue5504(); + + let _ = if gen_opt().is_some() { + 1 + } else if gen_opt().is_none() { + 2 + } else if gen_res().is_ok() { + 3 + } else if gen_res().is_err() { + 4 + } else { + 5 + }; +} + +fn gen_opt() -> Option<()> { + None +} + +fn gen_res() -> Result<(), ()> { + Ok(()) } fn takes_bool(_: bool) {} @@ -76,18 +95,25 @@ fn foo() {} fn bar() {} -fn does_something() -> bool { - if Ok::(4).is_ok() { - true - } else { - false - } +macro_rules! m { + () => { + Some(42u32) + }; } -fn returns_unit() { - if Ok::(4).is_ok() { - true - } else { - false - }; +fn issue5504() { + fn result_opt() -> Result, i32> { + Err(42) + } + + fn try_result_opt() -> Result { + while r#try!(result_opt()).is_some() {} + if r#try!(result_opt()).is_some() {} + Ok(42) + } + + try_result_opt(); + + if m!().is_some() {} + while m!().is_some() {} } diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs index 9a9b3fb5ca04..51912dade035 100644 --- a/tests/ui/redundant_pattern_matching.rs +++ b/tests/ui/redundant_pattern_matching.rs @@ -2,7 +2,7 @@ #![warn(clippy::all)] #![warn(clippy::redundant_pattern_matching)] -#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool)] +#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)] fn main() { if let Ok(_) = Ok::(42) {} @@ -83,12 +83,31 @@ fn main() { let _ = if let Ok(_) = Ok::(4) { true } else { false }; - let _ = does_something(); - let _ = returns_unit(); - let opt = Some(false); let x = if let Some(_) = opt { true } else { false }; takes_bool(x); + + issue5504(); + + let _ = if let Some(_) = gen_opt() { + 1 + } else if let None = gen_opt() { + 2 + } else if let Ok(_) = gen_res() { + 3 + } else if let Err(_) = gen_res() { + 4 + } else { + 5 + }; +} + +fn gen_opt() -> Option<()> { + None +} + +fn gen_res() -> Result<(), ()> { + Ok(()) } fn takes_bool(_: bool) {} @@ -97,18 +116,25 @@ fn foo() {} fn bar() {} -fn does_something() -> bool { - if let Ok(_) = Ok::(4) { - true - } else { - false - } +macro_rules! m { + () => { + Some(42u32) + }; } -fn returns_unit() { - if let Ok(_) = Ok::(4) { - true - } else { - false - }; +fn issue5504() { + fn result_opt() -> Result, i32> { + Err(42) + } + + fn try_result_opt() -> Result { + while let Some(_) = r#try!(result_opt()) {} + if let Some(_) = r#try!(result_opt()) {} + Ok(42) + } + + try_result_opt(); + + if let Some(_) = m!() {} + while let Some(_) = m!() {} } diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr index 6285a7f5fcd1..b58deb7954ef 100644 --- a/tests/ui/redundant_pattern_matching.stderr +++ b/tests/ui/redundant_pattern_matching.stderr @@ -137,22 +137,58 @@ LL | let _ = if let Ok(_) = Ok::(4) { true } else { false }; | -------^^^^^--------------------- help: try this: `if Ok::(4).is_ok()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:90:20 + --> $DIR/redundant_pattern_matching.rs:87:20 | LL | let x = if let Some(_) = opt { true } else { false }; | -------^^^^^^^------ help: try this: `if opt.is_some()` -error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:101:12 +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching.rs:92:20 + | +LL | let _ = if let Some(_) = gen_opt() { + | -------^^^^^^^------------ help: try this: `if gen_opt().is_some()` + +error: redundant pattern matching, consider using `is_none()` + --> $DIR/redundant_pattern_matching.rs:94:19 | -LL | if let Ok(_) = Ok::(4) { - | -------^^^^^-------------------- help: try this: `if Ok::(4).is_ok()` +LL | } else if let None = gen_opt() { + | -------^^^^------------ help: try this: `if gen_opt().is_none()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:109:12 + --> $DIR/redundant_pattern_matching.rs:96:19 + | +LL | } else if let Ok(_) = gen_res() { + | -------^^^^^------------ help: try this: `if gen_res().is_ok()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching.rs:98:19 + | +LL | } else if let Err(_) = gen_res() { + | -------^^^^^^------------ help: try this: `if gen_res().is_err()` + +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching.rs:131:19 + | +LL | while let Some(_) = r#try!(result_opt()) {} + | ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()` + +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching.rs:132:16 + | +LL | if let Some(_) = r#try!(result_opt()) {} + | -------^^^^^^^----------------------- help: try this: `if r#try!(result_opt()).is_some()` + +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching.rs:138:12 + | +LL | if let Some(_) = m!() {} + | -------^^^^^^^------- help: try this: `if m!().is_some()` + +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching.rs:139:15 | -LL | if let Ok(_) = Ok::(4) { - | -------^^^^^-------------------- help: try this: `if Ok::(4).is_ok()` +LL | while let Some(_) = m!() {} + | ----------^^^^^^^------- help: try this: `while m!().is_some()` -error: aborting due to 22 previous errors +error: aborting due to 28 previous errors diff --git a/tests/ui/toplevel_ref_arg.fixed b/tests/ui/toplevel_ref_arg.fixed index 9438abbe330d..33605aca0199 100644 --- a/tests/ui/toplevel_ref_arg.fixed +++ b/tests/ui/toplevel_ref_arg.fixed @@ -23,4 +23,7 @@ fn main() { // Make sure that allowing the lint works #[allow(clippy::toplevel_ref_arg)] let ref mut _x = 1_234_543; + + // ok + for ref _x in 0..10 {} } diff --git a/tests/ui/toplevel_ref_arg.rs b/tests/ui/toplevel_ref_arg.rs index ee630f12a601..59759f118933 100644 --- a/tests/ui/toplevel_ref_arg.rs +++ b/tests/ui/toplevel_ref_arg.rs @@ -23,4 +23,7 @@ fn main() { // Make sure that allowing the lint works #[allow(clippy::toplevel_ref_arg)] let ref mut _x = 1_234_543; + + // ok + for ref _x in 0..10 {} } diff --git a/tests/ui/unsafe_derive_deserialize.rs b/tests/ui/unsafe_derive_deserialize.rs new file mode 100644 index 000000000000..7bee9c499e1f --- /dev/null +++ b/tests/ui/unsafe_derive_deserialize.rs @@ -0,0 +1,60 @@ +#![warn(clippy::unsafe_derive_deserialize)] +#![allow(unused, clippy::missing_safety_doc)] + +extern crate serde; + +use serde::Deserialize; + +#[derive(Deserialize)] +pub struct A {} +impl A { + pub unsafe fn new(_a: i32, _b: i32) -> Self { + Self {} + } +} + +#[derive(Deserialize)] +pub struct B {} +impl B { + pub unsafe fn unsafe_method(&self) {} +} + +#[derive(Deserialize)] +pub struct C {} +impl C { + pub fn unsafe_block(&self) { + unsafe {} + } +} + +#[derive(Deserialize)] +pub struct D {} +impl D { + pub fn inner_unsafe_fn(&self) { + unsafe fn inner() {} + } +} + +// Does not derive `Deserialize`, should be ignored +pub struct E {} +impl E { + pub unsafe fn new(_a: i32, _b: i32) -> Self { + Self {} + } + + pub unsafe fn unsafe_method(&self) {} + + pub fn unsafe_block(&self) { + unsafe {} + } + + pub fn inner_unsafe_fn(&self) { + unsafe fn inner() {} + } +} + +// Does not have methods using `unsafe`, should be ignored +#[derive(Deserialize)] +pub struct F {} + +fn main() {} diff --git a/tests/ui/unsafe_derive_deserialize.stderr b/tests/ui/unsafe_derive_deserialize.stderr new file mode 100644 index 000000000000..1978bd95a670 --- /dev/null +++ b/tests/ui/unsafe_derive_deserialize.stderr @@ -0,0 +1,39 @@ +error: you are deriving `serde::Deserialize` on a type that has methods using `unsafe` + --> $DIR/unsafe_derive_deserialize.rs:8:10 + | +LL | #[derive(Deserialize)] + | ^^^^^^^^^^^ + | + = note: `-D clippy::unsafe-derive-deserialize` implied by `-D warnings` + = help: consider implementing `serde::Deserialize` manually. See https://serde.rs/impl-deserialize.html + = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: you are deriving `serde::Deserialize` on a type that has methods using `unsafe` + --> $DIR/unsafe_derive_deserialize.rs:16:10 + | +LL | #[derive(Deserialize)] + | ^^^^^^^^^^^ + | + = help: consider implementing `serde::Deserialize` manually. See https://serde.rs/impl-deserialize.html + = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: you are deriving `serde::Deserialize` on a type that has methods using `unsafe` + --> $DIR/unsafe_derive_deserialize.rs:22:10 + | +LL | #[derive(Deserialize)] + | ^^^^^^^^^^^ + | + = help: consider implementing `serde::Deserialize` manually. See https://serde.rs/impl-deserialize.html + = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: you are deriving `serde::Deserialize` on a type that has methods using `unsafe` + --> $DIR/unsafe_derive_deserialize.rs:30:10 + | +LL | #[derive(Deserialize)] + | ^^^^^^^^^^^ + | + = help: consider implementing `serde::Deserialize` manually. See https://serde.rs/impl-deserialize.html + = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 4 previous errors + diff --git a/tests/ui/while_let_on_iterator.fixed b/tests/ui/while_let_on_iterator.fixed new file mode 100644 index 000000000000..f5fcabf63fd3 --- /dev/null +++ b/tests/ui/while_let_on_iterator.fixed @@ -0,0 +1,160 @@ +// 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); + + while let Some(..) = values.iter().next() {} +} + +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 issue1654() { + // should not lint if the iterator is generated on every iteration + use std::collections::HashSet; + let mut values = HashSet::new(); + values.insert(1); + + while let Some(..) = values.iter().next() { + values.remove(&1); + } + + while let Some(..) = values.iter().map(|x| x + 1).next() {} + + let chars = "Hello, World!".char_indices(); + while let Some((i, ch)) = chars.clone().next() { + println!("{}: {}", i, ch); + } +} + +fn main() { + base(); + refutable(); + nested_loops(); + issue1121(); + issue2965(); + issue3670(); + issue1654(); +} diff --git a/tests/ui/while_let_on_iterator.rs b/tests/ui/while_let_on_iterator.rs index 84dfc34db150..04dce8a02898 100644 --- a/tests/ui/while_let_on_iterator.rs +++ b/tests/ui/while_let_on_iterator.rs @@ -1,7 +1,9 @@ +// run-rustfix + #![warn(clippy::while_let_on_iterator)] -#![allow(clippy::never_loop)] +#![allow(clippy::never_loop, unreachable_code, unused_mut)] -fn main() { +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 } } @@ -117,9 +119,7 @@ fn issue2965() { let mut values = HashSet::new(); values.insert(1); - while let Some(..) = values.iter().next() { - values.remove(&1); - } + while let Some(..) = values.iter().next() {} } fn issue3670() { @@ -130,3 +130,31 @@ fn issue3670() { let _ = elem.or_else(|| *iter.next()?); } } + +fn issue1654() { + // should not lint if the iterator is generated on every iteration + use std::collections::HashSet; + let mut values = HashSet::new(); + values.insert(1); + + while let Some(..) = values.iter().next() { + values.remove(&1); + } + + while let Some(..) = values.iter().map(|x| x + 1).next() {} + + let chars = "Hello, World!".char_indices(); + while let Some((i, ch)) = chars.clone().next() { + println!("{}: {}", i, ch); + } +} + +fn main() { + base(); + refutable(); + nested_loops(); + issue1121(); + issue2965(); + issue3670(); + issue1654(); +} diff --git a/tests/ui/while_let_on_iterator.stderr b/tests/ui/while_let_on_iterator.stderr index 03d2ef550668..6de138d7227b 100644 --- a/tests/ui/while_let_on_iterator.stderr +++ b/tests/ui/while_let_on_iterator.stderr @@ -1,34 +1,28 @@ 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 - | -LL | while let Some(..) = values.iter().next() { - | ^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in values.iter() { .. }` - -error: aborting due to 5 previous errors +error: aborting due to 4 previous errors diff --git a/util/fetch_prs_between.sh b/util/fetch_prs_between.sh index 881aba196e72..6865abf971b2 100755 --- a/util/fetch_prs_between.sh +++ b/util/fetch_prs_between.sh @@ -20,6 +20,7 @@ for pr in $(git log --oneline --grep "Merge #" --grep "Merge pull request" --gre fi echo "URL: https://github.com/rust-lang/rust-clippy/pull/$id" + echo "Markdown URL: [#$id](https://github.com/rust-lang/rust-clippy/pull/$id)" echo "$message" echo "---------------------------------------------------------" echo