From 675fd681aa19cc45d27051416db847438efd0f76 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 | 42 +++++---- tests/ui/crashes/ice-10148.stderr | 2 +- tests/ui/println_empty_string.fixed | 20 +++++ tests/ui/println_empty_string.rs | 24 ++++++ tests/ui/println_empty_string.stderr | 39 ++++++++- tests/ui/println_empty_string_unfixable.rs | 30 +++++++ .../ui/println_empty_string_unfixable.stderr | 85 +++++++++++++++++++ tests/ui/writeln_empty_string_unfixable.rs | 26 ++++++ .../ui/writeln_empty_string_unfixable.stderr | 47 ++++++++++ 9 files changed, 295 insertions(+), 20 deletions(-) create mode 100644 tests/ui/println_empty_string_unfixable.rs create mode 100644 tests/ui/println_empty_string_unfixable.stderr 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..1291f2489a21 100644 --- a/clippy_lints/src/write/empty_string.rs +++ b/clippy_lints/src/write/empty_string.rs @@ -1,37 +1,43 @@ 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, - ); + if span_extract_comments(cx.sess().source_map(), macro_call.span).is_empty() { + let closing_paren = cx.sess().source_map().span_extend_to_prev_char_before( + macro_call.span.shrink_to_hi(), + ')', + false, + ); + let mut 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::MachineApplicable); + } else { + // If there is a comment in the span of macro call, we don't provide an auto-fix suggestion. + diag.span_note(format_args.span, "remove the empty string"); + } }, ); } 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..6b1039ee8020 100644 --- a/tests/ui/println_empty_string.fixed +++ b/tests/ui/println_empty_string.fixed @@ -19,3 +19,23 @@ fn main() { //~^ println_empty_string } } + +#[rustfmt::skip] +fn issue_16167() { + //~v println_empty_string + println!( + ); + + match "a" { + _ => println!(), // there is a space between "" and comma + //~^ println_empty_string + } + + eprintln!(); // there is a tab between "" and comma + //~^ println_empty_string + + match "a" { + _ => eprintln!(), // tab and space between "" and comma + //~^ println_empty_string + } +} diff --git a/tests/ui/println_empty_string.rs b/tests/ui/println_empty_string.rs index 028ddb60dbce..db3b8e1a0eac 100644 --- a/tests/ui/println_empty_string.rs +++ b/tests/ui/println_empty_string.rs @@ -19,3 +19,27 @@ fn main() { //~^ println_empty_string } } + +#[rustfmt::skip] +fn issue_16167() { + //~v println_empty_string + println!( + "\ + \ + " + , + ); + + match "a" { + _ => println!("" ,), // there is a space between "" and comma + //~^ println_empty_string + } + + eprintln!("" ,); // there is a tab between "" and comma + //~^ println_empty_string + + match "a" { + _ => eprintln!("" ,), // tab and space between "" and comma + //~^ println_empty_string + } +} diff --git a/tests/ui/println_empty_string.stderr b/tests/ui/println_empty_string.stderr index 8b997aef9069..bdac1bb3b8ef 100644 --- a/tests/ui/println_empty_string.stderr +++ b/tests/ui/println_empty_string.stderr @@ -33,5 +33,42 @@ 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:26:5 + | +LL | / println!( +LL | |/ "\ +LL | || \ +LL | || " +LL | || , +LL | || ); + | ||____-^ + | |____| + | help: remove the empty string + +error: empty string literal in `println!` + --> tests/ui/println_empty_string.rs:34:14 + | +LL | _ => println!("" ,), // there is a space between "" and comma + | ^^^^^^^^^----^ + | | + | help: remove the empty string + +error: empty string literal in `eprintln!` + --> tests/ui/println_empty_string.rs:38:5 + | +LL | eprintln!("" ,); // there is a tab between "" and comma + | ^^^^^^^^^^-------^ + | | + | help: remove the empty string + +error: empty string literal in `eprintln!` + --> tests/ui/println_empty_string.rs:42:14 + | +LL | _ => eprintln!("" ,), // tab and space between "" and comma + | ^^^^^^^^^^--------^ + | | + | help: remove the empty string + +error: aborting due to 8 previous errors diff --git a/tests/ui/println_empty_string_unfixable.rs b/tests/ui/println_empty_string_unfixable.rs new file mode 100644 index 000000000000..d6c30f627a58 --- /dev/null +++ b/tests/ui/println_empty_string_unfixable.rs @@ -0,0 +1,30 @@ +#![allow(clippy::match_single_binding)] + +// If there is a comment in the span of macro call, we don't provide an auto-fix suggestion. +#[rustfmt::skip] +fn issue_16167() { + //~v println_empty_string + println!("" /* comment */); + //~v println_empty_string + eprintln!("" /* comment */); + + //~v println_empty_string + println!( // comment + ""); + //~v println_empty_string + eprintln!( // comment + ""); + + //~v println_empty_string + println!("", /* comment */); + + //~v println_empty_string + println!( + "\ + \ + ", + + // there is a comment in the macro span regardless of its position + + ); +} diff --git a/tests/ui/println_empty_string_unfixable.stderr b/tests/ui/println_empty_string_unfixable.stderr new file mode 100644 index 000000000000..648fd7cdbccd --- /dev/null +++ b/tests/ui/println_empty_string_unfixable.stderr @@ -0,0 +1,85 @@ +error: empty string literal in `println!` + --> tests/ui/println_empty_string_unfixable.rs:7:5 + | +LL | println!("" /* comment */); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: remove the empty string + --> tests/ui/println_empty_string_unfixable.rs:7:14 + | +LL | println!("" /* comment */); + | ^^ + = note: `-D clippy::println-empty-string` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::println_empty_string)]` + +error: empty string literal in `eprintln!` + --> tests/ui/println_empty_string_unfixable.rs:9:5 + | +LL | eprintln!("" /* comment */); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: remove the empty string + --> tests/ui/println_empty_string_unfixable.rs:9:15 + | +LL | eprintln!("" /* comment */); + | ^^ + +error: empty string literal in `println!` + --> tests/ui/println_empty_string_unfixable.rs:12:5 + | +LL | / println!( // comment +LL | | ""); + | |___________________^ + | +note: remove the empty string + --> tests/ui/println_empty_string_unfixable.rs:13:17 + | +LL | ""); + | ^^ + +error: empty string literal in `eprintln!` + --> tests/ui/println_empty_string_unfixable.rs:15:5 + | +LL | / eprintln!( // comment +LL | | ""); + | |___________________^ + | +note: remove the empty string + --> tests/ui/println_empty_string_unfixable.rs:16:17 + | +LL | ""); + | ^^ + +error: empty string literal in `println!` + --> tests/ui/println_empty_string_unfixable.rs:19:5 + | +LL | println!("", /* comment */); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: remove the empty string + --> tests/ui/println_empty_string_unfixable.rs:19:14 + | +LL | println!("", /* comment */); + | ^^ + +error: empty string literal in `println!` + --> tests/ui/println_empty_string_unfixable.rs:22:5 + | +LL | / println!( +LL | | "\ +LL | | \ +LL | | ", +... | +LL | | ); + | |_____^ + | +note: remove the empty string + --> tests/ui/println_empty_string_unfixable.rs:23:9 + | +LL | / "\ +LL | | \ +LL | | ", + | |_____________^ + +error: aborting due to 6 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..ca570fd1fc7b --- /dev/null +++ b/tests/ui/writeln_empty_string_unfixable.rs @@ -0,0 +1,26 @@ +#![allow(unused_must_use)] +#![warn(clippy::writeln_empty_string)] + +use std::io::Write; + +// If there is a comment in the span of macro call, we don't provide an auto-fix suggestion. +#[rustfmt::skip] +fn issue_16251() { + let mut v = Vec::new(); + + writeln!(v, /* comment */ ""); + //~^ writeln_empty_string + + writeln!(v, "" /* comment */); + //~^ writeln_empty_string + + //~v writeln_empty_string + writeln!(v, + "\ + \ + " + + // there is a comment in the macro span regardless of its position + + ); +} diff --git a/tests/ui/writeln_empty_string_unfixable.stderr b/tests/ui/writeln_empty_string_unfixable.stderr new file mode 100644 index 000000000000..0ed802ba84ba --- /dev/null +++ b/tests/ui/writeln_empty_string_unfixable.stderr @@ -0,0 +1,47 @@ +error: empty string literal in `writeln!` + --> tests/ui/writeln_empty_string_unfixable.rs:11:5 + | +LL | writeln!(v, /* comment */ ""); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: remove the empty string + --> tests/ui/writeln_empty_string_unfixable.rs:11:31 + | +LL | writeln!(v, /* comment */ ""); + | ^^ + = 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:14:5 + | +LL | writeln!(v, "" /* comment */); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: remove the empty string + --> tests/ui/writeln_empty_string_unfixable.rs:14:17 + | +LL | writeln!(v, "" /* comment */); + | ^^ + +error: empty string literal in `writeln!` + --> tests/ui/writeln_empty_string_unfixable.rs:18:5 + | +LL | / writeln!(v, +LL | | "\ +LL | | \ +LL | | " +... | +LL | | ); + | |_____^ + | +note: remove the empty string + --> tests/ui/writeln_empty_string_unfixable.rs:19:9 + | +LL | / "\ +LL | | \ +LL | | " + | |_____________^ + +error: aborting due to 3 previous errors +