Skip to content

Commit 740b72e

Browse files
borsbors
committed
Auto merge of #12849 - AurelienFT:manual-char-comparison-pattern, r=Alexendoo
Add lint to check manual pattern char comparison This PR adds a new lint asked in #12490 This lint catches manual char comparison in pattern of string functions and propose to use `char` or array of `char` instead. As it's my first contribution i'm not feeling very safe about not matching too much or missing some cases. Thanks in advance for taking time to review and propose feedback ! changelog: new lint [`manual_pattern_char_comparison`]
2 parents 9ddea51 + c86b19f commit 740b72e

18 files changed

+451
-163
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5532,6 +5532,7 @@ Released 2018-09-13
55325532
[`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
55335533
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
55345534
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
5535+
[`manual_pattern_char_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison
55355536
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
55365537
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns
55375538
[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid

‎clippy_lints/src/declared_lints.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
448448
crate::methods::SEEK_TO_START_INSTEAD_OF_REWIND_INFO,
449449
crate::methods::SHOULD_IMPLEMENT_TRAIT_INFO,
450450
crate::methods::SINGLE_CHAR_ADD_STR_INFO,
451-
crate::methods::SINGLE_CHAR_PATTERN_INFO,
452451
crate::methods::SKIP_WHILE_NEXT_INFO,
453452
crate::methods::STABLE_SORT_PRIMITIVE_INFO,
454453
crate::methods::STRING_EXTEND_CHARS_INFO,
@@ -656,6 +655,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
656655
crate::std_instead_of_core::ALLOC_INSTEAD_OF_CORE_INFO,
657656
crate::std_instead_of_core::STD_INSTEAD_OF_ALLOC_INFO,
658657
crate::std_instead_of_core::STD_INSTEAD_OF_CORE_INFO,
658+
crate::string_patterns::MANUAL_PATTERN_CHAR_COMPARISON_INFO,
659+
crate::string_patterns::SINGLE_CHAR_PATTERN_INFO,
659660
crate::strings::STRING_ADD_INFO,
660661
crate::strings::STRING_ADD_ASSIGN_INFO,
661662
crate::strings::STRING_FROM_UTF8_AS_BYTES_INFO,

‎clippy_lints/src/float_literal.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ impl<'tcx> LateLintPass<'tcx> for FloatLiteral {
103103
return;
104104
}
105105

106-
if is_whole && !sym_str.contains(|c| c == 'e' || c == 'E') {
106+
if is_whole && !sym_str.contains(['e', 'E']) {
107107
// Normalize the literal by stripping the fractional portion
108108
if sym_str.split('.').next().unwrap() != float_str {
109109
// If the type suffix is missing the suggestion would be

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ mod size_of_in_element_count;
326326
mod size_of_ref;
327327
mod slow_vector_initialization;
328328
mod std_instead_of_core;
329+
mod string_patterns;
329330
mod strings;
330331
mod strlen_on_c_strings;
331332
mod suspicious_operation_groupings;
@@ -1167,6 +1168,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
11671168
..Default::default()
11681169
})
11691170
});
1171+
store.register_late_pass(|_| Box::new(string_patterns::StringPatterns));
11701172
// add lints here, do not remove this comment, it's used in `new_lint`
11711173
}
11721174

‎clippy_lints/src/methods/mod.rs

-35
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ mod seek_from_current;
9494
mod seek_to_start_instead_of_rewind;
9595
mod single_char_add_str;
9696
mod single_char_insert_string;
97-
mod single_char_pattern;
9897
mod single_char_push_string;
9998
mod skip_while_next;
10099
mod stable_sort_primitive;
@@ -1141,38 +1140,6 @@ declare_clippy_lint! {
11411140
"not returning type containing `Self` in a `new` method"
11421141
}
11431142

1144-
declare_clippy_lint! {
1145-
/// ### What it does
1146-
/// Checks for string methods that receive a single-character
1147-
/// `str` as an argument, e.g., `_.split("x")`.
1148-
///
1149-
/// ### Why is this bad?
1150-
/// While this can make a perf difference on some systems,
1151-
/// benchmarks have proven inconclusive. But at least using a
1152-
/// char literal makes it clear that we are looking at a single
1153-
/// character.
1154-
///
1155-
/// ### Known problems
1156-
/// Does not catch multi-byte unicode characters. This is by
1157-
/// design, on many machines, splitting by a non-ascii char is
1158-
/// actually slower. Please do your own measurements instead of
1159-
/// relying solely on the results of this lint.
1160-
///
1161-
/// ### Example
1162-
/// ```rust,ignore
1163-
/// _.split("x");
1164-
/// ```
1165-
///
1166-
/// Use instead:
1167-
/// ```rust,ignore
1168-
/// _.split('x');
1169-
/// ```
1170-
#[clippy::version = "pre 1.29.0"]
1171-
pub SINGLE_CHAR_PATTERN,
1172-
pedantic,
1173-
"using a single-character str where a char could be used, e.g., `_.split(\"x\")`"
1174-
}
1175-
11761143
declare_clippy_lint! {
11771144
/// ### What it does
11781145
/// Checks for calling `.step_by(0)` on iterators which panics.
@@ -4169,7 +4136,6 @@ impl_lint_pass!(Methods => [
41694136
FLAT_MAP_OPTION,
41704137
INEFFICIENT_TO_STRING,
41714138
NEW_RET_NO_SELF,
4172-
SINGLE_CHAR_PATTERN,
41734139
SINGLE_CHAR_ADD_STR,
41744140
SEARCH_IS_SOME,
41754141
FILTER_NEXT,
@@ -4324,7 +4290,6 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
43244290
inefficient_to_string::check(cx, expr, method_call.ident.name, receiver, args);
43254291
single_char_add_str::check(cx, expr, receiver, args);
43264292
into_iter_on_ref::check(cx, expr, method_span, method_call.ident.name, receiver);
4327-
single_char_pattern::check(cx, expr, method_call.ident.name, receiver, args);
43284293
unnecessary_to_owned::check(cx, expr, method_call.ident.name, receiver, args, &self.msrv);
43294294
},
43304295
ExprKind::Binary(op, lhs, rhs) if op.node == hir::BinOpKind::Eq || op.node == hir::BinOpKind::Ne => {

‎clippy_lints/src/methods/path_buf_push_overwrite.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'t
2727
lit.span,
2828
"calling `push` with '/' or '\\' (file system root) will overwrite the previous path definition",
2929
"try",
30-
format!("\"{}\"", pushed_path_lit.trim_start_matches(|c| c == '/' || c == '\\')),
30+
format!("\"{}\"", pushed_path_lit.trim_start_matches(['/', '\\'])),
3131
Applicability::MachineApplicable,
3232
);
3333
}

‎clippy_lints/src/methods/single_char_insert_string.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
use super::utils::get_hint_if_single_char_arg;
21
use clippy_utils::diagnostics::span_lint_and_sugg;
3-
use clippy_utils::source::snippet_with_applicability;
2+
use clippy_utils::source::{snippet_with_applicability, str_literal_to_char_literal};
43
use rustc_ast::BorrowKind;
54
use rustc_errors::Applicability;
65
use rustc_hir::{self as hir, ExprKind};
@@ -11,7 +10,7 @@ use super::SINGLE_CHAR_ADD_STR;
1110
/// lint for length-1 `str`s as argument for `insert_str`
1211
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, receiver: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
1312
let mut applicability = Applicability::MachineApplicable;
14-
if let Some(extension_string) = get_hint_if_single_char_arg(cx, &args[1], &mut applicability, false) {
13+
if let Some(extension_string) = str_literal_to_char_literal(cx, &args[1], &mut applicability, false) {
1514
let base_string_snippet =
1615
snippet_with_applicability(cx, receiver.span.source_callsite(), "_", &mut applicability);
1716
let pos_arg = snippet_with_applicability(cx, args[0].span, "..", &mut applicability);

‎clippy_lints/src/methods/single_char_pattern.rs

-64
This file was deleted.

‎clippy_lints/src/methods/single_char_push_string.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
use super::utils::get_hint_if_single_char_arg;
21
use clippy_utils::diagnostics::span_lint_and_sugg;
3-
use clippy_utils::source::snippet_with_applicability;
2+
use clippy_utils::source::{snippet_with_applicability, str_literal_to_char_literal};
43
use rustc_ast::BorrowKind;
54
use rustc_errors::Applicability;
65
use rustc_hir::{self as hir, ExprKind};
@@ -11,7 +10,7 @@ use super::SINGLE_CHAR_ADD_STR;
1110
/// lint for length-1 `str`s as argument for `push_str`
1211
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, receiver: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
1312
let mut applicability = Applicability::MachineApplicable;
14-
if let Some(extension_string) = get_hint_if_single_char_arg(cx, &args[0], &mut applicability, false) {
13+
if let Some(extension_string) = str_literal_to_char_literal(cx, &args[0], &mut applicability, false) {
1514
let base_string_snippet =
1615
snippet_with_applicability(cx, receiver.span.source_callsite(), "..", &mut applicability);
1716
let sugg = format!("{base_string_snippet}.push({extension_string})");

‎clippy_lints/src/methods/utils.rs

-45
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
1-
use clippy_utils::source::snippet_with_applicability;
21
use clippy_utils::ty::is_type_diagnostic_item;
32
use clippy_utils::{get_parent_expr, path_to_local_id, usage};
4-
use rustc_ast::ast;
5-
use rustc_errors::Applicability;
63
use rustc_hir::intravisit::{walk_expr, Visitor};
74
use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, QPath, Stmt, StmtKind};
85
use rustc_lint::LateContext;
@@ -49,48 +46,6 @@ pub(super) fn derefs_to_slice<'tcx>(
4946
}
5047
}
5148

52-
pub(super) fn get_hint_if_single_char_arg(
53-
cx: &LateContext<'_>,
54-
arg: &Expr<'_>,
55-
applicability: &mut Applicability,
56-
ascii_only: bool,
57-
) -> Option<String> {
58-
if let ExprKind::Lit(lit) = &arg.kind
59-
&& let ast::LitKind::Str(r, style) = lit.node
60-
&& let string = r.as_str()
61-
&& let len = if ascii_only {
62-
string.len()
63-
} else {
64-
string.chars().count()
65-
}
66-
&& len == 1
67-
{
68-
let snip = snippet_with_applicability(cx, arg.span, string, applicability);
69-
let ch = if let ast::StrStyle::Raw(nhash) = style {
70-
let nhash = nhash as usize;
71-
// for raw string: r##"a"##
72-
&snip[(nhash + 2)..(snip.len() - 1 - nhash)]
73-
} else {
74-
// for regular string: "a"
75-
&snip[1..(snip.len() - 1)]
76-
};
77-
78-
let hint = format!(
79-
"'{}'",
80-
match ch {
81-
"'" => "\\'",
82-
r"\" => "\\\\",
83-
"\\\"" => "\"", // no need to escape `"` in `'"'`
84-
_ => ch,
85-
}
86-
);
87-
88-
Some(hint)
89-
} else {
90-
None
91-
}
92-
}
93-
9449
/// The core logic of `check_for_loop_iter` in `unnecessary_iter_cloned.rs`, this function wraps a
9550
/// use of `CloneOrCopyVisitor`.
9651
pub(super) fn clone_or_copy_needed<'tcx>(

‎clippy_lints/src/misc_early/zero_prefixed_literal.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rustc_span::Span;
66
use super::ZERO_PREFIXED_LITERAL;
77

88
pub(super) fn check(cx: &EarlyContext<'_>, lit_span: Span, lit_snip: &str) {
9-
let trimmed_lit_snip = lit_snip.trim_start_matches(|c| c == '_' || c == '0');
9+
let trimmed_lit_snip = lit_snip.trim_start_matches(['_', '0']);
1010
span_lint_and_then(
1111
cx,
1212
ZERO_PREFIXED_LITERAL,
@@ -20,7 +20,7 @@ pub(super) fn check(cx: &EarlyContext<'_>, lit_span: Span, lit_snip: &str) {
2020
Applicability::MaybeIncorrect,
2121
);
2222
// do not advise to use octal form if the literal cannot be expressed in base 8.
23-
if !lit_snip.contains(|c| c == '8' || c == '9') {
23+
if !lit_snip.contains(['8', '9']) {
2424
diag.span_suggestion(
2525
lit_span,
2626
"if you mean to use an octal constant, use `0o`",

0 commit comments

Comments
 (0)