From 4184cc369a8e08bc74560baf0f47bab14423e26e Mon Sep 17 00:00:00 2001 From: lengyijun Date: Tue, 24 Aug 2021 22:06:46 +0800 Subject: [PATCH 1/2] needless_option_as_deref --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 5 ++ clippy_lints/src/loops/never_loop.rs | 4 +- clippy_lints/src/needless_option_as_deref.rs | 69 ++++++++++++++++++++ tests/ui/needless_option_as_deref.fixed | 13 ++++ tests/ui/needless_option_as_deref.rs | 13 ++++ tests/ui/needless_option_as_deref.stderr | 16 +++++ 7 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 clippy_lints/src/needless_option_as_deref.rs create mode 100644 tests/ui/needless_option_as_deref.fixed create mode 100644 tests/ui/needless_option_as_deref.rs create mode 100644 tests/ui/needless_option_as_deref.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index d86414a87f4..0e131ed02af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2822,6 +2822,7 @@ Released 2018-09-13 [`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main [`needless_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_for_each [`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes +[`needless_option_as_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_as_deref [`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value [`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark [`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2b76eacb7d6..64b7ff199be 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -289,6 +289,7 @@ mod needless_borrow; mod needless_borrowed_ref; mod needless_continue; mod needless_for_each; +mod needless_option_as_deref; mod needless_pass_by_value; mod needless_question_mark; mod needless_update; @@ -847,6 +848,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE, needless_continue::NEEDLESS_CONTINUE, needless_for_each::NEEDLESS_FOR_EACH, + needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF, needless_pass_by_value::NEEDLESS_PASS_BY_VALUE, needless_question_mark::NEEDLESS_QUESTION_MARK, needless_update::NEEDLESS_UPDATE, @@ -1374,6 +1376,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(needless_bool::NEEDLESS_BOOL), LintId::of(needless_borrow::NEEDLESS_BORROW), LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE), + LintId::of(needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF), LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK), LintId::of(needless_update::NEEDLESS_UPDATE), LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD), @@ -1636,6 +1639,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(needless_bool::BOOL_COMPARISON), LintId::of(needless_bool::NEEDLESS_BOOL), LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE), + LintId::of(needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF), LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK), LintId::of(needless_update::NEEDLESS_UPDATE), LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD), @@ -1863,6 +1867,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(ptr::Ptr)); store.register_late_pass(|| Box::new(ptr_eq::PtrEq)); store.register_late_pass(|| Box::new(needless_bool::NeedlessBool)); + store.register_late_pass(|| Box::new(needless_option_as_deref::OptionNeedlessDeref)); store.register_late_pass(|| Box::new(needless_bool::BoolComparison)); store.register_late_pass(|| Box::new(needless_for_each::NeedlessForEach)); store.register_late_pass(|| Box::new(approx_const::ApproxConstant)); diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs index 2c46971d5f7..41956650c9f 100644 --- a/clippy_lints/src/loops/never_loop.rs +++ b/clippy_lints/src/loops/never_loop.rs @@ -87,7 +87,7 @@ fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult { let stmts = block.stmts.iter().map(stmt_to_expr); - let expr = once(block.expr.as_deref()); + let expr = once(block.expr); let mut iter = stmts.chain(expr).flatten(); never_loop_expr_seq(&mut iter, main_loop_id) } @@ -100,7 +100,7 @@ fn never_loop_expr_seq<'a, T: Iterator>>(es: &mut T, main_lo fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<&'tcx Expr<'tcx>> { match stmt.kind { StmtKind::Semi(e, ..) | StmtKind::Expr(e, ..) => Some(e), - StmtKind::Local(local) => local.init.as_deref(), + StmtKind::Local(local) => local.init, StmtKind::Item(..) => None, } } diff --git a/clippy_lints/src/needless_option_as_deref.rs b/clippy_lints/src/needless_option_as_deref.rs new file mode 100644 index 00000000000..67c697da04f --- /dev/null +++ b/clippy_lints/src/needless_option_as_deref.rs @@ -0,0 +1,69 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::in_macro; +use clippy_utils::source::snippet_opt; +use clippy_utils::ty::is_type_diagnostic_item; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::TyS; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for no-op uses of Option::{as_deref,as_deref_mut}, + /// for example, `Option<&T>::as_deref()` returns the same type. + /// + /// ### Why is this bad? + /// Redundant code and improving readability. + /// + /// ### Example + /// ```rust + /// let a = Some(&1); + /// let b = a.as_deref(); // goes from Option<&i32> to Option<&i32> + /// ``` + /// Could be written as: + /// ```rust + /// let a = Some(&1); + /// let b = a; + /// ``` + pub NEEDLESS_OPTION_AS_DEREF, + complexity, + "no-op use of `deref` or `deref_mut` method to `Option`." +} + +declare_lint_pass!(OptionNeedlessDeref=> [ + NEEDLESS_OPTION_AS_DEREF, +]); + +impl<'tcx> LateLintPass<'tcx> for OptionNeedlessDeref { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if expr.span.from_expansion() { + return; + } + if in_macro(expr.span) { + return; + } + let typeck = cx.typeck_results(); + let outer_ty = typeck.expr_ty(expr); + + if_chain! { + if is_type_diagnostic_item(cx,outer_ty,sym::option_type); + if let ExprKind::MethodCall(path, _, [sub_expr], _) = expr.kind; + let symbol = path.ident.as_str(); + if symbol=="as_deref" || symbol=="as_deref_mut"; + if TyS::same_type( outer_ty, typeck.expr_ty(sub_expr) ); + then{ + span_lint_and_sugg( + cx, + NEEDLESS_OPTION_AS_DEREF, + expr.span, + "derefed type is same as origin", + "try this", + snippet_opt(cx,sub_expr.span).unwrap(), + Applicability::MachineApplicable + ); + } + } + } +} diff --git a/tests/ui/needless_option_as_deref.fixed b/tests/ui/needless_option_as_deref.fixed new file mode 100644 index 00000000000..d721452ae88 --- /dev/null +++ b/tests/ui/needless_option_as_deref.fixed @@ -0,0 +1,13 @@ +// run-rustfix + +#[warn(clippy::needless_option_as_deref)] + +fn main() { + // should lint + let _: Option<&usize> = Some(&1); + let _: Option<&mut usize> = Some(&mut 1); + + // should not lint + let _ = Some(Box::new(1)).as_deref(); + let _ = Some(Box::new(1)).as_deref_mut(); +} diff --git a/tests/ui/needless_option_as_deref.rs b/tests/ui/needless_option_as_deref.rs new file mode 100644 index 00000000000..bb15512adf6 --- /dev/null +++ b/tests/ui/needless_option_as_deref.rs @@ -0,0 +1,13 @@ +// run-rustfix + +#[warn(clippy::needless_option_as_deref)] + +fn main() { + // should lint + let _: Option<&usize> = Some(&1).as_deref(); + let _: Option<&mut usize> = Some(&mut 1).as_deref_mut(); + + // should not lint + let _ = Some(Box::new(1)).as_deref(); + let _ = Some(Box::new(1)).as_deref_mut(); +} diff --git a/tests/ui/needless_option_as_deref.stderr b/tests/ui/needless_option_as_deref.stderr new file mode 100644 index 00000000000..5dd507b4a71 --- /dev/null +++ b/tests/ui/needless_option_as_deref.stderr @@ -0,0 +1,16 @@ +error: derefed type is same as origin + --> $DIR/needless_option_as_deref.rs:7:29 + | +LL | let _: Option<&usize> = Some(&1).as_deref(); + | ^^^^^^^^^^^^^^^^^^^ help: try this: `Some(&1)` + | + = note: `-D clippy::needless-option-as-deref` implied by `-D warnings` + +error: derefed type is same as origin + --> $DIR/needless_option_as_deref.rs:8:33 + | +LL | let _: Option<&mut usize> = Some(&mut 1).as_deref_mut(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `Some(&mut 1)` + +error: aborting due to 2 previous errors + From 37af742bbbd20b5fe5a753c0c23e5bab477298f1 Mon Sep 17 00:00:00 2001 From: lyj Date: Sun, 5 Sep 2021 08:33:04 +0800 Subject: [PATCH 2/2] Update clippy_lints/src/needless_option_as_deref.rs Co-authored-by: llogiq --- clippy_lints/src/needless_option_as_deref.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/clippy_lints/src/needless_option_as_deref.rs b/clippy_lints/src/needless_option_as_deref.rs index 67c697da04f..5024a881d2a 100644 --- a/clippy_lints/src/needless_option_as_deref.rs +++ b/clippy_lints/src/needless_option_as_deref.rs @@ -38,10 +38,7 @@ declare_lint_pass!(OptionNeedlessDeref=> [ impl<'tcx> LateLintPass<'tcx> for OptionNeedlessDeref { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if expr.span.from_expansion() { - return; - } - if in_macro(expr.span) { + if expr.span.from_expansion() || in_macro(expr.span) { return; } let typeck = cx.typeck_results();