From 72b3bdf619eece11b24a7d8efec4a7c0d97f83e1 Mon Sep 17 00:00:00 2001 From: Wenzhe Wang Date: Sun, 12 Nov 2023 18:37:03 +0800 Subject: [PATCH] refactor(linter): replace all `is_type_of_jest_fn_call` (#1228) --- .../src/rules/jest/expect_expect.rs | 4 +- .../src/rules/jest/no_conditional_expect.rs | 71 +++++++++++++------ crates/oxc_linter/src/rules/jest/no_hooks.rs | 19 +++-- .../oxc_linter/src/rules/jest/prefer_todo.rs | 6 +- crates/oxc_linter/src/utils/jest.rs | 18 ----- 5 files changed, 71 insertions(+), 47 deletions(-) diff --git a/crates/oxc_linter/src/rules/jest/expect_expect.rs b/crates/oxc_linter/src/rules/jest/expect_expect.rs index 8de26928d..dc1bff839 100644 --- a/crates/oxc_linter/src/rules/jest/expect_expect.rs +++ b/crates/oxc_linter/src/rules/jest/expect_expect.rs @@ -15,7 +15,7 @@ use crate::{ context::LintContext, rule::Rule, utils::{ - collect_possible_jest_call_node, get_node_name, is_type_of_jest_fn_call_new, JestFnKind, + collect_possible_jest_call_node, get_node_name, is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind, PossibleJestNode, }, }; @@ -98,7 +98,7 @@ fn run<'a>( let node = possible_jest_node.node; if let AstKind::CallExpression(call_expr) = node.kind() { let name = get_node_name(&call_expr.callee); - if is_type_of_jest_fn_call_new( + if is_type_of_jest_fn_call( call_expr, possible_jest_node, ctx, diff --git a/crates/oxc_linter/src/rules/jest/no_conditional_expect.rs b/crates/oxc_linter/src/rules/jest/no_conditional_expect.rs index 1dff6d28f..463a39b33 100644 --- a/crates/oxc_linter/src/rules/jest/no_conditional_expect.rs +++ b/crates/oxc_linter/src/rules/jest/no_conditional_expect.rs @@ -1,16 +1,21 @@ +use std::collections::HashMap; + use oxc_ast::{ast::Expression, AstKind}; use oxc_diagnostics::{ miette::{self, Diagnostic}, thiserror::Error, }; use oxc_macros::declare_oxc_lint; +use oxc_semantic::{AstNode, AstNodeId}; use oxc_span::Span; use crate::{ context::LintContext, rule::Rule, - utils::{is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind}, - AstNode, + utils::{ + collect_possible_jest_call_node, is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind, + PossibleJestNode, + }, }; #[derive(Debug, Error, Diagnostic)] @@ -53,27 +58,51 @@ declare_oxc_lint!( ); impl Rule for NoConditionalExpect { - fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - if let AstKind::CallExpression(call_expr) = node.kind() { - if !is_type_of_jest_fn_call(call_expr, node, ctx, &[JestFnKind::Expect]) { - return; - } - - let has_condition_or_catch = check_parents(node, ctx, false); - if has_condition_or_catch { - ctx.diagnostic(NoConditionalExpectDiagnostic(call_expr.span)); - } + fn run_once(&self, ctx: &LintContext) { + let possible_jest_nodes = collect_possible_jest_call_node(ctx); + let id_nodes_mapping = possible_jest_nodes.iter().fold(HashMap::new(), |mut acc, cur| { + acc.entry(cur.node.id()).or_insert(cur); + acc + }); + for node in &collect_possible_jest_call_node(ctx) { + run(node, &id_nodes_mapping, ctx); } } } -fn check_parents<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>, in_conditional: bool) -> bool { - let Some(parent) = ctx.nodes().parent_node(node.id()) else { +fn run<'a>( + possible_jest_node: &PossibleJestNode<'a, '_>, + id_nodes_mapping: &HashMap>, + ctx: &LintContext<'a>, +) { + let node = possible_jest_node.node; + if let AstKind::CallExpression(call_expr) = node.kind() { + if !is_type_of_jest_fn_call(call_expr, possible_jest_node, ctx, &[JestFnKind::Expect]) { + return; + } + + let has_condition_or_catch = check_parents(node, id_nodes_mapping, ctx, false); + if has_condition_or_catch { + ctx.diagnostic(NoConditionalExpectDiagnostic(call_expr.span)); + } + } +} + +fn check_parents<'a>( + node: &AstNode<'a>, + id_nodes_mapping: &HashMap>, + ctx: &LintContext<'a>, + in_conditional: bool, +) -> bool { + let Some(parent_node) = ctx.nodes().parent_node(node.id()) else { return false; }; - match parent.kind() { + match parent_node.kind() { AstKind::CallExpression(call_expr) => { + let Some(parent) = id_nodes_mapping.get(&parent_node.id()) else { + return check_parents(parent_node, id_nodes_mapping, ctx, in_conditional); + }; if is_type_of_jest_fn_call( call_expr, parent, @@ -85,7 +114,7 @@ fn check_parents<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>, in_conditional: if let Expression::MemberExpression(member_expr) = &call_expr.callee { if member_expr.static_property_name() == Some("catch") { - return check_parents(parent, ctx, true); + return check_parents(parent_node, id_nodes_mapping, ctx, true); } } } @@ -93,7 +122,10 @@ fn check_parents<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>, in_conditional: | AstKind::SwitchStatement(_) | AstKind::IfStatement(_) | AstKind::ConditionalExpression(_) - | AstKind::LogicalExpression(_) => return check_parents(parent, ctx, true), + | AstKind::AwaitExpression(_) + | AstKind::LogicalExpression(_) => { + return check_parents(parent_node, id_nodes_mapping, ctx, true) + } AstKind::Function(function) => { let Some(ident) = &function.id else { return false; @@ -107,15 +139,14 @@ fn check_parents<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>, in_conditional: let Some(parent) = ctx.nodes().parent_node(reference.node_id()) else { return false; }; - - check_parents(parent, ctx, in_conditional) + check_parents(parent, id_nodes_mapping, ctx, in_conditional) }); } AstKind::Program(_) => return false, _ => {} } - check_parents(parent, ctx, in_conditional) + check_parents(parent_node, id_nodes_mapping, ctx, in_conditional) } #[test] diff --git a/crates/oxc_linter/src/rules/jest/no_hooks.rs b/crates/oxc_linter/src/rules/jest/no_hooks.rs index ca47d7220..dcea3c3dc 100644 --- a/crates/oxc_linter/src/rules/jest/no_hooks.rs +++ b/crates/oxc_linter/src/rules/jest/no_hooks.rs @@ -9,8 +9,10 @@ use oxc_span::{GetSpan, Span}; use crate::{ context::LintContext, rule::Rule, - utils::{is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind}, - AstNode, + utils::{ + collect_possible_jest_call_node, is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind, + PossibleJestNode, + }, }; #[derive(Debug, Error, Diagnostic)] @@ -85,13 +87,22 @@ impl Rule for NoHooks { Self { allow } } - fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + fn run_once(&self, ctx: &LintContext) { + for possible_jest_node in collect_possible_jest_call_node(ctx) { + self.run(&possible_jest_node, ctx); + } + } +} + +impl NoHooks { + fn run<'a>(&self, possible_jest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>) { + let node = possible_jest_node.node; let AstKind::CallExpression(call_expr) = node.kind() else { return; }; if !is_type_of_jest_fn_call( call_expr, - node, + possible_jest_node, ctx, &[JestFnKind::General(JestGeneralFnKind::Hook)], ) { diff --git a/crates/oxc_linter/src/rules/jest/prefer_todo.rs b/crates/oxc_linter/src/rules/jest/prefer_todo.rs index d9e4245ac..abb92d837 100644 --- a/crates/oxc_linter/src/rules/jest/prefer_todo.rs +++ b/crates/oxc_linter/src/rules/jest/prefer_todo.rs @@ -14,8 +14,8 @@ use crate::{ fixer::Fix, rule::Rule, utils::{ - collect_possible_jest_call_node, is_type_of_jest_fn_call_new, JestFnKind, - JestGeneralFnKind, PossibleJestNode, + collect_possible_jest_call_node, is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind, + PossibleJestNode, }, }; @@ -70,7 +70,7 @@ fn run<'a>(possible_jest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>) if counts < 1 || should_filter_case(call_expr) || !is_string_type(&call_expr.arguments[0]) - || !is_type_of_jest_fn_call_new( + || !is_type_of_jest_fn_call( call_expr, possible_jest_node, ctx, diff --git a/crates/oxc_linter/src/utils/jest.rs b/crates/oxc_linter/src/utils/jest.rs index e891c015a..133555d43 100644 --- a/crates/oxc_linter/src/utils/jest.rs +++ b/crates/oxc_linter/src/utils/jest.rs @@ -99,24 +99,6 @@ pub fn is_jest_file(ctx: &LintContext) -> bool { } pub fn is_type_of_jest_fn_call<'a>( - call_expr: &'a CallExpression<'a>, - node: &AstNode<'a>, - ctx: &LintContext<'a>, - kinds: &[JestFnKind], -) -> bool { - let jest_fn_call = parse_jest_fn_call(call_expr, node, ctx); - if let Some(jest_fn_call) = jest_fn_call { - let kind = jest_fn_call.kind(); - if kinds.contains(&kind) { - return true; - } - } - - false -} - -// TODO: The new version of `is_type_of_jest_fn_call`, will rename to `is_type_of_jest_fn_call` after all replaced. -pub fn is_type_of_jest_fn_call_new<'a>( call_expr: &'a CallExpression<'a>, possible_jest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>,