diff options
Diffstat (limited to 'clippy_lints')
27 files changed, 543 insertions, 674 deletions
diff --git a/clippy_lints/src/almost_complete_letter_range.rs b/clippy_lints/src/almost_complete_letter_range.rs index 59a7c535400..073e4af1318 100644 --- a/clippy_lints/src/almost_complete_letter_range.rs +++ b/clippy_lints/src/almost_complete_letter_range.rs @@ -4,6 +4,7 @@ use clippy_utils::{meets_msrv, msrvs}; use rustc_ast::ast::{Expr, ExprKind, LitKind, Pat, PatKind, RangeEnd, RangeLimits}; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Span; @@ -79,6 +80,7 @@ fn check_range(cx: &EarlyContext<'_>, span: Span, start: &Expr, end: &Expr, sugg (LitKind::Byte(b'a') | LitKind::Char('a'), LitKind::Byte(b'z') | LitKind::Char('z')) | (LitKind::Byte(b'A') | LitKind::Char('A'), LitKind::Byte(b'Z') | LitKind::Char('Z')) ) + && !in_external_macro(cx.sess(), span) { span_lint_and_then( cx, diff --git a/clippy_lints/src/assertions_on_result_states.rs b/clippy_lints/src/assertions_on_result_states.rs index 7cd198ace86..656dc5feeb5 100644 --- a/clippy_lints/src/assertions_on_result_states.rs +++ b/clippy_lints/src/assertions_on_result_states.rs @@ -1,9 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::macros::{find_assert_args, root_macro_call_first_node, PanicExpn}; -use clippy_utils::path_res; use clippy_utils::source::snippet_with_context; -use clippy_utils::ty::{implements_trait, is_copy, is_type_diagnostic_item}; +use clippy_utils::ty::{has_debug_impl, is_copy, is_type_diagnostic_item}; use clippy_utils::usage::local_used_after_expr; +use clippy_utils::{is_expr_final_block_expr, path_res}; use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::{Expr, ExprKind}; @@ -58,6 +58,7 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates { return; } } + let semicolon = if is_expr_final_block_expr(cx.tcx, e) {";"} else {""}; let mut app = Applicability::MachineApplicable; match method_segment.ident.as_str() { "is_ok" if type_suitable_to_unwrap(cx, substs.type_at(1)) => { @@ -68,8 +69,9 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates { "called `assert!` with `Result::is_ok`", "replace with", format!( - "{}.unwrap()", - snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0 + "{}.unwrap(){}", + snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0, + semicolon ), app, ); @@ -82,8 +84,9 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates { "called `assert!` with `Result::is_err`", "replace with", format!( - "{}.unwrap_err()", - snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0 + "{}.unwrap_err(){}", + snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0, + semicolon ), app, ); @@ -94,13 +97,6 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates { } } -/// This checks whether a given type is known to implement Debug. -fn has_debug_impl<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { - cx.tcx - .get_diagnostic_item(sym::Debug) - .map_or(false, |debug| implements_trait(cx, ty, debug, &[])) -} - fn type_suitable_to_unwrap<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { has_debug_impl(cx, ty) && !ty.is_unit() && !ty.is_never() } diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index a4b8cbb0d82..51e98cda845 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -1,9 +1,9 @@ -use rustc_ast::{ExprPrecedence, LitKind}; +use rustc_ast::LitKind; use rustc_hir::{Block, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, source::snippet_block_with_applicability}; +use clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, sugg::Sugg}; use rustc_errors::Applicability; declare_clippy_lint! { @@ -55,27 +55,42 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx if let ExprKind::If(check, then, Some(else_)) = expr.kind && let Some(then_lit) = int_literal(then) && let Some(else_lit) = int_literal(else_) - && check_int_literal_equals_val(then_lit, 1) - && check_int_literal_equals_val(else_lit, 0) { + let inverted = if + check_int_literal_equals_val(then_lit, 1) + && check_int_literal_equals_val(else_lit, 0) { + false + } else if + check_int_literal_equals_val(then_lit, 0) + && check_int_literal_equals_val(else_lit, 1) { + true + } else { + // Expression isn't boolean, exit + return; + }; let mut applicability = Applicability::MachineApplicable; - let snippet = snippet_block_with_applicability(ctx, check.span, "..", None, &mut applicability); - let snippet_with_braces = { - let need_parens = should_have_parentheses(check); - let (left_paren, right_paren) = if need_parens {("(", ")")} else {("", "")}; - format!("{left_paren}{snippet}{right_paren}") + let snippet = { + let mut sugg = Sugg::hir_with_applicability(ctx, check, "..", &mut applicability); + if inverted { + sugg = !sugg; + } + sugg }; let ty = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type let suggestion = { let wrap_in_curly = is_else_clause(ctx.tcx, expr); - let (left_curly, right_curly) = if wrap_in_curly {("{", "}")} else {("", "")}; - format!( - "{left_curly}{ty}::from({snippet}){right_curly}" - ) + let mut s = Sugg::NonParen(format!("{ty}::from({snippet})").into()); + if wrap_in_curly { + s = s.blockify(); + } + s }; // when used in else clause if statement should be wrapped in curly braces + let into_snippet = snippet.clone().maybe_par(); + let as_snippet = snippet.as_ty(ty); + span_lint_and_then(ctx, BOOL_TO_INT_WITH_IF, expr.span, @@ -87,7 +102,7 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx suggestion, applicability, ); - diag.note(format!("`{snippet_with_braces} as {ty}` or `{snippet_with_braces}.into()` can also be valid options")); + diag.note(format!("`{as_snippet}` or `{into_snippet}.into()` can also be valid options")); }); }; } @@ -119,7 +134,3 @@ fn check_int_literal_equals_val<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>, expecte false } } - -fn should_have_parentheses<'tcx>(check: &'tcx rustc_hir::Expr<'tcx>) -> bool { - check.precedence().order() < ExprPrecedence::Cast.order() -} diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index 656d639f0ef..03d262d5a59 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -237,7 +237,7 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> { } }, &Term(n) => { - let snip = snippet_opt(self.cx, self.terminals[n as usize].span)?; + let snip = snippet_opt(self.cx, self.terminals[n as usize].span.source_callsite())?; self.output.push_str(&snip); }, } diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 45b5d79d445..501f9ef78ae 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -297,13 +297,10 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id) && position.lint_explicit_deref() => { + let ty_changed_count = usize::from(!deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr))); self.state = Some(( State::DerefMethod { - ty_changed_count: if deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr)) { - 0 - } else { - 1 - }, + ty_changed_count, is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), target_mut, }, diff --git a/clippy_lints/src/derivable_impls.rs b/clippy_lints/src/derivable_impls.rs index ef9eeecc6a9..06ae5abeaeb 100644 --- a/clippy_lints/src/derivable_impls.rs +++ b/clippy_lints/src/derivable_impls.rs @@ -1,5 +1,6 @@ -use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::{is_default_equivalent, peel_blocks}; +use rustc_errors::Applicability; use rustc_hir::{ def::{DefKind, Res}, Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind, @@ -100,15 +101,28 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls { ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)), _ => false, }; + if should_emit { - let path_string = cx.tcx.def_path_str(adt_def.did()); - span_lint_and_help( + let struct_span = cx.tcx.def_span(adt_def.did()); + span_lint_and_then( cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", - None, - &format!("try annotating `{}` with `#[derive(Default)]`", path_string), + |diag| { + diag.span_suggestion_hidden( + item.span, + "remove the manual implementation...", + String::new(), + Applicability::MachineApplicable + ); + diag.span_suggestion( + struct_span.shrink_to_lo(), + "...and instead derive it", + "#[derive(Default)]\n".to_string(), + Applicability::MachineApplicable + ); + } ); } } diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 0c5851cdbed..f10d8256953 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -71,12 +71,12 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat { let value = arg.param.value; if_chain! { if format_args.format_string.parts == [kw::Empty]; + if arg.format.is_default(); if match cx.typeck_results().expr_ty(value).peel_refs().kind() { ty::Adt(adt, _) => cx.tcx.is_diagnostic_item(sym::String, adt.did()), ty::Str => true, _ => false, }; - if !arg.format.has_string_formatting(); then { let is_new_string = match value.kind { ExprKind::Binary(..) => true, diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 2a55c48cf77..9e1eaf248b7 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -77,7 +77,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs { if let ExpnKind::Macro(_, name) = outermost_expn_data.kind; then { for arg in &format_args.args { - if arg.format.has_string_formatting() { + if !arg.format.is_default() { continue; } if is_aliased(&format_args, arg.param.value.hir_id) { diff --git a/clippy_lints/src/large_stack_arrays.rs b/clippy_lints/src/large_stack_arrays.rs index 0acbd81aec3..5857d81ab1f 100644 --- a/clippy_lints/src/large_stack_arrays.rs +++ b/clippy_lints/src/large_stack_arrays.rs @@ -1,7 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::source::snippet; -use if_chain::if_chain; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::layout::LayoutOf; use rustc_middle::ty::{self, ConstKind}; @@ -39,29 +38,28 @@ impl_lint_pass!(LargeStackArrays => [LARGE_STACK_ARRAYS]); impl<'tcx> LateLintPass<'tcx> for LargeStackArrays { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - if_chain! { - if let ExprKind::Repeat(_, _) = expr.kind; - if let ty::Array(element_type, cst) = cx.typeck_results().expr_ty(expr).kind(); - if let ConstKind::Value(ty::ValTree::Leaf(element_count)) = cst.kind(); - if let Ok(element_count) = element_count.try_to_machine_usize(cx.tcx); - if let Ok(element_size) = cx.layout_of(*element_type).map(|l| l.size.bytes()); - if self.maximum_allowed_size < element_count * element_size; - then { - span_lint_and_help( - cx, - LARGE_STACK_ARRAYS, - expr.span, - &format!( - "allocating a local array larger than {} bytes", - self.maximum_allowed_size - ), - None, - &format!( - "consider allocating on the heap with `vec!{}.into_boxed_slice()`", - snippet(cx, expr.span, "[...]") - ), - ); - } - } + if let ExprKind::Repeat(_, _) = expr.kind + && let ty::Array(element_type, cst) = cx.typeck_results().expr_ty(expr).kind() + && let ConstKind::Value(ty::ValTree::Leaf(element_count)) = cst.kind() + && let Ok(element_count) = element_count.try_to_machine_usize(cx.tcx) + && let Ok(element_size) = cx.layout_of(*element_type).map(|l| l.size.bytes()) + && !cx.tcx.hir().parent_iter(expr.hir_id) + .any(|(_, node)| matches!(node, Node::Item(Item { kind: ItemKind::Static(..), .. }))) + && self.maximum_allowed_size < element_count * element_size { + span_lint_and_help( + cx, + LARGE_STACK_ARRAYS, + expr.span, + &format!( + "allocating a local array larger than {} bytes", + self.maximum_allowed_size + ), + None, + &format!( + "consider allocating on the heap with `vec!{}.into_boxed_slice()`", + snippet(cx, expr.span, "[...]") + ), + ); + } } } diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 59d1760fe60..8718d5fa1da 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -171,6 +171,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(methods::ITERATOR_STEP_BY_ZERO), LintId::of(methods::ITER_CLONED_COLLECT), LintId::of(methods::ITER_COUNT), + LintId::of(methods::ITER_KV_MAP), LintId::of(methods::ITER_NEXT_SLICE), LintId::of(methods::ITER_NTH), LintId::of(methods::ITER_NTH_ZERO), @@ -351,7 +352,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(useless_conversion::USELESS_CONVERSION), LintId::of(vec::USELESS_VEC), LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH), - LintId::of(write::POSITIONAL_NAMED_FORMAT_PARAMETERS), LintId::of(write::PRINTLN_EMPTY_STRING), LintId::of(write::PRINT_LITERAL), LintId::of(write::PRINT_WITH_NEWLINE), diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs index aa247352f88..185189a6af5 100644 --- a/clippy_lints/src/lib.register_complexity.rs +++ b/clippy_lints/src/lib.register_complexity.rs @@ -40,6 +40,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(methods::GET_LAST_WITH_LEN), LintId::of(methods::INSPECT_FOR_EACH), LintId::of(methods::ITER_COUNT), + LintId::of(methods::ITER_KV_MAP), LintId::of(methods::MANUAL_FILTER_MAP), LintId::of(methods::MANUAL_FIND_MAP), LintId::of(methods::MANUAL_SPLIT_ONCE), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 962e6722006..02fcc8de507 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -313,6 +313,7 @@ store.register_lints(&[ methods::ITERATOR_STEP_BY_ZERO, methods::ITER_CLONED_COLLECT, methods::ITER_COUNT, + methods::ITER_KV_MAP, methods::ITER_NEXT_SLICE, methods::ITER_NTH, methods::ITER_NTH_ZERO, @@ -595,7 +596,6 @@ store.register_lints(&[ vec_init_then_push::VEC_INIT_THEN_PUSH, wildcard_imports::ENUM_GLOB_USE, wildcard_imports::WILDCARD_IMPORTS, - write::POSITIONAL_NAMED_FORMAT_PARAMETERS, write::PRINTLN_EMPTY_STRING, write::PRINT_LITERAL, write::PRINT_STDERR, diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index bede91f183e..6125d0f7a86 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/clippy_lints/src/lib.register_suspicious.rs @@ -35,5 +35,4 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec! LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL), LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL), LintId::of(swap_ptr_to_ref::SWAP_PTR_TO_REF), - LintId::of(write::POSITIONAL_NAMED_FORMAT_PARAMETERS), ]) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index ceaaf5c6d6e..298566cb5b6 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -40,7 +40,6 @@ extern crate rustc_lint; extern crate rustc_middle; extern crate rustc_mir_dataflow; extern crate rustc_parse; -extern crate rustc_parse_format; extern crate rustc_session; extern crate rustc_span; extern crate rustc_target; @@ -425,7 +424,6 @@ pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, sess: &Se }) }); - store.register_pre_expansion_pass(|| Box::new(write::Write::default())); store.register_pre_expansion_pass(move || Box::new(attrs::EarlyAttributes { msrv })); } @@ -524,7 +522,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: #[cfg(feature = "internal")] { if std::env::var("ENABLE_METADATA_COLLECTION").eq(&Ok("1".to_string())) { - store.register_late_pass(|| Box::new(utils::internal_lints::metadata_collector::MetadataCollector::new())); + store.register_late_pass(|_| Box::new(utils::internal_lints::metadata_collector::MetadataCollector::new())); return; } } @@ -879,6 +877,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ignore_publish: cargo_ignore_publish, }) }); + store.register_late_pass(|_| Box::new(write::Write::default())); store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef)); store.register_early_pass(|| Box::new(empty_structs_with_brackets::EmptyStructsWithBrackets)); store.register_late_pass(|_| Box::new(unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings)); diff --git a/clippy_lints/src/methods/err_expect.rs b/clippy_lints/src/methods/err_expect.rs index 570a1b87358..720d9a68c85 100644 --- a/clippy_lints/src/methods/err_expect.rs +++ b/clippy_lints/src/methods/err_expect.rs @@ -1,6 +1,6 @@ use super::ERR_EXPECT; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::ty::implements_trait; +use clippy_utils::ty::has_debug_impl; use clippy_utils::{meets_msrv, msrvs, ty::is_type_diagnostic_item}; use rustc_errors::Applicability; use rustc_lint::LateContext; @@ -28,7 +28,7 @@ pub(super) fn check( // Tests if the T type in a `Result<T, E>` is not None if let Some(data_type) = get_data_type(cx, result_type); // Tests if the T type in a `Result<T, E>` implements debug - if has_debug_impl(data_type, cx); + if has_debug_impl(cx, data_type); then { span_lint_and_sugg( @@ -51,10 +51,3 @@ fn get_data_type<'a>(cx: &LateContext<'_>, ty: Ty<'a>) -> Option<Ty<'a>> { _ => None, } } - -/// Given a type, very if the Debug trait has been impl'd -fn has_debug_impl<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool { - cx.tcx - .get_diagnostic_item(sym::Debug) - .map_or(false, |debug| implements_trait(cx, ty, debug, &[])) -} diff --git a/clippy_lints/src/methods/iter_kv_map.rs b/clippy_lints/src/methods/iter_kv_map.rs new file mode 100644 index 00000000000..a7eecabd684 --- /dev/null +++ b/clippy_lints/src/methods/iter_kv_map.rs @@ -0,0 +1,87 @@ +#![allow(unused_imports)] + +use super::ITER_KV_MAP; +use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::source::{snippet, snippet_with_applicability}; +use clippy_utils::sugg; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::visitors::is_local_used; +use rustc_hir::{BindingAnnotation, Body, BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::ty; +use rustc_span::sym; +use rustc_span::Span; + +/// lint use of: +/// - `hashmap.iter().map(|(_, v)| v)` +/// - `hashmap.into_iter().map(|(_, v)| v)` +/// on `HashMaps` and `BTreeMaps` in std + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + map_type: &'tcx str, // iter / into_iter + expr: &'tcx Expr<'tcx>, // .iter().map(|(_, v_| v)) + recv: &'tcx Expr<'tcx>, // hashmap + m_arg: &'tcx Expr<'tcx>, // |(_, v)| v +) { + if_chain! { + if !expr.span.from_expansion(); + if let ExprKind::Closure(c) = m_arg.kind; + if let Body {params: [p], value: body_expr, generator_kind: _ } = cx.tcx.hir().body(c.body); + if let PatKind::Tuple([key_pat, val_pat], _) = p.pat.kind; + + let (replacement_kind, binded_ident) = match (&key_pat.kind, &val_pat.kind) { + (key, PatKind::Binding(_, _, value, _)) if pat_is_wild(cx, key, m_arg) => ("value", value), + (PatKind::Binding(_, _, key, _), value) if pat_is_wild(cx, value, m_arg) => ("key", key), + _ => return, + }; + + let ty = cx.typeck_results().expr_ty(recv); + if is_type_diagnostic_item(cx, ty, sym::HashMap) || is_type_diagnostic_item(cx, ty, sym::BTreeMap); + + then { + let mut applicability = rustc_errors::Applicability::MachineApplicable; + let recv_snippet = snippet_with_applicability(cx, recv.span, "map", &mut applicability); + let into_prefix = if map_type == "into_iter" {"into_"} else {""}; + + if_chain! { + if let ExprKind::Path(rustc_hir::QPath::Resolved(_, path)) = body_expr.kind; + if let [local_ident] = path.segments; + if local_ident.ident.as_str() == binded_ident.as_str(); + + then { + span_lint_and_sugg( + cx, + ITER_KV_MAP, + expr.span, + &format!("iterating on a map's {}s", replacement_kind), + "try", + format!("{}.{}{}s()", recv_snippet, into_prefix, replacement_kind), + applicability, + ); + } else { + span_lint_and_sugg( + cx, + ITER_KV_MAP, + expr.span, + &format!("iterating on a map's {}s", replacement_kind), + "try", + format!("{}.{}{}s().map(|{}| {})", recv_snippet, into_prefix, replacement_kind, binded_ident, + snippet_with_applicability(cx, body_expr.span, "/* body */", &mut applicability)), + applicability, + ); + } + } + } + } +} + +/// Returns `true` if the pattern is a `PatWild`, or is an ident prefixed with `_` +/// that is not locally used. +fn pat_is_wild<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool { + match *pat { + PatKind::Wild => true, + PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => !is_local_used(cx, body, id), + _ => false, + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 41942b20ea1..cdde4c54d63 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -35,6 +35,7 @@ mod into_iter_on_ref; mod is_digit_ascii_radix; mod iter_cloned_collect; mod iter_count; +mod iter_kv_map; mod iter_next_slice; mod iter_nth; mod iter_nth_zero; @@ -3036,6 +3037,37 @@ declare_clippy_lint! { "use of `File::read_to_end` or `File::read_to_string`" } +declare_clippy_lint! { + /// ### What it does + /// + /// Checks for iterating a map (`HashMap` or `BTreeMap`) and + /// ignoring either the keys or values. + /// + /// ### Why is this bad? + /// + /// Readability. There are `keys` and `values` methods that + /// can be used to express that we only need the keys or the values. + /// + /// ### Example + /// + /// ``` + /// # use std::collections::HashMap; + /// let map: HashMap<u32, u32> = HashMap::new(); + /// let values = map.iter().map(|(_, value)| value).collect::<Vec<_>>(); + /// ``` + /// + /// Use instead: + /// ``` + /// # use std::collections::HashMap; + /// let map: HashMap<u32, u32> = HashMap::new(); + /// let values = map.values().collect::<Vec<_>>(); + /// ``` + #[clippy::version = "1.65.0"] + pub ITER_KV_MAP, + complexity, + "iterating on map using `iter` when `keys` or `values` would do" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option<RustcVersion>, @@ -3159,6 +3191,7 @@ impl_lint_pass!(Methods => [ UNNECESSARY_SORT_BY, VEC_RESIZE_TO_ZERO, VERBOSE_FILE_READS, + ITER_KV_MAP, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -3498,6 +3531,9 @@ impl Methods { (name @ ("map" | "map_err"), [m_arg]) => { if name == "map" { map_clone::check(cx, expr, recv, m_arg, self.msrv); + if let Some((map_name @ ("iter" | "into_iter"), recv2, _, _)) = method_call(recv) { + iter_kv_map::check(cx, map_name, expr, recv2, m_arg); + } } else { map_err_ignore::check(cx, expr, m_arg); } diff --git a/clippy_lints/src/methods/ok_expect.rs b/clippy_lints/src/methods/ok_expect.rs index d64a9f320d9..646fc4a7bcf 100644 --- a/clippy_lints/src/methods/ok_expect.rs +++ b/clippy_lints/src/methods/ok_expect.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; +use clippy_utils::ty::{has_debug_impl, is_type_diagnostic_item}; use if_chain::if_chain; use rustc_hir as hir; use rustc_lint::LateContext; @@ -15,7 +15,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result); let result_type = cx.typeck_results().expr_ty(recv); if let Some(error_type) = get_error_type(cx, result_type); - if has_debug_impl(error_type, cx); + if has_debug_impl(cx, error_type); then { span_lint_and_help( @@ -37,10 +37,3 @@ fn get_error_type<'a>(cx: &LateContext<'_>, ty: Ty<'a>) -> Option<Ty<'a>> { _ => None, } } - -/// This checks whether a given type is known to implement Debug. -fn has_debug_impl<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool { - cx.tcx - .get_diagnostic_item(sym::Debug) - .map_or(false, |debug| implements_trait(cx, ty, debug, &[])) -} diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs index 8d3cede70f7..79d784c342c 100644 --- a/clippy_lints/src/methods/unnecessary_to_owned.rs +++ b/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -7,7 +7,7 @@ use clippy_utils::visitors::find_all_ret_expressions; use clippy_utils::{fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, return_ty}; use clippy_utils::{meets_msrv, msrvs}; use rustc_errors::Applicability; -use rustc_hir::{def_id::DefId, BorrowKind, Expr, ExprKind, ItemKind, Node}; +use rustc_hir::{def_id::DefId, BorrowKind, Expr, ExprKind, ItemKind, LangItem, Node}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; use rustc_middle::mir::Mutability; @@ -268,7 +268,7 @@ fn check_other_call_arg<'tcx>( // We can't add an `&` when the trait is `Deref` because `Target = &T` won't match // `Target = T`. if n_refs > 0 || is_copy(cx, receiver_ty) || trait_predicate.def_id() != deref_trait_id; - let n_refs = max(n_refs, if is_copy(cx, receiver_ty) { 0 } else { 1 }); + let n_refs = max(n_refs, usize::from(!is_copy(cx, receiver_ty))); if let Some(receiver_snippet) = snippet_opt(cx, receiver.span); then { span_lint_and_sugg( @@ -379,6 +379,10 @@ fn can_change_type<'a>(cx: &LateContext<'a>, mut expr: &'a Expr<'a>, mut ty: Ty< Node::Expr(parent_expr) => { if let Some((callee_def_id, call_substs, recv, call_args)) = get_callee_substs_and_args(cx, parent_expr) { + if cx.tcx.lang_items().require(LangItem::IntoFutureIntoFuture) == Ok(callee_def_id) { + return false; + } + let fn_sig = cx.tcx.fn_sig(callee_def_id).skip_binder(); if let Some(arg_index) = recv.into_iter().chain(call_args).position(|arg| arg.hir_id == expr.hir_id) && let Some(param_ty) = fn_sig.inputs().get(arg_index) diff --git a/clippy_lints/src/module_style.rs b/clippy_lints/src/module_style.rs index 0a393657267..22071ab3044 100644 --- a/clippy_lints/src/module_style.rs +++ b/clippy_lints/src/module_style.rs @@ -2,7 +2,7 @@ use rustc_ast::ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_lint::{EarlyContext, EarlyLintPass, Level, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::{FileName, RealFileName, SourceFile, Span, SyntaxContext}; +use rustc_span::{FileName, SourceFile, Span, SyntaxContext}; use std::ffi::OsStr; use std::path::{Component, Path}; @@ -79,7 +79,7 @@ impl EarlyLintPass for ModStyle { let files = cx.sess().source_map().files(); - let RealFileName::LocalPath(trim_to_src) = &cx.sess().opts.working_dir else { return }; + let Some(trim_to_src) = cx.sess().opts.working_dir.local_path() else { return }; // `folder_segments` is all unique folder path segments `path/to/foo.rs` gives // `[path, to]` but not foo @@ -90,7 +90,7 @@ impl EarlyLintPass for ModStyle { // `{ foo => path/to/foo.rs, .. } let mut file_map = FxHashMap::default(); for file in files.iter() { - if let FileName::Real(RealFileName::LocalPath(lp)) = &file.name { + if let FileName::Real(name) = &file.name && let Some(lp) = name.local_path() { let path = if lp.is_relative() { lp } else if let Ok(relative) = lp.strip_prefix(trim_to_src) { diff --git a/clippy_lints/src/nonstandard_macro_braces.rs b/clippy_lints/src/nonstandard_macro_braces.rs index 4722c031006..f86dfb6b8df 100644 --- a/clippy_lints/src/nonstandard_macro_braces.rs +++ b/clippy_lints/src/nonstandard_macro_braces.rs @@ -184,6 +184,10 @@ fn macro_braces(conf: FxHashSet<MacroMatcher>) -> FxHashMap<String, (String, Str name: "vec", braces: ("[", "]"), ), + macro_matcher!( + name: "matches", + braces: ("(", ")"), + ), ] .into_iter() .collect::<FxHashMap<_, _>>(); diff --git a/clippy_lints/src/operators/arithmetic_side_effects.rs b/clippy_lints/src/operators/arithmetic_side_effects.rs index 83b69fbb311..d24c57c0a4b 100644 --- a/clippy_lints/src/operators/arithmetic_side_effects.rs +++ b/clippy_lints/src/operators/arithmetic_side_effects.rs @@ -42,27 +42,30 @@ impl ArithmeticSideEffects { } } - /// Checks assign operators (+=, -=, *=, /=) of integers in a non-constant environment that - /// won't overflow. - fn has_valid_assign_op(op: &Spanned<hir::BinOpKind>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool { - if !Self::is_literal_integer(rhs, rhs_refs) { - return false; + /// Assuming that `expr` is a literal integer, checks operators (+=, -=, *, /) in a + /// non-constant environment that won't overflow. + fn has_valid_op(op: &Spanned<hir::BinOpKind>, expr: &hir::Expr<'_>) -> bool { + if let hir::BinOpKind::Add | hir::BinOpKind::Sub = op.node + && let hir::ExprKind::Lit(ref lit) = expr.kind + && let ast::LitKind::Int(0, _) = lit.node + { + return true; + } + if let hir::BinOpKind::Div | hir::BinOpKind::Rem = op.node + && let hir::ExprKind::Lit(ref lit) = expr.kind + && !matches!(lit.node, ast::LitKind::Int(0, _)) + { + return true; } - if let hir::BinOpKind::Div | hir::BinOpKind::Mul = op.node - && let hir::ExprKind::Lit(ref lit) = rhs.kind - && let ast::LitKind::Int(1, _) = lit.node + if let hir::BinOpKind::Mul = op.node + && let hir::ExprKind::Lit(ref lit) = expr.kind + && let ast::LitKind::Int(0 | 1, _) = lit.node { return true; } false } - /// Checks "raw" binary operators (+, -, *, /) of integers in a non-constant environment - /// already handled by the CTFE. - fn has_valid_bin_op(lhs: &hir::Expr<'_>, lhs_refs: Ty<'_>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool { - Self::is_literal_integer(lhs, lhs_refs) && Self::is_literal_integer(rhs, rhs_refs) - } - /// Checks if the given `expr` has any of the inner `allowed` elements. fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { self.allowed.contains( @@ -83,7 +86,8 @@ impl ArithmeticSideEffects { } fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) { - span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, "arithmetic detected"); + let msg = "arithmetic operation that can potentially result in unexpected side-effects"; + span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, msg); self.expr_span = Some(expr.span); } @@ -115,13 +119,18 @@ impl ArithmeticSideEffects { if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) { return; } - let lhs_refs = cx.typeck_results().expr_ty(lhs).peel_refs(); - let rhs_refs = cx.typeck_results().expr_ty(rhs).peel_refs(); - let has_valid_assign_op = Self::has_valid_assign_op(op, rhs, rhs_refs); - if has_valid_assign_op || Self::has_valid_bin_op(lhs, lhs_refs, rhs, rhs_refs) { - return; + let has_valid_op = match ( + Self::is_literal_integer(lhs, cx.typeck_results().expr_ty(lhs).peel_refs()), + Self::is_literal_integer(rhs, cx.typeck_results().expr_ty(rhs).peel_refs()), + ) { + (true, true) => true, + (true, false) => Self::has_valid_op(op, lhs), + (false, true) => Self::has_valid_op(op, rhs), + (false, false) => false, + }; + if !has_valid_op { + self.issue_lint(cx, expr); } - self.issue_lint(cx, expr); } } diff --git a/clippy_lints/src/renamed_lints.rs b/clippy_lints/src/renamed_lints.rs index 6bea6dc0773..d320eea1c37 100644 --- a/clippy_lints/src/renamed_lints.rs +++ b/clippy_lints/src/renamed_lints.rs @@ -36,6 +36,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[ ("clippy::invalid_ref", "invalid_value"), ("clippy::mem_discriminant_non_enum", "enum_intrinsics_non_enums"), ("clippy::panic_params", "non_fmt_panics"), + ("clippy::positional_named_format_parameters", "named_arguments_used_positionally"), ("clippy::temporary_cstring_as_ptr", "temporary_cstring_as_ptr"), ("clippy::unknown_clippy_lints", "unknown_lints"), ("clippy::unused_label", "unused_labels"), diff --git a/clippy_lints/src/unused_peekable.rs b/clippy_lints/src/unused_peekable.rs index cc865643594..f1cebf0f992 100644 --- a/clippy_lints/src/unused_peekable.rs +++ b/clippy_lints/src/unused_peekable.rs @@ -6,6 +6,7 @@ use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir::lang_items::LangItem; use rustc_hir::{Block, Expr, ExprKind, HirId, Local, Node, PatKind, PathSegment, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::nested_filter::OnlyBodies; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -109,8 +110,14 @@ impl<'a, 'tcx> PeekableVisitor<'a, 'tcx> { } } -impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> { - fn visit_expr(&mut self, ex: &'_ Expr<'_>) { +impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> { + type NestedFilter = OnlyBodies; + + fn nested_visit_map(&mut self) -> Self::Map { + self.cx.tcx.hir() + } + + fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { if self.found_peek_call { return; } @@ -136,12 +143,11 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> { return; } - if args.iter().any(|arg| { - matches!(arg.kind, ExprKind::Path(_)) && arg_is_mut_peekable(self.cx, arg) - }) { + if args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg)) { self.found_peek_call = true; - return; } + + return; }, // Catch anything taking a Peekable mutably ExprKind::MethodCall( @@ -190,21 +196,21 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> { Node::Local(Local { init: Some(init), .. }) => { if arg_is_mut_peekable(self.cx, init) { self.found_peek_call = true; - return; } - break; + return; }, - Node::Stmt(stmt) => match stmt.kind { - StmtKind::Expr(_) | StmtKind::Semi(_) => {}, - _ => { - self.found_peek_call = true; - return; - }, + Node::Stmt(stmt) => { + match stmt.kind { + StmtKind::Local(_) | StmtKind::Item(_) => self.found_peek_call = true, + StmtKind::Expr(_) | StmtKind::Semi(_) => {}, + } + + return; }, Node::Block(_) | Node::ExprField(_) => {}, _ => { - break; + return; }, } } diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 486ea5e5ccf..44ab9bca795 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::ty::same_type_and_consts; -use clippy_utils::{meets_msrv, msrvs}; +use clippy_utils::{is_from_proc_macro, meets_msrv, msrvs}; use if_chain::if_chain; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; @@ -87,7 +87,7 @@ impl_lint_pass!(UseSelf => [USE_SELF]); const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element"; impl<'tcx> LateLintPass<'tcx> for UseSelf { - fn check_item(&mut self, _cx: &LateContext<'_>, item: &Item<'_>) { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &Item<'tcx>) { if matches!(item.kind, ItemKind::OpaqueTy(_)) { // skip over `ItemKind::OpaqueTy` in order to lint `foo() -> impl <..>` return; @@ -103,6 +103,7 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { if parameters.as_ref().map_or(true, |params| { !params.parenthesized && !params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_))) }); + if !is_from_proc_macro(cx, item); // expensive, should be last check then { StackItem::Check { impl_id: item.def_id, @@ -213,9 +214,6 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { hir_ty_to_ty(cx.tcx, hir_ty) }; if same_type_and_consts(ty, cx.tcx.type_of(impl_id)); - let hir = cx.tcx.hir(); - // prevents false positive on `#[derive(serde::Deserialize)]` - if !hir.span(hir.get_parent_node(hir_ty.hir_id)).in_derive_expansion(); then { span_lint(cx, hir_ty.span); } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index a8500beb257..2be3fa99c81 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -476,7 +476,7 @@ pub fn format_error(error: Box<dyn Error>) -> String { let mut msg = String::from(prefix); for row in 0..rows { - write!(msg, "\n").unwrap(); + writeln!(msg).unwrap(); for (column, column_width) in column_widths.iter().copied().enumerate() { let index = column * rows + row; let field = fields.get(index).copied().unwrap_or_default(); diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 640a09a7a91..06e7d701701 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -1,20 +1,12 @@ -use std::borrow::Cow; -use std::iter; -use std::ops::{Deref, Range}; - -use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then}; -use clippy_utils::source::{snippet, snippet_opt, snippet_with_applicability}; -use rustc_ast::ast::{Expr, ExprKind, Impl, Item, ItemKind, MacCall, Path, StrLit, StrStyle}; -use rustc_ast::ptr::P; -use rustc_ast::token::{self, LitKind}; -use rustc_ast::tokenstream::TokenStream; -use rustc_errors::{Applicability, DiagnosticBuilder}; -use rustc_lexer::unescape::{self, EscapeError}; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; -use rustc_parse::parser; +use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; +use clippy_utils::macros::{root_macro_call_first_node, FormatArgsExpn, MacroCall}; +use clippy_utils::source::snippet_opt; +use rustc_ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, HirIdMap, Impl, Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::symbol::{kw, Symbol}; -use rustc_span::{sym, BytePos, InnerSpan, Span, DUMMY_SP}; +use rustc_span::{sym, BytePos, Span}; declare_clippy_lint! { /// ### What it does @@ -74,13 +66,7 @@ declare_clippy_lint! { /// application and might forget to remove those prints afterward. /// /// ### Known problems - /// * Only catches `print!` and `println!` calls. - /// * The lint level is unaffected by crate attributes. The level can still - /// be set for functions, modules and other items. To change the level for - /// the entire crate, please use command line flags. More information and a - /// configuration example can be found in [clippy#6610]. - /// - /// [clippy#6610]: https://github.com/rust-lang/rust-clippy/issues/6610#issuecomment-977120558 + /// Only catches `print!` and `println!` calls. /// /// ### Example /// ```rust @@ -102,13 +88,7 @@ declare_clippy_lint! { /// application and might forget to remove those prints afterward. /// /// ### Known problems - /// * Only catches `eprint!` and `eprintln!` calls. - /// * The lint level is unaffected by crate attributes. The level can still - /// be set for functions, modules and other items. To change the level for - /// the entire crate, please use command line flags. More information and a - /// configuration example can be found in [clippy#6610]. - /// - /// [clippy#6610]: https://github.com/rust-lang/rust-clippy/issues/6610#issuecomment-977120558 + /// Only catches `eprint!` and `eprintln!` calls. /// /// ### Example /// ```rust @@ -149,10 +129,6 @@ declare_clippy_lint! { /// (c.f., https://github.com/matthiaskrgr/rust-str-bench) and unnecessary /// (i.e., just put the literal in the format string) /// - /// ### Known problems - /// Will also warn with macro calls as arguments that expand to literals - /// -- e.g., `println!("{}", env!("FOO"))`. - /// /// ### Example /// ```rust /// println!("{}", "foo"); @@ -234,10 +210,6 @@ declare_clippy_lint! { /// (c.f., https://github.com/matthiaskrgr/rust-str-bench) and unnecessary /// (i.e., just put the literal in the format string) /// - /// ### Known problems - /// Will also warn with macro calls as arguments that expand to literals - /// -- e.g., `writeln!(buf, "{}", env!("FOO"))`. - /// /// ### Example /// ```rust /// # use std::fmt::Write; @@ -257,28 +229,6 @@ declare_clippy_lint! { "writing a literal with a format string" } -declare_clippy_lint! { - /// ### What it does - /// This lint warns when a named parameter in a format string is used as a positional one. - /// - /// ### Why is this bad? - /// It may be confused for an assignment and obfuscates which parameter is being used. - /// - /// ### Example - /// ```rust - /// println!("{}", x = 10); - /// ``` - /// - /// Use instead: - /// ```rust - /// println!("{x}", x = 10); - /// ``` - #[clippy::version = "1.63.0"] - pub POSITIONAL_NAMED_FORMAT_PARAMETERS, - suspicious, - "named parameter in a format string is used positionally" -} - #[derive(Default)] pub struct Write { in_debug_impl: bool, @@ -294,537 +244,308 @@ impl_lint_pass!(Write => [ WRITE_WITH_NEWLINE, WRITELN_EMPTY_STRING, WRITE_LITERAL, - POSITIONAL_NAMED_FORMAT_PARAMETERS, ]); -impl EarlyLintPass for Write { - fn check_item(&mut self, _: &EarlyContext<'_>, item: &Item) { - if let ItemKind::Impl(box Impl { - of_trait: Some(trait_ref), - .. - }) = &item.kind - { - let trait_name = trait_ref - .path - .segments - .iter() - .last() - .expect("path has at least one segment") - .ident - .name; - if trait_name == sym::Debug { - self.in_debug_impl = true; - } +impl<'tcx> LateLintPass<'tcx> for Write { + fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if is_debug_impl(cx, item) { + self.in_debug_impl = true; } } - fn check_item_post(&mut self, _: &EarlyContext<'_>, _: &Item) { - self.in_debug_impl = false; + fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if is_debug_impl(cx, item) { + self.in_debug_impl = false; + } } - fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &MacCall) { - fn is_build_script(cx: &EarlyContext<'_>) -> bool { - // Cargo sets the crate name for build scripts to `build_script_build` - cx.sess() - .opts - .crate_name - .as_ref() - .map_or(false, |crate_name| crate_name == "build_script_build") - } + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return }; + let Some(diag_name) = cx.tcx.get_diagnostic_name(macro_call.def_id) else { return }; + let Some(name) = diag_name.as_str().strip_suffix("_macro") else { return }; - if mac.path == sym!(print) { - if !is_build_script(cx) { - span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`"); - } - self.lint_print_with_newline(cx, mac); - } else if mac.path == sym!(println) { - if !is_build_script(cx) { - span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`"); - } - self.lint_println_empty_string(cx, mac); - } else if mac.path == sym!(eprint) { - span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprint!`"); - self.lint_print_with_newline(cx, mac); - } else if mac.path == sym!(eprintln) { - span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprintln!`"); - self.lint_println_empty_string(cx, mac); - } else if mac.path == sym!(write) { - if let (Some(fmt_str), dest) = self.check_tts(cx, mac.args.inner_tokens(), true) { - if check_newlines(&fmt_str) { - let (nl_span, only_nl) = newline_span(&fmt_str); - let nl_span = match (dest, only_nl) { - // Special case of `write!(buf, "\n")`: Mark everything from the end of - // `buf` for removal so no trailing comma [`writeln!(buf, )`] remains. - (Some(dest_expr), true) => nl_span.with_lo(dest_expr.span.hi()), - _ => nl_span, - }; - span_lint_and_then( - cx, - WRITE_WITH_NEWLINE, - mac.span(), - "using `write!()` with a format string that ends in a single newline", - |err| { - err.multipart_suggestion( - "use `writeln!()` instead", - vec![(mac.path.span, String::from("writeln")), (nl_span, String::new())], - Applicability::MachineApplicable, - ); - }, - ); - } - } - } else if mac.path == sym!(writeln) { - if let (Some(fmt_str), expr) = self.check_tts(cx, mac.args.inner_tokens(), true) { - if fmt_str.symbol == kw::Empty { - let mut applicability = Applicability::MachineApplicable; - let suggestion = if let Some(e) = expr { - snippet_with_applicability(cx, e.span, "v", &mut applicability) - } else { - applicability = Applicability::HasPlaceholders; - Cow::Borrowed("v") - }; - - span_lint_and_sugg( - cx, - WRITELN_EMPTY_STRING, - mac.span(), - format!("using `writeln!({}, \"\")`", suggestion).as_str(), - "replace it with", - format!("writeln!({})", suggestion), - applicability, - ); + let is_build_script = cx + .sess() + .opts + .crate_name + .as_ref() + .map_or(false, |crate_name| crate_name == "build_script_build"); + + match diag_name { + sym::print_macro | sym::println_macro => { + if !is_build_script { + span_lint(cx, PRINT_STDOUT, macro_call.span, &format!("use of `{name}!`")); } - } + }, + sym::eprint_macro | sym::eprintln_macro => { + span_lint(cx, PRINT_STDERR, macro_call.span, &format!("use of `{name}!`")); + }, + sym::write_macro | sym::writeln_macro => {}, + _ => return, } - } -} -/// Given a format string that ends in a newline and its span, calculates the span of the -/// newline, or the format string itself if the format string consists solely of a newline. -/// Return this and a boolean indicating whether it only consisted of a newline. -fn newline_span(fmtstr: &StrLit) -> (Span, bool) { - let sp = fmtstr.span; - let contents = fmtstr.symbol.as_str(); + let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, macro_call.expn) else { return }; - if contents == r"\n" { - return (sp, true); - } - - let newline_sp_hi = sp.hi() - - match fmtstr.style { - StrStyle::Cooked => BytePos(1), - StrStyle::Raw(hashes) => BytePos((1 + hashes).into()), - }; + // ignore `writeln!(w)` and `write!(v, some_macro!())` + if format_args.format_string.span.from_expansion() { + return; + } - let newline_sp_len = if contents.ends_with('\n') { - BytePos(1) - } else if contents.ends_with(r"\n") { - BytePos(2) - } else { - panic!("expected format string to contain a newline"); - }; + match diag_name { + sym::print_macro | sym::eprint_macro | sym::write_macro => { + check_newline(cx, &format_args, ¯o_call, name); + }, + sym::println_macro | sym::eprintln_macro | sym::writeln_macro => { + check_empty_string(cx, &format_args, ¯o_call, name); + }, + _ => {}, + } - (sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi), false) -} + check_literal(cx, &format_args, name); -/// Stores a list of replacement spans for each argument, but only if all the replacements used an -/// empty format string. -#[derive(Default)] -struct SimpleFormatArgs { - unnamed: Vec<Vec<Span>>, - complex_unnamed: Vec<Vec<Span>>, - named: Vec<(Symbol, Vec<Span>)>, + if !self.in_debug_impl { + for arg in &format_args.args { + if arg.format.r#trait == sym::Debug { + span_lint(cx, USE_DEBUG, arg.span, "use of `Debug`-based formatting"); + } + } + } + } } -impl SimpleFormatArgs { - fn get_unnamed(&self) -> impl Iterator<Item = &[Span]> { - self.unnamed.iter().map(|x| match x.as_slice() { - // Ignore the dummy span added from out of order format arguments. - [DUMMY_SP] => &[], - x => x, - }) +fn is_debug_impl(cx: &LateContext<'_>, item: &Item<'_>) -> bool { + if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), .. }) = &item.kind + && let Some(trait_id) = trait_ref.trait_def_id() + { + cx.tcx.is_diagnostic_item(sym::Debug, trait_id) + } else { + false } +} - fn get_complex_unnamed(&self) -> impl Iterator<Item = &[Span]> { - self.complex_unnamed.iter().map(Vec::as_slice) - } +fn check_newline(cx: &LateContext<'_>, format_args: &FormatArgsExpn<'_>, macro_call: &MacroCall, name: &str) { + let format_string_parts = &format_args.format_string.parts; + let mut format_string_span = format_args.format_string.span; - fn get_named(&self, n: &Path) -> &[Span] { - self.named.iter().find(|x| *n == x.0).map_or(&[], |x| x.1.as_slice()) - } + let Some(last) = format_string_parts.last() else { return }; - fn push(&mut self, arg: rustc_parse_format::Argument<'_>, span: Span) { - use rustc_parse_format::{ - AlignUnknown, ArgumentImplicitlyIs, ArgumentIs, ArgumentNamed, CountImplied, FormatSpec, - }; + let count_vertical_whitespace = || { + format_string_parts + .iter() + .flat_map(|part| part.as_str().chars()) + .filter(|ch| matches!(ch, '\r' | '\n')) + .count() + }; - const SIMPLE: FormatSpec<'_> = FormatSpec { - fill: None, - align: AlignUnknown, - flags: 0, - precision: CountImplied, - precision_span: None, - width: CountImplied, - width_span: None, - ty: "", - ty_span: None, - }; + if last.as_str().ends_with('\n') + // ignore format strings with other internal vertical whitespace + && count_vertical_whitespace() == 1 - match arg.position { - ArgumentIs(n) | ArgumentImplicitlyIs(n) => { - if self.unnamed.len() <= n { - // Use a dummy span to mark all unseen arguments. - self.unnamed.resize_with(n, || vec![DUMMY_SP]); - if arg.format == SIMPLE { - self.unnamed.push(vec![span]); - } else { - self.unnamed.push(Vec::new()); - } - } else { - let args = &mut self.unnamed[n]; - match (args.as_mut_slice(), arg.format == SIMPLE) { - // A non-empty format string has been seen already. - ([], _) => (), - // Replace the dummy span, if it exists. - ([dummy @ DUMMY_SP], true) => *dummy = span, - ([_, ..], true) => args.push(span), - ([_, ..], false) => *args = Vec::new(), - } - } - }, - ArgumentNamed(n) => { - let n = Symbol::intern(n); - if let Some(x) = self.named.iter_mut().find(|x| x.0 == n) { - match x.1.as_slice() { - // A non-empty format string has been seen already. - [] => (), - [_, ..] if arg.format == SIMPLE => x.1.push(span), - [_, ..] => x.1 = Vec::new(), - } - } else if arg.format == SIMPLE { - self.named.push((n, vec![span])); - } else { - self.named.push((n, Vec::new())); - } - }, - }; - } + // ignore trailing arguments: `print!("Issue\n{}", 1265);` + && format_string_parts.len() > format_args.args.len() + { + let lint = if name == "write" { + format_string_span = expand_past_previous_comma(cx, format_string_span); - fn push_to_complex(&mut self, span: Span, position: usize) { - if self.complex_unnamed.len() <= position { - self.complex_unnamed.resize_with(position, Vec::new); - self.complex_unnamed.push(vec![span]); + WRITE_WITH_NEWLINE } else { - let args: &mut Vec<Span> = &mut self.complex_unnamed[position]; - args.push(span); - } - } - - fn push_complex( - &mut self, - cx: &EarlyContext<'_>, - arg: rustc_parse_format::Argument<'_>, - str_lit_span: Span, - fmt_span: Span, - ) { - use rustc_parse_format::{ArgumentImplicitlyIs, ArgumentIs, CountIsParam, CountIsStar}; - - let snippet = snippet_opt(cx, fmt_span); - - let end = snippet - .as_ref() - .and_then(|s| s.find(':')) - .or_else(|| fmt_span.hi().0.checked_sub(fmt_span.lo().0 + 1).map(|u| u as usize)); - - if let (ArgumentIs(n) | ArgumentImplicitlyIs(n), Some(end)) = (arg.position, end) { - let span = fmt_span.from_inner(InnerSpan::new(1, end)); - self.push_to_complex(span, n); + PRINT_WITH_NEWLINE }; - if let (CountIsParam(n) | CountIsStar(n), Some(span)) = (arg.format.precision, arg.format.precision_span) { - // We need to do this hack as precision spans should be converted from .* to .foo$ - let hack = if snippet.as_ref().and_then(|s| s.find('*')).is_some() { - 0 - } else { - 1 - }; + span_lint_and_then( + cx, + lint, + macro_call.span, + &format!("using `{name}!()` with a format string that ends in a single newline"), + |diag| { + let name_span = cx.sess().source_map().span_until_char(macro_call.span, '!'); + let Some(format_snippet) = snippet_opt(cx, format_string_span) else { return }; + + if format_string_parts.len() == 1 && last.as_str() == "\n" { + // print!("\n"), write!(f, "\n") + + diag.multipart_suggestion( + &format!("use `{name}ln!` instead"), + vec![(name_span, format!("{name}ln")), (format_string_span, String::new())], + Applicability::MachineApplicable, + ); + } else if format_snippet.ends_with("\\n\"") { + // print!("...\n"), write!(f, "...\n") - let span = str_lit_span.from_inner(InnerSpan { - start: span.start + 1, - end: span.end - hack, - }); - self.push_to_complex(span, n); - }; + let hi = format_string_span.hi(); + let newline_span = format_string_span.with_lo(hi - BytePos(3)).with_hi(hi - BytePos(1)); - if let (CountIsParam(n), Some(span)) = (arg.format.width, arg.format.width_span) { - let span = str_lit_span.from_inner(InnerSpan { - start: span.start, - end: span.end - 1, - }); - self.push_to_complex(span, n); - }; + diag.multipart_suggestion( + &format!("use `{name}ln!` instead"), + vec![(name_span, format!("{name}ln")), (newline_span, String::new())], + Applicability::MachineApplicable, + ); + } + }, + ); } } -impl Write { - /// Parses a format string into a collection of spans for each argument. This only keeps track - /// of empty format arguments. Will also lint usages of debug format strings outside of debug - /// impls. - fn parse_fmt_string(&self, cx: &EarlyContext<'_>, str_lit: &StrLit) -> Option<SimpleFormatArgs> { - use rustc_parse_format::{ParseMode, Parser, Piece}; - - let str_sym = str_lit.symbol_unescaped.as_str(); - let style = match str_lit.style { - StrStyle::Cooked => None, - StrStyle::Raw(n) => Some(n as usize), - }; - - let mut parser = Parser::new(str_sym, style, snippet_opt(cx, str_lit.span), false, ParseMode::Format); - let mut args = SimpleFormatArgs::default(); +fn check_empty_string(cx: &LateContext<'_>, format_args: &FormatArgsExpn<'_>, macro_call: &MacroCall, name: &str) { + if let [part] = &format_args.format_string.parts[..] + && let mut span = format_args.format_string.span + && part.as_str() == "\n" + { + let lint = if name == "writeln" { + span = expand_past_previous_comma(cx, span); - while let Some(arg) = parser.next() { - let arg = match arg { - Piece::String(_) => continue, - Piece::NextArgument(arg) => arg, - }; - let span = parser - .arg_places - .last() - .map_or(DUMMY_SP, |&x| str_lit.span.from_inner(InnerSpan::new(x.start, x.end))); - - if !self.in_debug_impl && arg.format.ty == "?" { - // FIXME: modify rustc's fmt string parser to give us the current span - span_lint(cx, USE_DEBUG, span, "use of `Debug`-based formatting"); - } - args.push(arg, span); - args.push_complex(cx, arg, str_lit.span, span); - } - - parser.errors.is_empty().then_some(args) - } - - /// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two - /// `Option`s. The first `Option` of the tuple is the macro's format string. It includes - /// the contents of the string, whether it's a raw string, and the span of the literal in the - /// source. The second `Option` in the tuple is, in the `write[ln]!` case, the expression the - /// `format_str` should be written to. - /// - /// Example: - /// - /// Calling this function on - /// ```rust - /// # use std::fmt::Write; - /// # let mut buf = String::new(); - /// # let something = "something"; - /// writeln!(buf, "string to write: {}", something); - /// ``` - /// will return - /// ```rust,ignore - /// (Some("string to write: {}"), Some(buf)) - /// ``` - fn check_tts<'a>(&self, cx: &EarlyContext<'a>, tts: TokenStream, is_write: bool) -> (Option<StrLit>, Option<Expr>) { - let mut parser = parser::Parser::new(&cx.sess().parse_sess, tts, false, None); - let expr = if is_write { - match parser - .parse_expr() - .map(rustc_ast::ptr::P::into_inner) - .map_err(DiagnosticBuilder::cancel) - { - // write!(e, ...) - Ok(p) if parser.eat(&token::Comma) => Some(p), - // write!(e) or error - e => return (None, e.ok()), - } + WRITELN_EMPTY_STRING } else { - None + PRINTLN_EMPTY_STRING }; - let fmtstr = match parser.parse_str_lit() { - Ok(fmtstr) => fmtstr, - Err(_) => return (None, expr), - }; + span_lint_and_then( + cx, + lint, + macro_call.span, + &format!("empty string literal in `{name}!`"), + |diag| { + diag.span_suggestion( + span, + "remove the empty string", + String::new(), + Applicability::MachineApplicable, + ); + }, + ); + } +} - let args = match self.parse_fmt_string(cx, &fmtstr) { - Some(args) => args, - None => return (Some(fmtstr), expr), - }; +fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgsExpn<'_>, name: &str) { + let mut counts = HirIdMap::<usize>::default(); + for param in format_args.params() { + *counts.entry(param.value.hir_id).or_default() += 1; + } - let lint = if is_write { WRITE_LITERAL } else { PRINT_LITERAL }; - let mut unnamed_args = args.get_unnamed(); - let mut complex_unnamed_args = args.get_complex_unnamed(); - loop { - if !parser.eat(&token::Comma) { - return (Some(fmtstr), expr); - } + for arg in &format_args.args { + let value = arg.param.value; - let comma_span = parser.prev_token.span; - let token_expr = if let Ok(expr) = parser.parse_expr().map_err(DiagnosticBuilder::cancel) { - expr - } else { - return (Some(fmtstr), None); - }; - let complex_unnamed_arg = complex_unnamed_args.next(); - - let (fmt_spans, lit) = match &token_expr.kind { - ExprKind::Lit(lit) => (unnamed_args.next().unwrap_or(&[]), lit), - ExprKind::Assign(lhs, rhs, _) => { - if let Some(span) = complex_unnamed_arg { - for x in span { - Self::report_positional_named_param(cx, *x, lhs, rhs); - } - } - match (&lhs.kind, &rhs.kind) { - (ExprKind::Path(_, p), ExprKind::Lit(lit)) => (args.get_named(p), lit), - _ => continue, + if counts[&value.hir_id] == 1 + && arg.format.is_default() + && let ExprKind::Lit(lit) = &value.kind + && !value.span.from_expansion() + && let Some(value_string) = snippet_opt(cx, value.span) + { + let (replacement, replace_raw) = match lit.node { + LitKind::Str(..) => extract_str_literal(&value_string), + LitKind::Char(ch) => ( + match ch { + '"' => "\\\"", + '\'' => "'", + _ => &value_string[1..value_string.len() - 1], } - }, - _ => { - unnamed_args.next(); - continue; - }, + .to_string(), + false, + ), + LitKind::Bool(b) => (b.to_string(), false), + _ => continue, + }; + + let lint = if name.starts_with("write") { + WRITE_LITERAL + } else { + PRINT_LITERAL }; - let replacement: String = match lit.token_lit.kind { - LitKind::StrRaw(_) | LitKind::ByteStrRaw(_) if matches!(fmtstr.style, StrStyle::Raw(_)) => { - lit.token_lit.symbol.as_str().replace('{', "{{").replace('}', "}}") + let format_string_is_raw = format_args.format_string.style.is_some(); + let replacement = match (format_string_is_raw, replace_raw) { + (false, false) => Some(replacement), + (false, true) => Some(replacement.replace('"', "\\\"").replace('\\', "\\\\")), + (true, false) => match conservative_unescape(&replacement) { + Ok(unescaped) => Some(unescaped), + Err(UnescapeErr::Lint) => None, + Err(UnescapeErr::Ignore) => continue, }, - LitKind::Str | LitKind::ByteStr if matches!(fmtstr.style, StrStyle::Cooked) => { - lit.token_lit.symbol.as_str().replace('{', "{{").replace('}', "}}") + (true, true) => { + if replacement.contains(['#', '"']) { + None + } else { + Some(replacement) + } }, - LitKind::StrRaw(_) - | LitKind::Str - | LitKind::ByteStrRaw(_) - | LitKind::ByteStr - | LitKind::Integer - | LitKind::Float - | LitKind::Err => continue, - LitKind::Byte | LitKind::Char => match lit.token_lit.symbol.as_str() { - "\"" if matches!(fmtstr.style, StrStyle::Cooked) => "\\\"", - "\"" if matches!(fmtstr.style, StrStyle::Raw(0)) => continue, - "\\\\" if matches!(fmtstr.style, StrStyle::Raw(_)) => "\\", - "\\'" => "'", - "{" => "{{", - "}" => "}}", - x if matches!(fmtstr.style, StrStyle::Raw(_)) && x.starts_with('\\') => continue, - x => x, - } - .into(), - LitKind::Bool => lit.token_lit.symbol.as_str().deref().into(), }; - if !fmt_spans.is_empty() { - span_lint_and_then( - cx, - lint, - token_expr.span, - "literal with an empty format string", - |diag| { + span_lint_and_then( + cx, + lint, + value.span, + "literal with an empty format string", + |diag| { + if let Some(replacement) = replacement { + // `format!("{}", "a")`, `format!("{named}", named = "b") + // ~~~~~ ~~~~~~~~~~~~~ + let value_span = expand_past_previous_comma(cx, value.span); + + let replacement = replacement.replace('{', "{{").replace('}', "}}"); diag.multipart_suggestion( "try this", - iter::once((comma_span.to(token_expr.span), String::new())) - .chain(fmt_spans.iter().copied().zip(iter::repeat(replacement))) - .collect(), + vec![(arg.span, replacement), (value_span, String::new())], Applicability::MachineApplicable, ); - }, - ); - } - } - } - - fn report_positional_named_param(cx: &EarlyContext<'_>, span: Span, lhs: &P<Expr>, _rhs: &P<Expr>) { - if let ExprKind::Path(_, _p) = &lhs.kind { - let mut applicability = Applicability::MachineApplicable; - let name = snippet_with_applicability(cx, lhs.span, "name", &mut applicability); - // We need to do this hack as precision spans should be converted from .* to .foo$ - let hack = snippet(cx, span, "").contains('*'); - - span_lint_and_sugg( - cx, - POSITIONAL_NAMED_FORMAT_PARAMETERS, - span, - &format!("named parameter {} is used as a positional parameter", name), - "replace it with", - if hack { - format!("{}$", name) - } else { - format!("{}", name) + } }, - applicability, ); - }; - } - - fn lint_println_empty_string(&self, cx: &EarlyContext<'_>, mac: &MacCall) { - if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { - if fmt_str.symbol == kw::Empty { - let name = mac.path.segments[0].ident.name; - span_lint_and_sugg( - cx, - PRINTLN_EMPTY_STRING, - mac.span(), - &format!("using `{}!(\"\")`", name), - "replace it with", - format!("{}!()", name), - Applicability::MachineApplicable, - ); - } - } - } - - fn lint_print_with_newline(&self, cx: &EarlyContext<'_>, mac: &MacCall) { - if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { - if check_newlines(&fmt_str) { - let name = mac.path.segments[0].ident.name; - let suggested = format!("{}ln", name); - span_lint_and_then( - cx, - PRINT_WITH_NEWLINE, - mac.span(), - &format!("using `{}!()` with a format string that ends in a single newline", name), - |err| { - err.multipart_suggestion( - &format!("use `{}!` instead", suggested), - vec![(mac.path.span, suggested), (newline_span(&fmt_str).0, String::new())], - Applicability::MachineApplicable, - ); - }, - ); - } } } } -/// Checks if the format string contains a single newline that terminates it. +/// Removes the raw marker, `#`s and quotes from a str, and returns if the literal is raw /// -/// Literal and escaped newlines are both checked (only literal for raw strings). -fn check_newlines(fmtstr: &StrLit) -> bool { - let mut has_internal_newline = false; - let mut last_was_cr = false; - let mut should_lint = false; - - let contents = fmtstr.symbol.as_str(); - - let mut cb = |r: Range<usize>, c: Result<char, EscapeError>| { - let c = match c { - Ok(c) => c, - Err(e) if !e.is_fatal() => return, - Err(e) => panic!("{:?}", e), - }; - - if r.end == contents.len() && c == '\n' && !last_was_cr && !has_internal_newline { - should_lint = true; - } else { - last_was_cr = c == '\r'; - if c == '\n' { - has_internal_newline = true; - } - } +/// `r#"a"#` -> (`a`, true) +/// +/// `"b"` -> (`b`, false) +fn extract_str_literal(literal: &str) -> (String, bool) { + let (literal, raw) = match literal.strip_prefix('r') { + Some(stripped) => (stripped.trim_matches('#'), true), + None => (literal, false), }; - match fmtstr.style { - StrStyle::Cooked => unescape::unescape_literal(contents, unescape::Mode::Str, &mut cb), - StrStyle::Raw(_) => unescape::unescape_literal(contents, unescape::Mode::RawStr, &mut cb), + (literal[1..literal.len() - 1].to_string(), raw) +} + +enum UnescapeErr { + /// Should still be linted, can be manually resolved by author, e.g. + /// + /// ```ignore + /// print!(r"{}", '"'); + /// ``` + Lint, + /// Should not be linted, e.g. + /// + /// ```ignore + /// print!(r"{}", '\r'); + /// ``` + Ignore, +} + +/// Unescape a normal string into a raw string +fn conservative_unescape(literal: &str) -> Result<String, UnescapeErr> { + let mut unescaped = String::with_capacity(literal.len()); + let mut chars = literal.chars(); + let mut err = false; + + while let Some(ch) = chars.next() { + match ch { + '#' => err = true, + '\\' => match chars.next() { + Some('\\') => unescaped.push('\\'), + Some('"') => err = true, + _ => return Err(UnescapeErr::Ignore), + }, + _ => unescaped.push(ch), + } } - should_lint + if err { Err(UnescapeErr::Lint) } else { Ok(unescaped) } +} + +// Expand from `writeln!(o, "")` to `writeln!(o, "")` +// ^^ ^^^^ +fn expand_past_previous_comma(cx: &LateContext<'_>, span: Span) -> Span { + let extended = cx.sess().source_map().span_extend_to_prev_char(span, ',', true); + extended.with_lo(extended.lo() - BytePos(1)) } |