test: improve test coverage #618
Conversation
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughBump Vitest dev dependencies, add a dedicated Vitest config with v8 coverage thresholds, adjust many v8/@preserve/exhaustive-guard comments, replace some null-checks with non-null assertions, and add/update unit tests and test mocks. No public API signature changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as makeNewlinesBetweenFixes
participant Compute as computeRangeReplacement
participant Fixes as fixesArray
Note over Caller,Compute: Previous flow — conditional push
Caller->>Compute: computeRangeReplacement(...)
alt rangeReplacement truthy
Compute-->>Caller: RangeReplacement
Caller->>Fixes: push(rangeReplacement)
else
Compute-->>Caller: undefined
Caller-->>Fixes: (no push)
end
Note over Caller,Compute: New flow — unconditional push with non-null assertion
Caller->>Compute: computeRangeReplacement(...)
Compute-->>Caller: RangeReplacement (assumed non-null)
Caller->>Fixes: push(rangeReplacement!)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #618 +/- ##
===========================================
Coverage 100.00% 100.00%
===========================================
Files 113 114 +1
Lines 9101 3313 -5788
Branches 1743 1003 -740
===========================================
- Hits 9101 3313 -5788 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
utils/make-newlines-between-fixes.ts (1)
165-172: Non‑null assertion hides a mismatch betweencomputeRangeReplacement’s type/docs and implementation
computeRangeReplacementis declared (and documented) as returningundefined | string, but its current implementation always returns astring. Asserting non‑null at the call site works today, but it relies on a hidden invariant and makes future changes tocomputeRangeReplacementeasier to break silently.Consider tightening the function contract instead and dropping the
!here:- let rangeReplacement = computeRangeReplacement({ + let rangeReplacement = computeRangeReplacement({ isOnSameLine: sortingNode.node.loc.end.line === nextSortingNode.node.loc.start.line, textBetweenNodes, newlinesBetween, - })! + }) fixes.push(fixer.replaceTextRange(rangeToReplace, rangeReplacement))Then update
computeRangeReplacement’s return type (and JSDoc) tostringonly, reflecting the actual behavior. This keeps coverage-friendly control flow while remaining type‑safe and self‑documenting.test/utils/get-enum-members.test.ts (1)
7-35: Good coverage of both enum AST shapesThe tests cleanly cover both the modern
membersproperty and the legacybody.membersshape, and the identity assertions (toBe) ensure you’re not accidentally cloning. If you ever see real-world nodes that expose bothmembersandbody.members, adding a third precedence test could be a small future enhancement, but what’s here is solid.utils/get-enum-members.ts (1)
24-30: Consider aligning doc wording with the nullish-coalescing orderThe implementation prefers
value.body?.membersand falls back tovalue.members, while the comment reads as thoughbody?.membersis the backward-compat fallback for older parsers. Behavior is fine given your supported shapes, but you may want to either flip the expression tovalue.members ?? value.body?.membersor tweak the comment so it matches the actual precedence.rules/sort-enums.ts (1)
134-140: Non‑null assertions on enum initializers are justified by the preconditionRelying on
members.every(({ initializer }) => initializer)before thereducemakesmember.initializer!and the unconditionalnumericValue: getExpressionNumberValue(member.initializer!)safe, and simplifies the per‑member logic. Just keep in mind that any future change relaxing that guard would need corresponding adjustments here.Also applies to: 159-167
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (24)
package.json(2 hunks)rules/sort-enums.ts(5 hunks)rules/sort-heritage-clauses.ts(1 hunks)rules/sort-imports.ts(2 hunks)rules/sort-object-types.ts(0 hunks)rules/sort-switch-case.ts(4 hunks)test/rules/sort-array-includes.test.ts(1 hunks)test/rules/sort-imports/read-closest-ts-config-by-path.test.ts(1 hunks)test/rules/sort-modules.test.ts(1 hunks)test/rules/sort-objects.test.ts(1 hunks)test/utils/get-enum-members.test.ts(1 hunks)test/utils/get-node-decorators.test.ts(1 hunks)test/utils/validate-rule-json-schema.ts(2 hunks)utils/complete.ts(1 hunks)utils/compute-group-name.ts(1 hunks)utils/compute-groups-names.ts(1 hunks)utils/compute-nodes-in-circular-dependencies.ts(2 hunks)utils/get-enum-members.ts(1 hunks)utils/get-eslint-disabled-lines.ts(3 hunks)utils/get-group-index.ts(1 hunks)utils/get-node-decorators.ts(1 hunks)utils/make-newlines-between-fixes.ts(1 hunks)vite.config.ts(0 hunks)vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- rules/sort-object-types.ts
- vite.config.ts
🧰 Additional context used
🧬 Code graph analysis (5)
test/utils/get-enum-members.test.ts (1)
utils/get-enum-members.ts (1)
getEnumMembers(24-31)
utils/compute-groups-names.ts (1)
utils/is-newlines-between-option.ts (1)
isNewlinesBetweenOption(36-40)
utils/compute-group-name.ts (1)
utils/is-newlines-between-option.ts (1)
isNewlinesBetweenOption(36-40)
test/utils/get-node-decorators.test.ts (1)
utils/get-node-decorators.ts (1)
getNodeDecorators(32-39)
utils/get-group-index.ts (1)
utils/is-newlines-between-option.ts (1)
isNewlinesBetweenOption(36-40)
🔇 Additional comments (20)
utils/complete.ts (1)
103-107: V8 ignore guard is well‑placed and non‑intrusiveThe new
/* v8 ignore else */comment correctly targets the defensive branch in the array comparison loop without affecting logic or readability.test/rules/sort-objects.test.ts (1)
923-1037: Additional destructuring test accurately exercises nested default caseThe new
validcase witha: { nested } = { nested: 'default' }andbcorrectly asserts that nested default initializers without cross‑property dependencies are treated as safe, which strengthens coverage without changing rule behavior.utils/compute-groups-names.ts (1)
17-30: Exhaustive-guard comments match existing control flowThe added
v8 ignore/@preservecomments accurately document theisNewlinesBetweenOptionbranch and the finalUnreachableCaseErroras exhaustive guards, without altering how unknown group options are handled.rules/sort-switch-case.ts (2)
140-195: Default‑clause ordering comment clarifies intentThe new “Ensure default is at the end” comment succinctly describes the surrounding swap logic and makes the invariant of
sortingNodesGroupWithDefaultexplicit, with no impact on behavior.
197-267: Block‑sorting guards and coverage hints are consistent with existing logicThe added comments around block ordering (including the
v8 ignore iffor theb === lastNodeGroupcomparator branch and the note about deferring fixes until the second iteration) clearly document defensive paths that are hard to hit in tests, without changing how groups are sorted or how fixes are generated.package.json (1)
63-132: Vitest tooling bump looks aligned; confirm CI on Vitest 4Upgrading
@vitest/coverage-v8andvitestto^4.0.13matches the new v8 coverage setup; just ensure your CI matrix (Node versions, reporters, coverage flags) has been exercised against Vitest 4 and still passes.utils/compute-group-name.ts (1)
16-31: Exhaustive-guard annotations are clear and behavior‑preservingThe new
v8 ignore/@preservecomments correctly mark theisNewlinesBetweenOptionbranch and finalUnreachableCaseErroras exhaustive guards for unsupported group shapes, without changing the semantics ofcomputeGroupName.test/utils/validate-rule-json-schema.ts (1)
21-44: Coverage-guard comments correctly identify defensive schema assertionsThe added
v8 ignore ifcomments on the schema‑description and weak index‑signature checks accurately describe them as defensive guards that are difficult to exercise in unit tests, while leaving the validation behavior unchanged.test/rules/sort-array-includes.test.ts (1)
5769-5795: New non-applicability tests look correct and targetedThe added cases accurately pin down that the rule ignores arrays passed through other methods and computed property access to
['includes'], which matches the intended scope without overfitting the implementation.utils/get-node-decorators.ts (1)
32-38: Decorator accessor remains simple and robustWrapping the return and keeping the
no-unnecessary-conditionsuppression preserves the intended “safe accessor” behavior while playing nicely with linting; no functional issues here.test/rules/sort-modules.test.ts (1)
23-60: New alphabetical tests nicely cover edge-module shapesThe added
validcases for empty files, single-statement files, export lists, and module declarations without bodies look correct and align with the rule’s expected “no-op” behavior; good targeted coverage with no behavior changes required.utils/compute-nodes-in-circular-dependencies.ts (1)
55-63: Coverage pragmas keep DFS logic intactThe added
v8 ignorecomments around thecycleStartIndex !== -1check and thedependencyElementguard don’t affect the DFS or cycle detection semantics; they simply mark defensive paths for coverage and are consistent with the existing structure.Also applies to: 69-77
rules/sort-enums.ts (1)
326-329: Updated v8 ignore guards around unreachable branches look consistentThe refined
v8 ignorecomments on the default branches ofgetBinaryExpressionNumberValue,computeNodeValueGetter, andgetUnaryExpressionNumberValuecorrectly annotate the “should be unreachable” paths without changing control flow or error behavior.Also applies to: 349-352, 396-399
utils/get-group-index.ts (1)
52-65: Exhaustive-guard comments correctly document non-matching group directivesThe added
v8 ignorecomments around theisNewlinesBetweenOptioncheck and theUnreachableCaseErrormake the intent of these defensive branches explicit without altering the matching behavior for real group entries.rules/sort-imports.ts (1)
590-608: Coverage hints around rare TS import-equals shapes are non-intrusiveThe
/* v8 ignore if */before the!qualifiedNameguard and the final/* v8 ignore next */before returningnullingetQualifiedNameDependencyNamesimply mark defensive/unreachable paths for coverage; the dependency resolution logic forTSImportEqualsDeclarationstays unchanged.Also applies to: 641-652
utils/get-eslint-disabled-lines.ts (1)
58-65: Refined disable/enable coverage guards keep state machine behavior intactThe added
v8 ignorecomments and the scopedeslint-enablecase preserve the original handling ofeslint-disable*/eslint-enabledirectives (including the “enable without prior disable” early-continue) while just annotating hard-to-hit or unreachable paths for coverage.Also applies to: 76-91
rules/sort-heritage-clauses.ts (1)
210-224: Heritage-clause name extraction remains defensive with clearer coverage hintsThe added
v8 ignorecomments just document the recursivepropertybranch and the final “unexpected expression” throw as exhaustive guards; the function still correctly handles identifiers, member-like expressions, and fails loudly on unsupported shapes.test/utils/get-node-decorators.test.ts (1)
1-46: New tests give direct, type-safe coverage forgetNodeDecoratorsThe helper type
NodeWithDecoratorsParameterplus the minimal AST stubs exercise both “decorators present” and “decorators missing” paths cleanly; this is a good, low-noise addition to utility coverage.vitest.config.ts (1)
1-19: LGTM! Clean Vitest configuration with strict coverage enforcement.The configuration correctly merges the base Vite config with test-specific settings and enforces 100% coverage thresholds across all metrics, aligning with the PR's goal of improving test coverage under Vitest 4's v8 provider.
test/rules/sort-imports/read-closest-ts-config-by-path.test.ts (1)
30-37: LGTM! Mock enhancement for coverage.The updated mock now exercises the
getCanonicalFileNamecallback by invoking it with a test value, ensuring that code paths passing this callback are covered. This aligns with the PR's goal of improving test coverage under Vitest 4.
| it('ignores computed property access', async () => { | ||
| await valid({ | ||
| code: dedent` | ||
| ['c', 'b', 'a']['includes']('a') |
There was a problem hiding this comment.
I didn't think of that use case ! 😄
I don't think it will every be used though.
There was a problem hiding this comment.
This is a new requirement for Vitest to ensure test coverage. It would have been necessary if it were a lie. We need tests to ensure that 'if' is 'false'.
| options: [options], | ||
| }) | ||
|
|
||
| await valid({ |
There was a problem hiding this comment.
I don't think that this test suits: a and b don't depend on each other.
However, I realize that the following is marked as valid even though it shouldn't:
let {
a = nested,
b: { nested } = { nested: 'default' },
} = obj;It's an uncommon use case !
There was a problem hiding this comment.
| } as unknown as TSESTree.Decorator | ||
| } | ||
|
|
||
| function createNodeWithDecorators( |
There was a problem hiding this comment.
[nit] You can write this in a concise way as well:
function createNodeWithDecorators(
decorators?: TSESTree.Decorator[],
): NodeWithDecoratorsParameter {
return {
type: 'ClassDeclaration',
...(decorators ? { decorators} : {}) ,
} as unknown as NodeWithDecoratorsParameter
}
}
azat-io
force-pushed
the
test/coverage
branch
from
a927a09 to
a36bddf
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rules/sort-enums.ts (1)
140-179: Non-null assertions onmember.initializerare consistent with the guardUsing
member.initializer!for bothextractDependenciesandgetExpressionNumberValueis safe given the early return when any member lacks an initializer (members.every(({ initializer }) => initializer)). This also makes control flow clearer than repeated optional checks.If you want to make the invariant more obvious to future readers, you could add a short comment near the
everycheck noting that later code assumesmember.initializeris always defined.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (24)
package.json(2 hunks)rules/sort-enums.ts(5 hunks)rules/sort-heritage-clauses.ts(1 hunks)rules/sort-imports.ts(2 hunks)rules/sort-object-types.ts(0 hunks)rules/sort-switch-case.ts(4 hunks)test/rules/sort-array-includes.test.ts(1 hunks)test/rules/sort-imports/read-closest-ts-config-by-path.test.ts(1 hunks)test/rules/sort-modules.test.ts(1 hunks)test/rules/sort-objects.test.ts(1 hunks)test/utils/get-enum-members.test.ts(1 hunks)test/utils/get-node-decorators.test.ts(1 hunks)test/utils/validate-rule-json-schema.ts(2 hunks)utils/complete.ts(1 hunks)utils/compute-group-name.ts(1 hunks)utils/compute-groups-names.ts(1 hunks)utils/compute-nodes-in-circular-dependencies.ts(2 hunks)utils/get-enum-members.ts(1 hunks)utils/get-eslint-disabled-lines.ts(3 hunks)utils/get-group-index.ts(1 hunks)utils/get-node-decorators.ts(1 hunks)utils/make-newlines-between-fixes.ts(1 hunks)vite.config.ts(0 hunks)vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- rules/sort-object-types.ts
- vite.config.ts
✅ Files skipped from review due to trivial changes (1)
- utils/complete.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- test/utils/get-enum-members.test.ts
- test/utils/validate-rule-json-schema.ts
- utils/make-newlines-between-fixes.ts
- test/rules/sort-objects.test.ts
- utils/compute-groups-names.ts
- utils/compute-group-name.ts
- vitest.config.ts
- utils/get-node-decorators.ts
- test/rules/sort-modules.test.ts
- test/rules/sort-array-includes.test.ts
- test/utils/get-node-decorators.test.ts
- rules/sort-switch-case.ts
- utils/get-group-index.ts
- rules/sort-imports.ts
- utils/get-eslint-disabled-lines.ts
🔇 Additional comments (9)
rules/sort-enums.ts (3)
299-335:getBinaryExpressionNumberValuedefault branch and coverage hint look goodThe
defaultarm returningnullfor unsupported operators is a sensible defensive fallback, and the/* v8 ignore next 2 -- @preserve Unsure if we can reach it. */comment is correctly positioned immediately before the unreachable default to keep coverage noise down.
345-358: UnreachablesortByValuedefault with v8 ignore is appropriateGiven the declared
sortByValueenum ('always' | 'ifNumericEnum' | 'never'), thedefaultcase throwingUnreachableCaseErroris a good safety net. The/* v8 ignore next 2 -- @preserve Unsure if we can reach it. */keeps this defensive path out of coverage metrics without changing behavior.
395-405: Unary operator fallback and coverage hint are consistent with the restFalling back to
nullfor unhandled unary operators matches the binary-expression helper’s behavior, and the new v8 ignore comment is aligned with thedefaultarm so coverage won’t be penalized for this defensive path.test/rules/sort-imports/read-closest-ts-config-by-path.test.ts (1)
30-37: LGTM! Mock now exercises the callback parameter.The updated mock signature properly exercises the
getCanonicalFileNamecallback by invoking it during cache creation. This ensures the code path that provides and uses this callback is covered by tests, aligning with the PR's objective to improve test coverage.rules/sort-heritage-clauses.ts (1)
221-225: LGTM! Coverage hints properly aligned for Vitest 4.The repositioned v8 ignore directives with
@preserveensure the coverage tool correctly excludes these defensive branches after transpilation. The exhaustive-guard comment on line 221 and the should-never-throw comment on line 225 accurately describe their purpose.utils/compute-nodes-in-circular-dependencies.ts (2)
55-62: Cycle index guard and v8 directive keep cycle detection behavior intactThe
cycleStartIndexguard remains purely defensive, and the added/* v8 ignore else */correctly documents the unreachable-else assumption without changing how cycles are collected frompath. No changes needed.
69-79: Dependency guard + v8 ignore comment preserve defensive behaviorWrapping the recursive call in
if (dependencyElement)with a/* v8 ignore next */comment keeps the previous defensive fallback (skip when lookup fails) while making the intended “missing dependency is unreachable” assumption explicit for coverage. Logic and control flow are unchanged.utils/get-enum-members.ts (1)
27-30: Parenthesized return + eslint-disable comment are behavior-neutralThe function still returns
value.body?.members ?? value.memberswith the same precedence and nullish-coalescing behavior; only formatting and comment placement changed. The inlineeslint-disable-next-lineclearly applies to the legacybodyaccess. All good.package.json (1)
86-86: Version bump to Vitest 4.0.13 is compatible and properly configured.Verification confirms Vitest 4.0.13 is a stable release with no breaking changes in this patch version. The migration from v3 to v4 has been properly implemented:
- vitest.config.ts correctly configured with v8 coverage provider and 100% thresholds
- vite.config.ts properly updated (test.coverage removed)
- @vitest/coverage-v8 matched to version 4.0.13
- Node engine constraint (^20.0.0 || >=22.0.0) is compatible with Vitest 4
Changes approved.
azat-io
force-pushed
the
test/coverage
branch
from
a36bddf to
d2dc998
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
utils/get-eslint-disabled-lines.ts (1)
76-90: Block-scopedeslint-enablecase and coverage hints are correct but slightly brittleWrapping the
'eslint-enable'case body in{ ... }is semantically neutral here (no new lexical bindings), andcontinuestill targets the surroundingforloop whilebreakstill exits theswitch, so runtime behavior is unchanged. The added v8 ignore hints for the defensiveif (!lineRulePermanentlyDisabled)and the unreachabledefaultcase also match their intent.The only minor downside is that
/* v8 ignore next ... */counts are easy to desynchronize if more statements are inserted into these cases in the future; a quick comment near them (or test that asserts coverage expectations) can help catch accidental drift later.utils/make-newlines-between-fixes.ts (1)
165-172: Avoid non‑null assertion by tighteningcomputeRangeReplacement’s return type
computeRangeReplacementis typed/JSDoc’d as returningundefined | string, but the implementation always returns astring. The!at the call site is only compensating for this mismatch and could hide future regressions ifundefinedis reintroduced.You can simplify and make this safer by narrowing the helper’s return type to
string, updating its JSDoc, and dropping the non‑null assertion:- let rangeReplacement = computeRangeReplacement({ - isOnSameLine: - sortingNode.node.loc.end.line === nextSortingNode.node.loc.start.line, - textBetweenNodes, - newlinesBetween, - })! - - fixes.push(fixer.replaceTextRange(rangeToReplace, rangeReplacement)) + let rangeReplacement = computeRangeReplacement({ + isOnSameLine: + sortingNode.node.loc.end.line === nextSortingNode.node.loc.start.line, + textBetweenNodes, + newlinesBetween, + }) + + fixes.push(fixer.replaceTextRange(rangeToReplace, rangeReplacement))And for the helper itself (JSDoc + signature) something along these lines:
/** * @returns Replacement text with correct newlines. */ function computeRangeReplacement({ textBetweenNodes, newlinesBetween, isOnSameLine, }: { textBetweenNodes: string newlinesBetween: number isOnSameLine: boolean }): string { // existing implementation unchanged }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (24)
package.json(2 hunks)rules/sort-enums.ts(5 hunks)rules/sort-heritage-clauses.ts(1 hunks)rules/sort-imports.ts(2 hunks)rules/sort-object-types.ts(0 hunks)rules/sort-switch-case.ts(4 hunks)test/rules/sort-array-includes.test.ts(1 hunks)test/rules/sort-imports/read-closest-ts-config-by-path.test.ts(1 hunks)test/rules/sort-modules.test.ts(1 hunks)test/rules/sort-objects.test.ts(1 hunks)test/utils/get-enum-members.test.ts(1 hunks)test/utils/get-node-decorators.test.ts(1 hunks)test/utils/validate-rule-json-schema.ts(2 hunks)utils/complete.ts(1 hunks)utils/compute-group-name.ts(1 hunks)utils/compute-groups-names.ts(1 hunks)utils/compute-nodes-in-circular-dependencies.ts(2 hunks)utils/get-enum-members.ts(1 hunks)utils/get-eslint-disabled-lines.ts(3 hunks)utils/get-group-index.ts(1 hunks)utils/get-node-decorators.ts(1 hunks)utils/make-newlines-between-fixes.ts(1 hunks)vite.config.ts(0 hunks)vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- rules/sort-object-types.ts
- vite.config.ts
✅ Files skipped from review due to trivial changes (2)
- test/utils/validate-rule-json-schema.ts
- rules/sort-switch-case.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- test/utils/get-enum-members.test.ts
- utils/compute-group-name.ts
- test/rules/sort-objects.test.ts
- rules/sort-enums.ts
- test/rules/sort-modules.test.ts
- utils/compute-nodes-in-circular-dependencies.ts
- utils/complete.ts
- utils/get-group-index.ts
- utils/get-node-decorators.ts
- vitest.config.ts
- test/rules/sort-imports/read-closest-ts-config-by-path.test.ts
- rules/sort-heritage-clauses.ts
- rules/sort-imports.ts
- test/utils/get-node-decorators.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
utils/compute-groups-names.ts (1)
utils/is-newlines-between-option.ts (1)
isNewlinesBetweenOption(36-40)
🔇 Additional comments (5)
utils/get-enum-members.ts (1)
27-30: Test coverage verified for both code paths.The tests confirm that both the direct
membersaccess and thebody?.membersfallback are exercised:
- Test 1 covers direct member access on modern AST shape
- Test 2 covers fallback to legacy
body.memberswhen neededThe removal of the v8 ignore directive is justified—both defensive paths are now explicitly tested.
package.json (1)
86-86: Vitest devDependency bumps look consistent with the new coverage setupMoving both
@vitest/coverage-v8andvitestto the same^4.0.13range matches the dedicated Vitest v4 coverage config and keeps tooling in sync. No issues from thepackage.jsonside.Please just double‑check against the Vitest 4 docs that
vitest --run --coveragestill maps to the expected coverage provider/settings in your newvitest.config.ts(in case any CLI flags changed semantics between v3 and v4).Also applies to: 131-131
test/rules/sort-array-includes.test.ts (1)
5477-5504: Additional “non-applicability” tests for sort-array-includes look solidThe new cases for
.map/.filter/.forEachand['includes']computed access clearly lock in the intended “ignore non-.includes/ computed property” behavior while driving coverage on those guard branches. No issues.utils/compute-groups-names.ts (1)
24-29: Exhaustive guard and v8 coverage hints look consistent and non-breakingThe new
/* v8 ignore else */and/* v8 ignore next */comments clearly target the unreachable “unsupported group option” path without changing the control flow: the three valid shapes are still handled explicitly, and everything else still falls through tonew UnreachableCaseError(group). This should be more stable than aignore next 3span against future transpilation changes, and it aligns with the exhaustive-guard pattern used elsewhere in the plugin.Given these directives depend on how the current Vitest + esbuild + ast-v8-to-istanbul stack interprets them, it’s worth re-checking coverage locally after any future tooling upgrades to ensure the hints still attach to the intended branches.
utils/get-eslint-disabled-lines.ts (1)
58-64: Includes‑rule false branch v8 ignore looks safe and non‑behavioralThe new
/* v8 ignore next 3 -- @preserve … */on the!includesRulepath is purely for coverage tooling and doesn’t alter control flow: the earlycontinueremains exactly as before and still correctly skips comments that don’t apply to the target rule. From a runtime perspective this segment remains sound.
Description
Clarified and repositioned our
/* v8 ignore ... */hints so they still align with the intended AST nodes under Vitest 4’s new coverage stack (esbuild 0.25+ast-v8-to-istanbul@0.3.8). Without this, the transpiled output moved those comments away from the relevant branches, causing them to count toward coverage again; the tests now confirm all defensive paths are exercised or explicitly ignored.Additional context
N/A.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following