From fd2f8fb12b23cca20689f81f1906d8b86416eb7c Mon Sep 17 00:00:00 2001 From: Wenzhe Wang Date: Thu, 24 Aug 2023 11:34:15 +0800 Subject: [PATCH] feat(linter): detect import (#778) Support test cases like ```js import { test } from '@jest/globals'; test('something'); ``` --- crates/oxc_linter/src/jest_ast_util.rs | 74 +++++++++++++++---- .../src/rules/jest/no_disabled_tests.rs | 9 +-- .../src/rules/jest/no_focused_tests.rs | 4 +- .../src/rules/jest/no_test_prefixes.rs | 40 ++++++---- .../src/rules/jest/valid_describe_callback.rs | 28 +++---- .../src/snapshots/no_disabled_tests.snap | 7 ++ .../src/snapshots/no_test_prefixes.snap | 24 ++++++ .../snapshots/valid_describe_callback.snap | 9 +++ 8 files changed, 149 insertions(+), 46 deletions(-) diff --git a/crates/oxc_linter/src/jest_ast_util.rs b/crates/oxc_linter/src/jest_ast_util.rs index e181e977a..e48ce948f 100644 --- a/crates/oxc_linter/src/jest_ast_util.rs +++ b/crates/oxc_linter/src/jest_ast_util.rs @@ -1,10 +1,13 @@ use std::borrow::Cow; use oxc_ast::{ - ast::{CallExpression, Expression, IdentifierName, IdentifierReference, MemberExpression}, + ast::{ + CallExpression, Expression, IdentifierName, IdentifierReference, + ImportDeclarationSpecifier, MemberExpression, ModuleDeclaration, + }, AstKind, }; -use oxc_semantic::AstNode; +use oxc_semantic::{AstNode, AstNodeId}; use oxc_span::{Atom, Span}; use crate::context::LintContext; @@ -12,7 +15,7 @@ use crate::context::LintContext; pub fn parse_general_jest_fn_call<'a>( call_expr: &'a CallExpression<'a>, node: &AstNode<'a>, - ctx: &LintContext, + ctx: &LintContext<'a>, ) -> Option> { let jest_fn_call = parse_jest_fn_call(call_expr, node, ctx)?; @@ -25,7 +28,7 @@ pub fn parse_general_jest_fn_call<'a>( pub fn parse_jest_fn_call<'a>( call_expr: &'a CallExpression<'a>, node: &AstNode<'a>, - ctx: &LintContext, + ctx: &LintContext<'a>, ) -> Option> { let callee = &call_expr.callee; @@ -55,7 +58,7 @@ pub fn parse_jest_fn_call<'a>( return None; } - if let (Some(first), Some(last)) = (chain.first(), chain.last()) { + if let Some(last) = chain.last() { // If we're an `each()`, ensure we're the outer CallExpression (i.e `.each()()`) if last.is_name_equal("each") && !matches!( @@ -70,8 +73,9 @@ pub fn parse_jest_fn_call<'a>( { return None; } - let Some(first_name )= first.name() else { return None }; - let kind = JestFnKind::from(&first_name); + + let name = resolved.original.unwrap_or(resolved.local).as_str(); + let kind = JestFnKind::from(name); let mut members = Vec::new(); let iter = chain.into_iter().skip(1); let rest = iter; @@ -82,7 +86,6 @@ pub fn parse_jest_fn_call<'a>( members.push(member); } - let name = resolved.local.as_str(); let is_valid_jest_call = if members.is_empty() { VALID_JEST_FN_CALL_CHAINS.iter().any(|chain| chain[0] == name) } else if members.len() == 1 { @@ -109,10 +112,11 @@ pub fn parse_jest_fn_call<'a>( if !is_valid_jest_call { return None; } + return Some(ParsedJestFnCall::GeneralJestFnCall(ParsedGeneralJestFnCall { kind, members, - raw: first_name, + name: Cow::Borrowed(name), })); } @@ -120,12 +124,47 @@ pub fn parse_jest_fn_call<'a>( } fn resolve_to_jest_fn<'a>( - call_expr: &'a CallExpression, - ctx: &'a LintContext, + call_expr: &'a CallExpression<'a>, + ctx: &LintContext<'a>, ) -> Option> { let ident = resolve_first_ident(&call_expr.callee)?; if ctx.semantic().is_reference_to_global_variable(ident) { - return Some(ResolvedJestFn { local: &ident.name }); + return Some(ResolvedJestFn { + local: &ident.name, + kind: JestFnFrom::Global, + original: None, + }); + } + + let node_id = get_import_decl_node_id(ident, ctx)?; + let node = ctx.nodes().get_node(node_id); + let AstKind::ModuleDeclaration(module_decl) = node.kind() else { return None; }; + let ModuleDeclaration::ImportDeclaration(import_decl) = module_decl else { return None; }; + + if import_decl.source.value == "@jest/globals" { + let original = import_decl.specifiers.iter().find_map(|specifier| match specifier { + ImportDeclarationSpecifier::ImportSpecifier(import_specifier) => { + Some(import_specifier.imported.name()) + } + _ => None, + }); + + return Some(ResolvedJestFn { local: &ident.name, kind: JestFnFrom::Import, original }); + } + None +} + +fn get_import_decl_node_id(ident: &IdentifierReference, ctx: &LintContext) -> Option { + let symbol_table = ctx.semantic().symbols(); + let reference_id = ident.reference_id.get()?; + let reference = symbol_table.get_reference(reference_id); + // import binding is not a write reference + if reference.is_write() { + return None; + } + let symbol_id = reference.symbol_id()?; + if symbol_table.get_flag(symbol_id).is_import_binding() { + return Some(symbol_table.get_declaration(symbol_id)); } None @@ -187,19 +226,28 @@ pub enum ParsedJestFnCall<'a> { pub struct ParsedGeneralJestFnCall<'a> { pub kind: JestFnKind, pub members: Vec>, - pub raw: Cow<'a, str>, + pub name: Cow<'a, str>, } pub struct ParsedExpectFnCall<'a> { pub kind: JestFnKind, pub members: Vec>, pub raw: Cow<'a, str>, + pub name: Cow<'a, str>, // pub args: Vec<&'a Expression<'a>> // TODO: add `modifiers`, `matcher` for this struct. } struct ResolvedJestFn<'a> { pub local: &'a Atom, + pub original: Option<&'a Atom>, + #[allow(unused)] + kind: JestFnFrom, +} + +pub enum JestFnFrom { + Global, + Import, } pub struct KnownMemberExpressionProperty<'a> { diff --git a/crates/oxc_linter/src/rules/jest/no_disabled_tests.rs b/crates/oxc_linter/src/rules/jest/no_disabled_tests.rs index 756ef0328..9d54fa20f 100644 --- a/crates/oxc_linter/src/rules/jest/no_disabled_tests.rs +++ b/crates/oxc_linter/src/rules/jest/no_disabled_tests.rs @@ -52,7 +52,7 @@ declare_oxc_lint!( /// }); /// ``` NoDisabledTests, - nursery + correctness ); #[derive(Debug, Error, Diagnostic)] @@ -86,7 +86,7 @@ impl Rule for NoDisabledTests { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { if let AstKind::CallExpression(call_expr) = node.kind() { if let Some(jest_fn_call) = parse_general_jest_fn_call(call_expr, node, ctx) { - let ParsedGeneralJestFnCall { kind, members, raw } = jest_fn_call; + let ParsedGeneralJestFnCall { kind, members, name } = jest_fn_call; // `test('foo')` let kind = match kind { JestFnKind::Expect | JestFnKind::Unknown => return, @@ -103,7 +103,7 @@ impl Rule for NoDisabledTests { // the only jest functions that are with "x" are "xdescribe", "xtest", and "xit" // `xdescribe('foo', () => {})` - if raw.starts_with('x') { + if name.starts_with('x') { let (error, help) = if matches!(kind, JestGeneralFnKind::Describe) { Message::DisabledSuiteWithX.details() } else { @@ -205,8 +205,7 @@ fn test() { ("it('contains a call to pending', function () { pending() })", None), ("pending()", None), ("describe('contains a call to pending', function () { pending() })", None), - // TODO: Continue work on it when [#510](https://github.com/Boshen/oxc/issues/510) solved - // ("import { test } from '@jest/globals';test('something');", None), + ("import { test } from '@jest/globals';test('something');", None), ]; Tester::new(NoDisabledTests::NAME, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/rules/jest/no_focused_tests.rs b/crates/oxc_linter/src/rules/jest/no_focused_tests.rs index b153b479e..9674e68a6 100644 --- a/crates/oxc_linter/src/rules/jest/no_focused_tests.rs +++ b/crates/oxc_linter/src/rules/jest/no_focused_tests.rs @@ -61,7 +61,7 @@ impl Rule for NoFocusedTests { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let AstKind::CallExpression(call_expr) = node.kind() else { return }; let Some(jest_fn_call) = parse_general_jest_fn_call(call_expr, node, ctx) else { return }; - let ParsedGeneralJestFnCall { kind, members, raw } = jest_fn_call; + let ParsedGeneralJestFnCall { kind, members, name } = jest_fn_call; if !matches!( kind, JestFnKind::General(JestGeneralFnKind::Describe | JestGeneralFnKind::Test) @@ -69,7 +69,7 @@ impl Rule for NoFocusedTests { return; } - if raw.starts_with('f') { + if name.starts_with('f') { ctx.diagnostic_with_fix(NoFocusedTestsDiagnostic(call_expr.span), || { let start = call_expr.span.start; Fix::delete(Span { start, end: start + 1 }) diff --git a/crates/oxc_linter/src/rules/jest/no_test_prefixes.rs b/crates/oxc_linter/src/rules/jest/no_test_prefixes.rs index f57ca0e25..9f153b729 100644 --- a/crates/oxc_linter/src/rules/jest/no_test_prefixes.rs +++ b/crates/oxc_linter/src/rules/jest/no_test_prefixes.rs @@ -48,19 +48,19 @@ declare_oxc_lint!( /// xdescribe('foo'); // invalid /// ``` NoTestPrefixes, - nursery + correctness ); fn get_preferred_node_names(jest_fn_call: &ParsedGeneralJestFnCall) -> Atom { - let ParsedGeneralJestFnCall { members, raw, .. } = jest_fn_call; + let ParsedGeneralJestFnCall { members, name, .. } = jest_fn_call; - let preferred_modifier = if raw.starts_with('f') { "only" } else { "skip" }; + let preferred_modifier = if name.starts_with('f') { "only" } else { "skip" }; let member_names = members .iter() .filter_map(KnownMemberExpressionProperty::name) .collect::>() .join("."); - let name_slice = &raw[1..]; + let name_slice = &name[1..]; if member_names.is_empty() { Atom::from(format!("{name_slice}.{preferred_modifier}")) @@ -73,14 +73,14 @@ impl Rule for NoTestPrefixes { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let AstKind::CallExpression(call_expr) = node.kind() else { return }; let Some(jest_fn_call) = parse_general_jest_fn_call(call_expr, node, ctx) else { return }; - let ParsedGeneralJestFnCall { kind, raw, .. } = &jest_fn_call; + let ParsedGeneralJestFnCall { kind, name, .. } = &jest_fn_call; let Some(kind) = kind.to_general() else {return}; if !matches!(kind, JestGeneralFnKind::Describe | JestGeneralFnKind::Test) { return; } - if !raw.starts_with('f') && !raw.starts_with('x') { + if !name.starts_with('f') && !name.starts_with('x') { return; } @@ -136,13 +136,27 @@ fn test() { ("xtest.each``('foo', function () {})", None), ("xit.each([])('foo', function () {})", None), ("xtest.each([])('foo', function () {})", None), - // TODO: Continue work on it when [#510](https://github.com/Boshen/oxc/issues/510) solved - // (r#"import { xit } from '@jest/globals'; - // xit("foo", function () {})"#, None), - // (r#"import { xit as skipThis } from '@jest/globals'; - // skipThis("foo", function () {})"#, None), - // (r#"import { fit as onlyThis } from '@jest/globals'; - // onlyThis("foo", function () {})"#, None) + ( + " + import { xit } from '@jest/globals'; + xit('foo', function () {}) + ", + None, + ), + ( + " + import { xit as skipThis } from '@jest/globals'; + skipThis('foo', function () {}) + ", + None, + ), + ( + " + import { fit as onlyThis } from '@jest/globals'; + onlyThis('foo', function () {}) + ", + None, + ), ]; Tester::new(NoTestPrefixes::NAME, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/rules/jest/valid_describe_callback.rs b/crates/oxc_linter/src/rules/jest/valid_describe_callback.rs index 490267a40..9e902a357 100644 --- a/crates/oxc_linter/src/rules/jest/valid_describe_callback.rs +++ b/crates/oxc_linter/src/rules/jest/valid_describe_callback.rs @@ -60,7 +60,7 @@ declare_oxc_lint!( /// ``` ValidDescribeCallback, // Because this rule has one test case not passed, will set to correctness when finished. - nursery + correctness ); impl Rule for ValidDescribeCallback { @@ -203,7 +203,7 @@ fn test() { }) }) }) - ", + ", None, ), ( @@ -213,17 +213,17 @@ fn test() { expect(await Promise.resolve(42)).toBe(42) }) }) - ", + ", None, ), ("if (hasOwnProperty(obj, key)) {}", None), ( " - describe.each` - foo | foe - ${'1'} | ${'2'} - `('$something', ({ foo, foe }) => {}); - ", + describe.each` + foo | foe + ${'1'} | ${'2'} + `('$something', ({ foo, foe }) => {}); + ", None, ), ]; @@ -245,11 +245,13 @@ fn test() { ("describe('foo', async function () {})", None), ("xdescribe('foo', async function () {})", None), ("fdescribe('foo', async function () {})", None), - // TODO - // (" - // import { fdescribe } from '@jest/globals'; - // fdescribe('foo', async function () {}) - // ", None), + ( + " + import { fdescribe } from '@jest/globals'; + fdescribe('foo', async function () {}) + ", + None, + ), ("describe.only('foo', async function () {})", None), ("describe.skip('foo', async function () {})", None), ( diff --git a/crates/oxc_linter/src/snapshots/no_disabled_tests.snap b/crates/oxc_linter/src/snapshots/no_disabled_tests.snap index 0e5617a7b..f26b49b26 100644 --- a/crates/oxc_linter/src/snapshots/no_disabled_tests.snap +++ b/crates/oxc_linter/src/snapshots/no_disabled_tests.snap @@ -177,4 +177,11 @@ expression: no_disabled_tests ╰──── help: "Remove pending() call" + ⚠ eslint(jest/no-disabled-tests): "Test is missing function argument" + ╭─[no_disabled_tests.tsx:1:1] + 1 │ import { test } from '@jest/globals';test('something'); + · ───────────────── + ╰──── + help: "Add function argument" + diff --git a/crates/oxc_linter/src/snapshots/no_test_prefixes.snap b/crates/oxc_linter/src/snapshots/no_test_prefixes.snap index b9947f847..a047e9590 100644 --- a/crates/oxc_linter/src/snapshots/no_test_prefixes.snap +++ b/crates/oxc_linter/src/snapshots/no_test_prefixes.snap @@ -62,4 +62,28 @@ expression: no_test_prefixes · ────────── ╰──── + ⚠ eslint(jest/no-test-prefixes): Use "it.skip" instead. + ╭─[no_test_prefixes.tsx:2:1] + 2 │ import { xit } from '@jest/globals'; + 3 │ xit('foo', function () {}) + · ─── + 4 │ + ╰──── + + ⚠ eslint(jest/no-test-prefixes): Use "it.skip" instead. + ╭─[no_test_prefixes.tsx:2:1] + 2 │ import { xit as skipThis } from '@jest/globals'; + 3 │ skipThis('foo', function () {}) + · ──────── + 4 │ + ╰──── + + ⚠ eslint(jest/no-test-prefixes): Use "it.only" instead. + ╭─[no_test_prefixes.tsx:2:1] + 2 │ import { fit as onlyThis } from '@jest/globals'; + 3 │ onlyThis('foo', function () {}) + · ──────── + 4 │ + ╰──── + diff --git a/crates/oxc_linter/src/snapshots/valid_describe_callback.snap b/crates/oxc_linter/src/snapshots/valid_describe_callback.snap index 541755c49..54311b3ce 100644 --- a/crates/oxc_linter/src/snapshots/valid_describe_callback.snap +++ b/crates/oxc_linter/src/snapshots/valid_describe_callback.snap @@ -114,6 +114,15 @@ expression: valid_describe_callback ╰──── help: "Remove `async` keyword" + ⚠ "No async describe callback" + ╭─[valid_describe_callback.tsx:2:1] + 2 │ import { fdescribe } from '@jest/globals'; + 3 │ fdescribe('foo', async function () {}) + · ──────────────────── + 4 │ + ╰──── + help: "Remove `async` keyword" + ⚠ "No async describe callback" ╭─[valid_describe_callback.tsx:1:1] 1 │ describe.only('foo', async function () {})