Skip to content

Commit

Permalink
Implement has_string_formatting
Browse files Browse the repository at this point in the history
  • Loading branch information
smoelius committed Oct 3, 2021
1 parent 032ae17 commit d54fc50
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 42 deletions.
4 changes: 2 additions & 2 deletions clippy_lints/src/format.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::higher::{FormatExpn, Formatting};
use clippy_utils::higher::FormatExpn;
use clippy_utils::source::{snippet_opt, snippet_with_applicability};
use clippy_utils::sugg::Sugg;
use if_chain::if_chain;
Expand Down Expand Up @@ -69,7 +69,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat {
_ => false,
};
if format_args.args().all(|arg| arg.is_display());
if !format_args.has_formatting(Formatting::PRECISION | Formatting::WIDTH);
if !format_args.has_string_formatting();
then {
let is_new_string = match value.kind {
ExprKind::Binary(..) => true,
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/format_args.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
use clippy_utils::higher::{FormatArgsArg, FormatArgsExpn, FormatExpn, Formatting};
use clippy_utils::higher::{FormatArgsArg, 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};
Expand Down Expand Up @@ -85,7 +85,7 @@ where
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.has_formatting(Formatting::all());
if !format_args.has_string_formatting();
then {
for (i, arg) in format_args.args().enumerate() {
if !arg.is_display() {
Expand Down
1 change: 0 additions & 1 deletion clippy_utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ edition = "2021"
publish = false

[dependencies]
bitflags = "1.2"
if_chain = "1.0"
rustc-semver = "1.1"

Expand Down
46 changes: 9 additions & 37 deletions clippy_utils/src/higher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#![deny(clippy::missing_docs_in_private_items)]

use crate::{is_expn_of, last_path_segment, match_def_path, paths};
use bitflags::bitflags;
use if_chain::if_chain;
use rustc_ast::ast::{self, LitKind};
use rustc_hir as hir;
Expand Down Expand Up @@ -571,9 +570,10 @@ impl FormatArgsExpn<'tcx> {
}
}

/// Returns true if any argument has the given formatting types.
pub fn has_formatting(&self, formatting: Formatting) -> bool {
self.args().any(|arg| arg.has_formatting(formatting))
/// Returns true if any argument uses formatting parameters that would have an effect on
/// strings.
pub fn has_string_formatting(&self) -> bool {
self.args().any(|arg| arg.has_string_formatting())
}

/// Returns an iterator over `FormatArgsArg`.
Expand Down Expand Up @@ -657,16 +657,6 @@ pub struct FormatArgsArg<'tcx> {
fmt: Option<&'tcx Expr<'tcx>>,
}

bitflags! {
pub struct Formatting: u32 {
const FILL = 0b0000_0001;
const ALIGN = 0b0000_0010;
const FLAGS = 0b0000_0100;
const PRECISION = 0b0000_1000;
const WIDTH = 0b0001_0000;
}
}

impl<'tcx> FormatArgsArg<'tcx> {
/// An element of `value_args` according to `position`
pub fn value(&self) -> &'tcx Expr<'tcx> {
Expand All @@ -683,12 +673,10 @@ impl<'tcx> FormatArgsArg<'tcx> {
self.fmt
}

/// Returns true if any formatting parameters are used like `{:+2}` instead of just `{}`. Note
/// that the check is performed using the logical OR of the flags. So, for example,
/// `has_formatting(Formatting:all())` checks whether any (not all) formatting types are
/// used.
/// Returns true if any formatting parameters are used that would have an effect on strings,
/// like `{:+2}` instead of just `{}`.
#[allow(clippy::nonminimal_bool)]
pub fn has_formatting(&self, formatting: Formatting) -> bool {
pub fn has_string_formatting(&self) -> bool {
self.fmt().map_or(false, |fmt| {
// `!` because these conditions check that `self` is unformatted.
!if_chain! {
Expand All @@ -708,27 +696,11 @@ impl<'tcx> FormatArgsArg<'tcx> {
let _ = assert_eq!(precision_field.ident.name, sym::precision);
let _ = assert_eq!(width_field.ident.name, sym::width);

if let ExprKind::Lit(lit) = &fill_field.expr.kind;
if let LitKind::Char(char) = lit.node;
if !formatting.contains(Formatting::FILL)
|| char == ' ';

if let ExprKind::Path(ref align_path) = align_field.expr.kind;
if !formatting.contains(Formatting::ALIGN)
|| last_path_segment(align_path).ident.name == sym::Unknown;

if let ExprKind::Lit(lit) = &flags_field.expr.kind;
if let LitKind::Int(u128, _) = lit.node;
if !formatting.contains(Formatting::FLAGS)
|| u128 == 0;

if let ExprKind::Path(ref precision_path) = precision_field.expr.kind;
if !formatting.contains(Formatting::PRECISION)
|| last_path_segment(precision_path).ident.name == sym::Implied;
if last_path_segment(precision_path).ident.name == sym::Implied;

if let ExprKind::Path(ref width_qpath) = width_field.expr.kind;
if !formatting.contains(Formatting::WIDTH)
|| last_path_segment(width_qpath).ident.name == sym::Implied;
if last_path_segment(width_qpath).ident.name == sym::Implied;

then { true } else { false }
}
Expand Down

0 comments on commit d54fc50

Please sign in to comment.