From: Jason Newcomb Date: Fri, 3 Jun 2022 17:16:02 +0000 (-0400) Subject: Move `MatchStrCaseMismatch` into `Matches` lint pass X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=8c8a52eeeb9ca47575200eacd1ab86bf46e8dc15;p=rust.git Move `MatchStrCaseMismatch` into `Matches` lint pass --- diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 46af1d8fb09..15e744229a1 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -140,7 +140,6 @@ LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), LintId::of(match_result_ok::MATCH_RESULT_OK), - LintId::of(match_str_case_mismatch::MATCH_STR_CASE_MISMATCH), LintId::of(matches::COLLAPSIBLE_MATCH), LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(matches::MANUAL_UNWRAP_OR), @@ -149,6 +148,7 @@ LintId::of(matches::MATCH_OVERLAPPING_ARM), LintId::of(matches::MATCH_REF_PATS), LintId::of(matches::MATCH_SINGLE_BINDING), + LintId::of(matches::MATCH_STR_CASE_MISMATCH), LintId::of(matches::NEEDLESS_MATCH), LintId::of(matches::REDUNDANT_PATTERN_MATCHING), LintId::of(matches::SINGLE_MATCH), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index 6bf2c4bbaed..50cdd0af923 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -39,7 +39,7 @@ LintId::of(loops::ITER_NEXT_LOOP), LintId::of(loops::NEVER_LOOP), LintId::of(loops::WHILE_IMMUTABLE_CONDITION), - LintId::of(match_str_case_mismatch::MATCH_STR_CASE_MISMATCH), + LintId::of(matches::MATCH_STR_CASE_MISMATCH), LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT), LintId::of(methods::CLONE_DOUBLE_REF), LintId::of(methods::ITERATOR_STEP_BY_ZERO), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index fb44df57de6..59ba295a887 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -259,7 +259,6 @@ map_unit_fn::OPTION_MAP_UNIT_FN, map_unit_fn::RESULT_MAP_UNIT_FN, match_result_ok::MATCH_RESULT_OK, - match_str_case_mismatch::MATCH_STR_CASE_MISMATCH, matches::COLLAPSIBLE_MATCH, matches::INFALLIBLE_DESTRUCTURING_MATCH, matches::MANUAL_UNWRAP_OR, @@ -271,6 +270,7 @@ matches::MATCH_REF_PATS, matches::MATCH_SAME_ARMS, matches::MATCH_SINGLE_BINDING, + matches::MATCH_STR_CASE_MISMATCH, matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS, matches::MATCH_WILD_ERR_ARM, matches::NEEDLESS_MATCH, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 50b67918fc0..7335b4a35c8 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -287,7 +287,6 @@ macro_rules! declare_clippy_lint { mod map_err_ignore; mod map_unit_fn; mod match_result_ok; -mod match_str_case_mismatch; mod matches; mod mem_forget; mod mem_replace; @@ -875,7 +874,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: )) }); store.register_late_pass(move || Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks)); - store.register_late_pass(|| Box::new(match_str_case_mismatch::MatchStrCaseMismatch)); store.register_late_pass(move || Box::new(format_args::FormatArgs)); store.register_late_pass(|| Box::new(trailing_empty_array::TrailingEmptyArray)); store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes)); diff --git a/clippy_lints/src/match_str_case_mismatch.rs b/clippy_lints/src/match_str_case_mismatch.rs deleted file mode 100644 index d97a878825a..00000000000 --- a/clippy_lints/src/match_str_case_mismatch.rs +++ /dev/null @@ -1,162 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::ty::is_type_diagnostic_item; -use rustc_ast::ast::LitKind; -use rustc_errors::Applicability; -use rustc_hir::intravisit::{walk_expr, Visitor}; -use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::lint::in_external_macro; -use rustc_middle::ty; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::symbol::Symbol; -use rustc_span::{sym, Span}; - -declare_clippy_lint! { - /// ### What it does - /// Checks for `match` expressions modifying the case of a string with non-compliant arms - /// - /// ### Why is this bad? - /// The arm is unreachable, which is likely a mistake - /// - /// ### Example - /// ```rust - /// # let text = "Foo"; - /// match &*text.to_ascii_lowercase() { - /// "foo" => {}, - /// "Bar" => {}, - /// _ => {}, - /// } - /// ``` - /// Use instead: - /// ```rust - /// # let text = "Foo"; - /// match &*text.to_ascii_lowercase() { - /// "foo" => {}, - /// "bar" => {}, - /// _ => {}, - /// } - /// ``` - #[clippy::version = "1.58.0"] - pub MATCH_STR_CASE_MISMATCH, - correctness, - "creation of a case altering match expression with non-compliant arms" -} - -declare_lint_pass!(MatchStrCaseMismatch => [MATCH_STR_CASE_MISMATCH]); - -#[derive(Debug)] -enum CaseMethod { - LowerCase, - AsciiLowerCase, - UpperCase, - AsciiUppercase, -} - -impl<'tcx> LateLintPass<'tcx> for MatchStrCaseMismatch { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if_chain! { - if !in_external_macro(cx.tcx.sess, expr.span); - if let ExprKind::Match(match_expr, arms, MatchSource::Normal) = expr.kind; - if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(match_expr).kind(); - if let ty::Str = ty.kind(); - then { - let mut visitor = MatchExprVisitor { - cx, - case_method: None, - }; - - visitor.visit_expr(match_expr); - - if let Some(case_method) = visitor.case_method { - if let Some((bad_case_span, bad_case_sym)) = verify_case(&case_method, arms) { - lint(cx, &case_method, bad_case_span, bad_case_sym.as_str()); - } - } - } - } - } -} - -struct MatchExprVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - case_method: Option, -} - -impl<'a, 'tcx> Visitor<'tcx> for MatchExprVisitor<'a, 'tcx> { - fn visit_expr(&mut self, ex: &'tcx Expr<'_>) { - match ex.kind { - ExprKind::MethodCall(segment, [receiver], _) if self.case_altered(segment.ident.as_str(), receiver) => {}, - _ => walk_expr(self, ex), - } - } -} - -impl<'a, 'tcx> MatchExprVisitor<'a, 'tcx> { - fn case_altered(&mut self, segment_ident: &str, receiver: &Expr<'_>) -> bool { - if let Some(case_method) = get_case_method(segment_ident) { - let ty = self.cx.typeck_results().expr_ty(receiver).peel_refs(); - - if is_type_diagnostic_item(self.cx, ty, sym::String) || ty.kind() == &ty::Str { - self.case_method = Some(case_method); - return true; - } - } - - false - } -} - -fn get_case_method(segment_ident_str: &str) -> Option { - match segment_ident_str { - "to_lowercase" => Some(CaseMethod::LowerCase), - "to_ascii_lowercase" => Some(CaseMethod::AsciiLowerCase), - "to_uppercase" => Some(CaseMethod::UpperCase), - "to_ascii_uppercase" => Some(CaseMethod::AsciiUppercase), - _ => None, - } -} - -fn verify_case<'a>(case_method: &'a CaseMethod, arms: &'a [Arm<'_>]) -> Option<(Span, Symbol)> { - let case_check = match case_method { - CaseMethod::LowerCase => |input: &str| -> bool { input.chars().all(|c| c.to_lowercase().next() == Some(c)) }, - CaseMethod::AsciiLowerCase => |input: &str| -> bool { !input.chars().any(|c| c.is_ascii_uppercase()) }, - CaseMethod::UpperCase => |input: &str| -> bool { input.chars().all(|c| c.to_uppercase().next() == Some(c)) }, - CaseMethod::AsciiUppercase => |input: &str| -> bool { !input.chars().any(|c| c.is_ascii_lowercase()) }, - }; - - for arm in arms { - if_chain! { - if let PatKind::Lit(Expr { - kind: ExprKind::Lit(lit), - .. - }) = arm.pat.kind; - if let LitKind::Str(symbol, _) = lit.node; - let input = symbol.as_str(); - if !case_check(input); - then { - return Some((lit.span, symbol)); - } - } - } - - None -} - -fn lint(cx: &LateContext<'_>, case_method: &CaseMethod, bad_case_span: Span, bad_case_str: &str) { - let (method_str, suggestion) = match case_method { - CaseMethod::LowerCase => ("to_lowercase", bad_case_str.to_lowercase()), - CaseMethod::AsciiLowerCase => ("to_ascii_lowercase", bad_case_str.to_ascii_lowercase()), - CaseMethod::UpperCase => ("to_uppercase", bad_case_str.to_uppercase()), - CaseMethod::AsciiUppercase => ("to_ascii_uppercase", bad_case_str.to_ascii_uppercase()), - }; - - span_lint_and_sugg( - cx, - MATCH_STR_CASE_MISMATCH, - bad_case_span, - "this `match` arm has a differing case than its expression", - &*format!("consider changing the case of this arm to respect `{}`", method_str), - format!("\"{}\"", suggestion), - Applicability::MachineApplicable, - ); -} diff --git a/clippy_lints/src/matches/match_str_case_mismatch.rs b/clippy_lints/src/matches/match_str_case_mismatch.rs new file mode 100644 index 00000000000..8302ce426e5 --- /dev/null +++ b/clippy_lints/src/matches/match_str_case_mismatch.rs @@ -0,0 +1,125 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::ty::is_type_diagnostic_item; +use rustc_ast::ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::intravisit::{walk_expr, Visitor}; +use rustc_hir::{Arm, Expr, ExprKind, PatKind}; +use rustc_lint::LateContext; +use rustc_middle::ty; +use rustc_span::symbol::Symbol; +use rustc_span::{sym, Span}; + +use super::MATCH_STR_CASE_MISMATCH; + +#[derive(Debug)] +enum CaseMethod { + LowerCase, + AsciiLowerCase, + UpperCase, + AsciiUppercase, +} + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) { + if_chain! { + if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(scrutinee).kind(); + if let ty::Str = ty.kind(); + then { + let mut visitor = MatchExprVisitor { + cx, + case_method: None, + }; + + visitor.visit_expr(scrutinee); + + if let Some(case_method) = visitor.case_method { + if let Some((bad_case_span, bad_case_sym)) = verify_case(&case_method, arms) { + lint(cx, &case_method, bad_case_span, bad_case_sym.as_str()); + } + } + } + } +} + +struct MatchExprVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + case_method: Option, +} + +impl<'a, 'tcx> Visitor<'tcx> for MatchExprVisitor<'a, 'tcx> { + fn visit_expr(&mut self, ex: &'tcx Expr<'_>) { + match ex.kind { + ExprKind::MethodCall(segment, [receiver], _) if self.case_altered(segment.ident.as_str(), receiver) => {}, + _ => walk_expr(self, ex), + } + } +} + +impl<'a, 'tcx> MatchExprVisitor<'a, 'tcx> { + fn case_altered(&mut self, segment_ident: &str, receiver: &Expr<'_>) -> bool { + if let Some(case_method) = get_case_method(segment_ident) { + let ty = self.cx.typeck_results().expr_ty(receiver).peel_refs(); + + if is_type_diagnostic_item(self.cx, ty, sym::String) || ty.kind() == &ty::Str { + self.case_method = Some(case_method); + return true; + } + } + + false + } +} + +fn get_case_method(segment_ident_str: &str) -> Option { + match segment_ident_str { + "to_lowercase" => Some(CaseMethod::LowerCase), + "to_ascii_lowercase" => Some(CaseMethod::AsciiLowerCase), + "to_uppercase" => Some(CaseMethod::UpperCase), + "to_ascii_uppercase" => Some(CaseMethod::AsciiUppercase), + _ => None, + } +} + +fn verify_case<'a>(case_method: &'a CaseMethod, arms: &'a [Arm<'_>]) -> Option<(Span, Symbol)> { + let case_check = match case_method { + CaseMethod::LowerCase => |input: &str| -> bool { input.chars().all(|c| c.to_lowercase().next() == Some(c)) }, + CaseMethod::AsciiLowerCase => |input: &str| -> bool { !input.chars().any(|c| c.is_ascii_uppercase()) }, + CaseMethod::UpperCase => |input: &str| -> bool { input.chars().all(|c| c.to_uppercase().next() == Some(c)) }, + CaseMethod::AsciiUppercase => |input: &str| -> bool { !input.chars().any(|c| c.is_ascii_lowercase()) }, + }; + + for arm in arms { + if_chain! { + if let PatKind::Lit(Expr { + kind: ExprKind::Lit(lit), + .. + }) = arm.pat.kind; + if let LitKind::Str(symbol, _) = lit.node; + let input = symbol.as_str(); + if !case_check(input); + then { + return Some((lit.span, symbol)); + } + } + } + + None +} + +fn lint(cx: &LateContext<'_>, case_method: &CaseMethod, bad_case_span: Span, bad_case_str: &str) { + let (method_str, suggestion) = match case_method { + CaseMethod::LowerCase => ("to_lowercase", bad_case_str.to_lowercase()), + CaseMethod::AsciiLowerCase => ("to_ascii_lowercase", bad_case_str.to_ascii_lowercase()), + CaseMethod::UpperCase => ("to_uppercase", bad_case_str.to_uppercase()), + CaseMethod::AsciiUppercase => ("to_ascii_uppercase", bad_case_str.to_ascii_uppercase()), + }; + + span_lint_and_sugg( + cx, + MATCH_STR_CASE_MISMATCH, + bad_case_span, + "this `match` arm has a differing case than its expression", + &*format!("consider changing the case of this arm to respect `{}`", method_str), + format!("\"{}\"", suggestion), + Applicability::MachineApplicable, + ); +} diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index 7fb449028e7..a58c71ca5f8 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -18,6 +18,7 @@ mod match_ref_pats; mod match_same_arms; mod match_single_binding; +mod match_str_case_mismatch; mod match_wild_enum; mod match_wild_err_arm; mod needless_match; @@ -716,6 +717,37 @@ "matching on vector elements can panic" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `match` expressions modifying the case of a string with non-compliant arms + /// + /// ### Why is this bad? + /// The arm is unreachable, which is likely a mistake + /// + /// ### Example + /// ```rust + /// # let text = "Foo"; + /// match &*text.to_ascii_lowercase() { + /// "foo" => {}, + /// "Bar" => {}, + /// _ => {}, + /// } + /// ``` + /// Use instead: + /// ```rust + /// # let text = "Foo"; + /// match &*text.to_ascii_lowercase() { + /// "foo" => {}, + /// "bar" => {}, + /// _ => {}, + /// } + /// ``` + #[clippy::version = "1.58.0"] + pub MATCH_STR_CASE_MISMATCH, + correctness, + "creation of a case altering match expression with non-compliant arms" +} + #[derive(Default)] pub struct Matches { msrv: Option, @@ -753,6 +785,7 @@ pub fn new(msrv: Option) -> Self { COLLAPSIBLE_MATCH, MANUAL_UNWRAP_OR, MATCH_ON_VEC_ITEMS, + MATCH_STR_CASE_MISMATCH, ]); impl<'tcx> LateLintPass<'tcx> for Matches { @@ -790,6 +823,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { match_as_ref::check(cx, ex, arms, expr); needless_match::check_match(cx, ex, arms, expr); match_on_vec_items::check(cx, ex); + match_str_case_mismatch::check(cx, ex, arms); if !in_constant(cx, expr.hir_id) { manual_unwrap_or::check(cx, expr, ex, arms);