Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 Vitest [asymmetric matchers](https://vitest.dev/api/expect.html#expect-stringcontaining) 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.
Comment thread
omar-y-abdi marked this conversation as resolved.
Outdated
27 changes: 22 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,29 @@ 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 {
if 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");
});

```
Loading