From 8f43c70dd5b6575298224eb21f25ec329a08f1dd Mon Sep 17 00:00:00 2001 From: irelaxcn Date: Fri, 19 Dec 2025 01:17:16 +0800 Subject: [PATCH] Fix `println_empty_string` suggestion caused error when there is a `,` after arg. Make `writeln_empty_string` don't provide an auto-fix suggestion when there is a comment in the macro call span. --- clippy_lints/src/write/empty_string.rs | 47 ++++++---- tests/ui/crashes/ice-10148.stderr | 2 +- tests/ui/println_empty_string.fixed | 35 ++++++++ tests/ui/println_empty_string.rs | 51 +++++++++++ tests/ui/println_empty_string.stderr | 90 ++++++++++++++++++- tests/ui/writeln_empty_string_unfixable.rs | 30 +++++++ .../ui/writeln_empty_string_unfixable.stderr | 61 +++++++++++++ 7 files changed, 296 insertions(+), 20 deletions(-) create mode 100644 tests/ui/writeln_empty_string_unfixable.rs create mode 100644 tests/ui/writeln_empty_string_unfixable.stderr diff --git a/clippy_lints/src/write/empty_string.rs b/clippy_lints/src/write/empty_string.rs index e7eb99eb34ec..23ede084fab5 100644 --- a/clippy_lints/src/write/empty_string.rs +++ b/clippy_lints/src/write/empty_string.rs @@ -1,37 +1,48 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::macros::MacroCall; use clippy_utils::source::expand_past_previous_comma; -use clippy_utils::sym; +use clippy_utils::{span_extract_comments, sym}; use rustc_ast::{FormatArgs, FormatArgsPiece}; use rustc_errors::Applicability; -use rustc_lint::LateContext; +use rustc_lint::{LateContext, LintContext}; use super::{PRINTLN_EMPTY_STRING, WRITELN_EMPTY_STRING}; pub(super) fn check(cx: &LateContext<'_>, format_args: &FormatArgs, macro_call: &MacroCall, name: &str) { if let [FormatArgsPiece::Literal(sym::LF)] = &format_args.template[..] { - let mut span = format_args.span; - - let lint = if name == "writeln" { - span = expand_past_previous_comma(cx, span); - - WRITELN_EMPTY_STRING - } else { - PRINTLN_EMPTY_STRING - }; + let is_writeln = name == "writeln"; span_lint_and_then( cx, - lint, + if is_writeln { + WRITELN_EMPTY_STRING + } else { + PRINTLN_EMPTY_STRING + }, macro_call.span, format!("empty string literal in `{name}!`"), |diag| { - diag.span_suggestion( - span, - "remove the empty string", - String::new(), - Applicability::MachineApplicable, - ); + let mut applicability = Applicability::MaybeIncorrect; + let mut span = format_args.span; + + // If the macro call is `println!` + // or there isn't a comment in the span of macro call, we provide an auto-fix suggestion. + if !is_writeln || span_extract_comments(cx.sess().source_map(), macro_call.span).is_empty() { + applicability = Applicability::MachineApplicable; + + let closing_paren = cx.sess().source_map().span_extend_to_prev_char_before( + macro_call.span.shrink_to_hi(), + ')', + false, + ); + span = format_args.span.with_hi(closing_paren.lo()); + + if is_writeln { + span = expand_past_previous_comma(cx, span); + } + } + + diag.span_suggestion(span, "remove the empty string", "", applicability); }, ); } diff --git a/tests/ui/crashes/ice-10148.stderr b/tests/ui/crashes/ice-10148.stderr index 639cf2dd442b..e91fb3778a31 100644 --- a/tests/ui/crashes/ice-10148.stderr +++ b/tests/ui/crashes/ice-10148.stderr @@ -2,7 +2,7 @@ error: empty string literal in `println!` --> tests/ui/crashes/ice-10148.rs:8:5 | LL | println!(with_span!(""something "")); - | ^^^^^^^^^^^^^^^^^^^^-----------^^^^^ + | ^^^^^^^^^^^^^^^^^^^^---------------^ | | | help: remove the empty string | diff --git a/tests/ui/println_empty_string.fixed b/tests/ui/println_empty_string.fixed index 05e262ec7786..305a7ac58678 100644 --- a/tests/ui/println_empty_string.fixed +++ b/tests/ui/println_empty_string.fixed @@ -19,3 +19,38 @@ fn main() { //~^ println_empty_string } } + +#[rustfmt::skip] +fn issue_16167() { + println!(); + //~^ println_empty_string + match "a" { + _ => println!(), + //~^ println_empty_string + } + + eprintln!(); + //~^^ println_empty_string + match "a" { + _ => eprintln!(), // tab + //~^ println_empty_string + } + + //~v println_empty_string + println!( + ); + match "a" { + //~v println_empty_string + _ => println!( + ), + } + + //~v println_empty_string + eprintln!( + ); + match "a" { + //~v println_empty_string + _ => eprintln!( + ), + } +} diff --git a/tests/ui/println_empty_string.rs b/tests/ui/println_empty_string.rs index 028ddb60dbce..d67696e518a7 100644 --- a/tests/ui/println_empty_string.rs +++ b/tests/ui/println_empty_string.rs @@ -19,3 +19,54 @@ fn main() { //~^ println_empty_string } } + +#[rustfmt::skip] +fn issue_16167() { + println!(""/* ,,, */); + //~^ println_empty_string + match "a" { + _ => println!("" ,), + //~^ println_empty_string + } + + eprintln!("" + ,); + //~^^ println_empty_string + match "a" { + _ => eprintln!("" ,), // tab + //~^ println_empty_string + } + + //~v println_empty_string + println!( + "\ + \ + " /* comment ,,, + , */ , + ); + match "a" { + //~v println_empty_string + _ => println!( + "\ + \ + " + //, other comment + , + ), + } + + //~v println_empty_string + eprintln!( + "\ + \ + ", + ); + match "a" { + //~v println_empty_string + _ => eprintln!( + "\ + \ + ", // , comment + ), + } +} diff --git a/tests/ui/println_empty_string.stderr b/tests/ui/println_empty_string.stderr index 8b997aef9069..bbc3196fd74a 100644 --- a/tests/ui/println_empty_string.stderr +++ b/tests/ui/println_empty_string.stderr @@ -33,5 +33,93 @@ LL | _ => eprintln!(""), | | | help: remove the empty string -error: aborting due to 4 previous errors +error: empty string literal in `println!` + --> tests/ui/println_empty_string.rs:25:5 + | +LL | println!(""/* ,,, */); + | ^^^^^^^^^-----------^ + | | + | help: remove the empty string + +error: empty string literal in `println!` + --> tests/ui/println_empty_string.rs:28:14 + | +LL | _ => println!("" ,), + | ^^^^^^^^^--------^ + | | + | help: remove the empty string + +error: empty string literal in `eprintln!` + --> tests/ui/println_empty_string.rs:32:5 + | +LL | eprintln!("" + | _____^ - + | |_______________| +LL | || ,); + | ||_________-^ + | |__________| + | help: remove the empty string + +error: empty string literal in `eprintln!` + --> tests/ui/println_empty_string.rs:36:14 + | +LL | _ => eprintln!("" ,), // tab + | ^^^^^^^^^^-------^ + | | + | help: remove the empty string + +error: empty string literal in `println!` + --> tests/ui/println_empty_string.rs:41:5 + | +LL | / println!( +LL | |/ "\ +LL | || \ +LL | || " /* comment ,,, +LL | || , */ , +LL | || ); + | ||____-^ + | |____| + | help: remove the empty string + +error: empty string literal in `println!` + --> tests/ui/println_empty_string.rs:49:14 + | +LL | _ => println!( + | _______________^ +LL | |/ "\ +LL | || \ +LL | || " +LL | || //, other comment +LL | || , +LL | || ), + | ||________-^ + | |________| + | help: remove the empty string + +error: empty string literal in `eprintln!` + --> tests/ui/println_empty_string.rs:59:5 + | +LL | / eprintln!( +LL | |/ "\ +LL | || \ +LL | || ", +LL | || ); + | ||____-^ + | |____| + | help: remove the empty string + +error: empty string literal in `eprintln!` + --> tests/ui/println_empty_string.rs:66:14 + | +LL | _ => eprintln!( + | _______________^ +LL | |/ "\ +LL | || \ +LL | || ", // , comment +LL | || ), + | ||________-^ + | |________| + | help: remove the empty string + +error: aborting due to 12 previous errors diff --git a/tests/ui/writeln_empty_string_unfixable.rs b/tests/ui/writeln_empty_string_unfixable.rs new file mode 100644 index 000000000000..8e1de3df94ac --- /dev/null +++ b/tests/ui/writeln_empty_string_unfixable.rs @@ -0,0 +1,30 @@ +//@no-rustfix +#![allow(unused_must_use)] +#![warn(clippy::writeln_empty_string)] + +use std::io::Write; + +fn issue_16251() { + let mut v = Vec::new(); + + writeln!(v, /* , */ ""); + //~^ writeln_empty_string + + writeln!(v, "" /* , */); + //~^ writeln_empty_string + + writeln!(v, /* comment */ ""); + //~^ writeln_empty_string + + writeln!(v, "" /* comment */); + //~^ writeln_empty_string + + writeln!(v, /* comment, comma */ ""); + //~^ writeln_empty_string + + writeln!(v, "" /* comment, comma */); + //~^ writeln_empty_string + + writeln!(/* */ v, ""); + //~^ writeln_empty_string +} diff --git a/tests/ui/writeln_empty_string_unfixable.stderr b/tests/ui/writeln_empty_string_unfixable.stderr new file mode 100644 index 000000000000..50debe2cc15b --- /dev/null +++ b/tests/ui/writeln_empty_string_unfixable.stderr @@ -0,0 +1,61 @@ +error: empty string literal in `writeln!` + --> tests/ui/writeln_empty_string_unfixable.rs:10:5 + | +LL | writeln!(v, /* , */ ""); + | ^^^^^^^^^^^^^^^^^^^^--^ + | | + | help: remove the empty string + | + = note: `-D clippy::writeln-empty-string` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::writeln_empty_string)]` + +error: empty string literal in `writeln!` + --> tests/ui/writeln_empty_string_unfixable.rs:13:5 + | +LL | writeln!(v, "" /* , */); + | ^^^^^^^^^^^^--^^^^^^^^^ + | | + | help: remove the empty string + +error: empty string literal in `writeln!` + --> tests/ui/writeln_empty_string_unfixable.rs:16:5 + | +LL | writeln!(v, /* comment */ ""); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^--^ + | | + | help: remove the empty string + +error: empty string literal in `writeln!` + --> tests/ui/writeln_empty_string_unfixable.rs:19:5 + | +LL | writeln!(v, "" /* comment */); + | ^^^^^^^^^^^^--^^^^^^^^^^^^^^^ + | | + | help: remove the empty string + +error: empty string literal in `writeln!` + --> tests/ui/writeln_empty_string_unfixable.rs:22:5 + | +LL | writeln!(v, /* comment, comma */ ""); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^--^ + | | + | help: remove the empty string + +error: empty string literal in `writeln!` + --> tests/ui/writeln_empty_string_unfixable.rs:25:5 + | +LL | writeln!(v, "" /* comment, comma */); + | ^^^^^^^^^^^^--^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: remove the empty string + +error: empty string literal in `writeln!` + --> tests/ui/writeln_empty_string_unfixable.rs:28:5 + | +LL | writeln!(/* */ v, ""); + | ^^^^^^^^^^^^^^^^^^^--^ + | | + | help: remove the empty string + +error: aborting due to 7 previous errors +