linter: panic in jest/expect-expect

[Boshen]

Found in https://github.com/oxc-project/oxlint-ecosystem-ci/actions/runs/9123766396/job/25086817261?pr=6

Reduced case:

test("event emitters bound to CLS context", function(t) {
    t.test("emitter with newListener that removes handler", function(t) {
        ee.on("newListener", function handler(event: any) {
            this.removeListener("newListener", handler);
        });
    });
});

Infinite recursion in

oxc/crates/oxc_linter/src/rules/jest/expect_expect.rs

Lines 179 to 188 in 27030b9

	let Some(node) = get_declaration_of_variable(ident, ctx) else {
	return false;
	};
	let AstKind::Function(function) = node.kind() else {
	return false;
	};
	let Some(body) = &function.body else {
	return false;
	};
	return check_statements(&body.statements, assert_function_names, ctx);

because it went back inside handler

[Boshen]

Why does it need to check for the arguments when matches_assert_function_name is false?

oxc/crates/oxc_linter/src/rules/jest/expect_expect.rs

Lines 170 to 174 in 27030b9

	if matches_assert_function_name(&name, assert_function_names) {
	return true;
	}
	let has_assert_function = check_arguments(call_expr, assert_function_names, ctx);

Edit: it failed on this case

it('should pass', () => somePromise().then(() => expect(true).toBeDefined()))

[mysteryven]

I found the current implementation was not correct. It finds assert function from test funtion to bottom, There will be so many cases I would forget(e.g. if (condition){ expect()}), I plan to follow the jest implementation, it needs to loop all nodes... any better ideas?

fn run_once(&self, ctx: &LintContext) {
        let test_functions = HashSet::new();

        for node in ctx.nodes() {
            if is_test_function(node) {
                test_functions.insert(node.span());
            } else if is_expect_call(node) {
                // try get nearest test function, if got, remove it from test_functions
            }
        }
    }

[Boshen]

Can we do a temporary fix first, and then figure out how to best write this?

[mysteryven]

Panic is resolved, and the below belongs to another issue.

I found the current implementation was not correct. It finds assert function from test funtion to bottom, There will be so many cases I would forget(e.g. if (condition){ expect()}), I plan to follow the jest implementation, it needs to loop all nodes... any better ideas?

fn run_once(&self, ctx: &LintContext) {
        let test_functions = HashSet::new();

        for node in ctx.nodes() {
            if is_test_function(node) {
                test_functions.insert(node.span());
            } else if is_expect_call(node) {
                // try get nearest test function, if got, remove it from test_functions
            }
        }
    }