Skip to content

Commit

Permalink
Add option_and_then_some lint
Browse files Browse the repository at this point in the history
  • Loading branch information
tesuji committed Aug 15, 2019
1 parent d829d9f commit c7e703f
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,7 @@ Released 2018-09-13
[`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
[`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
[`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
[`option_and_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_and_then_some
[`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none
[`option_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
[`option_map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 310 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 311 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
95 changes: 95 additions & 0 deletions clippy_lints/src/and_then_some.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
use crate::utils::paths;
use crate::utils::{in_macro_or_desugar, match_qpath, match_type, remove_blocks, snippet, span_lint_and_sugg};
use rustc::hir;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};
use rustc_errors::Applicability;

declare_clippy_lint! {
/// **What it does:** Checks for usage of `_.and_then(|x| Some(y))`.
///
/// **Why is this bad?** Readability, this can be written more concisely as
/// `_.map(|x| y)`.
///
/// **Known problems:** None
///
/// **Example:**
///
/// ```rust
/// let x = Some("foo");
/// x.and_then(|s| Some(s.len()));
/// ```
///
/// The correct use would be:
///
/// ```rust
/// let x = Some("foo");
/// x.map(|s| s.len());
/// ```
pub OPTION_AND_THEN_SOME,
complexity,
"using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`"
}

declare_lint_pass!(AndThenSomeLint => [OPTION_AND_THEN_SOME]);

const LINT_MSG: &str = "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`";

impl LateLintPass<'_, '_> for AndThenSomeLint {
/// Lint use of `_.and_then(|x| Some(y))` for `Option`s
fn check_expr(&mut self, cx: &LateContext<'_, '_>, e: &hir::Expr) {
if in_macro_or_desugar(e.span) {
return;
}

if let hir::ExprKind::MethodCall(ref method, _, ref args) = e.node {
if args.len() == 2 && method.ident.as_str() == "and_then" {
let ty = cx.tables.expr_ty(&args[0]);
if !match_type(cx, ty, &paths::OPTION) {
return;
}
match args[1].node {
hir::ExprKind::Closure(_, _, body_id, closure_args_span, _) => {
let closure_body = cx.tcx.hir().body(body_id);
let closure_expr = remove_blocks(&closure_body.value);

// let note = format!("{:?}", args[1]);
// span_note_and_lint(cx, OPTION_AND_THEN_SOME, closure_args_span, "Outer debugging
// information", closure_args_span, &note);

if let hir::ExprKind::Call(ref some_expr, ref some_args) = closure_expr.node {
if let hir::ExprKind::Path(ref qpath) = some_expr.node {
if match_qpath(qpath, &paths::OPTION_SOME) {
if some_args.len() == 1 {
let inner_expr = &some_args[0];
let some_inner_snip = snippet(cx, inner_expr.span, "..");
let closure_args_snip = snippet(cx, closure_args_span, "..");
let option_snip = snippet(cx, args[0].span, "..");
let note =
format!("{}.map({} {})", option_snip, closure_args_snip, some_inner_snip);
span_lint_and_sugg(
cx,
OPTION_AND_THEN_SOME,
e.span,
LINT_MSG,
"try this",
note,
Applicability::MachineApplicable,
);
}
}
}
}
},
// hir::ExprKind::Path(ref qpath) => {
// if match_qpath(qpath, &paths::OPTION_SOME) {
// let note = format!("{:?}", args[1]);
// span_note_and_lint(cx, OPTION_AND_THEN_SOME, args[1].span, "Some debugging information",
// args[1].span, &note); }
// },
_ => {},
}
}
}
}
}
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ mod consts;
mod utils;

// begin lints modules, do not remove this comment, it’s used in `update_lints`
pub mod and_then_some;
pub mod approx_const;
pub mod arithmetic;
pub mod assertions_on_constants;
Expand Down Expand Up @@ -596,6 +597,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reg.register_late_lint_pass(box integer_division::IntegerDivision);
reg.register_late_lint_pass(box inherent_to_string::InherentToString);
reg.register_late_lint_pass(box trait_bounds::TraitBounds);
reg.register_late_lint_pass(box and_then_some::AndThenSomeLint);

reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
arithmetic::FLOAT_ARITHMETIC,
Expand Down Expand Up @@ -681,6 +683,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
]);

reg.register_lint_group("clippy::all", Some("clippy"), vec![
and_then_some::OPTION_AND_THEN_SOME,
approx_const::APPROX_CONSTANT,
assertions_on_constants::ASSERTIONS_ON_CONSTANTS,
assign_ops::ASSIGN_OP_PATTERN,
Expand Down Expand Up @@ -1001,6 +1004,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
]);

reg.register_lint_group("clippy::complexity", Some("clippy_complexity"), vec![
and_then_some::OPTION_AND_THEN_SOME,
assign_ops::MISREFACTORED_ASSIGN_OP,
attrs::DEPRECATED_CFG_ATTR,
booleans::NONMINIMAL_BOOL,
Expand Down
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 310] = [
pub const ALL_LINTS: [Lint; 311] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -1316,6 +1316,13 @@ pub const ALL_LINTS: [Lint; 310] = [
deprecation: None,
module: "eq_op",
},
Lint {
name: "option_and_then_some",
group: "complexity",
desc: "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`",
deprecation: None,
module: "and_then_some",
},
Lint {
name: "option_map_or_none",
group: "style",
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/option_and_then_some.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// run-rustfix
#![deny(clippy::option_and_then_some)]

// need a main anyway, use it get rid of unused warnings too
pub fn main() {
let x = Some(5);
// the easiest cases
x.and_then(Some);
x.map(|o| o + 1);
// and an easy counter-example
x.and_then(|o| if o < 32 { Some(o) } else { None });

// Different type
let x: Result<u32, &str> = Ok(1);
x.and_then(Ok);
}
16 changes: 16 additions & 0 deletions tests/ui/option_and_then_some.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// run-rustfix
#![deny(clippy::option_and_then_some)]

// need a main anyway, use it get rid of unused warnings too
pub fn main() {
let x = Some(5);
// the easiest cases
x.and_then(Some);
x.and_then(|o| Some(o + 1));
// and an easy counter-example
x.and_then(|o| if o < 32 { Some(o) } else { None });

// Different type
let x: Result<u32, &str> = Ok(1);
x.and_then(Ok);
}
14 changes: 14 additions & 0 deletions tests/ui/option_and_then_some.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`
--> $DIR/option_and_then_some.rs:9:5
|
LL | x.and_then(|o| Some(o + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `x.map(|o| o + 1)`
|
note: lint level defined here
--> $DIR/option_and_then_some.rs:2:9
|
LL | #![deny(clippy::option_and_then_some)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

0 comments on commit c7e703f

Please sign in to comment.