Skip to content

Commit cd8eda5

Browse files
AlexendooAlexendoo
committed
Refactor FormatArgsExpn 
1 parent 84fb7e0 commit cd8eda5

12 files changed

+581
-254
lines changed

‎clippy_lints/src/format.rs

+11-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::macros::{root_macro_call_first_node, FormatArgsExpn};
3-
use clippy_utils::source::{snippet_opt, snippet_with_applicability};
3+
use clippy_utils::source::snippet_with_applicability;
44
use clippy_utils::sugg::Sugg;
55
use if_chain::if_chain;
66
use rustc_errors::Applicability;
@@ -56,29 +56,27 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat {
5656
};
5757

5858
let mut applicability = Applicability::MachineApplicable;
59-
if format_args.value_args.is_empty() {
60-
match *format_args.format_string_parts {
59+
if format_args.args.is_empty() {
60+
match *format_args.format_string.parts {
6161
[] => span_useless_format_empty(cx, call_site, "String::new()".to_owned(), applicability),
6262
[_] => {
63-
if let Some(s_src) = snippet_opt(cx, format_args.format_string_span) {
64-
// Simulate macro expansion, converting {{ and }} to { and }.
65-
let s_expand = s_src.replace("{{", "{").replace("}}", "}");
66-
let sugg = format!("{}.to_string()", s_expand);
67-
span_useless_format(cx, call_site, sugg, applicability);
68-
}
63+
// Simulate macro expansion, converting {{ and }} to { and }.
64+
let s_expand = format_args.format_string.snippet.replace("{{", "{").replace("}}", "}");
65+
let sugg = format!("{}.to_string()", s_expand);
66+
span_useless_format(cx, call_site, sugg, applicability);
6967
},
7068
[..] => {},
7169
}
72-
} else if let [value] = *format_args.value_args {
70+
} else if let [arg] = &*format_args.args {
71+
let value = arg.param.value;
7372
if_chain! {
74-
if format_args.format_string_parts == [kw::Empty];
73+
if format_args.format_string.parts == [kw::Empty];
7574
if match cx.typeck_results().expr_ty(value).peel_refs().kind() {
7675
ty::Adt(adt, _) => cx.tcx.is_diagnostic_item(sym::String, adt.did()),
7776
ty::Str => true,
7877
_ => false,
7978
};
80-
if let Some(args) = format_args.args();
81-
if args.iter().all(|arg| arg.format_trait == sym::Display && !arg.has_string_formatting());
79+
if !arg.format.has_string_formatting();
8280
then {
8381
let is_new_string = match value.kind {
8482
ExprKind::Binary(..) => true,

‎clippy_lints/src/format_args.rs

+14-17
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
22
use clippy_utils::is_diag_trait_item;
3-
use clippy_utils::macros::{is_format_macro, FormatArgsArg, FormatArgsExpn};
3+
use clippy_utils::macros::{is_format_macro, FormatArgsExpn};
44
use clippy_utils::source::snippet_opt;
55
use clippy_utils::ty::implements_trait;
66
use if_chain::if_chain;
7+
use itertools::Itertools;
78
use rustc_errors::Applicability;
8-
use rustc_hir::{Expr, ExprKind};
9+
use rustc_hir::{Expr, ExprKind, HirId};
910
use rustc_lint::{LateContext, LateLintPass};
1011
use rustc_middle::ty::adjustment::{Adjust, Adjustment};
1112
use rustc_middle::ty::Ty;
@@ -74,20 +75,16 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
7475
if let Some(macro_def_id) = outermost_expn_data.macro_def_id;
7576
if is_format_macro(cx, macro_def_id);
7677
if let ExpnKind::Macro(_, name) = outermost_expn_data.kind;
77-
if let Some(args) = format_args.args();
7878
then {
79-
for (i, arg) in args.iter().enumerate() {
80-
if arg.format_trait != sym::Display {
79+
for arg in &format_args.args {
80+
if arg.format.has_string_formatting() {
8181
continue;
8282
}
83-
if arg.has_string_formatting() {
83+
if is_aliased(&format_args, arg.param.value.hir_id) {
8484
continue;
8585
}
86-
if is_aliased(&args, i) {
87-
continue;
88-
}
89-
check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg.value);
90-
check_to_string_in_format_args(cx, name, arg.value);
86+
check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg.param.value);
87+
check_to_string_in_format_args(cx, name, arg.param.value);
9188
}
9289
}
9390
}
@@ -167,12 +164,12 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex
167164
}
168165
}
169166

170-
// Returns true if `args[i]` "refers to" or "is referred to by" another argument.
171-
fn is_aliased(args: &[FormatArgsArg<'_>], i: usize) -> bool {
172-
let value = args[i].value;
173-
args.iter()
174-
.enumerate()
175-
.any(|(j, arg)| i != j && std::ptr::eq(value, arg.value))
167+
// Returns true if `hir_id` is referred to by multiple format params
168+
fn is_aliased(args: &FormatArgsExpn<'_>, hir_id: HirId) -> bool {
169+
args.params()
170+
.filter(|param| param.value.hir_id == hir_id)
171+
.at_most_one()
172+
.is_err()
176173
}
177174

178175
fn count_needed_derefs<'tcx, I>(mut ty: Ty<'tcx>, mut iter: I) -> (usize, Ty<'tcx>)

‎clippy_lints/src/format_impl.rs

+5-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
2-
use clippy_utils::macros::{is_format_macro, root_macro_call_first_node, FormatArgsArg, FormatArgsExpn};
2+
use clippy_utils::macros::{is_format_macro, root_macro_call_first_node, FormatArg, FormatArgsExpn};
33
use clippy_utils::{get_parent_as_impl, is_diag_trait_item, path_to_local, peel_ref_operators};
44
use if_chain::if_chain;
55
use rustc_errors::Applicability;
@@ -168,10 +168,9 @@ fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>,
168168
if let macro_def_id = outer_macro.def_id;
169169
if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, outer_macro.expn);
170170
if is_format_macro(cx, macro_def_id);
171-
if let Some(args) = format_args.args();
172171
then {
173-
for arg in args {
174-
if arg.format_trait != impl_trait.name {
172+
for arg in format_args.args {
173+
if arg.format.r#trait != impl_trait.name {
175174
continue;
176175
}
177176
check_format_arg_self(cx, expr, &arg, impl_trait);
@@ -180,11 +179,11 @@ fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>,
180179
}
181180
}
182181

183-
fn check_format_arg_self(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &FormatArgsArg<'_>, impl_trait: FormatTrait) {
182+
fn check_format_arg_self(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &FormatArg<'_>, impl_trait: FormatTrait) {
184183
// Handle multiple dereferencing of references e.g. &&self
185184
// Handle dereference of &self -> self that is equivalent (i.e. via *self in fmt() impl)
186185
// Since the argument to fmt is itself a reference: &self
187-
let reference = peel_ref_operators(cx, arg.value);
186+
let reference = peel_ref_operators(cx, arg.param.value);
188187
let map = cx.tcx.hir();
189188
// Is the reference self?
190189
if path_to_local(reference).map(|x| map.name(x)) == Some(kw::SelfLower) {

‎clippy_lints/src/methods/uninit_assumed_init.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint;
2-
use clippy_utils::{is_expr_diagnostic_item, ty::is_uninit_value_valid_for_ty};
2+
use clippy_utils::{is_path_diagnostic_item, ty::is_uninit_value_valid_for_ty};
33
use if_chain::if_chain;
44
use rustc_hir as hir;
55
use rustc_lint::LateContext;
@@ -12,7 +12,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
1212
if_chain! {
1313
if let hir::ExprKind::Call(callee, args) = recv.kind;
1414
if args.is_empty();
15-
if is_expr_diagnostic_item(cx, callee, sym::maybe_uninit_uninit);
15+
if is_path_diagnostic_item(cx, callee, sym::maybe_uninit_uninit);
1616
if !is_uninit_value_valid_for_ty(cx, cx.typeck_results().expr_ty_adjusted(expr));
1717
then {
1818
span_lint(

‎clippy_lints/src/transmuting_null.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::consts::{constant_context, Constant};
22
use clippy_utils::diagnostics::span_lint;
3-
use clippy_utils::is_expr_diagnostic_item;
3+
use clippy_utils::is_path_diagnostic_item;
44
use if_chain::if_chain;
55
use rustc_ast::LitKind;
66
use rustc_hir::{Expr, ExprKind};
@@ -43,7 +43,7 @@ impl<'tcx> LateLintPass<'tcx> for TransmutingNull {
4343

4444
if_chain! {
4545
if let ExprKind::Call(func, [arg]) = expr.kind;
46-
if is_expr_diagnostic_item(cx, func, sym::transmute);
46+
if is_path_diagnostic_item(cx, func, sym::transmute);
4747

4848
then {
4949
// Catching transmute over constants that resolve to `null`.
@@ -72,7 +72,7 @@ impl<'tcx> LateLintPass<'tcx> for TransmutingNull {
7272
// `std::mem::transmute(std::ptr::null::<i32>())`
7373
if_chain! {
7474
if let ExprKind::Call(func1, []) = arg.kind;
75-
if is_expr_diagnostic_item(cx, func1, sym::ptr_null);
75+
if is_path_diagnostic_item(cx, func1, sym::ptr_null);
7676
then {
7777
span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG)
7878
}

‎clippy_utils/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ publish = false
77
[dependencies]
88
arrayvec = { version = "0.7", default-features = false }
99
if_chain = "1.0"
10+
itertools = "0.10.1"
1011
rustc-semver = "1.1"
1112

1213
[features]

‎clippy_utils/src/lib.rs

+10-5
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ extern crate rustc_infer;
2727
extern crate rustc_lexer;
2828
extern crate rustc_lint;
2929
extern crate rustc_middle;
30+
extern crate rustc_parse_format;
3031
extern crate rustc_session;
3132
extern crate rustc_span;
3233
extern crate rustc_target;
@@ -371,15 +372,19 @@ pub fn match_qpath(path: &QPath<'_>, segments: &[&str]) -> bool {
371372

372373
/// If the expression is a path, resolves it to a `DefId` and checks if it matches the given path.
373374
///
374-
/// Please use `is_expr_diagnostic_item` if the target is a diagnostic item.
375+
/// Please use `is_path_diagnostic_item` if the target is a diagnostic item.
375376
pub fn is_expr_path_def_path(cx: &LateContext<'_>, expr: &Expr<'_>, segments: &[&str]) -> bool {
376377
path_def_id(cx, expr).map_or(false, |id| match_def_path(cx, id, segments))
377378
}
378379

379-
/// If the expression is a path, resolves it to a `DefId` and checks if it matches the given
380-
/// diagnostic item.
381-
pub fn is_expr_diagnostic_item(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) -> bool {
382-
path_def_id(cx, expr).map_or(false, |id| cx.tcx.is_diagnostic_item(diag_item, id))
380+
/// If `maybe_path` is a path node which resolves to an item, resolves it to a `DefId` and checks if
381+
/// it matches the given diagnostic item.
382+
pub fn is_path_diagnostic_item<'tcx>(
383+
cx: &LateContext<'_>,
384+
maybe_path: &impl MaybePath<'tcx>,
385+
diag_item: Symbol,
386+
) -> bool {
387+
path_def_id(cx, maybe_path).map_or(false, |id| cx.tcx.is_diagnostic_item(diag_item, id))
383388
}
384389

385390
/// THIS METHOD IS DEPRECATED and will eventually be removed since it does not match against the

0 commit comments

Comments
 (0)