diff --git a/.changeset/fix-use-expect-false-negatives.md b/.changeset/fix-use-expect-false-negatives.md new file mode 100644 index 000000000000..1589cd042c06 --- /dev/null +++ b/.changeset/fix-use-expect-false-negatives.md @@ -0,0 +1,5 @@ +--- +"@biomejs/biome": patch +--- + +Fixed [#9174](https://github.com/biomejs/biome/issues/9174): [`useExpect`](https://biomejs.dev/linter/rules/use-expect/) now correctly rejects [asymmetric matchers](https://vitest.dev/api/expect.html#expect-stringcontaining) in Vitest or Jest like `expect.stringContaining()`, `expect.objectContaining()`, and utilities like `expect.extend()` that are not valid assertions. Previously these constructs caused false negatives, allowing tests without real assertions to pass the lint rule. diff --git a/crates/biome_js_analyze/src/frameworks/playwright.rs b/crates/biome_js_analyze/src/frameworks/playwright.rs index 709599e97fe5..94fb33fd87fe 100644 --- a/crates/biome_js_analyze/src/frameworks/playwright.rs +++ b/crates/biome_js_analyze/src/frameworks/playwright.rs @@ -219,12 +219,28 @@ fn is_expect_expression(expr: &AnyJsExpression) -> bool { false } AnyJsExpression::JsStaticMemberExpression(member) => { - // expect.soft(), expect.poll(), expect(...).method(), expect(...).not.method() if let Ok(object) = member.object() { - // Recursively check the object - this handles chained member expressions - // like expect(page).not where the object is itself a member expression - // NB: This is overly permissive for certain Vitest constructs (ex: `expect.stringContaining()`) - // that do not assert anything in and of themselves (see issue #9174) + // When the object is a bare `expect` identifier, only allow known + // assertion-qualifying members. This prevents asymmetric matchers + // (https://vitest.dev/api/expect.html#expect-stringcontaining) + // and utilities like `expect.extend()` from being counted as + // assertions. + if let AnyJsExpression::JsIdentifierExpression(id) = &object + && let Ok(name) = id.name() + && let Ok(token) = name.value_token() + && token.text_trimmed() == "expect" + { + if let Ok(member_name) = member.member() + && let Some(js_name) = member_name.as_js_name() + && let Ok(member_token) = js_name.value_token() + { + return matches!( + member_token.text_trimmed(), + "soft" | "poll" | "assertions" | "hasAssertions" + ); + } + return false; + } return is_expect_expression(&object); } false diff --git a/crates/biome_js_analyze/tests/specs/nursery/useExpect/invalid/asymmetric-matchers.js b/crates/biome_js_analyze/tests/specs/nursery/useExpect/invalid/asymmetric-matchers.js new file mode 100644 index 000000000000..54baca9a4164 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useExpect/invalid/asymmetric-matchers.js @@ -0,0 +1,17 @@ +/* should generate diagnostics */ + +test("only asymmetric matcher, not an assertion", () => { + const matcher = expect.stringContaining("foo"); +}); + +test("expect.extend is not an assertion", () => { + expect.extend({ + toBeWithinRange(received, floor, ceiling) { + return { pass: received >= floor && received <= ceiling }; + }, + }); +}); + +it("negated asymmetric matcher is not an assertion", () => { + const matcher = expect.not.objectContaining({ secret: expect.any(String) }); +}); diff --git a/crates/biome_js_analyze/tests/specs/nursery/useExpect/invalid/asymmetric-matchers.js.snap b/crates/biome_js_analyze/tests/specs/nursery/useExpect/invalid/asymmetric-matchers.js.snap new file mode 100644 index 000000000000..f6e73a699bfd --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useExpect/invalid/asymmetric-matchers.js.snap @@ -0,0 +1,99 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: asymmetric-matchers.js +--- +# Input +```js +/* should generate diagnostics */ + +test("only asymmetric matcher, not an assertion", () => { + const matcher = expect.stringContaining("foo"); +}); + +test("expect.extend is not an assertion", () => { + expect.extend({ + toBeWithinRange(received, floor, ceiling) { + return { pass: received >= floor && received <= ceiling }; + }, + }); +}); + +it("negated asymmetric matcher is not an assertion", () => { + const matcher = expect.not.objectContaining({ secret: expect.any(String) }); +}); + +``` + +# Diagnostics +``` +asymmetric-matchers.js:3:1 lint/nursery/useExpect ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i Test callback is missing an expect() assertion. + + 1 │ /* should generate diagnostics */ + 2 │ + > 3 │ test("only asymmetric matcher, not an assertion", () => { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 4 │ const matcher = expect.stringContaining("foo"); + > 5 │ }); + │ ^^ + 6 │ + 7 │ test("expect.extend is not an assertion", () => { + + i Tests without assertions may pass even when the behavior is broken. + + i Add an assertion using expect() to verify the expected behavior. + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` + +``` +asymmetric-matchers.js:7:1 lint/nursery/useExpect ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i Test callback is missing an expect() assertion. + + 5 │ }); + 6 │ + > 7 │ test("expect.extend is not an assertion", () => { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 8 │ expect.extend({ + ... + > 12 │ }); + > 13 │ }); + │ ^^ + 14 │ + 15 │ it("negated asymmetric matcher is not an assertion", () => { + + i Tests without assertions may pass even when the behavior is broken. + + i Add an assertion using expect() to verify the expected behavior. + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` + +``` +asymmetric-matchers.js:15:1 lint/nursery/useExpect ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i Test callback is missing an expect() assertion. + + 13 │ }); + 14 │ + > 15 │ it("negated asymmetric matcher is not an assertion", () => { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 16 │ const matcher = expect.not.objectContaining({ secret: expect.any(String) }); + > 17 │ }); + │ ^^ + 18 │ + + i Tests without assertions may pass even when the behavior is broken. + + i Add an assertion using expect() to verify the expected behavior. + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/useExpect/valid/assertion-counts.js b/crates/biome_js_analyze/tests/specs/nursery/useExpect/valid/assertion-counts.js new file mode 100644 index 000000000000..615aa6806b46 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useExpect/valid/assertion-counts.js @@ -0,0 +1,23 @@ +/* should not generate diagnostics */ + +test("expect.assertions is a valid assertion check", () => { + expect.assertions(1); + expect(true).toBe(true); +}); + +test("expect.hasAssertions is a valid assertion check", () => { + expect.hasAssertions(); + expect(1).toBe(1); +}); + +test("expect.assertions alone counts as assertion-aware", () => { + expect.assertions(0); +}); + +test("expect.soft is a valid assertion", async ({ page }) => { + await expect.soft(page.locator("h1")).toBeVisible(); +}); + +test("expect.poll is a valid assertion", async ({ page }) => { + await expect.poll(() => page.title()).toBe("Title"); +}); diff --git a/crates/biome_js_analyze/tests/specs/nursery/useExpect/valid/assertion-counts.js.snap b/crates/biome_js_analyze/tests/specs/nursery/useExpect/valid/assertion-counts.js.snap new file mode 100644 index 000000000000..54b96c6e4016 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useExpect/valid/assertion-counts.js.snap @@ -0,0 +1,31 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: assertion-counts.js +--- +# Input +```js +/* should not generate diagnostics */ + +test("expect.assertions is a valid assertion check", () => { + expect.assertions(1); + expect(true).toBe(true); +}); + +test("expect.hasAssertions is a valid assertion check", () => { + expect.hasAssertions(); + expect(1).toBe(1); +}); + +test("expect.assertions alone counts as assertion-aware", () => { + expect.assertions(0); +}); + +test("expect.soft is a valid assertion", async ({ page }) => { + await expect.soft(page.locator("h1")).toBeVisible(); +}); + +test("expect.poll is a valid assertion", async ({ page }) => { + await expect.poll(() => page.title()).toBe("Title"); +}); + +```