1
Fork 0

Merge commit '9e3cd88718' into clippyup

This commit is contained in:
flip1995 2021-05-20 12:30:31 +02:00
parent 4f3b49fffa
commit 97705b7ea6
80 changed files with 2351 additions and 690 deletions

View file

@ -2,6 +2,7 @@
uitest = "test --test compile-test" uitest = "test --test compile-test"
dev = "run --target-dir clippy_dev/target --package clippy_dev --bin clippy_dev --manifest-path clippy_dev/Cargo.toml --" 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 -- " 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] [build]
rustflags = ["-Zunstable-options"] rustflags = ["-Zunstable-options"]

View file

@ -90,6 +90,11 @@ jobs:
- name: Checkout - name: Checkout
uses: actions/checkout@v2.3.3 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 - name: Install toolchain
run: rustup show active-toolchain run: rustup show active-toolchain

View file

@ -6,11 +6,195 @@ document.
## Unreleased / In Rust Nightly ## 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 ## 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) [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) [#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. * [`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) [#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) [`publish = false`](https://doc.rust-lang.org/cargo/reference/manifest.html#the-publish-field)
is defined in the manifest is defined in the manifest
[#6650](https://github.com/rust-lang/rust-clippy/pull/6650) [#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 * [`useless_format`]: Improved the documentation example
[#6854](https://github.com/rust-lang/rust-clippy/pull/6854) [#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) [#6782](https://github.com/rust-lang/rust-clippy/pull/6782)
### Others ### 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) (`cargo clippy` and `cargo check` no longer shares the same build cache)
[#6687](https://github.com/rust-lang/rust-clippy/pull/6687) [#6687](https://github.com/rust-lang/rust-clippy/pull/6687)
* Cargo now re-runs Clippy if arguments after `--` provided to `cargo clippy` are changed. * 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 ## 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) [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 [`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 [`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_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_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_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 [`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 [`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_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 [`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_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_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 [`unused_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_self

View file

@ -49,7 +49,7 @@ rustc-workspace-hack = "1.0.0"
rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util" } rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util" }
[features] [features]
deny-warnings = [] deny-warnings = ["clippy_lints/deny-warnings"]
integration = ["tempfile"] integration = ["tempfile"]
internal-lints = ["clippy_lints/internal-lints"] internal-lints = ["clippy_lints/internal-lints"]
metadata-collector-lint = ["internal-lints", "clippy_lints/metadata-collector-lint"] metadata-collector-lint = ["internal-lints", "clippy_lints/metadata-collector-lint"]

View file

@ -1,5 +1,7 @@
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
#![feature(once_cell)] #![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 itertools::Itertools;
use regex::Regex; use regex::Regex;

View file

@ -1,4 +1,6 @@
#![cfg_attr(feature = "deny-warnings", deny(warnings))] #![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 clap::{App, Arg, ArgMatches, SubCommand};
use clippy_dev::{bless, fmt, ide_setup, new_lint, serve, stderr_length_check, update_lints}; use clippy_dev::{bless, fmt, ide_setup, new_lint, serve, stderr_length_check, update_lints};

View file

@ -30,7 +30,7 @@ rustc-semver = "1.1.0"
url = { version = "2.1.0", features = ["serde"] } url = { version = "2.1.0", features = ["serde"] }
[features] [features]
deny-warnings = [] deny-warnings = ["clippy_utils/deny-warnings"]
# build clippy with internal lints enabled, off by default # build clippy with internal lints enabled, off by default
internal-lints = ["clippy_utils/internal-lints"] internal-lints = ["clippy_utils/internal-lints"]
metadata-collector-lint = ["serde_json", "clippy_utils/metadata-collector-lint"] metadata-collector-lint = ["serde_json", "clippy_utils/metadata-collector-lint"]

View file

@ -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 { macro_rules! declare_deprecated_lint {
(pub $name: ident, $_reason: expr) => { { $(#[$attr:meta])* pub $name: ident, $_reason: expr} => {
declare_lint!(pub $name, Allow, "deprecated lint") $(#[$attr])*
#[allow(dead_code)]
pub static $name: ClippyDeprecatedLint = ClippyDeprecatedLint {};
} }
} }

View file

@ -12,7 +12,7 @@ use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map; use rustc_middle::hir::map::Map;
use rustc_middle::ty::{self, Ty}; use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{source_map::Span}; use rustc_span::source_map::Span;
declare_clippy_lint! { declare_clippy_lint! {
/// **What it does:** Checks for deriving `Hash` but implementing `PartialEq` /// **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. // there's a Copy impl for any instance of the adt.
if !is_copy(cx, ty) { if !is_copy(cx, ty) {
if ty_subs.non_erasable_generics().next().is_some() { if ty_subs.non_erasable_generics().next().is_some() {
let has_copy_impl = cx let has_copy_impl = cx.tcx.all_local_trait_impls(()).get(&copy_id).map_or(false, |impls| {
.tcx impls
.all_local_trait_impls(()) .iter()
.get(&copy_id) .any(|&id| matches!(cx.tcx.type_of(id).kind(), ty::Adt(adt, _) if ty_adt.did == adt.did))
.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 { if !has_copy_impl {
return; return;
} }

View file

@ -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()); let mut no_stars = String::with_capacity(doc.len());
for line in doc.lines() { for line in doc.lines() {
let mut chars = line.chars(); let mut chars = line.chars();
while let Some(c) = chars.next() { for c in &mut chars {
if c.is_whitespace() { if c.is_whitespace() {
no_stars.push(c); no_stars.push(c);
} else { } else {

View file

@ -323,7 +323,7 @@ fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) {
cx, cx,
SUBOPTIMAL_FLOPS, SUBOPTIMAL_FLOPS,
parent.span, parent.span,
"square can be computed more efficiently", "multiply and add expressions can be calculated more efficiently and accurately",
"consider using", "consider using",
format!( format!(
"{}.mul_add({}, {})", "{}.mul_add({}, {})",
@ -337,16 +337,6 @@ fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) {
return; 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,
);
} }
} }
} }

View file

@ -147,7 +147,11 @@ fn lint_implicit_returns(
visit_break_exprs(block, |break_expr, dest, sub_expr| { visit_break_exprs(block, |break_expr, dest, sub_expr| {
if dest.target_id.ok() == Some(expr.hir_id) { if dest.target_id.ok() == Some(expr.hir_id) {
if call_site_span.is_none() && break_expr.span.ctxt() == ctxt { 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 { } else {
// the break expression is from a macro call, add a return to the loop // the break expression is from a macro call, add a return to the loop
add_return = true; add_return = true;

View file

@ -58,7 +58,7 @@ declare_clippy_lint! {
/// Foo { x, y }; /// Foo { x, y };
/// ``` /// ```
pub INCONSISTENT_STRUCT_CONSTRUCTOR, pub INCONSISTENT_STRUCT_CONSTRUCTOR,
style, pedantic,
"the order of the field init shorthand is inconsistent with the order in the struct definition" "the order of the field init shorthand is inconsistent with the order in the struct definition"
} }

View file

@ -1,12 +1,13 @@
//! lint on inherent implementations //! lint on inherent implementations
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::in_macro; use clippy_utils::{in_macro, is_allowed};
use rustc_hir::def_id::DefIdMap; use rustc_data_structures::fx::FxHashMap;
use rustc_hir::{Crate, Impl, Item, ItemKind}; use rustc_hir::{def_id::LocalDefId, Crate, Item, ItemKind, Node};
use rustc_lint::{LateContext, LateLintPass}; 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 rustc_span::Span;
use std::collections::hash_map::Entry;
declare_clippy_lint! { declare_clippy_lint! {
/// **What it does:** Checks for multiple inherent implementations of a struct /// **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" "Multiple inherent impl that could be grouped"
} }
#[allow(clippy::module_name_repetitions)] declare_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]);
#[derive(Default)]
pub struct MultipleInherentImpl {
impls: DefIdMap<Span>,
}
impl_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]);
impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl { impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
fn check_item(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'_>) { fn check_crate_post(&mut self, cx: &LateContext<'tcx>, _: &'tcx Crate<'_>) {
if let ItemKind::Impl(Impl { // Map from a type to it's first impl block. Needed to distinguish generic arguments.
ref generics, // e.g. `Foo<Bar>` and `Foo<Baz>`
of_trait: None, let mut type_map = FxHashMap::default();
.. // List of spans to lint. (lint_span, first_span)
}) = item.kind let mut lint_spans = Vec::new();
{
// 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);
}
}
}
fn check_crate_post(&mut self, cx: &LateContext<'tcx>, krate: &'tcx Crate<'_>) { for (_, impl_ids) in cx
if !krate.items.is_empty() { .tcx
// Retrieve all inherent implementations from the crate, grouped by type .crate_inherent_impls(())
for impls in cx.tcx.crate_inherent_impls(()).inherent_impls.values() { .inherent_impls
// Filter out implementations that have generic params (type or lifetime) .iter()
let mut impl_spans = impls.iter().filter_map(|impl_def| self.impls.get(impl_def)); .filter(|(&id, impls)| {
if let Some(initial_span) = impl_spans.next() { impls.len() > 1
impl_spans.for_each(|additional_span| { // Check for `#[allow]` on the type definition
span_lint_and_then( && !is_allowed(
cx, cx,
MULTIPLE_INHERENT_IMPL, MULTIPLE_INHERENT_IMPL,
*additional_span, cx.tcx.hir().local_def_id_to_hir_id(id),
"multiple implementations of this structure", )
|diag| { })
diag.span_note(*initial_span, "first implementation here"); {
}, 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();
}
// `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),
}

View file

@ -163,6 +163,8 @@ macro_rules! extract_msrv_attr {
mod consts; mod consts;
#[macro_use] #[macro_use]
mod utils; mod utils;
#[cfg(feature = "metadata-collector-lint")]
mod deprecated_lints;
// begin lints modules, do not remove this comment, its used in `update_lints` // begin lints modules, do not remove this comment, its used in `update_lints`
mod absurd_extreme_comparisons; mod absurd_extreme_comparisons;
@ -289,6 +291,7 @@ mod mut_reference;
mod mutable_debug_assertion; mod mutable_debug_assertion;
mod mutex_atomic; mod mutex_atomic;
mod needless_arbitrary_self_type; mod needless_arbitrary_self_type;
mod needless_bitwise_bool;
mod needless_bool; mod needless_bool;
mod needless_borrow; mod needless_borrow;
mod needless_borrowed_ref; mod needless_borrowed_ref;
@ -363,6 +366,7 @@ mod unnecessary_sort_by;
mod unnecessary_wraps; mod unnecessary_wraps;
mod unnested_or_patterns; mod unnested_or_patterns;
mod unsafe_removed_from_name; mod unsafe_removed_from_name;
mod unused_async;
mod unused_io_amount; mod unused_io_amount;
mod unused_self; mod unused_self;
mod unused_unit; 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_ATOMIC,
mutex_atomic::MUTEX_INTEGER, mutex_atomic::MUTEX_INTEGER,
needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE, needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE,
needless_bitwise_bool::NEEDLESS_BITWISE_BOOL,
needless_bool::BOOL_COMPARISON, needless_bool::BOOL_COMPARISON,
needless_bool::NEEDLESS_BOOL, needless_bool::NEEDLESS_BOOL,
needless_borrow::NEEDLESS_BORROW, needless_borrow::NEEDLESS_BORROW,
@ -960,6 +965,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
unnecessary_wraps::UNNECESSARY_WRAPS, unnecessary_wraps::UNNECESSARY_WRAPS,
unnested_or_patterns::UNNESTED_OR_PATTERNS, unnested_or_patterns::UNNESTED_OR_PATTERNS,
unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
unused_async::UNUSED_ASYNC,
unused_io_amount::UNUSED_IO_AMOUNT, unused_io_amount::UNUSED_IO_AMOUNT,
unused_self::UNUSED_SELF, unused_self::UNUSED_SELF,
unused_unit::UNUSED_UNIT, 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")] #[cfg(feature = "metadata-collector-lint")]
{ {
if std::env::var("ENABLE_METADATA_COLLECTION").eq(&Ok("1".to_string())) { 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; 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(move || box types::Types::new(vec_box_size_threshold, type_complexity_threshold));
store.register_late_pass(|| box booleans::NonminimalBool); 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 eq_op::EqOp);
store.register_late_pass(|| box enum_clike::UnportableVariant); store.register_late_pass(|| box enum_clike::UnportableVariant);
store.register_late_pass(|| box float_literal::FloatLiteral); 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_early_pass(|| box suspicious_operation_groupings::SuspiciousOperationGroupings);
store.register_late_pass(|| box suspicious_trait_impl::SuspiciousImpl); store.register_late_pass(|| box suspicious_trait_impl::SuspiciousImpl);
store.register_late_pass(|| box map_unit_fn::MapUnit); 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 neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd);
store.register_late_pass(|| box unwrap::Unwrap); store.register_late_pass(|| box unwrap::Unwrap);
store.register_late_pass(|| box duration_subsec::DurationSubsec); 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(|| box manual_map::ManualMap);
store.register_late_pass(move || box if_then_some_else_none::IfThenSomeElseNone::new(msrv)); 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_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![ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(arithmetic::FLOAT_ARITHMETIC), 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(if_not_else::IF_NOT_ELSE),
LintId::of(implicit_hasher::IMPLICIT_HASHER), LintId::of(implicit_hasher::IMPLICIT_HASHER),
LintId::of(implicit_saturating_sub::IMPLICIT_SATURATING_SUB), 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(infinite_iter::MAYBE_INFINITE_ITER),
LintId::of(invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS), LintId::of(invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS),
LintId::of(items_after_statements::ITEMS_AFTER_STATEMENTS), 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::USED_UNDERSCORE_BINDING),
LintId::of(misc_early::UNSEPARATED_LITERAL_SUFFIX), LintId::of(misc_early::UNSEPARATED_LITERAL_SUFFIX),
LintId::of(mut_mut::MUT_MUT), LintId::of(mut_mut::MUT_MUT),
LintId::of(needless_bitwise_bool::NEEDLESS_BITWISE_BOOL),
LintId::of(needless_continue::NEEDLESS_CONTINUE), LintId::of(needless_continue::NEEDLESS_CONTINUE),
LintId::of(needless_for_each::NEEDLESS_FOR_EACH), LintId::of(needless_for_each::NEEDLESS_FOR_EACH),
LintId::of(needless_pass_by_value::NEEDLESS_PASS_BY_VALUE), 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(unit_types::LET_UNIT_VALUE),
LintId::of(unnecessary_wraps::UNNECESSARY_WRAPS), LintId::of(unnecessary_wraps::UNNECESSARY_WRAPS),
LintId::of(unnested_or_patterns::UNNESTED_OR_PATTERNS), LintId::of(unnested_or_patterns::UNNESTED_OR_PATTERNS),
LintId::of(unused_async::UNUSED_ASYNC),
LintId::of(unused_self::UNUSED_SELF), LintId::of(unused_self::UNUSED_SELF),
LintId::of(wildcard_imports::ENUM_GLOB_USE), LintId::of(wildcard_imports::ENUM_GLOB_USE),
LintId::of(wildcard_imports::WILDCARD_IMPORTS), 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(identity_op::IDENTITY_OP),
LintId::of(if_let_mutex::IF_LET_MUTEX), LintId::of(if_let_mutex::IF_LET_MUTEX),
LintId::of(if_let_some_result::IF_LET_SOME_RESULT), 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(indexing_slicing::OUT_OF_BOUNDS_INDEXING),
LintId::of(infinite_iter::INFINITE_ITER), LintId::of(infinite_iter::INFINITE_ITER),
LintId::of(inherent_to_string::INHERENT_TO_STRING), 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::MUST_USE_UNIT),
LintId::of(functions::RESULT_UNIT_ERR), LintId::of(functions::RESULT_UNIT_ERR),
LintId::of(if_let_some_result::IF_LET_SOME_RESULT), 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(inherent_to_string::INHERENT_TO_STRING),
LintId::of(len_zero::COMPARISON_TO_EMPTY), LintId::of(len_zero::COMPARISON_TO_EMPTY),
LintId::of(len_zero::LEN_WITHOUT_IS_EMPTY), LintId::of(len_zero::LEN_WITHOUT_IS_EMPTY),

View file

@ -1,16 +1,15 @@
use super::NEEDLESS_COLLECT; use super::NEEDLESS_COLLECT;
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; 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::sugg::Sugg;
use clippy_utils::ty::{is_type_diagnostic_item, match_type}; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_trait_method, path_to_local_id, paths}; use clippy_utils::{is_trait_method, path_to_local_id};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_block, walk_expr, NestedVisitorMap, Visitor}; 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_hir::{Block, Expr, ExprKind, GenericArg, GenericArgs, HirId, Local, Pat, PatKind, QPath, StmtKind, Ty};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::hir::map::Map; use rustc_middle::hir::map::Map;
use rustc_span::symbol::{sym, Ident}; use rustc_span::symbol::{sym, Ident};
use rustc_span::{MultiSpan, Span}; 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(generic_args) = chain_method.args;
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0); 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 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 { 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( span_lint_and_sugg(
cx, cx,
NEEDLESS_COLLECT, NEEDLESS_COLLECT,
@ -52,7 +65,7 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont
NEEDLESS_COLLECT_MSG, NEEDLESS_COLLECT_MSG,
"replace with", "replace with",
sugg, 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) || 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::vecdeque_type) ||
is_type_diagnostic_item(cx, ty, sym::BinaryHeap) || 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 Some(iter_calls) = detect_iter_and_into_iters(block, *ident);
if let [iter_call] = &*iter_calls; if let [iter_call] = &*iter_calls;
then { then {

View file

@ -1,170 +1,347 @@
use super::utils::{LoopNestVisitor, Nesting};
use super::WHILE_LET_ON_ITERATOR; use super::WHILE_LET_ON_ITERATOR;
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability; use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::implements_trait; use clippy_utils::{get_enclosing_loop, is_refutable, is_trait_method, match_def_path, paths, visitors::is_res_used};
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 if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_block, walk_expr, NestedVisitorMap, Visitor}; use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor};
use rustc_hir::{Expr, ExprKind, HirId, MatchSource, Node, PatKind}; use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, MatchSource, Node, PatKind, QPath, UnOp};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::hir::map::Map; use rustc_span::{symbol::sym, Span, Symbol};
use rustc_span::symbol::sym;
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Match(match_expr, arms, MatchSource::WhileLetDesugar) = expr.kind { let (scrutinee_expr, iter_expr, some_pat, loop_expr) = if_chain! {
let pat = &arms[0].pat.kind; if let ExprKind::Match(scrutinee_expr, [arm, _], MatchSource::WhileLetDesugar) = expr.kind;
if let (&PatKind::TupleStruct(ref qpath, pat_args, _), &ExprKind::MethodCall(method_path, _, method_args, _)) = // check for `Some(..)` pattern
(pat, &match_expr.kind) if let PatKind::TupleStruct(QPath::Resolved(None, pat_path), some_pat, _) = arm.pat.kind;
{ if let Res::Def(_, pat_did) = pat_path.res;
let iter_expr = &method_args[0]; if match_def_path(cx, pat_did, &paths::OPTION_SOME);
// check for call to `Iterator::next`
// Don't lint when the iterator is recreated on every iteration if let ExprKind::MethodCall(method_name, _, [iter_expr], _) = scrutinee_expr.kind;
if_chain! { if method_name.ident.name == sym::next;
if let ExprKind::MethodCall(..) | ExprKind::Call(..) = iter_expr.kind; if is_trait_method(cx, scrutinee_expr, sym::Iterator);
if let Some(iter_def_id) = cx.tcx.get_diagnostic_item(sym::Iterator); if let Some(iter_expr) = try_parse_iter_expr(cx, iter_expr);
if implements_trait(cx, cx.typeck_results().expr_ty(iter_expr), iter_def_id, &[]); // get the loop containing the match expression
then { if let Some((_, Node::Expr(loop_expr))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1);
return; if !uses_iter(cx, &iter_expr, arm.body);
}
}
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,
);
}
}
}
}
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 let Some(used_mutably) = mutated_variables(container, cx) {
if used_mutably.contains(&def_id) {
return true;
}
}
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);
}
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 { then {
return is_loop_nested(cx, loop_expr, iter_expr) (scrutinee_expr, iter_expr, some_pat, loop_expr)
} else {
return;
} }
}
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;
}; };
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()
};
// 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 {
""
};
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 { loop {
let parent = cx.tcx.hir().get_parent_node(id); match e.kind {
if parent == id { ExprKind::Path(ref path) => {
return false; 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,
} }
match cx.tcx.hir().find(parent) { }
Some(Node::Expr(expr)) => { }
if let ExprKind::Loop(..) = expr.kind {
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,
}
}
}
/// 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; 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;
} }
}, // Check if the expression is a parent field
Some(Node::Stmt(_)) => (), let mut fields_iter = tail_fields.iter();
_ => { while let Some(field) = fields_iter.next() {
return false; if *field == name.name && is_expr_same_field(cx, base, fields_iter.as_slice(), path_res) {
}, return true;
} }
id = parent; }
}
}
struct VarUsedAfterLoopVisitor {
def_id: HirId,
iter_expr_id: HirId,
past_while_let: bool,
var_used_after_while_let: bool,
}
impl<'tcx> Visitor<'tcx> for VarUsedAfterLoopVisitor {
type Map = Map<'tcx>;
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;
} }
} else if self.iter_expr_id == expr.hir_id {
self.past_while_let = true; // Check if the expression is a child field.
} let mut e = base;
walk_expr(self, expr); loop {
} match e.kind {
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { ExprKind::Field(..) if is_expr_same_field(cx, e, fields, path_res) => break true,
NestedVisitorMap::None 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,
}
}
/// 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,
}
};
(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
}
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);
}
} 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);
}
}
}
let mut v = V {
cx,
iter_expr,
uses_iter: false,
};
v.visit_expr(container);
v.uses_iter
}
#[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, 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);
}
}
}
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
} }
} }

View file

@ -47,7 +47,12 @@ pub struct MacroRefData {
impl MacroRefData { impl MacroRefData {
pub fn new(name: String, callee: Span, cx: &LateContext<'_>) -> Self { 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> // std lib paths are <::std::module::file type>
// so remove brackets, space and 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, '!'), "_"); let name = snippet(cx, cx.sess().source_map().span_until_char(call_site, '!'), "_");
if let Some(callee) = span.source_callee() { if let Some(callee) = span.source_callee() {
if !self.collected.contains(&call_site) { 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); self.collected.insert(call_site);
} }
} }
@ -174,7 +180,7 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
.push((*item).to_string()); .push((*item).to_string());
check_dup.push((*item).to_string()); check_dup.push((*item).to_string());
} }
} },
[root, rest @ ..] => { [root, rest @ ..] => {
if rest.iter().all(|item| !check_dup.contains(&(*item).to_string())) { if rest.iter().all(|item| !check_dup.contains(&(*item).to_string())) {
let filtered = rest let filtered = rest
@ -198,7 +204,7 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
.push(rest.join("::")); .push(rest.join("::"));
check_dup.extend(rest.iter().map(ToString::to_string)); check_dup.extend(rest.iter().map(ToString::to_string));
} }
} },
} }
} }
} }

View file

@ -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 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>> { fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> {
if_chain! { 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 is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk);
if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind; if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind;
if path_to_local_id(unwrap_arm.body, binding_hir_id); 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); if !contains_return_break_continue_macro(or_arm.body);
then { then {
Some(or_arm) Some(or_arm)
@ -98,10 +84,10 @@ fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if_chain! { if_chain! {
if let ExprKind::Match(scrutinee, match_arms, _) = expr.kind; if let ExprKind::Match(scrutinee, match_arms, _) = expr.kind;
let ty = cx.typeck_results().expr_ty(scrutinee); let ty = cx.typeck_results().expr_ty(scrutinee);
if let Some(case) = if is_type_diagnostic_item(cx, ty, sym::option_type) { if let Some(ty_name) = if is_type_diagnostic_item(cx, ty, sym::option_type) {
Some(Case::Option) Some("Option")
} else if is_type_diagnostic_item(cx, ty, sym::result_type) { } else if is_type_diagnostic_item(cx, ty, sym::result_type) {
Some(Case::Result) Some("Result")
} else { } else {
None None
}; };
@ -124,7 +110,7 @@ fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
MANUAL_UNWRAP_OR, expr.span, MANUAL_UNWRAP_OR, expr.span,
&format!("this pattern reimplements `{}`", case.unwrap_fn_path()), &format!("this pattern reimplements `{}::unwrap_or`", ty_name),
"replace with", "replace with",
format!( format!(
"{}.unwrap_or({})", "{}.unwrap_or({})",

View file

@ -1478,15 +1478,34 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A
); );
}, },
PatKind::Wild => { PatKind::Wild => {
span_lint_and_sugg( if ex.can_have_side_effects() {
cx, let indent = " ".repeat(indent_of(cx, expr.span).unwrap_or(0));
MATCH_SINGLE_BINDING, let sugg = format!(
expr.span, "{};\n{}{}",
"this match could be replaced by its body itself", snippet_with_applicability(cx, ex.span, "..", &mut applicability),
"consider using the match body instead", indent,
snippet_body, snippet_body
Applicability::MachineApplicable, );
); 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,
);
}
}, },
_ => (), _ => (),
} }

View file

@ -1838,16 +1838,18 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
} }
} }
wrong_self_convention::check( if sig.decl.implicit_self.has_implicit_self() {
cx, wrong_self_convention::check(
&name, cx,
item.vis.node.is_pub(), &name,
self_ty, item.vis.node.is_pub(),
first_arg_ty, self_ty,
first_arg.pat.span, first_arg_ty,
implements_trait, first_arg.pat.span,
false implements_trait,
); false
);
}
} }
} }
@ -1903,7 +1905,9 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
if_chain! { if_chain! {
if let TraitItemKind::Fn(ref sig, _) = item.kind; 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(); if let Some(first_arg_ty) = sig.decl.inputs.iter().next();
then { then {
let first_arg_span = first_arg_ty.span; let first_arg_span = first_arg_ty.span;
let first_arg_ty = hir_ty_to_ty(cx.tcx, first_arg_ty); let first_arg_ty = hir_ty_to_ty(cx.tcx, first_arg_ty);

View file

@ -22,7 +22,7 @@ const CONVENTIONS: [(&[Convention], &[SelfKind]); 9] = [
// Conversion using `to_` can use borrowed (non-Copy types) or owned (Copy types). // 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 // 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::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::StartsWith("to_"), Convention::NotEndsWith("_mut"), Convention::IsSelfTypeCopy(true),
Convention::IsTraitItem(false), Convention::ImplementsTrait(false)], &[SelfKind::Value]), Convention::IsTraitItem(false), Convention::ImplementsTrait(false)], &[SelfKind::Value]),
]; ];

View file

@ -660,7 +660,14 @@ fn in_attributes_expansion(expr: &Expr<'_>) -> bool {
use rustc_span::hygiene::MacroKind; use rustc_span::hygiene::MacroKind;
if expr.span.from_expansion() { if expr.span.from_expansion() {
let data = expr.span.ctxt().outer_expn_data(); 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 { } else {
false false
} }

View file

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

View file

@ -1,14 +1,13 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_lang_ctor;
use clippy_utils::source::snippet; 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 if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionSome, ResultOk}; use rustc_hir::LangItem::{OptionSome, ResultOk};
use rustc_hir::{Body, Expr, ExprKind, LangItem, MatchSource, QPath}; use rustc_hir::{Body, Expr, ExprKind, LangItem, MatchSource, QPath};
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::TyS;
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
declare_clippy_lint! { declare_clippy_lint! {
/// **What it does:** /// **What it does:**
@ -63,12 +62,6 @@ declare_clippy_lint! {
declare_lint_pass!(NeedlessQuestionMark => [NEEDLESS_QUESTION_MARK]); 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 { impl LateLintPass<'_> for NeedlessQuestionMark {
/* /*
* The question mark operator is compatible with both Result<T, E> and Option<T>, * 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<'_>) { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
let e = match &expr.kind { if let ExprKind::Ret(Some(e)) = expr.kind {
ExprKind::Ret(Some(e)) => e, check(cx, e);
_ => return,
};
if let Some(ok_some_call) = is_some_or_ok_call(cx, e) {
emit_lint(cx, &ok_some_call);
} }
} }
fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) { fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) {
// Function / Closure block check(cx, body.value.peel_blocks());
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);
}
};
} }
} }
fn emit_lint(cx: &LateContext<'_>, expr: &SomeOkCall<'_>) { fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
let (entire_expr, inner_expr) = match expr { let inner_expr = if_chain! {
SomeOkCall::OkCall(outer, inner) | SomeOkCall::SomeCall(outer, inner) => (outer, inner), 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( span_lint_and_sugg(
cx, cx,
NEEDLESS_QUESTION_MARK, NEEDLESS_QUESTION_MARK,
entire_expr.span, expr.span,
"question mark operator is useless here", "question mark operator is useless here",
"try", "try",
format!("{}", snippet(cx, inner_expr.span, r#""...""#)), format!("{}", snippet(cx, inner_expr.span, r#""...""#)),
Applicability::MachineApplicable, 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);
}

View file

@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::sugg::Sugg; use clippy_utils::sugg::Sugg;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::usage::contains_return_break_continue_macro; 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 if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::LangItem::OptionSome; use rustc_hir::LangItem::OptionSome;
@ -81,7 +81,6 @@ struct OptionIfLetElseOccurence {
method_sugg: String, method_sugg: String,
some_expr: String, some_expr: String,
none_expr: String, none_expr: String,
wrap_braces: bool,
} }
/// Extracts the body of a given arm. If the arm contains only an expression, /// 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 { fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: bool, as_mut: bool) -> String {
format!( format!(
"{}{}", "{}{}",
@ -161,6 +129,7 @@ fn detect_option_if_let_else<'tcx>(
if_chain! { if_chain! {
if !in_macro(expr.span); // Don't lint macros, because it behaves weirdly 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 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 arms.len() == 2;
if !is_result_ok(cx, cond_expr); // Don't lint on Result::ok because a different lint does it already 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; 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 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[0].body);
if !contains_return_break_continue_macro(arms[1].body); if !contains_return_break_continue_macro(arms[1].body);
then { then {
let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" }; let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" };
let some_body = extract_body_from_arm(&arms[0])?; let some_body = extract_body_from_arm(&arms[0])?;
let none_body = extract_body_from_arm(&arms[1])?; 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 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 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 { let (as_ref, as_mut) = match &cond_expr.kind {
ExprKind::AddrOf(_, Mutability::Not, _) => (true, false), ExprKind::AddrOf(_, Mutability::Not, _) => (true, false),
ExprKind::AddrOf(_, Mutability::Mut, _) => (false, true), ExprKind::AddrOf(_, Mutability::Mut, _) => (false, true),
@ -190,7 +159,6 @@ fn detect_option_if_let_else<'tcx>(
method_sugg: method_sugg.to_string(), method_sugg: method_sugg.to_string(),
some_expr: format!("|{}{}| {}", capture_mut, capture_name, Sugg::hir(cx, some_body, "..")), 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, "..")), none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, "..")),
wrap_braces,
}) })
} else { } else {
None 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(), format!("use Option::{} instead of an if let/else", detection.method_sugg).as_str(),
"try", "try",
format!( format!(
"{}{}.{}({}, {}){}", "{}.{}({}, {})",
if detection.wrap_braces { "{ " } else { "" }, detection.option, detection.method_sugg, detection.none_expr, detection.some_expr,
detection.option,
detection.method_sugg,
detection.none_expr,
detection.some_expr,
if detection.wrap_braces { " }" } else { "" },
), ),
Applicability::MaybeIncorrect, Applicability::MaybeIncorrect,
); );

View file

@ -8,7 +8,12 @@ use super::UNIT_CMP;
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
if expr.span.from_expansion() { if expr.span.from_expansion() {
if let Some(callee) = expr.span.source_callee() { 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 { if let ExprKind::Binary(ref cmp, left, _) = expr.kind {
let op = cmp.node; let op = cmp.node;
if op.is_comparison() && cx.typeck_results().expr_ty(left).is_unit() { if op.is_comparison() && cx.typeck_results().expr_ty(left).is_unit() {

View file

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

View file

@ -1,12 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_opt; use clippy_utils::source::snippet_opt;
use clippy_utils::ty::same_type_and_consts;
use clippy_utils::{in_macro, meets_msrv, msrvs}; use clippy_utils::{in_macro, meets_msrv, msrvs};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::{ use rustc_hir::{
def, self as hir,
def::{self, DefKind},
def_id::LocalDefId, def_id::LocalDefId,
intravisit::{walk_ty, NestedVisitorMap, Visitor}, intravisit::{walk_ty, NestedVisitorMap, Visitor},
Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Node, Path, PathSegment, 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_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::map::Map; 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_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{BytePos, Span}; 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 { fn should_lint_ty(hir_ty: &hir::Ty<'_>, ty: Ty<'_>, self_ty: Ty<'_>) -> bool {
if_chain! { 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; if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind;
then { then {
!matches!(path.res, def::Res::SelfTy(..)) !matches!(path.res, def::Res::SelfTy(..))

View file

@ -1,13 +1,13 @@
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
use clippy_utils::source::{snippet, snippet_with_macro_callsite}; use clippy_utils::source::{snippet, snippet_with_macro_callsite};
use clippy_utils::sugg::Sugg; 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 clippy_utils::{get_parent_expr, match_def_path, match_trait_method, paths};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, HirId, MatchSource}; use rustc_hir::{Expr, ExprKind, HirId, MatchSource};
use rustc_lint::{LateContext, LateLintPass}; 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_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::sym; 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" { if match_trait_method(cx, e, &paths::INTO) && &*name.ident.as_str() == "into" {
let a = cx.typeck_results().expr_ty(e); let a = cx.typeck_results().expr_ty(e);
let b = cx.typeck_results().expr_ty(&args[0]); 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(); let sugg = snippet_with_macro_callsite(cx, args[0].span, "<expr>").to_string();
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
@ -90,7 +90,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
} }
let a = cx.typeck_results().expr_ty(e); let a = cx.typeck_results().expr_ty(e);
let b = cx.typeck_results().expr_ty(&args[0]); 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(); let sugg = snippet(cx, args[0].span, "<expr>").into_owned();
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
@ -110,7 +110,8 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
if is_type_diagnostic_item(cx, a, sym::result_type); if is_type_diagnostic_item(cx, a, sym::result_type);
if let ty::Adt(_, substs) = a.kind(); if let ty::Adt(_, substs) = a.kind();
if let Some(a_type) = substs.types().next(); if let Some(a_type) = substs.types().next();
if TyS::same_type(a_type, b); if same_type_and_consts(a_type, b);
then { then {
span_lint_and_help( span_lint_and_help(
cx, cx,
@ -137,7 +138,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
if is_type_diagnostic_item(cx, a, sym::result_type); if is_type_diagnostic_item(cx, a, sym::result_type);
if let ty::Adt(_, substs) = a.kind(); if let ty::Adt(_, substs) = a.kind();
if let Some(a_type) = substs.types().next(); if let Some(a_type) = substs.types().next();
if TyS::same_type(a_type, b); if same_type_and_consts(a_type, b);
then { then {
let hint = format!("consider removing `{}()`", snippet(cx, path.span, "TryFrom::try_from")); 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_chain! {
if match_def_path(cx, def_id, &paths::FROM_FROM); if match_def_path(cx, def_id, &paths::FROM_FROM);
if TyS::same_type(a, b); if same_type_and_consts(a, b);
then { then {
let sugg = Sugg::hir_with_macro_callsite(cx, &args[0], "<expr>").maybe_par(); let sugg = Sugg::hir_with_macro_callsite(cx, &args[0], "<expr>").maybe_par();

View file

@ -26,13 +26,13 @@ impl TryConf {
macro_rules! define_Conf { macro_rules! define_Conf {
($( ($(
#[$doc:meta] #[doc = $doc:literal]
$(#[conf_deprecated($dep:literal)])? $(#[conf_deprecated($dep:literal)])?
($name:ident: $ty:ty = $default:expr), ($name:ident: $ty:ty = $default:expr),
)*) => { )*) => {
/// Clippy lint configuration /// Clippy lint configuration
pub struct Conf { pub struct Conf {
$(#[$doc] pub $name: $ty,)* $(#[doc = $doc] pub $name: $ty,)*
} }
mod defaults { mod defaults {
@ -89,6 +89,34 @@ macro_rules! define_Conf {
Ok(TryConf { conf, errors }) 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,
)
},
)+
]
}
}
}; };
} }

View file

@ -8,9 +8,6 @@
//! during any comparison or mapping. (Please take care of this, it's not fun to spend time on such //! during any comparison or mapping. (Please take care of this, it's not fun to spend time on such
//! a simple mistake) //! a simple mistake)
// # NITs
// - TODO xFrednet 2021-02-13: Collect depreciations and maybe renames
use if_chain::if_chain; use if_chain::if_chain;
use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::fx::FxHashMap;
use rustc_hir::{ use rustc_hir::{
@ -22,13 +19,14 @@ use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{sym, Loc, Span, Symbol}; use rustc_span::{sym, Loc, Span, Symbol};
use serde::{ser::SerializeStruct, Serialize, Serializer}; use serde::{ser::SerializeStruct, Serialize, Serializer};
use std::collections::BinaryHeap; use std::collections::BinaryHeap;
use std::fmt;
use std::fs::{self, OpenOptions}; use std::fs::{self, OpenOptions};
use std::io::prelude::*; use std::io::prelude::*;
use std::path::Path; use std::path::Path;
use crate::utils::internal_lints::is_lint_ref_type; use crate::utils::internal_lints::is_lint_ref_type;
use clippy_utils::{ 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, 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 /// These groups will be ignored by the lint group matcher. This is useful for collections like
/// `clippy::all` /// `clippy::all`
const IGNORED_LINT_GROUPS: [&str; 1] = ["clippy::all"]; const IGNORED_LINT_GROUPS: [&str; 1] = ["clippy::all"];
/// Lints within this group will be excluded from the collection /// Lints within this group will be excluded from the collection. These groups
const EXCLUDED_LINT_GROUPS: [&str; 1] = ["clippy::internal"]; /// 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] = [ const LINT_EMISSION_FUNCTIONS: [&[&str]; 7] = [
&["clippy_utils", "diagnostics", "span_lint"], &["clippy_utils", "diagnostics", "span_lint"],
@ -66,6 +108,7 @@ const SUGGESTION_FUNCTIONS: [&[&str]; 2] = [
&["clippy_utils", "diagnostics", "multispan_sugg"], &["clippy_utils", "diagnostics", "multispan_sugg"],
&["clippy_utils", "diagnostics", "multispan_sugg_with_applicability"], &["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` /// The index of the applicability name of `paths::APPLICABILITY_VALUES`
const APPLICABILITY_NAME_INDEX: usize = 2; const APPLICABILITY_NAME_INDEX: usize = 2;
@ -102,13 +145,33 @@ declare_clippy_lint! {
impl_lint_pass!(MetadataCollector => [INTERNAL_METADATA_COLLECTOR]); impl_lint_pass!(MetadataCollector => [INTERNAL_METADATA_COLLECTOR]);
#[allow(clippy::module_name_repetitions)] #[allow(clippy::module_name_repetitions)]
#[derive(Debug, Clone, Default)] #[derive(Debug, Clone)]
pub struct MetadataCollector { pub struct MetadataCollector {
/// All collected lints /// All collected lints
/// ///
/// We use a Heap here to have the lints added in alphabetic order in the export /// We use a Heap here to have the lints added in alphabetic order in the export
lints: BinaryHeap<LintMetadata>, lints: BinaryHeap<LintMetadata>,
applicability_info: FxHashMap<String, ApplicabilityInfo>, 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 { impl Drop for MetadataCollector {
@ -143,6 +206,7 @@ struct LintMetadata {
id: String, id: String,
id_span: SerializableSpan, id_span: SerializableSpan,
group: String, group: String,
level: &'static str,
docs: String, docs: String,
/// This field is only used in the output and will only be /// This field is only used in the output and will only be
/// mapped shortly before the actual output. /// mapped shortly before the actual output.
@ -150,11 +214,12 @@ struct LintMetadata {
} }
impl 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 { Self {
id, id,
id_span, id_span,
group, group,
level,
docs, docs,
applicability: None, applicability: None,
} }
@ -182,7 +247,7 @@ impl SerializableSpan {
let loc: Loc = cx.sess().source_map().lookup_char_pos(span.lo()); let loc: Loc = cx.sess().source_map().lookup_char_pos(span.lo());
Self { Self {
path: format!("{}", loc.file.name), path: format!("{}", loc.file.name.prefer_remapped()),
line: loc.line, 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 { impl<'hir> LateLintPass<'hir> for MetadataCollector {
/// Collecting lint declarations like: /// Collecting lint declarations like:
/// ```rust, ignore /// ```rust, ignore
@ -225,23 +379,48 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector {
/// } /// }
/// ``` /// ```
fn check_item(&mut self, cx: &LateContext<'hir>, item: &'hir Item<'_>) { fn check_item(&mut self, cx: &LateContext<'hir>, item: &'hir Item<'_>) {
if_chain! { if let ItemKind::Static(ref ty, Mutability::Not, _) = item.kind {
// item validation // Normal lint
if let ItemKind::Static(ref ty, Mutability::Not, _) = item.kind; if_chain! {
if is_lint_ref_type(cx, ty); // item validation
// blacklist check if is_lint_ref_type(cx, ty);
let lint_name = sym_to_string(item.ident.name).to_ascii_lowercase(); // blacklist check
if !BLACK_LISTED_LINTS.contains(&lint_name.as_str()); let lint_name = sym_to_string(item.ident.name).to_ascii_lowercase();
// metadata extraction if !BLACK_LISTED_LINTS.contains(&lint_name.as_str());
if let Some(group) = get_lint_group_or_lint(cx, &lint_name, item); // metadata extraction
if let Some(docs) = extract_attr_docs_or_lint(cx, item); if let Some((group, level)) = get_lint_group_and_level_or_lint(cx, &lint_name, item);
then { if let Some(mut docs) = extract_attr_docs_or_lint(cx, item);
self.lints.push(LintMetadata::new( then {
lint_name, if let Some(configuration_section) = self.get_lint_configs(&lint_name) {
SerializableSpan::from_item(cx, item), docs.push_str(&configuration_section);
group, }
docs,
)); 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/misc.rs:734:9
// - src/methods/mod.rs:3545:13 // - src/methods/mod.rs:3545:13
// - src/methods/mod.rs:3496: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; 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)); let result = cx.lint_store.check_lint_name(lint_name, Some(sym::clippy));
if let CheckLintNameResult::Tool(Ok(lint_lst)) = result { if let CheckLintNameResult::Tool(Ok(lint_lst)) = result {
get_lint_group(cx, lint_lst[0]) if let Some(group) = get_lint_group(cx, lint_lst[0]) {
.or_else(|| { if EXCLUDED_LINT_GROUPS.contains(&group.as_str()) {
lint_collection_error_item(cx, item, "Unable to determine lint group"); 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 None
}) }
.filter(|group| !EXCLUDED_LINT_GROUPS.contains(&group.as_str())) } else {
lint_collection_error_item(cx, item, "Unable to determine lint group");
None
}
} else { } else {
lint_collection_error_item(cx, item, "Unable to find lint in lint_store"); lint_collection_error_item(cx, item, "Unable to find lint in lint_store");
None None
@ -339,14 +535,31 @@ fn get_lint_group(cx: &LateContext<'_>, lint_id: LintId) -> Option<String> {
continue; continue;
} }
if lints.iter().any(|x| *x == lint_id) { if lints.iter().any(|group_lint| *group_lint == lint_id) {
return Some((*group_name).to_string()); let group = group_name.strip_prefix(CLIPPY_LINT_GROUP_PREFIX).unwrap_or(group_name);
return Some((*group).to_string());
} }
} }
None 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 // Lint emission
// ================================================================== // ==================================================================

View file

@ -279,8 +279,15 @@ impl EarlyLintPass for Write {
span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprintln!`"); span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprintln!`");
self.lint_println_empty_string(cx, mac); self.lint_println_empty_string(cx, mac);
} else if mac.path == sym!(write) { } 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) { 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( span_lint_and_then(
cx, cx,
WRITE_WITH_NEWLINE, WRITE_WITH_NEWLINE,
@ -289,10 +296,7 @@ impl EarlyLintPass for Write {
|err| { |err| {
err.multipart_suggestion( err.multipart_suggestion(
"use `writeln!()` instead", "use `writeln!()` instead",
vec![ vec![(mac.path.span, String::from("writeln")), (nl_span, String::new())],
(mac.path.span, String::from("writeln")),
(newline_span(&fmt_str), String::new()),
],
Applicability::MachineApplicable, 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 /// 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. /// 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 sp = fmtstr.span;
let contents = &fmtstr.symbol.as_str(); let contents = &fmtstr.symbol.as_str();
if *contents == r"\n" { if *contents == r"\n" {
return sp; return (sp, true);
} }
let newline_sp_hi = sp.hi() let newline_sp_hi = sp.hi()
@ -351,7 +356,7 @@ fn newline_span(fmtstr: &StrLit) -> Span {
panic!("expected format string to contain a newline"); 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 /// 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| {
err.multipart_suggestion( err.multipart_suggestion(
&format!("use `{}!` instead", suggested), &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, Applicability::MachineApplicable,
); );
}, },

View file

@ -14,6 +14,7 @@ unicode-normalization = "0.1"
rustc-semver="1.1.0" rustc-semver="1.1.0"
[features] [features]
deny-warnings = []
internal-lints = [] internal-lints = []
metadata-collector-lint = [] metadata-collector-lint = []

View file

@ -4,7 +4,14 @@
#![cfg_attr(bootstrap, feature(or_patterns))] #![cfg_attr(bootstrap, feature(or_patterns))]
#![feature(rustc_private)] #![feature(rustc_private)]
#![recursion_limit = "512"] #![recursion_limit = "512"]
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
#![allow(clippy::missing_errors_doc, clippy::missing_panics_doc, clippy::must_use_candidate)] #![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. // FIXME: switch to something more ergonomic here, once available.
// (Currently there is no way to opt into sysroot crates without `extern crate`.) // (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. /// Gets the parent node if it's an impl block.
pub fn get_parent_as_impl(tcx: TyCtxt<'_>, id: HirId) -> Option<&Impl<'_>> { pub fn get_parent_as_impl(tcx: TyCtxt<'_>, id: HirId) -> Option<&Impl<'_>> {
let map = tcx.hir(); 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 data = span.ctxt().outer_expn_data();
let new_span = data.call_site; 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 { if mac_name.as_str() == name {
return Some(new_span); 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 data = span.ctxt().outer_expn_data();
let new_span = data.call_site; 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 { if mac_name.as_str() == name {
return Some(new_span); return Some(new_span);
} }

View file

@ -289,7 +289,7 @@ fn has_enclosing_paren(sugg: impl AsRef<str>) -> bool {
let mut chars = sugg.as_ref().chars(); let mut chars = sugg.as_ref().chars();
if let Some('(') = chars.next() { if let Some('(') = chars.next() {
let mut depth = 1; let mut depth = 1;
while let Some(c) = chars.next() { for c in &mut chars {
if c == '(' { if c == '(' {
depth += 1; depth += 1;
} else if c == ')' { } else if c == ')' {

View file

@ -2,9 +2,8 @@
#![allow(clippy::module_name_repetitions)] #![allow(clippy::module_name_repetitions)]
use std::collections::HashMap;
use rustc_ast::ast::Mutability; use rustc_ast::ast::Mutability;
use rustc_data_structures::fx::FxHashMap;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::def_id::DefId; use rustc_hir::def_id::DefId;
use rustc_hir::{TyKind, Unsafety}; 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 /// Checks if `Ty` is normalizable. This function is useful
/// to avoid crashes on `layout_of`. /// to avoid crashes on `layout_of`.
pub fn is_normalizable<'tcx>(cx: &LateContext<'tcx>, param_env: ty::ParamEnv<'tcx>, ty: Ty<'tcx>) -> bool { 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>( fn is_normalizable_helper<'tcx>(
cx: &LateContext<'tcx>, cx: &LateContext<'tcx>,
param_env: ty::ParamEnv<'tcx>, param_env: ty::ParamEnv<'tcx>,
ty: Ty<'tcx>, ty: Ty<'tcx>,
cache: &mut HashMap<Ty<'tcx>, bool>, cache: &mut FxHashMap<Ty<'tcx>, bool>,
) -> bool { ) -> bool {
if let Some(&cached_result) = cache.get(ty) { if let Some(&cached_result) = cache.get(ty) {
return cached_result; return cached_result;
@ -322,3 +321,27 @@ pub fn walk_ptrs_ty_depth(ty: Ty<'_>) -> (Ty<'_>, usize) {
} }
inner(ty, 0) 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,
}
}

View file

@ -1,7 +1,7 @@
use crate::path_to_local_id; use crate::path_to_local_id;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::intravisit::{self, walk_expr, ErasedMap, NestedVisitorMap, Visitor}; 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_lint::LateContext;
use rustc_middle::hir::map::Map; 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>( pub fn visit_break_exprs<'tcx>(
node: impl Visitable<'tcx>, node: impl Visitable<'tcx>,
f: impl FnMut(&'tcx Expr<'tcx>, Destination, Option<&'tcx Expr<'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)); 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
}

View file

@ -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: If you've already cloned Clippy in the past, update it to the latest version:
```bash ```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 # upstream has to be the remote of the rust-lang/rust-clippy repo
git fetch upstream git fetch upstream
# make sure that you are on the master branch # make sure that you are on the master branch

View file

@ -1,3 +1,3 @@
[toolchain] [toolchain]
channel = "nightly-2021-05-06" channel = "nightly-2021-05-20"
components = ["llvm-tools-preview", "rustc-dev", "rust-src"] components = ["llvm-tools-preview", "rustc-dev", "rust-src"]

View file

@ -22,14 +22,12 @@ fn dogfood_clippy() {
return; return;
} }
let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); 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); let mut command = Command::new(&*CLIPPY_PATH);
command command
.current_dir(root_dir) .current_dir(root_dir)
.env("CLIPPY_DOGFOOD", "1") .env("CLIPPY_DOGFOOD", "1")
.env("CARGO_INCREMENTAL", "0") .env("CARGO_INCREMENTAL", "0")
.env("ENABLE_METADATA_COLLECTION", &enable_metadata_collection)
.arg("clippy") .arg("clippy")
.arg("--all-targets") .arg("--all-targets")
.arg("--all-features") .arg("--all-features")
@ -157,10 +155,9 @@ fn dogfood_subprojects() {
if cargo::is_rustc_test_suite() { if cargo::is_rustc_test_suite() {
return; return;
} }
let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
// NOTE: `path_dep` crate is omitted on purpose here // NOTE: `path_dep` crate is omitted on purpose here
for d in &[ for project in &[
"clippy_workspace_tests", "clippy_workspace_tests",
"clippy_workspace_tests/src", "clippy_workspace_tests/src",
"clippy_workspace_tests/subcrate", "clippy_workspace_tests/subcrate",
@ -170,34 +167,49 @@ fn dogfood_subprojects() {
"clippy_utils", "clippy_utils",
"rustc_tools_util", "rustc_tools_util",
] { ] {
let mut command = Command::new(&*CLIPPY_PATH); run_clippy_for_project(project);
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
// internal lints only exist if we build with the internal-lints feature
if cfg!(feature = "internal-lints") {
command.args(&["-D", "clippy::internal"]);
}
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());
} }
// NOTE: Since tests run in parallel we can't run cargo commands on the same workspace at the // 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. // same time, so we test this immediately after the dogfood for workspaces.
test_no_deps_ignores_path_deps_in_workspaces(); test_no_deps_ignores_path_deps_in_workspaces();
} }
#[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");
}
fn run_clippy_for_project(project: &str) {
let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
let mut command = Command::new(&*CLIPPY_PATH);
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"]);
}
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());
}

View file

@ -0,0 +1,10 @@
// edition:2018
#![allow(clippy::never_loop)]
async fn f() {
loop {
break;
}
}
fn main() {}

View file

@ -4,8 +4,6 @@
fn main() { fn main() {
let one = 1; let one = 1;
let x = 3f32; let x = 3f32;
let _ = x * x;
let _ = x * x;
let y = 4f32; let y = 4f32;
let _ = x.mul_add(x, y); let _ = x.mul_add(x, y);
@ -13,7 +11,10 @@ fn main() {
let _ = x.mul_add(x, y).sqrt(); let _ = x.mul_add(x, y).sqrt();
let _ = y.mul_add(y, x).sqrt(); let _ = y.mul_add(y, x).sqrt();
// Cases where the lint shouldn't be applied // Cases where the lint shouldn't be applied
let _ = x.powi(2);
let _ = x.powi(1 + 1);
let _ = x.powi(3); let _ = x.powi(3);
let _ = x.powi(4) + y;
let _ = x.powi(one + 1); let _ = x.powi(one + 1);
let _ = (x.powi(2) + y.powi(2)).sqrt(); let _ = (x.powi(2) + y.powi(2)).sqrt();
} }

View file

@ -4,8 +4,6 @@
fn main() { fn main() {
let one = 1; let one = 1;
let x = 3f32; let x = 3f32;
let _ = x.powi(2);
let _ = x.powi(1 + 1);
let y = 4f32; let y = 4f32;
let _ = x.powi(2) + y; let _ = x.powi(2) + y;
@ -13,7 +11,10 @@ fn main() {
let _ = (x.powi(2) + y).sqrt(); let _ = (x.powi(2) + y).sqrt();
let _ = (x + y.powi(2)).sqrt(); let _ = (x + y.powi(2)).sqrt();
// Cases where the lint shouldn't be applied // Cases where the lint shouldn't be applied
let _ = x.powi(2);
let _ = x.powi(1 + 1);
let _ = x.powi(3); let _ = x.powi(3);
let _ = x.powi(4) + y;
let _ = x.powi(one + 1); let _ = x.powi(one + 1);
let _ = (x.powi(2) + y.powi(2)).sqrt(); let _ = (x.powi(2) + y.powi(2)).sqrt();
} }

View file

@ -1,40 +1,28 @@
error: square can be computed more efficiently error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_powi.rs:7:13 --> $DIR/floating_point_powi.rs:9: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
| |
LL | let _ = x.powi(2) + y; LL | let _ = x.powi(2) + y;
| ^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, 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 error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_powi.rs:12:13 --> $DIR/floating_point_powi.rs:10:13
| |
LL | let _ = x + y.powi(2); LL | let _ = x + y.powi(2);
| ^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)` | ^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)`
error: square can be computed more efficiently error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_powi.rs:13:13 --> $DIR/floating_point_powi.rs:11:13
| |
LL | let _ = (x.powi(2) + y).sqrt(); LL | let _ = (x.powi(2) + y).sqrt();
| ^^^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, y)` | ^^^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, y)`
error: square can be computed more efficiently error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_powi.rs:14:13 --> $DIR/floating_point_powi.rs:12:13
| |
LL | let _ = (x + y.powi(2)).sqrt(); LL | let _ = (x + y.powi(2)).sqrt();
| ^^^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)` | ^^^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)`
error: aborting due to 6 previous errors error: aborting due to 4 previous errors

View file

@ -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() {} fn main() {}

View file

@ -31,5 +31,33 @@ LL | | fn first() {}
LL | | } 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

View file

@ -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() {} fn main() {}

View file

@ -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() {} fn main() {}

View file

@ -94,10 +94,7 @@ fn main() {
0 => println!("Disabled branch"), 0 => println!("Disabled branch"),
_ => println!("Enabled branch"), _ => println!("Enabled branch"),
} }
// Lint
let x = 1;
let y = 1;
println!("Single branch");
// Ok // Ok
let x = 1; let x = 1;
let y = 1; let y = 1;

View file

@ -106,15 +106,7 @@ fn main() {
0 => println!("Disabled branch"), 0 => println!("Disabled branch"),
_ => println!("Enabled branch"), _ => println!("Enabled branch"),
} }
// Lint
let x = 1;
let y = 1;
match match y {
0 => 1,
_ => 2,
} {
_ => println!("Single branch"),
}
// Ok // Ok
let x = 1; let x = 1;
let y = 1; let y = 1;

View file

@ -167,16 +167,5 @@ LL | unwrapped
LL | }) LL | })
| |
error: this match could be replaced by its body itself error: aborting due to 11 previous errors
--> $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

View file

@ -34,4 +34,20 @@ fn main() {
}, },
None => println!("nothing"), 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");
} }

View file

@ -34,4 +34,22 @@ fn main() {
}, },
None => println!("nothing"), 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"),
}
} }

View file

@ -30,5 +30,39 @@ LL | let (a, b) = get_tup();
LL | println!("a {:?} and b {:?}", a, b); 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

View file

@ -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
}
}

View file

@ -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
}
}

View file

@ -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

View file

@ -2,7 +2,7 @@
#![allow(unused, clippy::suspicious_map, clippy::iter_count)] #![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)] #[warn(clippy::needless_collect)]
#[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)] #[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)]
@ -13,9 +13,24 @@ fn main() {
// Empty // Empty
} }
sample.iter().cloned().any(|x| x == 1); 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 // Notice the `HashSet`--this should not be linted
sample.iter().collect::<HashSet<_>>().len(); sample.iter().collect::<HashSet<_>>().len();
// Neither should this // Neither should this
sample.iter().collect::<BTreeSet<_>>().len(); 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();
} }

View file

@ -2,7 +2,7 @@
#![allow(unused, clippy::suspicious_map, clippy::iter_count)] #![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)] #[warn(clippy::needless_collect)]
#[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)] #[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)]
@ -13,9 +13,24 @@ fn main() {
// Empty // Empty
} }
sample.iter().cloned().collect::<Vec<_>>().contains(&1); 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::<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 // Notice the `HashSet`--this should not be linted
sample.iter().collect::<HashSet<_>>().len(); sample.iter().collect::<HashSet<_>>().len();
// Neither should this // Neither should this
sample.iter().collect::<BTreeSet<_>>().len(); 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();
} }

View file

@ -19,10 +19,52 @@ LL | sample.iter().cloned().collect::<Vec<_>>().contains(&1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)`
error: avoid using `collect()` when not needed 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(); LL | sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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

View file

@ -94,6 +94,11 @@ where
Ok(x?) Ok(x?)
} }
// not quite needless
fn deref_ref(s: Option<&String>) -> Option<&str> {
Some(s?)
}
fn main() {} fn main() {}
// #6921 if a macro wraps an expr in Some( ) and the ? is in the macro use, // #6921 if a macro wraps an expr in Some( ) and the ? is in the macro use,

View file

@ -94,6 +94,11 @@ where
Ok(x?) Ok(x?)
} }
// not quite needless
fn deref_ref(s: Option<&String>) -> Option<&str> {
Some(s?)
}
fn main() {} fn main() {}
// #6921 if a macro wraps an expr in Some( ) and the ? is in the macro use, // #6921 if a macro wraps an expr in Some( ) and the ? is in the macro use,

View file

@ -67,7 +67,7 @@ LL | return Ok(t.magic?);
| ^^^^^^^^^^^^ help: try: `t.magic` | ^^^^^^^^^^^^ help: try: `t.magic`
error: question mark operator is useless here 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)?) }() LL | || -> Option<_> { Some(Some($expr)?) }()
| ^^^^^^^^^^^^^^^^^^ help: try: `Some($expr)` | ^^^^^^^^^^^^^^^^^^ help: try: `Some($expr)`

View file

@ -10,7 +10,11 @@ fn bad1(string: Option<&str>) -> (bool, &str) {
fn else_if_option(string: Option<&str>) -> Option<(bool, &str)> { fn else_if_option(string: Option<&str>) -> Option<(bool, &str)> {
if string.is_none() { if string.is_none() {
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>) { fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {

View file

@ -10,17 +10,6 @@ LL | | }
| |
= note: `-D clippy::option-if-let-else` implied by `-D warnings` = 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 error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:25:13 --> $DIR/option_if_let_else.rs:25:13
| |
@ -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 }; LL | let _ = if let Some(x) = optional { x + 2 } else { 5 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
error: aborting due to 12 previous errors error: aborting due to 11 previous errors

15
tests/ui/unused_async.rs Normal file
View file

@ -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();
}

View file

@ -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

View file

@ -462,3 +462,33 @@ mod issue6818 {
a: i32, 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()
}
}
}

View file

@ -462,3 +462,33 @@ mod issue6818 {
a: i32, 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()
}
}
}

View file

@ -162,5 +162,11 @@ error: unnecessary structure name repetition
LL | A::new::<submod::B>(submod::B {}) LL | A::new::<submod::B>(submod::B {})
| ^ help: use the applicable keyword: `Self` | ^ 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

View file

@ -70,4 +70,23 @@ fn main() {
let a: i32 = 1; let a: i32 = 1;
let b: i32 = 1; let b: i32 = 1;
let _ = (a + b) * 3; 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
}
} }

View file

@ -70,4 +70,23 @@ fn main() {
let a: i32 = 1; let a: i32 = 1;
let b: i32 = 1; let b: i32 = 1;
let _ = i32::from(a + b) * 3; 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
}
} }

View file

@ -70,5 +70,23 @@ error: useless conversion to the same type: `i32`
LL | let _ = i32::from(a + b) * 3; LL | let _ = i32::from(a + b) * 3;
| ^^^^^^^^^^^^^^^^ help: consider removing `i32::from()`: `(a + b)` | ^^^^^^^^^^^^^^^^ 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

View file

@ -1,7 +1,7 @@
// run-rustfix // run-rustfix
#![warn(clippy::while_let_on_iterator)] #![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() { fn base() {
let mut iter = 1..20; let mut iter = 1..20;
@ -38,13 +38,6 @@ fn base() {
println!("next: {:?}", iter.next()); println!("next: {:?}", iter.next());
} }
// or this
let mut iter = 1u32..20;
while let Some(_) = iter.next() {
break;
}
println!("Remaining iter {:?}", iter);
// or this // or this
let mut iter = 1u32..20; let mut iter = 1u32..20;
while let Some(_) = iter.next() { while let Some(_) = iter.next() {
@ -135,18 +128,6 @@ fn refutable2() {
fn nested_loops() { fn nested_loops() {
let a = [42, 1337]; 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 { loop {
let mut y = a.iter(); let mut y = a.iter();
@ -167,10 +148,8 @@ fn issue1121() {
} }
fn issue2965() { fn issue2965() {
// This should not cause an ICE and suggest: // This should not cause an ICE
//
// for _ in values.iter() {}
//
use std::collections::HashSet; use std::collections::HashSet;
let mut values = HashSet::new(); let mut values = HashSet::new();
values.insert(1); values.insert(1);
@ -205,13 +184,145 @@ fn issue1654() {
} }
} }
fn main() { fn issue6491() {
base(); // Used in outer loop, needs &mut
refutable(); let mut it = 1..40;
refutable2(); while let Some(n) = it.next() {
nested_loops(); for m in &mut it {
issue1121(); if m % 10 == 0 {
issue2965(); break;
issue3670(); }
issue1654(); 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() {
let mut it = 0..20;
for _ in it {
println!("test");
}
} }

View file

@ -1,7 +1,7 @@
// run-rustfix // run-rustfix
#![warn(clippy::while_let_on_iterator)] #![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() { fn base() {
let mut iter = 1..20; let mut iter = 1..20;
@ -38,13 +38,6 @@ fn base() {
println!("next: {:?}", iter.next()); println!("next: {:?}", iter.next());
} }
// or this
let mut iter = 1u32..20;
while let Some(_) = iter.next() {
break;
}
println!("Remaining iter {:?}", iter);
// or this // or this
let mut iter = 1u32..20; let mut iter = 1u32..20;
while let Some(_) = iter.next() { while let Some(_) = iter.next() {
@ -135,18 +128,6 @@ fn refutable2() {
fn nested_loops() { fn nested_loops() {
let a = [42, 1337]; 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 { loop {
let mut y = a.iter(); let mut y = a.iter();
@ -167,10 +148,8 @@ fn issue1121() {
} }
fn issue2965() { fn issue2965() {
// This should not cause an ICE and suggest: // This should not cause an ICE
//
// for _ in values.iter() {}
//
use std::collections::HashSet; use std::collections::HashSet;
let mut values = HashSet::new(); let mut values = HashSet::new();
values.insert(1); values.insert(1);
@ -205,13 +184,145 @@ fn issue1654() {
} }
} }
fn main() { fn issue6491() {
base(); // Used in outer loop, needs &mut
refutable(); let mut it = 1..40;
refutable2(); while let Some(n) = it.next() {
nested_loops(); while let Some(m) = it.next() {
issue1121(); if m % 10 == 0 {
issue2965(); break;
issue3670(); }
issue1654(); 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() {
let mut it = 0..20;
while let Some(..) = it.next() {
println!("test");
}
} }

View file

@ -19,28 +19,96 @@ LL | while let Some(_) = iter.next() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in iter` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in iter`
error: this loop could be written as a `for` loop 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() {} LL | while let Some([..]) = it.next() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for [..] in it` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for [..] in it`
error: this loop could be written as a `for` loop 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() {} LL | while let Some([_x]) = it.next() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for [_x] in it` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for [_x] in it`
error: this loop could be written as a `for` loop 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() { LL | while let Some(x @ [_]) = it.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x @ [_] in it` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x @ [_] in it`
error: this loop could be written as a `for` loop 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() { LL | while let Some(_) = y.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in y` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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

View file

@ -51,8 +51,8 @@ LL | write!(&mut v, "/n");
| |
help: use `writeln!()` instead 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 error: using `write!()` with a format string that ends in a single newline
--> $DIR/write_with_newline.rs:36:5 --> $DIR/write_with_newline.rs:36:5

View file

@ -23,7 +23,7 @@ mod issue6983 {
} }
struct FooNoCopy; struct FooNoCopy;
// trigger lint // don't trigger
impl ToU64 for FooNoCopy { impl ToU64 for FooNoCopy {
fn to_u64(self) -> u64 { fn to_u64(self) -> u64 {
2 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];
}
}

View file

@ -1,11 +1,19 @@
error: methods with the following characteristics: (`to_*` and `self` type is not `Copy`) usually take `self` by reference error: methods called `from_*` usually take no `self`
--> $DIR/wrong_self_convention2.rs:28:19 --> $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` = note: `-D clippy::wrong-self-convention` implied by `-D warnings`
= help: consider choosing a less ambiguous name = 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