diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2258ae765928..5a143cca874b 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -489,6 +489,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::STABLE_SORT_PRIMITIVE_INFO, crate::methods::STR_SPLIT_AT_NEWLINE_INFO, crate::methods::STRING_EXTEND_CHARS_INFO, + crate::methods::STRING_FROM_UTF8_AS_BYTES_INFO, crate::methods::STRING_LIT_CHARS_ANY_INFO, crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO, crate::methods::SUSPICIOUS_MAP_INFO, @@ -496,6 +497,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::SUSPICIOUS_SPLITN_INFO, crate::methods::SUSPICIOUS_TO_OWNED_INFO, crate::methods::SWAP_WITH_TEMPORARY_INFO, + crate::methods::TRIM_SPLIT_WHITESPACE_INFO, crate::methods::TYPE_ID_ON_BOX_INFO, crate::methods::UNBUFFERED_BYTES_INFO, crate::methods::UNINIT_ASSUMED_INIT_INFO, @@ -724,10 +726,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::strings::STR_TO_STRING_INFO, crate::strings::STRING_ADD_INFO, crate::strings::STRING_ADD_ASSIGN_INFO, - crate::strings::STRING_FROM_UTF8_AS_BYTES_INFO, crate::strings::STRING_LIT_AS_BYTES_INFO, crate::strings::STRING_SLICE_INFO, - crate::strings::TRIM_SPLIT_WHITESPACE_INFO, crate::strlen_on_c_strings::STRLEN_ON_C_STRINGS_INFO, crate::suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS_INFO, crate::suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f7f534177262..2dc7d4f626a9 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -747,7 +747,6 @@ rustc_lint::late_lint_methods!( UnnecessaryOwnedEmptyStrings: unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings = unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings, FormatPushString: format_push_string::FormatPushString = format_push_string::FormatPushString::new(format_args.clone()), LargeIncludeFile: large_include_file::LargeIncludeFile = large_include_file::LargeIncludeFile::new(conf), - TrimSplitWhitespace: strings::TrimSplitWhitespace = strings::TrimSplitWhitespace, RcCloneInVecInit: rc_clone_in_vec_init::RcCloneInVecInit = rc_clone_in_vec_init::RcCloneInVecInit, SwapPtrToRef: swap_ptr_to_ref::SwapPtrToRef = swap_ptr_to_ref::SwapPtrToRef, TypeParamMismatch: mismatching_type_param_order::TypeParamMismatch = mismatching_type_param_order::TypeParamMismatch, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 84decf478295..5b7d7a949461 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -120,12 +120,14 @@ mod stable_sort_primitive; mod str_split; mod str_splitn; mod string_extend_chars; +mod string_from_utf8_as_bytes; mod string_lit_chars_any; mod suspicious_command_arg_space; mod suspicious_map; mod suspicious_splitn; mod suspicious_to_owned; mod swap_with_temporary; +mod trim_split_white_space; mod type_id_on_box; mod unbuffered_bytes; mod uninit_assumed_init; @@ -3806,6 +3808,28 @@ declare_clippy_lint! { "using `x.extend(s.chars())` where s is a `&str` or `String`" } +declare_clippy_lint! { + /// ### What it does + /// Check if the string is transformed to byte array and casted back to string. + /// + /// ### Why is this bad? + /// It's unnecessary, the string can be used directly. + /// + /// ### Example + /// ```no_run + /// std::str::from_utf8(&"Hello World!".as_bytes()[6..11]).unwrap(); + /// ``` + /// + /// Use instead: + /// ```no_run + /// &"Hello World!"[6..11]; + /// ``` + #[clippy::version = "1.50.0"] + pub STRING_FROM_UTF8_AS_BYTES, + complexity, + "casting string slices to byte slices and back" +} + declare_clippy_lint! { /// ### What it does /// Checks for `.chars().any(|i| i == c)`. @@ -4044,6 +4068,27 @@ declare_clippy_lint! { "detect swap with a temporary value" } +declare_clippy_lint! { + /// ### What it does + /// Warns about calling `str::trim` (or variants) before `str::split_whitespace`. + /// + /// ### Why is this bad? + /// `split_whitespace` already ignores leading and trailing whitespace. + /// + /// ### Example + /// ```no_run + /// " A B C ".trim().split_whitespace(); + /// ``` + /// Use instead: + /// ```no_run + /// " A B C ".split_whitespace(); + /// ``` + #[clippy::version = "1.62.0"] + pub TRIM_SPLIT_WHITESPACE, + style, + "using `str::trim()` or alike before `str::split_whitespace`" +} + declare_clippy_lint! { /// ### What it does /// Looks for calls to `.type_id()` on a `Box`. @@ -5076,6 +5121,7 @@ impl_lint_pass!(Methods => [ SOME_FILTER, STABLE_SORT_PRIMITIVE, STRING_EXTEND_CHARS, + STRING_FROM_UTF8_AS_BYTES, STRING_LIT_CHARS_ANY, STR_SPLIT_AT_NEWLINE, SUSPICIOUS_COMMAND_ARG_SPACE, @@ -5084,6 +5130,7 @@ impl_lint_pass!(Methods => [ SUSPICIOUS_SPLITN, SUSPICIOUS_TO_OWNED, SWAP_WITH_TEMPORARY, + TRIM_SPLIT_WHITESPACE, TYPE_ID_ON_BOX, UNBUFFERED_BYTES, UNINIT_ASSUMED_INIT, @@ -5193,6 +5240,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { swap_with_temporary::check(cx, expr, func, args); ip_constant::check(cx, expr, func, args); clone_on_copy::check_function(cx, expr); + string_from_utf8_as_bytes::check_call(cx, expr, func, args); unwrap_expect_used::check_call( cx, expr, @@ -5997,7 +6045,9 @@ impl Methods { (sym::map_or, [def, map]) => { map_or_identity::check(cx, expr, recv, call_span, def, map); }, - + (sym::split_whitespace, []) => { + trim_split_white_space::check(cx, expr, recv, call_span); + }, (sym::to_string, []) => { inefficient_to_string::check(cx, expr, recv, self.msrv); }, diff --git a/clippy_lints/src/methods/string_from_utf8_as_bytes.rs b/clippy_lints/src/methods/string_from_utf8_as_bytes.rs new file mode 100644 index 000000000000..97506d795cd7 --- /dev/null +++ b/clippy_lints/src/methods/string_from_utf8_as_bytes.rs @@ -0,0 +1,46 @@ +use crate::methods::STRING_FROM_UTF8_AS_BYTES; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::res::{MaybeDef as _, MaybeQPath as _}; +use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; +use clippy_utils::{method_calls, sym}; +use rustc_ast::BorrowKind; +use rustc_errors::Applicability; +use rustc_hir::attrs::lang_items::LangItem; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; + +pub(super) fn check_call(cx: &LateContext<'_>, expr: &Expr<'_>, fun: &Expr<'_>, args: &[Expr<'_>]) { + // Find `std::str::converts::from_utf8` or `std::primitive::str::from_utf8` + + if let [bytes_arg] = args && let Some(sym::str_from_utf8 | sym::str_inherent_from_utf8) = + fun.res(cx).opt_diag_name(cx) + + // Find string::as_bytes + && let ExprKind::AddrOf(BorrowKind::Ref, _, inner) = bytes_arg.kind + && let ExprKind::Index(left, right, _) = inner.kind + && let (method_names, expressions, _) = method_calls(left, 1) + && method_names == [sym::as_bytes] + && expressions.len() == 1 + && expressions[0].1.is_empty() + + // Check for slicer + && let ExprKind::Struct(&qpath, _, _) = right.kind + && cx.tcx.qpath_is_lang_item(qpath, LangItem::Range) + { + let mut applicability = Applicability::MachineApplicable; + let string_expression = &expressions[0].0; + + let snippet_app = snippet_with_applicability(cx, string_expression.span, "..", &mut applicability); + let (right_snip, _) = snippet_with_context(cx, right.span, expr.span.ctxt(), "..", &mut applicability); + + span_lint_and_sugg( + cx, + STRING_FROM_UTF8_AS_BYTES, + expr.span, + "calling a slice of `as_bytes()` with `from_utf8` should be not necessary", + "try", + format!("Some(&{snippet_app}[{right_snip}])"), + applicability, + ); + } +} diff --git a/clippy_lints/src/methods/trim_split_white_space.rs b/clippy_lints/src/methods/trim_split_white_space.rs new file mode 100644 index 000000000000..99b55ca108ee --- /dev/null +++ b/clippy_lints/src/methods/trim_split_white_space.rs @@ -0,0 +1,36 @@ +use crate::methods::TRIM_SPLIT_WHITESPACE; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::sym; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_span::Span; +use rustc_span::def_id::DefId; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, split_recv: &Expr<'_>, split_ws_span: Span) { + let tyckres = cx.typeck_results(); + if let Some(split_ws_def_id) = tyckres.type_dependent_def_id(expr.hir_id) + && cx.tcx.is_diagnostic_item(sym::str_split_whitespace, split_ws_def_id) + && let ExprKind::MethodCall(path, _trim_recv, [], trim_span) = split_recv.kind + && let trim_fn_name @ (sym::trim | sym::trim_start | sym::trim_end) = path.ident.name + && let Some(trim_def_id) = tyckres.type_dependent_def_id(split_recv.hir_id) + && is_one_of_trim_diagnostic_items(cx, trim_def_id) + { + span_lint_and_sugg( + cx, + TRIM_SPLIT_WHITESPACE, + trim_span.with_hi(split_ws_span.lo()), + format!("found call to `str::{trim_fn_name}` before `str::split_whitespace`"), + format!("remove `{trim_fn_name}()`"), + String::new(), + Applicability::MachineApplicable, + ); + } +} + +fn is_one_of_trim_diagnostic_items(cx: &LateContext<'_>, trim_def_id: DefId) -> bool { + matches!( + cx.tcx.get_diagnostic_name(trim_def_id), + Some(sym::str_trim | sym::str_trim_start | sym::str_trim_end) + ) +} diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index d97a1205d437..73ecfa5cde0e 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -1,12 +1,11 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::res::{MaybeDef as _, MaybeQPath as _}; use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_context}; -use clippy_utils::{SpanlessEq, get_expr_use_or_unification_node, get_parent_expr, is_lint_allowed, method_calls, sym}; +use clippy_utils::{SpanlessEq, get_expr_use_or_unification_node, get_parent_expr, is_lint_allowed, sym}; use rustc_errors::Applicability; use rustc_hir::attrs::lang_items::LangItem; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::def_id::DefId; -use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, Node}; +use rustc_hir::{BinOpKind, Expr, ExprKind, Node}; use rustc_lint::{LateContext, LateLintPass, LintContext as _, declare_lint_pass}; use rustc_middle::ty; @@ -91,28 +90,6 @@ declare_clippy_lint! { "using `x = x + ..` where x is a `String` instead of `push_str()`" } -declare_clippy_lint! { - /// ### What it does - /// Check if the string is transformed to byte array and cast back to string. - /// - /// ### Why is this bad? - /// It's unnecessary, the string can be used directly. - /// - /// ### Example - /// ```no_run - /// std::str::from_utf8(&"Hello World!".as_bytes()[6..11]).unwrap(); - /// ``` - /// - /// Use instead: - /// ```no_run - /// &"Hello World!"[6..11]; - /// ``` - #[clippy::version = "1.50.0"] - pub STRING_FROM_UTF8_AS_BYTES, - complexity, - "casting string slices to byte slices and back" -} - declare_clippy_lint! { /// ### What it does /// Checks for the `as_bytes` method called on string literals @@ -183,37 +160,11 @@ declare_clippy_lint! { "slicing a string" } -declare_clippy_lint! { - /// ### What it does - /// Warns about calling `str::trim` (or variants) before `str::split_whitespace`. - /// - /// ### Why is this bad? - /// `split_whitespace` already ignores leading and trailing whitespace. - /// - /// ### Example - /// ```no_run - /// " A B C ".trim().split_whitespace(); - /// ``` - /// Use instead: - /// ```no_run - /// " A B C ".split_whitespace(); - /// ``` - #[clippy::version = "1.62.0"] - pub TRIM_SPLIT_WHITESPACE, - style, - "using `str::trim()` or alike before `str::split_whitespace`" -} - declare_lint_pass!(StrToString => [STR_TO_STRING]); declare_lint_pass!(StringAdd => [STRING_ADD, STRING_ADD_ASSIGN, STRING_SLICE]); -declare_lint_pass!(StringLitAsBytes => [ - STRING_FROM_UTF8_AS_BYTES, - STRING_LIT_AS_BYTES, -]); - -declare_lint_pass!(TrimSplitWhitespace => [TRIM_SPLIT_WHITESPACE]); +declare_lint_pass!(StringLitAsBytes => [STRING_LIT_AS_BYTES]); impl<'tcx> LateLintPass<'tcx> for StringAdd { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { @@ -286,45 +237,11 @@ impl<'tcx> LateLintPass<'tcx> for StringLitAsBytes { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { use rustc_ast::LitKind; - if let ExprKind::Call(fun, [bytes_arg]) = e.kind - // Find `std::str::converts::from_utf8` or `std::primitive::str::from_utf8` - && let Some(sym::str_from_utf8 | sym::str_inherent_from_utf8) = - fun.res(cx).opt_diag_name(cx) - - // Find string::as_bytes - && let ExprKind::AddrOf(BorrowKind::Ref, _, args) = bytes_arg.kind - && let ExprKind::Index(left, right, _) = args.kind - && let (method_names, expressions, _) = method_calls(left, 1) - && method_names == [sym::as_bytes] - && expressions.len() == 1 - && expressions[0].1.is_empty() - - // Check for slicer - && let ExprKind::Struct(&qpath, _, _) = right.kind - && cx.tcx.qpath_is_lang_item(qpath, LangItem::Range) - { - let mut applicability = Applicability::MachineApplicable; - let string_expression = &expressions[0].0; - - let snippet_app = snippet_with_applicability(cx, string_expression.span, "..", &mut applicability); - let (right_snip, _) = snippet_with_context(cx, right.span, e.span.ctxt(), "..", &mut applicability); - - span_lint_and_sugg( - cx, - STRING_FROM_UTF8_AS_BYTES, - e.span, - "calling a slice of `as_bytes()` with `from_utf8` should be not necessary", - "try", - format!("Some(&{snippet_app}[{right_snip}])"), - applicability, - ); - } - - if let ExprKind::MethodCall(path, receiver, ..) = &e.kind + if !e.span.in_external_macro(cx.sess().source_map()) + && let ExprKind::MethodCall(path, receiver, ..) = &e.kind && path.ident.name == sym::as_bytes && let ExprKind::Lit(lit) = &receiver.kind && let LitKind::Str(lit_content, _) = &lit.node - && !e.span.in_external_macro(cx.sess().source_map()) { let callsite = snippet(cx, receiver.span.source_callsite(), r#""foo""#); let mut applicability = Applicability::MachineApplicable; @@ -441,35 +358,3 @@ impl<'tcx> LateLintPass<'tcx> for StrToString { } } } - -impl<'tcx> LateLintPass<'tcx> for TrimSplitWhitespace { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) { - let tyckres = cx.typeck_results(); - if let ExprKind::MethodCall(path, split_recv, [], split_ws_span) = expr.kind - && path.ident.name == sym::split_whitespace - && let Some(split_ws_def_id) = tyckres.type_dependent_def_id(expr.hir_id) - && cx.tcx.is_diagnostic_item(sym::str_split_whitespace, split_ws_def_id) - && let ExprKind::MethodCall(path, _trim_recv, [], trim_span) = split_recv.kind - && let trim_fn_name @ (sym::trim | sym::trim_start | sym::trim_end) = path.ident.name - && let Some(trim_def_id) = tyckres.type_dependent_def_id(split_recv.hir_id) - && is_one_of_trim_diagnostic_items(cx, trim_def_id) - { - span_lint_and_sugg( - cx, - TRIM_SPLIT_WHITESPACE, - trim_span.with_hi(split_ws_span.lo()), - format!("found call to `str::{trim_fn_name}` before `str::split_whitespace`"), - format!("remove `{trim_fn_name}()`"), - String::new(), - Applicability::MachineApplicable, - ); - } - } -} - -fn is_one_of_trim_diagnostic_items(cx: &LateContext<'_>, trim_def_id: DefId) -> bool { - matches!( - cx.tcx.get_diagnostic_name(trim_def_id), - Some(sym::str_trim | sym::str_trim_start | sym::str_trim_end) - ) -}