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
This commit is contained in:
bors 2020-04-01 21:30:24 +00:00
commit 42796e11c5
5 changed files with 19 additions and 14 deletions

View file

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

View file

@ -108,7 +108,7 @@ declare_clippy_lint! {
/// }
/// ```
pub OPTION_OPTION,
complexity,
pedantic,
"usage of `Option<Option<T>>`"
}

View file

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

View file

@ -1,3 +1,5 @@
#![deny(clippy::option_option)]
fn input(_: Option<Option<u8>>) {}
fn output() -> Option<Option<u8>> {

View file

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