new uninlined_format_args lint to inline explicit arguments
Implement https://github.com/rust-lang/rust-clippy/issues/8368 - a new lint to inline format arguments such as `print!("{}", var)` into `print!("{var}")`. code | suggestion | comment ---|---|--- `print!("{}", var)` | `print!("{var}")` | simple variables `print!("{0}", var)` | `print!("{var}")` | positional variables `print!("{v}", v=var)` | `print!("{var}")` | named variables `print!("{0} {0}", var)` | `print!("{var} {var}")` | aliased variables `print!("{0:1$}", var, width)` | `print!("{var:width$}")` | width support `print!("{0:.1$}", var, prec)` | `print!("{var:.prec$}")` | precision support `print!("{:.*}", prec, var)` | `print!("{var:.prec$}")` | asterisk support code | suggestion | comment ---|---|--- `print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format string uses an indexed argument that cannot be inlined. Supporting this case requires re-indexing of the format string. changelog: [`uninlined_format_args`]: A new lint to inline format arguments, i.e. `print!("{}", var)` into `print!("{var}")`
This commit is contained in:
Yuri Astrakhan
parent
57c9daa09b
commit
5a71bbdf3f
15 changed files with 1451 additions and 29 deletions
|
|
@ -1,16 +1,18 @@
|
|||
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
|
||||
use clippy_utils::is_diag_trait_item;
|
||||
use clippy_utils::macros::{is_format_macro, FormatArgsExpn};
|
||||
use clippy_utils::source::snippet_opt;
|
||||
use clippy_utils::macros::FormatParamKind::{Implicit, Named, Numbered, Starred};
|
||||
use clippy_utils::macros::{is_format_macro, FormatArgsExpn, FormatParam, FormatParamUsage};
|
||||
use clippy_utils::source::{expand_past_previous_comma, snippet_opt};
|
||||
use clippy_utils::ty::implements_trait;
|
||||
use clippy_utils::{is_diag_trait_item, meets_msrv, msrvs};
|
||||
use if_chain::if_chain;
|
||||
use itertools::Itertools;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Expr, ExprKind, HirId};
|
||||
use rustc_hir::{Expr, ExprKind, HirId, Path, QPath};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty::adjustment::{Adjust, Adjustment};
|
||||
use rustc_middle::ty::Ty;
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use rustc_semver::RustcVersion;
|
||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||
use rustc_span::{sym, ExpnData, ExpnKind, Span, Symbol};
|
||||
|
||||
declare_clippy_lint! {
|
||||
|
|
@ -64,7 +66,67 @@ declare_clippy_lint! {
|
|||
"`to_string` applied to a type that implements `Display` in format args"
|
||||
}
|
||||
|
||||
declare_lint_pass!(FormatArgs => [FORMAT_IN_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS]);
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Detect when a variable is not inlined in a format string,
|
||||
/// and suggests to inline it.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// Non-inlined code is slightly more difficult to read and understand,
|
||||
/// as it requires arguments to be matched against the format string.
|
||||
/// The inlined syntax, where allowed, is simpler.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```rust
|
||||
/// # let var = 42;
|
||||
/// # let width = 1;
|
||||
/// # let prec = 2;
|
||||
/// format!("{}", var);
|
||||
/// format!("{v:?}", v = var);
|
||||
/// format!("{0} {0}", var);
|
||||
/// format!("{0:1$}", var, width);
|
||||
/// format!("{:.*}", prec, var);
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```rust
|
||||
/// # let var = 42;
|
||||
/// # let width = 1;
|
||||
/// # let prec = 2;
|
||||
/// format!("{var}");
|
||||
/// format!("{var:?}");
|
||||
/// format!("{var} {var}");
|
||||
/// format!("{var:width$}");
|
||||
/// format!("{var:.prec$}");
|
||||
/// ```
|
||||
///
|
||||
/// ### Known Problems
|
||||
///
|
||||
/// There may be a false positive if the format string is expanded from certain proc macros:
|
||||
///
|
||||
/// ```ignore
|
||||
/// println!(indoc!("{}"), var);
|
||||
/// ```
|
||||
///
|
||||
/// If a format string contains a numbered argument that cannot be inlined
|
||||
/// nothing will be suggested, e.g. `println!("{0}={1}", var, 1+2)`.
|
||||
#[clippy::version = "1.65.0"]
|
||||
pub UNINLINED_FORMAT_ARGS,
|
||||
pedantic,
|
||||
"using non-inlined variables in `format!` calls"
|
||||
}
|
||||
|
||||
impl_lint_pass!(FormatArgs => [FORMAT_IN_FORMAT_ARGS, UNINLINED_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS]);
|
||||
|
||||
pub struct FormatArgs {
|
||||
msrv: Option<RustcVersion>,
|
||||
}
|
||||
|
||||
impl FormatArgs {
|
||||
#[must_use]
|
||||
pub fn new(msrv: Option<RustcVersion>) -> Self {
|
||||
Self { msrv }
|
||||
}
|
||||
}
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for FormatArgs {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
|
||||
|
|
@ -86,9 +148,73 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
|
|||
check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg.param.value);
|
||||
check_to_string_in_format_args(cx, name, arg.param.value);
|
||||
}
|
||||
if meets_msrv(self.msrv, msrvs::FORMAT_ARGS_CAPTURE) {
|
||||
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
extract_msrv_attr!(LateContext);
|
||||
}
|
||||
|
||||
fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_site: Span) {
|
||||
if args.format_string.span.from_expansion() {
|
||||
return;
|
||||
}
|
||||
|
||||
let mut fixes = Vec::new();
|
||||
// If any of the arguments are referenced by an index number,
|
||||
// and that argument is not a simple variable and cannot be inlined,
|
||||
// we cannot remove any other arguments in the format string,
|
||||
// because the index numbers might be wrong after inlining.
|
||||
// Example of an un-inlinable format: print!("{}{1}", foo, 2)
|
||||
if !args.params().all(|p| check_one_arg(cx, &p, &mut fixes)) || fixes.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
// FIXME: Properly ignore a rare case where the format string is wrapped in a macro.
|
||||
// Example: `format!(indoc!("{}"), foo);`
|
||||
// If inlined, they will cause a compilation error:
|
||||
// > to avoid ambiguity, `format_args!` cannot capture variables
|
||||
// > when the format string is expanded from a macro
|
||||
// @Alexendoo explanation:
|
||||
// > indoc! is a proc macro that is producing a string literal with its span
|
||||
// > set to its input it's not marked as from expansion, and since it's compatible
|
||||
// > tokenization wise clippy_utils::is_from_proc_macro wouldn't catch it either
|
||||
// This might be a relatively expensive test, so do it only we are ready to replace.
|
||||
// See more examples in tests/ui/uninlined_format_args.rs
|
||||
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
UNINLINED_FORMAT_ARGS,
|
||||
call_site,
|
||||
"variables can be used directly in the `format!` string",
|
||||
|diag| {
|
||||
diag.multipart_suggestion("change this to", fixes, Applicability::MachineApplicable);
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
fn check_one_arg(cx: &LateContext<'_>, param: &FormatParam<'_>, fixes: &mut Vec<(Span, String)>) -> bool {
|
||||
if matches!(param.kind, Implicit | Starred | Named(_) | Numbered)
|
||||
&& let ExprKind::Path(QPath::Resolved(None, path)) = param.value.kind
|
||||
&& let Path { span, segments, .. } = path
|
||||
&& let [segment] = segments
|
||||
{
|
||||
let replacement = match param.usage {
|
||||
FormatParamUsage::Argument => segment.ident.name.to_string(),
|
||||
FormatParamUsage::Width => format!("{}$", segment.ident.name),
|
||||
FormatParamUsage::Precision => format!(".{}$", segment.ident.name),
|
||||
};
|
||||
fixes.push((param.span, replacement));
|
||||
let arg_span = expand_past_previous_comma(cx, *span);
|
||||
fixes.push((arg_span, String::new()));
|
||||
true // successful inlining, continue checking
|
||||
} else {
|
||||
// if we can't inline a numbered argument, we can't continue
|
||||
param.kind != Numbered
|
||||
}
|
||||
}
|
||||
|
||||
fn outermost_expn_data(expn_data: ExpnData) -> ExpnData {
|
||||
|
|
@ -170,7 +296,7 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex
|
|||
}
|
||||
}
|
||||
|
||||
// Returns true if `hir_id` is referred to by multiple format params
|
||||
/// Returns true if `hir_id` is referred to by multiple format params
|
||||
fn is_aliased(args: &FormatArgsExpn<'_>, hir_id: HirId) -> bool {
|
||||
args.params()
|
||||
.filter(|param| param.value.hir_id == hir_id)
|
||||
|
|
|
|||
|
|
@ -159,6 +159,7 @@ store.register_lints(&[
|
|||
format::USELESS_FORMAT,
|
||||
format_args::FORMAT_IN_FORMAT_ARGS,
|
||||
format_args::TO_STRING_IN_FORMAT_ARGS,
|
||||
format_args::UNINLINED_FORMAT_ARGS,
|
||||
format_impl::PRINT_IN_FORMAT_IMPL,
|
||||
format_impl::RECURSIVE_FORMAT_IMPL,
|
||||
format_push_string::FORMAT_PUSH_STRING,
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
|
|||
LintId::of(eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS),
|
||||
LintId::of(excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS),
|
||||
LintId::of(excessive_bools::STRUCT_EXCESSIVE_BOOLS),
|
||||
LintId::of(format_args::UNINLINED_FORMAT_ARGS),
|
||||
LintId::of(functions::MUST_USE_CANDIDATE),
|
||||
LintId::of(functions::TOO_MANY_LINES),
|
||||
LintId::of(if_not_else::IF_NOT_ELSE),
|
||||
|
|
|
|||
|
|
@ -855,7 +855,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
))
|
||||
});
|
||||
store.register_late_pass(move || Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks));
|
||||
store.register_late_pass(move || Box::new(format_args::FormatArgs));
|
||||
store.register_late_pass(move || Box::new(format_args::FormatArgs::new(msrv)));
|
||||
store.register_late_pass(|| Box::new(trailing_empty_array::TrailingEmptyArray));
|
||||
store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes));
|
||||
store.register_late_pass(|| Box::new(needless_late_init::NeedlessLateInit));
|
||||
|
|
|
|||
|
|
@ -213,7 +213,7 @@ define_Conf! {
|
|||
///
|
||||
/// Suppress lints whenever the suggested change would cause breakage for other crates.
|
||||
(avoid_breaking_exported_api: bool = true),
|
||||
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED.
|
||||
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS.
|
||||
///
|
||||
/// The minimum rust version that the project supports
|
||||
(msrv: Option<String> = None),
|
||||
|
|
|
|||
|
|
@ -1,12 +1,12 @@
|
|||
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
|
||||
use clippy_utils::macros::{root_macro_call_first_node, FormatArgsExpn, MacroCall};
|
||||
use clippy_utils::source::snippet_opt;
|
||||
use clippy_utils::source::{expand_past_previous_comma, snippet_opt};
|
||||
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
|
||||
|
|
@ -542,10 +542,3 @@ fn conservative_unescape(literal: &str) -> Result<String, UnescapeErr> {
|
|||
|
||||
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))
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue