diff --git a/clippy_lints/src/float_literal.rs b/clippy_lints/src/float_literal.rs index 30d4b25318f7f..55a9f61242b43 100644 --- a/clippy_lints/src/float_literal.rs +++ b/clippy_lints/src/float_literal.rs @@ -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}; @@ -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, ); } @@ -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, ); } diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index 0fcfb7b884970..cee14dc893b53 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -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; @@ -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 @@ -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 { diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 66435f954231c..9e6b63fafd094 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -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}; @@ -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> { - 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 { - 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, diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 58a909c845d6a..e1685c97a065d 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -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! { @@ -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 { diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 640cce1138063..dc8775b43b19f 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -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; diff --git a/clippy_lints/src/utils/numeric_literal.rs b/clippy_lints/src/utils/numeric_literal.rs new file mode 100644 index 0000000000000..8b3492724e101 --- /dev/null +++ b/clippy_lints/src/utils/numeric_literal.rs @@ -0,0 +1,227 @@ +use rustc_ast::ast::{Lit, LitFloatType, LitIntType, LitKind}; + +#[derive(Debug, PartialEq)] +pub(crate) 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(lit: &str, type_suffix: Option<&str>, float: bool) -> String { + NumericLiteral::new(lit, type_suffix, float).format() +} + +#[derive(Debug)] +pub(crate) struct NumericLiteral<'a> { + /// Which radix the literal was represented in. + pub radix: Radix, + /// The radix prefix, if present. + pub prefix: Option<&'a str>, + + /// The integer part of the number. + pub integer: &'a str, + /// The fraction part of the number. + pub fraction: Option<&'a str>, + /// The character used as exponent seperator (b'e' or b'E') and the exponent part. + pub exponent: Option<(char, &'a str)>, + + /// The type suffix, including preceding underscore if present. + pub suffix: Option<&'a str>, +} + +impl<'a> NumericLiteral<'a> { + pub fn from_lit(src: &'a str, lit: &Lit) -> Option> { + NumericLiteral::from_lit_kind(src, &lit.kind) + } + + pub fn from_lit_kind(src: &'a str, lit_kind: &LitKind) -> Option> { + 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] + pub 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, + } + } + + pub fn is_decimal(&self) -> bool { + self.radix == Radix::Decimal + } + + pub 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. + pub 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 + } + + pub 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 { + 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) +} diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 05cc4f33eaede..8cf3c07a2d427 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -15,8 +15,6 @@ use std::borrow::Cow; use std::convert::TryInto; use std::fmt::Display; -pub use crate::literal_representation::format_numeric_literal; - /// A helper type to build suggestion correctly handling parenthesis. pub enum Sugg<'a> { /// An expression that never needs parenthesis such as `1337` or `[0; 42]`. diff --git a/tests/ui/unnecessary_cast_fixable.fixed b/tests/ui/unnecessary_cast_fixable.fixed index 8c659df831e3d..fb89a9fce3d5b 100644 --- a/tests/ui/unnecessary_cast_fixable.fixed +++ b/tests/ui/unnecessary_cast_fixable.fixed @@ -14,4 +14,10 @@ fn main() { &v as &[i32]; 1.0 as f64; 1 as u64; + 0x10 as f32; + 0o10 as f32; + 0b10 as f32; + 0x11 as f64; + 0o11 as f64; + 0b11 as f64; } diff --git a/tests/ui/unnecessary_cast_fixable.rs b/tests/ui/unnecessary_cast_fixable.rs index 7bab5540c42f7..4a0c8620dc134 100644 --- a/tests/ui/unnecessary_cast_fixable.rs +++ b/tests/ui/unnecessary_cast_fixable.rs @@ -14,4 +14,10 @@ fn main() { &v as &[i32]; 1.0 as f64; 1 as u64; + 0x10 as f32; + 0o10 as f32; + 0b10 as f32; + 0x11 as f64; + 0o11 as f64; + 0b11 as f64; }