Skip to content

Commit 101b3bb

Browse files
Bertie690Bertie690
authored
fix(lint): consider more constructs as valid test assertions (#9178)
1 parent 8aab22f commit 101b3bb

7 files changed

Lines changed: 120 additions & 6 deletions

File tree

‎.changeset/busy-numbers-reply.md‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Fixed [#9172](https://github.com/biomejs/biome/issues/9172) and [#9168](https://github.com/biomejs/biome/issues/9168):
6+
Biome now considers more constructs as valid test assertions.
7+
8+
Previously, [`assert`](https://vitest.dev/api/assert.html), [`expectTypeOf`](https://vitest.dev/api/expect-typeof.html) and [`assertType`](https://vitest.dev/api/assert-type.html)
9+
were not recognized as valid assertions by Biome's linting rules, producing false positives in [`lint/nursery/useExpect`](https://biomejs.dev/linter/rules/use-expect) and other similar rules.
10+
11+
Now, these rules will no longer produce errors in test cases that used these constructs instead of `expect`:
12+
```ts
13+
import { expectTypeOf, assert, assertType } from 'vitest';
14+
15+
const myStr = "Hello from vitest!";
16+
it('should be a string', () => {
17+
expectTypeOf(myStr).toBeString();
18+
});
19+
test("should still be a string", () => {
20+
assertType<string>(myStr);
21+
});
22+
it.todo("should still still be a string", () => {
23+
assert(typeof myStr === "string");
24+
});
25+
```

‎crates/biome_js_analyze/src/frameworks/playwright.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,13 @@ fn is_expect_expression(expr: &AnyJsExpression) -> bool {
206206
if let Ok(name) = id.name()
207207
&& let Ok(token) = name.value_token()
208208
{
209-
return token.text_trimmed() == "expect";
209+
let text = token.text_trimmed();
210+
return text == "expect"
211+
// support chai-style `assert` syntax from Vitest
212+
|| text == "assert"
213+
// Include `expectTypeOf`/`assertType` for type assertions from `expect-type`
214+
|| text == "expectTypeOf"
215+
|| text == "assertType";
210216
}
211217
false
212218
}
@@ -215,6 +221,8 @@ fn is_expect_expression(expr: &AnyJsExpression) -> bool {
215221
if let Ok(object) = member.object() {
216222
// Recursively check the object - this handles chained member expressions
217223
// like expect(page).not where the object is itself a member expression
224+
// NB: This is overly permissive for certain Vitest constructs (ex: `expect.stringContaining()`)
225+
// that do not assert anything in and of themselves (see issue #9174)
218226
return is_expect_expression(&object);
219227
}
220228
false

‎crates/biome_js_analyze/src/lint/nursery/use_expect.rs‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,17 @@ use biome_rule_options::use_expect::UseExpectOptions;
99
use crate::frameworks::playwright::{contains_expect_call, get_test_callback, is_test_call};
1010

1111
declare_lint_rule! {
12-
/// Ensure that test functions contain at least one `expect()` assertion.
12+
/// Ensure that test functions contain at least one `expect()` or similar assertion.
1313
///
1414
/// Tests without assertions may pass even when behavior is broken, leading to
1515
/// false confidence in the test suite. This rule ensures that every test
16-
/// validates some expected behavior using `expect()`.
16+
/// validates some expected behavior using `expect()` or an allowed variant thereof.
17+
///
18+
/// ### Allowed `expect` variants
19+
///
20+
/// - [`assert`](https://www.chaijs.com/api/assert/)
21+
/// - [`expectTypeOf`](https://github.com/mmkal/expect-type)
22+
/// - [`assertType`](https://vitest.dev/api/assert-type)
1723
///
1824
/// ## Examples
1925
///
@@ -36,11 +42,30 @@ declare_lint_rule! {
3642
/// ```
3743
///
3844
/// ```js
39-
/// test("soft assertion", async ({ page }) => {
45+
/// it("soft assertion", async ({ page }) => {
4046
/// await page.goto("/");
4147
/// await expect.soft(page.locator("h1")).toBeVisible();
4248
/// });
4349
/// ```
50+
///
51+
/// Variant assertions are allowed:
52+
/// ```js
53+
/// it("returns bar when passed foo", () => {
54+
/// assert(myFunc("foo") === "bar", "didn't return bar");
55+
/// });
56+
/// ```
57+
///
58+
/// ```ts
59+
/// it("should allow passing 'foo' as an argument", () => {
60+
/// expectTypeOf(myFunc).toBeCallableWith("foo");
61+
/// });
62+
/// ```
63+
/// ```ts
64+
/// it("should have proper type", () => {
65+
/// assertType<(n: string) => string>(myFunc);
66+
/// });
67+
/// ```
68+
/// (This replicates the rule's behavior in eslint-plugin-vitest with `typecheck` set to `true`.)
4469
///
4570
pub UseExpect {
4671
version: "2.4.2",

‎crates/biome_js_analyze/tests/specs/nursery/useExpect/valid/vitest-expect-variants.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/* should not generate diagnostics */
2+
3+
it("should typecheck correctly", () => {
4+
expectTypeOf("foo").not.toEqualTypeOf("bar");
5+
});
6+
it("should be a string", () => {
7+
assertType<string>("foo");
8+
});
9+
10+
describe("math", () => {
11+
test("1+1", () => {
12+
assert(1 + 1 === 2);
13+
});
14+
});
15+
16+
type MyType<T> = (arg: T) => void;
17+
18+
describe("MyType", () => {
19+
it("should be contravariant", () => {
20+
// "foo" extends string => T<string> extends T<"foo">
21+
expectTypeOf("foo").toExtend<string>();
22+
expectTypeOf<MyType<string>>().toExtend<MyType<"foo">>();
23+
});
24+
});

‎crates/biome_js_analyze/tests/specs/nursery/useExpect/valid/vitest-expect-variants.ts.snap‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: vitest-expect-variants.ts
4+
---
5+
# Input
6+
```ts
7+
/* should not generate diagnostics */
8+
9+
it("should typecheck correctly", () => {
10+
expectTypeOf("foo").not.toEqualTypeOf("bar");
11+
});
12+
it("should be a string", () => {
13+
assertType<string>("foo");
14+
});
15+
16+
describe("math", () => {
17+
test("1+1", () => {
18+
assert(1 + 1 === 2);
19+
});
20+
});
21+
22+
type MyType<T> = (arg: T) => void;
23+
24+
describe("MyType", () => {
25+
it("should be contravariant", () => {
26+
// "foo" extends string => T<string> extends T<"foo">
27+
expectTypeOf("foo").toExtend<string>();
28+
expectTypeOf<MyType<string>>().toExtend<MyType<"foo">>();
29+
});
30+
});
31+
32+
```

‎packages/@biomejs/backend-jsonrpc/src/workspace.ts‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/@biomejs/biome/configuration_schema.json‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)