danabra.mov
An experimental TypeSpec syntax for Lexicon
wip
+363
-17
+360
PLAN.md
+360
PLAN.md
···
1
+
# Emitter Refactoring Plan
2
+
3
+
**Goal:** Improve robustness, correctness, and maintainability while keeping tests passing throughout.
4
+
5
+
**Constraints:**
6
+
- Work incrementally in small, testable chunks
7
+
- Preserve input/output behavior (no breaking changes)
8
+
- Structure matters more than brevity
9
+
- Throw early on unsupported patterns rather than fail silently
10
+
11
+
---
12
+
13
+
## Phase 1: Type Safety & Helper Usage (Low Risk)
14
+
15
+
### 1.1 Replace custom array detection with TypeSpec helper
16
+
17
+
**Current (emitter.ts:1456-1469):**
18
+
```typescript
19
+
private isArrayType(model: Model): boolean {
20
+
if (model.name === "Array" && model.namespace?.name === "TypeSpec") {
21
+
return true;
22
+
}
23
+
if (model.sourceModel) {
24
+
return this.isArrayType(model.sourceModel);
25
+
}
26
+
return false;
27
+
}
28
+
```
29
+
30
+
**Issue:** Custom recursive check that only follows `sourceModel` chain.
31
+
32
+
**Action:**
33
+
- Import `isArrayModelType` from `@typespec/compiler` (used in openapi3 emitter)
34
+
- Replace `this.isArrayType(model)` calls with `isArrayModelType(this.program, model)`
35
+
- Remove `isArrayType` method
36
+
- **Test checkpoint:** `npm test`
37
+
38
+
**Risk:** Low - TypeSpec helper is more robust
39
+
**Lines affected:** 355, 1105, 1456-1469
40
+
41
+
---
42
+
43
+
### 1.2 Fix diagnostic type signatures (remove `as any`)
44
+
45
+
**Current pattern:**
46
+
```typescript
47
+
this.program.reportDiagnostic({
48
+
code: "invalid-model-name",
49
+
message: "...",
50
+
target: model,
51
+
} as any);
52
+
```
53
+
54
+
**Issue:** 8+ diagnostics cast to `as any` - hiding type errors
55
+
56
+
**Action:**
57
+
- Check TypeSpec diagnostic API - likely just needs `severity` field
58
+
- Add proper type signature or create helper:
59
+
```typescript
60
+
private reportError(code: string, message: string, target: any) {
61
+
this.program.reportDiagnostic({
62
+
code,
63
+
message,
64
+
target,
65
+
severity: "error"
66
+
});
67
+
}
68
+
```
69
+
- Replace all `as any` diagnostic casts
70
+
- **Test checkpoint:** `npm test`
71
+
72
+
**Risk:** Very low - just fixing types
73
+
**Lines affected:** 327, 530, 540, 562, 570, 580, 697, 1237, 1241
74
+
75
+
---
76
+
77
+
### 1.3 Simplify Blob detection
78
+
79
+
**Current (emitter.ts:996-1005):** 5 different checks for Blob
80
+
81
+
**Analysis needed:**
82
+
- Understand when each condition actually triggers
83
+
- Check if `isBlob(this.program, model)` decorator check is sufficient
84
+
- If templates, check `isTemplateInstance(model)` and model.name/baseModel
85
+
86
+
**Action:**
87
+
- Add logging to see which conditions actually trigger in tests
88
+
- Simplify to canonical pattern:
89
+
```typescript
90
+
const isBlobModel = isBlob(this.program, model) ||
91
+
(isTemplateInstance(model) && model.templateNode && isBlob(this.program, model.templateNode));
92
+
```
93
+
- **Test checkpoint:** `npm test`
94
+
- If tests fail, understand why and adjust
95
+
96
+
**Risk:** Low-medium - but need to verify against test cases
97
+
**Lines affected:** 996-1005
98
+
99
+
---
100
+
101
+
## Phase 2: Union Handling (Medium Risk)
102
+
103
+
### 2.1 Audit union processing against lexicon spec
104
+
105
+
**Current issue:** Union loop (lines 1146-1254) only handles two cases:
106
+
1. String literals + `string` type → `knownValues`
107
+
2. Model refs → `union`
108
+
109
+
**What about:**
110
+
- Model refs + string literals?
111
+
- Integer literals + `integer` type?
112
+
- Boolean literals?
113
+
- Nested unions?
114
+
- Empty unions?
115
+
116
+
**Action:**
117
+
- Read lexicon spec sections on `union` and `string.knownValues`
118
+
- Map all possible TypeSpec union patterns to lexicon output
119
+
- Create decision tree:
120
+
```
121
+
Union variants:
122
+
├─ All string literals + string type? → knownValues
123
+
├─ All integer literals + integer type? → enum (if supported)
124
+
├─ All model refs? → union
125
+
├─ Model refs + literals? → ERROR (unsupported) or handle
126
+
├─ Contains unknown/never? → open union
127
+
└─ Other? → throw "Unsupported union pattern"
128
+
```
129
+
130
+
**Test checkpoint:** `npm test` after each case added
131
+
132
+
---
133
+
134
+
### 2.2 Implement explicit union handling
135
+
136
+
**Pattern:**
137
+
```typescript
138
+
// Collect variants by category
139
+
const variants = {
140
+
modelRefs: [] as string[],
141
+
stringLiterals: [] as string[],
142
+
integerLiterals: [] as number[],
143
+
hasStringType: false,
144
+
hasIntegerType: false,
145
+
hasUnknown: false,
146
+
};
147
+
148
+
for (const variant of unionType.variants.values()) {
149
+
// Categorize each variant
150
+
}
151
+
152
+
// Decision logic with explicit throws
153
+
if (variants.stringLiterals.length > 0 && variants.hasStringType && variants.modelRefs.length === 0) {
154
+
// knownValues case
155
+
return { type: "string", knownValues: variants.stringLiterals };
156
+
}
157
+
158
+
if (variants.modelRefs.length > 0 && variants.stringLiterals.length === 0) {
159
+
// union case
160
+
return { type: "union", refs: variants.modelRefs };
161
+
}
162
+
163
+
// Unsupported combinations
164
+
if (variants.modelRefs.length > 0 && variants.stringLiterals.length > 0) {
165
+
throw new Error(`Union contains both model refs and string literals - unsupported pattern at ${unionType}`);
166
+
}
167
+
168
+
// Unknown pattern
169
+
throw new Error(`Unsupported union pattern: ${JSON.stringify(variants)}`);
170
+
```
171
+
172
+
**Test checkpoint:** `npm test` - may reveal unsupported patterns in tests
173
+
174
+
**Risk:** Medium - may expose edge cases
175
+
**Lines affected:** 1123-1255
176
+
177
+
---
178
+
179
+
## Phase 3: Mutable State Refactoring (Higher Risk)
180
+
181
+
### 3.1 Research TypeSpec context patterns
182
+
183
+
**Action:**
184
+
- Read openapi3 and json-schema emitters for context passing
185
+
- Look for how they track "current scope" for reference generation
186
+
- Understand if they use:
187
+
- Context objects passed through call stack?
188
+
- Builder pattern with scoped state?
189
+
- Other pattern?
190
+
191
+
**Deliverable:** Document the pattern used by reference emitters
192
+
193
+
---
194
+
195
+
### 3.2 Design context-based reference system
196
+
197
+
**Current problem:**
198
+
```typescript
199
+
private currentLexiconId: string | null = null; // Set/unset during traversal
200
+
201
+
// In processNamespace:
202
+
this.currentLexiconId = lexiconId;
203
+
// ... do work ...
204
+
this.currentLexiconId = null;
205
+
206
+
// In getModelReference:
207
+
if (this.currentLexiconId) {
208
+
// Use for local vs global ref decision
209
+
}
210
+
```
211
+
212
+
**Issue:** Reference generation depends on traversal order and state timing
213
+
214
+
**Proposed solution:**
215
+
```typescript
216
+
// Pass lexicon context explicitly
217
+
private typeToLexiconDefinition(
218
+
type: Type,
219
+
prop?: ModelProperty,
220
+
isDefining?: boolean,
221
+
context?: { currentLexiconId: string } // NEW
222
+
): LexiconDefinition | null {
223
+
// Use context.currentLexiconId instead of this.currentLexiconId
224
+
}
225
+
```
226
+
227
+
**Alternative:** Create a builder class per lexicon
228
+
```typescript
229
+
class LexiconBuilder {
230
+
constructor(private lexiconId: string, private program: Program) {}
231
+
232
+
addModel(model: Model) { /* uses this.lexiconId for context */ }
233
+
typeToDefinition(type: Type) { /* knows its lexicon scope */ }
234
+
build(): LexiconDocument { /* returns final document */ }
235
+
}
236
+
```
237
+
238
+
**Action:**
239
+
- Choose pattern based on TypeSpec conventions research
240
+
- **Test checkpoint:** `npm test` after refactoring each method
241
+
242
+
**Risk:** High - touches reference generation throughout
243
+
**Lines affected:** 56-57, 108, 139, 207, 227, 255, 265, 307, and all reference generation methods
244
+
245
+
---
246
+
247
+
## Phase 4: Edge Cases & Validation (Lower Priority)
248
+
249
+
### 4.1 Add validation for unsupported namespace patterns
250
+
251
+
**Current:** Heuristics try to guess intent (lines 77-315)
252
+
253
+
**Action:**
254
+
- Document supported patterns explicitly
255
+
- Add early validation:
256
+
```typescript
257
+
// After determining what namespace contains
258
+
if (hasModels && hasOperations) {
259
+
throw new Error(`Namespace ${fullName} contains both models and operations - unclear intent. Put operations in separate namespace or models in .defs namespace.`);
260
+
}
261
+
```
262
+
263
+
**Test checkpoint:** May fail on edge cases - decide whether to support or reject
264
+
265
+
---
266
+
267
+
### 4.2 Template argument extraction cleanup
268
+
269
+
**Current (lines 1012-1042):** Handles multiple representations inconsistently
270
+
271
+
**Action:**
272
+
- Research correct TypeSpec API for template value extraction
273
+
- Use standard serialization (see `serializeValueAsJson` in json-schema emitter)
274
+
- Simplify to canonical approach
275
+
276
+
**Risk:** Medium - but after mutable state is fixed
277
+
**Lines affected:** 1012-1042
278
+
279
+
---
280
+
281
+
## Phase 5: Structural Improvements (Optional)
282
+
283
+
### 5.1 Consider decomposing typeToLexiconDefinition
284
+
285
+
**Only if:** Pattern emerges naturally from other refactorings
286
+
287
+
**Current:** 400-line switch statement
288
+
289
+
**Possible structure:**
290
+
```typescript
291
+
private typeToLexiconDefinition(type: Type, prop?: ModelProperty, context?: Context) {
292
+
switch (type.kind) {
293
+
case "Model":
294
+
return this.modelToDefinition(type as Model, prop, context);
295
+
case "Union":
296
+
return this.unionToDefinition(type as Union, prop, context);
297
+
// etc
298
+
}
299
+
}
300
+
```
301
+
302
+
**Don't force it** - only if it improves clarity
303
+
304
+
---
305
+
306
+
## Execution Strategy
307
+
308
+
### Iteration Pattern:
309
+
1. Make small, focused change
310
+
2. Run `npm test`
311
+
3. If tests fail:
312
+
- Understand why
313
+
- Either fix the change or understand the case and throw explicitly
314
+
4. Commit mentally (or actually) before next change
315
+
5. If stuck, revert and try smaller change
316
+
317
+
### Change Order:
318
+
1. **Phase 1.1** (array helper) - safest, establishes pattern
319
+
2. **Phase 1.2** (diagnostic types) - cleanup, low risk
320
+
3. **Phase 1.3** (blob detection) - need to understand the shape
321
+
4. **Phase 2.1** (union audit) - research and planning
322
+
5. **Phase 2.2** (union implementation) - may expose edge cases
323
+
6. **Phase 3.1-3.2** (mutable state) - highest value, highest risk
324
+
7. **Phase 4+** - as needed based on findings
325
+
326
+
### Recovery Strategy:
327
+
- If a phase gets too complex, STOP
328
+
- Document what was learned
329
+
- Try smaller scope
330
+
- Ask for guidance if unclear
331
+
332
+
---
333
+
334
+
## Success Criteria
335
+
336
+
✅ **Must have:**
337
+
- All tests pass
338
+
- No `currentLexiconId` mutable state
339
+
- Explicit throws on unsupported patterns (no silent failures)
340
+
- Union handling matches lexicon spec
341
+
342
+
✅ **Nice to have:**
343
+
- Reduced `as any` usage
344
+
- Use TypeSpec helpers where appropriate
345
+
- Clearer structure (without forcing it)
346
+
347
+
✅ **Avoid:**
348
+
- Breaking existing functionality
349
+
- Over-engineering "nice" code
350
+
- Getting stuck in large refactorings
351
+
352
+
---
353
+
354
+
## Next Steps
355
+
356
+
1. Read this plan
357
+
2. Start with Phase 1.1 (array helper replacement)
358
+
3. Work incrementally
359
+
4. Report findings and blockers
360
+
5. Adjust plan as understanding improves
+3
-17
packages/emitter/src/emitter.ts
+3
-17
packages/emitter/src/emitter.ts
···
15
15
getMaxValue,
16
16
getMaxItems,
17
17
getMinItems,
18
+
isArrayModelType,
18
19
} from "@typespec/compiler";
19
20
import { join, dirname } from "path";
20
21
import type {
···
352
353
353
354
// Check if this model is actually an array type (via `is` declaration)
354
355
// e.g., `model Preferences is SomeUnion[]`
355
-
if (this.isArrayType(model)) {
356
+
if (isArrayModelType(this.program, model)) {
356
357
const arrayDef = this.modelToLexiconArray(model);
357
358
if (arrayDef) {
358
359
const description = getDoc(this.program, model);
···
1102
1103
}
1103
1104
1104
1105
// Check for anonymous array types (inline arrays like `SomeModel[]` in property)
1105
-
if (this.isArrayType(type as Model)) {
1106
+
if (isArrayModelType(this.program, type as Model)) {
1106
1107
const array = this.modelToLexiconArray(type as Model, prop);
1107
1108
if (array && prop) {
1108
1109
const propDesc = getDoc(this.program, prop);
···
1451
1452
}
1452
1453
1453
1454
return primitive;
1454
-
}
1455
-
1456
-
private isArrayType(model: Model): boolean {
1457
-
// Direct check: is this the TypeSpec.Array model?
1458
-
if (model.name === "Array" && model.namespace?.name === "TypeSpec") {
1459
-
return true;
1460
-
}
1461
-
1462
-
// Follow the sourceModel chain (for `is` declarations)
1463
-
// e.g., `model Preferences is SomeUnion[]` -> sourceModel is Array
1464
-
if (model.sourceModel) {
1465
-
return this.isArrayType(model.sourceModel);
1466
-
}
1467
-
1468
-
return false;
1469
1455
}
1470
1456
1471
1457
private getModelReference(model: Model): string | null {