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) deepscan: bad char at comparison #1750

Merged
merged 1 commit into from
Dec 20, 2023
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 @@ -18,6 +18,7 @@ mod import {
mod deepscan {
pub mod bad_array_method_on_arguments;
pub mod bad_bitwise_operator;
pub mod bad_char_at_comparison;
pub mod bad_comparison_sequence;
pub mod bad_min_max_func;
pub mod bad_replace_all_arg;
Expand Down Expand Up @@ -249,6 +250,7 @@ mod oxc {
oxc_macros::declare_all_lint_rules! {
deepscan::bad_array_method_on_arguments,
deepscan::bad_bitwise_operator,
deepscan::bad_char_at_comparison,
deepscan::bad_comparison_sequence,
deepscan::bad_min_max_func,
deepscan::bad_replace_all_arg,
Expand Down
121 changes: 121 additions & 0 deletions crates/oxc_linter/src/rules/deepscan/bad_char_at_comparison.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
use oxc_ast::{ast::Expression, AstKind};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};
use oxc_syntax::operator::BinaryOperator;

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

#[derive(Debug, Error, Diagnostic)]
#[error("deepscan(bad-char-at-comparison): Invalid comparison with `charAt` method")]
#[diagnostic(severity(warning), help("`String.prototype.charAt` returns a string of length 1. If the return value is compared with a string of length greater than 1, the comparison will always be false."))]
struct BadCharAtComparisonDiagnostic(
#[label("`charAt` called here")] pub Span,
#[label("And compared with a string of length {2} here")] pub Span,
usize,
);

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

declare_oxc_lint!(
/// ### What it does
///
/// This rule warns when the return value of the `charAt` method is used to compare a string of length greater than 1.
///
/// ### Why is this bad?
///
/// The `charAt` method returns a string of length 1. If the return value is compared with a string of length greater than 1, the comparison will always be false.
///
/// ### Example
/// ```javascript
/// // Bad: The return value of the `charAt` method is compared with a string of length greater than 1.
/// a.charAt(4) === 'a2';
/// a.charAt(4) === '/n';
///
/// // Good: The return value of the `charAt` method is compared with a string of length 1.
/// a.charAt(4) === 'a'
/// a.charAt(4) === '\n';
/// ```
BadCharAtComparison,
correctness
);

impl Rule for BadCharAtComparison {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::CallExpression(call_expr) = node.kind() else { return };

if !is_method_call(call_expr, None, Some(&["charAt"]), Some(1), Some(1)) {
return;
}

let Some(parent) = ctx.nodes().parent_node(node.id()) else {
return;
};

let AstKind::BinaryExpression(binary_expr) = parent.kind() else { return };
if !matches!(
binary_expr.operator,
BinaryOperator::Equality
| BinaryOperator::Inequality
| BinaryOperator::StrictEquality
| BinaryOperator::StrictInequality
) {
return;
};

let comparison_with = if binary_expr.left.span() == call_expr.span {
&binary_expr.right
} else {
&binary_expr.left
};

if let Expression::StringLiteral(string_lit) = comparison_with {
if !is_string_valid(string_lit.value.as_str()) {
ctx.diagnostic(BadCharAtComparisonDiagnostic(
call_expr.span,
string_lit.span,
string_lit.value.len(),
));
}
}
}
}

fn is_string_valid(str: &str) -> bool {
if str.len() < 2 || str.chars().count() == 1 {
return true;
}

false
}

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

let pass = vec![
r"a.charAt(4) === 'a'",
"a.charAt(4) === '\\n'",
"a.charAt(4) === '\t'",
r"a.charAt(4) === 'a'",
r"a.charAt(4) === '\ufeff'",
r"a.charAt(4) !== '\ufeff'",
"chatAt(4) === 'a2'",
"new chatAt(4) === 'a'",
];

let fail = vec![
r"a.charAt(4) === 'aa'",
"a.charAt(4) === '/n'",
"a.charAt(3) === '/t'",
r"a.charAt(4) === 'ac'",
r"a.charAt(822) !== 'foo'",
r"a.charAt(4) === '\\ukeff'",
];

Tester::new_without_config(BadCharAtComparison::NAME, pass, fail).test_and_snapshot();
}
59 changes: 59 additions & 0 deletions crates/oxc_linter/src/snapshots/bad_char_at_comparison.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
---
source: crates/oxc_linter/src/tester.rs
expression: bad_char_at_comparison
---
⚠ deepscan(bad-char-at-comparison): Invalid comparison with `charAt` method
╭─[bad_char_at_comparison.tsx:1:1]
1 │ a.charAt(4) === 'aa'
· ─────┬───── ──┬─
· │ ╰── And compared with a string of length 2 here
· ╰── `charAt` called here
╰────
help: `String.prototype.charAt` returns a string of length 1. If the return value is compared with a string of length greater than 1, the comparison will always be false.

⚠ deepscan(bad-char-at-comparison): Invalid comparison with `charAt` method
╭─[bad_char_at_comparison.tsx:1:1]
1 │ a.charAt(4) === '/n'
· ─────┬───── ──┬─
· │ ╰── And compared with a string of length 2 here
· ╰── `charAt` called here
╰────
help: `String.prototype.charAt` returns a string of length 1. If the return value is compared with a string of length greater than 1, the comparison will always be false.

⚠ deepscan(bad-char-at-comparison): Invalid comparison with `charAt` method
╭─[bad_char_at_comparison.tsx:1:1]
1 │ a.charAt(3) === '/t'
· ─────┬───── ──┬─
· │ ╰── And compared with a string of length 2 here
· ╰── `charAt` called here
╰────
help: `String.prototype.charAt` returns a string of length 1. If the return value is compared with a string of length greater than 1, the comparison will always be false.

⚠ deepscan(bad-char-at-comparison): Invalid comparison with `charAt` method
╭─[bad_char_at_comparison.tsx:1:1]
1 │ a.charAt(4) === 'ac'
· ─────┬───── ──┬─
· │ ╰── And compared with a string of length 2 here
· ╰── `charAt` called here
╰────
help: `String.prototype.charAt` returns a string of length 1. If the return value is compared with a string of length greater than 1, the comparison will always be false.

⚠ deepscan(bad-char-at-comparison): Invalid comparison with `charAt` method
╭─[bad_char_at_comparison.tsx:1:1]
1 │ a.charAt(822) !== 'foo'
· ──────┬────── ──┬──
· │ ╰── And compared with a string of length 3 here
· ╰── `charAt` called here
╰────
help: `String.prototype.charAt` returns a string of length 1. If the return value is compared with a string of length greater than 1, the comparison will always be false.

⚠ deepscan(bad-char-at-comparison): Invalid comparison with `charAt` method
╭─[bad_char_at_comparison.tsx:1:1]
1 │ a.charAt(4) === '\\ukeff'
· ─────┬───── ────┬────
· │ ╰── And compared with a string of length 6 here
· ╰── `charAt` called here
╰────
help: `String.prototype.charAt` returns a string of length 1. If the return value is compared with a string of length greater than 1, the comparison will always be false.