From c5b3a719ed85ae48e813ee07cb9e7b56c6a763d9 Mon Sep 17 00:00:00 2001 From: Yukio Tanaka Date: Tue, 16 Mar 2021 19:46:40 +0900 Subject: [PATCH 1/3] Fix FP of `manual_unwrap_or` in const fn --- clippy_lints/src/manual_unwrap_or.rs | 33 +++++++++++++++++++++++----- tests/ui/manual_unwrap_or.fixed | 15 +++++++++++++ tests/ui/manual_unwrap_or.rs | 15 +++++++++++++ 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/manual_unwrap_or.rs b/clippy_lints/src/manual_unwrap_or.rs index 0e030e0e261..203f018a9e2 100644 --- a/clippy_lints/src/manual_unwrap_or.rs +++ b/clippy_lints/src/manual_unwrap_or.rs @@ -6,12 +6,12 @@ use clippy_utils::ty::is_type_diagnostic_item; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{Arm, Expr, ExprKind, Pat, PatKind}; +use rustc_hir::{hir_id::HirId, intravisit::FnKind, Arm, Body, Expr, ExprKind, FnDecl, Pat, PatKind, StmtKind}; use rustc_lint::LintContext; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; +use rustc_span::{source_map::Span, sym}; declare_clippy_lint! { /// **What it does:** @@ -44,11 +44,34 @@ declare_lint_pass!(ManualUnwrapOr => [MANUAL_UNWRAP_OR]); impl LateLintPass<'_> for ManualUnwrapOr { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if in_external_macro(cx.sess(), expr.span) { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + _: &'tcx FnDecl<'tcx>, + body: &'tcx Body<'tcx>, + span: Span, + _: HirId, + ) { + if in_external_macro(cx.sess(), span) { return; } - lint_manual_unwrap_or(cx, expr); + if_chain! { + if let FnKind::ItemFn(_, _, header, _) = kind; + if !header.is_const(); + let expr = &body.value; + if let ExprKind::Block(block, _) = expr.kind; + then { + for stmt in block.stmts { + if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = &stmt.kind { + lint_manual_unwrap_or(cx, expr); + } + } + if let Some(expr) = block.expr { + lint_manual_unwrap_or(cx, expr); + } + } + } } } diff --git a/tests/ui/manual_unwrap_or.fixed b/tests/ui/manual_unwrap_or.fixed index 81d903c15d3..2f57957f55b 100644 --- a/tests/ui/manual_unwrap_or.fixed +++ b/tests/ui/manual_unwrap_or.fixed @@ -136,4 +136,19 @@ fn result_unwrap_or() { }; } +// don't lint in const fn +const fn const_fn_unwrap_or() { + match Some(1) { + Some(s) => s, + None => 0, + }; +} + +const fn const_fn_unwrap() { + match Ok::<&str, &str>("Alice") { + Ok(s) => s, + Err(_) => "Bob", + }; +} + fn main() {} diff --git a/tests/ui/manual_unwrap_or.rs b/tests/ui/manual_unwrap_or.rs index 16105d379c3..1088047da75 100644 --- a/tests/ui/manual_unwrap_or.rs +++ b/tests/ui/manual_unwrap_or.rs @@ -175,4 +175,19 @@ fn method(self) -> Option { }; } +// don't lint in const fn +const fn const_fn_unwrap_or() { + match Some(1) { + Some(s) => s, + None => 0, + }; +} + +const fn const_fn_unwrap() { + match Ok::<&str, &str>("Alice") { + Ok(s) => s, + Err(_) => "Bob", + }; +} + fn main() {} From aa5f1f907831e0d7833f87063036895a78a0da1a Mon Sep 17 00:00:00 2001 From: Yukio Tanaka Date: Tue, 16 Mar 2021 19:56:47 +0900 Subject: [PATCH 2/3] Fix typo --- tests/ui/manual_unwrap_or.fixed | 4 ++-- tests/ui/manual_unwrap_or.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/ui/manual_unwrap_or.fixed b/tests/ui/manual_unwrap_or.fixed index 2f57957f55b..f1d3252230b 100644 --- a/tests/ui/manual_unwrap_or.fixed +++ b/tests/ui/manual_unwrap_or.fixed @@ -137,14 +137,14 @@ fn result_unwrap_or() { } // don't lint in const fn -const fn const_fn_unwrap_or() { +const fn const_fn_option_unwrap_or() { match Some(1) { Some(s) => s, None => 0, }; } -const fn const_fn_unwrap() { +const fn const_fn_result_unwrap_or() { match Ok::<&str, &str>("Alice") { Ok(s) => s, Err(_) => "Bob", diff --git a/tests/ui/manual_unwrap_or.rs b/tests/ui/manual_unwrap_or.rs index 1088047da75..c9eee25a5b1 100644 --- a/tests/ui/manual_unwrap_or.rs +++ b/tests/ui/manual_unwrap_or.rs @@ -176,14 +176,14 @@ fn method(self) -> Option { } // don't lint in const fn -const fn const_fn_unwrap_or() { +const fn const_fn_option_unwrap_or() { match Some(1) { Some(s) => s, None => 0, }; } -const fn const_fn_unwrap() { +const fn const_fn_result_unwrap_or() { match Ok::<&str, &str>("Alice") { Ok(s) => s, Err(_) => "Bob", From 02ceeb59d4853eecec3a52a7fbd9f6acc1c3f91c Mon Sep 17 00:00:00 2001 From: Yukio Tanaka Date: Wed, 17 Mar 2021 00:06:42 +0900 Subject: [PATCH 3/3] Use in_constant instead of is_const --- clippy_lints/src/manual_unwrap_or.rs | 35 +++++----------------------- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/clippy_lints/src/manual_unwrap_or.rs b/clippy_lints/src/manual_unwrap_or.rs index 203f018a9e2..615e2d5c2af 100644 --- a/clippy_lints/src/manual_unwrap_or.rs +++ b/clippy_lints/src/manual_unwrap_or.rs @@ -1,17 +1,17 @@ use crate::consts::constant_simple; use crate::utils; -use crate::utils::{path_to_local_id, sugg}; +use crate::utils::{in_constant, path_to_local_id, sugg}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt}; use clippy_utils::ty::is_type_diagnostic_item; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{hir_id::HirId, intravisit::FnKind, Arm, Body, Expr, ExprKind, FnDecl, Pat, PatKind, StmtKind}; +use rustc_hir::{Arm, Expr, ExprKind, Pat, PatKind}; use rustc_lint::LintContext; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::{source_map::Span, sym}; +use rustc_span::sym; declare_clippy_lint! { /// **What it does:** @@ -44,34 +44,11 @@ declare_lint_pass!(ManualUnwrapOr => [MANUAL_UNWRAP_OR]); impl LateLintPass<'_> for ManualUnwrapOr { - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - kind: FnKind<'tcx>, - _: &'tcx FnDecl<'tcx>, - body: &'tcx Body<'tcx>, - span: Span, - _: HirId, - ) { - if in_external_macro(cx.sess(), span) { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if in_external_macro(cx.sess(), expr.span) || in_constant(cx, expr.hir_id) { return; } - if_chain! { - if let FnKind::ItemFn(_, _, header, _) = kind; - if !header.is_const(); - let expr = &body.value; - if let ExprKind::Block(block, _) = expr.kind; - then { - for stmt in block.stmts { - if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = &stmt.kind { - lint_manual_unwrap_or(cx, expr); - } - } - if let Some(expr) = block.expr { - lint_manual_unwrap_or(cx, expr); - } - } - } + lint_manual_unwrap_or(cx, expr); } }