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

fix(linter): fix false positives in loss-of-precision lint #664

Merged
merged 5 commits into from
Jul 31, 2023
Merged

fix(linter): fix false positives in loss-of-precision lint #664

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
1d3fc3d
fix(linter): fix false positives in loss-of-precision lint
Devin-Yeung Jul 30, 2023
e476764
perf(linter): make regex static
Devin-Yeung Jul 30, 2023
0d60455
perf(linter): change `OnceLock` to `Lazy`
Devin-Yeung Jul 31, 2023
0f61a9b
chore(linter): code style fix
Devin-Yeung Jul 31, 2023
4c92015
change to correctness
Boshen Jul 31, 2023
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 Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/oxc_linter/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ phf = { workspace = true, features = ["macros"] }
num-traits = { workspace = true }

rust-lapper = "1.1.0"
once_cell = "1.18.0"

[dev-dependencies]
oxc_parser = { workspace = true }
Expand Down
222 changes: 117 additions & 105 deletions crates/oxc_linter/src/rules/eslint/no_loss_of_precision.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use once_cell::sync::Lazy;
use oxc_ast::ast::NumberLiteral;
use oxc_ast::AstKind;
use oxc_diagnostics::{
Expand All @@ -6,6 +7,7 @@ use oxc_diagnostics::{
};
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use regex::Regex;
use std::borrow::Cow;

use crate::{context::LintContext, rule::Rule, AstNode};
Expand Down Expand Up @@ -34,7 +36,7 @@ declare_oxc_lint!(
/// var x = 2e999;
/// ```
NoLossOfPrecision,
nursery // There are false positives, see https://github.com/web-infra-dev/oxc/issues/656
correctness
);

impl Rule for NoLossOfPrecision {
Expand All @@ -48,18 +50,110 @@ impl Rule for NoLossOfPrecision {
}
}

#[derive(Debug, Eq)]
pub struct NormalizedNum<'a> {
magnitude: isize,
coefficient: Cow<'a, str>,
pub struct RawNum<'a> {
int: &'a str,
frac: &'a str,
exp: isize,
}

impl<'a> PartialEq for NormalizedNum<'a> {
#[derive(Debug)]
pub struct ScientificNotation<'a> {
int: &'a str,
frac: Cow<'a, str>,
exp: isize,
scientific: bool,
precision: usize,
}

impl PartialEq for ScientificNotation<'_> {
fn eq(&self, other: &Self) -> bool {
if self.coefficient == "0" {
true
if self.int == other.int && self.frac == other.frac {
if self.int == "0" && self.frac == "" {
return true;
}
return self.exp == other.exp;
}
false
}
}

static RE: Lazy<Regex> = Lazy::new(|| {
Regex::new(r"-?0*(?P<int>0|[1-9]\d*)?(?:\.(?P<frac>\d+))?(?:[eE](?P<exp>[+-]?\d+))?").unwrap()
});

impl<'a> RawNum<'a> {
fn new(num: &str) -> Option<RawNum<'_>> {
if let Some(captures) = RE.captures(num) {
let int = captures.name("int").map_or("0", |m| m.as_str());
let frac = captures.name("frac").map_or("", |m| m.as_str());
let exp = captures.name("exp").map_or("0", |m| m.as_str());

let exp = match exp.parse::<isize>() {
Ok(x) => x,
Err(_) => return None,
};

Some(RawNum { int, frac, exp })
} else {
None
}
}

fn normalize(&mut self) -> ScientificNotation<'a> {
let scientific = self.exp != 0;
let precision = self.frac.len();
if self.int.starts_with('0') {
#[allow(clippy::cast_possible_wrap)]
let exp = self.exp - 1 - self.frac.chars().take_while(|&ch| ch == '0').count() as isize;
self.frac = self.frac.trim_start_matches('0');

match self.frac.len() {
0 => ScientificNotation {
int: "0",
frac: Cow::Borrowed(""),
exp,
scientific,
precision,
},
1 => ScientificNotation {
int: &self.frac[..1],
frac: Cow::Borrowed(""),
exp,
scientific,
precision,
},
_ => ScientificNotation {
int: &self.frac[..1],
frac: Cow::Borrowed(&self.frac[1..]),
exp,
scientific,
precision,
},
}
} else {
self.magnitude == other.magnitude && self.coefficient == other.coefficient
#[allow(clippy::cast_possible_wrap)]
let exp = self.exp + self.int.len() as isize - 1;
if self.int.len() == 1 {
ScientificNotation {
int: self.int,
frac: Cow::Borrowed(self.frac),
exp,
scientific,
precision,
}
} else {
ScientificNotation {
int: &self.int[..1],
frac: Cow::Owned(
format!("{}{}", &self.int[1..], self.frac)
.trim_end_matches('0')
.to_string(),
),
exp,
scientific,
precision,
}
}
}
}
}
Expand Down Expand Up @@ -89,107 +183,23 @@ impl NoLossOfPrecision {
}

fn base_ten_loses_precision(node: &'_ NumberLiteral) -> bool {
let normalized_raw_num =
if let Some(s) = Self::normalize(Self::get_raw(node)) { s } else { return true };
let precision = normalized_raw_num.coefficient.len();
let raw = Self::get_raw(node);
let raw = if let Some(s) = Self::normalize(&raw) { s } else { return true };

if precision > 100 {
if raw.frac.len() >= 100 {
return true;
}

let stored_num = Cow::Owned(format!("{:1$}", node.value, precision));
let normalized_stored_num =
if let Some(s) = Self::normalize(stored_num) { s } else { return true };
normalized_raw_num != normalized_stored_num
}

fn remove_leading_zeros(num: Cow<'_, str>) -> Cow<'_, str> {
if num.starts_with('0') {
Cow::Owned(
match num.trim_start_matches('0') {
"" => "0",
s => s,
}
.to_string(),
)
} else {
num
}
}

fn remove_trailing_zeros(num: Cow<'_, str>) -> Cow<'_, str> {
if num.ends_with('0') {
Cow::Owned(
match num.trim_end_matches('0') {
"" => "0",
s => s,
}
.to_string(),
)
} else {
num
}
}

fn normalize_int(num: Cow<'_, str>) -> NormalizedNum<'_> {
// specially deal with 0
if num == "0" {
return NormalizedNum { magnitude: 0, coefficient: Cow::Borrowed("0") };
}

#[allow(clippy::cast_possible_wrap)]
// the length of number is larger then isize is almost impossible in real-world codebase
let magnitude =
if num.starts_with('0') { num.len() as isize - 2 } else { num.len() as isize - 1 };
let significant_digits = Self::remove_leading_zeros(Self::remove_trailing_zeros(num));
NormalizedNum { magnitude, coefficient: significant_digits }
}

fn normalize_float(num: Cow<'_, str>) -> NormalizedNum<'_> {
let trimmed_float = Self::remove_leading_zeros(num);

return trimmed_float.strip_prefix('.').map_or_else(
|| {
let trimmed_float = trimmed_float.trim_end_matches('0');
// unwrap here will never panic, we guarantee the input contains a `.`
#[allow(clippy::cast_possible_wrap)]
let magnitude = (trimmed_float.find('.').unwrap() - 1) as isize;
NormalizedNum { coefficient: Cow::Owned(trimmed_float.replace('.', "")), magnitude }
},
|stripped| {
let decimal_digits = stripped.len();
let significant_digits =
Self::remove_leading_zeros(Cow::Owned(stripped.to_string()));
#[allow(clippy::cast_possible_wrap)]
// the length of number is larger then isize is almost impossible in real-world codebase
let magnitude = significant_digits.len() as isize - decimal_digits as isize - 1;
NormalizedNum { coefficient: significant_digits, magnitude }
},
);
}

fn normalize<'a, S: AsRef<str>>(num: S) -> Option<NormalizedNum<'a>> {
let split_num = num.as_ref().split(|c| c == 'e' || c == 'E').collect::<Vec<_>>();
let original_coefficient = Cow::Owned(split_num[0].to_owned());
let normalize_num = if num.as_ref().contains('.') {
Self::normalize_float(original_coefficient)
} else {
Self::normalize_int(original_coefficient)
};

let coefficient = normalize_num.coefficient;

let magnitude = if split_num.len() > 1 {
if let Ok(n) = split_num[1].parse::<isize>() {
n + normalize_num.magnitude
} else {
return None;
}
} else {
normalize_num.magnitude
let stored = match (raw.scientific, raw.precision) {
(true, _) => format!("{:.1$e}", node.value, raw.frac.len()),
(false, 0) => format!("{}", node.value),
(false, precision) => format!("{:.1$}", node.value, precision),
};
let stored = if let Some(s) = Self::normalize(&stored) { s } else { return true };
raw != stored
}

Some(NormalizedNum { magnitude, coefficient })
fn normalize(num: &str) -> Option<ScientificNotation<'_>> {
Some(RawNum::new(num)?.normalize())
}

pub fn lose_precision(node: &'_ NumberLiteral) -> bool {
Expand Down Expand Up @@ -270,6 +280,8 @@ fn test() {
("var x = 0x1FFF_FFFF_FFF_FFF", None),
("var x = 0X1_FFF_FFFF_FFF_FFF", None),
("var a = Infinity", None),
("var a = 480.00", None),
("var a = -30.00", None),
];

let fail = vec![
Expand Down