diff options
| author | bors <bors@rust-lang.org> | 2020-04-01 21:30:24 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2020-04-01 21:30:24 +0000 |
| commit | 42796e11c5187be4e2ad962db17f333a52c3a88a (patch) | |
| tree | 9c90acdedaf26870647399242b2cc26bba147445 | |
| parent | 326b22048a6305d7c918b748be1c081468917ac6 (diff) | |
| parent | f6e8da81f184efb4036db16940ef3bfc84a29984 (diff) | |
| download | rust-42796e11c5187be4e2ad962db17f333a52c3a88a.tar.gz rust-42796e11c5187be4e2ad962db17f333a52c3a88a.zip | |
Auto merge of #5401 - dtolnay:option, r=Manishearth
Downgrade option_option to pedantic
Based on a search of my work codebase (\>500k lines) for `Option<Option<`, it looks like a bunch of reasonable uses to me. The documented motivation for this lint is:
> an optional optional value is logically the same thing as an optional value but has an unneeded extra level of wrapping
which seems a bit bogus in practice. For example a typical usage would look like:
```rust
let mut host: Option<String> = None;
let mut port: Option<i32> = None;
let mut payload: Option<Option<String>> = None;
for each field {
match field.name {
"host" => host = Some(...),
"port" => port = Some(...),
"payload" => payload = Some(...), // can be null or string
_ => return error,
}
}
let host = host.ok_or(...)?;
let port = port.ok_or(...)?;
let payload = payload.ok_or(...)?;
do_thing(host, port, payload)
```
This lint seems to fit right in with the pedantic group; I don't think linting on occurrences of `Option<Option<T>>` by default is justified.
---
changelog: Remove option_option from default set of enabled lints
| -rw-r--r-- | clippy_lints/src/lib.rs | 3 | ||||
| -rw-r--r-- | clippy_lints/src/types.rs | 2 | ||||
| -rw-r--r-- | src/lintlist/mod.rs | 2 | ||||
| -rw-r--r-- | tests/ui/option_option.rs | 2 | ||||
| -rw-r--r-- | tests/ui/option_option.stderr | 24 |
5 files changed, 19 insertions, 14 deletions
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5d5b5d9a6da..4235cd40a22 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1125,6 +1125,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::CAST_SIGN_LOSS), LintId::of(&types::INVALID_UPCAST_COMPARISONS), LintId::of(&types::LINKEDLIST), + LintId::of(&types::OPTION_OPTION), LintId::of(&unicode::NON_ASCII_LITERAL), LintId::of(&unicode::UNICODE_NOT_NFC), LintId::of(&unused_self::UNUSED_SELF), @@ -1375,7 +1376,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), LintId::of(&types::IMPLICIT_HASHER), LintId::of(&types::LET_UNIT_VALUE), - LintId::of(&types::OPTION_OPTION), LintId::of(&types::TYPE_COMPLEXITY), LintId::of(&types::UNIT_ARG), LintId::of(&types::UNIT_CMP), @@ -1565,7 +1565,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&transmute::TRANSMUTE_PTR_TO_REF), LintId::of(&types::BORROWED_BOX), LintId::of(&types::CHAR_LIT_AS_U8), - LintId::of(&types::OPTION_OPTION), LintId::of(&types::TYPE_COMPLEXITY), LintId::of(&types::UNIT_ARG), LintId::of(&types::UNNECESSARY_CAST), diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 4ff2947378f..7fae477b832 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -108,7 +108,7 @@ declare_clippy_lint! { /// } /// ``` pub OPTION_OPTION, - complexity, + pedantic, "usage of `Option<Option<T>>`" } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 3b89f5d1947..9d135beae4f 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1566,7 +1566,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![ }, Lint { name: "option_option", - group: "complexity", + group: "pedantic", desc: "usage of `Option<Option<T>>`", deprecation: None, module: "types", diff --git a/tests/ui/option_option.rs b/tests/ui/option_option.rs index e2e649a8108..904c50e1403 100644 --- a/tests/ui/option_option.rs +++ b/tests/ui/option_option.rs @@ -1,3 +1,5 @@ +#![deny(clippy::option_option)] + fn input(_: Option<Option<u8>>) {} fn output() -> Option<Option<u8>> { diff --git a/tests/ui/option_option.stderr b/tests/ui/option_option.stderr index 9e9425cf954..79db186d7ea 100644 --- a/tests/ui/option_option.stderr +++ b/tests/ui/option_option.stderr @@ -1,55 +1,59 @@ error: consider using `Option<T>` instead of `Option<Option<T>>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:1:13 + --> $DIR/option_option.rs:3:13 | LL | fn input(_: Option<Option<u8>>) {} | ^^^^^^^^^^^^^^^^^^ | - = note: `-D clippy::option-option` implied by `-D warnings` +note: the lint level is defined here + --> $DIR/option_option.rs:1:9 + | +LL | #![deny(clippy::option_option)] + | ^^^^^^^^^^^^^^^^^^^^^ error: consider using `Option<T>` instead of `Option<Option<T>>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:3:16 + --> $DIR/option_option.rs:5:16 | LL | fn output() -> Option<Option<u8>> { | ^^^^^^^^^^^^^^^^^^ error: consider using `Option<T>` instead of `Option<Option<T>>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:7:27 + --> $DIR/option_option.rs:9:27 | LL | fn output_nested() -> Vec<Option<Option<u8>>> { | ^^^^^^^^^^^^^^^^^^ error: consider using `Option<T>` instead of `Option<Option<T>>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:12:30 + --> $DIR/option_option.rs:14:30 | LL | fn output_nested_nested() -> Option<Option<Option<u8>>> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: consider using `Option<T>` instead of `Option<Option<T>>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:17:8 + --> $DIR/option_option.rs:19:8 | LL | x: Option<Option<u8>>, | ^^^^^^^^^^^^^^^^^^ error: consider using `Option<T>` instead of `Option<Option<T>>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:21:23 + --> $DIR/option_option.rs:23:23 | LL | fn struct_fn() -> Option<Option<u8>> { | ^^^^^^^^^^^^^^^^^^ error: consider using `Option<T>` instead of `Option<Option<T>>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:27:22 + --> $DIR/option_option.rs:29:22 | LL | fn trait_fn() -> Option<Option<u8>>; | ^^^^^^^^^^^^^^^^^^ error: consider using `Option<T>` instead of `Option<Option<T>>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:31:11 + --> $DIR/option_option.rs:33:11 | LL | Tuple(Option<Option<u8>>), | ^^^^^^^^^^^^^^^^^^ error: consider using `Option<T>` instead of `Option<Option<T>>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:32:17 + --> $DIR/option_option.rs:34:17 | LL | Struct { x: Option<Option<u8>> }, | ^^^^^^^^^^^^^^^^^^ |
