Skip to content

refactor(mock): tighten strict schema name recovery (#3574 follow-up) - #3578

Merged
melloware merged 1 commit into
orval-labs:masterfrom
Hypenate:fix/3574-strict-mock-name-recovery
Jun 10, 2026
Merged

refactor(mock): tighten strict schema name recovery (#3574 follow-up)#3578
melloware merged 1 commit into
orval-labs:masterfrom
Hypenate:fix/3574-strict-mock-name-recovery

Conversation

@Hypenate

@Hypenate Hypenate commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #3575 (closes #3574).

#3575 landed the tags-split strict mock type fix; this PR addresses two review nits from that work:

Changes

  • Document regex couplingcollectStrictMockSchemaTypeNamesFromImplementation now notes it reverse-parses emitted factory syntax and that the robust long-term fix is threading schema names from array-item / faker getters at generation time.
  • Tighter alias recovery — Replace the broad prefix scan and Widget/WidgetMock filter with targeted patterns:
    • MockWithNullableOverrides first type param (unchanged)
    • Third generic alias → schema via inverse of getStrictMockTypeName
    • Non-overridable array return types like ): TenantInfoDtoMock[]

This fixes the residual edge case where both Widget and WidgetMock strict mocks in the same file would drop Widget, while keeping issue-3525-widget-mock-strict snapshot-stable.

Test plan

  • vitest run packages/mock/src/mock-types.test.ts
  • New test: both Widget and WidgetMock collected when present
  • Existing WidgetMock-only regression test passes
  • issue-3525-widget-mock-strict / issue-3574 fixtures unchanged

Relates to #3574

Summary by CodeRabbit

  • Tests

    • Added a unit test to verify collection of multiple strict-mock schema names across factory implementations.
  • Refactor

    • Improved internal pattern-matching for extracting strict-mock schema type names to increase accuracy and maintainability.
  • Documentation

    • Added clarifying inline comments describing handling of nested factories and name-threading considerations.

@melloware melloware added the mock Related to mock generation label Jun 9, 2026
@melloware melloware added this to the 8.17.0 milestone Jun 9, 2026
@Hypenate

Hypenate commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@wadakatu this PR addresses your comments on #3575

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6ee8c9fd-d569-4817-b8ff-127080f6dcec

📥 Commits

Reviewing files that changed from the base of the PR and between 50c00f6 and 6407ce2.

📒 Files selected for processing (3)
  • packages/mock/src/mock-types.test.ts
  • packages/mock/src/mock-types.ts
  • packages/mock/src/msw/index.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/mock/src/msw/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/mock/src/mock-types.test.ts
  • packages/mock/src/mock-types.ts

📝 Walkthrough

Walkthrough

This PR replaces single-regex alias scraping with multiple targeted regexes plus an alias-to-base-type helper, updates the collector to aggregate matches from all patterns, adds a unit test ensuring both Widget and WidgetMock are detected, and adds inline comments in the MSW generator.

Changes

Strict-mock schema-type name extraction improvements

Layer / File(s) Summary
Pattern-based extraction with alias inversion helper
packages/mock/src/mock-types.ts
Replaced single regex constant with three patterns targeting nullable override and strict-mock return type forms; introduced getBaseSchemaTypeFromAlias helper to reverse-parse strict-mock aliases to base schema type names.
Collection loop refactoring
packages/mock/src/mock-types.ts
Updated collectStrictMockSchemaTypeNamesFromImplementation to iterate over new regex patterns and aggregate schema names via the alias-inversion helper, removing previous alias-collision post-filtering.
Test coverage for multi-schema collection
packages/mock/src/mock-types.test.ts
Added test asserting both Widget and WidgetMock are collected when both strict-mock factory signatures are present; maintains prior edge-case tests.
Inline documentation notes
packages/mock/src/msw/index.ts
Inserted comments documenting nested split factory behavior and a regex-coupling note in generateDefinition's strictMockSchemaTypeNames branch.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • orval-labs/orval#3543: Overlaps on strict-mock schema type name extraction refactor and collector behavior.

Suggested reviewers

  • melloware
  • wadakatu

Poem

🐰 A rabbit nibbles through tangled mock lore,
Pulls aliases, patterns, and tests to the fore,
Where Widget and WidgetMock once blurred in the night,
Now captured and sorted — both names in the light,
A small hop for code, a tidy refactor delight.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: refactoring strict mock schema name recovery logic with improved pattern matching.
Linked Issues check ✅ Passed The PR implements targeted fixes to recover schema type names from strict mock factory signatures, addressing the root cause of missing type declarations in #3574.
Out of Scope Changes check ✅ Passed All changes are scoped to mock schema name recovery logic: unit tests, pattern-based extraction, and documentation comments related to the stated objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Match MockWithNullableOverrides alias and return-type patterns only.

Document regex coupling; keep Widget and WidgetMock when both appear.

Co-authored-by: Cursor <cursoragent@cursor.com>
@melloware
melloware force-pushed the fix/3574-strict-mock-name-recovery branch from 50c00f6 to 6407ce2 Compare June 9, 2026 14:21
@wadakatu

Copy link
Copy Markdown
Contributor

Verified locally — LGTM (non-blocking notes below)

What I checked:

  • mock-types.test.ts 25/25, full @orval/mock suite 233/233
  • Regenerated all 7 strict-mock fixture configs (issue3525*, issue3574StrictMockTagsSplit*) — zero churn, both on this branch and after merging current master (fix(mock): enable arrayItems option for MSW-only mock configs (#3576) #3577) locally
  • Regex coverage against actual emit shapes: nested split factories (value.ts / array-item-factory.ts) are always emitted overridable with the full MockWithNullableOverrides<T, O, TMock> signature, so they're covered by both the first-param and third-param patterns. The shapes the new patterns don't match (e.g. non-overridable scalar returns like (): StatusMock => in issue-3525/model/index.faker.ts) all come from paths that thread names structurally (faker writer) or are already covered by response-derived schemaTypeNames — I couldn't construct a false negative.
  • New combination enabled by fix(mock): enable arrayItems option for MSW-only mock configs (#3576) #3577 (strict mock + MSW-only + arrayItems: true, tags-split, nested $ref array items): generated output recovers the nested item alias via this scan and compiles clean with tsc --strict.

Non-blocking — the Widget/WidgetMock co-exist case still doesn't compile end-to-end:

The collection fix works (both names are now kept), but when schemas Widget and WidgetMock actually co-exist in one strict-mocked file, the emitted file still fails tsc: the strict alias for Widget is named WidgetMock, which collides with the schema import:

error TS2440: Import declaration conflicts with local declaration of 'WidgetMock'.
error TS2344: Type 'WidgetMock' does not satisfy the constraint 'Record<keyof Widget, unknown>'.

To be clear, this is not a regression — I ran the same spec against master and it also fails (TS2344 ×4, because the dropped Widget alias makes WidgetMock resolve to the schema import). Both before and after, this input produces non-compiling output; this PR just changes the failure mode. The remaining fix would be collision-aware alias naming (detect that {name}Mock is taken by a real schema and pick a different alias), which I'd suggest tracking as a follow-up issue rather than expanding this PR. Repro spec, strict + tags-split:

Order:
  type: object
  properties:
    widgets:
      type: array
      items:
        $ref: '#/components/schemas/Widget'
    widgetMocks:
      type: array
      items:
        $ref: '#/components/schemas/WidgetMock'

Nit: the ; alternative in STRICT_MOCK_SCHEMA_TYPE_FROM_MOCK_ALIAS_RETURN (): XxxMock;) never fires today — formatMockFactoryDeclaration always emits => right after the return annotation, and the scan runs before prettier. Harmless belt-and-suspenders, fine to keep.

@melloware
melloware merged commit 76256b4 into orval-labs:master Jun 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mock Related to mock generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(mock): prepend strict mock type declarations in tags-split MSW/faker output

3 participants