From e2a49271ac20ef3dd20a4f97752baa682cbd96c9 Mon Sep 17 00:00:00 2001 From: Wenzhe Wang Date: Sun, 17 Sep 2023 22:40:29 +0800 Subject: [PATCH] feat(linter): add eslint(jest/no-export) (#925) --- crates/oxc_linter/src/jest_ast_util.rs | 32 ++++- crates/oxc_linter/src/rules.rs | 2 + crates/oxc_linter/src/rules/jest/no_export.rs | 116 ++++++++++++++++++ .../oxc_linter/src/snapshots/no_export.snap | 64 ++++++++++ 4 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 crates/oxc_linter/src/rules/jest/no_export.rs create mode 100644 crates/oxc_linter/src/snapshots/no_export.snap diff --git a/crates/oxc_linter/src/jest_ast_util.rs b/crates/oxc_linter/src/jest_ast_util.rs index 9dca353f4..cc408d52c 100644 --- a/crates/oxc_linter/src/jest_ast_util.rs +++ b/crates/oxc_linter/src/jest_ast_util.rs @@ -12,6 +12,37 @@ use oxc_span::{Atom, Span}; use crate::context::LintContext; +const JEST_METHOD_NAMES: [&str; 14] = [ + "afterAll", + "afterEach", + "beforeAll", + "beforeEach", + "describe", + "expect", + "fdescribe", + "fit", + "it", + "jest", + "test", + "xdescribe", + "xit", + "xtest", +]; +pub fn is_jest_file(ctx: &LintContext) -> bool { + if JEST_METHOD_NAMES + .iter() + .any(|name| ctx.scopes().root_unresolved_references().contains_key(*name)) + { + return true; + }; + + let import_entries = &ctx.semantic().module_record().import_entries; + + return import_entries.iter().any(|import_entry| { + matches!(import_entry.module_request.name().as_str(), "@jest/globals") + }); +} + pub fn is_type_of_jest_fn_call<'a>( call_expr: &'a CallExpression<'a>, node: &AstNode<'a>, @@ -354,7 +385,6 @@ pub enum JestGeneralFnKind { pub enum ParsedJestFnCall<'a> { GeneralJestFnCall(ParsedGeneralJestFnCall<'a>), - #[allow(unused)] ExpectFnCall(ParsedExpectFnCall<'a>), } diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 7dbeeabc5..ae885b5fe 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -107,6 +107,7 @@ mod jest { pub mod no_conditional_expect; pub mod no_disabled_tests; pub mod no_done_callback; + pub mod no_export; pub mod no_focused_tests; pub mod no_interpolation_in_snapshots; pub mod no_jasmine_globals; @@ -212,6 +213,7 @@ oxc_macros::declare_all_lint_rules! { jest::no_interpolation_in_snapshots, jest::no_jasmine_globals, jest::no_mocks_import, + jest::no_export, unicorn::no_instanceof_array, unicorn::no_unnecessary_await, unicorn::no_thenable, diff --git a/crates/oxc_linter/src/rules/jest/no_export.rs b/crates/oxc_linter/src/rules/jest/no_export.rs new file mode 100644 index 000000000..e0a3be748 --- /dev/null +++ b/crates/oxc_linter/src/rules/jest/no_export.rs @@ -0,0 +1,116 @@ +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, jest_ast_util::is_jest_file, rule::Rule}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint(jest/no-export): Do not export from a test file.")] +#[diagnostic(severity(warning), help("If you want to share code between tests, move it into a separate file and import it from there."))] +struct NoExportDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct NoExport; + +declare_oxc_lint!( + /// ### What it does + /// + /// Prevents using exports if a file has one or more tests in it. + /// + /// ### Why is this bad? + /// + /// This rule aims to eliminate duplicate runs of tests by exporting things from test files. + /// If you import from a test file, then all the tests in that file will be run in each imported instance. + /// so bottom line, don't export from a test, but instead move helper functions into a separate file when they need to be shared across tests. + /// + /// ### Example + /// ```javascript + /// export function myHelper() {} + /// describe('a test', () => { + /// expect(1).toBe(1); + /// }); + /// ``` + NoExport, + restriction +); + +impl Rule for NoExport { + fn run_once(&self, ctx: &LintContext) { + // only used in jest files + if !is_jest_file(ctx) { + return; + } + + for local_export in &ctx.semantic().module_record().local_export_entries { + ctx.diagnostic(NoExportDiagnostic(local_export.span)); + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("describe('a test', () => { expect(1).toBe(1); })", None), + ("window.location = 'valid'", None), + ("module.somethingElse = 'foo';", None), + ("export const myThing = 'valid'", None), + ("export default function () {}", None), + ("module.exports = function(){}", None), + ("module.exports.myThing = 'valid';", None), + ]; + + let fail = vec![ + ("export const myThing = 'invalid'; test('a test', () => { expect(1).toBe(1);});", None), + ( + " + export const myThing = 'invalid'; + + test.each()('my code', () => { + expect(1).toBe(1); + }); + ", + None, + ), + ( + " + export const myThing = 'invalid'; + + test.each``('my code', () => { + expect(1).toBe(1); + }); + ", + None, + ), + ( + " + export const myThing = 'invalid'; + test.only.each``('my code', () => { + expect(1).toBe(1); + }); + ", + None, + ), + ("export default function() {}; test('a test', () => { expect(1).toBe(1);});", None), + ( + " + const foo = 1; + const bar = 2; + test('a test', () => {}) + + export {foo, bar}; + ", + None, + ), + // TODO: support `module.exports` + // ("module.exports['invalid'] = function() {}; test('a test', () => { expect(1).toBe(1);});", None), + // ("module.exports = function() {}; ; test('a test', () => { expect(1).toBe(1);});", None), + // ("module.export.invalid = function() {}; ; test('a test', () => { expect(1).toBe(1);});", None) + ]; + + Tester::new(NoExport::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_export.snap b/crates/oxc_linter/src/snapshots/no_export.snap new file mode 100644 index 000000000..c40a16e4e --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_export.snap @@ -0,0 +1,64 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: no_export +--- + ⚠ eslint(jest/no-export): Do not export from a test file. + ╭─[no_export.tsx:1:1] + 1 │ export const myThing = 'invalid'; test('a test', () => { expect(1).toBe(1);}); + · ────────────────────────── + ╰──── + help: If you want to share code between tests, move it into a separate file and import it from there. + + ⚠ eslint(jest/no-export): Do not export from a test file. + ╭─[no_export.tsx:1:1] + 1 │ + 2 │ export const myThing = 'invalid'; + · ────────────────────────── + 3 │ + ╰──── + help: If you want to share code between tests, move it into a separate file and import it from there. + + ⚠ eslint(jest/no-export): Do not export from a test file. + ╭─[no_export.tsx:1:1] + 1 │ + 2 │ export const myThing = 'invalid'; + · ────────────────────────── + 3 │ + ╰──── + help: If you want to share code between tests, move it into a separate file and import it from there. + + ⚠ eslint(jest/no-export): Do not export from a test file. + ╭─[no_export.tsx:1:1] + 1 │ + 2 │ export const myThing = 'invalid'; + · ────────────────────────── + 3 │ test.only.each``('my code', () => { + ╰──── + help: If you want to share code between tests, move it into a separate file and import it from there. + + ⚠ eslint(jest/no-export): Do not export from a test file. + ╭─[no_export.tsx:1:1] + 1 │ export default function() {}; test('a test', () => { expect(1).toBe(1);}); + · ───────────── + ╰──── + help: If you want to share code between tests, move it into a separate file and import it from there. + + ⚠ eslint(jest/no-export): Do not export from a test file. + ╭─[no_export.tsx:5:1] + 5 │ + 6 │ export {foo, bar}; + · ─── + 7 │ + ╰──── + help: If you want to share code between tests, move it into a separate file and import it from there. + + ⚠ eslint(jest/no-export): Do not export from a test file. + ╭─[no_export.tsx:5:1] + 5 │ + 6 │ export {foo, bar}; + · ─── + 7 │ + ╰──── + help: If you want to share code between tests, move it into a separate file and import it from there. + +