diff options
| author | Serial <69764315+Serial-ATA@users.noreply.github.com> | 2021-10-11 20:19:34 -0400 |
|---|---|---|
| committer | Serial <69764315+Serial-ATA@users.noreply.github.com> | 2021-10-11 20:19:34 -0400 |
| commit | 9e0ce14700070a5cf61ebdb542cefd70a1dd9b0c (patch) | |
| tree | fc85aa6be548957cd61ea83a0e9802366bc96f6e | |
| parent | 72723663a385fadba0614a2e79be4955e2fffbb5 (diff) | |
| download | rust-9e0ce14700070a5cf61ebdb542cefd70a1dd9b0c.tar.gz rust-9e0ce14700070a5cf61ebdb542cefd70a1dd9b0c.zip | |
Add match_str_case_mismatch lint
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_all.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_correctness.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/match_str_case_mismatch.rs | 166 | ||||
| -rw-r--r-- | tests/ui/match_str_case_mismatch.rs | 98 | ||||
| -rw-r--r-- | tests/ui/match_str_case_mismatch.stderr | 53 |
8 files changed, 323 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 13067e4e92a..1f79d9e73db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2837,6 +2837,7 @@ Released 2018-09-13 [`match_result_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_result_ok [`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms [`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding +[`match_str_case_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_str_case_mismatch [`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm [`match_wildcard_for_single_variants`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wildcard_for_single_variants [`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 3e6e0244754..e544c6607e8 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -118,6 +118,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ 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::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(matches::MATCH_AS_REF), LintId::of(matches::MATCH_LIKE_MATCHES_MACRO), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index e0ef7b3b8af..1c2441d1af2 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -36,6 +36,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve 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(mem_discriminant::MEM_DISCRIMINANT_NON_ENUM), LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT), LintId::of(methods::CLONE_DOUBLE_REF), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 3c4b720671a..d5673ad2c7c 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -226,6 +226,7 @@ store.register_lints(&[ map_unit_fn::RESULT_MAP_UNIT_FN, match_on_vec_items::MATCH_ON_VEC_ITEMS, match_result_ok::MATCH_RESULT_OK, + match_str_case_mismatch::MATCH_STR_CASE_MISMATCH, matches::INFALLIBLE_DESTRUCTURING_MATCH, matches::MATCH_AS_REF, matches::MATCH_BOOL, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 7e1bcbbd0ed..73d21eb5b52 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -265,6 +265,7 @@ mod map_err_ignore; mod map_unit_fn; mod match_on_vec_items; mod match_result_ok; +mod match_str_case_mismatch; mod matches; mod mem_discriminant; mod mem_forget; @@ -771,6 +772,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: let enable_raw_pointer_heuristic_for_send = conf.enable_raw_pointer_heuristic_for_send; store.register_late_pass(move || Box::new(non_send_fields_in_send_ty::NonSendFieldInSendTy::new(enable_raw_pointer_heuristic_for_send))); store.register_late_pass(move || Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks::default())); + store.register_late_pass(|| Box::new(match_str_case_mismatch::MatchStrCaseMismatch)); } #[rustfmt::skip] diff --git a/clippy_lints/src/match_str_case_mismatch.rs b/clippy_lints/src/match_str_case_mismatch.rs new file mode 100644 index 00000000000..dd8b5cb4a21 --- /dev/null +++ b/clippy_lints/src/match_str_case_mismatch.rs @@ -0,0 +1,166 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::ty::is_type_diagnostic_item; +use rustc_ast::ast::LitKind; +use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; +use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::map::Map; +use rustc_middle::lint::in_external_macro; +use rustc_middle::ty; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +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 + /// match &*text.to_ascii_lowercase() { + /// "foo" => {}, + /// "Bar" => {}, + /// _ => {}, + /// } + /// ``` + /// Use instead: + /// ```rust + /// match &*text.to_ascii_lowercase() { + /// "foo" => {}, + /// "bar" => {}, + /// _ => {}, + /// } + /// ``` + 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 LateLintPass<'_> 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) = verify_case(&case_method, arms) { + lint(cx, expr.span, &case_method, bad_case); + } + } + } + } + } +} + +struct MatchExprVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + case_method: Option<CaseMethod>, +} + +impl<'a, 'tcx> Visitor<'tcx> for MatchExprVisitor<'a, 'tcx> { + type Map = Map<'tcx>; + + fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { + NestedVisitorMap::None + } + + 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<CaseMethod> { + 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(case_method: &CaseMethod, arms: &'_ [Arm<'_>]) -> Option<Span> { + let mut bad_case = None; + + let case_check = match case_method { + CaseMethod::LowerCase => |input: &str| -> bool { input.chars().all(char::is_lowercase) }, + CaseMethod::AsciiLowerCase => |input: &str| -> bool { input.chars().all(|c| matches!(c, 'a'..='z')) }, + CaseMethod::UpperCase => |input: &str| -> bool { input.chars().all(char::is_uppercase) }, + CaseMethod::AsciiUppercase => |input: &str| -> bool { input.chars().all(|c| matches!(c, 'A'..='Z')) }, + }; + + 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; + if !case_check(&symbol.as_str()); + then { + bad_case = Some(lit.span); + break; + } + } + } + + bad_case +} + +fn lint(cx: &LateContext<'_>, expr_span: Span, case_method: &CaseMethod, bad_case_span: Span) { + let method_str = match case_method { + CaseMethod::LowerCase => "to_lower_case", + CaseMethod::AsciiLowerCase => "to_ascii_lowercase", + CaseMethod::UpperCase => "to_uppercase", + CaseMethod::AsciiUppercase => "to_ascii_uppercase", + }; + + span_lint_and_help( + cx, + MATCH_STR_CASE_MISMATCH, + expr_span, + "this `match` expression alters case, but has non-compliant arms", + Some(bad_case_span), + &*format!("consider changing the case of this arm to respect `{}`", method_str), + ); +} diff --git a/tests/ui/match_str_case_mismatch.rs b/tests/ui/match_str_case_mismatch.rs new file mode 100644 index 00000000000..208a4bba3d2 --- /dev/null +++ b/tests/ui/match_str_case_mismatch.rs @@ -0,0 +1,98 @@ +#![warn(clippy::match_str_case_mismatch)] + +// Valid + +fn as_str_match() { + let var = "BAR"; + + match var.to_ascii_lowercase().as_str() { + "foo" => {}, + "bar" => {}, + _ => {}, + } +} + +fn addrof_unary_match() { + let var = "BAR"; + + match &*var.to_ascii_lowercase() { + "foo" => {}, + "bar" => {}, + _ => {}, + } +} + +fn alternating_chain() { + let var = "BAR"; + + match &*var + .to_ascii_lowercase() + .to_uppercase() + .to_lowercase() + .to_ascii_uppercase() + { + "FOO" => {}, + "BAR" => {}, + _ => {}, + } +} + +fn unrelated_method() { + struct Item { + a: String, + } + + impl Item { + #[allow(clippy::wrong_self_convention)] + fn to_lowercase(self) -> String { + self.a + } + } + + let item = Item { a: String::from("BAR") }; + + match &*item.to_lowercase() { + "FOO" => {}, + "BAR" => {}, + _ => {}, + } +} + +// Invalid + +fn as_str_match_mismatch() { + let var = "BAR"; + + match var.to_ascii_lowercase().as_str() { + "foo" => {}, + "Bar" => {}, + _ => {}, + } +} + +fn addrof_unary_match_mismatch() { + let var = "BAR"; + + match &*var.to_ascii_lowercase() { + "foo" => {}, + "Bar" => {}, + _ => {}, + } +} + +fn alternating_chain_mismatch() { + let var = "BAR"; + + match &*var + .to_ascii_lowercase() + .to_uppercase() + .to_lowercase() + .to_ascii_uppercase() + { + "FOO" => {}, + "bAR" => {}, + _ => {}, + } +} + +fn main() {} diff --git a/tests/ui/match_str_case_mismatch.stderr b/tests/ui/match_str_case_mismatch.stderr new file mode 100644 index 00000000000..a5eab1f72f3 --- /dev/null +++ b/tests/ui/match_str_case_mismatch.stderr @@ -0,0 +1,53 @@ +error: this `match` expression alters case, but has non-compliant arms + --> $DIR/match_str_case_mismatch.rs:66:5 + | +LL | / match var.to_ascii_lowercase().as_str() { +LL | | "foo" => {}, +LL | | "Bar" => {}, +LL | | _ => {}, +LL | | } + | |_____^ + | + = note: `-D clippy::match-str-case-mismatch` implied by `-D warnings` +help: consider changing the case of this arm to respect `to_ascii_lowercase` + --> $DIR/match_str_case_mismatch.rs:68:9 + | +LL | "Bar" => {}, + | ^^^^^ + +error: this `match` expression alters case, but has non-compliant arms + --> $DIR/match_str_case_mismatch.rs:76:5 + | +LL | / match &*var.to_ascii_lowercase() { +LL | | "foo" => {}, +LL | | "Bar" => {}, +LL | | _ => {}, +LL | | } + | |_____^ + | +help: consider changing the case of this arm to respect `to_ascii_lowercase` + --> $DIR/match_str_case_mismatch.rs:78:9 + | +LL | "Bar" => {}, + | ^^^^^ + +error: this `match` expression alters case, but has non-compliant arms + --> $DIR/match_str_case_mismatch.rs:86:5 + | +LL | / match &*var +LL | | .to_ascii_lowercase() +LL | | .to_uppercase() +LL | | .to_lowercase() +... | +LL | | _ => {}, +LL | | } + | |_____^ + | +help: consider changing the case of this arm to respect `to_ascii_uppercase` + --> $DIR/match_str_case_mismatch.rs:93:9 + | +LL | "bAR" => {}, + | ^^^^^ + +error: aborting due to 3 previous errors + |
