From ce060e97eacb62bb2848e67597ae9a5b54acd1a6 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Thu, 21 Jul 2022 19:14:07 -0400 Subject: [PATCH] [WIP] Implement captured vars in format!() --- CHANGELOG.md | 3 +- clippy_lints/src/inline_format_args.rs | 148 ++++++++++ clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_nursery.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/write.rs | 13 +- clippy_utils/src/macros.rs | 60 ++-- clippy_utils/src/source.rs | 10 + tests/ui/inline_format_args.fixed | 100 +++++++ tests/ui/inline_format_args.rs | 101 +++++++ tests/ui/inline_format_args.stderr | 351 +++++++++++++++++++++++ 11 files changed, 756 insertions(+), 34 deletions(-) create mode 100644 clippy_lints/src/inline_format_args.rs create mode 100644 tests/ui/inline_format_args.fixed create mode 100644 tests/ui/inline_format_args.rs create mode 100644 tests/ui/inline_format_args.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d5c216c154a..ed031ed307f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3570,8 +3570,8 @@ Released 2018-09-13 [`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from [`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default [`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file -[`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map [`filter_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_identity +[`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map [`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next [`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next [`find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#find_map @@ -3631,6 +3631,7 @@ Released 2018-09-13 [`inline_asm_x86_att_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_att_syntax [`inline_asm_x86_intel_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_intel_syntax [`inline_fn_without_body`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_fn_without_body +[`inline_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_format_args [`inspect_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#inspect_for_each [`int_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#int_plus_one [`integer_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_arithmetic diff --git a/clippy_lints/src/inline_format_args.rs b/clippy_lints/src/inline_format_args.rs new file mode 100644 index 000000000000..c74eb31790e5 --- /dev/null +++ b/clippy_lints/src/inline_format_args.rs @@ -0,0 +1,148 @@ +// #![allow(unused_imports)] +// #![allow(unused_variables)] + +// use std::fs::{create_dir_all, OpenOptions}; +// use std::io::Write; + +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::macros::{FormatArgsArg, FormatArgsExpn, is_format_macro, root_macro_call_first_node}; +use clippy_utils::source::{expand_past_previous_comma, snippet}; +use if_chain::if_chain; + +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, Path, QPath}; +use rustc_hir::def::Res; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::ExpnData; + +declare_clippy_lint! { + /// ### What it does + /// + /// ### Why is this bad? + /// + /// ### Example + /// ```rust + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```rust + /// // example code which does not raise clippy warning + /// ``` + #[clippy::version = "1.64.0"] + pub INLINE_FORMAT_ARGS, + nursery, + "default lint description" +} +declare_lint_pass!(InlineFormatArgs => [INLINE_FORMAT_ARGS]); + +impl<'tcx> LateLintPass<'tcx> for InlineFormatArgs { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if_chain! { + // TODO: are all these needed? They wre mostly copy/pasted from other places. + if let Some(outer_macro) = root_macro_call_first_node(cx, expr); + if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, outer_macro.expn); + if !format_args.format_string_span.from_expansion(); + let expr_expn_data = expr.span.ctxt().outer_expn_data(); + let outermost_expn_data = outermost_expn_data(expr_expn_data); + if let Some(macro_def_id) = outermost_expn_data.macro_def_id; + if is_format_macro(cx, macro_def_id); + // TODO: do we need this? + // if let rustc_span::ExpnKind::Macro(_, name) = outermost_expn_data.kind; + if let Some(args) = format_args.args(cx); + if !args.is_empty(); + then { + let mut changes = None; + for (i, arg) in args.iter().enumerate() { + if_chain! { + // If this condition is expensive, may want to move it to the end of this if chain? + if arg.argument_span.is_empty() || snippet(cx, arg.argument_span, "").trim_end().is_empty(); + if let ExprKind::Path(QPath::Resolved(None, Path { span, res, segments })) = arg.value.kind; + if segments.len() == 1; + // TODO: do we need this? + if let Res::Local(_local_id) = res; + if !is_aliased(&args, i); + then { + let c = changes.get_or_insert_with(|| vec![]); + // TODO: is it ok to assume segments.len() == 1? + // if not, could use this instead: + // let var_name = snippet(cx, *span, "").trim_end(); + // if var_name.is_empty() { continue; } + let var_name = segments[0].ident.name; + c.push((arg.argument_span, var_name.to_string())); + let arg_span = expand_past_previous_comma(cx, *span); + c.push((arg_span, "".to_string())); + } + } + } + + if let Some(changes) = changes { + span_lint_and_then( + cx, + INLINE_FORMAT_ARGS, + outer_macro.span, + "REPLACE ME", + |diag| { + diag.multipart_suggestion( + &format!("some interesting message"), + changes, + Applicability::MachineApplicable, + ); + }, + ); + } + +// let dumps_dir = "expected"; +// create_dir_all(dumps_dir).unwrap(); +// let full_str = snippet(cx, outer_macro.span, "panic").to_string() +// .replace("/","--") +// .replace("\t","[TAB]") +// .replace("\n","[NL]") +// .replace("\\","--"); +// let filename = format!("{dumps_dir}/{full_str}.txt"); +// let mut file = OpenOptions::new() +// .write(true) +// .create(true) +// .truncate(true) +// .open(&filename) +// .unwrap(); +// writeln!(file, "NAME {name:?}").unwrap(); +// writeln!(file, "FMT {macro_def_id:?}").unwrap(); +// for (i, part) in format_args.format_string_parts.iter().enumerate() { +// writeln!(file, "FMT_PART {i} = {:?}", part).unwrap(); +// } +// for (i, part) in format_args.formatters.iter().enumerate() { +// writeln!(file, "FORMATTERS {i} = {:?}", part).unwrap(); +// } +// for (i, part) in format_args.specs.iter().enumerate() { +// writeln!(file, "SPECS {i} = {:#?}", part).unwrap(); +// } +// for (i, arg) in args.iter().enumerate() { +// writeln!(file, "#{i} = TRAIT={:?}, HAS_STRFMT={:?}, ALIAS={:?}, HAS_PRIM_FMT={:?}, SPAN={:?}, ARG_SPAN={:?}, SPEC={:#?}\nVALUE={:#?}", +// arg.format_trait, arg.has_string_formatting(), is_aliased(&args, i), arg.has_primitive_formatting(), +// snippet(cx, arg.span, "<<<..>>>"), +// snippet(cx, arg.argument_span, "<<<..>>>"), +// arg.spec, arg.value).unwrap(); +// } + } + } + } +} + +// TODO: if this is a common pattern, should this be in utils? +fn outermost_expn_data(expn_data: ExpnData) -> ExpnData { + if expn_data.call_site.from_expansion() { + outermost_expn_data(expn_data.call_site.ctxt().outer_expn_data()) + } else { + expn_data + } +} + +// Returns true if `args[i]` "refers to" or "is referred to by" another argument. +// TODO: this is not catching cases when the value is (also) used as precision or width +fn is_aliased(args: &[FormatArgsArg<'_>], i: usize) -> bool { + let value = args[i].value; + args.iter() + .enumerate() + .any(|(j, arg)| i != j && std::ptr::eq(value, arg.value)) +} diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index d40c6aec04ae..a6ef08e9b168 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -194,6 +194,7 @@ store.register_lints(&[ inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY, init_numbered_fields::INIT_NUMBERED_FIELDS, inline_fn_without_body::INLINE_FN_WITHOUT_BODY, + inline_format_args::INLINE_FORMAT_ARGS, int_plus_one::INT_PLUS_ONE, invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS, invalid_utf8_in_unchecked::INVALID_UTF8_IN_UNCHECKED, diff --git a/clippy_lints/src/lib.register_nursery.rs b/clippy_lints/src/lib.register_nursery.rs index f961952991f5..c48f08908850 100644 --- a/clippy_lints/src/lib.register_nursery.rs +++ b/clippy_lints/src/lib.register_nursery.rs @@ -12,6 +12,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![ LintId::of(floating_point_arithmetic::SUBOPTIMAL_FLOPS), LintId::of(future_not_send::FUTURE_NOT_SEND), LintId::of(index_refutable_slice::INDEX_REFUTABLE_SLICE), + LintId::of(inline_format_args::INLINE_FORMAT_ARGS), LintId::of(let_if_seq::USELESS_LET_IF_SEQ), LintId::of(methods::ITER_WITH_DRAIN), LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d60481c335e8..0489fef9cd36 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -252,6 +252,7 @@ mod inherent_impl; mod inherent_to_string; mod init_numbered_fields; mod inline_fn_without_body; +mod inline_format_args; mod int_plus_one; mod invalid_upcast_comparisons; mod invalid_utf8_in_unchecked; @@ -916,6 +917,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(operators::Operators::new(verbose_bit_mask_threshold))); store.register_late_pass(|| Box::new(invalid_utf8_in_unchecked::InvalidUtf8InUnchecked)); store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports)); + store.register_late_pass(|| Box::new(inline_format_args::InlineFormatArgs)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 0c73daeda7b1..bfee855b00fa 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -1,12 +1,12 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; use clippy_utils::macros::{root_macro_call_first_node, FormatArgsArg, FormatArgsExpn, MacroCall}; -use clippy_utils::source::snippet_opt; +use clippy_utils::source::{snippet_opt, expand_past_previous_comma}; 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::{sym, BytePos, Span}; +use rustc_span::{sym, BytePos}; declare_clippy_lint! { /// ### What it does @@ -363,8 +363,8 @@ fn check_newline(cx: &LateContext<'_>, format_args: &FormatArgsExpn<'_>, macro_c &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 }; + 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") @@ -546,10 +546,3 @@ fn conservative_unescape(literal: &str) -> Result { 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)) -} diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs index 1e5f8b510ea6..893b6e3fa7fe 100644 --- a/clippy_utils/src/macros.rs +++ b/clippy_utils/src/macros.rs @@ -68,7 +68,7 @@ impl MacroCall { } /// Returns an iterator of expansions that created the given span -pub fn expn_backtrace(mut span: Span) -> impl Iterator { +pub fn expn_backtrace(mut span: Span) -> impl Iterator { iter::from_fn(move || { let ctxt = span.ctxt(); if ctxt == SyntaxContext::root() { @@ -101,7 +101,7 @@ pub fn expn_is_local(expn: ExpnId) -> bool { /// Returns an iterator of macro expansions that created the given span. /// Note that desugaring expansions are skipped. -pub fn macro_backtrace(span: Span) -> impl Iterator { +pub fn macro_backtrace(span: Span) -> impl Iterator { expn_backtrace(span).filter_map(|(expn, data)| match data { ExpnData { kind: ExpnKind::Macro(kind, _), @@ -135,7 +135,7 @@ pub fn root_macro_call_first_node(cx: &LateContext<'_>, node: &impl HirNode) -> /// Like [`macro_backtrace`], but only returns macro calls where `node` is the "first node" of the /// macro call, as in [`first_node_in_macro`]. -pub fn first_node_macro_backtrace(cx: &LateContext<'_>, node: &impl HirNode) -> impl Iterator { +pub fn first_node_macro_backtrace(cx: &LateContext<'_>, node: &impl HirNode) -> impl Iterator { let span = node.span(); first_node_in_macro(cx, node) .into_iter() @@ -194,7 +194,7 @@ pub fn first_node_in_macro(cx: &LateContext<'_>, node: &impl HirNode) -> Option< /// Is `def_id` of `std::panic`, `core::panic` or any inner implementation macros pub fn is_panic(cx: &LateContext<'_>, def_id: DefId) -> bool { - let Some(name) = cx.tcx.get_diagnostic_name(def_id) else { return false }; + let Some(name) = cx.tcx.get_diagnostic_name(def_id) else { return false; }; matches!( name.as_str(), "core_panic_macro" @@ -221,15 +221,15 @@ impl<'a> PanicExpn<'a> { if !macro_backtrace(expr.span).any(|macro_call| is_panic(cx, macro_call.def_id)) { return None; } - let ExprKind::Call(callee, [arg]) = &expr.kind else { return None }; - let ExprKind::Path(QPath::Resolved(_, path)) = &callee.kind else { return None }; + let ExprKind::Call(callee, [arg]) = &expr.kind else { return None; }; + let ExprKind::Path(QPath::Resolved(_, path)) = &callee.kind else { return None; }; let result = match path.segments.last().unwrap().ident.as_str() { "panic" if arg.span.ctxt() == expr.span.ctxt() => Self::Empty, "panic" | "panic_str" => Self::Str(arg), "panic_display" => { - let ExprKind::AddrOf(_, _, e) = &arg.kind else { return None }; + let ExprKind::AddrOf(_, _, e) = &arg.kind else { return None; }; Self::Display(e) - }, + } "panic_fmt" => Self::Format(FormatArgsExpn::parse(cx, arg)?), _ => return None, }; @@ -281,7 +281,7 @@ fn find_assert_args_inner<'a, const N: usize>( true } }) - .visit_expr(expr); + .visit_expr(expr); let args = args.into_inner().ok()?; // if no `panic!(..)` is found, use `PanicExpn::Empty` // to indicate that the default assertion message is used @@ -309,7 +309,7 @@ fn find_assert_within_debug_assert<'a>( } false }) - .visit_expr(expr); + .visit_expr(expr); found } @@ -431,7 +431,7 @@ impl<'tcx> FormatArgsExpn<'tcx> { } true }) - .visit_expr(e); + .visit_expr(e); } else { // assume that any further exprs with a differing context are value args value_args.push(e); @@ -439,7 +439,7 @@ impl<'tcx> FormatArgsExpn<'tcx> { // don't walk anything not from the macro expansion (e.a. inputs) false }) - .visit_expr(expr); + .visit_expr(expr); Some(FormatArgsExpn { format_string_span: format_string_span?, format_string_parts, @@ -465,7 +465,7 @@ impl<'tcx> FormatArgsExpn<'tcx> { } false }) - .visit_expr(expr); + .visit_expr(expr); format_args } @@ -479,8 +479,9 @@ impl<'tcx> FormatArgsExpn<'tcx> { .map(|((value, &(_, format_trait)), span)| FormatArgsArg { value, format_trait, - span, spec: None, + span: span.0, + argument_span: span.1, }) .collect(); return Some(args); @@ -500,8 +501,9 @@ impl<'tcx> FormatArgsExpn<'tcx> { Some(FormatArgsArg { value: self.value_args[j], format_trait, - span, spec: Some(spec), + span: span.0, + argument_span: span.1, }) } else { None @@ -522,7 +524,7 @@ impl<'tcx> FormatArgsExpn<'tcx> { } /// Returns the spans covering the `{}`s in the format string - fn format_argument_spans(&self, cx: &LateContext<'tcx>) -> Option> { + fn format_argument_spans(&self, cx: &LateContext<'tcx>) -> Option> { let format_string = snippet_opt(cx, self.format_string_span)?.into_bytes(); let lo = self.format_string_span.lo(); let get = |i| format_string.get(i).copied(); @@ -530,19 +532,29 @@ impl<'tcx> FormatArgsExpn<'tcx> { let mut spans = Vec::new(); let mut i = 0; let mut arg_start = 0; + let mut spec_start = 0; loop { match (get(i), get(i + 1)) { (Some(b'{'), Some(b'{')) | (Some(b'}'), Some(b'}')) => { i += 1; - }, + } (Some(b'{'), _) => arg_start = i, - (Some(b'}'), _) => spans.push( - self.format_string_span - .with_lo(lo + BytePos::from_usize(arg_start)) - .with_hi(lo + BytePos::from_usize(i + 1)), - ), + (Some(b':'), _) => spec_start = i, + (Some(b'}'), _) => { + // If ':' was found for the current argument, spec_start will be > arg_start. + // Note that argument_span may be empty. + let arg_without_spec = if spec_start > arg_start { spec_start } else { i }; + spans.push(( + self.format_string_span + .with_lo(lo + BytePos::from_usize(arg_start)) + .with_hi(lo + BytePos::from_usize(i + 1)), + self.format_string_span + .with_lo(lo + BytePos::from_usize(arg_start + 1)) + .with_hi(lo + BytePos::from_usize(arg_without_spec)), + )); + } (None, _) => break, - _ => {}, + _ => {} } i += 1; @@ -562,6 +574,8 @@ pub struct FormatArgsArg<'tcx> { pub spec: Option<&'tcx Expr<'tcx>>, /// Span of the `{}` pub span: Span, + /// Span of the "argument" part of the `{arg:...}`. Can be empty. + pub argument_span: Span, } impl<'tcx> FormatArgsArg<'tcx> { diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index 1197fe914de4..b0a0641313c7 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -410,6 +410,16 @@ pub fn trim_span(sm: &SourceMap, span: Span) -> Span { .span() } +/// Expand a span to include a preceding comma +/// ```rust,ignore +/// writeln!(o, "") -> writeln!(o, "") +/// ^^ ^^^^ +/// ``` +pub 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)) +} + #[cfg(test)] mod test { use super::{reindent_multiline, without_block_comments}; diff --git a/tests/ui/inline_format_args.fixed b/tests/ui/inline_format_args.fixed new file mode 100644 index 000000000000..fcb22927b673 --- /dev/null +++ b/tests/ui/inline_format_args.fixed @@ -0,0 +1,100 @@ +// run-rustfix + +#![allow(clippy::eq_op)] +#![allow(clippy::format_in_format_args)] +#![allow(clippy::print_literal)] +#![allow(named_arguments_used_positionally)] +#![allow(unused_variables)] +#![warn(clippy::inline_format_args)] + +fn tester(fn_arg: i32) { + let local_value = 1; + let local_value2 = 2.0; + let local_opt:Option = Some(3); + let width = 4; + let prec = 5; + let val = 6; + + // make sure this file hasn't been corrupted with tabs converted to spaces + assert_eq!(" ", "\t \t"); + + println!("val='{local_value}'"); + println!("val='{local_value}'"); + println!("val='{local_value}'"); + println!("val='{local_value}'"); + println!("val='{local_value}'"); + println!("val='{\t }'", local_value); + println!("val='{\n }'", local_value); + println!("val='{local_value}'"); + println!("val='{local_value}'", local_value=local_value); + println!("val='{local_value}'", local_value=fn_arg); + println!("{local_value}"); + println!("{fn_arg}"); + println!("{local_value:?}"); + println!("{local_value:#?}"); + println!("{local_value:4}"); + println!("{local_value:04}"); + println!("{local_value:<3}"); + println!("{local_value:#010x}"); + println!("{local_value2:.1}"); + println!("Hello {} is {local_value2:.*}", "x", local_value); + println!("{0}", local_value); + println!("{0:?}", local_value); + println!("{0:#?}", local_value); + println!("{0:04}", local_value); + println!("{0:<3}", local_value); + println!("{0:#010x}", local_value); + println!("{0:.1}", local_value2); + println!("{0} {0}", local_value); + println!("{local_value} {local_value2}"); + println!("{1} {} {0} {}", local_value, local_value2); + println!("{0} {1}", local_value, local_value2); + println!("{1} {0}", local_value, local_value2); + println!("{1} {0} {1} {0}", local_value, local_value2); + println!("{}", local_opt.unwrap()); + println!("{local_value}, {}", local_opt.unwrap()); + println!("{1} {0}", "str", local_value); + println!("{v}", v = local_value); + println!("{local_value:0$}", width); + println!("{local_value:w$}", w = width); + println!("{local_value:.0$}", prec); + println!("{local_value:.p$}", p = prec); + println!("{width:0$}"); + println!("{val}"); + println!("{val}"); + println!("{val:0$}"); + println!("{0:0$}", v = val); + println!("{val:0$.0$}"); + println!("{0:0$.0$}", v = val); + println!("{0:0$.v$}", v = val); + println!("{0:v$.0$}", v = val); + println!("{v:0$.0$}", v = val); + println!("{v:v$.0$}", v = val); + println!("{v:0$.v$}", v = val); + println!("{v:v$.v$}", v = val); + println!("{width:w$}"); + println!("{prec:.0$}"); + println!("{prec:.p$}"); + println!("{width:0$.1$}", prec); + println!("{width:0$.w$}", w = prec); + println!("{local_value2:1$.2$}", width, prec); + // These cases crash rustc, but they were fixed in a recent nightly + // https://github.com/rust-lang/rust/issues/99633 + // TODO: uncomment these once clippy switches to the 1.63+ (64+?) + // println!("{:p$.w$}", local_value, w = width, p = prec); + // println!("{:p$.w$}", p = width, w = prec); + println!("{}", format!("{}", local_value)); + + // Should not be changed + println!(concat!("nope ", "{}"), local_value); + println!("val='{local_value}'"); + println!("val='{local_value }'"); + println!("val='{local_value }'"); // with tab + println!("val='{local_value\n}'"); + println!("val='{local_value + }'"); +} + +fn main() { + tester(42); +} diff --git a/tests/ui/inline_format_args.rs b/tests/ui/inline_format_args.rs new file mode 100644 index 000000000000..3138b5be1939 --- /dev/null +++ b/tests/ui/inline_format_args.rs @@ -0,0 +1,101 @@ +// run-rustfix + +#![allow(clippy::eq_op)] +#![allow(clippy::format_in_format_args)] +#![allow(clippy::print_literal)] +#![allow(named_arguments_used_positionally)] +#![allow(unused_variables)] +#![warn(clippy::inline_format_args)] + +fn tester(fn_arg: i32) { + let local_value = 1; + let local_value2 = 2.0; + let local_opt:Option = Some(3); + let width = 4; + let prec = 5; + let val = 6; + + // make sure this file hasn't been corrupted with tabs converted to spaces + assert_eq!(" ", "\t \t"); + + println!("val='{}'", local_value); + println!("val='{ }'", local_value); + println!("val='{ }'", local_value); + println!("val='{ }'", local_value); + println!("val='{ }'", local_value); + println!("val='{\t }'", local_value); + println!("val='{\n }'", local_value); + println!("val='{ + }'", local_value); + println!("val='{local_value}'", local_value=local_value); + println!("val='{local_value}'", local_value=fn_arg); + println!("{}", local_value); + println!("{}", fn_arg); + println!("{:?}", local_value); + println!("{:#?}", local_value); + println!("{:4}", local_value); + println!("{:04}", local_value); + println!("{:<3}", local_value); + println!("{:#010x}", local_value); + println!("{:.1}", local_value2); + println!("Hello {} is {:.*}", "x", local_value, local_value2); + println!("{0}", local_value); + println!("{0:?}", local_value); + println!("{0:#?}", local_value); + println!("{0:04}", local_value); + println!("{0:<3}", local_value); + println!("{0:#010x}", local_value); + println!("{0:.1}", local_value2); + println!("{0} {0}", local_value); + println!("{} {}", local_value, local_value2); + println!("{1} {} {0} {}", local_value, local_value2); + println!("{0} {1}", local_value, local_value2); + println!("{1} {0}", local_value, local_value2); + println!("{1} {0} {1} {0}", local_value, local_value2); + println!("{}", local_opt.unwrap()); + println!("{}, {}", local_value, local_opt.unwrap()); + println!("{1} {0}", "str", local_value); + println!("{v}", v = local_value); + println!("{local_value:0$}", width); + println!("{local_value:w$}", w = width); + println!("{local_value:.0$}", prec); + println!("{local_value:.p$}", p = prec); + println!("{:0$}", width); + println!("{}", val); + println!("{}", v = val); + println!("{:0$}", v = val); + println!("{0:0$}", v = val); + println!("{:0$.0$}", v = val); + println!("{0:0$.0$}", v = val); + println!("{0:0$.v$}", v = val); + println!("{0:v$.0$}", v = val); + println!("{v:0$.0$}", v = val); + println!("{v:v$.0$}", v = val); + println!("{v:0$.v$}", v = val); + println!("{v:v$.v$}", v = val); + println!("{:w$}", w = width); + println!("{:.0$}", prec); + println!("{:.p$}", p = prec); + println!("{:0$.1$}", width, prec); + println!("{:0$.w$}", width, w = prec); + println!("{:1$.2$}", local_value2, width, prec); + // These cases crash rustc, but they were fixed in a recent nightly + // https://github.com/rust-lang/rust/issues/99633 + // TODO: uncomment these once clippy switches to the 1.63+ (64+?) + // println!("{:p$.w$}", local_value, w = width, p = prec); + // println!("{:p$.w$}", p = width, w = prec); + println!("{}", format!("{}", local_value)); + + // Should not be changed + println!(concat!("nope ", "{}"), local_value); + println!("val='{local_value}'"); + println!("val='{local_value }'"); + println!("val='{local_value }'"); // with tab + println!("val='{local_value\n}'"); + println!("val='{local_value + }'"); +} + +fn main() { + tester(42); +} diff --git a/tests/ui/inline_format_args.stderr b/tests/ui/inline_format_args.stderr new file mode 100644 index 000000000000..74f05d31b0b7 --- /dev/null +++ b/tests/ui/inline_format_args.stderr @@ -0,0 +1,351 @@ +error: REPLACE ME + --> $DIR/inline_format_args.rs:21:5 + | +LL | println!("val='{}'", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::inline-format-args` implied by `-D warnings` +help: some interesting message + | +LL - println!("val='{}'", local_value); +LL + println!("val='{local_value}'"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:22:5 + | +LL | println!("val='{ }'", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("val='{ }'", local_value); +LL + println!("val='{local_value}'"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:23:5 + | +LL | println!("val='{ }'", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("val='{ }'", local_value); +LL + println!("val='{local_value}'"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:24:5 + | +LL | println!("val='{ }'", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("val='{ }'", local_value); +LL + println!("val='{local_value}'"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:25:5 + | +LL | println!("val='{ }'", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("val='{ }'", local_value); +LL + println!("val='{local_value}'"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:28:5 + | +LL | / println!("val='{ +LL | | }'", local_value); + | |_____________________^ + | +help: some interesting message + | +LL - println!("val='{ +LL + println!("val='{local_value}'"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:32:5 + | +LL | println!("{}", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{}", local_value); +LL + println!("{local_value}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:33:5 + | +LL | println!("{}", fn_arg); + | ^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{}", fn_arg); +LL + println!("{fn_arg}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:34:5 + | +LL | println!("{:?}", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:?}", local_value); +LL + println!("{local_value:?}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:35:5 + | +LL | println!("{:#?}", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:#?}", local_value); +LL + println!("{local_value:#?}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:36:5 + | +LL | println!("{:4}", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:4}", local_value); +LL + println!("{local_value:4}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:37:5 + | +LL | println!("{:04}", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:04}", local_value); +LL + println!("{local_value:04}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:38:5 + | +LL | println!("{:<3}", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:<3}", local_value); +LL + println!("{local_value:<3}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:39:5 + | +LL | println!("{:#010x}", local_value); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:#010x}", local_value); +LL + println!("{local_value:#010x}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:40:5 + | +LL | println!("{:.1}", local_value2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:.1}", local_value2); +LL + println!("{local_value2:.1}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:41:5 + | +LL | println!("Hello {} is {:.*}", "x", local_value, local_value2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("Hello {} is {:.*}", "x", local_value, local_value2); +LL + println!("Hello {} is {local_value2:.*}", "x", local_value); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:50:5 + | +LL | println!("{} {}", local_value, local_value2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{} {}", local_value, local_value2); +LL + println!("{local_value} {local_value2}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:56:5 + | +LL | println!("{}, {}", local_value, local_opt.unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{}, {}", local_value, local_opt.unwrap()); +LL + println!("{local_value}, {}", local_opt.unwrap()); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:63:5 + | +LL | println!("{:0$}", width); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:0$}", width); +LL + println!("{width:0$}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:64:5 + | +LL | println!("{}", val); + | ^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{}", val); +LL + println!("{val}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:65:5 + | +LL | println!("{}", v = val); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{}", v = val); +LL + println!("{val}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:66:5 + | +LL | println!("{:0$}", v = val); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:0$}", v = val); +LL + println!("{val:0$}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:68:5 + | +LL | println!("{:0$.0$}", v = val); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:0$.0$}", v = val); +LL + println!("{val:0$.0$}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:76:5 + | +LL | println!("{:w$}", w = width); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:w$}", w = width); +LL + println!("{width:w$}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:77:5 + | +LL | println!("{:.0$}", prec); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:.0$}", prec); +LL + println!("{prec:.0$}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:78:5 + | +LL | println!("{:.p$}", p = prec); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:.p$}", p = prec); +LL + println!("{prec:.p$}"); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:79:5 + | +LL | println!("{:0$.1$}", width, prec); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:0$.1$}", width, prec); +LL + println!("{width:0$.1$}", prec); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:80:5 + | +LL | println!("{:0$.w$}", width, w = prec); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:0$.w$}", width, w = prec); +LL + println!("{width:0$.w$}", w = prec); + | + +error: REPLACE ME + --> $DIR/inline_format_args.rs:81:5 + | +LL | println!("{:1$.2$}", local_value2, width, prec); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: some interesting message + | +LL - println!("{:1$.2$}", local_value2, width, prec); +LL + println!("{local_value2:1$.2$}", width, prec); + | + +error: aborting due to 29 previous errors