From 56f25e3e62dbdd8f84a9152bbfb73a35055363dd Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 10 Jun 2020 19:29:11 -0700 Subject: [PATCH 01/15] Downgrade unnested_or_patterns to pedantic --- clippy_lints/src/lib.rs | 3 +-- clippy_lints/src/unnested_or_patterns.rs | 2 +- src/lintlist/mod.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 021fbe932d895..19e1b00050a67 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1187,6 +1187,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::OPTION_OPTION), LintId::of(&unicode::NON_ASCII_LITERAL), LintId::of(&unicode::UNICODE_NOT_NFC), + LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS), LintId::of(&unused_self::UNUSED_SELF), LintId::of(&wildcard_imports::ENUM_GLOB_USE), LintId::of(&wildcard_imports::WILDCARD_IMPORTS), @@ -1440,7 +1441,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS), LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS), LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), - LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS), LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), LintId::of(&unwrap::PANICKING_UNWRAP), @@ -1624,7 +1624,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), - LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS), LintId::of(&unwrap::UNNECESSARY_UNWRAP), LintId::of(&useless_conversion::USELESS_CONVERSION), LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO), diff --git a/clippy_lints/src/unnested_or_patterns.rs b/clippy_lints/src/unnested_or_patterns.rs index 8c281126c32bf..4d3682263f14f 100644 --- a/clippy_lints/src/unnested_or_patterns.rs +++ b/clippy_lints/src/unnested_or_patterns.rs @@ -45,7 +45,7 @@ declare_clippy_lint! { /// } /// ``` pub UNNESTED_OR_PATTERNS, - complexity, + pedantic, "unnested or-patterns, e.g., `Foo(Bar) | Foo(Baz) instead of `Foo(Bar | Baz)`" } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index cac3cc6bdb316..edceb75518008 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -2329,7 +2329,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "unnested_or_patterns", - group: "complexity", + group: "pedantic", desc: "unnested or-patterns, e.g., `Foo(Bar) | Foo(Baz) instead of `Foo(Bar | Baz)`", deprecation: None, module: "unnested_or_patterns", From 840786a93976d5885bfe6c7878cecc99e4a56432 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Mon, 1 Jun 2020 00:22:29 +0200 Subject: [PATCH 02/15] clippy-driver: pass all args after "--rustc" to rustc. --- src/driver.rs | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/driver.rs b/src/driver.rs index 4453ae5ce4414..1956effa827b3 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -297,12 +297,6 @@ pub fn main() { exit(rustc_driver::catch_with_exit_code(move || { let mut orig_args: Vec = env::args().collect(); - if orig_args.iter().any(|a| a == "--version" || a == "-V") { - let version_info = rustc_tools_util::get_version_info!(); - println!("{}", version_info); - exit(0); - } - // Get the sysroot, looking from most specific to this invocation to the least: // - command line // - runtime environment @@ -348,6 +342,29 @@ pub fn main() { .map(|pb| pb.to_string_lossy().to_string()) .expect("need to specify SYSROOT env var during clippy compilation, or use rustup or multirust"); + // make "clippy-driver --rustc" work like a subcommand that passes further args to "rustc" + // for example `clippy-driver --rustc --version` will print the rustc version that clippy-driver + // uses + if let Some(pos) = orig_args.iter().position(|arg| arg == "--rustc") { + orig_args.remove(pos); + orig_args[0] = "rustc".to_string(); + + // if we call "rustc", we need to pass --sysroot here as well + let mut args: Vec = orig_args.clone(); + if !have_sys_root_arg { + args.extend(vec!["--sysroot".into(), sys_root]); + }; + + println!("args: {:?}", args); + return rustc_driver::run_compiler(&args, &mut DefaultCallbacks, None, None); + } + + if orig_args.iter().any(|a| a == "--version" || a == "-V") { + let version_info = rustc_tools_util::get_version_info!(); + println!("{}", version_info); + exit(0); + } + // Setting RUSTC_WRAPPER causes Cargo to pass 'rustc' as the first argument. // We're invoking the compiler programmatically, so we ignore this/ let wrapper_mode = orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref()); From 88ab10400b81338782c729e4193e47cd3ef1cab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Tue, 2 Jun 2020 14:50:44 +0200 Subject: [PATCH 03/15] add test and remove debug print --- .github/driver.sh | 5 +++++ src/driver.rs | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/driver.sh b/.github/driver.sh index a2e87f5eb3745..c797bdb14db87 100644 --- a/.github/driver.sh +++ b/.github/driver.sh @@ -26,4 +26,9 @@ unset CARGO_MANIFEST_DIR sed -e "s,tests/ui,\$DIR," -e "/= help/d" cstring.stderr > normalized.stderr diff normalized.stderr tests/ui/cstring.stderr + +# make sure "clippy-driver --rustc --arg" and "rustc --arg" behave the same +SYSROOT=`rustc --print sysroot` +diff <(LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver --rustc --version --verbose) <(rustc --version --verbose) + # TODO: CLIPPY_CONF_DIR / CARGO_MANIFEST_DIR diff --git a/src/driver.rs b/src/driver.rs index 1956effa827b3..5ef8d3cf80977 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -355,7 +355,6 @@ pub fn main() { args.extend(vec!["--sysroot".into(), sys_root]); }; - println!("args: {:?}", args); return rustc_driver::run_compiler(&args, &mut DefaultCallbacks, None, None); } From f1d5cd5d13bbd59c21e70e2e728c3db9829cf816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Sun, 7 Jun 2020 16:27:41 +0200 Subject: [PATCH 04/15] add test for compiler output when compiling with rustc/clippy-driver --- .github/driver.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/driver.sh b/.github/driver.sh index c797bdb14db87..f8139f36369c9 100644 --- a/.github/driver.sh +++ b/.github/driver.sh @@ -31,4 +31,9 @@ diff normalized.stderr tests/ui/cstring.stderr SYSROOT=`rustc --print sysroot` diff <(LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver --rustc --version --verbose) <(rustc --version --verbose) +# we can't run 2 rustcs on the same file at the same time +CLIPPY=`LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver tests/driver/main.rs` +RUSTC=`rustc tests/driver/main.rs` +diff <($CLIPPY) <($RUSTC) + # TODO: CLIPPY_CONF_DIR / CARGO_MANIFEST_DIR From 7a62380fc8f8ca39bc49b8f67a4d4929911cb036 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Tue, 9 Jun 2020 13:18:00 +0200 Subject: [PATCH 05/15] clippy-driver: fix test and add --rustc to --help output --- .github/driver.sh | 6 ++++-- src/driver.rs | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/driver.sh b/.github/driver.sh index f8139f36369c9..2c17c4203ae5c 100644 --- a/.github/driver.sh +++ b/.github/driver.sh @@ -31,9 +31,11 @@ diff normalized.stderr tests/ui/cstring.stderr SYSROOT=`rustc --print sysroot` diff <(LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver --rustc --version --verbose) <(rustc --version --verbose) + +echo "fn main() {}" > target/driver_test.rs # we can't run 2 rustcs on the same file at the same time -CLIPPY=`LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver tests/driver/main.rs` -RUSTC=`rustc tests/driver/main.rs` +CLIPPY=`LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver ./target/driver_test.rs --rustc` +RUSTC=`rustc ./target/driver_test.rs` diff <($CLIPPY) <($RUSTC) # TODO: CLIPPY_CONF_DIR / CARGO_MANIFEST_DIR diff --git a/src/driver.rs b/src/driver.rs index 5ef8d3cf80977..6faa5e9fe661f 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -207,6 +207,7 @@ Usage: Common options: -h, --help Print this message + --rustc Pass all args to rustc -V, --version Print version info and exit Other options are the same as `cargo check`. From b21ef2b365ef4f5c973327f3f6064b8be42d1dae Mon Sep 17 00:00:00 2001 From: Eduardo Broto Date: Sat, 13 Jun 2020 00:52:32 +0200 Subject: [PATCH 06/15] Fix ICE in consts::binop --- clippy_lints/src/consts.rs | 2 +- tests/ui/crashes/ice-5389.rs | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 tests/ui/crashes/ice-5389.rs diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index 81ddc8c0067c7..e3c6908b76b18 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -396,7 +396,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { let l = self.expr(left)?; let r = self.expr(right); match (l, r) { - (Constant::Int(l), Some(Constant::Int(r))) => match self.tables.expr_ty(left).kind { + (Constant::Int(l), Some(Constant::Int(r))) => match self.tables.expr_ty_opt(left)?.kind { ty::Int(ity) => { let l = sext(self.lcx.tcx, l, ity); let r = sext(self.lcx.tcx, r, ity); diff --git a/tests/ui/crashes/ice-5389.rs b/tests/ui/crashes/ice-5389.rs new file mode 100644 index 0000000000000..de262199004b0 --- /dev/null +++ b/tests/ui/crashes/ice-5389.rs @@ -0,0 +1,13 @@ +#![allow(clippy::explicit_counter_loop)] + +fn main() { + let v = vec![1, 2, 3]; + let mut i = 0; + let max_storage_size = [0; 128 * 1024]; + for item in &v { + bar(i, *item); + i += 1; + } +} + +fn bar(_: usize, _: u32) {} From dee794f4503dda6a10891b49a7cb2f8bb92e001b Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sat, 13 Jun 2020 12:42:58 +0200 Subject: [PATCH 07/15] typo --- clippy_lints/src/new_without_default.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index dd236535c18ad..42200385932b0 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -33,7 +33,7 @@ declare_clippy_lint! { /// } /// ``` /// - /// To fix the lint, and a `Default` implementation that delegates to `new`: + /// To fix the lint, add a `Default` implementation that delegates to `new`: /// /// ```ignore /// struct Foo(Bar); From f663a21c8f51db58ff73e2e0d9852d03e8916a5e Mon Sep 17 00:00:00 2001 From: Iain Brandram-Adams Date: Sun, 14 Jun 2020 01:24:36 +1200 Subject: [PATCH 08/15] Remove `bar` from blacklisted names --- clippy_lints/src/utils/conf.rs | 2 +- tests/ui/blacklisted_name.rs | 23 +++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 9e8e0ff30ec6b..418c4ded544c0 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -107,7 +107,7 @@ macro_rules! define_Conf { pub use self::helpers::Conf; define_Conf! { /// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about - (blacklisted_names, "blacklisted_names": Vec, ["foo", "bar", "baz", "quux"].iter().map(ToString::to_string).collect()), + (blacklisted_names, "blacklisted_names": Vec, ["foo", "baz", "quux"].iter().map(ToString::to_string).collect()), /// Lint: COGNITIVE_COMPLEXITY. The maximum cognitive complexity a function can have (cognitive_complexity_threshold, "cognitive_complexity_threshold": u64, 25), /// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY. Use the Cognitive Complexity lint instead. diff --git a/tests/ui/blacklisted_name.rs b/tests/ui/blacklisted_name.rs index ca9d8d16b787d..3d87eb0618157 100644 --- a/tests/ui/blacklisted_name.rs +++ b/tests/ui/blacklisted_name.rs @@ -12,29 +12,32 @@ fn test(foo: ()) {} fn main() { let foo = 42; - let bar = 42; let baz = 42; + let quux = 42; + // Unlike these others, `bar` is considered an acceptable name to use. + // See https://github.com/rust-lang/rust-clippy/issues/5225. - let barb = 42; - let barbaric = 42; + let food = 42; + let foodstuffs = 42; + let bazaar = 42; match (42, Some(1337), Some(0)) { - (foo, Some(bar), baz @ Some(_)) => (), + (foo, Some(baz), quux @ Some(_)) => (), _ => (), } } fn issue_1647(mut foo: u8) { - let mut bar = 0; - if let Some(mut baz) = Some(42) {} + let mut baz = 0; + if let Some(mut quux) = Some(42) {} } fn issue_1647_ref() { - let ref bar = 0; - if let Some(ref baz) = Some(42) {} + let ref baz = 0; + if let Some(ref quux) = Some(42) {} } fn issue_1647_ref_mut() { - let ref mut bar = 0; - if let Some(ref mut baz) = Some(42) {} + let ref mut baz = 0; + if let Some(ref mut quux) = Some(42) {} } From c020e45cd3c1f10d1271de8b47cea9e00b73f500 Mon Sep 17 00:00:00 2001 From: Iain Brandram-Adams Date: Sun, 14 Jun 2020 12:40:36 +1200 Subject: [PATCH 09/15] Update stderr to match, and reinforce comments --- tests/ui/blacklisted_name.rs | 4 +- tests/ui/blacklisted_name.stderr | 72 ++++++++++++++++---------------- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/tests/ui/blacklisted_name.rs b/tests/ui/blacklisted_name.rs index 3d87eb0618157..cb15bdd2f1b2d 100644 --- a/tests/ui/blacklisted_name.rs +++ b/tests/ui/blacklisted_name.rs @@ -14,8 +14,10 @@ fn main() { let foo = 42; let baz = 42; let quux = 42; - // Unlike these others, `bar` is considered an acceptable name to use. + // Unlike these others, `bar` is actually considered an acceptable name. + // Among many other legitimate uses, bar commonly refers to a period of time in music. // See https://github.com/rust-lang/rust-clippy/issues/5225. + let bar = 42; let food = 42; let foodstuffs = 42; diff --git a/tests/ui/blacklisted_name.stderr b/tests/ui/blacklisted_name.stderr index 44123829fb0f6..70dbdaece8b6b 100644 --- a/tests/ui/blacklisted_name.stderr +++ b/tests/ui/blacklisted_name.stderr @@ -12,77 +12,77 @@ error: use of a blacklisted/placeholder name `foo` LL | let foo = 42; | ^^^ -error: use of a blacklisted/placeholder name `bar` +error: use of a blacklisted/placeholder name `baz` --> $DIR/blacklisted_name.rs:15:9 | -LL | let bar = 42; +LL | let baz = 42; | ^^^ -error: use of a blacklisted/placeholder name `baz` +error: use of a blacklisted/placeholder name `quux` --> $DIR/blacklisted_name.rs:16:9 | -LL | let baz = 42; - | ^^^ +LL | let quux = 42; + | ^^^^ error: use of a blacklisted/placeholder name `foo` - --> $DIR/blacklisted_name.rs:22:10 + --> $DIR/blacklisted_name.rs:27:10 | -LL | (foo, Some(bar), baz @ Some(_)) => (), +LL | (foo, Some(baz), quux @ Some(_)) => (), | ^^^ -error: use of a blacklisted/placeholder name `bar` - --> $DIR/blacklisted_name.rs:22:20 +error: use of a blacklisted/placeholder name `baz` + --> $DIR/blacklisted_name.rs:27:20 | -LL | (foo, Some(bar), baz @ Some(_)) => (), +LL | (foo, Some(baz), quux @ Some(_)) => (), | ^^^ -error: use of a blacklisted/placeholder name `baz` - --> $DIR/blacklisted_name.rs:22:26 +error: use of a blacklisted/placeholder name `quux` + --> $DIR/blacklisted_name.rs:27:26 | -LL | (foo, Some(bar), baz @ Some(_)) => (), - | ^^^ +LL | (foo, Some(baz), quux @ Some(_)) => (), + | ^^^^ error: use of a blacklisted/placeholder name `foo` - --> $DIR/blacklisted_name.rs:27:19 + --> $DIR/blacklisted_name.rs:32:19 | LL | fn issue_1647(mut foo: u8) { | ^^^ -error: use of a blacklisted/placeholder name `bar` - --> $DIR/blacklisted_name.rs:28:13 +error: use of a blacklisted/placeholder name `baz` + --> $DIR/blacklisted_name.rs:33:13 | -LL | let mut bar = 0; +LL | let mut baz = 0; | ^^^ -error: use of a blacklisted/placeholder name `baz` - --> $DIR/blacklisted_name.rs:29:21 +error: use of a blacklisted/placeholder name `quux` + --> $DIR/blacklisted_name.rs:34:21 | -LL | if let Some(mut baz) = Some(42) {} - | ^^^ +LL | if let Some(mut quux) = Some(42) {} + | ^^^^ -error: use of a blacklisted/placeholder name `bar` - --> $DIR/blacklisted_name.rs:33:13 +error: use of a blacklisted/placeholder name `baz` + --> $DIR/blacklisted_name.rs:38:13 | -LL | let ref bar = 0; +LL | let ref baz = 0; | ^^^ -error: use of a blacklisted/placeholder name `baz` - --> $DIR/blacklisted_name.rs:34:21 +error: use of a blacklisted/placeholder name `quux` + --> $DIR/blacklisted_name.rs:39:21 | -LL | if let Some(ref baz) = Some(42) {} - | ^^^ +LL | if let Some(ref quux) = Some(42) {} + | ^^^^ -error: use of a blacklisted/placeholder name `bar` - --> $DIR/blacklisted_name.rs:38:17 +error: use of a blacklisted/placeholder name `baz` + --> $DIR/blacklisted_name.rs:43:17 | -LL | let ref mut bar = 0; +LL | let ref mut baz = 0; | ^^^ -error: use of a blacklisted/placeholder name `baz` - --> $DIR/blacklisted_name.rs:39:25 +error: use of a blacklisted/placeholder name `quux` + --> $DIR/blacklisted_name.rs:44:25 | -LL | if let Some(ref mut baz) = Some(42) {} - | ^^^ +LL | if let Some(ref mut quux) = Some(42) {} + | ^^^^ error: aborting due to 14 previous errors From 454ed47acf462c847956464ac3811546b733bd5b Mon Sep 17 00:00:00 2001 From: Iain Brandram-Adams Date: Sun, 14 Jun 2020 12:46:56 +1200 Subject: [PATCH 10/15] Update comment in conf.rs --- clippy_lints/src/utils/conf.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 418c4ded544c0..c41befbf147b8 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -106,7 +106,7 @@ macro_rules! define_Conf { pub use self::helpers::Conf; define_Conf! { - /// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about + /// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses (blacklisted_names, "blacklisted_names": Vec, ["foo", "baz", "quux"].iter().map(ToString::to_string).collect()), /// Lint: COGNITIVE_COMPLEXITY. The maximum cognitive complexity a function can have (cognitive_complexity_threshold, "cognitive_complexity_threshold": u64, 25), From 8a6f42a9707bbad1dad3f1511f793cd07c723bb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Sun, 14 Jun 2020 11:07:44 +0200 Subject: [PATCH 11/15] Fix typo in wildcard_imports --- clippy_lints/src/wildcard_imports.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index b637253bd0264..79f7705e281e5 100644 --- a/clippy_lints/src/wildcard_imports.rs +++ b/clippy_lints/src/wildcard_imports.rs @@ -36,7 +36,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// **What it does:** Checks for wildcard imports `use _::*`. /// - /// **Why is this bad?** wildcard imports can polute the namespace. This is especially bad if + /// **Why is this bad?** wildcard imports can pollute the namespace. This is especially bad if /// you try to import something through a wildcard, that already has been imported by name from /// a different source: /// From a7743e9084f9ccd7c966f98a14fa667c694d66ab Mon Sep 17 00:00:00 2001 From: Eduardo Broto Date: Wed, 17 Jun 2020 00:32:47 +0200 Subject: [PATCH 12/15] redundant_pattern_matching: avoid non-const fn in const context --- .../src/redundant_pattern_matching.rs | 80 ++++++++++++++----- tests/ui/redundant_pattern_matching.fixed | 42 ++++++++++ tests/ui/redundant_pattern_matching.rs | 42 ++++++++++ tests/ui/redundant_pattern_matching.stderr | 56 ++++++------- ...undant_pattern_matching_const_result.fixed | 46 +++++++++++ ...redundant_pattern_matching_const_result.rs | 52 ++++++++++++ ...ndant_pattern_matching_const_result.stderr | 46 +++++++++++ 7 files changed, 318 insertions(+), 46 deletions(-) create mode 100644 tests/ui/redundant_pattern_matching_const_result.fixed create mode 100644 tests/ui/redundant_pattern_matching_const_result.rs create mode 100644 tests/ui/redundant_pattern_matching_const_result.stderr diff --git a/clippy_lints/src/redundant_pattern_matching.rs b/clippy_lints/src/redundant_pattern_matching.rs index 7ee298e9833f2..b95b12c4eb522 100644 --- a/clippy_lints/src/redundant_pattern_matching.rs +++ b/clippy_lints/src/redundant_pattern_matching.rs @@ -1,10 +1,13 @@ -use crate::utils::{match_qpath, match_trait_method, paths, snippet, span_lint_and_then}; +use crate::utils::{in_constant, match_qpath, match_trait_method, paths, snippet, span_lint_and_then}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath}; +use rustc_hir::{Arm, Expr, ExprKind, HirId, MatchSource, PatKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_mir::const_eval::is_const_fn; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::source_map::Symbol; declare_clippy_lint! { /// **What it does:** Lint for redundant pattern matching over `Result` or @@ -64,26 +67,37 @@ fn find_sugg_for_if_let<'a, 'tcx>( arms: &[Arm<'_>], keyword: &'static str, ) { + fn find_suggestion(cx: &LateContext<'_, '_>, hir_id: HirId, path: &QPath<'_>) -> Option<&'static str> { + if match_qpath(path, &paths::RESULT_OK) && can_suggest(cx, hir_id, sym!(result_type), "is_ok") { + return Some("is_ok()"); + } + if match_qpath(path, &paths::RESULT_ERR) && can_suggest(cx, hir_id, sym!(result_type), "is_err") { + return Some("is_err()"); + } + if match_qpath(path, &paths::OPTION_SOME) && can_suggest(cx, hir_id, sym!(option_type), "is_some") { + return Some("is_some()"); + } + if match_qpath(path, &paths::OPTION_NONE) && can_suggest(cx, hir_id, sym!(option_type), "is_none") { + return Some("is_none()"); + } + None + } + + let hir_id = expr.hir_id; let good_method = match arms[0].pat.kind { PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => { if let PatKind::Wild = patterns[0].kind { - if match_qpath(path, &paths::RESULT_OK) { - "is_ok()" - } else if match_qpath(path, &paths::RESULT_ERR) { - "is_err()" - } else if match_qpath(path, &paths::OPTION_SOME) { - "is_some()" - } else { - return; - } + find_suggestion(cx, hir_id, path) } else { - return; + None } }, - - PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()", - - _ => return, + PatKind::Path(ref path) => find_suggestion(cx, hir_id, path), + _ => None, + }; + let good_method = match good_method { + Some(method) => method, + None => return, }; // check that `while_let_on_iterator` lint does not trigger @@ -128,6 +142,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_ if arms.len() == 2 { let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind); + let hir_id = expr.hir_id; let found_good_method = match node_pair { ( PatKind::TupleStruct(ref path_left, ref patterns_left, _), @@ -142,6 +157,8 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_ &paths::RESULT_ERR, "is_ok()", "is_err()", + || can_suggest(cx, hir_id, sym!(result_type), "is_ok"), + || can_suggest(cx, hir_id, sym!(result_type), "is_err"), ) } else { None @@ -160,6 +177,8 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_ &paths::OPTION_NONE, "is_some()", "is_none()", + || can_suggest(cx, hir_id, sym!(option_type), "is_some"), + || can_suggest(cx, hir_id, sym!(option_type), "is_none"), ) } else { None @@ -188,6 +207,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_ } } +#[allow(clippy::too_many_arguments)] fn find_good_method_for_match<'a>( arms: &[Arm<'_>], path_left: &QPath<'_>, @@ -196,6 +216,8 @@ fn find_good_method_for_match<'a>( expected_right: &[&str], should_be_left: &'a str, should_be_right: &'a str, + can_suggest_left: impl Fn() -> bool, + can_suggest_right: impl Fn() -> bool, ) -> Option<&'a str> { let body_node_pair = if match_qpath(path_left, expected_left) && match_qpath(path_right, expected_right) { (&(*arms[0].body).kind, &(*arms[1].body).kind) @@ -207,10 +229,32 @@ fn find_good_method_for_match<'a>( match body_node_pair { (ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => match (&lit_left.node, &lit_right.node) { - (LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left), - (LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right), + (LitKind::Bool(true), LitKind::Bool(false)) if can_suggest_left() => Some(should_be_left), + (LitKind::Bool(false), LitKind::Bool(true)) if can_suggest_right() => Some(should_be_right), _ => None, }, _ => None, } } + +fn can_suggest(cx: &LateContext<'_, '_>, hir_id: HirId, diag_item: Symbol, name: &str) -> bool { + if !in_constant(cx, hir_id) { + return true; + } + + // Avoid suggesting calls to non-`const fn`s in const contexts, see #5697. + cx.tcx + .get_diagnostic_item(diag_item) + .and_then(|def_id| { + cx.tcx.inherent_impls(def_id).iter().find_map(|imp| { + cx.tcx + .associated_items(*imp) + .in_definition_order() + .find_map(|item| match item.kind { + ty::AssocKind::Fn if item.ident.name.as_str() == name => Some(item.def_id), + _ => None, + }) + }) + }) + .map_or(false, |def_id| is_const_fn(cx.tcx, def_id)) +} diff --git a/tests/ui/redundant_pattern_matching.fixed b/tests/ui/redundant_pattern_matching.fixed index fc8cb0e747c73..6ba5cfb1d7177 100644 --- a/tests/ui/redundant_pattern_matching.fixed +++ b/tests/ui/redundant_pattern_matching.fixed @@ -1,5 +1,7 @@ // run-rustfix +#![feature(const_if_match)] +#![feature(const_loop)] #![warn(clippy::all)] #![warn(clippy::redundant_pattern_matching)] #![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)] @@ -67,6 +69,7 @@ fn main() { takes_bool(x); issue5504(); + issue5697(); let _ = if gen_opt().is_some() { 1 @@ -117,3 +120,42 @@ fn issue5504() { if m!().is_some() {} while m!().is_some() {} } + +// None of these should be linted because none of the suggested methods +// are `const fn` without toggling a feature. +const fn issue5697() { + if let Ok(_) = Ok::(42) {} + + if let Err(_) = Err::(42) {} + + if let Some(_) = Some(42) {} + + if let None = None::<()> {} + + while let Ok(_) = Ok::(10) {} + + while let Err(_) = Ok::(10) {} + + while let Some(_) = Some(42) {} + + while let None = None::<()> {} + + match Ok::(42) { + Ok(_) => true, + Err(_) => false, + }; + + match Err::(42) { + Ok(_) => false, + Err(_) => true, + }; + match Some(42) { + Some(_) => true, + None => false, + }; + + match None::<()> { + Some(_) => false, + None => true, + }; +} diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs index 51912dade0356..17de66f9ad0eb 100644 --- a/tests/ui/redundant_pattern_matching.rs +++ b/tests/ui/redundant_pattern_matching.rs @@ -1,5 +1,7 @@ // run-rustfix +#![feature(const_if_match)] +#![feature(const_loop)] #![warn(clippy::all)] #![warn(clippy::redundant_pattern_matching)] #![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)] @@ -88,6 +90,7 @@ fn main() { takes_bool(x); issue5504(); + issue5697(); let _ = if let Some(_) = gen_opt() { 1 @@ -138,3 +141,42 @@ fn issue5504() { if let Some(_) = m!() {} while let Some(_) = m!() {} } + +// None of these should be linted because none of the suggested methods +// are `const fn` without toggling a feature. +const fn issue5697() { + if let Ok(_) = Ok::(42) {} + + if let Err(_) = Err::(42) {} + + if let Some(_) = Some(42) {} + + if let None = None::<()> {} + + while let Ok(_) = Ok::(10) {} + + while let Err(_) = Ok::(10) {} + + while let Some(_) = Some(42) {} + + while let None = None::<()> {} + + match Ok::(42) { + Ok(_) => true, + Err(_) => false, + }; + + match Err::(42) { + Ok(_) => false, + Err(_) => true, + }; + match Some(42) { + Some(_) => true, + None => false, + }; + + match None::<()> { + Some(_) => false, + None => true, + }; +} diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr index b58deb7954efe..1b9a4b40a2f02 100644 --- a/tests/ui/redundant_pattern_matching.stderr +++ b/tests/ui/redundant_pattern_matching.stderr @@ -1,5 +1,5 @@ error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:8:12 + --> $DIR/redundant_pattern_matching.rs:10:12 | LL | if let Ok(_) = Ok::(42) {} | -------^^^^^--------------------- help: try this: `if Ok::(42).is_ok()` @@ -7,67 +7,67 @@ LL | if let Ok(_) = Ok::(42) {} = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:10:12 + --> $DIR/redundant_pattern_matching.rs:12:12 | LL | if let Err(_) = Err::(42) {} | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:12:12 + --> $DIR/redundant_pattern_matching.rs:14:12 | LL | if let None = None::<()> {} | -------^^^^------------- help: try this: `if None::<()>.is_none()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:14:12 + --> $DIR/redundant_pattern_matching.rs:16:12 | LL | if let Some(_) = Some(42) {} | -------^^^^^^^----------- help: try this: `if Some(42).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:16:12 + --> $DIR/redundant_pattern_matching.rs:18:12 | LL | if let Some(_) = Some(42) { | -------^^^^^^^----------- help: try this: `if Some(42).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:22:15 + --> $DIR/redundant_pattern_matching.rs:24:15 | LL | while let Some(_) = Some(42) {} | ----------^^^^^^^----------- help: try this: `while Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:24:15 + --> $DIR/redundant_pattern_matching.rs:26:15 | LL | while let None = Some(42) {} | ----------^^^^----------- help: try this: `while Some(42).is_none()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:26:15 + --> $DIR/redundant_pattern_matching.rs:28:15 | LL | while let None = None::<()> {} | ----------^^^^------------- help: try this: `while None::<()>.is_none()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:28:15 + --> $DIR/redundant_pattern_matching.rs:30:15 | LL | while let Ok(_) = Ok::(10) {} | ----------^^^^^--------------------- help: try this: `while Ok::(10).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:30:15 + --> $DIR/redundant_pattern_matching.rs:32:15 | LL | while let Err(_) = Ok::(10) {} | ----------^^^^^^--------------------- help: try this: `while Ok::(10).is_err()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:33:15 + --> $DIR/redundant_pattern_matching.rs:35:15 | LL | while let Some(_) = v.pop() { | ----------^^^^^^^---------- help: try this: `while v.pop().is_some()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:49:5 + --> $DIR/redundant_pattern_matching.rs:51:5 | LL | / match Ok::(42) { LL | | Ok(_) => true, @@ -76,7 +76,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:54:5 + --> $DIR/redundant_pattern_matching.rs:56:5 | LL | / match Ok::(42) { LL | | Ok(_) => false, @@ -85,7 +85,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_err()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:59:5 + --> $DIR/redundant_pattern_matching.rs:61:5 | LL | / match Err::(42) { LL | | Ok(_) => false, @@ -94,7 +94,7 @@ LL | | }; | |_____^ help: try this: `Err::(42).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:64:5 + --> $DIR/redundant_pattern_matching.rs:66:5 | LL | / match Err::(42) { LL | | Ok(_) => true, @@ -103,7 +103,7 @@ LL | | }; | |_____^ help: try this: `Err::(42).is_ok()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:69:5 + --> $DIR/redundant_pattern_matching.rs:71:5 | LL | / match Some(42) { LL | | Some(_) => true, @@ -112,7 +112,7 @@ LL | | }; | |_____^ help: try this: `Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:74:5 + --> $DIR/redundant_pattern_matching.rs:76:5 | LL | / match None::<()> { LL | | Some(_) => false, @@ -121,7 +121,7 @@ LL | | }; | |_____^ help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:79:13 + --> $DIR/redundant_pattern_matching.rs:81:13 | LL | let _ = match None::<()> { | _____________^ @@ -131,61 +131,61 @@ LL | | }; | |_____^ help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:84:20 + --> $DIR/redundant_pattern_matching.rs:86:20 | LL | let _ = if let Ok(_) = Ok::(4) { true } else { false }; | -------^^^^^--------------------- help: try this: `if Ok::(4).is_ok()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:87:20 + --> $DIR/redundant_pattern_matching.rs:89:20 | LL | let x = if let Some(_) = opt { true } else { false }; | -------^^^^^^^------ help: try this: `if opt.is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:92:20 + --> $DIR/redundant_pattern_matching.rs:95:20 | LL | let _ = if let Some(_) = gen_opt() { | -------^^^^^^^------------ help: try this: `if gen_opt().is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:94:19 + --> $DIR/redundant_pattern_matching.rs:97:19 | LL | } else if let None = gen_opt() { | -------^^^^------------ help: try this: `if gen_opt().is_none()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:96:19 + --> $DIR/redundant_pattern_matching.rs:99:19 | LL | } else if let Ok(_) = gen_res() { | -------^^^^^------------ help: try this: `if gen_res().is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:98:19 + --> $DIR/redundant_pattern_matching.rs:101:19 | LL | } else if let Err(_) = gen_res() { | -------^^^^^^------------ help: try this: `if gen_res().is_err()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:131:19 + --> $DIR/redundant_pattern_matching.rs:134:19 | LL | while let Some(_) = r#try!(result_opt()) {} | ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:132:16 + --> $DIR/redundant_pattern_matching.rs:135:16 | LL | if let Some(_) = r#try!(result_opt()) {} | -------^^^^^^^----------------------- help: try this: `if r#try!(result_opt()).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:138:12 + --> $DIR/redundant_pattern_matching.rs:141:12 | LL | if let Some(_) = m!() {} | -------^^^^^^^------- help: try this: `if m!().is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:139:15 + --> $DIR/redundant_pattern_matching.rs:142:15 | LL | while let Some(_) = m!() {} | ----------^^^^^^^------- help: try this: `while m!().is_some()` diff --git a/tests/ui/redundant_pattern_matching_const_result.fixed b/tests/ui/redundant_pattern_matching_const_result.fixed new file mode 100644 index 0000000000000..c8bc5458067d3 --- /dev/null +++ b/tests/ui/redundant_pattern_matching_const_result.fixed @@ -0,0 +1,46 @@ +// run-rustfix + +#![feature(const_if_match)] +#![feature(const_loop)] +#![feature(const_result)] +#![warn(clippy::redundant_pattern_matching)] +#![allow(unused)] + +// Test that results are linted with the feature enabled. + +const fn issue_5697() { + if Ok::(42).is_ok() {} + + if Err::(42).is_err() {} + + while Ok::(10).is_ok() {} + + while Ok::(10).is_err() {} + + Ok::(42).is_ok(); + + Err::(42).is_err(); + + // These should not be linted until `const_option` is implemented. + // See https://github.com/rust-lang/rust/issues/67441 + + if let Some(_) = Some(42) {} + + if let None = None::<()> {} + + while let Some(_) = Some(42) {} + + while let None = None::<()> {} + + match Some(42) { + Some(_) => true, + None => false, + }; + + match None::<()> { + Some(_) => false, + None => true, + }; +} + +fn main() {} diff --git a/tests/ui/redundant_pattern_matching_const_result.rs b/tests/ui/redundant_pattern_matching_const_result.rs new file mode 100644 index 0000000000000..75f37ec15c622 --- /dev/null +++ b/tests/ui/redundant_pattern_matching_const_result.rs @@ -0,0 +1,52 @@ +// run-rustfix + +#![feature(const_if_match)] +#![feature(const_loop)] +#![feature(const_result)] +#![warn(clippy::redundant_pattern_matching)] +#![allow(unused)] + +// Test that results are linted with the feature enabled. + +const fn issue_5697() { + if let Ok(_) = Ok::(42) {} + + if let Err(_) = Err::(42) {} + + while let Ok(_) = Ok::(10) {} + + while let Err(_) = Ok::(10) {} + + match Ok::(42) { + Ok(_) => true, + Err(_) => false, + }; + + match Err::(42) { + Ok(_) => false, + Err(_) => true, + }; + + // These should not be linted until `const_option` is implemented. + // See https://github.com/rust-lang/rust/issues/67441 + + if let Some(_) = Some(42) {} + + if let None = None::<()> {} + + while let Some(_) = Some(42) {} + + while let None = None::<()> {} + + match Some(42) { + Some(_) => true, + None => false, + }; + + match None::<()> { + Some(_) => false, + None => true, + }; +} + +fn main() {} diff --git a/tests/ui/redundant_pattern_matching_const_result.stderr b/tests/ui/redundant_pattern_matching_const_result.stderr new file mode 100644 index 0000000000000..c32292f0eee8b --- /dev/null +++ b/tests/ui/redundant_pattern_matching_const_result.stderr @@ -0,0 +1,46 @@ +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching_const_result.rs:12:12 + | +LL | if let Ok(_) = Ok::(42) {} + | -------^^^^^--------------------- help: try this: `if Ok::(42).is_ok()` + | + = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_const_result.rs:14:12 + | +LL | if let Err(_) = Err::(42) {} + | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching_const_result.rs:16:15 + | +LL | while let Ok(_) = Ok::(10) {} + | ----------^^^^^--------------------- help: try this: `while Ok::(10).is_ok()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_const_result.rs:18:15 + | +LL | while let Err(_) = Ok::(10) {} + | ----------^^^^^^--------------------- help: try this: `while Ok::(10).is_err()` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching_const_result.rs:20:5 + | +LL | / match Ok::(42) { +LL | | Ok(_) => true, +LL | | Err(_) => false, +LL | | }; + | |_____^ help: try this: `Ok::(42).is_ok()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_const_result.rs:25:5 + | +LL | / match Err::(42) { +LL | | Ok(_) => false, +LL | | Err(_) => true, +LL | | }; + | |_____^ help: try this: `Err::(42).is_err()` + +error: aborting due to 6 previous errors + From f3a40f5eb1cedebd374ebcd52341dfec60f3ea10 Mon Sep 17 00:00:00 2001 From: sozysozbot Date: Thu, 18 Jun 2020 06:50:04 +0900 Subject: [PATCH 13/15] Fix typo extending it's lifetime -> extending its lifetime --- clippy_lints/src/let_underscore.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/let_underscore.rs b/clippy_lints/src/let_underscore.rs index 710dec8d33fc9..acd628bbaca59 100644 --- a/clippy_lints/src/let_underscore.rs +++ b/clippy_lints/src/let_underscore.rs @@ -35,7 +35,7 @@ declare_clippy_lint! { /// **What it does:** Checks for `let _ = sync_lock` /// /// **Why is this bad?** This statement immediately drops the lock instead of - /// extending it's lifetime to the end of the scope, which is often not intended. + /// extending its lifetime to the end of the scope, which is often not intended. /// To extend lock lifetime to the end of the scope, use an underscore-prefixed /// name instead (i.e. _lock). If you want to explicitly drop the lock, /// `std::mem::drop` conveys your intention better and is less error-prone. From ed083cc95987ef37b7ed897f632b4a7a1baaff60 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sat, 13 Jun 2020 18:22:38 +0200 Subject: [PATCH 14/15] Use lints in Clippy that are enabled in rustc bootstrap --- clippy_lints/src/lib.rs | 17 ++++++++++------- src/driver.rs | 6 +++++- src/main.rs | 2 ++ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 021fbe932d895..25c1c2d73b6c4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1,19 +1,22 @@ // error-pattern:cargo-clippy #![feature(bindings_after_at)] -#![feature(box_syntax)] #![feature(box_patterns)] +#![feature(box_syntax)] +#![feature(concat_idents)] +#![feature(crate_visibility_modifier)] +#![feature(drain_filter)] #![feature(or_patterns)] #![feature(rustc_private)] #![feature(stmt_expr_attributes)] -#![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)] #![recursion_limit = "512"] -#![warn(rust_2018_idioms, trivial_casts, trivial_numeric_casts)] -#![deny(rustc::internal)] #![cfg_attr(feature = "deny-warnings", deny(warnings))] -#![feature(crate_visibility_modifier)] -#![feature(concat_idents)] -#![feature(drain_filter)] +#![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)] +#![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 +#![deny(rustc::internal)] // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) diff --git a/src/driver.rs b/src/driver.rs index 4453ae5ce4414..10f56fb2070f3 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -1,5 +1,9 @@ -#![cfg_attr(feature = "deny-warnings", deny(warnings))] #![feature(rustc_private)] +#![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)] +// warn on rustc internal lints +#![deny(rustc::internal)] // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) diff --git a/src/main.rs b/src/main.rs index bc43a34ed5d4a..6739a4cf2245e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,6 @@ #![cfg_attr(feature = "deny-warnings", deny(warnings))] +// warn on lints, that are included in `rust-lang/rust`s bootstrap +#![warn(rust_2018_idioms, unused_lifetimes)] use rustc_tools_util::VersionInfo; use std::env; From b886c06c1a52a57890b318cc7304f7384e696ec8 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sat, 13 Jun 2020 18:22:49 +0200 Subject: [PATCH 15/15] Fix fallout --- clippy_lints/src/loops.rs | 6 +++--- clippy_lints/src/suspicious_trait_impl.rs | 2 +- clippy_lints/src/trivially_copy_pass_by_ref.rs | 2 +- clippy_lints/src/utils/sugg.rs | 4 ++-- src/driver.rs | 6 ++++-- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index ae1aa66be5cda..9020b47a146fa 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1497,7 +1497,7 @@ struct MutatePairDelegate<'a, 'tcx> { span_high: Option, } -impl<'a, 'tcx> Delegate<'tcx> for MutatePairDelegate<'a, 'tcx> { +impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> { fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: ConsumeMode) {} fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, bk: ty::BorrowKind) { @@ -1525,7 +1525,7 @@ impl<'a, 'tcx> Delegate<'tcx> for MutatePairDelegate<'a, 'tcx> { } } -impl<'a, 'tcx> MutatePairDelegate<'a, 'tcx> { +impl MutatePairDelegate<'_, '_> { fn mutation_span(&self) -> (Option, Option) { (self.span_low, self.span_high) } @@ -2292,7 +2292,7 @@ struct HasBreakOrReturnVisitor { has_break_or_return: bool, } -impl<'a, 'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor { +impl<'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor { type Map = Map<'tcx>; fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index a9e6fa329c0f0..cf71c3144a2eb 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -184,7 +184,7 @@ struct BinaryExprVisitor { in_binary_expr: bool, } -impl<'a, 'tcx> Visitor<'tcx> for BinaryExprVisitor { +impl<'tcx> Visitor<'tcx> for BinaryExprVisitor { type Map = Map<'tcx>; fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { diff --git a/clippy_lints/src/trivially_copy_pass_by_ref.rs b/clippy_lints/src/trivially_copy_pass_by_ref.rs index 8e0cb94317aff..146ac4b09d5a4 100644 --- a/clippy_lints/src/trivially_copy_pass_by_ref.rs +++ b/clippy_lints/src/trivially_copy_pass_by_ref.rs @@ -58,7 +58,7 @@ pub struct TriviallyCopyPassByRef { limit: u64, } -impl<'a, 'tcx> TriviallyCopyPassByRef { +impl<'tcx> TriviallyCopyPassByRef { pub fn new(limit: Option, target: &SessionConfig) -> Self { let limit = limit.unwrap_or_else(|| { let bit_width = u64::from(target.ptr_width); diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 73758b7eeb7eb..e919b1522d89a 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -509,7 +509,7 @@ fn indentation(cx: &T, span: Span) -> Option { } /// Convenience extension trait for `DiagnosticBuilder`. -pub trait DiagnosticBuilderExt<'a, T: LintContext> { +pub trait DiagnosticBuilderExt { /// Suggests to add an attribute to an item. /// /// Correctly handles indentation of the attribute and item. @@ -556,7 +556,7 @@ pub trait DiagnosticBuilderExt<'a, T: LintContext> { fn suggest_remove_item(&mut self, cx: &T, item: Span, msg: &str, applicability: Applicability); } -impl<'a, 'b, 'c, T: LintContext> DiagnosticBuilderExt<'c, T> for rustc_errors::DiagnosticBuilder<'b> { +impl DiagnosticBuilderExt for rustc_errors::DiagnosticBuilder<'_> { fn suggest_item_with_attr( &mut self, cx: &T, diff --git a/src/driver.rs b/src/driver.rs index 10f56fb2070f3..3d12436e9af8e 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -8,6 +8,8 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) #[allow(unused_extern_crates)] +extern crate rustc_data_structures; +#[allow(unused_extern_crates)] extern crate rustc_driver; #[allow(unused_extern_crates)] extern crate rustc_errors; @@ -97,7 +99,7 @@ impl rustc_driver::Callbacks for ClippyCallbacks { #[allow(clippy::find_map, clippy::filter_map)] fn describe_lints() { use lintlist::{Level, Lint, ALL_LINTS, LINT_LEVELS}; - use std::collections::HashSet; + use rustc_data_structures::fx::FxHashSet; println!( " @@ -141,7 +143,7 @@ Available lint options: let scoped = |x: &str| format!("clippy::{}", x); - let lint_groups: HashSet<_> = lints.iter().map(|lint| lint.group).collect(); + let lint_groups: FxHashSet<_> = lints.iter().map(|lint| lint.group).collect(); println!("Lint checks provided by clippy:\n"); println!(" {} {:7.7} meaning", padded("name"), "default");