forked from rust-lang/rust-clippy
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add
format_in_format_args and to_string_in_format_args lints
Fixes rust-lang#7667 and rust-lang#7729
- Loading branch information
Showing
14 changed files
with
423 additions
and
46 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,262 @@ | ||
| use clippy_utils::diagnostics::span_lint_and_sugg; | ||
| use clippy_utils::format::{check_unformatted, is_display_arg}; | ||
| use clippy_utils::higher::{FormatArgsExpn, FormatExpn}; | ||
| use clippy_utils::source::snippet_opt; | ||
| use clippy_utils::ty::implements_trait; | ||
| use clippy_utils::{get_trait_def_id, match_def_path, paths}; | ||
| use if_chain::if_chain; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir::{Expr, ExprKind}; | ||
| use rustc_lint::{LateContext, LateLintPass}; | ||
| use rustc_session::{declare_lint_pass, declare_tool_lint}; | ||
| use rustc_span::{BytePos, ExpnKind, Span, Symbol}; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// ### What it does | ||
| /// Warns on `format!` within the arguments of another macro that does | ||
| /// formatting such as `format!` itself, `write!` or `println!`. Suggests | ||
| /// inlining the `format!` call. | ||
| /// | ||
| /// ### Why is this bad? | ||
| /// The recommended code is both shorter and avoids a temporary allocation. | ||
| /// | ||
| /// ### Example | ||
| /// ```rust | ||
| /// # use std::panic::Location; | ||
| /// println!("error: {}", format!("something failed at {}", Location::caller())); | ||
| /// ``` | ||
| /// Use instead: | ||
| /// ```rust | ||
| /// # use std::panic::Location; | ||
| /// println!("error: something failed at {}", Location::caller()); | ||
| /// ``` | ||
| pub FORMAT_IN_FORMAT_ARGS, | ||
| perf, | ||
| "`format!` used in a macro that does formatting" | ||
| } | ||
|
|
||
| declare_lint_pass!(FormatInFormatArgs => [FORMAT_IN_FORMAT_ARGS]); | ||
|
|
||
| impl<'tcx> LateLintPass<'tcx> for FormatInFormatArgs { | ||
| fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | ||
| check_expr(cx, expr, format_in_format_args); | ||
| } | ||
| } | ||
|
|
||
| declare_clippy_lint! { | ||
| /// ### What it does | ||
| /// Checks for [`ToString::to_string`](https://doc.rust-lang.org/std/string/trait.ToString.html#tymethod.to_string) | ||
| /// applied to a type that implements [`Display`](https://doc.rust-lang.org/std/fmt/trait.Display.html) | ||
| /// in a macro that does formatting. | ||
| /// | ||
| /// ### Why is this bad? | ||
| /// Since the type implements `Display`, the use of `to_string` is | ||
| /// unnecessary. | ||
| /// | ||
| /// ### Example | ||
| /// ```rust | ||
| /// # use std::panic::Location; | ||
| /// println!("error: something failed at {}", Location::caller().to_string()); | ||
| /// ``` | ||
| /// Use instead: | ||
| /// ```rust | ||
| /// # use std::panic::Location; | ||
| /// println!("error: something failed at {}", Location::caller()); | ||
| /// ``` | ||
| pub TO_STRING_IN_FORMAT_ARGS, | ||
| perf, | ||
| "`to_string` applied to a type that implements `Display` in format args" | ||
| } | ||
|
|
||
| declare_lint_pass!(ToStringInFormatArgs => [TO_STRING_IN_FORMAT_ARGS]); | ||
|
|
||
| impl<'tcx> LateLintPass<'tcx> for ToStringInFormatArgs { | ||
| fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | ||
| check_expr(cx, expr, to_string_in_format_args); | ||
| } | ||
| } | ||
|
|
||
| fn check_expr<'tcx, F>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, check_value: F) | ||
| where | ||
| F: Fn(&LateContext<'_>, &FormatArgsExpn<'_>, Span, Symbol, usize, &Expr<'_>) -> bool, | ||
| { | ||
| if_chain! { | ||
| if let Some(format_args) = FormatArgsExpn::parse(expr); | ||
| let call_site = expr.span.ctxt().outer_expn_data().call_site; | ||
| if call_site.from_expansion(); | ||
| let expn_data = call_site.ctxt().outer_expn_data(); | ||
| if let ExpnKind::Macro(_, name) = expn_data.kind; | ||
| if format_args.fmt_expr.map_or(true, check_unformatted); | ||
| then { | ||
| assert_eq!(format_args.args.len(), format_args.value_args.len()); | ||
| for (i, (arg, value)) in format_args.args.iter().zip(format_args.value_args.iter()).enumerate() { | ||
| if !is_display_arg(arg) { | ||
| continue; | ||
| } | ||
| if check_value(cx, &format_args, expn_data.call_site, name, i, value) { | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn format_in_format_args( | ||
| cx: &LateContext<'_>, | ||
| format_args: &FormatArgsExpn<'_>, | ||
| call_site: Span, | ||
| name: Symbol, | ||
| i: usize, | ||
| value: &Expr<'_>, | ||
| ) -> bool { | ||
| if_chain! { | ||
| if let Some(FormatExpn{ format_args: inner_format_args, .. }) = FormatExpn::parse(value); | ||
| if let Some(format_string) = snippet_opt(cx, format_args.format_string_span); | ||
| if let Some(inner_format_string) = snippet_opt(cx, inner_format_args.format_string_span); | ||
| if let Some((sugg, applicability)) = format_in_format_args_sugg( | ||
| cx, | ||
| name, | ||
| &format_string, | ||
| &format_args.value_args, | ||
| i, | ||
| &inner_format_string, | ||
| &inner_format_args.value_args | ||
| ); | ||
| then { | ||
| span_lint_and_sugg( | ||
| cx, | ||
| FORMAT_IN_FORMAT_ARGS, | ||
| trim_semicolon(cx, call_site), | ||
| &format!("`format!` in `{}!` args", name), | ||
| "inline the `format!` call", | ||
| sugg, | ||
| applicability, | ||
| ); | ||
| // Report only the first instance. | ||
| return true; | ||
| } | ||
| } | ||
| false | ||
| } | ||
|
|
||
| fn to_string_in_format_args( | ||
| cx: &LateContext<'_>, | ||
| _format_args: &FormatArgsExpn<'_>, | ||
| _call_site: Span, | ||
| name: Symbol, | ||
| _i: usize, | ||
| value: &Expr<'_>, | ||
| ) -> bool { | ||
| if_chain! { | ||
| if let ExprKind::MethodCall(_, _, args, _) = value.kind; | ||
| if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(value.hir_id); | ||
| if match_def_path(cx, method_def_id, &paths::TO_STRING_METHOD); | ||
| if let Some(receiver) = args.get(0); | ||
| let ty = cx.typeck_results().expr_ty(receiver); | ||
| if let Some(display_trait_id) = get_trait_def_id(cx, &paths::DISPLAY_TRAIT); | ||
| if implements_trait(cx, ty, display_trait_id, &[]); | ||
| if let Some(snippet) = snippet_opt(cx, value.span); | ||
| if let Some(dot) = snippet.rfind('.'); | ||
| then { | ||
| let span = value.span.with_lo( | ||
| value.span.lo() + BytePos(u32::try_from(dot).unwrap()) | ||
| ); | ||
| span_lint_and_sugg( | ||
| cx, | ||
| TO_STRING_IN_FORMAT_ARGS, | ||
| span, | ||
| &format!("`to_string` applied to a type that implements `Display` in `{}!` args", name), | ||
| "remove this", | ||
| String::new(), | ||
| Applicability::MachineApplicable, | ||
| ); | ||
| } | ||
| } | ||
| false | ||
| } | ||
|
|
||
| fn format_in_format_args_sugg( | ||
| cx: &LateContext<'_>, | ||
| name: Symbol, | ||
| format_string: &str, | ||
| values: &[&Expr<'_>], | ||
| i: usize, | ||
| inner_format_string: &str, | ||
| inner_values: &[&Expr<'_>], | ||
| ) -> Option<(String, Applicability)> { | ||
| let (left, right) = split_format_string(format_string, i); | ||
| // If the inner format string is raw, the user is on their own. | ||
| let (new_format_string, applicability) = if inner_format_string.starts_with('r') { | ||
| (left + ".." + &right, Applicability::HasPlaceholders) | ||
| } else { | ||
| ( | ||
| left + &trim_quotes(inner_format_string) + &right, | ||
| Applicability::MachineApplicable, | ||
| ) | ||
| }; | ||
| let values = values | ||
| .iter() | ||
| .map(|value| snippet_opt(cx, value.span)) | ||
| .collect::<Option<Vec<_>>>()?; | ||
| let inner_values = inner_values | ||
| .iter() | ||
| .map(|value| snippet_opt(cx, value.span)) | ||
| .collect::<Option<Vec<_>>>()?; | ||
| let new_values = itertools::join( | ||
| values | ||
| .iter() | ||
| .take(i) | ||
| .chain(inner_values.iter()) | ||
| .chain(values.iter().skip(i + 1)), | ||
| ", ", | ||
| ); | ||
| Some(( | ||
| format!(r#"{}!({}, {})"#, name, new_format_string, new_values), | ||
| applicability, | ||
| )) | ||
| } | ||
|
|
||
| fn split_format_string(format_string: &str, i: usize) -> (String, String) { | ||
| let mut iter = format_string.chars(); | ||
| for j in 0..=i { | ||
| assert!(advance(&mut iter) == '}' || j < i); | ||
| } | ||
|
|
||
| let right = iter.collect::<String>(); | ||
|
|
||
| let size = format_string.len(); | ||
| let right_size = right.len(); | ||
| let left_size = size - right_size; | ||
| assert!(left_size >= 2); | ||
| let left = std::str::from_utf8(&format_string.as_bytes()[..left_size - 2]) | ||
| .unwrap() | ||
| .to_owned(); | ||
| (left, right) | ||
| } | ||
|
|
||
| fn advance(iter: &mut std::str::Chars<'_>) -> char { | ||
| loop { | ||
| let first_char = iter.next().unwrap(); | ||
| if first_char != '{' { | ||
| continue; | ||
| } | ||
| let second_char = iter.next().unwrap(); | ||
| if second_char == '{' { | ||
| continue; | ||
| } | ||
| return second_char; | ||
| } | ||
| } | ||
|
|
||
| fn trim_quotes(string_literal: &str) -> String { | ||
| let len = string_literal.chars().count(); | ||
| assert!(len >= 2); | ||
| string_literal.chars().skip(1).take(len - 2).collect() | ||
| } | ||
|
|
||
| fn trim_semicolon(cx: &LateContext<'_>, span: Span) -> Span { | ||
| snippet_opt(cx, span).map_or(span, |snippet| { | ||
| let snippet = snippet.trim_end_matches(';'); | ||
| span.with_hi(span.lo() + BytePos(u32::try_from(snippet.len()).unwrap())) | ||
| }) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.