Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-use-expect-false-negatives.md
Original file line number Diff line number Diff line change
@@ -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.
26 changes: 21 additions & 5 deletions crates/biome_js_analyze/src/frameworks/playwright.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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) });
});
Original file line number Diff line number Diff line change
@@ -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.


```
Original file line number Diff line number Diff line change
@@ -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");
});
Original file line number Diff line number Diff line change
@@ -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");
});

```