fix(noPlaywrightMissingAwait): fix false positives with jest-dom matchers#9153
fix(noPlaywrightMissingAwait): fix false positives with jest-dom matchers#9153abossenbroek wants to merge 46 commits intobiomejs:mainfrom
Conversation
Implements 12 Playwright lint rules from eslint-plugin-playwright: - noPlaywrightElementHandle: Disallows ElementHandle usage - noPlaywrightEval: Disallows $eval and $$eval usage - noPlaywrightForceOption: Disallows force option on actions - noPlaywrightMissingAwait: Requires await on async Playwright methods - noPlaywrightNetworkidle: Disallows networkidle wait option - noPlaywrightPagePause: Disallows page.pause() in tests - noPlaywrightSkippedTest: Disallows .skip and .fixme annotations - noPlaywrightUselessAwait: Disallows unnecessary awaits - noPlaywrightWaitForNavigation: Disallows deprecated waitForNavigation - noPlaywrightWaitForSelector: Disallows waitForSelector usage - noPlaywrightWaitForTimeout: Disallows waitForTimeout usage - usePlaywrightValidDescribeCallback: Validates describe callback functions All rules are added to the nursery group with corresponding ESLint migration mappings for eslint-plugin-playwright. Closes biomejs#7796 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change "."{{method}}"()" to "."{method}"()" to correctly interpolate
the method variable instead of outputting literal {method}.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Playwright's clear() method also supports the force option. Add "clear" alphabetically between "check" and "click". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Expressions like `await (expect(...).toBeVisible())` were incorrectly flagged because the rule didn't traverse JS_PARENTHESIZED_EXPRESSION nodes when checking for await. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change valid example from waitForSelector to locator-based approach to avoid conflict with noPlaywrightWaitForSelector rule. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change "Prefers locators over element handles" to "Prefers locators to element handles" for correct grammar. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add #![allow(clippy::large_stack_arrays)] to spec_tests.rs. The gen_tests! macro generates 1300+ test functions, and clippy flags internal compiler representations from macro expansion as large arrays. This is a false positive since our actual arrays are properly sized (< 500 bytes each). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove pub from ElementHandleCall fields for consistency with EvalMethodCall - Reuse shared get_page_or_frame_name helper instead of local duplicate - Add rustdoc comments to InvalidReason enum - Add clarifying comment to changeset code example Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename expectPlaywrightExpect → usePlaywrightExpect (rules must start with use/no) - Extract shared functions to frameworks/playwright.rs: - collect_member_names() with TokenText to avoid allocations - is_test_call() for detecting test/it calls - get_test_callback() returning LAST function argument - Fix noPlaywrightForceOption to handle parenthesized objects - Fix noPlaywrightMissingAwait to guard expect.poll with has_expect_in_chain - Fix noPlaywrightWaitForSelector to use replace_node_transfer_trivia - Fix noPlaywrightUselessAwait to handle .not modifier - Fix noPlaywrightConditionalExpect to avoid duplicate diagnostics - Refactor rules to use shared code from playwright.rs - Add #[derive(Debug)] to state enums Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Extract `is_expect_call` from use_playwright_expect.rs to frameworks/playwright.rs - Update function to handle all expect chain patterns: - expect() direct calls - expect.soft(), expect.poll() member expressions - expect(x).toBe() chained method calls - Refactor no_playwright_conditional_expect.rs to use shared function - Rename local helper to is_top_level_expect_call for clarity Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive test coverage for all Playwright expect modifiers per PR biomejs#8960 review feedback about handling .not/.resolves chains. Test cases added: - .resolves, .rejects modifiers - expect.poll(), expect.soft() variants - Chained modifiers: .resolves.not, .rejects.not, .soft().not Both invalid (conditional) and valid (unconditional) scenarios covered with unit tests verifying is_expect_call handles all modifier patterns. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Refactor is_expect_call to recursively traverse member expression chains, properly handling patterns like expect(page).not.toHaveTitle and expect(data).resolves.not.toBeNull. Also simplify TokenText comparisons across Playwright rules by using direct equality checks instead of .text() method calls. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…st_call `is_test_call()` incorrectly matched `test.describe()`, `test.beforeEach()`, and similar non-test member expressions as test calls by only checking the object part and ignoring the member name. This could cause false positives in `usePlaywrightExpect` when describe blocks or hooks lacked `expect()` calls. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Address PR review feedback by moving contains_expect_call from use_playwright_expect.rs to the shared frameworks/playwright.rs module. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add `await expect.poll(() => getValue()).not.toBe(null)` to the valid test cases for noPlaywrightUselessAwait, ensuring expect.poll with a chained .not modifier has explicit coverage. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add unit tests to lock in the intended behavior for `is_test_call`, ensuring the fix from commit a425297 (excluding describe blocks, hooks, and steps) is preserved. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Delete 4 invalid changesets (fix/note changesets for unreleased rules) - Reorder diagnostic notes: WHAT → WHY → HOW across all 14 rules - Move helper structs/consts/fns below `impl Rule` blocks in all rules - Change `&TokenText` → `&str` in helper function parameters - Refactor `is_sync_expect_call` to use idiomatic let-else guards - Add missing WHY/HOW diagnostic notes to noPlaywrightMissingAwait - Remove unused `method` variable in noPlaywrightElementHandle - Format with rustfmt Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix error handling in playwright.rs: use `member.object().ok()?` and
`let Ok(arg) = arg else { continue }` patterns (C22, C23)
- Add function boundary guard in no_playwright_missing_await (C19)
- Support `describe.fixme(...)` in no_playwright_skipped_test (C20)
- Handle `test.describe.fixme` in use_playwright_valid_describe_callback (C21)
- Fix domains.json.snap to include `playwright` domain (CI fix)
- Add test cases for all new patterns
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ation Move duplicated helpers to shared modules: - Extract is_describe_mode/is_describe_modifier to playwright.rs - Extract has_bool_property/has_string_property to playwright.rs - Add find_member_in_chain utility to playwright.rs - Add is_in_async_context to ast_utils.rs - Replace local unwrap_parenthesized with omit_parentheses() - Remove dead code comment in is_properly_handled Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…mejs#8960 Add catch clause detection to noPlaywrightConditionalExpect rule so that expect() inside catch blocks is flagged as conditional. Add test coverage for bare test.skip() with no arguments and test.describe.fixme() pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…pedTests Merge the nursery `noPlaywrightSkippedTest` rule into the existing `suspicious/noSkippedTests` rule, adding Playwright-specific patterns (.fixme, test.describe, test.step, bracket notation, bare calls) and an `allowConditional` option. Remove the now-redundant nursery rule, its tests, options, and codegen references. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove allowConditional option from noSkippedTests (can't add options in patch release) - Rename noPlaywrightConditionalExpect → noConditionalExpect with Test domain and Jest/Vitest sources - Rename usePlaywrightExpect → useExpect with Test domain and Jest/Vitest sources - Update changesets and diagnostic categories Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…omejs#8960 Add toMatchAriaSnapshot to ASYNC_PLAYWRIGHT_MATCHERS and remove stale #[allow(dead_code)] on SkipState::annotation field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…mejs#8960 - Add `domains: &[RuleDomain::Test]` to NoSkippedTests metadata - Remove unreachable JS_SWITCH_STATEMENT arm in no_conditional_expect - Extract `annotation_for` helper to deduplicate skip/fixme logic - Add || and ?? test cases for noConditionalExpect Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix clippy collapsible_if warning in expr_ext.rs and update CLI snapshot for should_enable_domain_via_cli to account for useExpect rule firing on test domain fixtures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rrow/as_ref, and map_or Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merge origin/main, keeping Playwright additions alongside main's new entries (Types domain, EslintE18e, EslintBetterTailwindcss) and dropping rules that main removed from nursery (useMaxParams, useQwikMethodUsage, useQwikValidLexicalScope). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…k/biome into feat/playwright-eslint-rules
…hers For matchers shared between Playwright (async) and jest-dom (sync), the rule now checks whether expect()'s argument is a Playwright locator or page object before flagging. Playwright-only matchers are still flagged unconditionally. Fixes biomejs#9115 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: d4b40dd The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Closing — branch was out of date with too many commits. Will reopen with a clean branch. |
|
Caution Review failedThe pull request is closed. WalkthroughThis PR introduces comprehensive Playwright testing support to the Biome linter. It adds 13 new lint rules covering Playwright-specific patterns (element handles, eval methods, force options, missing awaits, deprecated APIs), a framework module with utilities for detecting Playwright patterns, infrastructure for a new Playwright rule domain and ESLint source, and extensive test coverage. It also enhances the existing noSkippedTests rule to recognise Playwright-specific skip/fixme patterns. The changes include rule options types, AST utilities for async boundary detection, and test expression pattern matching updates. Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
noPlaywrightMissingAwaitruns on non-Playwright test files using jest-dom matchers (🐛noPlaywrightMissingAwaitfalse positives with jest-dom matchers #9115).ASYNC_PLAYWRIGHT_MATCHERSintoPLAYWRIGHT_ONLY_MATCHERS(19, flagged unconditionally) andOVERLAPPING_MATCHERS(12 shared with jest-dom, only flagged whenexpect()'s argument is a Playwright locator/page).find_expect_first_arg()helper that extracts the first argument from anexpect()call chain and guards overlapping matchers withis_playwright_call_chain().Fixes #9115
Test plan
valid/jest-dom-matchers.jstest with all 12 overlapping matchers — no diagnostics emittedcargo test -p biome_js_analyze -- no_playwright_missing_await)cargo clippy -p biome_js_analyze -- -D warnings— no warningsAI disclosure
This PR was written primarily by Claude Code (Claude Opus 4.6).
🤖 Generated with Claude Code