Skip to content

Commit

Permalink
Auto merge of #10561 - Alexendoo:format-args-ast-3, r=Manishearth
Browse files Browse the repository at this point in the history
Replace remaining usage of `FormatArgsExpn`

Closes #10233

Removes `FormatArgsExpn` & friends now that they're unused

changelog: none

r? `@Manishearth`
  • Loading branch information
bors committed Mar 28, 2023
2 parents 84e42fb + 6589d79 commit 58fb801
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 803 deletions.
29 changes: 14 additions & 15 deletions clippy_lints/src/explicit_write.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::macros::FormatArgsExpn;
use clippy_utils::macros::{find_format_args, format_args_inputs_span};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{is_expn_of, match_function_call, paths};
use if_chain::if_chain;
Expand All @@ -8,7 +8,7 @@ use rustc_hir::def::Res;
use rustc_hir::{BindingAnnotation, Block, BlockCheckMode, Expr, ExprKind, Node, PatKind, QPath, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
use rustc_span::{sym, ExpnId};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -43,23 +43,22 @@ declare_lint_pass!(ExplicitWrite => [EXPLICIT_WRITE]);

impl<'tcx> LateLintPass<'tcx> for ExplicitWrite {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
// match call to unwrap
if let ExprKind::MethodCall(unwrap_fun, write_call, [], _) = expr.kind;
if unwrap_fun.ident.name == sym::unwrap;
// match call to unwrap
if let ExprKind::MethodCall(unwrap_fun, write_call, [], _) = expr.kind
&& unwrap_fun.ident.name == sym::unwrap
// match call to write_fmt
if let ExprKind::MethodCall(write_fun, write_recv, [write_arg], _) = look_in_block(cx, &write_call.kind);
if write_fun.ident.name == sym!(write_fmt);
&& let ExprKind::MethodCall(write_fun, write_recv, [write_arg], _) = look_in_block(cx, &write_call.kind)
&& write_fun.ident.name == sym!(write_fmt)
// match calls to std::io::stdout() / std::io::stderr ()
if let Some(dest_name) = if match_function_call(cx, write_recv, &paths::STDOUT).is_some() {
&& let Some(dest_name) = if match_function_call(cx, write_recv, &paths::STDOUT).is_some() {
Some("stdout")
} else if match_function_call(cx, write_recv, &paths::STDERR).is_some() {
Some("stderr")
} else {
None
};
if let Some(format_args) = FormatArgsExpn::parse(cx, write_arg);
then {
}
{
find_format_args(cx, write_arg, ExpnId::root(), |format_args| {
let calling_macro =
// ordering is important here, since `writeln!` uses `write!` internally
if is_expn_of(write_call.span, "writeln").is_some() {
Expand Down Expand Up @@ -92,7 +91,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite {
let mut applicability = Applicability::MachineApplicable;
let inputs_snippet = snippet_with_applicability(
cx,
format_args.inputs_span(),
format_args_inputs_span(format_args),
"..",
&mut applicability,
);
Expand All @@ -104,8 +103,8 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite {
"try this",
format!("{prefix}{sugg_mac}!({inputs_snippet})"),
applicability,
)
}
);
});
}
}
}
Expand Down
91 changes: 44 additions & 47 deletions clippy_lints/src/format.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::macros::{root_macro_call_first_node, FormatArgsExpn};
use clippy_utils::source::snippet_with_context;
use clippy_utils::macros::{find_format_arg_expr, find_format_args, root_macro_call_first_node};
use clippy_utils::source::{snippet_opt, snippet_with_context};
use clippy_utils::sugg::Sugg;
use if_chain::if_chain;
use rustc_ast::{FormatArgsPiece, FormatOptions, FormatTrait};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::kw;
use rustc_span::{sym, Span};

declare_clippy_lint! {
Expand Down Expand Up @@ -44,55 +43,53 @@ declare_lint_pass!(UselessFormat => [USELESS_FORMAT]);

impl<'tcx> LateLintPass<'tcx> for UselessFormat {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
let (format_args, call_site) = if_chain! {
if let Some(macro_call) = root_macro_call_first_node(cx, expr);
if cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id);
if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, macro_call.expn);
then {
(format_args, macro_call.span)
} else {
return
}
};
let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return };
if !cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id) {
return;
}

find_format_args(cx, expr, macro_call.expn, |format_args| {
let mut applicability = Applicability::MachineApplicable;
let call_site = macro_call.span;

let mut applicability = Applicability::MachineApplicable;
if format_args.args.is_empty() {
match *format_args.format_string.parts {
[] => span_useless_format_empty(cx, call_site, "String::new()".to_owned(), applicability),
[_] => {
match (format_args.arguments.all_args(), &format_args.template[..]) {
([], []) => span_useless_format_empty(cx, call_site, "String::new()".to_owned(), applicability),
([], [_]) => {
// Simulate macro expansion, converting {{ and }} to { and }.
let s_expand = format_args.format_string.snippet.replace("{{", "{").replace("}}", "}");
let Some(snippet) = snippet_opt(cx, format_args.span) else { return };
let s_expand = snippet.replace("{{", "{").replace("}}", "}");
let sugg = format!("{s_expand}.to_string()");
span_useless_format(cx, call_site, sugg, applicability);
},
[..] => {},
}
} else if let [arg] = &*format_args.args {
let value = arg.param.value;
if_chain! {
if format_args.format_string.parts == [kw::Empty];
if arg.format.is_default();
if match cx.typeck_results().expr_ty(value).peel_refs().kind() {
ty::Adt(adt, _) => Some(adt.did()) == cx.tcx.lang_items().string(),
ty::Str => true,
_ => false,
};
then {
let is_new_string = match value.kind {
ExprKind::Binary(..) => true,
ExprKind::MethodCall(path, ..) => path.ident.name == sym::to_string,
_ => false,
};
let sugg = if is_new_string {
snippet_with_context(cx, value.span, call_site.ctxt(), "..", &mut applicability).0.into_owned()
} else {
let sugg = Sugg::hir_with_context(cx, value, call_site.ctxt(), "<arg>", &mut applicability);
format!("{}.to_string()", sugg.maybe_par())
};
span_useless_format(cx, call_site, sugg, applicability);
}
([arg], [piece]) => {
if let Ok(value) = find_format_arg_expr(expr, arg)
&& let FormatArgsPiece::Placeholder(placeholder) = piece
&& placeholder.format_trait == FormatTrait::Display
&& placeholder.format_options == FormatOptions::default()
&& match cx.typeck_results().expr_ty(value).peel_refs().kind() {
ty::Adt(adt, _) => Some(adt.did()) == cx.tcx.lang_items().string(),
ty::Str => true,
_ => false,
}
{
let is_new_string = match value.kind {
ExprKind::Binary(..) => true,
ExprKind::MethodCall(path, ..) => path.ident.name == sym::to_string,
_ => false,
};
let sugg = if is_new_string {
snippet_with_context(cx, value.span, call_site.ctxt(), "..", &mut applicability).0.into_owned()
} else {
let sugg = Sugg::hir_with_context(cx, value, call_site.ctxt(), "<arg>", &mut applicability);
format!("{}.to_string()", sugg.maybe_par())
};
span_useless_format(cx, call_site, sugg, applicability);

}
},
_ => {},
}
};
});
}
}

Expand Down
60 changes: 38 additions & 22 deletions clippy_lints/src/format_impl.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
use clippy_utils::macros::{is_format_macro, root_macro_call_first_node, FormatArg, FormatArgsExpn};
use clippy_utils::macros::{find_format_arg_expr, find_format_args, is_format_macro, root_macro_call_first_node};
use clippy_utils::{get_parent_as_impl, is_diag_trait_item, path_to_local, peel_ref_operators};
use if_chain::if_chain;
use rustc_ast::{FormatArgsPiece, FormatTrait};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Impl, ImplItem, ImplItemKind, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::Span;
use rustc_span::{sym, symbol::kw, Symbol};

declare_clippy_lint! {
Expand Down Expand Up @@ -89,7 +91,7 @@ declare_clippy_lint! {
}

#[derive(Clone, Copy)]
struct FormatTrait {
struct FormatTraitNames {
/// e.g. `sym::Display`
name: Symbol,
/// `f` in `fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {}`
Expand All @@ -99,7 +101,7 @@ struct FormatTrait {
#[derive(Default)]
pub struct FormatImpl {
// Whether we are inside Display or Debug trait impl - None for neither
format_trait_impl: Option<FormatTrait>,
format_trait_impl: Option<FormatTraitNames>,
}

impl FormatImpl {
Expand Down Expand Up @@ -161,43 +163,57 @@ fn check_to_string_in_display(cx: &LateContext<'_>, expr: &Expr<'_>) {
}
}

fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, impl_trait: FormatTrait) {
fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, impl_trait: FormatTraitNames) {
// Check each arg in format calls - do we ever use Display on self (directly or via deref)?
if_chain! {
if let Some(outer_macro) = root_macro_call_first_node(cx, expr);
if let macro_def_id = outer_macro.def_id;
if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, outer_macro.expn);
if is_format_macro(cx, macro_def_id);
then {
for arg in format_args.args {
if arg.format.r#trait != impl_trait.name {
continue;
if let Some(outer_macro) = root_macro_call_first_node(cx, expr)
&& let macro_def_id = outer_macro.def_id
&& is_format_macro(cx, macro_def_id)
{
find_format_args(cx, expr, outer_macro.expn, |format_args| {
for piece in &format_args.template {
if let FormatArgsPiece::Placeholder(placeholder) = piece
&& let trait_name = match placeholder.format_trait {
FormatTrait::Display => sym::Display,
FormatTrait::Debug => sym::Debug,
FormatTrait::LowerExp => sym!(LowerExp),
FormatTrait::UpperExp => sym!(UpperExp),
FormatTrait::Octal => sym!(Octal),
FormatTrait::Pointer => sym::Pointer,
FormatTrait::Binary => sym!(Binary),
FormatTrait::LowerHex => sym!(LowerHex),
FormatTrait::UpperHex => sym!(UpperHex),
}
&& trait_name == impl_trait.name
&& let Ok(index) = placeholder.argument.index
&& let Some(arg) = format_args.arguments.all_args().get(index)
&& let Ok(arg_expr) = find_format_arg_expr(expr, arg)
{
check_format_arg_self(cx, expr.span, arg_expr, impl_trait);
}
check_format_arg_self(cx, expr, &arg, impl_trait);
}
}
});
}
}

fn check_format_arg_self(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &FormatArg<'_>, impl_trait: FormatTrait) {
fn check_format_arg_self(cx: &LateContext<'_>, span: Span, arg: &Expr<'_>, impl_trait: FormatTraitNames) {
// Handle multiple dereferencing of references e.g. &&self
// Handle dereference of &self -> self that is equivalent (i.e. via *self in fmt() impl)
// Since the argument to fmt is itself a reference: &self
let reference = peel_ref_operators(cx, arg.param.value);
let reference = peel_ref_operators(cx, arg);
let map = cx.tcx.hir();
// Is the reference self?
if path_to_local(reference).map(|x| map.name(x)) == Some(kw::SelfLower) {
let FormatTrait { name, .. } = impl_trait;
let FormatTraitNames { name, .. } = impl_trait;
span_lint(
cx,
RECURSIVE_FORMAT_IMPL,
expr.span,
span,
&format!("using `self` as `{name}` in `impl {name}` will cause infinite recursion"),
);
}
}

fn check_print_in_format_impl(cx: &LateContext<'_>, expr: &Expr<'_>, impl_trait: FormatTrait) {
fn check_print_in_format_impl(cx: &LateContext<'_>, expr: &Expr<'_>, impl_trait: FormatTraitNames) {
if_chain! {
if let Some(macro_call) = root_macro_call_first_node(cx, expr);
if let Some(name) = cx.tcx.get_diagnostic_name(macro_call.def_id);
Expand Down Expand Up @@ -227,7 +243,7 @@ fn check_print_in_format_impl(cx: &LateContext<'_>, expr: &Expr<'_>, impl_trait:
}
}

fn is_format_trait_impl(cx: &LateContext<'_>, impl_item: &ImplItem<'_>) -> Option<FormatTrait> {
fn is_format_trait_impl(cx: &LateContext<'_>, impl_item: &ImplItem<'_>) -> Option<FormatTraitNames> {
if_chain! {
if impl_item.ident.name == sym::fmt;
if let ImplItemKind::Fn(_, body_id) = impl_item.kind;
Expand All @@ -241,7 +257,7 @@ fn is_format_trait_impl(cx: &LateContext<'_>, impl_item: &ImplItem<'_>) -> Optio
.and_then(|param| param.pat.simple_ident())
.map(|ident| ident.name);

Some(FormatTrait {
Some(FormatTraitNames {
name,
formatter_name,
})
Expand Down
27 changes: 14 additions & 13 deletions clippy_lints/src/methods/expect_fun_call.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::macros::{root_macro_call_first_node, FormatArgsExpn};
use clippy_utils::macros::{find_format_args, format_args_inputs_span, root_macro_call_first_node};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
use rustc_errors::Applicability;
Expand Down Expand Up @@ -136,18 +136,19 @@ pub(super) fn check<'tcx>(
if !cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id) {
return;
}
let Some(format_args) = FormatArgsExpn::find_nested(cx, arg_root, macro_call.expn) else { return };
let span = format_args.inputs_span();
let sugg = snippet_with_applicability(cx, span, "..", &mut applicability);
span_lint_and_sugg(
cx,
EXPECT_FUN_CALL,
span_replace_word,
&format!("use of `{name}` followed by a function call"),
"try this",
format!("unwrap_or_else({closure_args} panic!({sugg}))"),
applicability,
);
find_format_args(cx, arg_root, macro_call.expn, |format_args| {
let span = format_args_inputs_span(format_args);
let sugg = snippet_with_applicability(cx, span, "..", &mut applicability);
span_lint_and_sugg(
cx,
EXPECT_FUN_CALL,
span_replace_word,
&format!("use of `{name}` followed by a function call"),
"try this",
format!("unwrap_or_else({closure_args} panic!({sugg}))"),
applicability,
);
});
return;
}

Expand Down
1 change: 0 additions & 1 deletion clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ extern crate rustc_lexer;
extern crate rustc_lint;
extern crate rustc_middle;
extern crate rustc_mir_dataflow;
extern crate rustc_parse_format;
extern crate rustc_session;
extern crate rustc_span;
extern crate rustc_target;
Expand Down
Loading

0 comments on commit 58fb801

Please sign in to comment.