From 1a1adad81d0fd7d0c64d0a72575a6b4db69c035a Mon Sep 17 00:00:00 2001 From: flip1995 Date: Fri, 26 Mar 2021 10:56:05 +0100 Subject: [PATCH 1/3] Add MSRV option to unnested_or_patterns lint --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/unnested_or_patterns.rs | 44 +++++++++++++++++++----- clippy_lints/src/utils/conf.rs | 2 +- tests/ui/min_rust_version_attr.rs | 6 ++++ tests/ui/min_rust_version_attr.stderr | 8 ++--- 5 files changed, 48 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 3a9236d87..f01361311 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1079,6 +1079,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box missing_const_for_fn::MissingConstForFn::new(msrv)); store.register_late_pass(move || box needless_question_mark::NeedlessQuestionMark::new(msrv)); store.register_late_pass(move || box casts::Casts::new(msrv)); + store.register_early_pass(move || box unnested_or_patterns::UnnestedOrPatterns::new(msrv)); store.register_late_pass(|| box size_of_in_element_count::SizeOfInElementCount); store.register_late_pass(|| box map_clone::MapClone); @@ -1254,7 +1255,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(move || box non_expressive_names::NonExpressiveNames { single_char_binding_names_threshold, }); - store.register_early_pass(|| box unnested_or_patterns::UnnestedOrPatterns); store.register_late_pass(|| box macro_use::MacroUseImports::default()); store.register_late_pass(|| box map_identity::MapIdentity); store.register_late_pass(|| box pattern_type_mismatch::PatternTypeMismatch); diff --git a/clippy_lints/src/unnested_or_patterns.rs b/clippy_lints/src/unnested_or_patterns.rs index 2b9479365..9376a2cf6 100644 --- a/clippy_lints/src/unnested_or_patterns.rs +++ b/clippy_lints/src/unnested_or_patterns.rs @@ -1,15 +1,19 @@ #![allow(clippy::wildcard_imports, clippy::enum_glob_use)] -use clippy_utils::ast_utils::{eq_field_pat, eq_id, eq_pat, eq_path}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::over; +use clippy_utils::{ + ast_utils::{eq_field_pat, eq_id, eq_pat, eq_path}, + meets_msrv, +}; use rustc_ast::mut_visit::*; use rustc_ast::ptr::P; use rustc_ast::{self as ast, Pat, PatKind, PatKind::*, DUMMY_NODE_ID}; use rustc_ast_pretty::pprust; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::DUMMY_SP; use std::cell::Cell; @@ -50,26 +54,50 @@ declare_clippy_lint! { "unnested or-patterns, e.g., `Foo(Bar) | Foo(Baz) instead of `Foo(Bar | Baz)`" } -declare_lint_pass!(UnnestedOrPatterns => [UNNESTED_OR_PATTERNS]); +const UNNESTED_OR_PATTERNS_MSRV: RustcVersion = RustcVersion::new(1, 53, 0); + +#[derive(Clone, Copy)] +pub struct UnnestedOrPatterns { + msrv: Option, +} + +impl UnnestedOrPatterns { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(UnnestedOrPatterns => [UNNESTED_OR_PATTERNS]); impl EarlyLintPass for UnnestedOrPatterns { fn check_arm(&mut self, cx: &EarlyContext<'_>, a: &ast::Arm) { - lint_unnested_or_patterns(cx, &a.pat); + if meets_msrv(self.msrv.as_ref(), &UNNESTED_OR_PATTERNS_MSRV) { + lint_unnested_or_patterns(cx, &a.pat); + } } fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { - if let ast::ExprKind::Let(pat, _) = &e.kind { - lint_unnested_or_patterns(cx, pat); + if meets_msrv(self.msrv.as_ref(), &UNNESTED_OR_PATTERNS_MSRV) { + if let ast::ExprKind::Let(pat, _) = &e.kind { + lint_unnested_or_patterns(cx, pat); + } } } fn check_param(&mut self, cx: &EarlyContext<'_>, p: &ast::Param) { - lint_unnested_or_patterns(cx, &p.pat); + if meets_msrv(self.msrv.as_ref(), &UNNESTED_OR_PATTERNS_MSRV) { + lint_unnested_or_patterns(cx, &p.pat); + } } fn check_local(&mut self, cx: &EarlyContext<'_>, l: &ast::Local) { - lint_unnested_or_patterns(cx, &l.pat); + if meets_msrv(self.msrv.as_ref(), &UNNESTED_OR_PATTERNS_MSRV) { + lint_unnested_or_patterns(cx, &l.pat); + } } + + extract_msrv_attr!(EarlyContext); } fn lint_unnested_or_patterns(cx: &EarlyContext<'_>, pat: &Pat) { diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 9139a0966..b372f5b0e 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: REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN. The minimum rust version that the project supports + /// Lint: REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS. The minimum rust version that the project supports (msrv, "msrv": Option, None), /// 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()), diff --git a/tests/ui/min_rust_version_attr.rs b/tests/ui/min_rust_version_attr.rs index 0f47f1cbc..49ace1ca1 100644 --- a/tests/ui/min_rust_version_attr.rs +++ b/tests/ui/min_rust_version_attr.rs @@ -123,6 +123,11 @@ fn missing_const_for_fn() -> i32 { 1 } +fn unnest_or_patterns() { + struct TS(u8, u8); + if let TS(0, x) | TS(1, x) = TS(0, 0) {} +} + fn main() { filter_map_next(); checked_conversion(); @@ -138,6 +143,7 @@ fn main() { replace_with_default(); map_unwrap_or(); missing_const_for_fn(); + unnest_or_patterns(); } mod meets_msrv { diff --git a/tests/ui/min_rust_version_attr.stderr b/tests/ui/min_rust_version_attr.stderr index e3e3b335c..8d3575c2d 100644 --- a/tests/ui/min_rust_version_attr.stderr +++ b/tests/ui/min_rust_version_attr.stderr @@ -1,12 +1,12 @@ error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:150:24 + --> $DIR/min_rust_version_attr.rs:156:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::manual-strip` implied by `-D warnings` note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:149:9 + --> $DIR/min_rust_version_attr.rs:155:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,13 +17,13 @@ LL | assert_eq!(.to_uppercase(), "WORLD!"); | error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:162:24 + --> $DIR/min_rust_version_attr.rs:168:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:161:9 + --> $DIR/min_rust_version_attr.rs:167:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 9ce9989f5902a2f563b7e06d1fbae326f0fed514 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Fri, 26 Mar 2021 10:57:51 +0100 Subject: [PATCH 2/3] Improve doc on how to add MSRV to a lint --- doc/adding_lints.md | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/doc/adding_lints.md b/doc/adding_lints.md index 575853996..9fb22c1ea 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -388,18 +388,19 @@ pass. [`FnKind::Fn`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/visit/enum.FnKind.html#variant.Fn [ident]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/symbol/struct.Ident.html -## Specifying the lint's minimum supported Rust version (msrv) +## Specifying the lint's minimum supported Rust version (MSRV) -Projects supporting older versions of Rust would need to disable a lint if it targets features -present in later versions. Support for this can be added by specifying an msrv in your lint like so, +Projects supporting older versions of Rust would need to disable a lint if it +targets features present in later versions. Support for this can be added by +specifying an MSRV in your lint like so, ```rust const MANUAL_STRIP_MSRV: RustcVersion = RustcVersion::new(1, 45, 0); ``` -The project's msrv will also have to be an attribute in the lint so you'll have to add a struct -and constructor for your lint. The project's msrv needs to be passed when the lint is registered -in `lib.rs` +The project's MSRV will also have to be an attribute in the lint so you'll have +to add a struct and constructor for your lint. The project's MSRV needs to be +passed when the lint is registered in `lib.rs` ```rust pub struct ManualStrip { @@ -414,8 +415,8 @@ impl ManualStrip { } ``` -The project's msrv can then be matched against the lint's msrv in the LintPass using the `meets_msrv` utility -function. +The project's MSRV can then be matched against the lint's `msrv` in the LintPass +using the `meets_msrv` utility function. ``` rust if !meets_msrv(self.msrv.as_ref(), &MANUAL_STRIP_MSRV) { @@ -423,9 +424,10 @@ if !meets_msrv(self.msrv.as_ref(), &MANUAL_STRIP_MSRV) { } ``` -The project's msrv can also be specified as an inner attribute, which overrides the value from -`clippy.toml`. This can be accounted for using the `extract_msrv_attr!(LintContext)` macro and passing -LateContext/EarlyContext. +The project's MSRV can also be specified as an inner attribute, which overrides +the value from `clippy.toml`. This can be accounted for using the +`extract_msrv_attr!(LintContext)` macro and passing +`LateContext`/`EarlyContext`. ```rust impl<'tcx> LateLintPass<'tcx> for ManualStrip { @@ -436,8 +438,20 @@ impl<'tcx> LateLintPass<'tcx> for ManualStrip { } ``` -Once the msrv is added to the lint, a relevant test case should be added to `tests/ui/min_rust_version_attr.rs` -which verifies that the lint isn't emitted if the project's msrv is lower. +Once the `msrv` is added to the lint, a relevant test case should be added to +`tests/ui/min_rust_version_attr.rs` which verifies that the lint isn't emitted +if the project's MSRV is lower. + +As a last step, the lint should be added to the lint documentation. This is done +in `clippy_lints/src/utils/conf.rs`: + +```rust +define_Conf! { + /// Lint: LIST, OF, LINTS, . The minimum rust version that the project supports + (msrv, "msrv": Option, None), + ... +} +``` ## Author lint From 5279b5948cf5acfb4787cbbe1dc4c568cd1cbfdb Mon Sep 17 00:00:00 2001 From: flip1995 Date: Fri, 26 Mar 2021 10:58:37 +0100 Subject: [PATCH 3/3] Fix trailing whitespaces in doc/adding_lints.md --- doc/adding_lints.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/adding_lints.md b/doc/adding_lints.md index 9fb22c1ea..ceb45a2c3 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -547,9 +547,9 @@ Before submitting your PR make sure you followed all of the basic requirements: ## Adding configuration to a lint -Clippy supports the configuration of lints values using a `clippy.toml` file in the workspace +Clippy supports the configuration of lints values using a `clippy.toml` file in the workspace directory. Adding a configuration to a lint can be useful for thresholds or to constrain some -behavior that can be seen as a false positive for some users. Adding a configuration is done +behavior that can be seen as a false positive for some users. Adding a configuration is done in the following steps: 1. Adding a new configuration entry to [clippy_utils::conf](/clippy_utils/src/conf.rs) @@ -558,10 +558,10 @@ in the following steps: /// Lint: LINT_NAME. (configuration_ident, "configuration_value": Type, DefaultValue), ``` - The configuration value and identifier should usually be the same. The doc comment will be + The configuration value and identifier should usually be the same. The doc comment will be automatically added to the lint documentation. 2. Adding the configuration value to the lint impl struct: - 1. This first requires the definition of a lint impl struct. Lint impl structs are usually + 1. This first requires the definition of a lint impl struct. Lint impl structs are usually generated with the `declare_lint_pass!` macro. This struct needs to be defined manually to add some kind of metadata to it: ```rust @@ -578,7 +578,7 @@ in the following steps: LINT_NAME ]); ``` - + 2. Next add the configuration value and a corresponding creation method like this: ```rust #[derive(Copy, Clone)] @@ -598,7 +598,7 @@ in the following steps: ``` 3. Passing the configuration value to the lint impl struct: - First find the struct construction in the [clippy_lints lib file](/clippy_lints/src/lib.rs). + First find the struct construction in the [clippy_lints lib file](/clippy_lints/src/lib.rs). The configuration value is now cloned or copied into a local value that is then passed to the impl struct like this: ```rust @@ -615,9 +615,9 @@ in the following steps: 4. Adding tests: 1. The default configured value can be tested like any normal lint in [`tests/ui`](/tests/ui). - 2. The configuration itself will be tested separately in [`tests/ui-toml`](/tests/ui-toml). - Simply add a new subfolder with a fitting name. This folder contains a `clippy.toml` file - with the configuration value and a rust file that should be linted by Clippy. The test can + 2. The configuration itself will be tested separately in [`tests/ui-toml`](/tests/ui-toml). + Simply add a new subfolder with a fitting name. This folder contains a `clippy.toml` file + with the configuration value and a rust file that should be linted by Clippy. The test can otherwise be written as usual. ## Cheatsheet