OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Implemented option_option lint

Browse Source
This commit is contained in:
Michael Wright 2017-12-26 07:25:13 +02:00
parent f0d0fc69de
commit 6737bae9b1
5 changed files with 163 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
clippy_lints/src/lib.rs
View File

@ -599,6 +599,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
types::IMPLICIT_HASHER,
types::LET_UNIT_VALUE,
types::LINKEDLIST,
types::OPTION_OPTION,
types::TYPE_COMPLEXITY,
types::UNIT_CMP,
types::UNNECESSARY_CAST,

77
clippy_lints/src/types.rs
View File

@ -42,6 +42,26 @@ declare_lint! {
"usage of `Box<Vec<T>>`, vector elements are already on the heap"
}
/// **What it does:** Checks for use of `Option<Option<_>>` in function signatures and type
/// definitions
///
/// **Why is this bad?** `Option<_>` represents an optional value. `Option<Option<_>>`
/// represents an optional optional value which is logically the same thing as an optional
/// value but has an unneeded extra level of wrapping.
///
/// **Known problems:** None.
///
/// **Example**
/// ```rust
/// fn x() -> Option<Option<u32>> {
/// None
/// }
declare_lint! {
pub OPTION_OPTION,
Warn,
"usage of `Option<Option<T>>`"
}
/// **What it does:** Checks for usage of any `LinkedList`, suggesting to use a
/// `Vec` or a `VecDeque` (formerly called `RingBuf`).
///
@ -97,7 +117,7 @@ declare_lint! {
impl LintPass for TypePass {
fn get_lints(&self) -> LintArray {
lint_array!(BOX_VEC, LINKEDLIST, BORROWED_BOX)
lint_array!(BOX_VEC, OPTION_OPTION, LINKEDLIST, BORROWED_BOX)
}
}
@ -142,6 +162,23 @@ fn check_fn_decl(cx: &LateContext, decl: &FnDecl) {
}
}
/// Check if `qpath` has last segment with type parameter matching `path`
fn match_type_parameter(cx: &LateContext, qpath: &QPath, path: &[&str]) -> bool {
let last = last_path_segment(qpath);
if_chain! {
if let Some(ref params) = last.parameters;
if !params.parenthesized;
if let Some(vec) = params.types.get(0);
if let TyPath(ref qpath) = vec.node;
if let Some(did) = opt_def_id(cx.tables.qpath_def(qpath, cx.tcx.hir.node_to_hir_id(vec.id)));
if match_def_path(cx.tcx, did, path);
then {
return true;
}
}
false
}
/// Recursively check for `TypePass` lints in the given type. Stop at the first
/// lint found.
///
@ -157,24 +194,26 @@ fn check_ty(cx: &LateContext, ast_ty: &hir::Ty, is_local: bool) {
let def = cx.tables.qpath_def(qpath, hir_id);
if let Some(def_id) = opt_def_id(def) {
if Some(def_id) == cx.tcx.lang_items().owned_box() {
let last = last_path_segment(qpath);
if_chain! {
if let Some(ref params) = last.parameters;
if !params.parenthesized;
if let Some(vec) = params.types.get(0);
if let TyPath(ref qpath) = vec.node;
if let Some(did) = opt_def_id(cx.tables.qpath_def(qpath, cx.tcx.hir.node_to_hir_id(vec.id)));
if match_def_path(cx.tcx, did, &paths::VEC);
then {
span_help_and_lint(
cx,
BOX_VEC,
ast_ty.span,
"you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`",
"`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.",
);
return; // don't recurse into the type
}
if match_type_parameter(cx, qpath, &paths::VEC) {
span_help_and_lint(
cx,
BOX_VEC,
ast_ty.span,
"you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`",
"`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.",
);
return; // don't recurse into the type
}
} else if match_def_path(cx.tcx, def_id, &paths::OPTION) {
if match_type_parameter(cx, qpath, &paths::OPTION) {
span_help_and_lint(
cx,
OPTION_OPTION,
ast_ty.span,
"consider using `Option<T>` instead of `Option<Option<T>>`",
"`Option<_>` is easier to use than `Option<Option<_>`",
);
return; // don't recurse into the type
}
} else if match_def_path(cx.tcx, def_id, &paths::LINKED_LIST) {
span_help_and_lint(

2
tests/ui/needless_pass_by_value.rs
View File

@ -2,7 +2,7 @@
#![warn(needless_pass_by_value)]
#![allow(dead_code, single_match, if_let_redundant_pattern_matching, many_single_char_names)]
#![allow(dead_code, single_match, if_let_redundant_pattern_matching, many_single_char_names, option_option)]
use std::borrow::Borrow;
use std::convert::AsRef;

46
tests/ui/option_option.rs Normal file
View File

@ -0,0 +1,46 @@
fn input(_: Option<Option<u8>>) {
}
fn output() -> Option<Option<u8>> {
None
}
fn output_nested() -> Vec<Option<Option<u8>>> {
vec![None]
}
// The lint only generates one warning for this
fn output_nested_nested() -> Option<Option<Option<u8>>> {
None
}
struct Struct {
x: Option<Option<u8>>,
}
enum Enum {
Tuple(Option<Option<u8>>),
Struct{x: Option<Option<u8>>},
}
// The lint allows this
type OptionOption = Option<Option<u32>>;
// The lint allows this
fn output_type_alias() -> OptionOption {
None
}
fn main() {
input(None);
output();
output_nested();
// The lint allows this
let local: Option<Option<u8>> = None;
// The lint allows this
let expr = Some(Some(true));
}

57
tests/ui/option_option.stderr Normal file
View File

@ -0,0 +1,57 @@
error: consider using `Option<T>` instead of `Option<Option<T>>`
--> $DIR/option_option.rs:1:13
|
1 | fn input(_: Option<Option<u8>>) {
| ^^^^^^^^^^^^^^^^^^
|
= note: `-D option-option` implied by `-D warnings`
= help: `Option<_>` is easier to use than `Option<Option<_>`
error: consider using `Option<T>` instead of `Option<Option<T>>`
--> $DIR/option_option.rs:4:16
|
4 | fn output() -> Option<Option<u8>> {
| ^^^^^^^^^^^^^^^^^^
|
= help: `Option<_>` is easier to use than `Option<Option<_>`
error: consider using `Option<T>` instead of `Option<Option<T>>`
--> $DIR/option_option.rs:8:27
|
8 | fn output_nested() -> Vec<Option<Option<u8>>> {
| ^^^^^^^^^^^^^^^^^^
|
= help: `Option<_>` is easier to use than `Option<Option<_>`
error: consider using `Option<T>` instead of `Option<Option<T>>`
--> $DIR/option_option.rs:13:30
|
13 | fn output_nested_nested() -> Option<Option<Option<u8>>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: `Option<_>` is easier to use than `Option<Option<_>`
error: consider using `Option<T>` instead of `Option<Option<T>>`
--> $DIR/option_option.rs:18:8
|
18 | x: Option<Option<u8>>,
| ^^^^^^^^^^^^^^^^^^
|
= help: `Option<_>` is easier to use than `Option<Option<_>`
error: consider using `Option<T>` instead of `Option<Option<T>>`
--> $DIR/option_option.rs:22:11
|
22 | Tuple(Option<Option<u8>>),
| ^^^^^^^^^^^^^^^^^^
|
= help: `Option<_>` is easier to use than `Option<Option<_>`
error: consider using `Option<T>` instead of `Option<Option<T>>`
--> $DIR/option_option.rs:23:15
|
23 | Struct{x: Option<Option<u8>>},
| ^^^^^^^^^^^^^^^^^^
|
= help: `Option<_>` is easier to use than `Option<Option<_>`