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.

Sign up for GitHub

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

Resolve false positives of unnecessary_cast for non-decimal integers #5257

Merged
merged 5 commits into from
Mar 4, 2020
Merged

Resolve false positives of unnecessary_cast for non-decimal integers #5257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9ff4581
Test for unnecessary_cast of hex int literal.
mlegner Mar 2, 2020
6673cc8
Resolve false positives for hex int cast.
mlegner Mar 2, 2020
73deb72
Included binary and octal cases.
mlegner Mar 3, 2020
d14fdc0
Move NumericLiteral to its own module.
mlegner Mar 3, 2020
185fa0d
Simplify if_chain.
mlegner Mar 4, 2020
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
7 changes: 3 additions & 4 deletions clippy_lints/src/float_literal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::utils::span_lint_and_sugg;
use crate::utils::sugg::format_numeric_literal;
use crate::utils::{numeric_literal, span_lint_and_sugg};
use if_chain::if_chain;
use rustc::ty;
use rustc_ast::ast::{FloatTy, LitFloatType, LitKind};
Expand Down Expand Up @@ -109,7 +108,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatLiteral {
expr.span,
"literal cannot be represented as the underlying type without loss of precision",
"consider changing the type or replacing it with",
format_numeric_literal(&float_str, type_suffix, true),
numeric_literal::format(&float_str, type_suffix, true),
Applicability::MachineApplicable,
);
}
Expand All @@ -120,7 +119,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatLiteral {
expr.span,
"float has excessive precision",
"consider changing the type or truncating it to",
format_numeric_literal(&float_str, type_suffix, true),
numeric_literal::format(&float_str, type_suffix, true),
Applicability::MachineApplicable,
);
}
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/floating_point_arithmetic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::consts::{
constant, constant_simple, Constant,
Constant::{F32, F64},
};
use crate::utils::{higher, span_lint_and_sugg, sugg, SpanlessEq};
use crate::utils::{higher, numeric_literal, span_lint_and_sugg, sugg, SpanlessEq};
use if_chain::if_chain;
use rustc::ty;
use rustc_errors::Applicability;
Expand All @@ -14,7 +14,7 @@ use rustc_span::source_map::Spanned;
use rustc_ast::ast;
use std::f32::consts as f32_consts;
use std::f64::consts as f64_consts;
use sugg::{format_numeric_literal, Sugg};
use sugg::Sugg;

declare_clippy_lint! {
/// **What it does:** Looks for floating-point expressions that
Expand Down Expand Up @@ -276,7 +276,7 @@ fn check_powf(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) {
format!(
"{}.powi({})",
Sugg::hir(cx, &args[0], ".."),
format_numeric_literal(&exponent.to_string(), None, false)
numeric_literal::format(&exponent.to_string(), None, false)
),
)
} else {
Expand Down
226 changes: 6 additions & 220 deletions clippy_lints/src/literal_representation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
//! Lints concerned with the grouping of digits with underscores in integral or
//! floating-point literal expressions.

use crate::utils::{in_macro, snippet_opt, span_lint_and_sugg};
use crate::utils::{
in_macro,
numeric_literal::{NumericLiteral, Radix},
snippet_opt, span_lint_and_sugg,
};
use if_chain::if_chain;
use rustc::lint::in_external_macro;
use rustc_ast::ast::{Expr, ExprKind, Lit, LitFloatType, LitIntType, LitKind};
use rustc_ast::ast::{Expr, ExprKind, Lit, LitKind};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
Expand Down Expand Up @@ -103,224 +107,6 @@ declare_clippy_lint! {
"using decimal representation when hexadecimal would be better"
}

#[derive(Debug, PartialEq)]
pub(super) enum Radix {
Binary,
Octal,
Decimal,
Hexadecimal,
}

impl Radix {
/// Returns a reasonable digit group size for this radix.
#[must_use]
fn suggest_grouping(&self) -> usize {
match *self {
Self::Binary | Self::Hexadecimal => 4,
Self::Octal | Self::Decimal => 3,
}
}
}

/// A helper method to format numeric literals with digit grouping.
/// `lit` must be a valid numeric literal without suffix.
pub fn format_numeric_literal(lit: &str, type_suffix: Option<&str>, float: bool) -> String {
NumericLiteral::new(lit, type_suffix, float).format()
}

#[derive(Debug)]
pub(super) struct NumericLiteral<'a> {
/// Which radix the literal was represented in.
radix: Radix,
/// The radix prefix, if present.
prefix: Option<&'a str>,

/// The integer part of the number.
integer: &'a str,
/// The fraction part of the number.
fraction: Option<&'a str>,
/// The character used as exponent seperator (b'e' or b'E') and the exponent part.
exponent: Option<(char, &'a str)>,

/// The type suffix, including preceding underscore if present.
suffix: Option<&'a str>,
}

impl<'a> NumericLiteral<'a> {
fn from_lit(src: &'a str, lit: &Lit) -> Option<NumericLiteral<'a>> {
if lit.kind.is_numeric() && src.chars().next().map_or(false, |c| c.is_digit(10)) {
let (unsuffixed, suffix) = split_suffix(&src, &lit.kind);
let float = if let LitKind::Float(..) = lit.kind { true } else { false };
Some(NumericLiteral::new(unsuffixed, suffix, float))
} else {
None
}
}

#[must_use]
fn new(lit: &'a str, suffix: Option<&'a str>, float: bool) -> Self {
// Determine delimiter for radix prefix, if present, and radix.
let radix = if lit.starts_with("0x") {
Radix::Hexadecimal
} else if lit.starts_with("0b") {
Radix::Binary
} else if lit.starts_with("0o") {
Radix::Octal
} else {
Radix::Decimal
};

// Grab part of the literal after prefix, if present.
let (prefix, mut sans_prefix) = if let Radix::Decimal = radix {
(None, lit)
} else {
let (p, s) = lit.split_at(2);
(Some(p), s)
};

if suffix.is_some() && sans_prefix.ends_with('_') {
// The '_' before the suffix isn't part of the digits
sans_prefix = &sans_prefix[..sans_prefix.len() - 1];
}

let (integer, fraction, exponent) = Self::split_digit_parts(sans_prefix, float);

Self {
radix,
prefix,
integer,
fraction,
exponent,
suffix,
}
}

fn split_digit_parts(digits: &str, float: bool) -> (&str, Option<&str>, Option<(char, &str)>) {
let mut integer = digits;
let mut fraction = None;
let mut exponent = None;

if float {
for (i, c) in digits.char_indices() {
match c {
'.' => {
integer = &digits[..i];
fraction = Some(&digits[i + 1..]);
},
'e' | 'E' => {
if integer.len() > i {
integer = &digits[..i];
} else {
fraction = Some(&digits[integer.len() + 1..i]);
};
exponent = Some((c, &digits[i + 1..]));
break;
},
_ => {},
}
}
}

(integer, fraction, exponent)
}

/// Returns literal formatted in a sensible way.
fn format(&self) -> String {
let mut output = String::new();

if let Some(prefix) = self.prefix {
output.push_str(prefix);
}

let group_size = self.radix.suggest_grouping();

Self::group_digits(
&mut output,
self.integer,
group_size,
true,
self.radix == Radix::Hexadecimal,
);

if let Some(fraction) = self.fraction {
output.push('.');
Self::group_digits(&mut output, fraction, group_size, false, false);
}

if let Some((separator, exponent)) = self.exponent {
output.push(separator);
Self::group_digits(&mut output, exponent, group_size, true, false);
}

if let Some(suffix) = self.suffix {
output.push('_');
output.push_str(suffix);
}

output
}

fn group_digits(output: &mut String, input: &str, group_size: usize, partial_group_first: bool, pad: bool) {
debug_assert!(group_size > 0);

let mut digits = input.chars().filter(|&c| c != '_');

let first_group_size;

if partial_group_first {
first_group_size = (digits.clone().count() - 1) % group_size + 1;
if pad {
for _ in 0..group_size - first_group_size {
output.push('0');
}
}
} else {
first_group_size = group_size;
}

for _ in 0..first_group_size {
if let Some(digit) = digits.next() {
output.push(digit);
}
}

for (c, i) in digits.zip((0..group_size).cycle()) {
if i == 0 {
output.push('_');
}
output.push(c);
}
}
}

fn split_suffix<'a>(src: &'a str, lit_kind: &LitKind) -> (&'a str, Option<&'a str>) {
debug_assert!(lit_kind.is_numeric());
if let Some(suffix_length) = lit_suffix_length(lit_kind) {
let (unsuffixed, suffix) = src.split_at(src.len() - suffix_length);
(unsuffixed, Some(suffix))
} else {
(src, None)
}
}

fn lit_suffix_length(lit_kind: &LitKind) -> Option<usize> {
debug_assert!(lit_kind.is_numeric());
let suffix = match lit_kind {
LitKind::Int(_, int_lit_kind) => match int_lit_kind {
LitIntType::Signed(int_ty) => Some(int_ty.name_str()),
LitIntType::Unsigned(uint_ty) => Some(uint_ty.name_str()),
LitIntType::Unsuffixed => None,
},
LitKind::Float(_, float_lit_kind) => match float_lit_kind {
LitFloatType::Suffixed(float_ty) => Some(float_ty.name_str()),
LitFloatType::Unsuffixed => None,
},
_ => None,
};

suffix.map(str::len)
}

enum WarningType {
UnreadableLiteral,
InconsistentDigitGrouping,
Expand Down
41 changes: 22 additions & 19 deletions clippy_lints/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ use crate::consts::{constant, Constant};
use crate::utils::paths;
use crate::utils::{
clip, comparisons, differing_macro_contexts, higher, in_constant, int_bits, last_path_segment, match_def_path,
match_path, method_chain_args, multispan_sugg, qpath_res, same_tys, sext, snippet, snippet_opt,
snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg,
span_lint_and_then, unsext,
match_path, method_chain_args, multispan_sugg, numeric_literal::NumericLiteral, qpath_res, same_tys, sext, snippet,
snippet_opt, snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help,
span_lint_and_sugg, span_lint_and_then, unsext,
};

declare_clippy_lint! {
Expand Down Expand Up @@ -1210,22 +1210,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Casts {
let (cast_from, cast_to) = (cx.tables.expr_ty(ex), cx.tables.expr_ty(expr));
lint_fn_to_numeric_cast(cx, expr, ex, cast_from, cast_to);
if let ExprKind::Lit(ref lit) = ex.kind {
if let LitKind::Int(n, _) = lit.node {
if cast_to.is_floating_point() {
let from_nbits = 128 - n.leading_zeros();
let to_nbits = fp_ty_mantissa_nbits(cast_to);
if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits {
span_lint_and_sugg(
cx,
UNNECESSARY_CAST,
expr.span,
&format!("casting integer literal to `{}` is unnecessary", cast_to),
"try",
format!("{}_{}", n, cast_to),
Applicability::MachineApplicable,
);
return;
}
if_chain! {
if let LitKind::Int(n, _) = lit.node;
if let Some(src) = snippet_opt(cx, lit.span);
if cast_to.is_floating_point();
if let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node);
let from_nbits = 128 - n.leading_zeros();
let to_nbits = fp_ty_mantissa_nbits(cast_to);
if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal();
then {
span_lint_and_sugg(
cx,
UNNECESSARY_CAST,
expr.span,
&format!("casting integer literal to `{}` is unnecessary", cast_to),
"try",
format!("{}_{}", n, cast_to),
Applicability::MachineApplicable,
);
return;
}
}
match lit.node {
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub mod higher;
mod hir_utils;
pub mod inspector;
pub mod internal_lints;
pub mod numeric_literal;
pub mod paths;
pub mod ptr;
pub mod sugg;
Expand Down
Loading