about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2020-04-01 21:30:24 +0000
committerbors <bors@rust-lang.org>2020-04-01 21:30:24 +0000
commit42796e11c5187be4e2ad962db17f333a52c3a88a (patch)
tree9c90acdedaf26870647399242b2cc26bba147445
parent326b22048a6305d7c918b748be1c081468917ac6 (diff)
parentf6e8da81f184efb4036db16940ef3bfc84a29984 (diff)
downloadrust-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.rs3
-rw-r--r--clippy_lints/src/types.rs2
-rw-r--r--src/lintlist/mod.rs2
-rw-r--r--tests/ui/option_option.rs2
-rw-r--r--tests/ui/option_option.stderr24
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>> },
    |                 ^^^^^^^^^^^^^^^^^^