Skip to content

Commit 52fa719

Browse files
AlexendooAlexendoo
committed
Add unused_format_specs lint
1 parent 45dd9f3 commit 52fa719

13 files changed

+382
-28
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4315,6 +4315,7 @@ Released 2018-09-13
43154315
[`unstable_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#unstable_as_slice
43164316
[`unused_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_async
43174317
[`unused_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_collect
4318+
[`unused_format_specs`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_format_specs
43184319
[`unused_io_amount`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_io_amount
43194320
[`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label
43204321
[`unused_peekable`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_peekable

‎clippy_lints/src/format_args.rs

+129-22
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
22
use clippy_utils::macros::FormatParamKind::{Implicit, Named, Numbered, Starred};
3-
use clippy_utils::macros::{is_format_macro, is_panic, FormatArgsExpn, FormatParam, FormatParamUsage};
3+
use clippy_utils::macros::{
4+
is_format_macro, is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage,
5+
};
46
use clippy_utils::source::snippet_opt;
5-
use clippy_utils::ty::implements_trait;
7+
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
68
use clippy_utils::{is_diag_trait_item, meets_msrv, msrvs};
79
use if_chain::if_chain;
810
use itertools::Itertools;
@@ -117,7 +119,43 @@ declare_clippy_lint! {
117119
"using non-inlined variables in `format!` calls"
118120
}
119121

120-
impl_lint_pass!(FormatArgs => [FORMAT_IN_FORMAT_ARGS, UNINLINED_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS]);
122+
declare_clippy_lint! {
123+
/// ### What it does
124+
/// Detects [formatting parameters] that have no effect on the output of a
125+
/// `format!()`, `println!()` or similar macro.
126+
///
127+
/// ### Why is this bad?
128+
/// Shorter format specifiers are easier to read, it may also indicate that
129+
/// an expected formatting operation such as adding padding isn't happening.
130+
///
131+
/// ### Example
132+
/// ```rust
133+
/// println!("{:.}", 1.0);
134+
///
135+
/// println!("not padded: {:5}", format_args!("..."));
136+
/// ```
137+
/// Use instead:
138+
/// ```rust
139+
/// println!("{}", 1.0);
140+
///
141+
/// println!("not padded: {}", format_args!("..."));
142+
/// // OR
143+
/// println!("padded: {:5}", format!("..."));
144+
/// ```
145+
///
146+
/// [formatting parameters]: https://doc.rust-lang.org/std/fmt/index.html#formatting-parameters
147+
#[clippy::version = "1.66.0"]
148+
pub UNUSED_FORMAT_SPECS,
149+
complexity,
150+
"use of a format specifier that has no effect"
151+
}
152+
153+
impl_lint_pass!(FormatArgs => [
154+
FORMAT_IN_FORMAT_ARGS,
155+
TO_STRING_IN_FORMAT_ARGS,
156+
UNINLINED_FORMAT_ARGS,
157+
UNUSED_FORMAT_SPECS,
158+
]);
121159

122160
pub struct FormatArgs {
123161
msrv: Option<RustcVersion>,
@@ -132,34 +170,103 @@ impl FormatArgs {
132170

133171
impl<'tcx> LateLintPass<'tcx> for FormatArgs {
134172
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
135-
if_chain! {
136-
if let Some(format_args) = FormatArgsExpn::parse(cx, expr);
137-
let expr_expn_data = expr.span.ctxt().outer_expn_data();
138-
let outermost_expn_data = outermost_expn_data(expr_expn_data);
139-
if let Some(macro_def_id) = outermost_expn_data.macro_def_id;
140-
if is_format_macro(cx, macro_def_id);
141-
if let ExpnKind::Macro(_, name) = outermost_expn_data.kind;
142-
then {
143-
for arg in &format_args.args {
144-
if !arg.format.is_default() {
145-
continue;
146-
}
147-
if is_aliased(&format_args, arg.param.value.hir_id) {
148-
continue;
149-
}
150-
check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg.param.value);
151-
check_to_string_in_format_args(cx, name, arg.param.value);
173+
if let Some(format_args) = FormatArgsExpn::parse(cx, expr)
174+
&& let expr_expn_data = expr.span.ctxt().outer_expn_data()
175+
&& let outermost_expn_data = outermost_expn_data(expr_expn_data)
176+
&& let Some(macro_def_id) = outermost_expn_data.macro_def_id
177+
&& is_format_macro(cx, macro_def_id)
178+
&& let ExpnKind::Macro(_, name) = outermost_expn_data.kind
179+
{
180+
for arg in &format_args.args {
181+
check_unused_format_specifier(cx, arg);
182+
if !arg.format.is_default() {
183+
continue;
152184
}
153-
if meets_msrv(self.msrv, msrvs::FORMAT_ARGS_CAPTURE) {
154-
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id);
185+
if is_aliased(&format_args, arg.param.value.hir_id) {
186+
continue;
155187
}
188+
check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg.param.value);
189+
check_to_string_in_format_args(cx, name, arg.param.value);
190+
}
191+
if meets_msrv(self.msrv, msrvs::FORMAT_ARGS_CAPTURE) {
192+
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id);
156193
}
157194
}
158195
}
159196

160197
extract_msrv_attr!(LateContext);
161198
}
162199

200+
fn check_unused_format_specifier(cx: &LateContext<'_>, arg: &FormatArg<'_>) {
201+
let param_ty = cx.typeck_results().expr_ty(arg.param.value).peel_refs();
202+
203+
if let Count::Implied(Some(mut span)) = arg.format.precision
204+
&& !span.is_empty()
205+
{
206+
span_lint_and_then(
207+
cx,
208+
UNUSED_FORMAT_SPECS,
209+
span,
210+
"empty precision specifier has no effect",
211+
|diag| {
212+
if param_ty.is_floating_point() {
213+
diag.note("a precision specifier is not required to format floats");
214+
}
215+
216+
if arg.format.is_default() {
217+
// If there's no other specifiers remove the `:` too
218+
span = arg.format_span();
219+
}
220+
221+
diag.span_suggestion_verbose(span, "remove the `.`", "", Applicability::MachineApplicable);
222+
},
223+
);
224+
}
225+
226+
if is_type_diagnostic_item(cx, param_ty, sym::Arguments) && !arg.format.is_default_for_trait() {
227+
span_lint_and_then(
228+
cx,
229+
UNUSED_FORMAT_SPECS,
230+
arg.span,
231+
"format specifiers have no effect on `format_args!()`",
232+
|diag| {
233+
let mut suggest_format = |spec, span| {
234+
let message = format!("for the {spec} to apply consider using `format!()`");
235+
236+
if let Some(mac_call) = root_macro_call(arg.param.value.span)
237+
&& cx.tcx.is_diagnostic_item(sym::format_args_macro, mac_call.def_id)
238+
&& arg.span.eq_ctxt(mac_call.span)
239+
{
240+
diag.span_suggestion(
241+
cx.sess().source_map().span_until_char(mac_call.span, '!'),
242+
message,
243+
"format",
244+
Applicability::MaybeIncorrect,
245+
);
246+
} else if let Some(span) = span {
247+
diag.span_help(span, message);
248+
}
249+
};
250+
251+
if !arg.format.width.is_implied() {
252+
suggest_format("width", arg.format.width.span());
253+
}
254+
255+
if !arg.format.precision.is_implied() {
256+
suggest_format("precision", arg.format.precision.span());
257+
}
258+
259+
diag.span_suggestion_verbose(
260+
arg.format_span(),
261+
"if the current behavior is intentional, remove the format specifiers",
262+
"",
263+
Applicability::MaybeIncorrect,
264+
);
265+
},
266+
);
267+
}
268+
}
269+
163270
fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_site: Span, def_id: DefId) {
164271
if args.format_string.span.from_expansion() {
165272
return;

‎clippy_lints/src/lib.register_all.rs

+1
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
7272
LintId::of(format::USELESS_FORMAT),
7373
LintId::of(format_args::FORMAT_IN_FORMAT_ARGS),
7474
LintId::of(format_args::TO_STRING_IN_FORMAT_ARGS),
75+
LintId::of(format_args::UNUSED_FORMAT_SPECS),
7576
LintId::of(format_impl::PRINT_IN_FORMAT_IMPL),
7677
LintId::of(format_impl::RECURSIVE_FORMAT_IMPL),
7778
LintId::of(formatting::POSSIBLE_MISSING_COMMA),

‎clippy_lints/src/lib.register_complexity.rs

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
1313
LintId::of(double_parens::DOUBLE_PARENS),
1414
LintId::of(explicit_write::EXPLICIT_WRITE),
1515
LintId::of(format::USELESS_FORMAT),
16+
LintId::of(format_args::UNUSED_FORMAT_SPECS),
1617
LintId::of(functions::TOO_MANY_ARGUMENTS),
1718
LintId::of(int_plus_one::INT_PLUS_ONE),
1819
LintId::of(lifetimes::EXTRA_UNUSED_LIFETIMES),

‎clippy_lints/src/lib.register_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ store.register_lints(&[
164164
format_args::FORMAT_IN_FORMAT_ARGS,
165165
format_args::TO_STRING_IN_FORMAT_ARGS,
166166
format_args::UNINLINED_FORMAT_ARGS,
167+
format_args::UNUSED_FORMAT_SPECS,
167168
format_impl::PRINT_IN_FORMAT_IMPL,
168169
format_impl::RECURSIVE_FORMAT_IMPL,
169170
format_push_string::FORMAT_PUSH_STRING,

‎clippy_utils/src/macros.rs

+35-6
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,7 @@ pub enum Count<'tcx> {
627627
/// `FormatParamKind::Numbered`.
628628
Param(FormatParam<'tcx>),
629629
/// Not specified.
630-
Implied,
630+
Implied(Option<Span>),
631631
}
632632

633633
impl<'tcx> Count<'tcx> {
@@ -638,8 +638,10 @@ impl<'tcx> Count<'tcx> {
638638
inner: Option<rpf::InnerSpan>,
639639
values: &FormatArgsValues<'tcx>,
640640
) -> Option<Self> {
641+
let span = inner.map(|inner| span_from_inner(values.format_string_span, inner));
642+
641643
Some(match count {
642-
rpf::Count::CountIs(val) => Self::Is(val, span_from_inner(values.format_string_span, inner?)),
644+
rpf::Count::CountIs(val) => Self::Is(val, span?),
643645
rpf::Count::CountIsName(name, _) => Self::Param(FormatParam::new(
644646
FormatParamKind::Named(Symbol::intern(name)),
645647
usage,
@@ -661,12 +663,12 @@ impl<'tcx> Count<'tcx> {
661663
inner?,
662664
values,
663665
)?),
664-
rpf::Count::CountImplied => Self::Implied,
666+
rpf::Count::CountImplied => Self::Implied(span),
665667
})
666668
}
667669

668670
pub fn is_implied(self) -> bool {
669-
matches!(self, Count::Implied)
671+
matches!(self, Count::Implied(_))
670672
}
671673

672674
pub fn param(self) -> Option<FormatParam<'tcx>> {
@@ -675,6 +677,14 @@ impl<'tcx> Count<'tcx> {
675677
_ => None,
676678
}
677679
}
680+
681+
pub fn span(self) -> Option<Span> {
682+
match self {
683+
Count::Is(_, span) => Some(span),
684+
Count::Param(param) => Some(param.span),
685+
Count::Implied(span) => span,
686+
}
687+
}
678688
}
679689

680690
/// Specification for the formatting of an argument in the format string. See
@@ -738,8 +748,13 @@ impl<'tcx> FormatSpec<'tcx> {
738748
/// Returns true if this format spec is unchanged from the default. e.g. returns true for `{}`,
739749
/// `{foo}` and `{2}`, but false for `{:?}`, `{foo:5}` and `{3:.5}`
740750
pub fn is_default(&self) -> bool {
741-
self.r#trait == sym::Display
742-
&& self.width.is_implied()
751+
self.r#trait == sym::Display && self.is_default_for_trait()
752+
}
753+
754+
/// Has no other formatting specifiers than setting the format trait. returns true for `{}`,
755+
/// `{foo}`, `{:?}`, but false for `{foo:5}`, `{3:.5?}`
756+
pub fn is_default_for_trait(&self) -> bool {
757+
self.width.is_implied()
743758
&& self.precision.is_implied()
744759
&& self.align == Alignment::AlignUnknown
745760
&& self.flags == 0
@@ -757,6 +772,20 @@ pub struct FormatArg<'tcx> {
757772
pub span: Span,
758773
}
759774

775+
impl<'tcx> FormatArg<'tcx> {
776+
/// Span of the `:` and format specifiers
777+
///
778+
/// ```ignore
779+
/// format!("{:.}"), format!("{foo:.}")
780+
/// ^^ ^^
781+
/// ```
782+
pub fn format_span(&self) -> Span {
783+
let base = self.span.data();
784+
785+
Span::new(self.param.span.hi(), base.hi - BytePos(1), base.ctxt, base.parent)
786+
}
787+
}
788+
760789
/// A parsed `format_args!` expansion.
761790
#[derive(Debug)]
762791
pub struct FormatArgsExpn<'tcx> {

‎src/docs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,7 @@ docs! {
557557
"unseparated_literal_suffix",
558558
"unsound_collection_transmute",
559559
"unused_async",
560+
"unused_format_specs",
560561
"unused_io_amount",
561562
"unused_peekable",
562563
"unused_rounding",

‎src/docs/unused_format_specs.txt

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
### What it does
2+
Detects [formatting parameters] that have no effect on the output of a
3+
`format!()`, `println!()` or similar macro.
4+
5+
### Why is this bad?
6+
Shorter format specifiers are easier to read, it may also indicate that
7+
an expected formatting operation such as adding padding isn't happening.
8+
9+
### Example
10+
```
11+
println!("{:.}", 1.0);
12+
13+
println!("not padded: {:5}", format_args!("..."));
14+
```
15+
Use instead:
16+
```
17+
println!("{}", 1.0);
18+
19+
println!("not padded: {}", format_args!("..."));
20+
// OR
21+
println!("padded: {:5}", format!("..."));
22+
```
23+
24+
[formatting parameters]: https://doc.rust-lang.org/std/fmt/index.html#formatting-parameters

‎tests/ui/unused_format_specs.fixed

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::unused_format_specs)]
4+
#![allow(unused)]
5+
6+
fn main() {
7+
let f = 1.0f64;
8+
println!("{}", 1.0);
9+
println!("{f} {f:?}");
10+
11+
println!("{}", 1);
12+
}
13+
14+
fn should_not_lint() {
15+
let f = 1.0f64;
16+
println!("{:.1}", 1.0);
17+
println!("{f:.w$} {f:.*?}", 3, w = 2);
18+
}

‎tests/ui/unused_format_specs.rs

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::unused_format_specs)]
4+
#![allow(unused)]
5+
6+
fn main() {
7+
let f = 1.0f64;
8+
println!("{:.}", 1.0);
9+
println!("{f:.} {f:.?}");
10+
11+
println!("{:.}", 1);
12+
}
13+
14+
fn should_not_lint() {
15+
let f = 1.0f64;
16+
println!("{:.1}", 1.0);
17+
println!("{f:.w$} {f:.*?}", 3, w = 2);
18+
}

0 commit comments

Comments
 (0)