Skip to content

Commit

Permalink
Add lint to check manual pattern char comparison and merge its code w…
Browse files Browse the repository at this point in the history
…ith single_char_pattern lint
  • Loading branch information
AurelienFT committed Jun 11, 2024
1 parent 9ddea51 commit c86b19f
Show file tree
Hide file tree
Showing 18 changed files with 451 additions and 163 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5532,6 +5532,7 @@ Released 2018-09-13
[`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
[`manual_pattern_char_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns
[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid
Expand Down
3 changes: 2 additions & 1 deletion clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::SEEK_TO_START_INSTEAD_OF_REWIND_INFO,
crate::methods::SHOULD_IMPLEMENT_TRAIT_INFO,
crate::methods::SINGLE_CHAR_ADD_STR_INFO,
crate::methods::SINGLE_CHAR_PATTERN_INFO,
crate::methods::SKIP_WHILE_NEXT_INFO,
crate::methods::STABLE_SORT_PRIMITIVE_INFO,
crate::methods::STRING_EXTEND_CHARS_INFO,
Expand Down Expand Up @@ -656,6 +655,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::std_instead_of_core::ALLOC_INSTEAD_OF_CORE_INFO,
crate::std_instead_of_core::STD_INSTEAD_OF_ALLOC_INFO,
crate::std_instead_of_core::STD_INSTEAD_OF_CORE_INFO,
crate::string_patterns::MANUAL_PATTERN_CHAR_COMPARISON_INFO,
crate::string_patterns::SINGLE_CHAR_PATTERN_INFO,
crate::strings::STRING_ADD_INFO,
crate::strings::STRING_ADD_ASSIGN_INFO,
crate::strings::STRING_FROM_UTF8_AS_BYTES_INFO,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/float_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl<'tcx> LateLintPass<'tcx> for FloatLiteral {
return;
}

if is_whole && !sym_str.contains(|c| c == 'e' || c == 'E') {
if is_whole && !sym_str.contains(['e', 'E']) {
// Normalize the literal by stripping the fractional portion
if sym_str.split('.').next().unwrap() != float_str {
// If the type suffix is missing the suggestion would be
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ mod size_of_in_element_count;
mod size_of_ref;
mod slow_vector_initialization;
mod std_instead_of_core;
mod string_patterns;
mod strings;
mod strlen_on_c_strings;
mod suspicious_operation_groupings;
Expand Down Expand Up @@ -1167,6 +1168,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
..Default::default()
})
});
store.register_late_pass(|_| Box::new(string_patterns::StringPatterns));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
35 changes: 0 additions & 35 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ mod seek_from_current;
mod seek_to_start_instead_of_rewind;
mod single_char_add_str;
mod single_char_insert_string;
mod single_char_pattern;
mod single_char_push_string;
mod skip_while_next;
mod stable_sort_primitive;
Expand Down Expand Up @@ -1141,38 +1140,6 @@ declare_clippy_lint! {
"not returning type containing `Self` in a `new` method"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for string methods that receive a single-character
/// `str` as an argument, e.g., `_.split("x")`.
///
/// ### Why is this bad?
/// While this can make a perf difference on some systems,
/// benchmarks have proven inconclusive. But at least using a
/// char literal makes it clear that we are looking at a single
/// character.
///
/// ### Known problems
/// Does not catch multi-byte unicode characters. This is by
/// design, on many machines, splitting by a non-ascii char is
/// actually slower. Please do your own measurements instead of
/// relying solely on the results of this lint.
///
/// ### Example
/// ```rust,ignore
/// _.split("x");
/// ```
///
/// Use instead:
/// ```rust,ignore
/// _.split('x');
/// ```
#[clippy::version = "pre 1.29.0"]
pub SINGLE_CHAR_PATTERN,
pedantic,
"using a single-character str where a char could be used, e.g., `_.split(\"x\")`"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for calling `.step_by(0)` on iterators which panics.
Expand Down Expand Up @@ -4169,7 +4136,6 @@ impl_lint_pass!(Methods => [
FLAT_MAP_OPTION,
INEFFICIENT_TO_STRING,
NEW_RET_NO_SELF,
SINGLE_CHAR_PATTERN,
SINGLE_CHAR_ADD_STR,
SEARCH_IS_SOME,
FILTER_NEXT,
Expand Down Expand Up @@ -4324,7 +4290,6 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
inefficient_to_string::check(cx, expr, method_call.ident.name, receiver, args);
single_char_add_str::check(cx, expr, receiver, args);
into_iter_on_ref::check(cx, expr, method_span, method_call.ident.name, receiver);
single_char_pattern::check(cx, expr, method_call.ident.name, receiver, args);
unnecessary_to_owned::check(cx, expr, method_call.ident.name, receiver, args, &self.msrv);
},
ExprKind::Binary(op, lhs, rhs) if op.node == hir::BinOpKind::Eq || op.node == hir::BinOpKind::Ne => {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/path_buf_push_overwrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'t
lit.span,
"calling `push` with '/' or '\\' (file system root) will overwrite the previous path definition",
"try",
format!("\"{}\"", pushed_path_lit.trim_start_matches(|c| c == '/' || c == '\\')),
format!("\"{}\"", pushed_path_lit.trim_start_matches(['/', '\\'])),
Applicability::MachineApplicable,
);
}
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/methods/single_char_insert_string.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use super::utils::get_hint_if_single_char_arg;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::source::{snippet_with_applicability, str_literal_to_char_literal};
use rustc_ast::BorrowKind;
use rustc_errors::Applicability;
use rustc_hir::{self as hir, ExprKind};
Expand All @@ -11,7 +10,7 @@ use super::SINGLE_CHAR_ADD_STR;
/// lint for length-1 `str`s as argument for `insert_str`
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, receiver: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
let mut applicability = Applicability::MachineApplicable;
if let Some(extension_string) = get_hint_if_single_char_arg(cx, &args[1], &mut applicability, false) {
if let Some(extension_string) = str_literal_to_char_literal(cx, &args[1], &mut applicability, false) {
let base_string_snippet =
snippet_with_applicability(cx, receiver.span.source_callsite(), "_", &mut applicability);
let pos_arg = snippet_with_applicability(cx, args[0].span, "..", &mut applicability);
Expand Down
64 changes: 0 additions & 64 deletions clippy_lints/src/methods/single_char_pattern.rs

This file was deleted.

5 changes: 2 additions & 3 deletions clippy_lints/src/methods/single_char_push_string.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use super::utils::get_hint_if_single_char_arg;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::source::{snippet_with_applicability, str_literal_to_char_literal};
use rustc_ast::BorrowKind;
use rustc_errors::Applicability;
use rustc_hir::{self as hir, ExprKind};
Expand All @@ -11,7 +10,7 @@ use super::SINGLE_CHAR_ADD_STR;
/// lint for length-1 `str`s as argument for `push_str`
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, receiver: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
let mut applicability = Applicability::MachineApplicable;
if let Some(extension_string) = get_hint_if_single_char_arg(cx, &args[0], &mut applicability, false) {
if let Some(extension_string) = str_literal_to_char_literal(cx, &args[0], &mut applicability, false) {
let base_string_snippet =
snippet_with_applicability(cx, receiver.span.source_callsite(), "..", &mut applicability);
let sugg = format!("{base_string_snippet}.push({extension_string})");
Expand Down
45 changes: 0 additions & 45 deletions clippy_lints/src/methods/utils.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{get_parent_expr, path_to_local_id, usage};
use rustc_ast::ast;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, Visitor};
use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, QPath, Stmt, StmtKind};
use rustc_lint::LateContext;
Expand Down Expand Up @@ -49,48 +46,6 @@ pub(super) fn derefs_to_slice<'tcx>(
}
}

pub(super) fn get_hint_if_single_char_arg(
cx: &LateContext<'_>,
arg: &Expr<'_>,
applicability: &mut Applicability,
ascii_only: bool,
) -> Option<String> {
if let ExprKind::Lit(lit) = &arg.kind
&& let ast::LitKind::Str(r, style) = lit.node
&& let string = r.as_str()
&& let len = if ascii_only {
string.len()
} else {
string.chars().count()
}
&& len == 1
{
let snip = snippet_with_applicability(cx, arg.span, string, applicability);
let ch = if let ast::StrStyle::Raw(nhash) = style {
let nhash = nhash as usize;
// for raw string: r##"a"##
&snip[(nhash + 2)..(snip.len() - 1 - nhash)]
} else {
// for regular string: "a"
&snip[1..(snip.len() - 1)]
};

let hint = format!(
"'{}'",
match ch {
"'" => "\\'",
r"\" => "\\\\",
"\\\"" => "\"", // no need to escape `"` in `'"'`
_ => ch,
}
);

Some(hint)
} else {
None
}
}

/// The core logic of `check_for_loop_iter` in `unnecessary_iter_cloned.rs`, this function wraps a
/// use of `CloneOrCopyVisitor`.
pub(super) fn clone_or_copy_needed<'tcx>(
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/misc_early/zero_prefixed_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_span::Span;
use super::ZERO_PREFIXED_LITERAL;

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

0 comments on commit c86b19f

Please sign in to comment.