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

Move check for lossy whole-number floats out of excessive_precision #5202

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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,7 @@ Released 2018-09-13
[`let_unit_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value
[`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist
[`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug
[`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
[`manual_mul_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_mul_add
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 355 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 356 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,53 @@ declare_clippy_lint! {
/// **What it does:** Checks for float literals with a precision greater
/// than that supported by the underlying type.
///
/// **Why is this bad?** Rust will silently lose precision during conversion
/// to a float.
/// **Why is this bad?** Rust will truncate the literal silently.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// // Bad
/// let a: f32 = 0.123_456_789_9; // 0.123_456_789
/// let b: f32 = 16_777_217.0; // 16_777_216.0
/// let v: f32 = 0.123_456_789_9;
/// println!("{}", v); // 0.123_456_789
///
/// // Good
/// let a: f64 = 0.123_456_789_9;
/// let b: f64 = 16_777_216.0;
/// let v: f64 = 0.123_456_789_9;
/// println!("{}", v); // 0.123_456_789_9
/// ```
pub EXCESSIVE_PRECISION,
correctness,
style,
"excessive precision for float literal"
}

declare_lint_pass!(ExcessivePrecision => [EXCESSIVE_PRECISION]);
declare_clippy_lint! {
/// **What it does:** Checks for whole number float literals that
/// cannot be represented as the underlying type without loss.
///
/// **Why is this bad?** Rust will silently lose precision during
/// conversion to a float.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// // Bad
/// let _: f32 = 16_777_217.0; // 16_777_216.0
///
/// // Good
/// let _: f32 = 16_777_216.0;
/// let _: f64 = 16_777_217.0;
/// ```
pub LOSSY_FLOAT_LITERAL,
restriction,
"lossy whole number float literals"
}

declare_lint_pass!(FloatLiteral => [EXCESSIVE_PRECISION, LOSSY_FLOAT_LITERAL]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatLiteral {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>) {
if_chain! {
let ty = cx.tables.expr_ty(expr);
Expand All @@ -52,26 +75,41 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision {
// since we'll need the truncated string anyway.
let digits = count_digits(&sym_str);
let max = max_digits(fty);
let float_str = match fty {
FloatTy::F32 => sym_str.parse::<f32>().map(|f| formatter.format(f)),
FloatTy::F64 => sym_str.parse::<f64>().map(|f| formatter.format(f)),
}.unwrap();
let type_suffix = match lit_float_ty {
LitFloatType::Suffixed(FloatTy::F32) => Some("f32"),
LitFloatType::Suffixed(FloatTy::F64) => Some("f64"),
_ => None
};
let (is_whole, mut float_str) = match fty {
FloatTy::F32 => {
let value = sym_str.parse::<f32>().unwrap();

(value.fract() == 0.0, formatter.format(value))
},
FloatTy::F64 => {
let value = sym_str.parse::<f64>().unwrap();

(value.fract() == 0.0, formatter.format(value))
},
};

if is_whole_number(&sym_str, fty) {
if is_whole && !sym_str.contains(|c| c == 'e' || c == 'E') {
// Normalize the literal by stripping the fractional portion
if sym_str.split('.').next().unwrap() != float_str {
// If the type suffix is missing the suggestion would be
// incorrectly interpreted as an integer so adding a `.0`
// suffix to prevent that.
if type_suffix.is_none() {
float_str.push_str(".0");
}

span_lint_and_sugg(
cx,
EXCESSIVE_PRECISION,
LOSSY_FLOAT_LITERAL,
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(format!("{}.0", float_str).as_str(), type_suffix, true),
format_numeric_literal(&float_str, type_suffix, true),
Applicability::MachineApplicable,
);
}
Expand All @@ -91,15 +129,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision {
}
}

// Checks whether a float literal is a whole number
#[must_use]
fn is_whole_number(sym_str: &str, fty: FloatTy) -> bool {
match fty {
FloatTy::F32 => sym_str.parse::<f32>().unwrap().fract() == 0.0,
FloatTy::F64 => sym_str.parse::<f64>().unwrap().fract() == 0.0,
}
}

#[must_use]
fn max_digits(fty: FloatTy) -> u32 {
match fty {
Expand Down
12 changes: 7 additions & 5 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,10 @@ pub mod escape;
pub mod eta_reduction;
pub mod eval_order_dependence;
pub mod excessive_bools;
pub mod excessive_precision;
pub mod exit;
pub mod explicit_write;
pub mod fallible_impl_from;
pub mod float_literal;
pub mod format;
pub mod formatting;
pub mod functions;
Expand Down Expand Up @@ -534,10 +534,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&eval_order_dependence::EVAL_ORDER_DEPENDENCE,
&excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS,
&excessive_bools::STRUCT_EXCESSIVE_BOOLS,
&excessive_precision::EXCESSIVE_PRECISION,
&exit::EXIT,
&explicit_write::EXPLICIT_WRITE,
&fallible_impl_from::FALLIBLE_IMPL_FROM,
&float_literal::EXCESSIVE_PRECISION,
&float_literal::LOSSY_FLOAT_LITERAL,
&format::USELESS_FORMAT,
&formatting::POSSIBLE_MISSING_COMMA,
&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
Expand Down Expand Up @@ -836,7 +837,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box eq_op::EqOp);
store.register_late_pass(|| box enum_glob_use::EnumGlobUse);
store.register_late_pass(|| box enum_clike::UnportableVariant);
store.register_late_pass(|| box excessive_precision::ExcessivePrecision);
store.register_late_pass(|| box float_literal::FloatLiteral);
let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold;
store.register_late_pass(move || box bit_mask::BitMask::new(verbose_bit_mask_threshold));
store.register_late_pass(|| box ptr::Ptr);
Expand Down Expand Up @@ -1016,6 +1017,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&dbg_macro::DBG_MACRO),
LintId::of(&else_if_without_else::ELSE_IF_WITHOUT_ELSE),
LintId::of(&exit::EXIT),
LintId::of(&float_literal::LOSSY_FLOAT_LITERAL),
LintId::of(&implicit_return::IMPLICIT_RETURN),
LintId::of(&indexing_slicing::INDEXING_SLICING),
LintId::of(&inherent_impl::MULTIPLE_INHERENT_IMPL),
Expand Down Expand Up @@ -1159,8 +1161,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&eta_reduction::REDUNDANT_CLOSURE),
LintId::of(&eval_order_dependence::DIVERGING_SUB_EXPRESSION),
LintId::of(&eval_order_dependence::EVAL_ORDER_DEPENDENCE),
LintId::of(&excessive_precision::EXCESSIVE_PRECISION),
LintId::of(&explicit_write::EXPLICIT_WRITE),
LintId::of(&float_literal::EXCESSIVE_PRECISION),
LintId::of(&format::USELESS_FORMAT),
LintId::of(&formatting::POSSIBLE_MISSING_COMMA),
LintId::of(&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
Expand Down Expand Up @@ -1386,6 +1388,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&enum_variants::MODULE_INCEPTION),
LintId::of(&eq_op::OP_REF),
LintId::of(&eta_reduction::REDUNDANT_CLOSURE),
LintId::of(&float_literal::EXCESSIVE_PRECISION),
LintId::of(&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING),
LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
Expand Down Expand Up @@ -1566,7 +1569,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT),
LintId::of(&eq_op::EQ_OP),
LintId::of(&erasing_op::ERASING_OP),
LintId::of(&excessive_precision::EXCESSIVE_PRECISION),
LintId::of(&formatting::POSSIBLE_MISSING_COMMA),
LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF),
LintId::of(&indexing_slicing::OUT_OF_BOUNDS_INDEXING),
Expand Down
13 changes: 10 additions & 3 deletions src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 355] = [
pub const ALL_LINTS: [Lint; 356] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -492,10 +492,10 @@ pub const ALL_LINTS: [Lint; 355] = [
},
Lint {
name: "excessive_precision",
group: "correctness",
group: "style",
desc: "excessive precision for float literal",
deprecation: None,
module: "excessive_precision",
module: "float_literal",
},
Lint {
name: "exit",
Expand Down Expand Up @@ -1001,6 +1001,13 @@ pub const ALL_LINTS: [Lint; 355] = [
deprecation: None,
module: "booleans",
},
Lint {
name: "lossy_float_literal",
group: "restriction",
desc: "lossy whole number float literals",
deprecation: None,
module: "float_literal",
},
Lint {
name: "main_recursion",
group: "style",
Expand Down
22 changes: 0 additions & 22 deletions tests/ui/excessive_precision.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -60,26 +60,4 @@ fn main() {

// issue #2840
let num = 0.000_000_000_01e-10f64;

// Lossy whole-number float literals
let _: f32 = 16_777_216.0;
let _: f32 = 16_777_220.0;
let _: f32 = 16_777_220.0;
let _: f32 = 16_777_220.0;
let _ = 16_777_220.0_f32;
let _: f32 = -16_777_220.0;
let _: f64 = 9_007_199_254_740_992.0;
let _: f64 = 9_007_199_254_740_992.0;
let _: f64 = 9_007_199_254_740_992.0;
let _ = 9_007_199_254_740_992.0_f64;
let _: f64 = -9_007_199_254_740_992.0;

// Lossless whole number float literals
let _: f32 = 16_777_216.0;
let _: f32 = 16_777_218.0;
let _: f32 = 16_777_220.0;
let _: f32 = -16_777_216.0;
let _: f32 = -16_777_220.0;
let _: f64 = 9_007_199_254_740_992.0;
let _: f64 = -9_007_199_254_740_992.0;
}
22 changes: 0 additions & 22 deletions tests/ui/excessive_precision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,26 +60,4 @@ fn main() {

// issue #2840
let num = 0.000_000_000_01e-10f64;

// Lossy whole-number float literals
let _: f32 = 16_777_217.0;
let _: f32 = 16_777_219.0;
let _: f32 = 16_777_219.;
let _: f32 = 16_777_219.000;
let _ = 16_777_219f32;
let _: f32 = -16_777_219.0;
let _: f64 = 9_007_199_254_740_993.0;
let _: f64 = 9_007_199_254_740_993.;
let _: f64 = 9_007_199_254_740_993.000;
let _ = 9_007_199_254_740_993f64;
let _: f64 = -9_007_199_254_740_993.0;

// Lossless whole number float literals
let _: f32 = 16_777_216.0;
let _: f32 = 16_777_218.0;
let _: f32 = 16_777_220.0;
let _: f32 = -16_777_216.0;
let _: f32 = -16_777_220.0;
let _: f64 = 9_007_199_254_740_992.0;
let _: f64 = -9_007_199_254_740_992.0;
}
68 changes: 1 addition & 67 deletions tests/ui/excessive_precision.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -108,71 +108,5 @@ error: float has excessive precision
LL | let bad_bige32: f32 = 1.123_456_788_888E-10;
| ^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8E-10`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:65:18
|
LL | let _: f32 = 16_777_217.0;
| ^^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_216.0`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:66:18
|
LL | let _: f32 = 16_777_219.0;
| ^^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_220.0`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:67:18
|
LL | let _: f32 = 16_777_219.;
| ^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_220.0`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:68:18
|
LL | let _: f32 = 16_777_219.000;
| ^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_220.0`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:69:13
|
LL | let _ = 16_777_219f32;
| ^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_220.0_f32`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:70:19
|
LL | let _: f32 = -16_777_219.0;
| ^^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_220.0`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:71:18
|
LL | let _: f64 = 9_007_199_254_740_993.0;
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `9_007_199_254_740_992.0`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:72:18
|
LL | let _: f64 = 9_007_199_254_740_993.;
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `9_007_199_254_740_992.0`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:73:18
|
LL | let _: f64 = 9_007_199_254_740_993.000;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `9_007_199_254_740_992.0`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:74:13
|
LL | let _ = 9_007_199_254_740_993f64;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `9_007_199_254_740_992.0_f64`

error: literal cannot be represented as the underlying type without loss of precision
--> $DIR/excessive_precision.rs:75:19
|
LL | let _: f64 = -9_007_199_254_740_993.0;
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `9_007_199_254_740_992.0`

error: aborting due to 29 previous errors
error: aborting due to 18 previous errors

Loading