feat(linter): bad bitwise operator rule (#246)

This commit is contained in:
Wenzhe Wang 2023-04-02 23:34:37 +08:00 committed by GitHub
parent 236d53ad9d
commit b17335d724
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 346 additions and 0 deletions

View file

@ -27,6 +27,7 @@ oxc_macros::declare_all_lint_rules! {
no_unsafe_negation,
no_bitwise,
deepscan::uninvoked_array_callback,
deepscan::bad_bitwise_operator,
use_isnan,
valid_typeof,
typescript::isolated_declaration

View file

@ -0,0 +1,235 @@
use oxc_ast::{
ast::{AssignmentOperator, BinaryOperator, Expression, UnaryOperator, BinaryExpression},
AstKind, Span,
};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_macros::declare_oxc_lint;
use crate::{context::LintContext, rule::Rule, AstNode};
#[derive(Debug, Error, Diagnostic)]
#[error("Bad bitwise operator")]
#[diagnostic(
severity(warning),
help("Bitwise operator '{0}' seems unintended. Did you mean logical operator '{1}'?")
)]
struct BadBitwiseOperatorDiagnostic(&'static str, &'static str, #[label] pub Span);
#[derive(Debug, Error, Diagnostic)]
#[error("Bad bitwise operator")]
#[diagnostic(
severity(warning),
help(
"Bitwise operator '|=' seems unintended. Consider using non-compound assignment and logical operator '||' instead."
)
)]
struct BadBitwiseOrOperatorDiagnostic(#[label] pub Span);
/// `https://deepscan.io/docs/rules/bad-bitwise-operator`
#[derive(Debug, Default, Clone)]
pub struct BadBitwiseOperator;
declare_oxc_lint!(
/// ### What it does
/// This rule applies when bitwise operators are used where logical operators are expected.
///
/// ### Why is this bad?
/// Bitwise operators have different results from logical operators and a `TypeError` exception may be thrown because short-circuit evaluation is not applied.
/// (In short-circuit evaluation, right operand evaluation is skipped according to left operand value, e.g. `x` is `false` in `x && y`.)
///
/// It is obvious that logical operators are expected in the following code patterns:
/// ```javascript
/// e && e.x
/// e || {}
/// e || ''
/// ```
///
/// ### Example
/// ```javascript
/// if (obj & obj.prop) {
/// console.log(obj.prop);
/// }
/// options = options | {};
/// input |= '';
/// ```
BadBitwiseOperator,
correctness
);
impl Rule for BadBitwiseOperator {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
match node.get().kind() {
AstKind::BinaryExpression(bin_expr) => {
if is_mistype_short_circuit(node) {
ctx.diagnostic(BadBitwiseOperatorDiagnostic("&", "&&", bin_expr.span));
} else if is_mistype_option_fallback(node) {
ctx.diagnostic(BadBitwiseOperatorDiagnostic("|", "||", bin_expr.span));
}
}
AstKind::AssignmentExpression(assign_expr) => {
if assign_expr.operator == AssignmentOperator::BitwiseOR && !is_numeric_expr(&assign_expr.right, true) {
ctx.diagnostic(BadBitwiseOrOperatorDiagnostic(assign_expr.span));
}
}
_ => {}
}
}
}
fn is_mistype_short_circuit(node: &AstNode) -> bool {
match node.get().kind() {
AstKind::BinaryExpression(bin_expr) => {
if bin_expr.operator != BinaryOperator::BitwiseAnd {
return false;
}
let Expression::Identifier(left_ident) = &bin_expr.left else { return false };
if let Expression::MemberExpression(member_expr) = &bin_expr.right {
if let Expression::Identifier(ident) = member_expr.object() {
return ident.name == left_ident.name;
}
}
false
}
_ => false,
}
}
fn is_mistype_option_fallback(node: &AstNode) -> bool {
match node.get().kind() {
AstKind::BinaryExpression(binary_expr) => {
if binary_expr.operator != BinaryOperator::BitwiseOR {
return false;
}
if let Expression::Identifier(_) = &binary_expr.left
&& !is_numeric_expr(&binary_expr.right, true) {
return true
}
false
}
_ => false,
}
}
fn is_numeric_expr(expr: &Expression, is_outer_most: bool) -> bool {
match expr {
Expression::NumberLiteral(_)
| Expression::NullLiteral(_)
// TODO: handle type inference
| Expression::Identifier(_) => true,
Expression::UnaryExpression(unary_expr) => {
if is_outer_most {
unary_expr.operator != UnaryOperator::Typeof && unary_expr.operator != UnaryOperator::LogicalNot
} else {
unary_expr.operator != UnaryOperator::Typeof
}
}
Expression::BinaryExpression(binary_expr) => !is_string_concat(binary_expr),
Expression::ParenthesizedExpression(paren_expr) => {
is_numeric_expr(&paren_expr.expression, false)
}
_ => {
if is_outer_most {
expr.is_undefined()
} else {
!expr.is_string_literal()
}
}
}
}
fn is_string_concat(binary_expr: &BinaryExpression) -> bool {
binary_expr.operator == BinaryOperator::Addition && (
contains_string_literal(&binary_expr.left) || contains_string_literal(&binary_expr.right)
)
}
fn contains_string_literal(expr: &Expression) -> bool {
match expr {
Expression::StringLiteral(_) => true,
Expression::UnaryExpression(unary_expr) => unary_expr.operator == UnaryOperator::Typeof,
Expression::BinaryExpression(binary_expr) => is_string_concat(binary_expr),
Expression::ParenthesizedExpression(paren_expr) => {
contains_string_literal(&paren_expr.expression)
}
_ => false,
}
}
#[test]
fn test() {
use crate::tester::Tester;
let pass = vec![
("var a = obj && obj.a", None),
("var a = obj || obj.a", None),
("var a = obj1 & obj2.a", None),
("var a = b | c", None),
("var a = options || {}", None),
("var a = options | 1", None),
("var a = options | undefined", None),
("var a = options | null", None),
("var a = options | ~{}", None),
("var a = options | (1 + 2 + (3 + !''))", None),
("var a = options | (1 + 2 + (3 + !4))", None),
("var a = options | (1 + 2 + (3 + +false))", None),
("var a = options | (1 + 2 + (3 * '4'))", None),
("var a = options | (1 + 2 + (3 * ('4' + 5)))", None),
("var a = options | (1 + 2 + (3 + 4))", None),
("var a = options | (1 + {})", None),
("var a = 1 | {}", None),
("var a = 1 | ''", None),
("var a = 1 | true", None),
("var a = {} | 1", None),
("var a = '' | 1", None),
("var a = true | 1", None),
("var a = b | (1 + 2 + (3 + c))", None),
("var a = 1 + '2' - 3", None),
("var a = '1' + 2 - 3", None),
("input |= 1", None),
("input |= undefined", None),
("input |= null", None),
("input |= (1 + 1)", None),
("input |= (1 + true)", None),
("input |= (1 + 2 * '3')", None),
("input |= (1 + (2 + {}))", None),
("input |= ('1' + 2 - 3)", None),
("input |= (1 + '2' - 3)", None),
("input |= a", None),
("input |= ~{}", None),
// TODO
// ("var a = 1; input |= a", None),
// ("var a = 1; var b = a | {}", None),
];
let fail = vec![
("var a = obj & obj.a", None),
("var a = options | {}", None),
("var a = options | !{}", None),
("var a = options | typeof {}", None),
("var a = options | ''", None),
("var a = options | true", None),
("var a = options | false", None),
("var a = options | (1 + 2 + typeof {})", None),
("var a = options | (1 + 2 + (3 + ''))", None),
("var a = options | (1 + 2 + (3 + '4'))", None),
("input |= ''", None),
("input |= (1 + '')", None),
("input |= (1 + (3 + '1'))", None),
("input |= !{}", None),
("input |= typeof {}", None),
// TODO
// ("var input; var a = '1'; input |= a", None),
// ("var a = '1'; var b = a | {}", None),
];
Tester::new(BadBitwiseOperator::NAME, pass, fail).test_and_snapshot();
}

View file

@ -0,0 +1,110 @@
---
source: crates/oxc_linter/src/tester.rs
expression: bad_bitwise_operator
---
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ var a = obj & obj.a
· ───────────
╰────
help: Bitwise operator '&' seems unintended. Did you mean logical operator '&&'?
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ var a = options | {}
· ────────────
╰────
help: Bitwise operator '|' seems unintended. Did you mean logical operator '||'?
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ var a = options | !{}
· ─────────────
╰────
help: Bitwise operator '|' seems unintended. Did you mean logical operator '||'?
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ var a = options | typeof {}
· ───────────────────
╰────
help: Bitwise operator '|' seems unintended. Did you mean logical operator '||'?
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ var a = options | ''
· ────────────
╰────
help: Bitwise operator '|' seems unintended. Did you mean logical operator '||'?
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ var a = options | true
· ──────────────
╰────
help: Bitwise operator '|' seems unintended. Did you mean logical operator '||'?
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ var a = options | false
· ───────────────
╰────
help: Bitwise operator '|' seems unintended. Did you mean logical operator '||'?
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ var a = options | (1 + 2 + typeof {})
· ─────────────────────────────
╰────
help: Bitwise operator '|' seems unintended. Did you mean logical operator '||'?
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ var a = options | (1 + 2 + (3 + ''))
· ────────────────────────────
╰────
help: Bitwise operator '|' seems unintended. Did you mean logical operator '||'?
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ var a = options | (1 + 2 + (3 + '4'))
· ─────────────────────────────
╰────
help: Bitwise operator '|' seems unintended. Did you mean logical operator '||'?
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ input |= ''
· ───────────
╰────
help: Bitwise operator '|=' seems unintended. Consider using non-compound assignment and logical operator '||' instead.
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ input |= (1 + '')
· ─────────────────
╰────
help: Bitwise operator '|=' seems unintended. Consider using non-compound assignment and logical operator '||' instead.
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ input |= (1 + (3 + '1'))
· ────────────────────────
╰────
help: Bitwise operator '|=' seems unintended. Consider using non-compound assignment and logical operator '||' instead.
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ input |= !{}
· ────────────
╰────
help: Bitwise operator '|=' seems unintended. Consider using non-compound assignment and logical operator '||' instead.
⚠ Bad bitwise operator
╭─[bad_bitwise_operator.tsx:1:1]
1 │ input |= typeof {}
· ──────────────────
╰────
help: Bitwise operator '|=' seems unintended. Consider using non-compound assignment and logical operator '||' instead.