diff --git a/CHANGELOG.md b/CHANGELOG.md index 795eba1dfeaf..1e8688819a88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ document. [92b4b68...master](https://github.com/rust-lang/rust-clippy/compare/92b4b68...master) +### New Lints + +* Added [`unnecessary_trailing_comma`] to `style` (single-line format-like macros only) + [#13965](https://github.com/rust-lang/rust-clippy/issues/13965) + ## Rust 1.93 Current stable, released 2026-01-22 @@ -7136,6 +7141,7 @@ Released 2018-09-13 [`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by [`unnecessary_struct_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_struct_initialization [`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned +[`unnecessary_trailing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_trailing_comma [`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap [`unnecessary_wraps`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps [`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index a04d133b0d72..18974e8210a6 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -173,6 +173,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::format_args::TO_STRING_IN_FORMAT_ARGS_INFO, crate::format_args::UNINLINED_FORMAT_ARGS_INFO, crate::format_args::UNNECESSARY_DEBUG_FORMATTING_INFO, + crate::format_args::UNNECESSARY_TRAILING_COMMA_INFO, crate::format_args::UNUSED_FORMAT_SPECS_INFO, crate::format_impl::PRINT_IN_FORMAT_IMPL_INFO, crate::format_impl::RECURSIVE_FORMAT_IMPL_INFO, diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 5fb1a0b80f1a..3eb358917a0e 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -28,7 +28,7 @@ use rustc_middle::ty::adjustment::{Adjust, Adjustment, DerefAdjustKind}; use rustc_middle::ty::{self, GenericArg, List, TraitRef, Ty, TyCtxt, Upcast}; use rustc_session::impl_lint_pass; use rustc_span::edition::Edition::Edition2021; -use rustc_span::{Span, Symbol, sym}; +use rustc_span::{BytePos, Pos, Span, Symbol, sym}; use rustc_trait_selection::infer::TyCtxtInferExt; use rustc_trait_selection::traits::{Obligation, ObligationCause, Selection, SelectionContext}; @@ -229,6 +229,35 @@ declare_clippy_lint! { "formatting a pointer" } +declare_clippy_lint! { + /// ### What it does + /// Suggests removing an unnecessary trailing comma before the closing parenthesis in + /// single-line macro invocations. + /// + /// ### Why is this bad? + /// The trailing comma is redundant and removing it is more consistent with how + /// `rustfmt` formats regular function calls. + /// + /// ### Known limitations + /// This lint currently only runs on format-like macros (e.g. `format!`, `println!`, + /// `write!`) because it relies on format-argument parsing; applying it to arbitrary + /// user macros could cause incorrect suggestions. It may be extended to other + /// macros in the future. Only single-line macro invocations are linted. + /// + /// ### Example + /// ```no_run + /// println!("Foo={}", 1,); + /// ``` + /// Use instead: + /// ```no_run + /// println!("Foo={}", 1); + /// ``` + #[clippy::version = "1.95.0"] + pub UNNECESSARY_TRAILING_COMMA, + pedantic, + "unnecessary trailing comma before closing parenthesis" +} + impl_lint_pass!(FormatArgs<'_> => [ FORMAT_IN_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS, @@ -236,6 +265,7 @@ impl_lint_pass!(FormatArgs<'_> => [ UNNECESSARY_DEBUG_FORMATTING, UNUSED_FORMAT_SPECS, POINTER_FORMAT, + UNNECESSARY_TRAILING_COMMA, ]); #[expect(clippy::struct_field_names)] @@ -280,6 +310,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs<'tcx> { has_pointer_format: &mut self.has_pointer_format, }; + linter.check_trailing_comma(); linter.check_templates(); if self.msrv.meets(cx, msrvs::FORMAT_ARGS_CAPTURE) { @@ -302,6 +333,29 @@ struct FormatArgsExpr<'a, 'tcx> { } impl<'tcx> FormatArgsExpr<'_, 'tcx> { + /// Check if there is a comma after the last format macro arg. + fn check_trailing_comma(&self) { + let span = self.macro_call.span; + if let Some(src) = span.get_source_text(self.cx) + && let Some(src) = src.strip_suffix([')', ']', '}']) + && let src = src.trim_end_matches(|c: char| c.is_whitespace() && c != '\n') + && let Some(src) = src.strip_suffix(',') + && let src = src.trim_end_matches(|c: char| c.is_whitespace() && c != '\n') + && !src.ends_with('\n') + { + span_lint_and_sugg( + self.cx, + UNNECESSARY_TRAILING_COMMA, + span.with_lo(span.lo() + BytePos::from_usize(src.len())) + .with_hi(span.hi() - BytePos(1)), + "unnecessary trailing comma", + "remove the trailing comma", + String::new(), + Applicability::MachineApplicable, + ); + } + } + fn check_templates(&mut self) { for piece in &self.format_args.template { if let FormatArgsPiece::Placeholder(placeholder) = piece diff --git a/tests/ui/println_empty_string.fixed b/tests/ui/println_empty_string.fixed index 6b1039ee8020..2c2901bc715a 100644 --- a/tests/ui/println_empty_string.fixed +++ b/tests/ui/println_empty_string.fixed @@ -1,4 +1,4 @@ -#![allow(clippy::match_single_binding)] +#![allow(clippy::match_single_binding, clippy::unnecessary_trailing_comma)] fn main() { println!(); diff --git a/tests/ui/println_empty_string.rs b/tests/ui/println_empty_string.rs index db3b8e1a0eac..bc2971f54f2c 100644 --- a/tests/ui/println_empty_string.rs +++ b/tests/ui/println_empty_string.rs @@ -1,4 +1,4 @@ -#![allow(clippy::match_single_binding)] +#![allow(clippy::match_single_binding, clippy::unnecessary_trailing_comma)] fn main() { println!(); diff --git a/tests/ui/unnecessary_trailing_comma.fixed b/tests/ui/unnecessary_trailing_comma.fixed new file mode 100644 index 000000000000..499d9ee1ca23 --- /dev/null +++ b/tests/ui/unnecessary_trailing_comma.fixed @@ -0,0 +1,86 @@ +// run-rustfix +#![warn(clippy::unnecessary_trailing_comma)] + +fn main() {} + +// fmt breaks - https://github.com/rust-lang/rustfmt/issues/6797 +#[rustfmt::skip] +fn simple() { + println!["Foo(,)"]; + println!("Foo"); //~ unnecessary_trailing_comma + println!{"Foo"}; //~ unnecessary_trailing_comma + println!["Foo"]; //~ unnecessary_trailing_comma + println!("Foo={}", 1); //~ unnecessary_trailing_comma + println!(concat!("b", "o", "o")); //~ unnecessary_trailing_comma + println!("Foo(,)"); //~ unnecessary_trailing_comma + println!("Foo[,]"); //~ unnecessary_trailing_comma + println!["Foo(,)"]; //~ unnecessary_trailing_comma + println!["Foo[,]"]; //~ unnecessary_trailing_comma + println!["Foo{{,}}"]; //~ unnecessary_trailing_comma + println!{"Foo{{,}}"}; //~ unnecessary_trailing_comma + println!{"Foo(,)"}; //~ unnecessary_trailing_comma + println!{"Foo[,]"}; //~ unnecessary_trailing_comma + println!["Foo(,"]; //~ unnecessary_trailing_comma + println!["Foo[,"]; //~ unnecessary_trailing_comma + println!["Foo{{,}}"]; //~ unnecessary_trailing_comma + println!{"Foo{{,}}"}; //~ unnecessary_trailing_comma + println!{"Foo(,"}; //~ unnecessary_trailing_comma + println!{"Foo[,"}; //~ unnecessary_trailing_comma + + // This should eventually work, but requires more work + println!(concat!("Foo", "=", "{}"), 1,); + println!("No params", /*"a,){ */); + println!("No params" /* "a,){*/, /*"a,){ */); + + // No trailing comma - no lint + println!("{}", 1); + println!(concat!("b", "o", "o")); + println!(concat!("Foo", "=", "{}"), 1); + + println!("Foo" ); + println!{"Foo" }; + println!["Foo" ]; + println!("Foo={}", 1); + println!(concat!("b", "o", "o")); + println!("Foo(,)"); + println!("Foo[,]"); + println!["Foo(,)"]; + println!["Foo[,]"]; + println!["Foo{{,}}"]; + println!{"Foo{{,}}"}; + println!{"Foo(,)"}; + println!{"Foo[,]"}; + println!["Foo(,"]; + println!["Foo[,"]; + println!["Foo{{,}}"]; + println!{"Foo{{,}}"}; + println!{"Foo(,"}; + println!{"Foo[,"}; + + // Multi-line macro - must NOT lint (single-line only) + println!( + "very long string to prevent fmt from making it into a single line: {}", + 1, + ); + + print!("{}" + , 1 + ,); +} + +// The macro invocation itself should never be fixed +// The call to println! on the other hand might be ok to suggest in the future + +macro_rules! from_macro { + (0,) => { + println!("Foo",); + }; + (1,) => { + println!("Foo={}", 1,); + }; +} + +fn from_macro() { + from_macro!(0,); + from_macro!(1,); +} diff --git a/tests/ui/unnecessary_trailing_comma.rs b/tests/ui/unnecessary_trailing_comma.rs new file mode 100644 index 000000000000..15dea27b887b --- /dev/null +++ b/tests/ui/unnecessary_trailing_comma.rs @@ -0,0 +1,86 @@ +// run-rustfix +#![warn(clippy::unnecessary_trailing_comma)] + +fn main() {} + +// fmt breaks - https://github.com/rust-lang/rustfmt/issues/6797 +#[rustfmt::skip] +fn simple() { + println!["Foo(,)"]; + println!("Foo" , ); //~ unnecessary_trailing_comma + println!{"Foo" , }; //~ unnecessary_trailing_comma + println!["Foo" , ]; //~ unnecessary_trailing_comma + println!("Foo={}", 1 , ); //~ unnecessary_trailing_comma + println!(concat!("b", "o", "o") , ); //~ unnecessary_trailing_comma + println!("Foo(,)",); //~ unnecessary_trailing_comma + println!("Foo[,]" , ); //~ unnecessary_trailing_comma + println!["Foo(,)", ]; //~ unnecessary_trailing_comma + println!["Foo[,]", ]; //~ unnecessary_trailing_comma + println!["Foo{{,}}", ]; //~ unnecessary_trailing_comma + println!{"Foo{{,}}", }; //~ unnecessary_trailing_comma + println!{"Foo(,)", }; //~ unnecessary_trailing_comma + println!{"Foo[,]", }; //~ unnecessary_trailing_comma + println!["Foo(,", ]; //~ unnecessary_trailing_comma + println!["Foo[,", ]; //~ unnecessary_trailing_comma + println!["Foo{{,}}", ]; //~ unnecessary_trailing_comma + println!{"Foo{{,}}", }; //~ unnecessary_trailing_comma + println!{"Foo(,", }; //~ unnecessary_trailing_comma + println!{"Foo[,", }; //~ unnecessary_trailing_comma + + // This should eventually work, but requires more work + println!(concat!("Foo", "=", "{}"), 1,); + println!("No params", /*"a,){ */); + println!("No params" /* "a,){*/, /*"a,){ */); + + // No trailing comma - no lint + println!("{}", 1); + println!(concat!("b", "o", "o")); + println!(concat!("Foo", "=", "{}"), 1); + + println!("Foo" ); + println!{"Foo" }; + println!["Foo" ]; + println!("Foo={}", 1); + println!(concat!("b", "o", "o")); + println!("Foo(,)"); + println!("Foo[,]"); + println!["Foo(,)"]; + println!["Foo[,]"]; + println!["Foo{{,}}"]; + println!{"Foo{{,}}"}; + println!{"Foo(,)"}; + println!{"Foo[,]"}; + println!["Foo(,"]; + println!["Foo[,"]; + println!["Foo{{,}}"]; + println!{"Foo{{,}}"}; + println!{"Foo(,"}; + println!{"Foo[,"}; + + // Multi-line macro - must NOT lint (single-line only) + println!( + "very long string to prevent fmt from making it into a single line: {}", + 1, + ); + + print!("{}" + , 1 + ,); +} + +// The macro invocation itself should never be fixed +// The call to println! on the other hand might be ok to suggest in the future + +macro_rules! from_macro { + (0,) => { + println!("Foo",); + }; + (1,) => { + println!("Foo={}", 1,); + }; +} + +fn from_macro() { + from_macro!(0,); + from_macro!(1,); +} diff --git a/tests/ui/unnecessary_trailing_comma.stderr b/tests/ui/unnecessary_trailing_comma.stderr new file mode 100644 index 000000000000..06fd5b1861a5 --- /dev/null +++ b/tests/ui/unnecessary_trailing_comma.stderr @@ -0,0 +1,119 @@ +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:10:19 + | +LL | println!("Foo" , ); + | ^^^ help: remove the trailing comma + | + = note: `-D clippy::unnecessary-trailing-comma` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_trailing_comma)]` + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:11:19 + | +LL | println!{"Foo" , }; + | ^^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:12:19 + | +LL | println!["Foo" , ]; + | ^^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:13:27 + | +LL | println!("Foo={}", 1 , ); + | ^^^^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:14:36 + | +LL | println!(concat!("b", "o", "o") , ); + | ^^^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:15:22 + | +LL | println!("Foo(,)",); + | ^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:16:22 + | +LL | println!("Foo[,]" , ); + | ^^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:17:22 + | +LL | println!["Foo(,)", ]; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:18:22 + | +LL | println!["Foo[,]", ]; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:19:24 + | +LL | println!["Foo{{,}}", ]; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:20:24 + | +LL | println!{"Foo{{,}}", }; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:21:22 + | +LL | println!{"Foo(,)", }; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:22:22 + | +LL | println!{"Foo[,]", }; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:23:21 + | +LL | println!["Foo(,", ]; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:24:21 + | +LL | println!["Foo[,", ]; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:25:24 + | +LL | println!["Foo{{,}}", ]; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:26:24 + | +LL | println!{"Foo{{,}}", }; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:27:21 + | +LL | println!{"Foo(,", }; + | ^^ help: remove the trailing comma + +error: unnecessary trailing comma + --> tests/ui/unnecessary_trailing_comma.rs:28:21 + | +LL | println!{"Foo[,", }; + | ^^ help: remove the trailing comma + +error: aborting due to 19 previous errors +