An experimental TypeSpec syntax for Lexicon
ok
-360
-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