Skip to content

Commit

Permalink
feat(linter): support unicorn/prefer-query-selector (#1068)
Browse files Browse the repository at this point in the history
  • Loading branch information
Dunqing authored Oct 30, 2023
1 parent 6da5a3f commit d4c05ff
Show file tree
Hide file tree
Showing 5 changed files with 385 additions and 1 deletion.
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ mod unicorn {
pub mod no_unnecessary_await;
pub mod prefer_array_flat_map;
pub mod prefer_logical_operator_over_ternary;
pub mod prefer_query_selector;
pub mod require_number_to_fixed_digits_argument;
pub mod switch_case_braces;
pub mod text_encoding_identifier_case;
Expand Down Expand Up @@ -268,6 +269,7 @@ oxc_macros::declare_all_lint_rules! {
unicorn::switch_case_braces,
unicorn::text_encoding_identifier_case,
unicorn::throw_new_error,
unicorn::prefer_query_selector,
react::jsx_key,
react::jsx_no_comment_text_nodes,
react::jsx_no_duplicate_props,
Expand Down
201 changes: 201 additions & 0 deletions crates/oxc_linter/src/rules/unicorn/prefer_query_selector.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
use miette::diagnostic;
use oxc_ast::{
ast::{Argument, Expression},
AstKind,
};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::{self, Error},
};
use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};
use phf::phf_map;

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

#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-unicorn(prefer-query-selector): Prefer `.{0}()` over `.{1}()`.")]
#[diagnostic(severity(Warning), help("It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors)."))]
struct PreferQuerySelectorDiagnostic(&'static str, &'static str, #[label] pub Span);

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

const DISALLOWED_IDENTIFIER_NAMES: phf::Map<&'static str, &'static str> = phf_map!(
"getElementById" => "querySelector",
"getElementsByClassName" => "querySelectorAll",
"getElementsByTagName" => "querySelectorAll"
);

declare_oxc_lint!(
/// ### What it does
///
/// Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`.
///
/// ### Example
/// ```javascript
/// // Bad
/// document.getElementById('foo');
/// document.getElementsByClassName('foo bar');
/// document.getElementsByTagName('main');
/// document.getElementsByClassName(fn());
///
/// // Good
/// document.querySelector('#foo');
/// document.querySelector('.bar');
/// document.querySelector('main #foo .bar');
/// document.querySelectorAll('.foo .bar');
/// document.querySelectorAll('li a');
/// document.querySelector('li').querySelectorAll('a');
/// ```
PreferQuerySelector,
pedantic
);

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

if call_expr.optional || call_expr.arguments.len() != 1 {
return;
}

let Expression::MemberExpression(member_expr) = &call_expr.callee else {
return;
};

if member_expr.optional()
|| member_expr.is_computed()
|| is_node_value_not_dom_node(member_expr.object())
{
return;
}

let Argument::Expression(argument_expr) = call_expr.arguments.get(0).unwrap() else {
return;
};

let Some((property_span, property_name)) = member_expr.static_property_info() else {
return;
};

for (cur_property_name, preferred_selector) in &DISALLOWED_IDENTIFIER_NAMES {
if cur_property_name != &property_name {
continue;
}

let diagnostic =
PreferQuerySelectorDiagnostic(preferred_selector, cur_property_name, property_span);

if argument_expr.is_null() {
return ctx.diagnostic_with_fix(diagnostic, || {
return Fix::new(*preferred_selector, property_span);
});
}

let literal_value = match argument_expr {
Expression::StringLiteral(literal) => Some(literal.value.trim()),
Expression::TemplateLiteral(literal) => {
if literal.expressions.len() == 0 {
literal.quasis.get(0).unwrap().value.cooked.as_deref().map(str::trim)
} else {
None
}
}
_ => None,
};

if let Some(literal_value) = literal_value {
return ctx.diagnostic_with_fix(diagnostic, || {
if literal_value.is_empty() {
return Fix::new(*preferred_selector, property_span);
}

let source_text = argument_expr.span().source_text(ctx.source_text());
let quotes_symbol = source_text.chars().next().unwrap();
let sharp = if cur_property_name.eq(&"getElementById") { "#" } else { "" };
return Fix::new(
format!(
"{preferred_selector}({quotes_symbol}{sharp}{literal_value}{quotes_symbol}"
),
property_span.merge(&argument_expr.span()),
);
});
}

ctx.diagnostic(diagnostic);
}
}
}

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

let pass = vec![
"new document.getElementById(foo);",
"getElementById(foo);",
"document['getElementById'](bar);",
"document[getElementById](bar);",
"document.foo(bar);",
"document.getElementById();",
"document?.getElementById('foo');",
"document.getElementById?.('foo');",
"document.getElementsByClassName(\"foo\", \"bar\");",
"document.getElementById(...[\"id\"]);",
"document.querySelector(\"#foo\");",
"document.querySelector(\".bar\");",
"document.querySelector(\"main #foo .bar\");",
"document.querySelectorAll(\".foo .bar\");",
"document.querySelectorAll(\"li a\");",
"document.querySelector(\"li\").querySelectorAll(\"a\");",
];

let fail = vec![
"document.getElementById(\"foo\");",
"document.getElementsByClassName(\"foo\");",
"document.getElementsByClassName(\"foo bar\");",
"document.getElementsByTagName(\"foo\");",
"document.getElementById(\"\");",
"document.getElementById('foo');",
"document.getElementsByClassName('foo');",
"document.getElementsByClassName('foo bar');",
"document.getElementsByTagName('foo');",
"document.getElementsByClassName('');",
"document.getElementById(`foo`);",
"document.getElementsByClassName(`foo`);",
"document.getElementsByClassName(`foo bar`);",
"document.getElementsByTagName(`foo`);",
"document.getElementsByTagName(``);",
"document.getElementsByClassName(`${fn()}`);",
"document.getElementsByClassName(`foo ${undefined}`);",
"document.getElementsByClassName(null);",
"document.getElementsByTagName(null);",
"document.getElementsByClassName(fn());",
"document.getElementsByClassName(\"foo\" + fn());",
"document.getElementsByClassName(foo + \"bar\");",
"e.getElementById(3)",
];

let fix = vec![
("document.getElementsByTagName('foo');", "document.querySelectorAll('foo');", None),
(
"document.getElementsByClassName(`foo bar`);",
"document.querySelectorAll(`foo bar`);",
None,
),
("document.getElementsByClassName(null);", "document.querySelectorAll(null);", None),
("document.getElementsByTagName(` `);", "document.querySelectorAll(` `);", None),
("document.getElementById(`id`);", "document.querySelector(`#id`);", None),
(
"document.getElementsByClassName(foo + \"bar\");",
"document.getElementsByClassName(foo + \"bar\");",
None,
),
("document.getElementsByClassName(fn());", "document.getElementsByClassName(fn());", None),
];

Tester::new_without_config(PreferQuerySelector::NAME, pass, fail)
.expect_fix(fix)
.test_and_snapshot();
}
166 changes: 166 additions & 0 deletions crates/oxc_linter/src/snapshots/prefer_query_selector.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
---
source: crates/oxc_linter/src/tester.rs
expression: prefer_query_selector
---
eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelector()` over `.getElementById()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementById("foo");
· ──────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName("foo");
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName("foo bar");
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByTagName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByTagName("foo");
· ────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelector()` over `.getElementById()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementById("");
· ──────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelector()` over `.getElementById()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementById('foo');
· ──────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName('foo');
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName('foo bar');
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByTagName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByTagName('foo');
· ────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName('');
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelector()` over `.getElementById()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementById(`foo`);
· ──────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName(`foo`);
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName(`foo bar`);
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByTagName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByTagName(`foo`);
· ────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByTagName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByTagName(``);
· ────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName(`${fn()}`);
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName(`foo ${undefined}`);
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName(null);
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByTagName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByTagName(null);
· ────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName(fn());
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName("foo" + fn());
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelectorAll()` over `.getElementsByClassName()`.
╭─[prefer_query_selector.tsx:1:1]
1document.getElementsByClassName(foo + "bar");
· ──────────────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).

eslint-plugin-unicorn(prefer-query-selector): Prefer `.querySelector()` over `.getElementById()`.
╭─[prefer_query_selector.tsx:1:1]
1e.getElementById(3)
· ──────────────
╰────
help: It's better to use the same method to query DOM elements. This helps keep consistency and it lends itself to future improvements (e.g. more specific selectors).


3 changes: 2 additions & 1 deletion crates/oxc_linter/src/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod jest;
mod react;
mod unicorn;

pub use self::{jest::*, react::*};
pub use self::{jest::*, react::*, unicorn::*};
Loading

0 comments on commit d4c05ff

Please sign in to comment.