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

feat(linter) eslint plugin unicorn: prefer math trunc #1466

Merged
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
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ mod unicorn {
pub mod prefer_dom_node_dataset;
pub mod prefer_event_target;
pub mod prefer_logical_operator_over_ternary;
pub mod prefer_math_trunc;
pub mod prefer_optional_catch_binding;
pub mod prefer_query_selector;
pub mod prefer_regexp_test;
Expand Down Expand Up @@ -346,6 +347,7 @@ oxc_macros::declare_all_lint_rules! {
unicorn::prefer_dom_node_dataset,
unicorn::prefer_event_target,
unicorn::prefer_logical_operator_over_ternary,
unicorn::prefer_math_trunc,
unicorn::prefer_optional_catch_binding,
unicorn::prefer_query_selector,
unicorn::prefer_regexp_test,
Expand Down
183 changes: 183 additions & 0 deletions crates/oxc_linter/src/rules/unicorn/prefer_math_trunc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
use oxc_ast::{ast::Expression, AstKind};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::{self, Error},
};
use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};
use oxc_syntax::operator::{AssignmentOperator, BinaryOperator, UnaryOperator};

use crate::{context::LintContext, rule::Rule, AstNode};

#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-unicorn(prefer-math-trunc): Prefer `Math.trunc()` over instead of `{1} 0`.")]
#[diagnostic(severity(warning))]
struct PreferMathTruncDiagnostic(#[label] pub Span, pub &'static str);

#[derive(Debug, Default, Clone)]
pub struct PreferMathTrunc;

declare_oxc_lint!(
/// ### What it does
///
/// Prefers use of [`Math.trunc()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/trunc) instead of bitwise operations for clarity and more reliable results.
///
/// It prevents the use of the following bitwise operations:
/// - `x | 0` ([`bitwise OR`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Bitwise_OR) with 0)
/// - `~~x` (two [`bitwise NOT`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Bitwise_NOT))
/// - `x >> 0` ([`Signed Right Shift`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Right_shift) with 0)
/// - `x << 0` ([`Left Shift`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Left_shift) with 0)
/// - `x ^ 0` ([`bitwise XOR Shift`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Bitwise_XOR) with 0)
///
/// ### Why is this bad?
///
/// Using bitwise operations to truncate numbers is not clear and do not work in [some cases](https://stackoverflow.com/a/34706108/11687747).
///
/// ### Example
/// ```javascript
/// // Bad
/// const foo = 1.1 | 0;
///
/// // Good
/// const foo = Math.trunc(1.1);
/// ```
PreferMathTrunc,
pedantic
);

impl Rule for PreferMathTrunc {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let operator = match node.kind() {
AstKind::UnaryExpression(unary_expr) => {
if !matches!(unary_expr.operator, UnaryOperator::BitwiseNot) {
return;
}
let Expression::UnaryExpression(inner_unary_expr) = &unary_expr.argument else {
return;
};
if !matches!(inner_unary_expr.operator, UnaryOperator::BitwiseNot) {
return;
};

if let Expression::UnaryExpression(inner_inner_unary_expr) =
&inner_unary_expr.argument
{
if matches!(inner_inner_unary_expr.operator, UnaryOperator::BitwiseNot) {
return;
}
}

UnaryOperator::BitwiseNot.as_str()
}
AstKind::BinaryExpression(bin_expr) => {
let Expression::NumberLiteral(right_num_lit) = &bin_expr.right else {
return;
};
if right_num_lit.value != 0.0 {
return;
}
if !matches!(
bin_expr.operator,
BinaryOperator::BitwiseOR
| BinaryOperator::ShiftRight
| BinaryOperator::ShiftRightZeroFill
| BinaryOperator::ShiftLeft
| BinaryOperator::BitwiseXOR
) {
return;
}

bin_expr.operator.as_str()
}
AstKind::AssignmentExpression(assignment_expr) => {
let Expression::NumberLiteral(right_num_lit) = &assignment_expr.right else {
return;
};

if right_num_lit.value != 0.0 {
return;
}

if !matches!(
assignment_expr.operator,
AssignmentOperator::BitwiseOR
| AssignmentOperator::ShiftRight
| AssignmentOperator::ShiftRightZeroFill
| AssignmentOperator::ShiftLeft
| AssignmentOperator::BitwiseXOR
) {
return;
}

assignment_expr.operator.as_str()
}
_ => {
return;
}
};

ctx.diagnostic(PreferMathTruncDiagnostic(node.kind().span(), operator));
}
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
r"const foo = 1 | 1;",
r"const foo = 0 | 1;",
r"const foo = 1.4 | +0;",
r"const foo = 1.4 | -0;",
r"const foo = 1.4 | (.5 - 0.5);",
r"const foo = 1.4 & 0xFFFFFFFF",
r"const foo = 1.4 & 0xFF",
r"const foo = 1.4 & 0x0",
r"const foo = 1.4 & 0",
r"const foo = ~3.9;",
r"const foo = 1.1 >> 1",
r"const foo = 0 << 1",
];

let fail = vec![
r"const foo = 1.1 | 0;",
r"const foo = 111 | 0;",
r"const foo = (1 + 2 / 3.4) | 0;",
r"const foo = bar((1.4 | 0) + 2);",
r"const foo = (0, 1.4) | 0;",
r"function foo() {return.1 | 0;}",
r"const foo = 1.4 | 0.;",
r"const foo = 1.4 | .0;",
r"const foo = 1.4 | 0.0000_0000_0000;",
r"const foo = 1.4 | 0b0;",
r"const foo = 1.4 | 0x0000_0000_0000;",
r"const foo = 1.4 | 0o0;",
r"const foo = 1.23 | 0 | 4;",
r"const foo = ~~3.9;",
r"const foo = ~~111;",
r"const foo = ~~(1 + 2 / 3.4);",
r"const foo = ~~1 + 2 / 3.4;",
r"const foo = ~~(0, 1.4);",
r"const foo = ~~~10.01;",
r"const foo = ~~(~10.01);",
r"const foo = ~(~~10.01);",
r"const foo = ~~-10.01;",
r"const foo = ~~~~10.01;",
r"function foo() {return~~3.9;}",
r"const foo = bar >> 0;",
r"const foo = bar << 0;",
r"const foo = bar ^ 0;",
r"function foo() {return.1 ^0;}",
r"function foo() {return[foo][0] ^= 0;};",
r"const foo = /* first comment */ 3.4 | 0; // A B C",
r"const foo = /* first comment */ ~~3.4; // A B C",
r"const foo = /* will keep */ 3.4 /* will remove 1 */ | /* will remove 2 */ 0;",
r"const foo = /* will keep */ ~ /* will remove 1 */ ~ /* will remove 2 */ 3.4;",
r"const foo = ~~bar | 0;",
r"const foo = ~~(bar| 0);",
r"const foo = bar | 0 | 0;",
r"const foo = ~~~~((bar | 0 | 0) >> 0 >> 0 << 0 << 0 ^ 0 ^0);",
];

Tester::new_without_config(PreferMathTrunc::NAME, pass, fail).test_and_snapshot();
}
Loading
Loading