diff options
| author | flip1995 <philipp.krones@embecosm.com> | 2021-05-20 12:30:31 +0200 |
|---|---|---|
| committer | flip1995 <philipp.krones@embecosm.com> | 2021-05-20 13:07:57 +0200 |
| commit | 14f55fa7498077236de62bc1b531ba15ba45a5fd (patch) | |
| tree | 595170c2ce41fec45be88d5ec6bc22159ca5e81c | |
| parent | 5ab0f37087b61b2ea51f364498cddc20ecff74fa (diff) | |
| parent | 9e3cd88718cd1912a515d26dbd9c4019fd5a9577 (diff) | |
| download | rust-14f55fa7498077236de62bc1b531ba15ba45a5fd.tar.gz rust-14f55fa7498077236de62bc1b531ba15ba45a5fd.zip | |
Merge commit '9e3cd88718cd1912a515d26dbd9c4019fd5a9577' into clippyup
80 files changed, 2329 insertions, 668 deletions
diff --git a/src/tools/clippy/.cargo/config b/src/tools/clippy/.cargo/config index 9b5add4df1c..e95ea224cb6 100644 --- a/src/tools/clippy/.cargo/config +++ b/src/tools/clippy/.cargo/config @@ -2,6 +2,7 @@ uitest = "test --test compile-test" dev = "run --target-dir clippy_dev/target --package clippy_dev --bin clippy_dev --manifest-path clippy_dev/Cargo.toml --" lintcheck = "run --target-dir lintcheck/target --package lintcheck --bin lintcheck --manifest-path lintcheck/Cargo.toml -- " +collect-metadata = "test --test dogfood --features metadata-collector-lint -- run_metadata_collection_lint --ignored" [build] rustflags = ["-Zunstable-options"] diff --git a/src/tools/clippy/.github/workflows/clippy_bors.yml b/src/tools/clippy/.github/workflows/clippy_bors.yml index ae6f1aa1b30..f27fee87dc1 100644 --- a/src/tools/clippy/.github/workflows/clippy_bors.yml +++ b/src/tools/clippy/.github/workflows/clippy_bors.yml @@ -90,6 +90,11 @@ jobs: - name: Checkout uses: actions/checkout@v2.3.3 + # FIXME: should not be necessary once 1.24.2 is the default version on the windows runner + - name: Update rustup + run: rustup self update + if: runner.os == 'Windows' + - name: Install toolchain run: rustup show active-toolchain diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index 204d56e2a98..da5a0712c95 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/CHANGELOG.md @@ -6,11 +6,195 @@ document. ## Unreleased / In Rust Nightly -[6ed6f1e...master](https://github.com/rust-lang/rust-clippy/compare/6ed6f1e...master) +[7c7683c...master](https://github.com/rust-lang/rust-clippy/compare/7c7683c...master) + +## Rust 1.53 + +Current beta, release 2021-06-17 + +[6ed6f1e...7c7683c](https://github.com/rust-lang/rust-clippy/compare/6ed6f1e...7c7683c) + +### New Lints + +* [`option_filter_map`] + [#6342](https://github.com/rust-lang/rust-clippy/pull/6342) +* [`branches_sharing_code`] + [#6463](https://github.com/rust-lang/rust-clippy/pull/6463) +* [`needless_for_each`] + [#6706](https://github.com/rust-lang/rust-clippy/pull/6706) +* [`if_then_some_else_none`] + [#6859](https://github.com/rust-lang/rust-clippy/pull/6859) +* [`non_octal_unix_permissions`] + [#7001](https://github.com/rust-lang/rust-clippy/pull/7001) +* [`unnecessary_self_imports`] + [#7072](https://github.com/rust-lang/rust-clippy/pull/7072) +* [`bool_assert_comparison`] + [#7083](https://github.com/rust-lang/rust-clippy/pull/7083) +* [`cloned_instead_of_copied`] + [#7098](https://github.com/rust-lang/rust-clippy/pull/7098) +* [`flat_map_option`] + [#7101](https://github.com/rust-lang/rust-clippy/pull/7101) + +### Moves and Deprecations + +* Deprecate [`filter_map`] lint + [#7059](https://github.com/rust-lang/rust-clippy/pull/7059) +* Move [`transmute_ptr_to_ptr`] to `pedantic` + [#7102](https://github.com/rust-lang/rust-clippy/pull/7102) + +### Enhancements + +* [`mem_replace_with_default`]: Also lint on common std constructors + [#6820](https://github.com/rust-lang/rust-clippy/pull/6820) +* [`wrong_self_convention`]: Also lint on `to_*_mut` methods + [#6828](https://github.com/rust-lang/rust-clippy/pull/6828) +* [`wildcard_enum_match_arm`], [`match_wildcard_for_single_variants`]: + [#6863](https://github.com/rust-lang/rust-clippy/pull/6863) + * Attempt to find a common path prefix in suggestion + * Don't lint on `Option` and `Result` + * Consider `Self` prefix +* [`explicit_deref_methods`]: Also lint on chained `deref` calls + [#6865](https://github.com/rust-lang/rust-clippy/pull/6865) +* [`or_fun_call`]: Also lint on `unsafe` blocks + [#6928](https://github.com/rust-lang/rust-clippy/pull/6928) +* [`vec_box`], [`linkedlist`], [`option_option`]: Also lint in `const` and + `static` items [#6938](https://github.com/rust-lang/rust-clippy/pull/6938) +* [`search_is_some`]: Also check for `is_none` + [#6942](https://github.com/rust-lang/rust-clippy/pull/6942) +* [`string_lit_as_bytes`]: Also lint on `into_bytes` + [#6959](https://github.com/rust-lang/rust-clippy/pull/6959) +* [`len_without_is_empty`]: Also lint if function signatures of `len` and + `is_empty` don't match + [#6980](https://github.com/rust-lang/rust-clippy/pull/6980) +* [`redundant_pattern_matching`]: Also lint if the pattern is a `&` pattern + [#6991](https://github.com/rust-lang/rust-clippy/pull/6991) +* [`clone_on_copy`]: Also lint on chained method calls taking `self` by value + [#7000](https://github.com/rust-lang/rust-clippy/pull/7000) +* [`missing_panics_doc`]: Also lint on `assert_eq!` and `assert_ne!` + [#7029](https://github.com/rust-lang/rust-clippy/pull/7029) +* [`needless_return`]: Also lint in `async` functions + [#7067](https://github.com/rust-lang/rust-clippy/pull/7067) +* [`unused_io_amount`]: Also lint on expressions like `_.read().ok()?` + [#7100](https://github.com/rust-lang/rust-clippy/pull/7100) +* [`iter_cloned_collect`]: Also lint on large arrays, since const-generics are + now stable [#7138](https://github.com/rust-lang/rust-clippy/pull/7138) + +### False Positive Fixes + +* [`upper_case_acronyms`]: No longer lints on public items + [#6805](https://github.com/rust-lang/rust-clippy/pull/6805) +* [`suspicious_map`]: No longer lints when side effects may occur inside the + `map` call [#6831](https://github.com/rust-lang/rust-clippy/pull/6831) +* [`manual_map`], [`manual_unwrap_or`]: No longer lints in `const` functions + [#6917](https://github.com/rust-lang/rust-clippy/pull/6917) +* [`wrong_self_convention`]: Now respects `Copy` types + [#6924](https://github.com/rust-lang/rust-clippy/pull/6924) +* [`needless_question_mark`]: No longer lints if the `?` and the `Some(..)` come + from different macro contexts [#6935](https://github.com/rust-lang/rust-clippy/pull/6935) +* [`map_entry`]: Better detect if the entry API can be used + [#6937](https://github.com/rust-lang/rust-clippy/pull/6937) +* [`or_fun_call`]: No longer lints on some `len` function calls + [#6950](https://github.com/rust-lang/rust-clippy/pull/6950) +* [`new_ret_no_self`]: No longer lints when `Self` is returned with different + generic arguments [#6952](https://github.com/rust-lang/rust-clippy/pull/6952) +* [`upper_case_acronyms`]: No longer lints on public items + [#6981](https://github.com/rust-lang/rust-clippy/pull/6981) +* [`explicit_into_iter_loop`]: Only lint when `into_iter` is an implementation + of `IntoIterator` [#6982](https://github.com/rust-lang/rust-clippy/pull/6982) +* [`expl_impl_clone_on_copy`]: Take generic constraints into account before + suggesting to use `derive` instead + [#6993](https://github.com/rust-lang/rust-clippy/pull/6993) +* [`missing_panics_doc`]: No longer lints when only debug-assertions are used + [#6996](https://github.com/rust-lang/rust-clippy/pull/6996) +* [`clone_on_copy`]: Only lint when using the `Clone` trait + [#7000](https://github.com/rust-lang/rust-clippy/pull/7000) +* [`wrong_self_convention`]: No longer lints inside a trait implementation + [#7002](https://github.com/rust-lang/rust-clippy/pull/7002) +* [`redundant_clone`]: No longer lints when the cloned value is modified while + the clone is in use + [#7011](https://github.com/rust-lang/rust-clippy/pull/7011) +* [`same_item_push`]: No longer lints if the `Vec` is used in the loop body + [#7018](https://github.com/rust-lang/rust-clippy/pull/7018) +* [`cargo_common_metadata`]: Remove author requirement + [#7026](https://github.com/rust-lang/rust-clippy/pull/7026) +* [`panic_in_result_fn`]: No longer lints on `debug_assert` family + [#7060](https://github.com/rust-lang/rust-clippy/pull/7060) +* [`panic`]: No longer wrongfully lints on `debug_assert` with message + [#7063](https://github.com/rust-lang/rust-clippy/pull/7063) +* [`wrong_self_convention`]: No longer lints in trait implementations where no + `self` is involved [#7064](https://github.com/rust-lang/rust-clippy/pull/7064) +* [`missing_const_for_fn`]: No longer lints when unstable `const` function is + involved [#7076](https://github.com/rust-lang/rust-clippy/pull/7076) +* [`suspicious_else_formatting`]: Allow Allman style braces + [#7087](https://github.com/rust-lang/rust-clippy/pull/7087) +* [`inconsistent_struct_constructor`]: No longer lints in macros + [#7097](https://github.com/rust-lang/rust-clippy/pull/7097) +* [`single_component_path_imports`]: No longer lints on macro re-exports + [#7120](https://github.com/rust-lang/rust-clippy/pull/7120) + +### Suggestion Fixes/Improvements + +* [`redundant_pattern_matching`]: Add a note when applying this lint would + change the drop order + [#6568](https://github.com/rust-lang/rust-clippy/pull/6568) +* [`write_literal`], [`print_literal`]: Add auto-applicable suggestion + [#6821](https://github.com/rust-lang/rust-clippy/pull/6821) +* [`manual_map`]: Fix suggestion for complex `if let ... else` chains + [#6856](https://github.com/rust-lang/rust-clippy/pull/6856) +* [`inconsistent_struct_constructor`]: Make lint description and message clearer + [#6892](https://github.com/rust-lang/rust-clippy/pull/6892) +* [`map_entry`]: Now suggests `or_insert`, `insert_with` or `match _.entry(_)` + as appropriate [#6937](https://github.com/rust-lang/rust-clippy/pull/6937) +* [`manual_flatten`]: Suggest to insert `copied` if necessary + [#6962](https://github.com/rust-lang/rust-clippy/pull/6962) +* [`redundant_slicing`]: Fix suggestion when a re-borrow might be required or + when the value is from a macro call + [#6975](https://github.com/rust-lang/rust-clippy/pull/6975) +* [`match_wildcard_for_single_variants`]: Fix suggestion for hidden variant + [#6988](https://github.com/rust-lang/rust-clippy/pull/6988) +* [`clone_on_copy`]: Correct suggestion when the cloned value is a macro call + [#7000](https://github.com/rust-lang/rust-clippy/pull/7000) +* [`manual_map`]: Fix suggestion at the end of an if chain + [#7004](https://github.com/rust-lang/rust-clippy/pull/7004) +* Fix needless parenthesis output in multiple lint suggestions + [#7013](https://github.com/rust-lang/rust-clippy/pull/7013) +* [`needless_collect`]: Better explanation in the lint message + [#7020](https://github.com/rust-lang/rust-clippy/pull/7020) +* [`useless_vec`]: Now considers mutability + [#7036](https://github.com/rust-lang/rust-clippy/pull/7036) +* [`useless_format`]: Wrap the content in braces if necessary + [#7092](https://github.com/rust-lang/rust-clippy/pull/7092) +* [`single_match`]: Don't suggest an equality check for types which don't + implement `PartialEq` + [#7093](https://github.com/rust-lang/rust-clippy/pull/7093) +* [`from_over_into`]: Mention type in help message + [#7099](https://github.com/rust-lang/rust-clippy/pull/7099) +* [`manual_unwrap_or`]: Fix invalid code suggestion due to a macro call + [#7136](https://github.com/rust-lang/rust-clippy/pull/7136) + +### ICE Fixes + +* [`macro_use_imports`] + [#7022](https://github.com/rust-lang/rust-clippy/pull/7022) +* [`missing_panics_doc`] + [#7034](https://github.com/rust-lang/rust-clippy/pull/7034) +* [`tabs_in_doc_comments`] + [#7039](https://github.com/rust-lang/rust-clippy/pull/7039) +* [`missing_const_for_fn`] + [#7128](https://github.com/rust-lang/rust-clippy/pull/7128) + +### Others + +* [Clippy's lint + list](https://rust-lang.github.io/rust-clippy/master/index.html) now supports + themes [#7030](https://github.com/rust-lang/rust-clippy/pull/7030) +* Lints that were uplifted to `rustc` now mention the new `rustc` name in the + deprecation warning + [#7056](https://github.com/rust-lang/rust-clippy/pull/7056) ## Rust 1.52 -Current beta, release 2021-05-06 +Current stable, released 2021-05-06 [3e41797...6ed6f1e](https://github.com/rust-lang/rust-clippy/compare/3e41797...6ed6f1e) @@ -99,7 +283,7 @@ Current beta, release 2021-05-06 [#6682](https://github.com/rust-lang/rust-clippy/pull/6682) * [`unit_arg`]: No longer lints on unit arguments when they come from a path expression. [#6601](https://github.com/rust-lang/rust-clippy/pull/6601) -* [`cargo_common_metadata`]: No longer lints if +* [`cargo_common_metadata`]: No longer lints if [`publish = false`](https://doc.rust-lang.org/cargo/reference/manifest.html#the-publish-field) is defined in the manifest [#6650](https://github.com/rust-lang/rust-clippy/pull/6650) @@ -124,11 +308,11 @@ Current beta, release 2021-05-06 * [`useless_format`]: Improved the documentation example [#6854](https://github.com/rust-lang/rust-clippy/pull/6854) -* Clippy's [`README.md`]: Includes a new subsection on running Clippy as a rustc wrapper +* Clippy's [`README.md`]: Includes a new subsection on running Clippy as a rustc wrapper [#6782](https://github.com/rust-lang/rust-clippy/pull/6782) ### Others -* Running `cargo clippy` after `cargo check` now works as expected +* Running `cargo clippy` after `cargo check` now works as expected (`cargo clippy` and `cargo check` no longer shares the same build cache) [#6687](https://github.com/rust-lang/rust-clippy/pull/6687) * Cargo now re-runs Clippy if arguments after `--` provided to `cargo clippy` are changed. @@ -145,7 +329,7 @@ Current beta, release 2021-05-06 ## Rust 1.51 -Current stable, released 2021-03-25 +Released 2021-03-25 [4911ab1...3e41797](https://github.com/rust-lang/rust-clippy/compare/4911ab1...3e41797) @@ -2365,6 +2549,7 @@ Released 2018-09-13 [`mutex_integer`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer [`naive_bytecount`]: https://rust-lang.github.io/rust-clippy/master/index.html#naive_bytecount [`needless_arbitrary_self_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_arbitrary_self_type +[`needless_bitwise_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_bitwise_bool [`needless_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_bool [`needless_borrow`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow [`needless_borrowed_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrowed_reference @@ -2538,6 +2723,7 @@ Released 2018-09-13 [`unsound_collection_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsound_collection_transmute [`unstable_as_mut_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#unstable_as_mut_slice [`unstable_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#unstable_as_slice +[`unused_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_async [`unused_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_collect [`unused_io_amount`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_io_amount [`unused_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_self diff --git a/src/tools/clippy/Cargo.toml b/src/tools/clippy/Cargo.toml index f010e609604..848476a9d05 100644 --- a/src/tools/clippy/Cargo.toml +++ b/src/tools/clippy/Cargo.toml @@ -49,7 +49,7 @@ rustc-workspace-hack = "1.0.0" rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util" } [features] -deny-warnings = [] +deny-warnings = ["clippy_lints/deny-warnings"] integration = ["tempfile"] internal-lints = ["clippy_lints/internal-lints"] metadata-collector-lint = ["internal-lints", "clippy_lints/metadata-collector-lint"] diff --git a/src/tools/clippy/clippy_dev/src/lib.rs b/src/tools/clippy/clippy_dev/src/lib.rs index 1e5a140e964..69f42aca8b6 100644 --- a/src/tools/clippy/clippy_dev/src/lib.rs +++ b/src/tools/clippy/clippy_dev/src/lib.rs @@ -1,5 +1,7 @@ -#![cfg_attr(feature = "deny-warnings", deny(warnings))] #![feature(once_cell)] +#![cfg_attr(feature = "deny-warnings", deny(warnings))] +// warn on lints, that are included in `rust-lang/rust`s bootstrap +#![warn(rust_2018_idioms, unused_lifetimes)] use itertools::Itertools; use regex::Regex; diff --git a/src/tools/clippy/clippy_dev/src/main.rs b/src/tools/clippy/clippy_dev/src/main.rs index f4da783502c..7040c257c83 100644 --- a/src/tools/clippy/clippy_dev/src/main.rs +++ b/src/tools/clippy/clippy_dev/src/main.rs @@ -1,4 +1,6 @@ #![cfg_attr(feature = "deny-warnings", deny(warnings))] +// warn on lints, that are included in `rust-lang/rust`s bootstrap +#![warn(rust_2018_idioms, unused_lifetimes)] use clap::{App, Arg, ArgMatches, SubCommand}; use clippy_dev::{bless, fmt, ide_setup, new_lint, serve, stderr_length_check, update_lints}; diff --git a/src/tools/clippy/clippy_lints/Cargo.toml b/src/tools/clippy/clippy_lints/Cargo.toml index 7ceb1da6a6e..48f2972ec58 100644 --- a/src/tools/clippy/clippy_lints/Cargo.toml +++ b/src/tools/clippy/clippy_lints/Cargo.toml @@ -30,7 +30,7 @@ rustc-semver = "1.1.0" url = { version = "2.1.0", features = ["serde"] } [features] -deny-warnings = [] +deny-warnings = ["clippy_utils/deny-warnings"] # build clippy with internal lints enabled, off by default internal-lints = ["clippy_utils/internal-lints"] metadata-collector-lint = ["serde_json", "clippy_utils/metadata-collector-lint"] diff --git a/src/tools/clippy/clippy_lints/src/deprecated_lints.rs b/src/tools/clippy/clippy_lints/src/deprecated_lints.rs index 4688b3d5105..50ffc2e3f19 100644 --- a/src/tools/clippy/clippy_lints/src/deprecated_lints.rs +++ b/src/tools/clippy/clippy_lints/src/deprecated_lints.rs @@ -1,6 +1,13 @@ +/// This struct fakes the `Lint` declaration that is usually created by `declare_lint!`. This +/// enables the simple extraction of the metadata without changing the current deprecation +/// declaration. +pub struct ClippyDeprecatedLint; + macro_rules! declare_deprecated_lint { - (pub $name: ident, $_reason: expr) => { - declare_lint!(pub $name, Allow, "deprecated lint") + { $(#[$attr:meta])* pub $name: ident, $_reason: expr} => { + $(#[$attr])* + #[allow(dead_code)] + pub static $name: ClippyDeprecatedLint = ClippyDeprecatedLint {}; } } diff --git a/src/tools/clippy/clippy_lints/src/derive.rs b/src/tools/clippy/clippy_lints/src/derive.rs index e742cd626ab..840c1eba79d 100644 --- a/src/tools/clippy/clippy_lints/src/derive.rs +++ b/src/tools/clippy/clippy_lints/src/derive.rs @@ -12,7 +12,7 @@ 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}; +use rustc_span::source_map::Span; declare_clippy_lint! { /// **What it does:** Checks for deriving `Hash` but implementing `PartialEq` @@ -310,15 +310,11 @@ fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &T // there's a Copy impl for any instance of the adt. if !is_copy(cx, ty) { if ty_subs.non_erasable_generics().next().is_some() { - let has_copy_impl = cx - .tcx - .all_local_trait_impls(()) - .get(©_id) - .map_or(false, |impls| { - impls - .iter() - .any(|&id| matches!(cx.tcx.type_of(id).kind(), ty::Adt(adt, _) if ty_adt.did == adt.did)) - }); + let has_copy_impl = cx.tcx.all_local_trait_impls(()).get(©_id).map_or(false, |impls| { + impls + .iter() + .any(|&id| matches!(cx.tcx.type_of(id).kind(), ty::Adt(adt, _) if ty_adt.did == adt.did)) + }); if !has_copy_impl { return; } diff --git a/src/tools/clippy/clippy_lints/src/doc.rs b/src/tools/clippy/clippy_lints/src/doc.rs index fb53b55ebd6..e67ec4e06c5 100644 --- a/src/tools/clippy/clippy_lints/src/doc.rs +++ b/src/tools/clippy/clippy_lints/src/doc.rs @@ -383,7 +383,7 @@ pub fn strip_doc_comment_decoration(doc: &str, comment_kind: CommentKind, span: let mut no_stars = String::with_capacity(doc.len()); for line in doc.lines() { let mut chars = line.chars(); - while let Some(c) = chars.next() { + for c in &mut chars { if c.is_whitespace() { no_stars.push(c); } else { diff --git a/src/tools/clippy/clippy_lints/src/floating_point_arithmetic.rs b/src/tools/clippy/clippy_lints/src/floating_point_arithmetic.rs index e0b687b0205..08f28cd54c5 100644 --- a/src/tools/clippy/clippy_lints/src/floating_point_arithmetic.rs +++ b/src/tools/clippy/clippy_lints/src/floating_point_arithmetic.rs @@ -323,7 +323,7 @@ fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) { cx, SUBOPTIMAL_FLOPS, parent.span, - "square can be computed more efficiently", + "multiply and add expressions can be calculated more efficiently and accurately", "consider using", format!( "{}.mul_add({}, {})", @@ -337,16 +337,6 @@ fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) { return; } } - - span_lint_and_sugg( - cx, - SUBOPTIMAL_FLOPS, - expr.span, - "square can be computed more efficiently", - "consider using", - format!("{} * {}", Sugg::hir(cx, &args[0], ".."), Sugg::hir(cx, &args[0], "..")), - Applicability::MachineApplicable, - ); } } } diff --git a/src/tools/clippy/clippy_lints/src/implicit_return.rs b/src/tools/clippy/clippy_lints/src/implicit_return.rs index 30174fa2100..260a8f50157 100644 --- a/src/tools/clippy/clippy_lints/src/implicit_return.rs +++ b/src/tools/clippy/clippy_lints/src/implicit_return.rs @@ -147,7 +147,11 @@ fn lint_implicit_returns( visit_break_exprs(block, |break_expr, dest, sub_expr| { if dest.target_id.ok() == Some(expr.hir_id) { if call_site_span.is_none() && break_expr.span.ctxt() == ctxt { - lint_break(cx, break_expr.span, sub_expr.unwrap().span); + // At this point sub_expr can be `None` in async functions which either diverge, or return the + // unit type. + if let Some(sub_expr) = sub_expr { + lint_break(cx, break_expr.span, sub_expr.span); + } } else { // the break expression is from a macro call, add a return to the loop add_return = true; diff --git a/src/tools/clippy/clippy_lints/src/inconsistent_struct_constructor.rs b/src/tools/clippy/clippy_lints/src/inconsistent_struct_constructor.rs index d138c3a8acf..3b635071f28 100644 --- a/src/tools/clippy/clippy_lints/src/inconsistent_struct_constructor.rs +++ b/src/tools/clippy/clippy_lints/src/inconsistent_struct_constructor.rs @@ -58,7 +58,7 @@ declare_clippy_lint! { /// Foo { x, y }; /// ``` pub INCONSISTENT_STRUCT_CONSTRUCTOR, - style, + pedantic, "the order of the field init shorthand is inconsistent with the order in the struct definition" } diff --git a/src/tools/clippy/clippy_lints/src/inherent_impl.rs b/src/tools/clippy/clippy_lints/src/inherent_impl.rs index 4e0b1ae78df..ee41c4aea2f 100644 --- a/src/tools/clippy/clippy_lints/src/inherent_impl.rs +++ b/src/tools/clippy/clippy_lints/src/inherent_impl.rs @@ -1,12 +1,13 @@ //! lint on inherent implementations -use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::in_macro; -use rustc_hir::def_id::DefIdMap; -use rustc_hir::{Crate, Impl, Item, ItemKind}; +use clippy_utils::diagnostics::span_lint_and_note; +use clippy_utils::{in_macro, is_allowed}; +use rustc_data_structures::fx::FxHashMap; +use rustc_hir::{def_id::LocalDefId, Crate, Item, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::Span; +use std::collections::hash_map::Entry; declare_clippy_lint! { /// **What it does:** Checks for multiple inherent implementations of a struct @@ -40,51 +41,96 @@ declare_clippy_lint! { "Multiple inherent impl that could be grouped" } -#[allow(clippy::module_name_repetitions)] -#[derive(Default)] -pub struct MultipleInherentImpl { - impls: DefIdMap<Span>, -} - -impl_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]); +declare_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]); impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl { - fn check_item(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'_>) { - if let ItemKind::Impl(Impl { - ref generics, - of_trait: None, - .. - }) = item.kind + fn check_crate_post(&mut self, cx: &LateContext<'tcx>, _: &'tcx Crate<'_>) { + // Map from a type to it's first impl block. Needed to distinguish generic arguments. + // e.g. `Foo<Bar>` and `Foo<Baz>` + let mut type_map = FxHashMap::default(); + // List of spans to lint. (lint_span, first_span) + let mut lint_spans = Vec::new(); + + for (_, impl_ids) in cx + .tcx + .crate_inherent_impls(()) + .inherent_impls + .iter() + .filter(|(&id, impls)| { + impls.len() > 1 + // Check for `#[allow]` on the type definition + && !is_allowed( + cx, + MULTIPLE_INHERENT_IMPL, + cx.tcx.hir().local_def_id_to_hir_id(id), + ) + }) { - // Remember for each inherent implementation encountered its span and generics - // but filter out implementations that have generic params (type or lifetime) - // or are derived from a macro - if !in_macro(item.span) && generics.params.is_empty() { - self.impls.insert(item.def_id.to_def_id(), item.span); + for impl_id in impl_ids.iter().map(|id| id.expect_local()) { + match type_map.entry(cx.tcx.type_of(impl_id)) { + Entry::Vacant(e) => { + // Store the id for the first impl block of this type. The span is retrieved lazily. + e.insert(IdOrSpan::Id(impl_id)); + }, + Entry::Occupied(mut e) => { + if let Some(span) = get_impl_span(cx, impl_id) { + let first_span = match *e.get() { + IdOrSpan::Span(s) => s, + IdOrSpan::Id(id) => { + if let Some(s) = get_impl_span(cx, id) { + // Remember the span of the first block. + *e.get_mut() = IdOrSpan::Span(s); + s + } else { + // The first impl block isn't considered by the lint. Replace it with the + // current one. + *e.get_mut() = IdOrSpan::Span(span); + continue; + } + }, + }; + lint_spans.push((span, first_span)); + } + }, + } } + + // Switching to the next type definition, no need to keep the current entries around. + type_map.clear(); } - } - fn check_crate_post(&mut self, cx: &LateContext<'tcx>, krate: &'tcx Crate<'_>) { - if !krate.items.is_empty() { - // Retrieve all inherent implementations from the crate, grouped by type - for impls in cx.tcx.crate_inherent_impls(()).inherent_impls.values() { - // Filter out implementations that have generic params (type or lifetime) - let mut impl_spans = impls.iter().filter_map(|impl_def| self.impls.get(impl_def)); - if let Some(initial_span) = impl_spans.next() { - impl_spans.for_each(|additional_span| { - span_lint_and_then( - cx, - MULTIPLE_INHERENT_IMPL, - *additional_span, - "multiple implementations of this structure", - |diag| { - diag.span_note(*initial_span, "first implementation here"); - }, - ) - }) - } - } + // `TyCtxt::crate_inherent_impls` doesn't have a defined order. Sort the lint output first. + lint_spans.sort_by_key(|x| x.0.lo()); + for (span, first_span) in lint_spans { + span_lint_and_note( + cx, + MULTIPLE_INHERENT_IMPL, + span, + "multiple implementations of this structure", + Some(first_span), + "first implementation here", + ); } } } + +/// Gets the span for the given impl block unless it's not being considered by the lint. +fn get_impl_span(cx: &LateContext<'_>, id: LocalDefId) -> Option<Span> { + let id = cx.tcx.hir().local_def_id_to_hir_id(id); + if let Node::Item(&Item { + kind: ItemKind::Impl(ref impl_item), + span, + .. + }) = cx.tcx.hir().get(id) + { + (!in_macro(span) && impl_item.generics.params.is_empty() && !is_allowed(cx, MULTIPLE_INHERENT_IMPL, id)) + .then(|| span) + } else { + None + } +} + +enum IdOrSpan { + Id(LocalDefId), + Span(Span), +} diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 725aa54157e..eb85cca0bd3 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -163,6 +163,8 @@ macro_rules! extract_msrv_attr { mod consts; #[macro_use] mod utils; +#[cfg(feature = "metadata-collector-lint")] +mod deprecated_lints; // begin lints modules, do not remove this comment, it’s used in `update_lints` mod absurd_extreme_comparisons; @@ -289,6 +291,7 @@ mod mut_reference; mod mutable_debug_assertion; mod mutex_atomic; mod needless_arbitrary_self_type; +mod needless_bitwise_bool; mod needless_bool; mod needless_borrow; mod needless_borrowed_ref; @@ -363,6 +366,7 @@ mod unnecessary_sort_by; mod unnecessary_wraps; mod unnested_or_patterns; mod unsafe_removed_from_name; +mod unused_async; mod unused_io_amount; mod unused_self; mod unused_unit; @@ -834,6 +838,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: mutex_atomic::MUTEX_ATOMIC, mutex_atomic::MUTEX_INTEGER, needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE, + needless_bitwise_bool::NEEDLESS_BITWISE_BOOL, needless_bool::BOOL_COMPARISON, needless_bool::NEEDLESS_BOOL, needless_borrow::NEEDLESS_BORROW, @@ -960,6 +965,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: unnecessary_wraps::UNNECESSARY_WRAPS, unnested_or_patterns::UNNESTED_OR_PATTERNS, unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, + unused_async::UNUSED_ASYNC, unused_io_amount::UNUSED_IO_AMOUNT, unused_self::UNUSED_SELF, unused_unit::UNUSED_UNIT, @@ -1008,7 +1014,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: #[cfg(feature = "metadata-collector-lint")] { if std::env::var("ENABLE_METADATA_COLLECTION").eq(&Ok("1".to_string())) { - store.register_late_pass(|| box utils::internal_lints::metadata_collector::MetadataCollector::default()); + store.register_late_pass(|| box utils::internal_lints::metadata_collector::MetadataCollector::new()); } } @@ -1019,6 +1025,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: let type_complexity_threshold = conf.type_complexity_threshold; store.register_late_pass(move || box types::Types::new(vec_box_size_threshold, type_complexity_threshold)); store.register_late_pass(|| box booleans::NonminimalBool); + store.register_late_pass(|| box needless_bitwise_bool::NeedlessBitwiseBool); store.register_late_pass(|| box eq_op::EqOp); store.register_late_pass(|| box enum_clike::UnportableVariant); store.register_late_pass(|| box float_literal::FloatLiteral); @@ -1155,7 +1162,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| box suspicious_operation_groupings::SuspiciousOperationGroupings); store.register_late_pass(|| box suspicious_trait_impl::SuspiciousImpl); store.register_late_pass(|| box map_unit_fn::MapUnit); - store.register_late_pass(|| box inherent_impl::MultipleInherentImpl::default()); + store.register_late_pass(|| box inherent_impl::MultipleInherentImpl); store.register_late_pass(|| box neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd); store.register_late_pass(|| box unwrap::Unwrap); store.register_late_pass(|| box duration_subsec::DurationSubsec); @@ -1272,6 +1279,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box manual_map::ManualMap); store.register_late_pass(move || box if_then_some_else_none::IfThenSomeElseNone::new(msrv)); store.register_early_pass(|| box bool_assert_comparison::BoolAssertComparison); + store.register_late_pass(|| box unused_async::UnusedAsync); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(arithmetic::FLOAT_ARITHMETIC), @@ -1365,6 +1373,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(if_not_else::IF_NOT_ELSE), LintId::of(implicit_hasher::IMPLICIT_HASHER), LintId::of(implicit_saturating_sub::IMPLICIT_SATURATING_SUB), + LintId::of(inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR), LintId::of(infinite_iter::MAYBE_INFINITE_ITER), LintId::of(invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS), LintId::of(items_after_statements::ITEMS_AFTER_STATEMENTS), @@ -1392,6 +1401,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(misc::USED_UNDERSCORE_BINDING), LintId::of(misc_early::UNSEPARATED_LITERAL_SUFFIX), LintId::of(mut_mut::MUT_MUT), + LintId::of(needless_bitwise_bool::NEEDLESS_BITWISE_BOOL), LintId::of(needless_continue::NEEDLESS_CONTINUE), LintId::of(needless_for_each::NEEDLESS_FOR_EACH), LintId::of(needless_pass_by_value::NEEDLESS_PASS_BY_VALUE), @@ -1415,6 +1425,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(unit_types::LET_UNIT_VALUE), LintId::of(unnecessary_wraps::UNNECESSARY_WRAPS), LintId::of(unnested_or_patterns::UNNESTED_OR_PATTERNS), + LintId::of(unused_async::UNUSED_ASYNC), LintId::of(unused_self::UNUSED_SELF), LintId::of(wildcard_imports::ENUM_GLOB_USE), LintId::of(wildcard_imports::WILDCARD_IMPORTS), @@ -1511,7 +1522,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: 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(inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR), LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING), LintId::of(infinite_iter::INFINITE_ITER), LintId::of(inherent_to_string::INHERENT_TO_STRING), @@ -1764,7 +1774,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(functions::MUST_USE_UNIT), LintId::of(functions::RESULT_UNIT_ERR), LintId::of(if_let_some_result::IF_LET_SOME_RESULT), - LintId::of(inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR), LintId::of(inherent_to_string::INHERENT_TO_STRING), LintId::of(len_zero::COMPARISON_TO_EMPTY), LintId::of(len_zero::LEN_WITHOUT_IS_EMPTY), diff --git a/src/tools/clippy/clippy_lints/src/loops/needless_collect.rs b/src/tools/clippy/clippy_lints/src/loops/needless_collect.rs index 9662a0b22a3..d3406780888 100644 --- a/src/tools/clippy/clippy_lints/src/loops/needless_collect.rs +++ b/src/tools/clippy/clippy_lints/src/loops/needless_collect.rs @@ -1,16 +1,15 @@ use super::NEEDLESS_COLLECT; use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; -use clippy_utils::source::snippet; +use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::{is_type_diagnostic_item, match_type}; -use clippy_utils::{is_trait_method, path_to_local_id, paths}; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{is_trait_method, path_to_local_id}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_block, walk_expr, NestedVisitorMap, Visitor}; use rustc_hir::{Block, Expr, ExprKind, GenericArg, GenericArgs, HirId, Local, Pat, PatKind, QPath, StmtKind, Ty}; use rustc_lint::LateContext; use rustc_middle::hir::map::Map; - use rustc_span::symbol::{sym, Ident}; use rustc_span::{MultiSpan, Span}; @@ -28,23 +27,37 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont if let Some(generic_args) = chain_method.args; if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0); if let Some(ty) = cx.typeck_results().node_type_opt(ty.hir_id); - if is_type_diagnostic_item(cx, ty, sym::vec_type) - || is_type_diagnostic_item(cx, ty, sym::vecdeque_type) - || match_type(cx, ty, &paths::BTREEMAP) - || is_type_diagnostic_item(cx, ty, sym::hashmap_type); - if let Some(sugg) = match &*method.ident.name.as_str() { - "len" => Some("count()".to_string()), - "is_empty" => Some("next().is_none()".to_string()), - "contains" => { - let contains_arg = snippet(cx, args[1].span, "??"); - let (arg, pred) = contains_arg - .strip_prefix('&') - .map_or(("&x", &*contains_arg), |s| ("x", s)); - Some(format!("any(|{}| x == {})", arg, pred)) - } - _ => None, - }; then { + let mut applicability = Applicability::MachineApplicable; + let is_empty_sugg = "next().is_none()".to_string(); + let method_name = &*method.ident.name.as_str(); + let sugg = if is_type_diagnostic_item(cx, ty, sym::vec_type) || + is_type_diagnostic_item(cx, ty, sym::vecdeque_type) || + is_type_diagnostic_item(cx, ty, sym::LinkedList) || + is_type_diagnostic_item(cx, ty, sym::BinaryHeap) { + match method_name { + "len" => "count()".to_string(), + "is_empty" => is_empty_sugg, + "contains" => { + let contains_arg = snippet_with_applicability(cx, args[1].span, "??", &mut applicability); + let (arg, pred) = contains_arg + .strip_prefix('&') + .map_or(("&x", &*contains_arg), |s| ("x", s)); + format!("any(|{}| x == {})", arg, pred) + } + _ => return, + } + } + else if is_type_diagnostic_item(cx, ty, sym::BTreeMap) || + is_type_diagnostic_item(cx, ty, sym::hashmap_type) { + match method_name { + "is_empty" => is_empty_sugg, + _ => return, + } + } + else { + return; + }; span_lint_and_sugg( cx, NEEDLESS_COLLECT, @@ -52,7 +65,7 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont NEEDLESS_COLLECT_MSG, "replace with", sugg, - Applicability::MachineApplicable, + applicability, ); } } @@ -86,7 +99,7 @@ fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCo if is_type_diagnostic_item(cx, ty, sym::vec_type) || is_type_diagnostic_item(cx, ty, sym::vecdeque_type) || is_type_diagnostic_item(cx, ty, sym::BinaryHeap) || - match_type(cx, ty, &paths::LINKED_LIST); + is_type_diagnostic_item(cx, ty, sym::LinkedList); if let Some(iter_calls) = detect_iter_and_into_iters(block, *ident); if let [iter_call] = &*iter_calls; then { diff --git a/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs b/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs index 82715d9bafa..63560047578 100644 --- a/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs @@ -1,170 +1,347 @@ -use super::utils::{LoopNestVisitor, Nesting}; use super::WHILE_LET_ON_ITERATOR; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::ty::implements_trait; -use clippy_utils::usage::mutated_variables; -use clippy_utils::{ - get_enclosing_block, is_refutable, is_trait_method, last_path_segment, path_to_local, path_to_local_id, -}; +use clippy_utils::{get_enclosing_loop, is_refutable, is_trait_method, match_def_path, paths, visitors::is_res_used}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::{walk_block, walk_expr, NestedVisitorMap, Visitor}; -use rustc_hir::{Expr, ExprKind, HirId, MatchSource, Node, PatKind}; +use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}; +use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, MatchSource, Node, PatKind, QPath, UnOp}; use rustc_lint::LateContext; -use rustc_middle::hir::map::Map; -use rustc_span::symbol::sym; +use rustc_span::{symbol::sym, Span, Symbol}; pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Match(match_expr, arms, MatchSource::WhileLetDesugar) = expr.kind { - let pat = &arms[0].pat.kind; - if let (&PatKind::TupleStruct(ref qpath, pat_args, _), &ExprKind::MethodCall(method_path, _, method_args, _)) = - (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) = cx.tcx.get_diagnostic_item(sym::Iterator); - if implements_trait(cx, cx.typeck_results().expr_ty(iter_expr), iter_def_id, &[]); - then { - return; - } - } + let (scrutinee_expr, iter_expr, some_pat, loop_expr) = if_chain! { + if let ExprKind::Match(scrutinee_expr, [arm, _], MatchSource::WhileLetDesugar) = expr.kind; + // check for `Some(..)` pattern + if let PatKind::TupleStruct(QPath::Resolved(None, pat_path), some_pat, _) = arm.pat.kind; + if let Res::Def(_, pat_did) = pat_path.res; + if match_def_path(cx, pat_did, &paths::OPTION_SOME); + // check for call to `Iterator::next` + if let ExprKind::MethodCall(method_name, _, [iter_expr], _) = scrutinee_expr.kind; + if method_name.ident.name == sym::next; + if is_trait_method(cx, scrutinee_expr, sym::Iterator); + if let Some(iter_expr) = try_parse_iter_expr(cx, iter_expr); + // get the loop containing the match expression + if let Some((_, Node::Expr(loop_expr))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1); + if !uses_iter(cx, &iter_expr, arm.body); + then { + (scrutinee_expr, iter_expr, some_pat, loop_expr) + } else { + return; + } + }; - let lhs_constructor = last_path_segment(qpath); - if method_path.ident.name == sym::next - && is_trait_method(cx, match_expr, sym::Iterator) - && lhs_constructor.ident.name == sym::Some - && (pat_args.is_empty() - || !is_refutable(cx, pat_args[0]) - && !is_used_inside(cx, iter_expr, arms[0].body) - && !is_iterator_used_after_while_let(cx, iter_expr) - && !is_nested(cx, expr, &method_args[0])) - { - 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_with_applicability(cx, pat_args[0].span, "_", &mut applicability).into_owned() - }; - span_lint_and_sugg( - cx, - WHILE_LET_ON_ITERATOR, - 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, - ); - } + let mut applicability = Applicability::MachineApplicable; + let loop_var = if let Some(some_pat) = some_pat.first() { + if is_refutable(cx, some_pat) { + // Refutable patterns don't work with for loops. + return; } - } -} + snippet_with_applicability(cx, some_pat.span, "..", &mut applicability) + } else { + "_".into() + }; -fn is_used_inside<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, container: &'tcx Expr<'_>) -> bool { - let def_id = match path_to_local(expr) { - Some(id) => id, - None => return false, + // If the iterator is a field or the iterator is accessed after the loop is complete it needs to be + // borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used + // afterwards a mutable borrow of a field isn't necessary. + let ref_mut = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) { + "&mut " + } else { + "" }; - if let Some(used_mutably) = mutated_variables(container, cx) { - if used_mutably.contains(&def_id) { - return true; + + let iterator = snippet_with_applicability(cx, iter_expr.span, "_", &mut applicability); + span_lint_and_sugg( + cx, + WHILE_LET_ON_ITERATOR, + expr.span.with_hi(scrutinee_expr.span.hi()), + "this loop could be written as a `for` loop", + "try", + format!("for {} in {}{}", loop_var, ref_mut, iterator), + applicability, + ); +} + +#[derive(Debug)] +struct IterExpr { + /// The span of the whole expression, not just the path and fields stored here. + span: Span, + /// The fields used, in order of child to parent. + fields: Vec<Symbol>, + /// The path being used. + path: Res, +} +/// Parses any expression to find out which field of which variable is used. Will return `None` if +/// the expression might have side effects. +fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option<IterExpr> { + let span = e.span; + let mut fields = Vec::new(); + loop { + match e.kind { + ExprKind::Path(ref path) => { + break Some(IterExpr { + span, + fields, + path: cx.qpath_res(path, e.hir_id), + }); + }, + ExprKind::Field(base, name) => { + fields.push(name.name); + e = base; + }, + // Dereferencing a pointer has no side effects and doesn't affect which field is being used. + ExprKind::Unary(UnOp::Deref, base) if cx.typeck_results().expr_ty(base).is_ref() => e = base, + + // Shouldn't have side effects, but there's no way to trace which field is used. So forget which fields have + // already been seen. + ExprKind::Index(base, idx) if !idx.can_have_side_effects() => { + fields.clear(); + e = base; + }, + ExprKind::Unary(UnOp::Deref, base) => { + fields.clear(); + e = base; + }, + + // No effect and doesn't affect which field is being used. + ExprKind::DropTemps(base) | ExprKind::AddrOf(_, _, base) | ExprKind::Type(base, _) => e = base, + _ => break None, } } - false } -fn is_iterator_used_after_while_let<'tcx>(cx: &LateContext<'tcx>, iter_expr: &'tcx Expr<'_>) -> bool { - let def_id = match path_to_local(iter_expr) { - Some(id) => id, - None => return false, - }; - let mut visitor = VarUsedAfterLoopVisitor { - def_id, - iter_expr_id: iter_expr.hir_id, - past_while_let: false, - var_used_after_while_let: false, - }; - if let Some(enclosing_block) = get_enclosing_block(cx, def_id) { - walk_block(&mut visitor, enclosing_block); +fn is_expr_same_field(cx: &LateContext<'_>, mut e: &Expr<'_>, mut fields: &[Symbol], path_res: Res) -> bool { + loop { + match (&e.kind, fields) { + (&ExprKind::Field(base, name), [head_field, tail_fields @ ..]) if name.name == *head_field => { + e = base; + fields = tail_fields; + }, + (ExprKind::Path(path), []) => { + break cx.qpath_res(path, e.hir_id) == path_res; + }, + (&(ExprKind::DropTemps(base) | ExprKind::AddrOf(_, _, base) | ExprKind::Type(base, _)), _) => e = base, + _ => break false, + } } - visitor.var_used_after_while_let } -fn is_nested(cx: &LateContext<'_>, match_expr: &Expr<'_>, iter_expr: &Expr<'_>) -> bool { - if_chain! { - if let Some(loop_block) = get_enclosing_block(cx, match_expr.hir_id); - let parent_node = cx.tcx.hir().get_parent_node(loop_block.hir_id); - if let Some(Node::Expr(loop_expr)) = cx.tcx.hir().find(parent_node); - then { - return is_loop_nested(cx, loop_expr, iter_expr) - } +/// Checks if the given expression is the same field as, is a child of, or is the parent of the +/// given field. Used to check if the expression can be used while the given field is borrowed +/// mutably. e.g. if checking for `x.y`, then `x.y`, `x.y.z`, and `x` will all return true, but +/// `x.z`, and `y` will return false. +fn is_expr_same_child_or_parent_field(cx: &LateContext<'_>, expr: &Expr<'_>, fields: &[Symbol], path_res: Res) -> bool { + match expr.kind { + ExprKind::Field(base, name) => { + if let Some((head_field, tail_fields)) = fields.split_first() { + if name.name == *head_field && is_expr_same_field(cx, base, fields, path_res) { + return true; + } + // Check if the expression is a parent field + let mut fields_iter = tail_fields.iter(); + while let Some(field) = fields_iter.next() { + if *field == name.name && is_expr_same_field(cx, base, fields_iter.as_slice(), path_res) { + return true; + } + } + } + + // Check if the expression is a child field. + let mut e = base; + loop { + match e.kind { + ExprKind::Field(..) if is_expr_same_field(cx, e, fields, path_res) => break true, + ExprKind::Field(base, _) | ExprKind::DropTemps(base) | ExprKind::Type(base, _) => e = base, + ExprKind::Path(ref path) if fields.is_empty() => { + break cx.qpath_res(path, e.hir_id) == path_res; + }, + _ => break false, + } + } + }, + // If the path matches, this is either an exact match, or the expression is a parent of the field. + ExprKind::Path(ref path) => cx.qpath_res(path, expr.hir_id) == path_res, + ExprKind::DropTemps(base) | ExprKind::Type(base, _) | ExprKind::AddrOf(_, _, base) => { + is_expr_same_child_or_parent_field(cx, base, fields, path_res) + }, + _ => false, } - false } -fn is_loop_nested(cx: &LateContext<'_>, loop_expr: &Expr<'_>, iter_expr: &Expr<'_>) -> bool { - let mut id = loop_expr.hir_id; - let iter_id = if let Some(id) = path_to_local(iter_expr) { - id - } else { - return true; +/// Strips off all field and path expressions. This will return true if a field or path has been +/// skipped. Used to skip them after failing to check for equality. +fn skip_fields_and_path(expr: &'tcx Expr<'_>) -> (Option<&'tcx Expr<'tcx>>, bool) { + let mut e = expr; + let e = loop { + match e.kind { + ExprKind::Field(base, _) | ExprKind::DropTemps(base) | ExprKind::Type(base, _) => e = base, + ExprKind::Path(_) => return (None, true), + _ => break e, + } }; - loop { - let parent = cx.tcx.hir().get_parent_node(id); - if parent == id { - return false; + (Some(e), e.hir_id != expr.hir_id) +} + +/// Checks if the given expression uses the iterator. +fn uses_iter(cx: &LateContext<'tcx>, iter_expr: &IterExpr, container: &'tcx Expr<'_>) -> bool { + struct V<'a, 'b, 'tcx> { + cx: &'a LateContext<'tcx>, + iter_expr: &'b IterExpr, + uses_iter: bool, + } + impl Visitor<'tcx> for V<'_, '_, 'tcx> { + type Map = ErasedMap<'tcx>; + fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { + NestedVisitorMap::None } - match cx.tcx.hir().find(parent) { - Some(Node::Expr(expr)) => { - if let ExprKind::Loop(..) = expr.kind { - return true; - }; - }, - Some(Node::Block(block)) => { - let mut block_visitor = LoopNestVisitor { - hir_id: id, - iterator: iter_id, - nesting: Nesting::Unknown, - }; - walk_block(&mut block_visitor, block); - if block_visitor.nesting == Nesting::RuledOut { - return false; + + fn visit_expr(&mut self, e: &'tcx Expr<'_>) { + if self.uses_iter { + // return + } else if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) { + self.uses_iter = true; + } else if let (e, true) = skip_fields_and_path(e) { + if let Some(e) = e { + self.visit_expr(e); } - }, - Some(Node::Stmt(_)) => (), - _ => { - return false; - }, + } else if let ExprKind::Closure(_, _, id, _, _) = e.kind { + if is_res_used(self.cx, self.iter_expr.path, id) { + self.uses_iter = true; + } + } else { + walk_expr(self, e); + } } - id = parent; } -} -struct VarUsedAfterLoopVisitor { - def_id: HirId, - iter_expr_id: HirId, - past_while_let: bool, - var_used_after_while_let: bool, + let mut v = V { + cx, + iter_expr, + uses_iter: false, + }; + v.visit_expr(container); + v.uses_iter } -impl<'tcx> Visitor<'tcx> for VarUsedAfterLoopVisitor { - type Map = Map<'tcx>; +#[allow(clippy::too_many_lines)] +fn needs_mutable_borrow(cx: &LateContext<'tcx>, iter_expr: &IterExpr, loop_expr: &'tcx Expr<'_>) -> bool { + struct AfterLoopVisitor<'a, 'b, 'tcx> { + cx: &'a LateContext<'tcx>, + iter_expr: &'b IterExpr, + loop_id: HirId, + after_loop: bool, + used_iter: bool, + } + impl Visitor<'tcx> for AfterLoopVisitor<'_, '_, 'tcx> { + type Map = ErasedMap<'tcx>; + fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { + NestedVisitorMap::None + } - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if self.past_while_let { - if path_to_local_id(expr, self.def_id) { - self.var_used_after_while_let = true; + fn visit_expr(&mut self, e: &'tcx Expr<'_>) { + if self.used_iter { + return; + } + if self.after_loop { + if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) { + self.used_iter = true; + } else if let (e, true) = skip_fields_and_path(e) { + if let Some(e) = e { + self.visit_expr(e); + } + } else if let ExprKind::Closure(_, _, id, _, _) = e.kind { + self.used_iter = is_res_used(self.cx, self.iter_expr.path, id); + } else { + walk_expr(self, e); + } + } else if self.loop_id == e.hir_id { + self.after_loop = true; + } else { + walk_expr(self, e); + } + } + } + + struct NestedLoopVisitor<'a, 'b, 'tcx> { + cx: &'a LateContext<'tcx>, + iter_expr: &'b IterExpr, + local_id: HirId, + loop_id: HirId, + after_loop: bool, + found_local: bool, + used_after: bool, + } + impl Visitor<'tcx> for NestedLoopVisitor<'a, 'b, 'tcx> { + type Map = ErasedMap<'tcx>; + fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { + NestedVisitorMap::None + } + + fn visit_local(&mut self, l: &'tcx Local<'_>) { + if !self.after_loop { + l.pat.each_binding_or_first(&mut |_, id, _, _| { + if id == self.local_id { + self.found_local = true; + } + }); + } + if let Some(e) = l.init { + self.visit_expr(e); + } + } + + fn visit_expr(&mut self, e: &'tcx Expr<'_>) { + if self.used_after { + return; + } + if self.after_loop { + if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) { + self.used_after = true; + } else if let (e, true) = skip_fields_and_path(e) { + if let Some(e) = e { + self.visit_expr(e); + } + } else if let ExprKind::Closure(_, _, id, _, _) = e.kind { + self.used_after = is_res_used(self.cx, self.iter_expr.path, id); + } else { + walk_expr(self, e); + } + } else if e.hir_id == self.loop_id { + self.after_loop = true; + } else { + walk_expr(self, e); } - } else if self.iter_expr_id == expr.hir_id { - self.past_while_let = true; } - walk_expr(self, expr); } - fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { - NestedVisitorMap::None + + if let Some(e) = get_enclosing_loop(cx.tcx, loop_expr) { + // The iterator expression will be used on the next iteration unless it is declared within the outer + // loop. + let local_id = match iter_expr.path { + Res::Local(id) => id, + _ => return true, + }; + let mut v = NestedLoopVisitor { + cx, + iter_expr, + local_id, + loop_id: loop_expr.hir_id, + after_loop: false, + found_local: false, + used_after: false, + }; + v.visit_expr(e); + v.used_after || !v.found_local + } else { + let mut v = AfterLoopVisitor { + cx, + iter_expr, + loop_id: loop_expr.hir_id, + after_loop: false, + used_iter: false, + }; + v.visit_expr(&cx.tcx.hir().body(cx.enclosing_body.unwrap()).value); + v.used_iter } } diff --git a/src/tools/clippy/clippy_lints/src/macro_use.rs b/src/tools/clippy/clippy_lints/src/macro_use.rs index 314bf11e2d6..914b583186c 100644 --- a/src/tools/clippy/clippy_lints/src/macro_use.rs +++ b/src/tools/clippy/clippy_lints/src/macro_use.rs @@ -47,7 +47,12 @@ pub struct MacroRefData { impl MacroRefData { pub fn new(name: String, callee: Span, cx: &LateContext<'_>) -> Self { - let mut path = cx.sess().source_map().span_to_filename(callee).prefer_local().to_string(); + let mut path = cx + .sess() + .source_map() + .span_to_filename(callee) + .prefer_local() + .to_string(); // std lib paths are <::std::module::file type> // so remove brackets, space and type. @@ -96,7 +101,8 @@ impl MacroUseImports { let name = snippet(cx, cx.sess().source_map().span_until_char(call_site, '!'), "_"); if let Some(callee) = span.source_callee() { if !self.collected.contains(&call_site) { - self.mac_refs.push(MacroRefData::new(name.to_string(), callee.def_site, cx)); + self.mac_refs + .push(MacroRefData::new(name.to_string(), callee.def_site, cx)); self.collected.insert(call_site); } } @@ -174,7 +180,7 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports { .push((*item).to_string()); check_dup.push((*item).to_string()); } - } + }, [root, rest @ ..] => { if rest.iter().all(|item| !check_dup.contains(&(*item).to_string())) { let filtered = rest @@ -198,7 +204,7 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports { .push(rest.join("::")); check_dup.extend(rest.iter().map(ToString::to_string)); } - } + }, } } } diff --git a/src/tools/clippy/clippy_lints/src/manual_unwrap_or.rs b/src/tools/clippy/clippy_lints/src/manual_unwrap_or.rs index 520162559e5..2f579edd6ad 100644 --- a/src/tools/clippy/clippy_lints/src/manual_unwrap_or.rs +++ b/src/tools/clippy/clippy_lints/src/manual_unwrap_or.rs @@ -53,21 +53,6 @@ impl LateLintPass<'_> for ManualUnwrapOr { } } -#[derive(Copy, Clone)] -enum Case { - Option, - Result, -} - -impl Case { - fn unwrap_fn_path(&self) -> &str { - match self { - Case::Option => "Option::unwrap_or", - Case::Result => "Result::unwrap_or", - } - } -} - fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> { if_chain! { @@ -86,6 +71,7 @@ fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk); if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind; if path_to_local_id(unwrap_arm.body, binding_hir_id); + if cx.typeck_results().expr_adjustments(unwrap_arm.body).is_empty(); if !contains_return_break_continue_macro(or_arm.body); then { Some(or_arm) @@ -98,10 +84,10 @@ fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if_chain! { if let ExprKind::Match(scrutinee, match_arms, _) = expr.kind; let ty = cx.typeck_results().expr_ty(scrutinee); - if let Some(case) = if is_type_diagnostic_item(cx, ty, sym::option_type) { - Some(Case::Option) + if let Some(ty_name) = if is_type_diagnostic_item(cx, ty, sym::option_type) { + Some("Option") } else if is_type_diagnostic_item(cx, ty, sym::result_type) { - Some(Case::Result) + Some("Result") } else { None }; @@ -124,7 +110,7 @@ fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { span_lint_and_sugg( cx, MANUAL_UNWRAP_OR, expr.span, - &format!("this pattern reimplements `{}`", case.unwrap_fn_path()), + &format!("this pattern reimplements `{}::unwrap_or`", ty_name), "replace with", format!( "{}.unwrap_or({})", diff --git a/src/tools/clippy/clippy_lints/src/matches.rs b/src/tools/clippy/clippy_lints/src/matches.rs index a70e8b26087..fcd37687010 100644 --- a/src/tools/clippy/clippy_lints/src/matches.rs +++ b/src/tools/clippy/clippy_lints/src/matches.rs @@ -1478,15 +1478,34 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A ); }, PatKind::Wild => { - span_lint_and_sugg( - cx, - MATCH_SINGLE_BINDING, - expr.span, - "this match could be replaced by its body itself", - "consider using the match body instead", - snippet_body, - Applicability::MachineApplicable, - ); + if ex.can_have_side_effects() { + let indent = " ".repeat(indent_of(cx, expr.span).unwrap_or(0)); + let sugg = format!( + "{};\n{}{}", + snippet_with_applicability(cx, ex.span, "..", &mut applicability), + indent, + snippet_body + ); + span_lint_and_sugg( + cx, + MATCH_SINGLE_BINDING, + expr.span, + "this match could be replaced by its scrutinee and body", + "consider using the scrutinee and body instead", + sugg, + applicability, + ) + } else { + span_lint_and_sugg( + cx, + MATCH_SINGLE_BINDING, + expr.span, + "this match could be replaced by its body itself", + "consider using the match body instead", + snippet_body, + Applicability::MachineApplicable, + ); + } }, _ => (), } diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index 0b1b6304def..e0d29682146 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -1838,16 +1838,18 @@ impl<'tcx> LateLintPass<'tcx> for Methods { } } - wrong_self_convention::check( - cx, - &name, - item.vis.node.is_pub(), - self_ty, - first_arg_ty, - first_arg.pat.span, - implements_trait, - false - ); + if sig.decl.implicit_self.has_implicit_self() { + wrong_self_convention::check( + cx, + &name, + item.vis.node.is_pub(), + self_ty, + first_arg_ty, + first_arg.pat.span, + implements_trait, + false + ); + } } } @@ -1903,7 +1905,9 @@ impl<'tcx> LateLintPass<'tcx> for Methods { if_chain! { if let TraitItemKind::Fn(ref sig, _) = item.kind; + if sig.decl.implicit_self.has_implicit_self(); if let Some(first_arg_ty) = sig.decl.inputs.iter().next(); + then { let first_arg_span = first_arg_ty.span; let first_arg_ty = hir_ty_to_ty(cx.tcx, first_arg_ty); diff --git a/src/tools/clippy/clippy_lints/src/methods/wrong_self_convention.rs b/src/tools/clippy/clippy_lints/src/methods/wrong_self_convention.rs index 6e2bcb113c2..1773c26c251 100644 --- a/src/tools/clippy/clippy_lints/src/methods/wrong_self_convention.rs +++ b/src/tools/clippy/clippy_lints/src/methods/wrong_self_convention.rs @@ -22,7 +22,7 @@ const CONVENTIONS: [(&[Convention], &[SelfKind]); 9] = [ // Conversion using `to_` can use borrowed (non-Copy types) or owned (Copy types). // Source: https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv (&[Convention::StartsWith("to_"), Convention::NotEndsWith("_mut"), Convention::IsSelfTypeCopy(false), - Convention::IsTraitItem(false)], &[SelfKind::Ref]), + Convention::IsTraitItem(false), Convention::ImplementsTrait(false)], &[SelfKind::Ref]), (&[Convention::StartsWith("to_"), Convention::NotEndsWith("_mut"), Convention::IsSelfTypeCopy(true), Convention::IsTraitItem(false), Convention::ImplementsTrait(false)], &[SelfKind::Value]), ]; diff --git a/src/tools/clippy/clippy_lints/src/misc.rs b/src/tools/clippy/clippy_lints/src/misc.rs index 6966d798c53..b5d2549242b 100644 --- a/src/tools/clippy/clippy_lints/src/misc.rs +++ b/src/tools/clippy/clippy_lints/src/misc.rs @@ -660,7 +660,14 @@ fn in_attributes_expansion(expr: &Expr<'_>) -> bool { use rustc_span::hygiene::MacroKind; if expr.span.from_expansion() { let data = expr.span.ctxt().outer_expn_data(); - matches!(data.kind, ExpnKind::Macro { kind: MacroKind::Attr, name: _, proc_macro: _ }) + matches!( + data.kind, + ExpnKind::Macro { + kind: MacroKind::Attr, + name: _, + proc_macro: _ + } + ) } else { false } diff --git a/src/tools/clippy/clippy_lints/src/needless_bitwise_bool.rs b/src/tools/clippy/clippy_lints/src/needless_bitwise_bool.rs new file mode 100644 index 00000000000..b30bfbd4294 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/needless_bitwise_bool.rs @@ -0,0 +1,86 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::in_macro; +use clippy_utils::source::snippet_opt; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** + /// Checks for uses of bitwise and/or operators between booleans, where performance may be improved by using + /// a lazy and. + /// + /// **Why is this bad?** + /// The bitwise operators do not support short-circuiting, so it may hinder code performance. + /// Additionally, boolean logic "masked" as bitwise logic is not caught by lints like `unnecessary_fold` + /// + /// **Known problems:** + /// This lint evaluates only when the right side is determined to have no side effects. At this time, that + /// determination is quite conservative. + /// + /// **Example:** + /// + /// ```rust + /// let (x,y) = (true, false); + /// if x & !y {} // where both x and y are booleans + /// ``` + /// Use instead: + /// ```rust + /// let (x,y) = (true, false); + /// if x && !y {} + /// ``` + pub NEEDLESS_BITWISE_BOOL, + pedantic, + "Boolean expressions that use bitwise rather than lazy operators" +} + +declare_lint_pass!(NeedlessBitwiseBool => [NEEDLESS_BITWISE_BOOL]); + +fn is_bitwise_operation(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let ty = cx.typeck_results().expr_ty(expr); + if_chain! { + if !in_macro(expr.span); + if let (&ExprKind::Binary(ref op, _, right), &ty::Bool) = (&expr.kind, &ty.kind()); + if op.node == BinOpKind::BitAnd || op.node == BinOpKind::BitOr; + if let ExprKind::Call(..) | ExprKind::MethodCall(..) | ExprKind::Binary(..) | ExprKind::Unary(..) = right.kind; + if !right.can_have_side_effects(); + then { + return true; + } + } + false +} + +fn suggession_snippet(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> { + if let ExprKind::Binary(ref op, left, right) = expr.kind { + if let (Some(l_snippet), Some(r_snippet)) = (snippet_opt(cx, left.span), snippet_opt(cx, right.span)) { + let op_snippet = match op.node { + BinOpKind::BitAnd => "&&", + _ => "||", + }; + return Some(format!("{} {} {}", l_snippet, op_snippet, r_snippet)); + } + } + None +} + +impl LateLintPass<'_> for NeedlessBitwiseBool { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + if is_bitwise_operation(cx, expr) { + span_lint_and_then( + cx, + NEEDLESS_BITWISE_BOOL, + expr.span, + "use of bitwise operator instead of lazy operator between booleans", + |diag| { + if let Some(sugg) = suggession_snippet(cx, expr) { + diag.span_suggestion(expr.span, "try", sugg, Applicability::MachineApplicable); + } + }, + ); + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/needless_question_mark.rs b/src/tools/clippy/clippy_lints/src/needless_question_mark.rs index d8417c7dc70..c64491c63e2 100644 --- a/src/tools/clippy/clippy_lints/src/needless_question_mark.rs +++ b/src/tools/clippy/clippy_lints/src/needless_question_mark.rs @@ -1,14 +1,13 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_lang_ctor; use clippy_utils::source::snippet; -use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{differing_macro_contexts, is_lang_ctor}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionSome, ResultOk}; use rustc_hir::{Body, Expr, ExprKind, LangItem, MatchSource, QPath}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::TyS; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; declare_clippy_lint! { /// **What it does:** @@ -63,12 +62,6 @@ declare_clippy_lint! { declare_lint_pass!(NeedlessQuestionMark => [NEEDLESS_QUESTION_MARK]); -#[derive(Debug)] -enum SomeOkCall<'a> { - SomeCall(&'a Expr<'a>, &'a Expr<'a>), - OkCall(&'a Expr<'a>, &'a Expr<'a>), -} - impl LateLintPass<'_> for NeedlessQuestionMark { /* * The question mark operator is compatible with both Result<T, E> and Option<T>, @@ -90,104 +83,37 @@ impl LateLintPass<'_> for NeedlessQuestionMark { */ fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) { - let e = match &expr.kind { - ExprKind::Ret(Some(e)) => e, - _ => return, - }; - - if let Some(ok_some_call) = is_some_or_ok_call(cx, e) { - emit_lint(cx, &ok_some_call); + if let ExprKind::Ret(Some(e)) = expr.kind { + check(cx, e); } } fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) { - // Function / Closure block - let expr_opt = if let ExprKind::Block(block, _) = &body.value.kind { - block.expr - } else { - // Single line closure - Some(&body.value) - }; - - if_chain! { - if let Some(expr) = expr_opt; - if let Some(ok_some_call) = is_some_or_ok_call(cx, expr); - then { - emit_lint(cx, &ok_some_call); - } - }; + check(cx, body.value.peel_blocks()); } } -fn emit_lint(cx: &LateContext<'_>, expr: &SomeOkCall<'_>) { - let (entire_expr, inner_expr) = match expr { - SomeOkCall::OkCall(outer, inner) | SomeOkCall::SomeCall(outer, inner) => (outer, inner), +fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { + let inner_expr = if_chain! { + if let ExprKind::Call(path, [arg]) = &expr.kind; + if let ExprKind::Path(ref qpath) = &path.kind; + if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk); + if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar) = &arg.kind; + if let ExprKind::Call(called, [inner_expr]) = &inner_expr_with_q.kind; + if let ExprKind::Path(QPath::LangItem(LangItem::TryTraitBranch, _)) = &called.kind; + if expr.span.ctxt() == inner_expr.span.ctxt(); + let expr_ty = cx.typeck_results().expr_ty(expr); + let inner_ty = cx.typeck_results().expr_ty(inner_expr); + if TyS::same_type(expr_ty, inner_ty); + then { inner_expr } else { return; } }; - span_lint_and_sugg( cx, NEEDLESS_QUESTION_MARK, - entire_expr.span, + expr.span, "question mark operator is useless here", "try", format!("{}", snippet(cx, inner_expr.span, r#""...""#)), Applicability::MachineApplicable, ); } - -fn is_some_or_ok_call<'a>(cx: &'a LateContext<'_>, expr: &'a Expr<'_>) -> Option<SomeOkCall<'a>> { - if_chain! { - // Check outer expression matches CALL_IDENT(ARGUMENT) format - if let ExprKind::Call(path, args) = &expr.kind; - if let ExprKind::Path(ref qpath) = &path.kind; - if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk); - - // Extract inner expression from ARGUMENT - if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar) = &args[0].kind; - if let ExprKind::Call(called, args) = &inner_expr_with_q.kind; - if args.len() == 1; - - if let ExprKind::Path(QPath::LangItem(LangItem::TryTraitBranch, _)) = &called.kind; - then { - // Extract inner expr type from match argument generated by - // question mark operator - let inner_expr = &args[0]; - - // if the inner expr is inside macro but the outer one is not, do not lint (#6921) - if differing_macro_contexts(expr.span, inner_expr.span) { - return None; - } - - let inner_ty = cx.typeck_results().expr_ty(inner_expr); - let outer_ty = cx.typeck_results().expr_ty(expr); - - // Check if outer and inner type are Option - let outer_is_some = is_type_diagnostic_item(cx, outer_ty, sym::option_type); - let inner_is_some = is_type_diagnostic_item(cx, inner_ty, sym::option_type); - - // Check for Option MSRV - if outer_is_some && inner_is_some { - return Some(SomeOkCall::SomeCall(expr, inner_expr)); - } - - // Check if outer and inner type are Result - let outer_is_result = is_type_diagnostic_item(cx, outer_ty, sym::result_type); - let inner_is_result = is_type_diagnostic_item(cx, inner_ty, sym::result_type); - - // Additional check: if the error type of the Result can be converted - // via the From trait, then don't match - let does_not_call_from = !has_implicit_error_from(cx, expr, inner_expr); - - // Must meet Result MSRV - if outer_is_result && inner_is_result && does_not_call_from { - return Some(SomeOkCall::OkCall(expr, inner_expr)); - } - } - } - - None -} - -fn has_implicit_error_from(cx: &LateContext<'_>, entire_expr: &Expr<'_>, inner_result_expr: &Expr<'_>) -> bool { - return cx.typeck_results().expr_ty(entire_expr) != cx.typeck_results().expr_ty(inner_result_expr); -} diff --git a/src/tools/clippy/clippy_lints/src/option_if_let_else.rs b/src/tools/clippy/clippy_lints/src/option_if_let_else.rs index e527adbb892..b6af4175edf 100644 --- a/src/tools/clippy/clippy_lints/src/option_if_let_else.rs +++ b/src/tools/clippy/clippy_lints/src/option_if_let_else.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::usage::contains_return_break_continue_macro; -use clippy_utils::{eager_or_lazy, get_enclosing_block, in_macro, is_lang_ctor}; +use clippy_utils::{eager_or_lazy, in_macro, is_else_clause, is_lang_ctor}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::LangItem::OptionSome; @@ -81,7 +81,6 @@ struct OptionIfLetElseOccurence { method_sugg: String, some_expr: String, none_expr: String, - wrap_braces: bool, } /// Extracts the body of a given arm. If the arm contains only an expression, @@ -106,37 +105,6 @@ fn extract_body_from_arm<'a>(arm: &'a Arm<'a>) -> Option<&'a Expr<'a>> { } } -/// If this is the else body of an if/else expression, then we need to wrap -/// it in curly braces. Otherwise, we don't. -fn should_wrap_in_braces(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - get_enclosing_block(cx, expr.hir_id).map_or(false, |parent| { - let mut should_wrap = false; - - if let Some(Expr { - kind: - ExprKind::Match( - _, - arms, - MatchSource::IfLetDesugar { - contains_else_clause: true, - }, - ), - .. - }) = parent.expr - { - should_wrap = expr.hir_id == arms[1].body.hir_id; - } else if let Some(Expr { - kind: ExprKind::If(_, _, Some(else_clause)), - .. - }) = parent.expr - { - should_wrap = expr.hir_id == else_clause.hir_id; - } - - should_wrap - }) -} - fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: bool, as_mut: bool) -> String { format!( "{}{}", @@ -161,6 +129,7 @@ fn detect_option_if_let_else<'tcx>( if_chain! { if !in_macro(expr.span); // Don't lint macros, because it behaves weirdly if let ExprKind::Match(cond_expr, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind; + if !is_else_clause(cx.tcx, expr); if arms.len() == 2; if !is_result_ok(cx, cond_expr); // Don't lint on Result::ok because a different lint does it already if let PatKind::TupleStruct(struct_qpath, &[inner_pat], _) = &arms[0].pat.kind; @@ -168,13 +137,13 @@ fn detect_option_if_let_else<'tcx>( if let PatKind::Binding(bind_annotation, _, id, _) = &inner_pat.kind; if !contains_return_break_continue_macro(arms[0].body); if !contains_return_break_continue_macro(arms[1].body); + then { let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" }; let some_body = extract_body_from_arm(&arms[0])?; let none_body = extract_body_from_arm(&arms[1])?; let method_sugg = if eager_or_lazy::is_eagerness_candidate(cx, none_body) { "map_or" } else { "map_or_else" }; let capture_name = id.name.to_ident_string(); - let wrap_braces = should_wrap_in_braces(cx, expr); let (as_ref, as_mut) = match &cond_expr.kind { ExprKind::AddrOf(_, Mutability::Not, _) => (true, false), ExprKind::AddrOf(_, Mutability::Mut, _) => (false, true), @@ -190,7 +159,6 @@ fn detect_option_if_let_else<'tcx>( method_sugg: method_sugg.to_string(), some_expr: format!("|{}{}| {}", capture_mut, capture_name, Sugg::hir(cx, some_body, "..")), none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, "..")), - wrap_braces, }) } else { None @@ -208,13 +176,8 @@ impl<'tcx> LateLintPass<'tcx> for OptionIfLetElse { format!("use Option::{} instead of an if let/else", detection.method_sugg).as_str(), "try", format!( - "{}{}.{}({}, {}){}", - if detection.wrap_braces { "{ " } else { "" }, - detection.option, - detection.method_sugg, - detection.none_expr, - detection.some_expr, - if detection.wrap_braces { " }" } else { "" }, + "{}.{}({}, {})", + detection.option, detection.method_sugg, detection.none_expr, detection.some_expr, ), Applicability::MaybeIncorrect, ); diff --git a/src/tools/clippy/clippy_lints/src/unit_types/unit_cmp.rs b/src/tools/clippy/clippy_lints/src/unit_types/unit_cmp.rs index d22f7d9a96b..04542146516 100644 --- a/src/tools/clippy/clippy_lints/src/unit_types/unit_cmp.rs +++ b/src/tools/clippy/clippy_lints/src/unit_types/unit_cmp.rs @@ -8,7 +8,12 @@ use super::UNIT_CMP; pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { if expr.span.from_expansion() { if let Some(callee) = expr.span.source_callee() { - if let ExpnKind::Macro { kind: MacroKind::Bang, name: symbol, proc_macro: _ } = callee.kind { + if let ExpnKind::Macro { + kind: MacroKind::Bang, + name: symbol, + proc_macro: _, + } = callee.kind + { if let ExprKind::Binary(ref cmp, left, _) = expr.kind { let op = cmp.node; if op.is_comparison() && cx.typeck_results().expr_ty(left).is_unit() { diff --git a/src/tools/clippy/clippy_lints/src/unused_async.rs b/src/tools/clippy/clippy_lints/src/unused_async.rs new file mode 100644 index 00000000000..18ee07d3a95 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/unused_async.rs @@ -0,0 +1,92 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use rustc_hir::intravisit::{walk_expr, walk_fn, FnKind, NestedVisitorMap, Visitor}; +use rustc_hir::{Body, Expr, ExprKind, FnDecl, FnHeader, HirId, IsAsync, Item, ItemKind, YieldSource}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::map::Map; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; + +declare_clippy_lint! { + /// **What it does:** Checks for functions that are declared `async` but have no `.await`s inside of them. + /// + /// **Why is this bad?** Async functions with no async code create overhead, both mentally and computationally. + /// Callers of async methods either need to be calling from an async function themselves or run it on an executor, both of which + /// causes runtime overhead and hassle for the caller. + /// + /// **Known problems:** None + /// + /// **Example:** + /// + /// ```rust + /// // Bad + /// async fn get_random_number() -> i64 { + /// 4 // Chosen by fair dice roll. Guaranteed to be random. + /// } + /// let number_future = get_random_number(); + /// + /// // Good + /// fn get_random_number_improved() -> i64 { + /// 4 // Chosen by fair dice roll. Guaranteed to be random. + /// } + /// let number_future = async { get_random_number_improved() }; + /// ``` + pub UNUSED_ASYNC, + pedantic, + "finds async functions with no await statements" +} + +declare_lint_pass!(UnusedAsync => [UNUSED_ASYNC]); + +struct AsyncFnVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + found_await: bool, +} + +impl<'a, 'tcx> Visitor<'tcx> for AsyncFnVisitor<'a, 'tcx> { + type Map = Map<'tcx>; + + fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { + if let ExprKind::Yield(_, YieldSource::Await { .. }) = ex.kind { + self.found_await = true; + } + walk_expr(self, ex); + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { + NestedVisitorMap::OnlyBodies(self.cx.tcx.hir()) + } +} + +impl<'tcx> LateLintPass<'tcx> for UnusedAsync { + fn check_item(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + if let ItemKind::Trait(..) = item.kind { + return; + } + } + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + fn_kind: FnKind<'tcx>, + fn_decl: &'tcx FnDecl<'tcx>, + body: &Body<'tcx>, + span: Span, + hir_id: HirId, + ) { + if let FnKind::ItemFn(_, _, FnHeader { asyncness, .. }, _) = &fn_kind { + if matches!(asyncness, IsAsync::Async) { + let mut visitor = AsyncFnVisitor { cx, found_await: false }; + walk_fn(&mut visitor, fn_kind, fn_decl, body.id(), span, hir_id); + if !visitor.found_await { + span_lint_and_help( + cx, + UNUSED_ASYNC, + span, + "unused `async` for function with no await statements", + None, + "consider removing the `async` from this function", + ); + } + } + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/use_self.rs b/src/tools/clippy/clippy_lints/src/use_self.rs index aa4d16633ff..2ad6fa77f48 100644 --- a/src/tools/clippy/clippy_lints/src/use_self.rs +++ b/src/tools/clippy/clippy_lints/src/use_self.rs @@ -1,12 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_opt; +use clippy_utils::ty::same_type_and_consts; use clippy_utils::{in_macro, meets_msrv, msrvs}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir as hir; -use rustc_hir::def::DefKind; use rustc_hir::{ - def, + self as hir, + def::{self, DefKind}, def_id::LocalDefId, intravisit::{walk_ty, NestedVisitorMap, Visitor}, Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Node, Path, PathSegment, @@ -14,7 +14,7 @@ use rustc_hir::{ }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; -use rustc_middle::ty::{AssocKind, Ty, TyS}; +use rustc_middle::ty::{AssocKind, Ty}; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{BytePos, Span}; @@ -459,7 +459,7 @@ fn in_impl(cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'_>) -> bool { fn should_lint_ty(hir_ty: &hir::Ty<'_>, ty: Ty<'_>, self_ty: Ty<'_>) -> bool { if_chain! { - if TyS::same_type(ty, self_ty); + if same_type_and_consts(ty, self_ty); if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind; then { !matches!(path.res, def::Res::SelfTy(..)) diff --git a/src/tools/clippy/clippy_lints/src/useless_conversion.rs b/src/tools/clippy/clippy_lints/src/useless_conversion.rs index 7edb280be73..2be99fb761b 100644 --- a/src/tools/clippy/clippy_lints/src/useless_conversion.rs +++ b/src/tools/clippy/clippy_lints/src/useless_conversion.rs @@ -1,13 +1,13 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; use clippy_utils::source::{snippet, snippet_with_macro_callsite}; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts}; use clippy_utils::{get_parent_expr, match_def_path, match_trait_method, paths}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, HirId, MatchSource}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, TyS}; +use rustc_middle::ty; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::sym; @@ -67,7 +67,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { if match_trait_method(cx, e, &paths::INTO) && &*name.ident.as_str() == "into" { let a = cx.typeck_results().expr_ty(e); let b = cx.typeck_results().expr_ty(&args[0]); - if TyS::same_type(a, b) { + if same_type_and_consts(a, b) { let sugg = snippet_with_macro_callsite(cx, args[0].span, "<expr>").to_string(); span_lint_and_sugg( cx, @@ -90,7 +90,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { } let a = cx.typeck_results().expr_ty(e); let b = cx.typeck_results().expr_ty(&args[0]); - if TyS::same_type(a, b) { + if same_type_and_consts(a, b) { let sugg = snippet(cx, args[0].span, "<expr>").into_owned(); span_lint_and_sugg( cx, @@ -110,7 +110,8 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { if is_type_diagnostic_item(cx, a, sym::result_type); if let ty::Adt(_, substs) = a.kind(); if let Some(a_type) = substs.types().next(); - if TyS::same_type(a_type, b); + if same_type_and_consts(a_type, b); + then { span_lint_and_help( cx, @@ -137,7 +138,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { if is_type_diagnostic_item(cx, a, sym::result_type); if let ty::Adt(_, substs) = a.kind(); if let Some(a_type) = substs.types().next(); - if TyS::same_type(a_type, b); + if same_type_and_consts(a_type, b); then { let hint = format!("consider removing `{}()`", snippet(cx, path.span, "TryFrom::try_from")); @@ -154,7 +155,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { if_chain! { if match_def_path(cx, def_id, &paths::FROM_FROM); - if TyS::same_type(a, b); + if same_type_and_consts(a, b); then { let sugg = Sugg::hir_with_macro_callsite(cx, &args[0], "<expr>").maybe_par(); diff --git a/src/tools/clippy/clippy_lints/src/utils/conf.rs b/src/tools/clippy/clippy_lints/src/utils/conf.rs index 52c1dc3bdd2..fd2dddb3b96 100644 --- a/src/tools/clippy/clippy_lints/src/utils/conf.rs +++ b/src/tools/clippy/clippy_lints/src/utils/conf.rs @@ -26,13 +26,13 @@ impl TryConf { macro_rules! define_Conf { ($( - #[$doc:meta] + #[doc = $doc:literal] $(#[conf_deprecated($dep:literal)])? ($name:ident: $ty:ty = $default:expr), )*) => { /// Clippy lint configuration pub struct Conf { - $(#[$doc] pub $name: $ty,)* + $(#[doc = $doc] pub $name: $ty,)* } mod defaults { @@ -89,6 +89,34 @@ macro_rules! define_Conf { Ok(TryConf { conf, errors }) } } + + #[cfg(feature = "metadata-collector-lint")] + pub mod metadata { + use crate::utils::internal_lints::metadata_collector::ClippyConfiguration; + + macro_rules! wrap_option { + () => (None); + ($x:literal) => (Some($x)); + } + + pub(crate) fn get_configuration_metadata() -> Vec<ClippyConfiguration> { + vec![ + $( + { + let deprecation_reason = wrap_option!($($dep)?); + + ClippyConfiguration::new( + stringify!($name), + stringify!($ty), + format!("{:?}", super::defaults::$name()), + $doc, + deprecation_reason, + ) + }, + )+ + ] + } + } }; } diff --git a/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs index e85637ca758..e9fa043b20f 100644 --- a/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -8,9 +8,6 @@ //! during any comparison or mapping. (Please take care of this, it's not fun to spend time on such //! a simple mistake) -// # NITs -// - TODO xFrednet 2021-02-13: Collect depreciations and maybe renames - use if_chain::if_chain; use rustc_data_structures::fx::FxHashMap; use rustc_hir::{ @@ -22,13 +19,14 @@ use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{sym, Loc, Span, Symbol}; use serde::{ser::SerializeStruct, Serialize, Serializer}; use std::collections::BinaryHeap; +use std::fmt; use std::fs::{self, OpenOptions}; use std::io::prelude::*; use std::path::Path; use crate::utils::internal_lints::is_lint_ref_type; use clippy_utils::{ - diagnostics::span_lint, last_path_segment, match_function_call, match_path, paths, ty::match_type, + diagnostics::span_lint, last_path_segment, match_def_path, match_function_call, match_path, paths, ty::match_type, ty::walk_ptrs_ty_depth, }; @@ -39,8 +37,52 @@ const BLACK_LISTED_LINTS: [&str; 3] = ["lint_author", "deep_code_inspection", "i /// These groups will be ignored by the lint group matcher. This is useful for collections like /// `clippy::all` const IGNORED_LINT_GROUPS: [&str; 1] = ["clippy::all"]; -/// Lints within this group will be excluded from the collection -const EXCLUDED_LINT_GROUPS: [&str; 1] = ["clippy::internal"]; +/// Lints within this group will be excluded from the collection. These groups +/// have to be defined without the `clippy::` prefix. +const EXCLUDED_LINT_GROUPS: [&str; 1] = ["internal"]; +/// Collected deprecated lint will be assigned to this group in the JSON output +const DEPRECATED_LINT_GROUP_STR: &str = "deprecated"; +/// This is the lint level for deprecated lints that will be displayed in the lint list +const DEPRECATED_LINT_LEVEL: &str = "none"; +/// This array holds Clippy's lint groups with their corresponding default lint level. The +/// lint level for deprecated lints is set in `DEPRECATED_LINT_LEVEL`. +const DEFAULT_LINT_LEVELS: [(&str, &str); 8] = [ + ("correctness", "deny"), + ("restriction", "allow"), + ("style", "warn"), + ("pedantic", "allow"), + ("complexity", "warn"), + ("perf", "warn"), + ("cargo", "allow"), + ("nursery", "allow"), +]; +/// This prefix is in front of the lint groups in the lint store. The prefix will be trimmed +/// to only keep the actual lint group in the output. +const CLIPPY_LINT_GROUP_PREFIX: &str = "clippy::"; + +/// This template will be used to format the configuration section in the lint documentation. +/// The `configurations` parameter will be replaced with one or multiple formatted +/// `ClippyConfiguration` instances. See `CONFIGURATION_VALUE_TEMPLATE` for further customizations +macro_rules! CONFIGURATION_SECTION_TEMPLATE { + () => { + r#" +**Configuration** +This lint has the following configuration variables: + +{configurations} +"# + }; +} +/// This template will be used to format an individual `ClippyConfiguration` instance in the +/// lint documentation. +/// +/// The format function will provide strings for the following parameters: `name`, `ty`, `doc` and +/// `default` +macro_rules! CONFIGURATION_VALUE_TEMPLATE { + () => { + "* {name}: {ty}: {doc} (defaults to `{default}`)\n" + }; +} const LINT_EMISSION_FUNCTIONS: [&[&str]; 7] = [ &["clippy_utils", "diagnostics", "span_lint"], @@ -66,6 +108,7 @@ const SUGGESTION_FUNCTIONS: [&[&str]; 2] = [ &["clippy_utils", "diagnostics", "multispan_sugg"], &["clippy_utils", "diagnostics", "multispan_sugg_with_applicability"], ]; +const DEPRECATED_LINT_TYPE: [&str; 3] = ["clippy_lints", "deprecated_lints", "ClippyDeprecatedLint"]; /// The index of the applicability name of `paths::APPLICABILITY_VALUES` const APPLICABILITY_NAME_INDEX: usize = 2; @@ -102,13 +145,33 @@ declare_clippy_lint! { impl_lint_pass!(MetadataCollector => [INTERNAL_METADATA_COLLECTOR]); #[allow(clippy::module_name_repetitions)] -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone)] pub struct MetadataCollector { /// All collected lints /// /// We use a Heap here to have the lints added in alphabetic order in the export lints: BinaryHeap<LintMetadata>, applicability_info: FxHashMap<String, ApplicabilityInfo>, + config: Vec<ClippyConfiguration>, +} + +impl MetadataCollector { + pub fn new() -> Self { + Self { + lints: BinaryHeap::<LintMetadata>::default(), + applicability_info: FxHashMap::<String, ApplicabilityInfo>::default(), + config: collect_configs(), + } + } + + fn get_lint_configs(&self, lint_name: &str) -> Option<String> { + self.config + .iter() + .filter(|config| config.lints.iter().any(|lint| lint == lint_name)) + .map(ToString::to_string) + .reduce(|acc, x| acc + &x) + .map(|configurations| format!(CONFIGURATION_SECTION_TEMPLATE!(), configurations = configurations)) + } } impl Drop for MetadataCollector { @@ -143,6 +206,7 @@ struct LintMetadata { id: String, id_span: SerializableSpan, group: String, + level: &'static str, docs: String, /// This field is only used in the output and will only be /// mapped shortly before the actual output. @@ -150,11 +214,12 @@ struct LintMetadata { } impl LintMetadata { - fn new(id: String, id_span: SerializableSpan, group: String, docs: String) -> Self { + fn new(id: String, id_span: SerializableSpan, group: String, level: &'static str, docs: String) -> Self { Self { id, id_span, group, + level, docs, applicability: None, } @@ -182,7 +247,7 @@ impl SerializableSpan { let loc: Loc = cx.sess().source_map().lookup_char_pos(span.lo()); Self { - path: format!("{}", loc.file.name), + path: format!("{}", loc.file.name.prefer_remapped()), line: loc.line, } } @@ -214,6 +279,95 @@ impl Serialize for ApplicabilityInfo { } } +// ================================================================== +// Configuration +// ================================================================== +#[derive(Debug, Clone, Default)] +pub struct ClippyConfiguration { + name: String, + config_type: &'static str, + default: String, + lints: Vec<String>, + doc: String, + deprecation_reason: Option<&'static str>, +} + +impl ClippyConfiguration { + pub fn new( + name: &'static str, + config_type: &'static str, + default: String, + doc_comment: &'static str, + deprecation_reason: Option<&'static str>, + ) -> Self { + let (lints, doc) = parse_config_field_doc(doc_comment) + .unwrap_or_else(|| (vec![], "[ERROR] MALFORMED DOC COMMENT".to_string())); + + Self { + name: to_kebab(name), + lints, + doc, + config_type, + default, + deprecation_reason, + } + } +} + +fn collect_configs() -> Vec<ClippyConfiguration> { + crate::utils::conf::metadata::get_configuration_metadata() +} + +/// This parses the field documentation of the config struct. +/// +/// ```rust, ignore +/// parse_config_field_doc(cx, "Lint: LINT_NAME_1, LINT_NAME_2. Papa penguin, papa penguin") +/// ``` +/// +/// Would yield: +/// ```rust, ignore +/// Some(["lint_name_1", "lint_name_2"], "Papa penguin, papa penguin") +/// ``` +fn parse_config_field_doc(doc_comment: &str) -> Option<(Vec<String>, String)> { + const DOC_START: &str = " Lint: "; + if_chain! { + if doc_comment.starts_with(DOC_START); + if let Some(split_pos) = doc_comment.find('.'); + then { + let mut doc_comment = doc_comment.to_string(); + let documentation = doc_comment.split_off(split_pos); + + doc_comment.make_ascii_lowercase(); + let lints: Vec<String> = doc_comment.split_off(DOC_START.len()).split(", ").map(str::to_string).collect(); + + Some((lints, documentation)) + } else { + None + } + } +} + +/// Transforms a given `snake_case_string` to a tasty `kebab-case-string` +fn to_kebab(config_name: &str) -> String { + config_name.replace('_', "-") +} + +impl fmt::Display for ClippyConfiguration { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + CONFIGURATION_VALUE_TEMPLATE!(), + name = self.name, + ty = self.config_type, + doc = self.doc, + default = self.default + ) + } +} + +// ================================================================== +// Lint pass +// ================================================================== impl<'hir> LateLintPass<'hir> for MetadataCollector { /// Collecting lint declarations like: /// ```rust, ignore @@ -225,23 +379,48 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { /// } /// ``` fn check_item(&mut self, cx: &LateContext<'hir>, item: &'hir Item<'_>) { - if_chain! { - // item validation - if let ItemKind::Static(ref ty, Mutability::Not, _) = item.kind; - if is_lint_ref_type(cx, ty); - // blacklist check - let lint_name = sym_to_string(item.ident.name).to_ascii_lowercase(); - if !BLACK_LISTED_LINTS.contains(&lint_name.as_str()); - // metadata extraction - if let Some(group) = get_lint_group_or_lint(cx, &lint_name, item); - if let Some(docs) = extract_attr_docs_or_lint(cx, item); - then { - self.lints.push(LintMetadata::new( - lint_name, - SerializableSpan::from_item(cx, item), - group, - docs, - )); + if let ItemKind::Static(ref ty, Mutability::Not, _) = item.kind { + // Normal lint + if_chain! { + // item validation + if is_lint_ref_type(cx, ty); + // blacklist check + let lint_name = sym_to_string(item.ident.name).to_ascii_lowercase(); + if !BLACK_LISTED_LINTS.contains(&lint_name.as_str()); + // metadata extraction + if let Some((group, level)) = get_lint_group_and_level_or_lint(cx, &lint_name, item); + if let Some(mut docs) = extract_attr_docs_or_lint(cx, item); + then { + if let Some(configuration_section) = self.get_lint_configs(&lint_name) { + docs.push_str(&configuration_section); + } + + self.lints.push(LintMetadata::new( + lint_name, + SerializableSpan::from_item(cx, item), + group, + level, + docs, + )); + } + } + + if_chain! { + if is_deprecated_lint(cx, ty); + // blacklist check + let lint_name = sym_to_string(item.ident.name).to_ascii_lowercase(); + if !BLACK_LISTED_LINTS.contains(&lint_name.as_str()); + // Metadata the little we can get from a deprecated lint + if let Some(docs) = extract_attr_docs_or_lint(cx, item); + then { + self.lints.push(LintMetadata::new( + lint_name, + SerializableSpan::from_item(cx, item), + DEPRECATED_LINT_GROUP_STR.to_string(), + DEPRECATED_LINT_LEVEL, + docs, + )); + } } } } @@ -268,7 +447,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { // - src/misc.rs:734:9 // - src/methods/mod.rs:3545:13 // - src/methods/mod.rs:3496:13 - // We are basically unable to resolve the lint name it self. + // We are basically unable to resolve the lint name itself. return; } @@ -318,15 +497,32 @@ fn extract_attr_docs(cx: &LateContext<'_>, item: &Item<'_>) -> Option<String> { }) } -fn get_lint_group_or_lint(cx: &LateContext<'_>, lint_name: &str, item: &'hir Item<'_>) -> Option<String> { +fn get_lint_group_and_level_or_lint( + cx: &LateContext<'_>, + lint_name: &str, + item: &'hir Item<'_>, +) -> Option<(String, &'static str)> { let result = cx.lint_store.check_lint_name(lint_name, Some(sym::clippy)); if let CheckLintNameResult::Tool(Ok(lint_lst)) = result { - get_lint_group(cx, lint_lst[0]) - .or_else(|| { - lint_collection_error_item(cx, item, "Unable to determine lint group"); + if let Some(group) = get_lint_group(cx, lint_lst[0]) { + if EXCLUDED_LINT_GROUPS.contains(&group.as_str()) { + return None; + } + + if let Some(level) = get_lint_level_from_group(&group) { + Some((group, level)) + } else { + lint_collection_error_item( + cx, + item, + &format!("Unable to determine lint level for found group `{}`", group), + ); None - }) - .filter(|group| !EXCLUDED_LINT_GROUPS.contains(&group.as_str())) + } + } else { + lint_collection_error_item(cx, item, "Unable to determine lint group"); + None + } } else { lint_collection_error_item(cx, item, "Unable to find lint in lint_store"); None @@ -339,14 +535,31 @@ fn get_lint_group(cx: &LateContext<'_>, lint_id: LintId) -> Option<String> { continue; } - if lints.iter().any(|x| *x == lint_id) { - return Some((*group_name).to_string()); + if lints.iter().any(|group_lint| *group_lint == lint_id) { + let group = group_name.strip_prefix(CLIPPY_LINT_GROUP_PREFIX).unwrap_or(group_name); + return Some((*group).to_string()); } } None } +fn get_lint_level_from_group(lint_group: &str) -> Option<&'static str> { + DEFAULT_LINT_LEVELS + .iter() + .find_map(|(group_name, group_level)| (*group_name == lint_group).then(|| *group_level)) +} + +fn is_deprecated_lint(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool { + if let hir::TyKind::Path(ref path) = ty.kind { + if let hir::def::Res::Def(DefKind::Struct, def_id) = cx.qpath_res(path, ty.hir_id) { + return match_def_path(cx, def_id, &DEPRECATED_LINT_TYPE); + } + } + + false +} + // ================================================================== // Lint emission // ================================================================== diff --git a/src/tools/clippy/clippy_lints/src/write.rs b/src/tools/clippy/clippy_lints/src/write.rs index 7e962472c07..d0e79efa70d 100644 --- a/src/tools/clippy/clippy_lints/src/write.rs +++ b/src/tools/clippy/clippy_lints/src/write.rs @@ -279,8 +279,15 @@ impl EarlyLintPass for Write { span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprintln!`"); self.lint_println_empty_string(cx, mac); } else if mac.path == sym!(write) { - if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), true) { + if let (Some(fmt_str), dest) = self.check_tts(cx, mac.args.inner_tokens(), true) { if check_newlines(&fmt_str) { + let (nl_span, only_nl) = newline_span(&fmt_str); + let nl_span = match (dest, only_nl) { + // Special case of `write!(buf, "\n")`: Mark everything from the end of + // `buf` for removal so no trailing comma [`writeln!(buf, )`] remains. + (Some(dest_expr), true) => Span::new(dest_expr.span.hi(), nl_span.hi(), nl_span.ctxt()), + _ => nl_span, + }; span_lint_and_then( cx, WRITE_WITH_NEWLINE, @@ -289,10 +296,7 @@ impl EarlyLintPass for Write { |err| { err.multipart_suggestion( "use `writeln!()` instead", - vec![ - (mac.path.span, String::from("writeln")), - (newline_span(&fmt_str), String::new()), - ], + vec![(mac.path.span, String::from("writeln")), (nl_span, String::new())], Applicability::MachineApplicable, ); }, @@ -329,12 +333,13 @@ impl EarlyLintPass for Write { /// Given a format string that ends in a newline and its span, calculates the span of the /// newline, or the format string itself if the format string consists solely of a newline. -fn newline_span(fmtstr: &StrLit) -> Span { +/// Return this and a boolean indicating whether it only consisted of a newline. +fn newline_span(fmtstr: &StrLit) -> (Span, bool) { let sp = fmtstr.span; let contents = &fmtstr.symbol.as_str(); if *contents == r"\n" { - return sp; + return (sp, true); } let newline_sp_hi = sp.hi() @@ -351,7 +356,7 @@ fn newline_span(fmtstr: &StrLit) -> Span { panic!("expected format string to contain a newline"); }; - sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi) + (sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi), false) } /// Stores a list of replacement spans for each argument, but only if all the replacements used an @@ -613,7 +618,7 @@ impl Write { |err| { err.multipart_suggestion( &format!("use `{}!` instead", suggested), - vec![(mac.path.span, suggested), (newline_span(&fmt_str), String::new())], + vec![(mac.path.span, suggested), (newline_span(&fmt_str).0, String::new())], Applicability::MachineApplicable, ); }, diff --git a/src/tools/clippy/clippy_utils/Cargo.toml b/src/tools/clippy/clippy_utils/Cargo.toml index 0a1d4e11142..93ed3b18400 100644 --- a/src/tools/clippy/clippy_utils/Cargo.toml +++ b/src/tools/clippy/clippy_utils/Cargo.toml @@ -14,6 +14,7 @@ unicode-normalization = "0.1" rustc-semver="1.1.0" [features] +deny-warnings = [] internal-lints = [] metadata-collector-lint = [] diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 9ac9500b4eb..d6dcd4abbd9 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -4,7 +4,14 @@ #![cfg_attr(bootstrap, feature(or_patterns))] #![feature(rustc_private)] #![recursion_limit = "512"] +#![cfg_attr(feature = "deny-warnings", deny(warnings))] #![allow(clippy::missing_errors_doc, clippy::missing_panics_doc, clippy::must_use_candidate)] +// warn on the same lints as `clippy_lints` +#![warn(trivial_casts, trivial_numeric_casts)] +// warn on lints, that are included in `rust-lang/rust`s bootstrap +#![warn(rust_2018_idioms, unused_lifetimes)] +// warn on rustc internal lints +#![warn(rustc::internal)] // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) @@ -855,6 +862,24 @@ pub fn get_enclosing_block<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Optio }) } +/// Gets the loop enclosing the given expression, if any. +pub fn get_enclosing_loop(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> { + let map = tcx.hir(); + for (_, node) in map.parent_iter(expr.hir_id) { + match node { + Node::Expr( + e @ Expr { + kind: ExprKind::Loop(..), + .. + }, + ) => return Some(e), + Node::Expr(_) | Node::Stmt(_) | Node::Block(_) | Node::Local(_) | Node::Arm(_) => (), + _ => break, + } + } + None +} + /// Gets the parent node if it's an impl block. pub fn get_parent_as_impl(tcx: TyCtxt<'_>, id: HirId) -> Option<&Impl<'_>> { let map = tcx.hir(); @@ -947,7 +972,12 @@ pub fn is_expn_of(mut span: Span, name: &str) -> Option<Span> { let data = span.ctxt().outer_expn_data(); let new_span = data.call_site; - if let ExpnKind::Macro { kind: MacroKind::Bang, name: mac_name, proc_macro: _ } = data.kind { + if let ExpnKind::Macro { + kind: MacroKind::Bang, + name: mac_name, + proc_macro: _, + } = data.kind + { if mac_name.as_str() == name { return Some(new_span); } @@ -975,7 +1005,12 @@ pub fn is_direct_expn_of(span: Span, name: &str) -> Option<Span> { let data = span.ctxt().outer_expn_data(); let new_span = data.call_site; - if let ExpnKind::Macro { kind: MacroKind::Bang, name: mac_name, proc_macro: _ } = data.kind { + if let ExpnKind::Macro { + kind: MacroKind::Bang, + name: mac_name, + proc_macro: _, + } = data.kind + { if mac_name.as_str() == name { return Some(new_span); } diff --git a/src/tools/clippy/clippy_utils/src/sugg.rs b/src/tools/clippy/clippy_utils/src/sugg.rs index 0633a19391f..0c950661757 100644 --- a/src/tools/clippy/clippy_utils/src/sugg.rs +++ b/src/tools/clippy/clippy_utils/src/sugg.rs @@ -289,7 +289,7 @@ fn has_enclosing_paren(sugg: impl AsRef<str>) -> bool { let mut chars = sugg.as_ref().chars(); if let Some('(') = chars.next() { let mut depth = 1; - while let Some(c) = chars.next() { + for c in &mut chars { if c == '(' { depth += 1; } else if c == ')' { diff --git a/src/tools/clippy/clippy_utils/src/ty.rs b/src/tools/clippy/clippy_utils/src/ty.rs index 64a80f2554f..c36e215f184 100644 --- a/src/tools/clippy/clippy_utils/src/ty.rs +++ b/src/tools/clippy/clippy_utils/src/ty.rs @@ -2,9 +2,8 @@ #![allow(clippy::module_name_repetitions)] -use std::collections::HashMap; - use rustc_ast::ast::Mutability; +use rustc_data_structures::fx::FxHashMap; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::{TyKind, Unsafety}; @@ -184,14 +183,14 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { /// Checks if `Ty` is normalizable. This function is useful /// to avoid crashes on `layout_of`. pub fn is_normalizable<'tcx>(cx: &LateContext<'tcx>, param_env: ty::ParamEnv<'tcx>, ty: Ty<'tcx>) -> bool { - is_normalizable_helper(cx, param_env, ty, &mut HashMap::new()) + is_normalizable_helper(cx, param_env, ty, &mut FxHashMap::default()) } fn is_normalizable_helper<'tcx>( cx: &LateContext<'tcx>, param_env: ty::ParamEnv<'tcx>, ty: Ty<'tcx>, - cache: &mut HashMap<Ty<'tcx>, bool>, + cache: &mut FxHashMap<Ty<'tcx>, bool>, ) -> bool { if let Some(&cached_result) = cache.get(ty) { return cached_result; @@ -322,3 +321,27 @@ pub fn walk_ptrs_ty_depth(ty: Ty<'_>) -> (Ty<'_>, usize) { } inner(ty, 0) } + +/// Returns `true` if types `a` and `b` are same types having same `Const` generic args, +/// otherwise returns `false` +pub fn same_type_and_consts(a: Ty<'tcx>, b: Ty<'tcx>) -> bool { + match (&a.kind(), &b.kind()) { + (&ty::Adt(did_a, substs_a), &ty::Adt(did_b, substs_b)) => { + if did_a != did_b { + return false; + } + + substs_a + .iter() + .zip(substs_b.iter()) + .all(|(arg_a, arg_b)| match (arg_a.unpack(), arg_b.unpack()) { + (GenericArgKind::Const(inner_a), GenericArgKind::Const(inner_b)) => inner_a == inner_b, + (GenericArgKind::Type(type_a), GenericArgKind::Type(type_b)) => { + same_type_and_consts(type_a, type_b) + }, + _ => true, + }) + }, + _ => a == b, + } +} diff --git a/src/tools/clippy/clippy_utils/src/visitors.rs b/src/tools/clippy/clippy_utils/src/visitors.rs index d431bdf34ee..ecdc666b5f6 100644 --- a/src/tools/clippy/clippy_utils/src/visitors.rs +++ b/src/tools/clippy/clippy_utils/src/visitors.rs @@ -1,7 +1,7 @@ use crate::path_to_local_id; use rustc_hir as hir; use rustc_hir::intravisit::{self, walk_expr, ErasedMap, NestedVisitorMap, Visitor}; -use rustc_hir::{Arm, Block, Body, Destination, Expr, ExprKind, HirId, Stmt}; +use rustc_hir::{def::Res, Arm, Block, Body, BodyId, Destination, Expr, ExprKind, HirId, Stmt}; use rustc_lint::LateContext; use rustc_middle::hir::map::Map; @@ -218,6 +218,7 @@ impl<'tcx> Visitable<'tcx> for &'tcx Arm<'tcx> { } } +/// Calls the given function for each break expression. pub fn visit_break_exprs<'tcx>( node: impl Visitable<'tcx>, f: impl FnMut(&'tcx Expr<'tcx>, Destination, Option<&'tcx Expr<'tcx>>), @@ -239,3 +240,36 @@ pub fn visit_break_exprs<'tcx>( node.visit(&mut V(f)); } + +/// Checks if the given resolved path is used in the given body. +pub fn is_res_used(cx: &LateContext<'_>, res: Res, body: BodyId) -> bool { + struct V<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + res: Res, + found: bool, + } + impl Visitor<'tcx> for V<'_, 'tcx> { + type Map = Map<'tcx>; + fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { + NestedVisitorMap::OnlyBodies(self.cx.tcx.hir()) + } + + fn visit_expr(&mut self, e: &'tcx Expr<'_>) { + if self.found { + return; + } + + if let ExprKind::Path(p) = &e.kind { + if self.cx.qpath_res(p, e.hir_id) == self.res { + self.found = true; + } + } else { + walk_expr(self, e) + } + } + } + + let mut v = V { cx, res, found: false }; + v.visit_expr(&cx.tcx.hir().body(body).value); + v.found +} diff --git a/src/tools/clippy/doc/basics.md b/src/tools/clippy/doc/basics.md index 5226875cc21..e2e307ce4f6 100644 --- a/src/tools/clippy/doc/basics.md +++ b/src/tools/clippy/doc/basics.md @@ -28,6 +28,8 @@ git clone git@github.com:<your-username>/rust-clippy If you've already cloned Clippy in the past, update it to the latest version: ```bash +# If the upstream remote has not been added yet +git remote add upstream https://github.com/rust-lang/rust-clippy # upstream has to be the remote of the rust-lang/rust-clippy repo git fetch upstream # make sure that you are on the master branch diff --git a/src/tools/clippy/rust-toolchain b/src/tools/clippy/rust-toolchain index 593162f09a7..cb8cb0978f6 100644 --- a/src/tools/clippy/rust-toolchain +++ b/src/tools/clippy/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2021-05-06" +channel = "nightly-2021-05-20" components = ["llvm-tools-preview", "rustc-dev", "rust-src"] diff --git a/src/tools/clippy/tests/dogfood.rs b/src/tools/clippy/tests/dogfood.rs index 6524fd4706c..5d9f128753f 100644 --- a/src/tools/clippy/tests/dogfood.rs +++ b/src/tools/clippy/tests/dogfood.rs @@ -22,14 +22,12 @@ fn dogfood_clippy() { return; } let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - let enable_metadata_collection = std::env::var("ENABLE_METADATA_COLLECTION").unwrap_or_else(|_| "0".to_string()); let mut command = Command::new(&*CLIPPY_PATH); command .current_dir(root_dir) .env("CLIPPY_DOGFOOD", "1") .env("CARGO_INCREMENTAL", "0") - .env("ENABLE_METADATA_COLLECTION", &enable_metadata_collection) .arg("clippy") .arg("--all-targets") .arg("--all-features") @@ -157,10 +155,9 @@ fn dogfood_subprojects() { if cargo::is_rustc_test_suite() { return; } - let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); // NOTE: `path_dep` crate is omitted on purpose here - for d in &[ + for project in &[ "clippy_workspace_tests", "clippy_workspace_tests/src", "clippy_workspace_tests/subcrate", @@ -170,34 +167,49 @@ fn dogfood_subprojects() { "clippy_utils", "rustc_tools_util", ] { - let mut command = Command::new(&*CLIPPY_PATH); - command - .current_dir(root_dir.join(d)) - .env("CLIPPY_DOGFOOD", "1") - .env("CARGO_INCREMENTAL", "0") - .arg("clippy") - .arg("--all-targets") - .arg("--all-features") - .arg("--") - .args(&["-D", "clippy::all"]) - .args(&["-D", "clippy::pedantic"]) - .arg("-Cdebuginfo=0"); // disable debuginfo to generate less data in the target dir + run_clippy_for_project(project); + } + + // NOTE: Since tests run in parallel we can't run cargo commands on the same workspace at the + // same time, so we test this immediately after the dogfood for workspaces. + test_no_deps_ignores_path_deps_in_workspaces(); +} - // internal lints only exist if we build with the internal-lints feature - if cfg!(feature = "internal-lints") { - command.args(&["-D", "clippy::internal"]); - } +#[test] +#[ignore] +#[cfg(feature = "metadata-collector-lint")] +fn run_metadata_collection_lint() { + std::env::set_var("ENABLE_METADATA_COLLECTION", "1"); + run_clippy_for_project("clippy_lints"); +} - let output = command.output().unwrap(); +fn run_clippy_for_project(project: &str) { + let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - println!("status: {}", output.status); - println!("stdout: {}", String::from_utf8_lossy(&output.stdout)); - println!("stderr: {}", String::from_utf8_lossy(&output.stderr)); + let mut command = Command::new(&*CLIPPY_PATH); - assert!(output.status.success()); + command + .current_dir(root_dir.join(project)) + .env("CLIPPY_DOGFOOD", "1") + .env("CARGO_INCREMENTAL", "0") + .arg("clippy") + .arg("--all-targets") + .arg("--all-features") + .arg("--") + .args(&["-D", "clippy::all"]) + .args(&["-D", "clippy::pedantic"]) + .arg("-Cdebuginfo=0"); // disable debuginfo to generate less data in the target dir + + // internal lints only exist if we build with the internal-lints feature + if cfg!(feature = "internal-lints") { + command.args(&["-D", "clippy::internal"]); } - // NOTE: Since tests run in parallel we can't run cargo commands on the same workspace at the - // same time, so we test this immediately after the dogfood for workspaces. - test_no_deps_ignores_path_deps_in_workspaces(); + let output = command.output().unwrap(); + + println!("status: {}", output.status); + println!("stdout: {}", String::from_utf8_lossy(&output.stdout)); + println!("stderr: {}", String::from_utf8_lossy(&output.stderr)); + + assert!(output.status.success()); } diff --git a/src/tools/clippy/tests/ui/crashes/ice-7231.rs b/src/tools/clippy/tests/ui/crashes/ice-7231.rs new file mode 100644 index 00000000000..5595d8d1d62 --- /dev/null +++ b/src/tools/clippy/tests/ui/crashes/ice-7231.rs @@ -0,0 +1,10 @@ +// edition:2018 +#![allow(clippy::never_loop)] + +async fn f() { + loop { + break; + } +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/floating_point_powi.fixed b/src/tools/clippy/tests/ui/floating_point_powi.fixed index 56762400593..85f7c531e39 100644 --- a/src/tools/clippy/tests/ui/floating_point_powi.fixed +++ b/src/tools/clippy/tests/ui/floating_point_powi.fixed @@ -4,8 +4,6 @@ fn main() { let one = 1; let x = 3f32; - let _ = x * x; - let _ = x * x; let y = 4f32; let _ = x.mul_add(x, y); @@ -13,7 +11,10 @@ fn main() { let _ = x.mul_add(x, y).sqrt(); let _ = y.mul_add(y, x).sqrt(); // Cases where the lint shouldn't be applied + let _ = x.powi(2); + let _ = x.powi(1 + 1); let _ = x.powi(3); + let _ = x.powi(4) + y; let _ = x.powi(one + 1); let _ = (x.powi(2) + y.powi(2)).sqrt(); } diff --git a/src/tools/clippy/tests/ui/floating_point_powi.rs b/src/tools/clippy/tests/ui/floating_point_powi.rs index 1f800e4628d..ece61d1bec4 100644 --- a/src/tools/clippy/tests/ui/floating_point_powi.rs +++ b/src/tools/clippy/tests/ui/floating_point_powi.rs @@ -4,8 +4,6 @@ fn main() { let one = 1; let x = 3f32; - let _ = x.powi(2); - let _ = x.powi(1 + 1); let y = 4f32; let _ = x.powi(2) + y; @@ -13,7 +11,10 @@ fn main() { let _ = (x.powi(2) + y).sqrt(); let _ = (x + y.powi(2)).sqrt(); // Cases where the lint shouldn't be applied + let _ = x.powi(2); + let _ = x.powi(1 + 1); let _ = x.powi(3); + let _ = x.powi(4) + y; let _ = x.powi(one + 1); let _ = (x.powi(2) + y.powi(2)).sqrt(); } diff --git a/src/tools/clippy/tests/ui/floating_point_powi.stderr b/src/tools/clippy/tests/ui/floating_point_powi.stderr index d5a5f1bcca1..37d840988bb 100644 --- a/src/tools/clippy/tests/ui/floating_point_powi.stderr +++ b/src/tools/clippy/tests/ui/floating_point_powi.stderr @@ -1,40 +1,28 @@ -error: square can be computed more efficiently - --> $DIR/floating_point_powi.rs:7:13 - | -LL | let _ = x.powi(2); - | ^^^^^^^^^ help: consider using: `x * x` - | - = note: `-D clippy::suboptimal-flops` implied by `-D warnings` - -error: square can be computed more efficiently - --> $DIR/floating_point_powi.rs:8:13 - | -LL | let _ = x.powi(1 + 1); - | ^^^^^^^^^^^^^ help: consider using: `x * x` - -error: square can be computed more efficiently - --> $DIR/floating_point_powi.rs:11:13 +error: multiply and add expressions can be calculated more efficiently and accurately + --> $DIR/floating_point_powi.rs:9:13 | LL | let _ = x.powi(2) + y; | ^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, y)` + | + = note: `-D clippy::suboptimal-flops` implied by `-D warnings` -error: square can be computed more efficiently - --> $DIR/floating_point_powi.rs:12:13 +error: multiply and add expressions can be calculated more efficiently and accurately + --> $DIR/floating_point_powi.rs:10:13 | LL | let _ = x + y.powi(2); | ^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)` -error: square can be computed more efficiently - --> $DIR/floating_point_powi.rs:13:13 +error: multiply and add expressions can be calculated more efficiently and accurately + --> $DIR/floating_point_powi.rs:11:13 | LL | let _ = (x.powi(2) + y).sqrt(); | ^^^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, y)` -error: square can be computed more efficiently - --> $DIR/floating_point_powi.rs:14:13 +error: multiply and add expressions can be calculated more efficiently and accurately + --> $DIR/floating_point_powi.rs:12:13 | LL | let _ = (x + y.powi(2)).sqrt(); | ^^^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)` -error: aborting due to 6 previous errors +error: aborting due to 4 previous errors diff --git a/src/tools/clippy/tests/ui/impl.rs b/src/tools/clippy/tests/ui/impl.rs index 1c46e3a5337..39443775015 100644 --- a/src/tools/clippy/tests/ui/impl.rs +++ b/src/tools/clippy/tests/ui/impl.rs @@ -33,4 +33,35 @@ impl fmt::Debug for MyStruct { } } +// issue #5772 +struct WithArgs<T>(T); +impl WithArgs<u32> { + fn f1() {} +} +impl WithArgs<u64> { + fn f2() {} +} +impl WithArgs<u64> { + fn f3() {} +} + +// Ok, the struct is allowed to have multiple impls. +#[allow(clippy::multiple_inherent_impl)] +struct Allowed; +impl Allowed {} +impl Allowed {} +impl Allowed {} + +struct AllowedImpl; +#[allow(clippy::multiple_inherent_impl)] +impl AllowedImpl {} +// Ok, the first block is skipped by this lint. +impl AllowedImpl {} + +struct OneAllowedImpl; +impl OneAllowedImpl {} +#[allow(clippy::multiple_inherent_impl)] +impl OneAllowedImpl {} +impl OneAllowedImpl {} // Lint, only one of the three blocks is allowed. + fn main() {} diff --git a/src/tools/clippy/tests/ui/impl.stderr b/src/tools/clippy/tests/ui/impl.stderr index aab688cc2d8..8703ecac93e 100644 --- a/src/tools/clippy/tests/ui/impl.stderr +++ b/src/tools/clippy/tests/ui/impl.stderr @@ -31,5 +31,33 @@ LL | | fn first() {} LL | | } | |_^ -error: aborting due to 2 previous errors +error: multiple implementations of this structure + --> $DIR/impl.rs:44:1 + | +LL | / impl WithArgs<u64> { +LL | | fn f3() {} +LL | | } + | |_^ + | +note: first implementation here + --> $DIR/impl.rs:41:1 + | +LL | / impl WithArgs<u64> { +LL | | fn f2() {} +LL | | } + | |_^ + +error: multiple implementations of this structure + --> $DIR/impl.rs:65:1 + | +LL | impl OneAllowedImpl {} // Lint, only one of the three blocks is allowed. + | ^^^^^^^^^^^^^^^^^^^^^^ + | +note: first implementation here + --> $DIR/impl.rs:62:1 + | +LL | impl OneAllowedImpl {} + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors diff --git a/src/tools/clippy/tests/ui/manual_unwrap_or.fixed b/src/tools/clippy/tests/ui/manual_unwrap_or.fixed index e7a29596b73..3717f962745 100644 --- a/src/tools/clippy/tests/ui/manual_unwrap_or.fixed +++ b/src/tools/clippy/tests/ui/manual_unwrap_or.fixed @@ -163,4 +163,19 @@ mod issue6965 { } } +use std::rc::Rc; +fn format_name(name: Option<&Rc<str>>) -> &str { + match name { + None => "<anon>", + Some(name) => name, + } +} + +fn implicit_deref_ref() { + let _: &str = match Some(&"bye") { + None => "hi", + Some(s) => s, + }; +} + fn main() {} diff --git a/src/tools/clippy/tests/ui/manual_unwrap_or.rs b/src/tools/clippy/tests/ui/manual_unwrap_or.rs index 66006b6c616..989adde1f5b 100644 --- a/src/tools/clippy/tests/ui/manual_unwrap_or.rs +++ b/src/tools/clippy/tests/ui/manual_unwrap_or.rs @@ -205,4 +205,19 @@ mod issue6965 { } } +use std::rc::Rc; +fn format_name(name: Option<&Rc<str>>) -> &str { + match name { + None => "<anon>", + Some(name) => name, + } +} + +fn implicit_deref_ref() { + let _: &str = match Some(&"bye") { + None => "hi", + Some(s) => s, + }; +} + fn main() {} diff --git a/src/tools/clippy/tests/ui/match_single_binding.fixed b/src/tools/clippy/tests/ui/match_single_binding.fixed index 526e94b10bd..30bf6402253 100644 --- a/src/tools/clippy/tests/ui/match_single_binding.fixed +++ b/src/tools/clippy/tests/ui/match_single_binding.fixed @@ -94,10 +94,7 @@ fn main() { 0 => println!("Disabled branch"), _ => println!("Enabled branch"), } - // Lint - let x = 1; - let y = 1; - println!("Single branch"); + // Ok let x = 1; let y = 1; diff --git a/src/tools/clippy/tests/ui/match_single_binding.rs b/src/tools/clippy/tests/ui/match_single_binding.rs index 6a2ca7c5e93..d8bb80d8b96 100644 --- a/src/tools/clippy/tests/ui/match_single_binding.rs +++ b/src/tools/clippy/tests/ui/match_single_binding.rs @@ -106,15 +106,7 @@ fn main() { 0 => println!("Disabled branch"), _ => println!("Enabled branch"), } - // Lint - let x = 1; - let y = 1; - match match y { - 0 => 1, - _ => 2, - } { - _ => println!("Single branch"), - } + // Ok let x = 1; let y = 1; diff --git a/src/tools/clippy/tests/ui/match_single_binding.stderr b/src/tools/clippy/tests/ui/match_single_binding.stderr index cbbf5d29c02..795c8c3e24d 100644 --- a/src/tools/clippy/tests/ui/match_single_binding.stderr +++ b/src/tools/clippy/tests/ui/match_single_binding.stderr @@ -167,16 +167,5 @@ LL | unwrapped LL | }) | -error: this match could be replaced by its body itself - --> $DIR/match_single_binding.rs:112:5 - | -LL | / match match y { -LL | | 0 => 1, -LL | | _ => 2, -LL | | } { -LL | | _ => println!("Single branch"), -LL | | } - | |_____^ help: consider using the match body instead: `println!("Single branch");` - -error: aborting due to 12 previous errors +error: aborting due to 11 previous errors diff --git a/src/tools/clippy/tests/ui/match_single_binding2.fixed b/src/tools/clippy/tests/ui/match_single_binding2.fixed index e73a85b73d7..a91fcc2125d 100644 --- a/src/tools/clippy/tests/ui/match_single_binding2.fixed +++ b/src/tools/clippy/tests/ui/match_single_binding2.fixed @@ -34,4 +34,20 @@ fn main() { }, None => println!("nothing"), } + + fn side_effects() {} + + // Lint (scrutinee has side effects) + // issue #7094 + side_effects(); + println!("Side effects"); + + // Lint (scrutinee has side effects) + // issue #7094 + let x = 1; + match x { + 0 => 1, + _ => 2, + }; + println!("Single branch"); } diff --git a/src/tools/clippy/tests/ui/match_single_binding2.rs b/src/tools/clippy/tests/ui/match_single_binding2.rs index 7362cb390e5..476386ebabe 100644 --- a/src/tools/clippy/tests/ui/match_single_binding2.rs +++ b/src/tools/clippy/tests/ui/match_single_binding2.rs @@ -34,4 +34,22 @@ fn main() { }, None => println!("nothing"), } + + fn side_effects() {} + + // Lint (scrutinee has side effects) + // issue #7094 + match side_effects() { + _ => println!("Side effects"), + } + + // Lint (scrutinee has side effects) + // issue #7094 + let x = 1; + match match x { + 0 => 1, + _ => 2, + } { + _ => println!("Single branch"), + } } diff --git a/src/tools/clippy/tests/ui/match_single_binding2.stderr b/src/tools/clippy/tests/ui/match_single_binding2.stderr index bc18d191aa3..4372f55af87 100644 --- a/src/tools/clippy/tests/ui/match_single_binding2.stderr +++ b/src/tools/clippy/tests/ui/match_single_binding2.stderr @@ -30,5 +30,39 @@ LL | let (a, b) = get_tup(); LL | println!("a {:?} and b {:?}", a, b); | -error: aborting due to 2 previous errors +error: this match could be replaced by its scrutinee and body + --> $DIR/match_single_binding2.rs:42:5 + | +LL | / match side_effects() { +LL | | _ => println!("Side effects"), +LL | | } + | |_____^ + | +help: consider using the scrutinee and body instead + | +LL | side_effects(); +LL | println!("Side effects"); + | + +error: this match could be replaced by its scrutinee and body + --> $DIR/match_single_binding2.rs:49:5 + | +LL | / match match x { +LL | | 0 => 1, +LL | | _ => 2, +LL | | } { +LL | | _ => println!("Single branch"), +LL | | } + | |_____^ + | +help: consider using the scrutinee and body instead + | +LL | match x { +LL | 0 => 1, +LL | _ => 2, +LL | }; +LL | println!("Single branch"); + | + +error: aborting due to 4 previous errors diff --git a/src/tools/clippy/tests/ui/needless_bitwise_bool.fixed b/src/tools/clippy/tests/ui/needless_bitwise_bool.fixed new file mode 100644 index 00000000000..5e1ea663a10 --- /dev/null +++ b/src/tools/clippy/tests/ui/needless_bitwise_bool.fixed @@ -0,0 +1,40 @@ +// run-rustfix + +#![warn(clippy::needless_bitwise_bool)] + +fn returns_bool() -> bool { + true +} + +const fn const_returns_bool() -> bool { + false +} + +fn main() { + let (x, y) = (false, true); + if x & y { + println!("true") + } + if returns_bool() & x { + println!("true") + } + if !returns_bool() & returns_bool() { + println!("true") + } + if y && !x { + println!("true") + } + + // BELOW: lints we hope to catch as `Expr::can_have_side_effects` improves. + if y & !const_returns_bool() { + println!("true") // This is a const function, in an UnOp + } + + if y & "abcD".is_empty() { + println!("true") // This is a const method call + } + + if y & (0 < 1) { + println!("true") // This is a BinOp with no side effects + } +} diff --git a/src/tools/clippy/tests/ui/needless_bitwise_bool.rs b/src/tools/clippy/tests/ui/needless_bitwise_bool.rs new file mode 100644 index 00000000000..f3075fba0a2 --- /dev/null +++ b/src/tools/clippy/tests/ui/needless_bitwise_bool.rs @@ -0,0 +1,40 @@ +// run-rustfix + +#![warn(clippy::needless_bitwise_bool)] + +fn returns_bool() -> bool { + true +} + +const fn const_returns_bool() -> bool { + false +} + +fn main() { + let (x, y) = (false, true); + if x & y { + println!("true") + } + if returns_bool() & x { + println!("true") + } + if !returns_bool() & returns_bool() { + println!("true") + } + if y & !x { + println!("true") + } + + // BELOW: lints we hope to catch as `Expr::can_have_side_effects` improves. + if y & !const_returns_bool() { + println!("true") // This is a const function, in an UnOp + } + + if y & "abcD".is_empty() { + println!("true") // This is a const method call + } + + if y & (0 < 1) { + println!("true") // This is a BinOp with no side effects + } +} diff --git a/src/tools/clippy/tests/ui/needless_bitwise_bool.stderr b/src/tools/clippy/tests/ui/needless_bitwise_bool.stderr new file mode 100644 index 00000000000..63c88ef63f5 --- /dev/null +++ b/src/tools/clippy/tests/ui/needless_bitwise_bool.stderr @@ -0,0 +1,10 @@ +error: use of bitwise operator instead of lazy operator between booleans + --> $DIR/needless_bitwise_bool.rs:24:8 + | +LL | if y & !x { + | ^^^^^^ help: try: `y && !x` + | + = note: `-D clippy::needless-bitwise-bool` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/src/tools/clippy/tests/ui/needless_collect.fixed b/src/tools/clippy/tests/ui/needless_collect.fixed index af6c7bf15ea..6ecbbcb6249 100644 --- a/src/tools/clippy/tests/ui/needless_collect.fixed +++ b/src/tools/clippy/tests/ui/needless_collect.fixed @@ -2,7 +2,7 @@ #![allow(unused, clippy::suspicious_map, clippy::iter_count)] -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList}; #[warn(clippy::needless_collect)] #[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)] @@ -13,9 +13,24 @@ fn main() { // Empty } sample.iter().cloned().any(|x| x == 1); - sample.iter().map(|x| (x, x)).count(); + // #7164 HashMap's and BTreeMap's `len` usage should not be linted + sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len(); + sample.iter().map(|x| (x, x)).collect::<BTreeMap<_, _>>().len(); + + sample.iter().map(|x| (x, x)).next().is_none(); + sample.iter().map(|x| (x, x)).next().is_none(); + // Notice the `HashSet`--this should not be linted sample.iter().collect::<HashSet<_>>().len(); // Neither should this sample.iter().collect::<BTreeSet<_>>().len(); + + sample.iter().count(); + sample.iter().next().is_none(); + sample.iter().cloned().any(|x| x == 1); + sample.iter().any(|x| x == &1); + + // `BinaryHeap` doesn't have `contains` method + sample.iter().count(); + sample.iter().next().is_none(); } diff --git a/src/tools/clippy/tests/ui/needless_collect.rs b/src/tools/clippy/tests/ui/needless_collect.rs index 6ae14f370b1..8dc69bcf5b3 100644 --- a/src/tools/clippy/tests/ui/needless_collect.rs +++ b/src/tools/clippy/tests/ui/needless_collect.rs @@ -2,7 +2,7 @@ #![allow(unused, clippy::suspicious_map, clippy::iter_count)] -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList}; #[warn(clippy::needless_collect)] #[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)] @@ -13,9 +13,24 @@ fn main() { // Empty } sample.iter().cloned().collect::<Vec<_>>().contains(&1); + // #7164 HashMap's and BTreeMap's `len` usage should not be linted sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len(); + sample.iter().map(|x| (x, x)).collect::<BTreeMap<_, _>>().len(); + + sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().is_empty(); + sample.iter().map(|x| (x, x)).collect::<BTreeMap<_, _>>().is_empty(); + // Notice the `HashSet`--this should not be linted sample.iter().collect::<HashSet<_>>().len(); // Neither should this sample.iter().collect::<BTreeSet<_>>().len(); + + sample.iter().collect::<LinkedList<_>>().len(); + sample.iter().collect::<LinkedList<_>>().is_empty(); + sample.iter().cloned().collect::<LinkedList<_>>().contains(&1); + sample.iter().collect::<LinkedList<_>>().contains(&&1); + + // `BinaryHeap` doesn't have `contains` method + sample.iter().collect::<BinaryHeap<_>>().len(); + sample.iter().collect::<BinaryHeap<_>>().is_empty(); } diff --git a/src/tools/clippy/tests/ui/needless_collect.stderr b/src/tools/clippy/tests/ui/needless_collect.stderr index 2a9539d5975..039091627a8 100644 --- a/src/tools/clippy/tests/ui/needless_collect.stderr +++ b/src/tools/clippy/tests/ui/needless_collect.stderr @@ -19,10 +19,52 @@ LL | sample.iter().cloned().collect::<Vec<_>>().contains(&1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)` error: avoid using `collect()` when not needed - --> $DIR/needless_collect.rs:16:35 + --> $DIR/needless_collect.rs:20:35 | -LL | sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()` +LL | sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()` -error: aborting due to 4 previous errors +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:21:35 + | +LL | sample.iter().map(|x| (x, x)).collect::<BTreeMap<_, _>>().is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()` + +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:28:19 + | +LL | sample.iter().collect::<LinkedList<_>>().len(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()` + +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:29:19 + | +LL | sample.iter().collect::<LinkedList<_>>().is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()` + +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:30:28 + | +LL | sample.iter().cloned().collect::<LinkedList<_>>().contains(&1); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)` + +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:31:19 + | +LL | sample.iter().collect::<LinkedList<_>>().contains(&&1); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == &1)` + +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:34:19 + | +LL | sample.iter().collect::<BinaryHeap<_>>().len(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()` + +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:35:19 + | +LL | sample.iter().collect::<BinaryHeap<_>>().is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()` + +error: aborting due to 11 previous errors diff --git a/src/tools/clippy/tests/ui/needless_question_mark.fixed b/src/tools/clippy/tests/ui/needless_question_mark.fixed index 52ddd9d2dc8..f1fc81aa12b 100644 --- a/src/tools/clippy/tests/ui/needless_question_mark.fixed +++ b/src/tools/clippy/tests/ui/needless_question_mark.fixed @@ -94,6 +94,11 @@ where Ok(x?) } +// not quite needless +fn deref_ref(s: Option<&String>) -> Option<&str> { + Some(s?) +} + fn main() {} // #6921 if a macro wraps an expr in Some( ) and the ? is in the macro use, diff --git a/src/tools/clippy/tests/ui/needless_question_mark.rs b/src/tools/clippy/tests/ui/needless_question_mark.rs index 1ea4ba0d83f..44a0c5f61b5 100644 --- a/src/tools/clippy/tests/ui/needless_question_mark.rs +++ b/src/tools/clippy/tests/ui/needless_question_mark.rs @@ -94,6 +94,11 @@ where Ok(x?) } +// not quite needless +fn deref_ref(s: Option<&String>) -> Option<&str> { + Some(s?) +} + fn main() {} // #6921 if a macro wraps an expr in Some( ) and the ? is in the macro use, diff --git a/src/tools/clippy/tests/ui/needless_question_mark.stderr b/src/tools/clippy/tests/ui/needless_question_mark.stderr index f1f05d1af3a..02bf50d077a 100644 --- a/src/tools/clippy/tests/ui/needless_question_mark.stderr +++ b/src/tools/clippy/tests/ui/needless_question_mark.stderr @@ -67,7 +67,7 @@ LL | return Ok(t.magic?); | ^^^^^^^^^^^^ help: try: `t.magic` error: question mark operator is useless here - --> $DIR/needless_question_mark.rs:115:27 + --> $DIR/needless_question_mark.rs:120:27 | LL | || -> Option<_> { Some(Some($expr)?) }() | ^^^^^^^^^^^^^^^^^^ help: try: `Some($expr)` diff --git a/src/tools/clippy/tests/ui/option_if_let_else.fixed b/src/tools/clippy/tests/ui/option_if_let_else.fixed index 47e7460fa7a..769ccc14bc1 100644 --- a/src/tools/clippy/tests/ui/option_if_let_else.fixed +++ b/src/tools/clippy/tests/ui/option_if_let_else.fixed @@ -10,7 +10,11 @@ fn bad1(string: Option<&str>) -> (bool, &str) { fn else_if_option(string: Option<&str>) -> Option<(bool, &str)> { if string.is_none() { None - } else { string.map_or(Some((false, "")), |x| Some((true, x))) } + } else if let Some(x) = string { + Some((true, x)) + } else { + Some((false, "")) + } } fn unop_bad(string: &Option<&str>, mut num: Option<i32>) { diff --git a/src/tools/clippy/tests/ui/option_if_let_else.stderr b/src/tools/clippy/tests/ui/option_if_let_else.stderr index 7aab068800a..4ebb068d22e 100644 --- a/src/tools/clippy/tests/ui/option_if_let_else.stderr +++ b/src/tools/clippy/tests/ui/option_if_let_else.stderr @@ -11,17 +11,6 @@ LL | | } = note: `-D clippy::option-if-let-else` implied by `-D warnings` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:17:12 - | -LL | } else if let Some(x) = string { - | ____________^ -LL | | Some((true, x)) -LL | | } else { -LL | | Some((false, "")) -LL | | } - | |_____^ help: try: `{ string.map_or(Some((false, "")), |x| Some((true, x))) }` - -error: use Option::map_or instead of an if let/else --> $DIR/option_if_let_else.rs:25:13 | LL | let _ = if let Some(s) = *string { s.len() } else { 0 }; @@ -159,5 +148,5 @@ error: use Option::map_or instead of an if let/else LL | let _ = if let Some(x) = optional { x + 2 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)` -error: aborting due to 12 previous errors +error: aborting due to 11 previous errors diff --git a/src/tools/clippy/tests/ui/unused_async.rs b/src/tools/clippy/tests/ui/unused_async.rs new file mode 100644 index 00000000000..4f4203f5fdb --- /dev/null +++ b/src/tools/clippy/tests/ui/unused_async.rs @@ -0,0 +1,15 @@ +// edition:2018 +#![warn(clippy::unused_async)] + +async fn foo() -> i32 { + 4 +} + +async fn bar() -> i32 { + foo().await +} + +fn main() { + foo(); + bar(); +} diff --git a/src/tools/clippy/tests/ui/unused_async.stderr b/src/tools/clippy/tests/ui/unused_async.stderr new file mode 100644 index 00000000000..8b834d205b1 --- /dev/null +++ b/src/tools/clippy/tests/ui/unused_async.stderr @@ -0,0 +1,13 @@ +error: unused `async` for function with no await statements + --> $DIR/unused_async.rs:4:1 + | +LL | / async fn foo() -> i32 { +LL | | 4 +LL | | } + | |_^ + | + = note: `-D clippy::unused-async` implied by `-D warnings` + = help: consider removing the `async` from this function + +error: aborting due to previous error + diff --git a/src/tools/clippy/tests/ui/use_self.fixed b/src/tools/clippy/tests/ui/use_self.fixed index 1282befdfb3..631da6fe066 100644 --- a/src/tools/clippy/tests/ui/use_self.fixed +++ b/src/tools/clippy/tests/ui/use_self.fixed @@ -462,3 +462,33 @@ mod issue6818 { a: i32, } } + +mod issue7206 { + struct MyStruct<const C: char>; + impl From<MyStruct<'a'>> for MyStruct<'b'> { + fn from(_s: MyStruct<'a'>) -> Self { + Self + } + } + + // keep linting non-`Const` generic args + struct S<'a> { + inner: &'a str, + } + + struct S2<T> { + inner: T, + } + + impl<T> S2<T> { + fn new() -> Self { + unimplemented!(); + } + } + + impl<'a> S2<S<'a>> { + fn new_again() -> Self { + Self::new() + } + } +} diff --git a/src/tools/clippy/tests/ui/use_self.rs b/src/tools/clippy/tests/ui/use_self.rs index 7aaac7b2414..7a10d755faa 100644 --- a/src/tools/clippy/tests/ui/use_self.rs +++ b/src/tools/clippy/tests/ui/use_self.rs @@ -462,3 +462,33 @@ mod issue6818 { a: i32, } } + +mod issue7206 { + struct MyStruct<const C: char>; + impl From<MyStruct<'a'>> for MyStruct<'b'> { + fn from(_s: MyStruct<'a'>) -> Self { + Self + } + } + + // keep linting non-`Const` generic args + struct S<'a> { + inner: &'a str, + } + + struct S2<T> { + inner: T, + } + + impl<T> S2<T> { + fn new() -> Self { + unimplemented!(); + } + } + + impl<'a> S2<S<'a>> { + fn new_again() -> Self { + S2::new() + } + } +} diff --git a/src/tools/clippy/tests/ui/use_self.stderr b/src/tools/clippy/tests/ui/use_self.stderr index a32a9b9157d..cf6222c9b45 100644 --- a/src/tools/clippy/tests/ui/use_self.stderr +++ b/src/tools/clippy/tests/ui/use_self.stderr @@ -162,5 +162,11 @@ error: unnecessary structure name repetition LL | A::new::<submod::B>(submod::B {}) | ^ help: use the applicable keyword: `Self` -error: aborting due to 27 previous errors +error: unnecessary structure name repetition + --> $DIR/use_self.rs:491:13 + | +LL | S2::new() + | ^^ help: use the applicable keyword: `Self` + +error: aborting due to 28 previous errors diff --git a/src/tools/clippy/tests/ui/useless_conversion.fixed b/src/tools/clippy/tests/ui/useless_conversion.fixed index 03977de9455..76aa82068d6 100644 --- a/src/tools/clippy/tests/ui/useless_conversion.fixed +++ b/src/tools/clippy/tests/ui/useless_conversion.fixed @@ -70,4 +70,23 @@ fn main() { let a: i32 = 1; let b: i32 = 1; let _ = (a + b) * 3; + + // see #7205 + let s: Foo<'a'> = Foo; + let _: Foo<'b'> = s.into(); + let s2: Foo<'a'> = Foo; + let _: Foo<'a'> = s2; + let s3: Foo<'a'> = Foo; + let _ = s3; + let s4: Foo<'a'> = Foo; + let _ = vec![s4, s4, s4].into_iter(); +} + +#[derive(Copy, Clone)] +struct Foo<const C: char>; + +impl From<Foo<'a'>> for Foo<'b'> { + fn from(_s: Foo<'a'>) -> Self { + Foo + } } diff --git a/src/tools/clippy/tests/ui/useless_conversion.rs b/src/tools/clippy/tests/ui/useless_conversion.rs index f6e094c1661..ccee7abb404 100644 --- a/src/tools/clippy/tests/ui/useless_conversion.rs +++ b/src/tools/clippy/tests/ui/useless_conversion.rs @@ -70,4 +70,23 @@ fn main() { let a: i32 = 1; let b: i32 = 1; let _ = i32::from(a + b) * 3; + + // see #7205 + let s: Foo<'a'> = Foo; + let _: Foo<'b'> = s.into(); + let s2: Foo<'a'> = Foo; + let _: Foo<'a'> = s2.into(); + let s3: Foo<'a'> = Foo; + let _ = Foo::<'a'>::from(s3); + let s4: Foo<'a'> = Foo; + let _ = vec![s4, s4, s4].into_iter().into_iter(); +} + +#[derive(Copy, Clone)] +struct Foo<const C: char>; + +impl From<Foo<'a'>> for Foo<'b'> { + fn from(_s: Foo<'a'>) -> Self { + Foo + } } diff --git a/src/tools/clippy/tests/ui/useless_conversion.stderr b/src/tools/clippy/tests/ui/useless_conversion.stderr index 26a33595031..e6760f700f3 100644 --- a/src/tools/clippy/tests/ui/useless_conversion.stderr +++ b/src/tools/clippy/tests/ui/useless_conversion.stderr @@ -70,5 +70,23 @@ error: useless conversion to the same type: `i32` LL | let _ = i32::from(a + b) * 3; | ^^^^^^^^^^^^^^^^ help: consider removing `i32::from()`: `(a + b)` -error: aborting due to 11 previous errors +error: useless conversion to the same type: `Foo<'a'>` + --> $DIR/useless_conversion.rs:78:23 + | +LL | let _: Foo<'a'> = s2.into(); + | ^^^^^^^^^ help: consider removing `.into()`: `s2` + +error: useless conversion to the same type: `Foo<'a'>` + --> $DIR/useless_conversion.rs:80:13 + | +LL | let _ = Foo::<'a'>::from(s3); + | ^^^^^^^^^^^^^^^^^^^^ help: consider removing `Foo::<'a'>::from()`: `s3` + +error: useless conversion to the same type: `std::vec::IntoIter<Foo<'a'>>` + --> $DIR/useless_conversion.rs:82:13 + | +LL | let _ = vec![s4, s4, s4].into_iter().into_iter(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()` + +error: aborting due to 14 previous errors diff --git a/src/tools/clippy/tests/ui/while_let_on_iterator.fixed b/src/tools/clippy/tests/ui/while_let_on_iterator.fixed index 749393db124..c3e2cf0c4a4 100644 --- a/src/tools/clippy/tests/ui/while_let_on_iterator.fixed +++ b/src/tools/clippy/tests/ui/while_let_on_iterator.fixed @@ -1,7 +1,7 @@ // run-rustfix #![warn(clippy::while_let_on_iterator)] -#![allow(clippy::never_loop, unreachable_code, unused_mut)] +#![allow(clippy::never_loop, unreachable_code, unused_mut, dead_code)] fn base() { let mut iter = 1..20; @@ -41,13 +41,6 @@ fn base() { // 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; } } @@ -135,18 +128,6 @@ fn refutable2() { 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(); @@ -167,10 +148,8 @@ fn issue1121() { } fn issue2965() { - // This should not cause an ICE and suggest: - // - // for _ in values.iter() {} - // + // This should not cause an ICE + use std::collections::HashSet; let mut values = HashSet::new(); values.insert(1); @@ -205,13 +184,145 @@ fn issue1654() { } } +fn issue6491() { + // Used in outer loop, needs &mut + let mut it = 1..40; + while let Some(n) = it.next() { + for m in &mut it { + if m % 10 == 0 { + break; + } + println!("doing something with m: {}", m); + } + println!("n still is {}", n); + } + + // This is fine, inner loop uses a new iterator. + let mut it = 1..40; + for n in it { + let mut it = 1..40; + for m in it { + if m % 10 == 0 { + break; + } + println!("doing something with m: {}", m); + } + + // Weird binding shouldn't change anything. + let (mut it, _) = (1..40, 0); + for m in it { + if m % 10 == 0 { + break; + } + println!("doing something with m: {}", m); + } + + // Used after the loop, needs &mut. + let mut it = 1..40; + for m in &mut it { + if m % 10 == 0 { + break; + } + println!("doing something with m: {}", m); + } + println!("next item {}", it.next().unwrap()); + + println!("n still is {}", n); + } +} + +fn issue6231() { + // Closure in the outer loop, needs &mut + let mut it = 1..40; + let mut opt = Some(0); + while let Some(n) = opt.take().or_else(|| it.next()) { + for m in &mut it { + if n % 10 == 0 { + break; + } + println!("doing something with m: {}", m); + } + println!("n still is {}", n); + } +} + +fn issue1924() { + struct S<T>(T); + impl<T: Iterator<Item = u32>> S<T> { + fn f(&mut self) -> Option<u32> { + // Used as a field. + for i in &mut self.0 { + if !(3..=7).contains(&i) { + return Some(i); + } + } + None + } + + fn f2(&mut self) -> Option<u32> { + // Don't lint, self borrowed inside the loop + while let Some(i) = self.0.next() { + if i == 1 { + return self.f(); + } + } + None + } + } + impl<T: Iterator<Item = u32>> S<(S<T>, Option<u32>)> { + fn f3(&mut self) -> Option<u32> { + // Don't lint, self borrowed inside the loop + while let Some(i) = self.0.0.0.next() { + if i == 1 { + return self.0.0.f(); + } + } + while let Some(i) = self.0.0.0.next() { + if i == 1 { + return self.f3(); + } + } + // This one is fine, a different field is borrowed + for i in &mut self.0.0.0 { + if i == 1 { + return self.0.1.take(); + } else { + self.0.1 = Some(i); + } + } + None + } + } + + struct S2<T>(T, u32); + impl<T: Iterator<Item = u32>> Iterator for S2<T> { + type Item = u32; + fn next(&mut self) -> Option<u32> { + self.0.next() + } + } + + // Don't lint, field of the iterator is accessed in the loop + let mut it = S2(1..40, 0); + while let Some(n) = it.next() { + if n == it.1 { + break; + } + } + + // Needs &mut, field of the iterator is accessed after the loop + let mut it = S2(1..40, 0); + for n in &mut it { + if n == 0 { + break; + } + } + println!("iterator field {}", it.1); +} + fn main() { - base(); - refutable(); - refutable2(); - nested_loops(); - issue1121(); - issue2965(); - issue3670(); - issue1654(); + let mut it = 0..20; + for _ in it { + println!("test"); + } } diff --git a/src/tools/clippy/tests/ui/while_let_on_iterator.rs b/src/tools/clippy/tests/ui/while_let_on_iterator.rs index 30e3b82a7cc..1717006a449 100644 --- a/src/tools/clippy/tests/ui/while_let_on_iterator.rs +++ b/src/tools/clippy/tests/ui/while_let_on_iterator.rs @@ -1,7 +1,7 @@ // run-rustfix #![warn(clippy::while_let_on_iterator)] -#![allow(clippy::never_loop, unreachable_code, unused_mut)] +#![allow(clippy::never_loop, unreachable_code, unused_mut, dead_code)] fn base() { let mut iter = 1..20; @@ -41,13 +41,6 @@ fn base() { // 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; } } @@ -135,18 +128,6 @@ fn refutable2() { 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(); @@ -167,10 +148,8 @@ fn issue1121() { } fn issue2965() { - // This should not cause an ICE and suggest: - // - // for _ in values.iter() {} - // + // This should not cause an ICE + use std::collections::HashSet; let mut values = HashSet::new(); values.insert(1); @@ -205,13 +184,145 @@ fn issue1654() { } } +fn issue6491() { + // Used in outer loop, needs &mut + let mut it = 1..40; + while let Some(n) = it.next() { + while let Some(m) = it.next() { + if m % 10 == 0 { + break; + } + println!("doing something with m: {}", m); + } + println!("n still is {}", n); + } + + // This is fine, inner loop uses a new iterator. + let mut it = 1..40; + while let Some(n) = it.next() { + let mut it = 1..40; + while let Some(m) = it.next() { + if m % 10 == 0 { + break; + } + println!("doing something with m: {}", m); + } + + // Weird binding shouldn't change anything. + let (mut it, _) = (1..40, 0); + while let Some(m) = it.next() { + if m % 10 == 0 { + break; + } + println!("doing something with m: {}", m); + } + + // Used after the loop, needs &mut. + let mut it = 1..40; + while let Some(m) = it.next() { + if m % 10 == 0 { + break; + } + println!("doing something with m: {}", m); + } + println!("next item {}", it.next().unwrap()); + + println!("n still is {}", n); + } +} + +fn issue6231() { + // Closure in the outer loop, needs &mut + let mut it = 1..40; + let mut opt = Some(0); + while let Some(n) = opt.take().or_else(|| it.next()) { + while let Some(m) = it.next() { + if n % 10 == 0 { + break; + } + println!("doing something with m: {}", m); + } + println!("n still is {}", n); + } +} + +fn issue1924() { + struct S<T>(T); + impl<T: Iterator<Item = u32>> S<T> { + fn f(&mut self) -> Option<u32> { + // Used as a field. + while let Some(i) = self.0.next() { + if i < 3 || i > 7 { + return Some(i); + } + } + None + } + + fn f2(&mut self) -> Option<u32> { + // Don't lint, self borrowed inside the loop + while let Some(i) = self.0.next() { + if i == 1 { + return self.f(); + } + } + None + } + } + impl<T: Iterator<Item = u32>> S<(S<T>, Option<u32>)> { + fn f3(&mut self) -> Option<u32> { + // Don't lint, self borrowed inside the loop + while let Some(i) = self.0.0.0.next() { + if i == 1 { + return self.0.0.f(); + } + } + while let Some(i) = self.0.0.0.next() { + if i == 1 { + return self.f3(); + } + } + // This one is fine, a different field is borrowed + while let Some(i) = self.0.0.0.next() { + if i == 1 { + return self.0.1.take(); + } else { + self.0.1 = Some(i); + } + } + None + } + } + + struct S2<T>(T, u32); + impl<T: Iterator<Item = u32>> Iterator for S2<T> { + type Item = u32; + fn next(&mut self) -> Option<u32> { + self.0.next() + } + } + + // Don't lint, field of the iterator is accessed in the loop + let mut it = S2(1..40, 0); + while let Some(n) = it.next() { + if n == it.1 { + break; + } + } + + // Needs &mut, field of the iterator is accessed after the loop + let mut it = S2(1..40, 0); + while let Some(n) = it.next() { + if n == 0 { + break; + } + } + println!("iterator field {}", it.1); +} + fn main() { - base(); - refutable(); - refutable2(); - nested_loops(); - issue1121(); - issue2965(); - issue3670(); - issue1654(); + let mut it = 0..20; + while let Some(..) = it.next() { + println!("test"); + } } diff --git a/src/tools/clippy/tests/ui/while_let_on_iterator.stderr b/src/tools/clippy/tests/ui/while_let_on_iterator.stderr index 6554977c798..eff559bef7e 100644 --- a/src/tools/clippy/tests/ui/while_let_on_iterator.stderr +++ b/src/tools/clippy/tests/ui/while_let_on_iterator.stderr @@ -19,28 +19,96 @@ LL | while let Some(_) = iter.next() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in iter` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:101:9 + --> $DIR/while_let_on_iterator.rs:94:9 | LL | while let Some([..]) = it.next() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for [..] in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:108:9 + --> $DIR/while_let_on_iterator.rs:101:9 | LL | while let Some([_x]) = it.next() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for [_x] in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:121:9 + --> $DIR/while_let_on_iterator.rs:114:9 | LL | while let Some(x @ [_]) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x @ [_] in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:153:9 + --> $DIR/while_let_on_iterator.rs:134:9 | LL | while let Some(_) = y.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in y` -error: aborting due to 7 previous errors +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:191:9 + | +LL | while let Some(m) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:202:5 + | +LL | while let Some(n) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in it` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:204:9 + | +LL | while let Some(m) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:213:9 + | +LL | while let Some(m) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:222:9 + | +LL | while let Some(m) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:239:9 + | +LL | while let Some(m) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:254:13 + | +LL | while let Some(i) = self.0.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in &mut self.0` + +error: manual `!RangeInclusive::contains` implementation + --> $DIR/while_let_on_iterator.rs:255:20 + | +LL | if i < 3 || i > 7 { + | ^^^^^^^^^^^^^^ help: use: `!(3..=7).contains(&i)` + | + = note: `-D clippy::manual-range-contains` implied by `-D warnings` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:286:13 + | +LL | while let Some(i) = self.0.0.0.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in &mut self.0.0.0` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:315:5 + | +LL | while let Some(n) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in &mut it` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:325:5 + | +LL | while let Some(..) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it` + +error: aborting due to 18 previous errors diff --git a/src/tools/clippy/tests/ui/write_with_newline.stderr b/src/tools/clippy/tests/ui/write_with_newline.stderr index a14e86122ee..cecc2ea9406 100644 --- a/src/tools/clippy/tests/ui/write_with_newline.stderr +++ b/src/tools/clippy/tests/ui/write_with_newline.stderr @@ -51,8 +51,8 @@ LL | write!(&mut v, "/n"); | help: use `writeln!()` instead | -LL | writeln!(&mut v, ); - | ^^^^^^^ -- +LL | writeln!(&mut v); + | ^^^^^^^ -- error: using `write!()` with a format string that ends in a single newline --> $DIR/write_with_newline.rs:36:5 diff --git a/src/tools/clippy/tests/ui/wrong_self_convention2.rs b/src/tools/clippy/tests/ui/wrong_self_convention2.rs index ae3a740d405..3a72174d03d 100644 --- a/src/tools/clippy/tests/ui/wrong_self_convention2.rs +++ b/src/tools/clippy/tests/ui/wrong_self_convention2.rs @@ -23,7 +23,7 @@ mod issue6983 { } struct FooNoCopy; - // trigger lint + // don't trigger impl ToU64 for FooNoCopy { fn to_u64(self) -> u64 { 2 @@ -42,3 +42,30 @@ mod issue7032 { } } } + +mod issue7179 { + pub struct S(i32); + + impl S { + // don't trigger (`s` is not `self`) + pub fn from_be(s: Self) -> Self { + S(i32::from_be(s.0)) + } + + // lint + pub fn from_be_self(self) -> Self { + S(i32::from_be(self.0)) + } + } + + trait T { + // don't trigger (`s` is not `self`) + fn from_be(s: Self) -> Self; + // lint + fn from_be_self(self) -> Self; + } + + trait Foo: Sized { + fn as_byte_slice(slice: &[Self]) -> &[u8]; + } +} diff --git a/src/tools/clippy/tests/ui/wrong_self_convention2.stderr b/src/tools/clippy/tests/ui/wrong_self_convention2.stderr index 0ca1a390974..d2d74ce099e 100644 --- a/src/tools/clippy/tests/ui/wrong_self_convention2.stderr +++ b/src/tools/clippy/tests/ui/wrong_self_convention2.stderr @@ -1,11 +1,19 @@ -error: methods with the following characteristics: (`to_*` and `self` type is not `Copy`) usually take `self` by reference - --> $DIR/wrong_self_convention2.rs:28:19 +error: methods called `from_*` usually take no `self` + --> $DIR/wrong_self_convention2.rs:56:29 | -LL | fn to_u64(self) -> u64 { - | ^^^^ +LL | pub fn from_be_self(self) -> Self { + | ^^^^ | = note: `-D clippy::wrong-self-convention` implied by `-D warnings` = help: consider choosing a less ambiguous name -error: aborting due to previous error +error: methods called `from_*` usually take no `self` + --> $DIR/wrong_self_convention2.rs:65:25 + | +LL | fn from_be_self(self) -> Self; + | ^^^^ + | + = help: consider choosing a less ambiguous name + +error: aborting due to 2 previous errors |