Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace remaining usage of FormatArgsExpn #10561

Merged
merged 1 commit into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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