From d4c05ff7b35ccf505d3f9c35582297cd3813bb84 Mon Sep 17 00:00:00 2001 From: Dunqing Date: Mon, 30 Oct 2023 17:04:35 +0800 Subject: [PATCH] feat(linter): support unicorn/prefer-query-selector (#1068) Refer to https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-query-selector.md link: #684 --- crates/oxc_linter/src/rules.rs | 2 + .../rules/unicorn/prefer_query_selector.rs | 201 ++++++++++++++++++ .../src/snapshots/prefer_query_selector.snap | 166 +++++++++++++++ crates/oxc_linter/src/utils/mod.rs | 3 +- crates/oxc_linter/src/utils/unicorn.rs | 14 ++ 5 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 crates/oxc_linter/src/rules/unicorn/prefer_query_selector.rs create mode 100644 crates/oxc_linter/src/snapshots/prefer_query_selector.snap create mode 100644 crates/oxc_linter/src/utils/unicorn.rs diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 8640d1a9b..eaf480f3d 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -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; @@ -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, diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_query_selector.rs b/crates/oxc_linter/src/rules/unicorn/prefer_query_selector.rs new file mode 100644 index 000000000..39f6c1075 --- /dev/null +++ b/crates/oxc_linter/src/rules/unicorn/prefer_query_selector.rs @@ -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(); +} diff --git a/crates/oxc_linter/src/snapshots/prefer_query_selector.snap b/crates/oxc_linter/src/snapshots/prefer_query_selector.snap new file mode 100644 index 000000000..dc5a8d184 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/prefer_query_selector.snap @@ -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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ document.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] + 1 │ e.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). + + diff --git a/crates/oxc_linter/src/utils/mod.rs b/crates/oxc_linter/src/utils/mod.rs index 4e1e91c04..760ae04d8 100644 --- a/crates/oxc_linter/src/utils/mod.rs +++ b/crates/oxc_linter/src/utils/mod.rs @@ -1,4 +1,5 @@ mod jest; mod react; +mod unicorn; -pub use self::{jest::*, react::*}; +pub use self::{jest::*, react::*, unicorn::*}; diff --git a/crates/oxc_linter/src/utils/unicorn.rs b/crates/oxc_linter/src/utils/unicorn.rs new file mode 100644 index 000000000..4f22005c1 --- /dev/null +++ b/crates/oxc_linter/src/utils/unicorn.rs @@ -0,0 +1,14 @@ +use oxc_ast::ast::Expression; + +pub fn is_node_value_not_dom_node(expr: &Expression) -> bool { + matches!( + expr, + Expression::ArrayExpression(_) + | Expression::ArrowExpression(_) + | Expression::ClassExpression(_) + | Expression::FunctionExpression(_) + | Expression::ObjectExpression(_) + | Expression::TemplateLiteral(_) + | Expression::StringLiteral(_) + ) +}