redundant_pattern_matching: avoid non-const fn calls in const contexts

clippy_lints/src/redundant_pattern_matching.rs

@@ -1,10 +1,13 @@
-use crate::utils::{match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
+use crate::utils::{in_constant, match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
 use if_chain::if_chain;
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
-use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath};
+use rustc_hir::{Arm, Expr, ExprKind, HirId, MatchSource, PatKind, QPath};
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_mir::const_eval::is_const_fn;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::source_map::Symbol;
 
 declare_clippy_lint! {
  /// **What it does:** Lint for redundant pattern matching over `Result` or
@@ -64,26 +67,37 @@ fn find_sugg_for_if_let<'a, 'tcx>(
  arms: &[Arm<'_>],
  keyword: &'static str,
 ) {
+ fn find_suggestion(cx: &LateContext<'_, '_>, hir_id: HirId, path: &QPath<'_>) -> Option<&'static str> {
+ if match_qpath(path, &paths::RESULT_OK) && can_suggest(cx, hir_id, sym!(result_type), "is_ok") {
+ return Some("is_ok()");
+ }
+ if match_qpath(path, &paths::RESULT_ERR) && can_suggest(cx, hir_id, sym!(result_type), "is_err") {
+ return Some("is_err()");
+ }
+ if match_qpath(path, &paths::OPTION_SOME) && can_suggest(cx, hir_id, sym!(option_type), "is_some") {
+ return Some("is_some()");
+ }
+ if match_qpath(path, &paths::OPTION_NONE) && can_suggest(cx, hir_id, sym!(option_type), "is_none") {
+ return Some("is_none()");
+ }
+ None
+ }
+
+ let hir_id = expr.hir_id;
  let good_method = match arms[0].pat.kind {
  PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
  if let PatKind::Wild = patterns[0].kind {
- if match_qpath(path, &paths::RESULT_OK) {
- "is_ok()"
- } else if match_qpath(path, &paths::RESULT_ERR) {
- "is_err()"
- } else if match_qpath(path, &paths::OPTION_SOME) {
- "is_some()"
- } else {
- return;
- }
+ find_suggestion(cx, hir_id, path)
  } else {
- return;
+ None
  }
  },
-
- PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
-
- _ => return,
+ PatKind::Path(ref path) => find_suggestion(cx, hir_id, path),
+ _ => None,
+ };
+ let good_method = match good_method {
+ Some(method) => method,
+ None => return,
  };
 
  // check that `while_let_on_iterator` lint does not trigger
@@ -128,6 +142,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
  if arms.len() == 2 {
  let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
 
+ let hir_id = expr.hir_id;
  let found_good_method = match node_pair {
  (
  PatKind::TupleStruct(ref path_left, ref patterns_left, _),
@@ -142,6 +157,8 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
  &paths::RESULT_ERR,
  "is_ok()",
  "is_err()",
+ || can_suggest(cx, hir_id, sym!(result_type), "is_ok"),
+ || can_suggest(cx, hir_id, sym!(result_type), "is_err"),
  )
  } else {
  None
@@ -160,6 +177,8 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
  &paths::OPTION_NONE,
  "is_some()",
  "is_none()",
+ || can_suggest(cx, hir_id, sym!(option_type), "is_some"),
+ || can_suggest(cx, hir_id, sym!(option_type), "is_none"),
  )
  } else {
  None
@@ -188,6 +207,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
  }
 }
 
+#[allow(clippy::too_many_arguments)]
 fn find_good_method_for_match<'a>(
  arms: &[Arm<'_>],
  path_left: &QPath<'_>,
@@ -196,6 +216,8 @@ fn find_good_method_for_match<'a>(
  expected_right: &[&str],
  should_be_left: &'a str,
  should_be_right: &'a str,
+ can_suggest_left: impl Fn() -> bool,
+ can_suggest_right: impl Fn() -> bool,
 ) -> Option<&'a str> {
  let body_node_pair = if match_qpath(path_left, expected_left) && match_qpath(path_right, expected_right) {
  (&(*arms[0].body).kind, &(*arms[1].body).kind)
@@ -207,10 +229,32 @@ fn find_good_method_for_match<'a>(
 
  match body_node_pair {
  (ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => match (&lit_left.node, &lit_right.node) {
- (LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
- (LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
+ (LitKind::Bool(true), LitKind::Bool(false)) if can_suggest_left() => Some(should_be_left),
+ (LitKind::Bool(false), LitKind::Bool(true)) if can_suggest_right() => Some(should_be_right),
  _ => None,
  },
  _ => None,
  }
 }
+
+fn can_suggest(cx: &LateContext<'_, '_>, hir_id: HirId, diag_item: Symbol, name: &str) -> bool {
+ if !in_constant(cx, hir_id) {
+ return true;
+ }
+
+ // Avoid suggesting calls to non-`const fn`s in const contexts, see #5697.
+ cx.tcx
+ .get_diagnostic_item(diag_item)
+ .and_then(|def_id| {
+ cx.tcx.inherent_impls(def_id).iter().find_map(|imp| {
+ cx.tcx
+ .associated_items(*imp)
+ .in_definition_order()
+ .find_map(|item| match item.kind {
+ ty::AssocKind::Fn if item.ident.name.as_str() == name => Some(item.def_id),
+ _ => None,
+ })
+ })
+ })
+ .map_or(false, |def_id| is_const_fn(cx.tcx, def_id))
+}

tests/ui/redundant_pattern_matching.fixed

@@ -1,5 +1,7 @@
 // run-rustfix
 
+#![feature(const_if_match)]
+#![feature(const_loop)]
 #![warn(clippy::all)]
 #![warn(clippy::redundant_pattern_matching)]
 #![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]
@@ -67,6 +69,7 @@ fn main() {
  takes_bool(x);
 
  issue5504();
+ issue5697();
 
  let _ = if gen_opt().is_some() {
  1
@@ -117,3 +120,42 @@ fn issue5504() {
  if m!().is_some() {}
  while m!().is_some() {}
 }
+
+// None of these should be linted because none of the suggested methods
+// are `const fn` without toggling a feature.
+const fn issue5697() {
+ if let Ok(_) = Ok::<i32, i32>(42) {}
+
+ if let Err(_) = Err::<i32, i32>(42) {}
+
+ if let Some(_) = Some(42) {}
+
+ if let None = None::<()> {}
+
+ while let Ok(_) = Ok::<i32, i32>(10) {}
+
+ while let Err(_) = Ok::<i32, i32>(10) {}
+
+ while let Some(_) = Some(42) {}
+
+ while let None = None::<()> {}
+
+ match Ok::<i32, i32>(42) {
+ Ok(_) => true,
+ Err(_) => false,
+ };
+
+ match Err::<i32, i32>(42) {
+ Ok(_) => false,
+ Err(_) => true,
+ };
+ match Some(42) {
+ Some(_) => true,
+ None => false,
+ };
+
+ match None::<()> {
+ Some(_) => false,
+ None => true,
+ };
+}

tests/ui/redundant_pattern_matching.rs

@@ -1,5 +1,7 @@
 // run-rustfix
 
+#![feature(const_if_match)]
+#![feature(const_loop)]
 #![warn(clippy::all)]
 #![warn(clippy::redundant_pattern_matching)]
 #![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]
@@ -88,6 +90,7 @@ fn main() {
  takes_bool(x);
 
  issue5504();
+ issue5697();
 
  let _ = if let Some(_) = gen_opt() {
  1
@@ -138,3 +141,42 @@ fn issue5504() {
  if let Some(_) = m!() {}
  while let Some(_) = m!() {}
 }
+
+// None of these should be linted because none of the suggested methods
+// are `const fn` without toggling a feature.
+const fn issue5697() {
+ if let Ok(_) = Ok::<i32, i32>(42) {}
+
+ if let Err(_) = Err::<i32, i32>(42) {}
+
+ if let Some(_) = Some(42) {}
+
+ if let None = None::<()> {}
+
+ while let Ok(_) = Ok::<i32, i32>(10) {}
+
+ while let Err(_) = Ok::<i32, i32>(10) {}
+
+ while let Some(_) = Some(42) {}
+
+ while let None = None::<()> {}
+
+ match Ok::<i32, i32>(42) {
+ Ok(_) => true,
+ Err(_) => false,
+ };
+
+ match Err::<i32, i32>(42) {
+ Ok(_) => false,
+ Err(_) => true,
+ };
+ match Some(42) {
+ Some(_) => true,
+ None => false,
+ };
+
+ match None::<()> {
+ Some(_) => false,
+ None => true,
+ };
+}