Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MSRV options to unnested_or_patterns #6977

Merged
merged 3 commits into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
44 changes: 36 additions & 8 deletions clippy_lints/src/unnested_or_patterns.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<RustcVersion>,
}

impl UnnestedOrPatterns {
#[must_use]
pub fn new(msrv: Option<RustcVersion>) -> 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) {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>, 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<String>, ["foo", "baz", "quux"].iter().map(ToString::to_string).collect()),
Expand Down
58 changes: 36 additions & 22 deletions doc/adding_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -414,18 +415,19 @@ 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) {
return;
}
```

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 {
Expand All @@ -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_NEWLY_ADDED_LINT>. The minimum rust version that the project supports
(msrv, "msrv": Option<String>, None),
...
}
```

## Author lint

Expand Down Expand Up @@ -533,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)
Expand All @@ -544,10 +558,10 @@ in the following steps:
/// Lint: LINT_NAME. <The configuration field doc comment>
(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
Expand All @@ -564,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)]
Expand All @@ -584,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
Expand All @@ -601,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
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/min_rust_version_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -138,6 +143,7 @@ fn main() {
replace_with_default();
map_unwrap_or();
missing_const_for_fn();
unnest_or_patterns();
}

mod meets_msrv {
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/min_rust_version_attr.stderr
Original file line number Diff line number Diff line change
@@ -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, ") {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -17,13 +17,13 @@ LL | assert_eq!(<stripped>.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, ") {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down