Skip to content

Commit 60b84f2

Browse files
the-ultclaude
andauthored
fix(core,hono): generalize the #3634 tag-identity fix and fix the same bug in hono (#3641)
* fix(angular): match multi-word tags with acronym prefixes in tags-split mode `getRelevantVerbOptionsForTag` compared tags via `camel()`, which converts "AB Widget" → "aBWidget" but the dictionary key from `generateTargetTags` is `kebab("AB Widget")` = "ab-widget", and `camel("ab-widget")` = "abWidget". The mismatch caused the tag's operations to be invisible to the header builder, so `hasBuiltInFilteredQueryParams` was always false and `filterParams` was never emitted for acronym-prefixed multi-word tags. Fix: compare using `kebab()` in both places — it is the canonical form already used as the tag dictionary key, and it round-trips correctly for all tag shapes. Closes #3634 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(core): unify tag-bucket identity behind a single shared module The rule "which file does an operation belong to in tags/tags-split mode" (`kebab(tags[0] ?? 'default')`) was hand-copied across 6 sites in 4 packages, in two slightly different variants. Each copy independently decided how to derive and compare the tag key, so the build-the-key side and the look-up-the- key side could disagree — the exact failure behind the Angular `filterParams` bug (#3634), and a still-live identical bug in @orval/hono. Introduce one source of truth in @orval/core: - getTagKey(tag?) — canonical, idempotent bucket key (kebab-based) - getOperationTagKey(op) — key for an operation's primary tag - isOperationInTagBucket(op, key) — canonical-vs-canonical membership test Migrate every consumer to it: core target-tags writer, angular (getRelevantVerbOptionsForTag, http-resource paths), query (angular header), mock (array-item factory scope), and hono. hono fixes (#3634 class, previously broken for multi-word/acronym tags): - getVerbOptionGroupByTag now keys by the canonical tag, so tags-split handler/zod/context files land in canonical (kebab) directories that match the composite route's import paths instead of raw `"AB Widget"/` dirs. - the composite-route handler filter compared a raw `tags[0]` against the canonical key and silently produced an empty import block; it now uses isOperationInTagBucket. Verified end-to-end with an acronym tag ("AB Widget"): angular tags-split emits the `filterParams` definition alongside its call sites, and hono composite routes import and wire up the tag's handlers from a matching dir. Single-word tags are unaffected (kebab is a no-op) — all sample snapshots are byte-identical to before. New unit tests cover key derivation, idempotency, and acronym/default-bucket matching. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(core): lock multi-word/acronym tag handling with a CI-gated regression Adds issue-3634.yaml — a `Widget` (single-word control) + `AB Widget` (acronym trigger) spec where both operations carry a query param — and wires it through two orval-tests configs whose generated output is snapshotted and compiled in CI (`test:snapshots` + `orval-tests build`): - angular tags-split: the `ab-widget` service must DEFINE `filterParams`, not just call it. A regression drops the helper definition (the #3634 symptom) and fails both the snapshot diff and the TypeScript compile (TS2304). - hono composite-routes tags-split: handler/zod/context files must land in the canonical `ab-widget/` directory and the composite `routes.ts` must import the tag's handlers from there. A regression reverts to a raw `AB Widget/` directory and an empty handler import block. None of the existing duplicate-tag fixtures cover this: `Dup Tag`/`dup tag` all camelCase to the same `dupTag`, so the previous (buggy) camel-based matching grouped them correctly. Only an acronym-prefixed tag exposes the camel-vs-kebab divergence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(core): treat empty/whitespace tags as the default bucket getTagKey's docstring promised that "missing or empty tags" map to the DefaultTag bucket, but the implementation only fell back for null/undefined, so a spec with `tags: ['']` would route to a `""` bucket. Trim and fall back before kebab-casing so empty/whitespace tags resolve to `default`. Tighten isOperationInTagBucket to treat only `undefined` (not any falsy value) as the "no tag filter" case, so an accidental empty-string tagKey normalises to the default bucket instead of silently matching every operation. JSDoc updated to match, and unit tests now cover the empty/whitespace fallback for both functions. Addresses CodeRabbit and Copilot review feedback on #3641. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(core,angular): route default-bucket exports and empty tags through the canonical helper Generalize the tag-identity fix to two more call sites so they agree with `isOperationInTagBucket`, the single source of truth for tag-bucket identity: - core/target-tags: the footer `operationNames` filter excluded untagged operations (`tags.length > 0`), so the implicit `default` bucket file could miss its per-operation `*ClientResult` exports. Filter the already default-normalised `operations` array via `isOperationInTagBucket`. - angular/utils: `getRelevantVerbOptionsForTag` treated an empty-string tag as "no filter" (`if (!tag)`); guard on `tag == null` so an empty/whitespace tag normalises to the `default` bucket like the core writer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(core): cover untagged operations in the default-bucket footer Regression test for the target-tags fix: an untagged operation routed into the implicit `default` bucket must have its name passed to that bucket's footer (which drives per-operation `*ClientResult` return-type exports). Fails against the old `tags.length > 0` filter, which dropped untagged operations entirely. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(core): regenerate issue-3103 default-bucket snapshot for footer fix The untagged `getApiProduct` operation routes into the implicit `default` bucket; with the target-tags footer fix it now correctly emits its `GetApiProductClientResult` return-type export, which the committed snapshot predated. This is end-to-end coverage of the default-bucket footer fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: stop tracking .impeccable tool cache The `.impeccable/hook.cache.json` session-cache file was accidentally committed in this branch (contains a local session UUID and absolute paths). Untrack it and add `.impeccable` to .gitignore. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: The Ult <the-ult@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 7320d75 commit 60b84f2

34 files changed

Lines changed: 1128 additions & 61 deletions

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
mock-backend
22
dist
33
.rpt2_cache
4+
.impeccable
45

56
### macOS ###
67
*.DS_Store

packages/angular/src/http-resource.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@ import {
1818
getFileInfo,
1919
getFullRoute,
2020
GetterPropType,
21+
getOperationTagKey,
22+
getTagKey,
2123
isObject,
2224
isSyntheticDefaultImportsAllow,
2325
jsDoc,
24-
kebab,
2526
makeRouteSafe,
2627
type NormalizedOutputOptions,
2728
type OpenApiInfoObject,
@@ -172,7 +173,7 @@ const getVerbOptionsRecord = (
172173
);
173174

174175
const getPrimaryTag = (verbOption: GeneratorVerbOptions): string =>
175-
kebab(verbOption.tags[0] ?? 'default');
176+
getOperationTagKey(verbOption);
176177

177178
const hasRetrievalOperations = (
178179
verbOptions: Record<string, GeneratorVerbOptions>,
@@ -1495,11 +1496,11 @@ const getHttpResourceExtraFilePath = (
14951496

14961497
switch (output.mode) {
14971498
case OutputMode.TAGS: {
1498-
const normalizedTag = kebab(tag ?? 'default');
1499+
const normalizedTag = getTagKey(tag);
14991500
return upath.joinSafe(dirname, `${normalizedTag}.resource${extension}`);
15001501
}
15011502
case OutputMode.TAGS_SPLIT: {
1502-
const normalizedTag = kebab(tag ?? 'default');
1503+
const normalizedTag = getTagKey(tag);
15031504
return upath.joinSafe(
15041505
dirname,
15051506
normalizedTag,

packages/angular/src/utils.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,14 +482,28 @@ describe('getRelevantVerbOptionsForTag', () => {
482482
expect(result[0].operationId).toBe('op1');
483483
});
484484

485-
it('matches tags case-insensitively via camelCase normalisation', () => {
485+
it('matches tags case-insensitively via kebab normalisation', () => {
486486
const verbOptions = {
487487
op1: makeVerb('op1', ['Pet-Store']),
488488
};
489489
const result = getRelevantVerbOptionsForTag(verbOptions, 'pet-store');
490490
expect(result).toHaveLength(1);
491491
});
492492

493+
it('matches multi-word tags with acronym prefixes (e.g. "AB Widget" → "ab-widget")', () => {
494+
const verbOptions = {
495+
op1: makeVerb('op1', ['AB Widget']),
496+
op2: makeVerb('op2', ['Widget']),
497+
};
498+
const abResult = getRelevantVerbOptionsForTag(verbOptions, 'ab-widget');
499+
expect(abResult).toHaveLength(1);
500+
expect(abResult[0].operationId).toBe('op1');
501+
502+
const widgetResult = getRelevantVerbOptionsForTag(verbOptions, 'widget');
503+
expect(widgetResult).toHaveLength(1);
504+
expect(widgetResult[0].operationId).toBe('op2');
505+
});
506+
493507
it('returns empty array when no verbs match the tag', () => {
494508
const verbOptions = {
495509
op1: makeVerb('op1', ['pets']),
@@ -500,4 +514,14 @@ describe('getRelevantVerbOptionsForTag', () => {
500514
it('returns empty array for empty verbOptions', () => {
501515
expect(getRelevantVerbOptionsForTag({}, 'pets')).toHaveLength(0);
502516
});
517+
518+
it('treats an empty-string tag as the default bucket, not as "no filter"', () => {
519+
const verbOptions = {
520+
untagged: makeVerb('untagged', []),
521+
tagged: makeVerb('tagged', ['pets']),
522+
};
523+
const result = getRelevantVerbOptionsForTag(verbOptions, '');
524+
expect(result).toHaveLength(1);
525+
expect(result[0].operationId).toBe('untagged');
526+
});
503527
});

packages/angular/src/utils.ts

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import {
2-
camel,
3-
DefaultTag,
42
type GeneratorVerbOptions,
53
getAngularFilteredParamsHelperBody,
64
getDefaultContentType,
75
isBoolean,
86
isObject,
7+
isOperationInTagBucket,
98
type NormalizedOutputOptions,
109
pascal,
1110
type ResReqTypesValue,
@@ -143,28 +142,23 @@ export const createRouteRegistry = () => {
143142
/**
144143
* Returns only the operations that belong to the current tag output.
145144
*
146-
* In `tags` / `tags-split` mode the writer may route untagged operations into
147-
* the implicit `default` bucket. When a generated tag file targets that bucket
148-
* we also include operations whose original tag list was empty; a literal
149-
* user-defined `default` tag is treated like any other tag unless untagged
150-
* operations are present in the same output.
145+
* Tag matching is delegated to {@link isOperationInTagBucket}, the single source
146+
* of truth for tag-bucket identity. Untagged operations resolve to the implicit
147+
* `default` bucket, matching how the core writer routes them in
148+
* `tags` / `tags-split` mode.
151149
*/
152150
export const getRelevantVerbOptionsForTag = (
153151
verbOptions: Record<string, GeneratorVerbOptions>,
154152
tag?: string,
155153
): GeneratorVerbOptions[] => {
156154
const allVerbOptions = Object.values(verbOptions);
157-
if (!tag) return allVerbOptions;
155+
// Only an absent tag means "no filter"; an empty/whitespace tag is a real
156+
// bucket key that `isOperationInTagBucket` normalises to `default`, matching
157+
// the core writer instead of silently matching every operation.
158+
if (tag == null) return allVerbOptions;
158159

159-
const camelTag = camel(tag);
160-
const includeUntaggedOperations =
161-
tag === DefaultTag &&
162-
allVerbOptions.some((verbOption) => verbOption.tags.length === 0);
163-
164-
return allVerbOptions.filter(
165-
(verbOption) =>
166-
verbOption.tags.some((currentTag) => camel(currentTag) === camelTag) ||
167-
(includeUntaggedOperations && verbOption.tags.length === 0),
160+
return allVerbOptions.filter((verbOption) =>
161+
isOperationInTagBucket(verbOption, tag),
168162
);
169163
};
170164

packages/core/src/utils/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@ export * from './resolve-version';
2020
export * from './schemas-options';
2121
export * from './sort';
2222
export * from './string';
23+
export * from './tags';
2324
export * from './tsconfig';
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { getOperationTagKey, getTagKey, isOperationInTagBucket } from './tags';
4+
import { kebab } from './case';
5+
6+
describe('getTagKey', () => {
7+
it('kebab-cases a simple tag', () => {
8+
expect(getTagKey('pets')).toBe('pets');
9+
});
10+
11+
it('kebab-cases multi-word tags', () => {
12+
expect(getTagKey('Pet Store')).toBe('pet-store');
13+
});
14+
15+
it('normalises acronym-prefixed multi-word tags (the bug class)', () => {
16+
// `camel("AB Widget")` would give "aBWidget", which does NOT round-trip
17+
// through the bucket key. `kebab` does.
18+
expect(getTagKey('AB Widget')).toBe('ab-widget');
19+
});
20+
21+
it('is idempotent: applying it to its own output is a no-op', () => {
22+
for (const tag of [
23+
'pets',
24+
'Pet Store',
25+
'AB Widget',
26+
'user-admin',
27+
'HTTPServer',
28+
'v1.users',
29+
]) {
30+
const once = getTagKey(tag);
31+
expect(getTagKey(once)).toBe(once);
32+
}
33+
});
34+
35+
it('falls back to the default bucket for missing tags', () => {
36+
expect(getTagKey()).toBe('default');
37+
expect(getTagKey(undefined)).toBe('default');
38+
});
39+
40+
it('falls back to the default bucket for empty/whitespace tags', () => {
41+
expect(getTagKey('')).toBe('default');
42+
expect(getTagKey(' ')).toBe('default');
43+
});
44+
45+
it('agrees with a raw kebab() call on the same input', () => {
46+
for (const tag of ['pets', 'Pet Store', 'AB Widget', 'user/admin']) {
47+
expect(getTagKey(tag)).toBe(kebab(tag));
48+
}
49+
});
50+
});
51+
52+
describe('getOperationTagKey', () => {
53+
it('derives the key from the first tag', () => {
54+
expect(getOperationTagKey({ tags: ['AB Widget', 'Other'] })).toBe(
55+
'ab-widget',
56+
);
57+
});
58+
59+
it('maps untagged operations to the default bucket', () => {
60+
expect(getOperationTagKey({ tags: [] })).toBe('default');
61+
});
62+
});
63+
64+
describe('isOperationInTagBucket', () => {
65+
it('matches an operation to its own bucket regardless of spelling', () => {
66+
const op = { tags: ['AB Widget'] };
67+
expect(isOperationInTagBucket(op, 'ab-widget')).toBe(true);
68+
// A caller that passes the raw tag still matches, because both sides are
69+
// normalised.
70+
expect(isOperationInTagBucket(op, 'AB Widget')).toBe(true);
71+
});
72+
73+
it('does not match an operation in a different bucket', () => {
74+
expect(isOperationInTagBucket({ tags: ['Widget'] }, 'ab-widget')).toBe(
75+
false,
76+
);
77+
});
78+
79+
it('matches every operation when no tag bucket is given', () => {
80+
expect(isOperationInTagBucket({ tags: ['anything'] })).toBe(true);
81+
expect(isOperationInTagBucket({ tags: [] }, undefined)).toBe(true);
82+
});
83+
84+
it('treats an empty/whitespace tag bucket as the default bucket, not match-all', () => {
85+
expect(isOperationInTagBucket({ tags: [] }, '')).toBe(true);
86+
expect(isOperationInTagBucket({ tags: ['anything'] }, '')).toBe(false);
87+
});
88+
89+
it('places untagged operations in the default bucket', () => {
90+
expect(isOperationInTagBucket({ tags: [] }, 'default')).toBe(true);
91+
expect(isOperationInTagBucket({ tags: [] }, 'pets')).toBe(false);
92+
});
93+
});

packages/core/src/utils/tags.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { DefaultTag } from '../types';
2+
import { kebab } from './case';
3+
4+
/**
5+
* Canonical bucket key for a single OpenAPI tag.
6+
*
7+
* In `tags` / `tags-split` mode operations are routed into files by their first
8+
* tag. This function is the **single source of truth** for turning a tag (or a
9+
* missing tag) into the key that identifies that file bucket. Every place that
10+
* groups operations by tag, derives a per-tag file/directory name, or checks
11+
* whether an operation belongs to a tag MUST go through here so that the
12+
* "build the key" side and the "look the key up" side can never disagree.
13+
*
14+
* The result is `kebab`-cased. `kebab` is idempotent
15+
* (`kebab(kebab(x)) === kebab(x)`), so it is always safe to call this on a value
16+
* that is already a canonical key. Other case functions (`camel`, `pascal`) are
17+
* NOT safe here: they do not round-trip through the bucket key for tags
18+
* containing acronyms or spaces (e.g. `"AB Widget"`), which is exactly the class
19+
* of bug this module exists to prevent.
20+
*
21+
* Missing or empty tags map to the implicit {@link DefaultTag} bucket.
22+
*/
23+
export function getTagKey(tag?: string): string {
24+
const normalizedTag = tag?.trim();
25+
return kebab(normalizedTag ? normalizedTag : DefaultTag);
26+
}
27+
28+
/**
29+
* Canonical bucket key for an operation, derived from its primary (first) tag.
30+
*
31+
* Untagged operations resolve to the {@link DefaultTag} bucket.
32+
*/
33+
export function getOperationTagKey(operation: { tags: string[] }): string {
34+
return getTagKey(operation.tags[0]);
35+
}
36+
37+
/**
38+
* Whether an operation belongs to the given tag bucket.
39+
*
40+
* Both sides are normalised through {@link getTagKey}, so the comparison is
41+
* correct regardless of how `tagKey` was spelled or cased by the caller. An
42+
* absent (`undefined`) `tagKey` matches every operation (the "no tag filter"
43+
* case); an empty/whitespace `tagKey` is normalised to the {@link DefaultTag}
44+
* bucket like any other tag.
45+
*
46+
* Prefer this over hand-rolling `operation.tags[0] === tagKey`: a raw tag
47+
* compared against a canonical key silently fails for multi-word/acronym tags.
48+
*/
49+
export function isOperationInTagBucket(
50+
operation: { tags: string[] },
51+
tagKey?: string,
52+
): boolean {
53+
if (tagKey == null) return true;
54+
return getOperationTagKey(operation) === getTagKey(tagKey);
55+
}

packages/core/src/writers/tags-mode.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,3 +410,66 @@ describe('writeTagsMode — schemas import extension follows tsconfig module', (
410410
);
411411
});
412412
});
413+
414+
// Regression: footer `operationNames` drives per-operation return-type exports
415+
// (e.g. Angular `*ClientResult` aliases). Untagged operations are routed into
416+
// the implicit `default` bucket by `addDefaultTagIfEmpty`, so the default
417+
// bucket's footer must receive their names too. The old filter excluded them
418+
// via a `tags.length > 0` guard, so the default-tag file silently dropped its
419+
// footer exports.
420+
421+
describe('writeTagsMode — default-bucket footer includes untagged operations', () => {
422+
let tmpDir: string;
423+
424+
beforeEach(() => {
425+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'orval-tags-mode-'));
426+
});
427+
428+
afterEach(() => {
429+
fs.removeSync(tmpDir);
430+
vi.restoreAllMocks();
431+
});
432+
433+
it('passes untagged operation names to the default-bucket footer', async () => {
434+
const target = path.join(tmpDir, 'petstore.ts');
435+
const baseProps = createSplitModeProps(target);
436+
437+
const footerSpy = vi.fn((_args: { operationNames: string[] }) => ({
438+
implementation: '',
439+
implementationMock: '',
440+
}));
441+
442+
const props = {
443+
...baseProps,
444+
builder: {
445+
...baseProps.builder,
446+
footer: footerSpy,
447+
operations: {
448+
listPets: createSplitModeOperation({
449+
tags: ['pets'],
450+
operationName: 'listPets',
451+
}),
452+
getHealth: createSplitModeOperation({
453+
tags: [],
454+
operationName: 'getHealth',
455+
}),
456+
},
457+
} as unknown as typeof baseProps.builder,
458+
output: createSplitModeOutput(target, { mode: OutputMode.TAGS }),
459+
};
460+
461+
await writeTagsMode({ ...props, needSchema: false });
462+
463+
const operationNamesByBucket = footerSpy.mock.calls.map(
464+
([args]) => args.operationNames,
465+
);
466+
467+
// The untagged op must reach a footer call (the `default` bucket), not be
468+
// dropped entirely, and it must not leak into the `pets` bucket.
469+
const defaultBucket = operationNamesByBucket.find((names) =>
470+
names.includes('getHealth'),
471+
);
472+
expect(defaultBucket).toEqual(['getHealth']);
473+
expect(operationNamesByBucket).toContainEqual(['listPets']);
474+
});
475+
});

0 commit comments

Comments
 (0)