-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: re-organize handlers #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughRefactors TypeBox handler architecture: introduces base classes (collection/object-like/type-reference), adds new handlers (collection, object, reference, operators), updates existing handlers to strict Node typing, centralizes orchestration in TypeBoxTypeHandlers with new dispatch/caching, adds a template-literal processor, updates parsers to fallback to Any, and adjusts getTypeBoxType behavior. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant G as getTypeBoxType
participant H as TypeBoxTypeHandlers
participant K as Kind Map
participant R as TypeRef Map
participant F as Fallbacks
participant X as Specific Handler
participant U as makeTypeCall
C->>G: getTypeBoxType(node?)
alt node is undefined
G-->>C: U('Any')
else
G->>H: getHandler(node)
alt Node.isTypeReference
H->>R: lookup by identifier text
R-->>H: handler or undefined
else
H->>K: lookup by SyntaxKind
K-->>H: handler or undefined
end
alt found handler
H-->>G: X
else
H->>F: iterate canHandle(node)
F-->>H: first match or none
H-->>G: handler or throw
end
G->>X: handle(node)
X->>U: construct TypeBox call(s)
U-->>X: ts.Expression
X-->>G: ts.Expression
G-->>C: ts.Expression
end
sequenceDiagram
autonumber
participant PF as parse-function-declarations
participant PT as parse-type-aliases
participant G as getTypeBoxType
participant U as makeTypeCall
PF->>PF: param typeNode?
alt missing
PF->>U: Any
else
PF->>G: getTypeBoxType(typeNode)
end
PF->>PF: return typeNode?
alt missing
PF->>U: Any
else
PF->>G: getTypeBoxType(typeNode)
end
PT->>PT: alias typeNode?
alt missing
PT->>U: Any
else
PT->>G: getTypeBoxType(typeNode)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/handlers/typebox/reference/pick-type-handler.ts (1)
20-48: Bug: Type.Pick second argument must be an array of property keys, not Type.Literal/Type.UnionTypeBox expects the keys parameter for Type.Pick to be an array of strings (property keys), e.g., Type.Pick(T, ['type', 'x']). Constructing Type.Literal or Type.Union here is incorrect and will produce invalid output. Replace the schema construction with a simple string array expression derived from the literal union. (github.com)
Apply this diff to build a string array for keys and handle fallbacks cleanly:
- let pickKeys: string[] = [] - if (Node.isUnionTypeNode(keysType)) { - pickKeys = keysType.getTypeNodes().map((unionType) => { - if (Node.isLiteralTypeNode(unionType)) { - const literalExpression = unionType.getLiteral() - if (Node.isStringLiteral(literalExpression)) { - return literalExpression.getLiteralText() - } - } - return '' // Should not happen if keys are string literals - }) - } else if (Node.isLiteralTypeNode(keysType)) { - const literalExpression = keysType.getLiteral() - if (Node.isStringLiteral(literalExpression)) { - pickKeys = [literalExpression.getLiteralText()] - } - } - - let typeboxKeys: ts.Expression - if (pickKeys.length === 1) { - typeboxKeys = makeTypeCall('Literal', [ts.factory.createStringLiteral(pickKeys[0]!)]) - } else { - typeboxKeys = makeTypeCall('Union', [ - ts.factory.createArrayLiteralExpression( - pickKeys.map((k) => makeTypeCall('Literal', [ts.factory.createStringLiteral(k)])), - false, - ), - ]) - } + const pickKeys: string[] = [] + if (Node.isUnionTypeNode(keysType)) { + for (const unionType of keysType.getTypeNodes()) { + if (Node.isLiteralTypeNode(unionType)) { + const lit = unionType.getLiteral() + if (Node.isStringLiteral(lit)) { + pickKeys.push(lit.getLiteralText()) + } + } + } + } else if (Node.isLiteralTypeNode(keysType)) { + const lit = keysType.getLiteral() + if (Node.isStringLiteral(lit)) { + pickKeys.push(lit.getLiteralText()) + } + } + + // Build a plain string array for keys (matches Type.Pick API) + const keysArray = ts.factory.createArrayLiteralExpression( + pickKeys.map((k) => ts.factory.createStringLiteral(k)), + false, + )And finalize the call with the array:
- return makeTypeCall('Pick', [typeboxObjectType, typeboxKeys]) + return makeTypeCall('Pick', [typeboxObjectType, keysArray])src/handlers/typebox/typebox-type-handlers.ts (1)
94-135: Critical:getHandlernow throws, breaking fallback intypebox-call.tsThe change in
getHandlerfrom returningundefinedto throwing an error breaks existing calls that rely on a falsy return to drive fallback logic. In particular:
- In src/utils/typebox-call.ts at line 18:
Previously, missing handlers yieldedconst handler = handlers.getHandler(node) return handler ? handler.handle(node) : makeTypeCall('Any')undefined, triggering themakeTypeCall('Any')fallback. Now,getHandler(node)will throw instead of returningundefined, so the fallback branch will never be reached—and the exception propagates unhandled.You must restore or account for the previous behavior:
- Either wrap the call in a
try/catchand fallback tomakeTypeCall('Any')on error:- const handler = handlers.getHandler(node) - return handler ? handler.handle(node) : makeTypeCall('Any') + let handler: BaseTypeHandler | undefined + try { + handler = handlers.getHandler(node) + } catch { + return makeTypeCall('Any') + } + return handler.handle(node)- Or revert
getHandlerto returnundefinedfor unhandled nodes and only throw for truly unexpected cases, if you want to preserve the simpler call sites.[src/utils/typebox-call.ts:18]
src/handlers/typebox/reference/omit-type-handler.ts (1)
20-31: Avoid emitting bogus '' key and guard against Type.Union([]); add numeric-key support + robust fallback.
- Returning '' for non-literal branches silently generates an empty-string key, which is incorrect and can corrupt the resulting schema.
- If no valid literal keys are derived, the code currently builds Type.Union([]), which is invalid.
- Numeric property keys (valid PropertyKey) aren’t supported; we should map numeric literals to Type.Literal as well.
- For aliases, keyof, template-literal keys, or any non-literal cases, delegate to getTypeBoxType(keysType) instead of guessing.
Apply this refactor (build key expressions directly and fall back when empty):
- let omitKeys: string[] = [] - if (Node.isUnionTypeNode(keysType)) { - omitKeys = keysType.getTypeNodes().map((unionType) => { - if (Node.isLiteralTypeNode(unionType)) { - const literalExpression = unionType.getLiteral() - if (Node.isStringLiteral(literalExpression)) { - return literalExpression.getLiteralText() - } - } - return '' // Should not happen if keys are string literals - }) - } else if (Node.isLiteralTypeNode(keysType)) { - const literalExpression = keysType.getLiteral() - if (Node.isStringLiteral(literalExpression)) { - omitKeys = [literalExpression.getLiteralText()] - } - } - - let typeboxKeys: ts.Expression - if (omitKeys.length === 1) { - typeboxKeys = makeTypeCall('Literal', [ts.factory.createStringLiteral(omitKeys[0]!)]) - } else { - typeboxKeys = makeTypeCall('Union', [ - ts.factory.createArrayLiteralExpression( - omitKeys.map((k) => makeTypeCall('Literal', [ts.factory.createStringLiteral(k)])), - true, - ), - ]) - } + let keyExprs: ts.Expression[] = [] + + const pushLiteral = (lit: Node) => { + if (Node.isStringLiteral(lit)) { + keyExprs.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(lit.getLiteralText())])) + return true + } + if (Node.isNumericLiteral(lit)) { + keyExprs.push(makeTypeCall('Literal', [ts.factory.createNumericLiteral(lit.getText())])) + return true + } + return false + } + + if (Node.isUnionTypeNode(keysType)) { + for (const ut of keysType.getTypeNodes()) { + if (Node.isLiteralTypeNode(ut)) { + pushLiteral(ut.getLiteral()) + } + } + } else if (Node.isLiteralTypeNode(keysType)) { + pushLiteral(keysType.getLiteral()) + } + + let typeboxKeys: ts.Expression + if (keyExprs.length === 1) { + typeboxKeys = keyExprs[0] + } else if (keyExprs.length > 1) { + typeboxKeys = makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(keyExprs, true)]) + } else { + // Fallback: delegate complex/non-literal cases (aliases, keyof, template literals, etc.) + typeboxKeys = getTypeBoxType(keysType) + }Also applies to: 38-48
🧹 Nitpick comments (33)
src/handlers/typebox/template-literal-type-processor.ts (3)
11-24: Broaden top-level kind coverage (boolean) and clarify fallback semantics.
- Consider handling
ts.SyntaxKind.BooleanKeywordso`${boolean}`narrows to the two literal possibilities instead of a genericType.String. If you prefer to keep a superset, document the intended loss of precision.- Fallback to
Type.Stringis fine for permissive behavior; if you intend resilient codegen, keep this, otherwise considerType.Anyto match other fallbacks in the codebase.Apply this minimal change to cover boolean at the top level:
static processType(node: ts.TypeNode): ts.Expression { switch (node.kind) { case ts.SyntaxKind.StringKeyword: return makeTypeCall('String') case ts.SyntaxKind.NumberKeyword: return makeTypeCall('Number') + case ts.SyntaxKind.BooleanKeyword: + // `${boolean}` => "true" | "false" + return makeTypeCall('Union', [ + ts.factory.createArrayLiteralExpression([ + makeTypeCall('Literal', [ts.factory.createStringLiteral('true')]), + makeTypeCall('Literal', [ts.factory.createStringLiteral('false')]), + ]), + ]) case ts.SyntaxKind.LiteralType: return this.processLiteralType(node as ts.LiteralTypeNode) case ts.SyntaxKind.UnionType: return this.processUnionType(node as ts.UnionTypeNode) default: return makeTypeCall('String') } }
29-40: Handle boolean literal types; avoid losing precision for${true}/${false}.Currently, boolean literals fall through to
Type.String. Add explicit handling fortrue/falseliterals.private static processLiteralType(literalType: ts.LiteralTypeNode): ts.Expression { if (ts.isStringLiteral(literalType.literal)) { return makeTypeCall('Literal', [ts.factory.createStringLiteral(literalType.literal.text)]) } if (ts.isNumericLiteral(literalType.literal)) { return makeTypeCall('Literal', [ts.factory.createNumericLiteral(literalType.literal.text)]) } - // Fallback for other literals (boolean, etc.) - return makeTypeCall('String') + // Boolean literal kinds + if (literalType.literal.kind === ts.SyntaxKind.TrueKeyword) { + return makeTypeCall('Literal', [ts.factory.createTrue()]) + } + if (literalType.literal.kind === ts.SyntaxKind.FalseKeyword) { + return makeTypeCall('Literal', [ts.factory.createFalse()]) + } + + // Fallback for other literals + return makeTypeCall('String') }
45-57: ReuseprocessLiteralTypeand cover numeric/boolean literals inside unions.Only string literals are recognized in unions; numeric and boolean literals degrade to
Type.String. ReuseprocessLiteralTypeto keep precision and reduce duplication.private static processUnionType(unionType: ts.UnionTypeNode): ts.Expression { - const unionParts = unionType.types.map((t) => { - if (t.kind === ts.SyntaxKind.LiteralType) { - const literalType = t as ts.LiteralTypeNode - if (ts.isStringLiteral(literalType.literal)) { - return makeTypeCall('Literal', [ts.factory.createStringLiteral(literalType.literal.text)]) - } - } - return makeTypeCall('String') // fallback - }) + const unionParts: ts.Expression[] = [] + for (const t of unionType.types) { + if (t.kind === ts.SyntaxKind.LiteralType) { + unionParts.push(this.processLiteralType(t as ts.LiteralTypeNode)) + continue + } + // Non-literal union members default to string to keep template-literal compatibility + unionParts.push(makeTypeCall('String')) + } return makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(unionParts)]) }ARCHITECTURE.md (1)
217-220: Update docs to reflect actual fallback and error behaviorsThe current implementation of
getTypeBoxTypeonly falls back toType.Anywhen it’s called with an undefined node (handled by callers), but will throw an error if no handler matches a given AST node type. The docs should accurately describe both behaviors:• When a node is
undefined, codegen will produceType.Any.
• When a node is defined but lacks a matching handler,getTypeBoxTypethrows to surface the unhandled AST type.Please update ARCHITECTURE.md around lines 217–220 as follows:
- Uses optimized lookup mechanisms for performance with O(1) syntax kind-based lookups and type reference name mappings. + Uses optimized lookup mechanisms for performance with O(1) syntax kind-based lookups and type reference name mappings. + When the input node is `undefined`, `getTypeBoxType` gracefully falls back to `Type.Any`. If a defined node has no registered handler, `getTypeBoxType` throws an error (`No handler found for type: ...`) to signal an unhandled AST node type.src/parsers/parse-function-declarations.ts (1)
35-52: Treat parameters with default values as optional.TS treats parameters with initializers as optional even without a
?. Wrap such params inType.Optional(...)for parity with the type system.- // Check if parameter is optional - if (param.hasQuestionToken()) { + // Optional if it has a question token or a default initializer + if (param.hasQuestionToken() || param.hasInitializer()) { return ts.factory.createCallExpression( ts.factory.createPropertyAccessExpression( ts.factory.createIdentifier('Type'), ts.factory.createIdentifier('Optional'), ), undefined, [paramType], ) }Additional note: consider explicit handling for rest parameters (
param.isRestParameter()), e.g., wrapping element type inType.Array(...)if that matches yourType.Functioncontract.src/handlers/typebox/typeof-type-handler.ts (2)
11-13: Fix misleading comment abouttypeof.
The comment conflates TSTypeQuerywith the runtimetypeofoperator. Please correct or remove to avoid propagating a wrong mental model.
1-3: Optional: implement proper resolution fortypeof X.
If you want first-class support now, resolve the symbol for theexprNameand derive the schema from the declaration’s type node; otherwise keep the safeAnyfallback above.High-level approach:
- Resolve
exprNamesymbol → declaration(s)- Prefer a declaration’s explicit type node if present; otherwise infer from initializer
- Delegate to
getTypeBoxType(typeNode); fallback toAnyif uncertainThis requires importing
getTypeBoxTypehere and using ts-morph’s symbol/declaration APIs. Happy to draft a full implementation if you confirm the desired edge-case behavior (e.g., classes, namespaces, const enums).src/handlers/typebox/reference/required-type-handler.ts (2)
7-14: Consider supporting selective keys overload forRequired<T, K>.
TypeBox supports making only specific properties required. If that’s in scope, extend the handler to accept an optional second argument of keys and thread it through toType.Required(schema, keys).Example diff sketch:
- protected readonly expectedArgumentCount = 1 + protected readonly expectedArgumentCount = 1 // keep validation in base strict + // Optionally: relax via a new hook to allow 1 or 2 args if your base supports it - const [innerType] = this.extractTypeArguments(typeRef) - return makeTypeCall('Required', [getTypeBoxType(innerType)]) + const args = this.extractTypeArguments(typeRef) + const schemaArg = getTypeBoxType(args[0]) + const keyArg = args[1] ? getTypeBoxType(args[1]) : undefined + return makeTypeCall('Required', keyArg ? [schemaArg, keyArg] : [schemaArg])
10-15: Guardrail: object-like only (optional).
IfinnerTypecan be non-object, consider a lightweight runtime check (or let TypeBox error at runtime) depending on your error-handling philosophy. Marking this as optional for now.src/handlers/typebox/readonly-type-handler.ts (1)
15-16: Note on scope (optional).
This handler coversreadonly T(type operator). Ensure object-literal property-levelreadonlyis handled elsewhere in the object-like handlers to preserve modifiers during codegen.src/handlers/typebox/collection/union-type-handler.ts (1)
9-11: Optional: flatten nested unions and dedupe to avoid nestedType.UnionnodesIf
processTypeCollectiondoesn’t already flatten/dedupe, consider doing it here to keep output minimal and stable.Example approach (if not handled in base):
- handle(node: UnionTypeNode): ts.Expression { - return this.processTypeCollection(node.getTypeNodes(), 'Union') - } + handle(node: UnionTypeNode): ts.Expression { + const items = node.getTypeNodes() + .flatMap((n) => (Node.isUnionTypeNode(n) ? n.getTypeNodes() : [n])) + return this.processTypeCollection(items, 'Union') + }src/handlers/typebox/keyof-type-handler.ts (1)
11-15: Optional: add defensive fallback ifgetTypeNode()is ever undefinedLow risk, but a tiny guard avoids surprises if upstream changes introduce malformed nodes.
- const operandType = node.getTypeNode() - const typeboxOperand = getTypeBoxType(operandType) + const operandType = node.getTypeNode() + const typeboxOperand = operandType ? getTypeBoxType(operandType) : getTypeBoxType()src/handlers/typebox/type-reference-handler.ts (1)
10-10: Consider aligning the override signature with the base class for consistency.BaseTypeHandler.handle expects (node: Node). Overriding with (node: TypeReferenceNode) relies on method parameter bivariance. Keeping the same signature and asserting inside reduces surprise for maintainers.
src/handlers/typebox/literal-type-handler.ts (1)
20-22: Use unescaped string value to avoid double-escaping and quoting pitfalls.literal.getText().slice(1, -1) can mishandle escapes and quote styles. Prefer getLiteralText() behind a type guard.
Example refactor:
if (SyntaxKind.StringLiteral === literalKind) { const value = Node.isStringLiteral(literal) ? literal.getLiteralText() : literal.getText().slice(1, -1) return makeTypeCall('Literal', [ts.factory.createStringLiteral(value)]) }src/handlers/typebox/object/interface-type-handler.ts (1)
9-11: Consider supporting heritage clauses (extends) and index signatures.Interfaces commonly extend others and/or define index signatures. Ensure ObjectLikeBaseHandler aggregates inherited properties and handles string/number index signatures; otherwise fields from parents or indexers will be lost.
Would you like me to wire a follow-up handler in ObjectLikeBaseHandler to merge properties from node.getExtends() targets and convert index signatures to Type.Record/Type.Dict where applicable?
src/handlers/typebox/reference/record-type-handler.ts (1)
10-18: Validate key schema compatibility with Type.Record.Depending on your TypeBox version, Record may restrict key schemas (e.g., String, Number, TemplateLiteral, or unions of Literals). Passing Any/Unknown/complex unions could produce invalid or overly permissive schemas.
- Option: detect unsupported key schemas and fall back to Type.String() with a warning.
- Option: support key-of unions by transforming union-of-literals into a KeyOf/Record index strategy if present elsewhere in your handlers.
If helpful, I can add a guard like isSupportedKeySchema(keyType) in TypeReferenceBaseHandler and route to Any/String appropriately.
src/handlers/typebox/simple-type-handler.ts (3)
29-31: Guard lookup with hasOwnProperty for safer kind checksUsing the in operator works at runtime but also traverses the prototype chain. Prefer hasOwnProperty to make the intent explicit and avoid potential surprises if kindToTypeBox were ever created with a non-default prototype.
- canHandle(node: Node): boolean { - return node.getKind() in kindToTypeBox - } + canHandle(node: Node): boolean { + return Object.prototype.hasOwnProperty.call(kindToTypeBox, node.getKind()) + }
33-35: Defensive fallback in handle()canHandle() should prevent invalid calls, but adding a defensive fallback keeps the handler resilient to orchestration mistakes and makes failures easier to diagnose.
- handle(node: Node): ts.Expression { - return makeTypeCall(kindToTypeBox[node.getKind() as SimpleKinds]) - } + handle(node: Node): ts.Expression { + const method = kindToTypeBox[node.getKind() as SimpleKinds] + if (!method) { + // Defensive: should never happen if canHandle() is respected + return makeTypeCall('Any') + } + return makeTypeCall(method) + }
5-6: Possible dead constantTypeBoxType is declared but not used in this file. If it’s not referenced elsewhere, consider removing it to reduce noise.
src/handlers/typebox/type-operator-handler.ts (1)
11-15: Improve error text and consider a non-throw fallback in non-strict modeTwo points:
- node.getOperator() yields a SyntaxKind enum value; the message prints a number. Convert to a readable token.
- Depending on your desired strictness, you may want to return Type.Any instead of throwing to avoid aborting generation for rare operators. If you keep the throw (which is fine in strict mode), at least improve the message.
-import { Node, ts, TypeOperatorTypeNode } from 'ts-morph' +import { Node, ts, TypeOperatorTypeNode } from 'ts-morph' +import { makeTypeCall } from '@daxserver/validation-schema-codegen/utils/typebox-codegen-utils' @@ - handle(node: TypeOperatorTypeNode): ts.Expression { - // Fallback for any unhandled type operators - throw new Error(`Unhandled type operator: ${node.getOperator()}`) - // return makeTypeCall('Any') - } + handle(node: TypeOperatorTypeNode): ts.Expression { + const op = ts.SyntaxKind[node.getOperator()] ?? String(node.getOperator()) + // Strict default: surface clear error. If you prefer non-strict behavior, return Any instead. + // return makeTypeCall('Any') + throw new Error(`Unhandled type operator: ${op}`) + }src/handlers/typebox/reference/pick-type-handler.ts (1)
10-16: Consider stronger validation and error messaging for unsupported KIf K isn’t a (union of) string literals (e.g., it’s keyof T or another type), we currently return Any only when keysType is missing. You might want to either:
- throw a descriptive error for unsupported forms, or
- fall back to generating Pick(T, []) which yields an empty object, or
- attempt to resolve K via KeyOf handling before bailing.
Given the broader operator handlers, surfacing a clear error is likely preferable.
src/handlers/typebox/indexed-access-type-handler.ts (2)
29-64: Resolve typeof A[number] across files and inferred declarationsThe special-case currently searches only the same source file and only via explicit type annotations. This misses common cases:
- A is imported from another module.
- A is inferred from an initializer (e.g., const A = ['a', 'b'] as const) with no explicit type node.
Two incremental improvements:
- Use the type checker to resolve the identifier symbol and iterate declarations (not just the current source file).
- When no typeNode exists on a variable declaration, inspect the initializer for a const tuple and extract string literals.
Example approach (outside current ranges) to resolve declarations via symbol:
const symbol = exprName.getSymbol() for (const decl of symbol?.getDeclarations() ?? []) { const tn = Node.isVariableDeclaration(decl) ? (decl.getTypeNode() ?? decl.getInitializer()) : Node.isTypeAliasDeclaration(decl) ? decl.getTypeNode() : undefined const tupleUnion = this.extractTupleUnion(tn) if (tupleUnion) return tupleUnion }And extend extractTupleUnion to handle NamedTupleMember and array literal initializers (['a','b'] as const).
97-101: Optional: tighten Union array formattingts.factory.createArrayLiteralExpression(literalTypes) defaults multiLine to false in most printers, but if you want consistent one-liners, pass false explicitly as you do elsewhere for consistency.
- return makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literalTypes)]) + return makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literalTypes, false)])src/handlers/typebox/function-type-handler.ts (2)
11-26: Parameter handling: consider rest parameters and special casesCurrent logic handles optional parameters but not rest parameters (...args). TypeBox represents variadic functions via Type.Rest and a Tuple of parameter types. Consider detecting param.isRestParameter() and emitting Type.Function(Type.Rest(Type.Tuple([...normalParams, Type.Array(elemType)])), returnType) to preserve semantics. See docs for Function and Rest usage. (npm.youzanyun.com)
Sketch inside current structure:
const last = parameters.at(-1) if (last?.isRestParameter()) { const elemType = getTypeBoxType(last.getTypeNode()) // expects Array<T> / T[] const head = parameters.slice(0, -1).map(paramToTypeBox) // existing optional logic const tuple = makeTypeCall('Tuple', [ts.factory.createArrayLiteralExpression([...head, elemType], false)]) return makeTypeCall('Function', [makeTypeCall('Rest', [tuple]), returnTypeBox]) }
28-35: Return type fallbackgetTypeBoxType(returnType) will yield Any if the node is missing. FunctionTypeNode typically has an explicit return type, so returning Any is a silent degradation. If returnType is absent, consider defaulting to Type.Void for parity with typical TS inference of () => void or surfacing a diagnostic.
src/handlers/typebox/type-query-handler.ts (1)
3-3: Type import ordering.Consider grouping the TypeScript imports together for better organization.
import { BaseTypeHandler } from '@daxserver/validation-schema-codegen/handlers/typebox/base-type-handler' import { makeTypeCall } from '@daxserver/validation-schema-codegen/utils/typebox-codegen-utils' -import { Node, ts, TypeQueryNode } from 'ts-morph' +import { Node, TypeQueryNode, ts } from 'ts-morph'src/handlers/typebox/typebox-type-handlers.ts (2)
98-99: Consider performance implications of text-based caching.Using
node.getText()as part of the cache key could be inefficient for large type nodes. Consider using a more lightweight key generation strategy, such as combining node kind with a hash of the text or using node position information.- const nodeText = node.getText() - const cacheKey = `${nodeKind}-${nodeText}` + // Use position for more efficient caching + const pos = node.getPos() + const end = node.getEnd() + const cacheKey = `${nodeKind}-${pos}-${end}`
112-119: Potential redundant type checking.The code performs
Node.isTypeReference(node)check after already checkingnodeKind === SyntaxKind.TypeReference. These should be equivalent, making one of the checks redundant.- if (!handler && nodeKind === SyntaxKind.TypeReference && Node.isTypeReference(node)) { + if (!handler && Node.isTypeReference(node)) { const typeNameNode = node.getTypeName()src/handlers/typebox/reference/type-reference-base-handler.ts (2)
8-17: Consider using Node.isTypeReference for consistency.The
canHandlemethod performs manual kind checking and casting. Consider using the ts-morph type guard for better type safety.canHandle(node: Node): boolean { - if (node.getKind() !== SyntaxKind.TypeReference) { + if (!Node.isTypeReference(node)) { return false } - const typeRef = node as TypeReferenceNode - const typeName = typeRef.getTypeName().getText() + const typeName = node.getTypeName().getText() return this.supportedTypeNames.includes(typeName) }
19-25: Redundant validation in validateTypeReference.This method performs the same check as
canHandlebut throws an error. Since handlers are selected based oncanHandle, this validation might be redundant in normal flow.Consider using Node.isTypeReference type guard with assertion:
protected validateTypeReference(node: Node): TypeReferenceNode { - if (node.getKind() !== SyntaxKind.TypeReference) { - throw new Error(`Expected TypeReference node, got ${node.getKind()}`) - } - - return node as TypeReferenceNode + if (!Node.isTypeReference(node)) { + throw new Error(`Expected TypeReference node, got ${node.getKindName()}`) + } + return node }src/handlers/typebox/reference/omit-type-handler.ts (1)
20-31: Optional: de-duplicate keys to avoid redundant union members.If the source union contains duplicates, you’ll emit duplicate Literal members. Not functionally wrong, but easy to tidy while you’re iterating.
If you keep a string/number set while pushing literals, you can skip duplicates with negligible overhead.
src/handlers/typebox/object/object-like-base-handler.ts (2)
18-27: Optional: centralize “already optional” detection.The Optional detection logic appears in multiple handlers. Consider a small helper (e.g., isTypeCall(valueExpr, 'Optional')) to keep this consistent and testable.
29-33: Use TypeScript’s identifier validator instead of a regex.ts.isIdentifierText handles Unicode and edge cases better than the ASCII regex.
- const nameNode = /^[A-Za-z_$][A-Za-z0-9_$]*$/.test(propName) - ? ts.factory.createIdentifier(propName) - : ts.factory.createStringLiteral(propName) + const nameNode = ts.isIdentifierText(propName, ts.ScriptTarget.ESNext) + ? ts.factory.createIdentifier(propName) + : ts.factory.createStringLiteral(propName)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (40)
ARCHITECTURE.md(1 hunks)src/handlers/typebox/array-type-handler.ts(0 hunks)src/handlers/typebox/base-type-handler.ts(1 hunks)src/handlers/typebox/collection/array-type-handler.ts(1 hunks)src/handlers/typebox/collection/collection-base-handler.ts(1 hunks)src/handlers/typebox/collection/intersection-type-handler.ts(1 hunks)src/handlers/typebox/collection/tuple-type-handler.ts(1 hunks)src/handlers/typebox/collection/union-type-handler.ts(1 hunks)src/handlers/typebox/function-type-handler.ts(2 hunks)src/handlers/typebox/indexed-access-type-handler.ts(2 hunks)src/handlers/typebox/interface-type-handler.ts(0 hunks)src/handlers/typebox/intersection-type-handler.ts(0 hunks)src/handlers/typebox/keyof-type-handler.ts(1 hunks)src/handlers/typebox/literal-type-handler.ts(1 hunks)src/handlers/typebox/object/interface-type-handler.ts(1 hunks)src/handlers/typebox/object/object-like-base-handler.ts(2 hunks)src/handlers/typebox/object/object-type-handler.ts(1 hunks)src/handlers/typebox/partial-type-handler.ts(0 hunks)src/handlers/typebox/readonly-type-handler.ts(1 hunks)src/handlers/typebox/record-type-handler.ts(0 hunks)src/handlers/typebox/reference/omit-type-handler.ts(2 hunks)src/handlers/typebox/reference/partial-type-handler.ts(1 hunks)src/handlers/typebox/reference/pick-type-handler.ts(2 hunks)src/handlers/typebox/reference/record-type-handler.ts(1 hunks)src/handlers/typebox/reference/required-type-handler.ts(1 hunks)src/handlers/typebox/reference/type-reference-base-handler.ts(1 hunks)src/handlers/typebox/required-type-handler.ts(0 hunks)src/handlers/typebox/simple-type-handler.ts(1 hunks)src/handlers/typebox/template-literal-type-handler.ts(1 hunks)src/handlers/typebox/template-literal-type-processor.ts(1 hunks)src/handlers/typebox/tuple-type-handler.ts(0 hunks)src/handlers/typebox/type-operator-handler.ts(1 hunks)src/handlers/typebox/type-query-handler.ts(1 hunks)src/handlers/typebox/type-reference-handler.ts(1 hunks)src/handlers/typebox/typebox-type-handlers.ts(4 hunks)src/handlers/typebox/typeof-type-handler.ts(1 hunks)src/handlers/typebox/union-type-handler.ts(0 hunks)src/parsers/parse-function-declarations.ts(3 hunks)src/parsers/parse-type-aliases.ts(2 hunks)src/utils/typebox-call.ts(1 hunks)
💤 Files with no reviewable changes (8)
- src/handlers/typebox/tuple-type-handler.ts
- src/handlers/typebox/required-type-handler.ts
- src/handlers/typebox/record-type-handler.ts
- src/handlers/typebox/partial-type-handler.ts
- src/handlers/typebox/intersection-type-handler.ts
- src/handlers/typebox/array-type-handler.ts
- src/handlers/typebox/union-type-handler.ts
- src/handlers/typebox/interface-type-handler.ts
🧰 Additional context used
🧬 Code graph analysis (22)
src/handlers/typebox/collection/intersection-type-handler.ts (1)
src/handlers/typebox/intersection-type-handler.ts (1)
IntersectionTypeHandler(5-21)
src/parsers/parse-type-aliases.ts (2)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/collection/tuple-type-handler.ts (1)
src/handlers/typebox/tuple-type-handler.ts (1)
TupleTypeHandler(5-21)
src/handlers/typebox/typeof-type-handler.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/parsers/parse-function-declarations.ts (2)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/readonly-type-handler.ts (2)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/reference/partial-type-handler.ts (3)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)src/handlers/typebox/partial-type-handler.ts (1)
PartialTypeHandler(5-26)
src/handlers/typebox/simple-type-handler.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/reference/required-type-handler.ts (3)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)src/handlers/typebox/required-type-handler.ts (1)
RequiredTypeHandler(4-47)
src/utils/typebox-call.ts (2)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)src/handlers/typebox/typebox-type-handlers.ts (1)
TypeBoxTypeHandlers(25-139)
src/handlers/typebox/template-literal-type-handler.ts (2)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)src/handlers/typebox/template-literal-type-processor.ts (1)
TemplateLiteralTypeProcessor(7-58)
src/handlers/typebox/literal-type-handler.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/template-literal-type-processor.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/function-type-handler.ts (1)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)
src/handlers/typebox/collection/collection-base-handler.ts (2)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/reference/record-type-handler.ts (3)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)src/handlers/typebox/record-type-handler.ts (1)
RecordTypeHandler(5-27)
src/handlers/typebox/keyof-type-handler.ts (2)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/object/object-like-base-handler.ts (1)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)
src/handlers/typebox/reference/omit-type-handler.ts (2)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)
src/handlers/typebox/reference/pick-type-handler.ts (2)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)
src/handlers/typebox/indexed-access-type-handler.ts (1)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)
src/handlers/typebox/typebox-type-handlers.ts (16)
src/handlers/typebox/simple-type-handler.ts (1)
SimpleTypeHandler(28-36)src/handlers/typebox/literal-type-handler.ts (1)
LiteralTypeHandler(5-35)src/handlers/typebox/object/object-type-handler.ts (1)
ObjectTypeHandler(4-12)src/handlers/typebox/collection/array-type-handler.ts (1)
ArrayTypeHandler(4-12)src/handlers/typebox/collection/tuple-type-handler.ts (1)
TupleTypeHandler(4-12)src/handlers/typebox/collection/union-type-handler.ts (1)
UnionTypeHandler(4-12)src/handlers/typebox/collection/intersection-type-handler.ts (1)
IntersectionTypeHandler(4-12)src/handlers/typebox/reference/record-type-handler.ts (1)
RecordTypeHandler(6-19)src/handlers/typebox/reference/partial-type-handler.ts (1)
PartialTypeHandler(6-18)src/handlers/typebox/reference/pick-type-handler.ts (1)
PickTypeHandler(6-52)src/handlers/typebox/reference/omit-type-handler.ts (1)
OmitTypeHandler(6-52)src/handlers/typebox/reference/required-type-handler.ts (1)
RequiredTypeHandler(6-16)src/handlers/typebox/type-reference-handler.ts (1)
TypeReferenceHandler(5-20)src/handlers/typebox/keyof-type-handler.ts (1)
KeyOfTypeHandler(6-17)src/handlers/typebox/typeof-type-handler.ts (1)
TypeofTypeHandler(5-15)src/handlers/typebox/readonly-type-handler.ts (1)
ReadonlyTypeHandler(6-18)
🪛 LanguageTool
ARCHITECTURE.md
[grammar] ~205-~205: There might be a mistake here.
Context: ..., undefined, any, unknown, void. - : Handles TypeScript literal types (e.g....
(QB_NEW_EN)
[grammar] ~207-~207: There might be a mistake here.
Context: ..., optional parameters, and return types. - : Handles TypeScript template literal ty...
(QB_NEW_EN)
🔇 Additional comments (29)
src/parsers/parse-type-aliases.ts (1)
21-23: LGTM: sensibleAnyfallback for undefined alias node.The explicit fallback to
Type.Anykeeps codegen robust for malformed or partially parsed aliases and aligns with the newgetTypeBoxTypecontract.src/handlers/typebox/typeof-type-handler.ts (1)
5-8: canHandle logic looks correct for TypeQuery nodes.
Detecting TypeQuery viaNode.isTypeQuery(node)aligns with ts-morph semantics and the new dispatcher’s node-driven design.src/handlers/typebox/collection/intersection-type-handler.ts (2)
5-11: Clean delegation to collection base; API usage looks good.
processTypeCollection(node.getTypeNodes(), 'Intersect')is consistent with TypeBox’sType.Intersect([])and centralizes validation/flattening in the base.
1-12: No legacy IntersectionTypeHandler remains
Verification shows only the collection-based handler (src/handlers/typebox/collection/intersection-type-handler.ts) exists and is instantiated intypebox-type-handlers.ts(line 38). The old non-collection file is absent. No further action required.src/handlers/typebox/reference/required-type-handler.ts (1)
6-15: Solid implementation for Required.
- Correctly validates the type reference and argument count via the base.
- Generates
Type.Required(<inner>)usinggetTypeBoxTypefor the operand.src/handlers/typebox/collection/array-type-handler.ts (2)
5-11: Straightforward and consistent Array handling.
Delegating withprocessSingleType(elementType, 'Array')matches the collection base pattern and keeps logic uniform across handlers.
5-11: Readonly arrays are already correctly routed through both handlers.I verified in
TypeBoxTypeHandlersthat:
SyntaxKind.TypeOperatornodes (e.g.readonly T[]) don’t match anysyntaxKindHandlersentry.- They fall through to the
fallbackHandlersin order, andReadonlyTypeHandler(checking forSyntaxKind.ReadonlyKeyword) is the first to match.- Inside its
handle, it callsgetTypeBoxTypeon the operand (theArrayTypeNode), which dispatches toArrayTypeHandler.- The result is therefore
Type.Readonly(Type.Array(...))as intended.No additional handlers or dispatch‐precedence adjustments are needed here.
src/handlers/typebox/readonly-type-handler.ts (2)
7-9: Good detection ofreadonlytype operator.
Node.isTypeOperatorTypeNode(node) && node.getOperator() === SyntaxKind.ReadonlyKeywordis precise and avoids colliding with property-level readonly modifiers.
11-17: Correct transformation to TypeBoxReadonly.
Resolving the operand viagetTypeBoxTypeand wrapping withType.Readonly(...)matches TypeBox utility semantics.src/handlers/typebox/collection/tuple-type-handler.ts (2)
5-7: canHandle looks goodAccurately narrows to TupleTypeNode.
1-2: Path alias mapping is correctly configuredThe
tsconfig.jsonalready defines the necessary path mapping for@daxserver/validation-schema-codegen/*to./src/*, so the import will resolve as expected:
- tsconfig.json (lines 26–29):
"baseUrl": ".", "paths": { "@daxserver/validation-schema-codegen/*": ["./src/*"], "@test-fixtures/*": ["./tests/*"] }No further changes are required.
src/handlers/typebox/collection/union-type-handler.ts (1)
5-7: canHandle is correctClean guard for UnionTypeNode.
src/handlers/typebox/object/object-type-handler.ts (1)
5-7: canHandle matches TypeLiteral usageLooks good.
src/handlers/typebox/reference/partial-type-handler.ts (2)
6-8: Good use of TypeReference base: clear contract for name and arity
supportedTypeNamesandexpectedArgumentCountmake the handler precise and maintainable.
10-17: Add coverage and guard for non-object operands toPartialWe currently only exercise
Partial<{…}>in our tests (see lines 51 and 187 intests/handlers/typebox/utility-types.test.ts), but TypeBox’sType.Partialis intended only for object schemas. To prevent unexpected behavior with primitives or arrays, please:
Add tests for non-object
Partialusages intests/handlers/typebox/utility-types.test.ts
•type T1 = Partial<string>;
•type T2 = Partial<number[]>;
Verify that the generated schema falls back toType.Any()(or that a diagnostics warning is emitted).Optionally guard in
partial-type-handler.tsbefore callingType.Partial:handle(node: Node): ts.Expression { const typeRef = this.validateTypeReference(node) const [innerType] = this.extractTypeArguments(typeRef) const typeBoxType = getTypeBoxType(innerType)
- // Guard against non-object schemas:
- if (!ts.isTypeLiteralNode(innerType) && !ts.isInterfaceDeclaration(innerType)) {
return makeTypeCall('Any')- }
return makeTypeCall('Partial', [typeBoxType])
}- **Document the chosen behavior** (fallback vs. warning) so it’s clear to future maintainers. </blockquote></details> <details> <summary>src/handlers/typebox/type-reference-handler.ts (1)</summary><blockquote> `6-8`: **Good, precise gating for TypeReference nodes.** The input guard is correct and keeps handle() focused on TypeReference nodes. </blockquote></details> <details> <summary>src/handlers/typebox/base-type-handler.ts (1)</summary><blockquote> `4-5`: **API simplification looks good.** The abstract surface is now minimal and clear; removing the injected callback reduces coupling. </blockquote></details> <details> <summary>src/handlers/typebox/object/interface-type-handler.ts (1)</summary><blockquote> `5-7`: **Correct applicability check for interfaces.** Gating on InterfaceDeclaration is appropriate. </blockquote></details> <details> <summary>src/handlers/typebox/reference/record-type-handler.ts (1)</summary><blockquote> `7-9`: **Good specialization: explicit support and arity guarded.** Declaring supported type names and expected arity tightens validation. </blockquote></details> <details> <summary>src/handlers/typebox/type-operator-handler.ts (1)</summary><blockquote> `5-9`: **No action needed—fallback handler isn’t registered** I checked the registration in `src/handlers/typebox/typebox-type-handlers.ts` and there’s no instantiation of `TypeOperatorHandler`, so this fallback never actually runs and cannot shadow any specific operator handlers. You can safely ignore this precedence concern. > Likely an incorrect or invalid review comment. </blockquote></details> <details> <summary>src/handlers/typebox/type-query-handler.ts (1)</summary><blockquote> `10-28`: **No changes required for TypeQueryHandler.** The current implementation intentionally emits plain identifiers for `typeof` expressions—leveraging `Static<typeof …>` for runtime type capture—which aligns with existing tests in `advanced-types.test.ts`. The dot-to-underscore conversion for qualified names is also explicitly validated there. All behavior matches the design, so we can resolve this review comment. </blockquote></details> <details> <summary>src/handlers/typebox/collection/collection-base-handler.ts (1)</summary><blockquote> `1-23`: **LGTM! Well-structured base class for collection handlers.** The `CollectionBaseHandler` provides a clean abstraction for collection-type handlers with appropriate helper methods. The separation of concerns is good - single type processing, multiple type processing, and validation are clearly defined. </blockquote></details> <details> <summary>src/handlers/typebox/template-literal-type-handler.ts (2)</summary><blockquote> `24-40`: **Good separation of concerns with TemplateLiteralTypeProcessor.** The delegation to `TemplateLiteralTypeProcessor` for processing template literal types provides good separation of concerns and centralizes the complex type mapping logic. --- `15-20`: **No safer ts-morph API exists for extracting template literal parts; direct compilerNode.access is necessary.** A review of the ts-morph docs shows that the `TemplateHead` and `TemplateSpan` wrapper classes do not provide methods such as `getLiteralValue()` or `getLiteralText()` for obtaining the raw text of their literal portions—only `getParent`/`getParentOrThrow` are available on those types ([jsdocs.io](https://www.jsdocs.io/package/ts-morph?utm_source=chatgpt.com)). A codebase scan likewise revealed no alternative helper methods for this use case ([ts-morph.com](https://ts-morph.com/details/literals?utm_source=chatgpt.com)). Directly casting to the compiler node and reading the `.text` property remains the only way to retrieve the literal text in this context. You can safely ignore the suggestion to replace these accesses with a higher-level API. > Likely an incorrect or invalid review comment. </blockquote></details> <details> <summary>src/handlers/typebox/reference/omit-type-handler.ts (3)</summary><blockquote> `6-13`: **Good migration to TypeReferenceBaseHandler and declarative config.** Extending TypeReferenceBaseHandler with supportedTypeNames=['Omit'] and expectedArgumentCount=2 is clean and consistent with the new architecture. The handle(Node) entrypoint is aligned with the centralized dispatcher. --- `14-16`: **Confirm fallback semantics for missing keys.** When keysType is absent, this returns Type.Any. Is that the intended global policy for malformed or partial type references in this refactor? An alternative would be to return Omit(objectType, Type.Never) (no-op) or just the objectType itself. Please confirm consistency with other reference handlers. --- `18-18`: **LGTM on delegating object type to the central pipeline.** Using getTypeBoxType(objectType) aligns with the new orchestrator and gracefully falls back to Any when objectType is missing. </blockquote></details> <details> <summary>src/handlers/typebox/object/object-like-base-handler.ts (2)</summary><blockquote> `6-9`: **Solid abstraction: property processing and object creation split is clean.** The separation of concerns via processProperties(...) and createObjectType(...) is a good base for specialized object-like handlers. --- `39-44`: **LGTM on object construction.** Returning makeTypeCall('Object', [objectLiteral]) keeps generation consistent with other handlers. </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| This directory contains a collection of specialized handler modules, each responsible for converting a specific type of TypeScript AST node into its corresponding TypeBox schema. The handlers follow a hierarchical architecture with specialized base classes to reduce code duplication and ensure consistent behavior. | ||
|
|
||
| #### Base Handler Classes | ||
|
|
||
| - <mcfile name="base-type-handler.ts" path="src/handlers/typebox/base-type-handler.ts"></mcfile>: The root abstract base class that defines the common interface for all type handlers. Provides the `canHandle` and `handle` methods, along with utility functions like `makeTypeCall` for creating TypeBox expressions. | ||
| - <mcfile name="type-reference-base-handler.ts" path="src/handlers/typebox/reference/type-reference-base-handler.ts"></mcfile>: Specialized base class for utility type handlers that work with TypeScript type references. Provides `validateTypeReference` and `extractTypeArguments` methods for consistent handling of generic utility types like `Partial<T>`, `Pick<T, K>`, etc. | ||
| - <mcfile name="object-like-base-handler.ts" path="src/handlers/typebox/object/object-like-base-handler.ts"></mcfile>: Base class for handlers that process object-like structures (objects and interfaces). Provides `processProperties`, `extractProperties`, and `createObjectType` methods for consistent property handling and TypeBox object creation. | ||
| - <mcfile name="collection-base-handler.ts" path="src/handlers/typebox/collection/collection-base-handler.ts"></mcfile>: Base class for handlers that work with collections of types (arrays, tuples, unions, intersections). Provides `processTypeCollection`, `processSingleType`, and `validateNonEmptyCollection` methods for consistent type collection processing. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify BaseTypeHandler description and reference to makeTypeCall.
makeTypeCall lives in src/utils/typebox-codegen-utils.ts, not in the base handler. Reword to avoid implying it’s provided by the base class. Also consider listing the new processor you added in this PR so readers can find it.
- - <mcfile name="base-type-handler.ts" path="src/handlers/typebox/base-type-handler.ts"></mcfile>: The root abstract base class that defines the common interface for all type handlers. Provides the `canHandle` and `handle` methods, along with utility functions like `makeTypeCall` for creating TypeBox expressions.
+ - <mcfile name="base-type-handler.ts" path="src/handlers/typebox/base-type-handler.ts"></mcfile>: The root abstract base class that defines the common interface for all type handlers. It provides the `canHandle` and `handle` methods. Implementations use helpers such as `makeTypeCall` (see <mcfile name="typebox-codegen-utils.ts" path="src/utils/typebox-codegen-utils.ts"></mcfile>) to create TypeBox expressions.And add the missing processor reference:
- - <mcfile name="template-literal-type-handler.ts" path="src/handlers/typebox/template-literal-type-handler.ts"></mcfile>: Handles TypeScript template literal types (e.g., `` `hello-${string}` ``). Parses template literals into components, handling literal text, embedded types (string, number, unions), and string/numeric literals.
+ - <mcfile name="template-literal-type-handler.ts" path="src/handlers/typebox/template-literal-type-handler.ts"></mcfile>: Handles TypeScript template literal types (e.g., `` `hello-${string}` ``). Parses template literals into components, handling literal text and embedded types (string/number/boolean, unions).
+ - <mcfile name="template-literal-type-processor.ts" path="src/handlers/typebox/template-literal-type-processor.ts"></mcfile>: Utility used by the TemplateLiteralTypeHandler to translate substitution nodes into precise TypeBox expressions (literals, unions, or fallbacks).Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ARCHITECTURE.md around lines 172 to 180, the Base Handler Classes section
incorrectly implies makeTypeCall is provided by the base handler and omits the
new processor added in this PR; update the text to state that makeTypeCall is
located in src/utils/typebox-codegen-utils.ts (not in base-type-handler.ts),
reword the base-type-handler description to only list methods it actually
provides (canHandle, handle, etc.), and add a short entry naming and linking the
new processor file you introduced so readers can find it.
| - <mcfile name="simple-type-handler.ts" path="src/handlers/typebox/simple-type-handler.ts"></mcfile>: Handles basic TypeScript types like `string`, `number`, `boolean`, `null`, `undefined`, `any`, `unknown`, `void`. | ||
| - <mcfile name="literal-type-handler.ts" path="src/handlers/typebox/literal-type-handler.ts"></mcfile>: Handles TypeScript literal types (e.g., `'hello'`, `123`, `true`). | ||
| - <mcfile name="function-type-handler.ts" path="src/handlers/typebox/function-type-handler.ts"></mcfile>: Handles TypeScript function types and function declarations, including parameter types, optional parameters, and return types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor grammar/style polish in handler list.
Tighten wording and ensure consistency in the list entries (LanguageTool flagged this area).
- - <mcfile name="literal-type-handler.ts" path="src/handlers/typebox/literal-type-handler.ts"></mcfile>: Handles TypeScript literal types (e.g., `'hello'`, `123`, `true`).
+ - <mcfile name="literal-type-handler.ts" path="src/handlers/typebox/literal-type-handler.ts"></mcfile>: Handles TypeScript literal types (for example, `'hello'`, `123`, `true`).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - <mcfile name="simple-type-handler.ts" path="src/handlers/typebox/simple-type-handler.ts"></mcfile>: Handles basic TypeScript types like `string`, `number`, `boolean`, `null`, `undefined`, `any`, `unknown`, `void`. | |
| - <mcfile name="literal-type-handler.ts" path="src/handlers/typebox/literal-type-handler.ts"></mcfile>: Handles TypeScript literal types (e.g., `'hello'`, `123`, `true`). | |
| - <mcfile name="function-type-handler.ts" path="src/handlers/typebox/function-type-handler.ts"></mcfile>: Handles TypeScript function types and function declarations, including parameter types, optional parameters, and return types. | |
| - <mcfile name="simple-type-handler.ts" path="src/handlers/typebox/simple-type-handler.ts"></mcfile>: Handles basic TypeScript types like `string`, `number`, `boolean`, `null`, `undefined`, `any`, `unknown`, `void`. | |
| - <mcfile name="literal-type-handler.ts" path="src/handlers/typebox/literal-type-handler.ts"></mcfile>: Handles TypeScript literal types (for example, `'hello'`, `123`, `true`). | |
| - <mcfile name="function-type-handler.ts" path="src/handlers/typebox/function-type-handler.ts"></mcfile>: Handles TypeScript function types and function declarations, including parameter types, optional parameters, and return types. |
🧰 Tools
🪛 LanguageTool
[grammar] ~205-~205: There might be a mistake here.
Context: ..., undefined, any, unknown, void. - : Handles TypeScript literal types (e.g....
(QB_NEW_EN)
[grammar] ~207-~207: There might be a mistake here.
Context: ..., optional parameters, and return types. - : Handles TypeScript template literal ty...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In ARCHITECTURE.md around lines 205 to 207, the three handler list entries need
parallel, tightened wording and consistent punctuation; revise each bullet to
start with the same verb ("Handles"), remove redundant phrasing (avoid repeating
"TypeScript" where unnecessary), make examples parenthetical and parallel (e.g.,
"(e.g., 'hello', 123, true)"), and ensure consistent punctuation style across
all items (same sentence fragment style and use of periods or no periods).
| handle(node: TupleTypeNode): ts.Expression { | ||
| return this.processTypeCollection(node.getElements(), 'Tuple') | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: tuple elements may be NamedTupleMember/RestTypeNode — pass their inner TypeNodes to avoid generating Any
node.getElements() returns element nodes (e.g., NamedTupleMember, RestTypeNode), not always TypeNodes. Forwarding them directly to a generic collection processor risks falling back to Any. Extract the element type nodes first.
Apply this diff:
- handle(node: TupleTypeNode): ts.Expression {
- return this.processTypeCollection(node.getElements(), 'Tuple')
- }
+ handle(node: TupleTypeNode): ts.Expression {
+ const elementTypeNodes = node.getElements().map((el) => {
+ if (Node.isNamedTupleMember(el) || Node.isRestTypeNode(el)) {
+ return el.getTypeNode()
+ }
+ // Fallback: if it's already a TypeNode, keep as is.
+ return Node.isTypeNode(el) ? el : undefined
+ }).filter((n): n is Node => !!n)
+
+ return this.processTypeCollection(elementTypeNodes, 'Tuple')
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| handle(node: TupleTypeNode): ts.Expression { | |
| return this.processTypeCollection(node.getElements(), 'Tuple') | |
| } | |
| handle(node: TupleTypeNode): ts.Expression { | |
| const elementTypeNodes = node | |
| .getElements() | |
| .map((el) => { | |
| if (Node.isNamedTupleMember(el) || Node.isRestTypeNode(el)) { | |
| return el.getTypeNode() | |
| } | |
| // Fallback: if it's already a TypeNode, keep as is. | |
| return Node.isTypeNode(el) ? el : undefined | |
| }) | |
| .filter((n): n is Node => !!n) | |
| return this.processTypeCollection(elementTypeNodes, 'Tuple') | |
| } |
🤖 Prompt for AI Agents
In src/handlers/typebox/collection/tuple-type-handler.ts around lines 9 to 11,
the handler forwards node.getElements() directly but those elements can be
NamedTupleMember or RestTypeNode (not raw TypeNodes), causing the processor to
fall back to Any; update the method to map node.getElements() to an array of
inner TypeNodes by extracting .type from NamedTupleMember and .type from
RestTypeNode (or using the element itself if it's already a TypeNode), then pass
that array of TypeNodes to this.processTypeCollection('Tuple') so the correct
inner types are processed.
| import { BaseTypeHandler } from '@daxserver/validation-schema-codegen/handlers/typebox/base-type-handler' | ||
| import { getTypeBoxType } from '@daxserver/validation-schema-codegen/utils/typebox-call' | ||
| import { makeTypeCall } from '@daxserver/validation-schema-codegen/utils/typebox-codegen-utils' | ||
| import { Node, SyntaxKind, ts, TypeOperatorTypeNode } from 'ts-morph' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type guard/type name mismatch: use TypeOperatorNode and Node.isTypeOperatorNode
TypeOperatorTypeNode and Node.isTypeOperatorTypeNode don’t exist in ts-morph; this will not compile. Use TypeOperatorNode and Node.isTypeOperatorNode.
Apply this diff:
-import { Node, SyntaxKind, ts, TypeOperatorTypeNode } from 'ts-morph'
+import { Node, SyntaxKind, ts, TypeOperatorNode } from 'ts-morph'
@@
- canHandle(node: Node): boolean {
- return Node.isTypeOperatorTypeNode(node) && node.getOperator() === SyntaxKind.KeyOfKeyword
- }
+ canHandle(node: Node): boolean {
+ return Node.isTypeOperatorNode(node) && node.getOperator() === SyntaxKind.KeyOfKeyword
+ }
@@
- handle(node: TypeOperatorTypeNode): ts.Expression {
+ handle(node: TypeOperatorNode): ts.Expression {
const operandType = node.getTypeNode()
- const typeboxOperand = getTypeBoxType(operandType)
-
- return makeTypeCall('KeyOf', [typeboxOperand])
+ const typeboxOperand = getTypeBoxType(operandType)
+ return makeTypeCall('KeyOf', [typeboxOperand])
}Also applies to: 7-9, 11-15
🤖 Prompt for AI Agents
In src/handlers/typebox/keyof-type-handler.ts around lines 1-4 (and also adjust
occurrences at lines 7-9 and 11-15), the code imports and checks for the
non-existent ts-morph symbol TypeOperatorTypeNode and calls
Node.isTypeOperatorTypeNode; replace the import and type with TypeOperatorNode
and change runtime checks to Node.isTypeOperatorNode. Update the import list
from 'ts-morph' to include TypeOperatorNode instead of TypeOperatorTypeNode, and
replace any use of TypeOperatorTypeNode and Node.isTypeOperatorTypeNode in the
file with TypeOperatorNode and Node.isTypeOperatorNode respectively so the code
compiles.
| import { BaseTypeHandler } from '@daxserver/validation-schema-codegen/handlers/typebox/base-type-handler' | ||
| import { getTypeBoxType } from '@daxserver/validation-schema-codegen/utils/typebox-call' | ||
| import { makeTypeCall } from '@daxserver/validation-schema-codegen/utils/typebox-codegen-utils' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Ensure the handler is registered and covered by tests
Make sure the central dispatcher wires KeyOfTypeHandler and add tests for:
keyof { a: string; b: number }->Type.KeyOf(Type.Object(...))- Nested:
keyof Partial<T>
🏁 Script executed:
#!/bin/bash
rg -nP 'new\s+KeyOfTypeHandler\s*\('
rg -nP 'keyof\s*{'Length of output: 318
KeyOfTypeHandler registration verified; basic tests present; nested Partial test missing
- Registration confirmed in
src/handlers/typebox/typebox-type-handlers.tsat line 45:
const keyOfTypeHandler = new KeyOfTypeHandler() - Tests for the basic
keyof { a: string; b: number }scenario exist intests/handlers/typebox/utility-types.test.tsat lines 17 and 153 - No existing test covers the nested case
keyof Partial<T>—please add a unit test for this scenario
🤖 Prompt for AI Agents
In src/handlers/typebox/keyof-type-handler.ts lines 1-3: add a unit test to
cover the nested case "keyof Partial<T>" — open
tests/handlers/typebox/utility-types.test.ts (around existing tests at ~lines 17
and 153) and add a new test that defines an interface/type T with properties
(e.g., a:string, b:number), applies keyof Partial<T>, generates the TypeBox
schema via the same test helper used in existing tests, and assert the produced
TypeBox representation matches the expected union of the property names ("a" |
"b") or the equivalent TypeBox call; follow the style and helpers of the
existing tests to ensure proper setup and teardown.
| handle(node: TypeLiteralNode): ts.Expression { | ||
| return this.createObjectType(this.processProperties(node.getProperties())) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
❓ Verification inconclusive
Coverage gap: index signatures and method signatures in type literals are ignored
node.getProperties() skips index signatures, call/construct signatures, and methods. If you need { [k: string]: T } or callable shapes, this will silently drop members.
Consider extending the base to support:
node.getIndexSignatures()-> TypeBoxRecordor equivalent- method/call signatures -> decide whether to reject or represent as
Any
Scan for existing handling to avoid duplication:
🏁 Script executed:
#!/bin/bash
rg -nP 'getIndexSignatures\s*\('
rg -nP 'CallSignature|MethodSignature|ConstructSignature'Length of output: 94
Extend ObjectTypeHandler to Support Index and Call Signatures
I verified that there is currently no handling for index signatures or call/construct/method signatures in the codebase:
- Ran
rg -nP 'getIndexSignatures\s*\('→ no matches - Ran
rg -nP 'CallSignature|MethodSignature|ConstructSignature'→ no matches
This confirms that handle(node: TypeLiteralNode) in
src/handlers/typebox/object/object-type-handler.ts (lines 9–11)
only covers ordinary properties and will silently drop:
- Index signatures (
{ [key: string]: T }) - Call signatures (callable shapes)
- Construct signatures
- Method signatures
To prevent these members from being lost, please refactor as follows:
• In handle(node: TypeLiteralNode):
- After
node.getProperties(), callnode.getIndexSignatures()and map each signature to a TypeBoxRecord(or equivalent), for example:const indexSigs = node.getIndexSignatures().map(sig => { const keyType = this.checker.getIndexTypeOfSignature(sig); const valueType = this.handleTypeNode(sig.type!); return Type.Record(keyType, valueType); });
• Handle call and construct signatures via node.getCallSignatures() and node.getConstructSignatures():
- Decide whether to map these to TypeBox
Function/Constructoror fallback toType.Any, and include them in the returned expression.
• HandleMethodSignaturenodes (e.g. vianode.getMethods()) similarly, either by representing them as functions or rejecting them explicitly with a clear error message.
• Add unit tests for each case to ensure coverage and prevent regressions.
Tagging as because silent dropping of members is a breaking behavior that should be addressed before merging.
🤖 Prompt for AI Agents
In src/handlers/typebox/object/object-type-handler.ts around lines 9–11, the
current handle(node: TypeLiteralNode) only processes ordinary properties and
drops index, call/construct and method signatures; update this method to also
call node.getIndexSignatures(), node.getCallSignatures(),
node.getConstructSignatures(), and node.getMethods(), mapping each index
signature to a Type.Record by using the checker to obtain the key type and
this.handleTypeNode to obtain the value type, mapping call/construct signatures
to an appropriate TypeBox Function/Constructor representation (or to Type.Any as
a documented fallback), and mapping MethodSignature nodes to function types or
throwing a clear, explicit error if unsupported; ensure all generated
expressions (properties + index record(s) + function/constructor/method
representations) are combined into the returned expression and add unit tests
covering index signatures, call signatures, construct signatures, and method
signatures to prevent regressions.
| handle(node: TypeReferenceNode): ts.Expression { | ||
| const referencedType = node.getTypeName() | ||
|
|
||
| handle(typeNode: Node): ts.Expression { | ||
| if (!Node.isTypeReference(typeNode)) { | ||
| return makeTypeCall('Any') | ||
| } | ||
| const referencedType = typeNode.getTypeName() | ||
| if (Node.isIdentifier(referencedType)) { | ||
| const typeName = referencedType.getText() | ||
| return ts.factory.createIdentifier(typeName) | ||
| } | ||
|
|
||
| return makeTypeCall('Any') | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Handle QualifiedName references (e.g., Namespace.Type) instead of falling back to Any.
Currently only identifiers are supported; qualified names degrade to Any. This will drop namespaced references and imported type namespaces.
Apply the following diff to support both Identifier and QualifiedName:
- handle(node: TypeReferenceNode): ts.Expression {
- const referencedType = node.getTypeName()
-
- if (Node.isIdentifier(referencedType)) {
- const typeName = referencedType.getText()
- return ts.factory.createIdentifier(typeName)
- }
-
- return makeTypeCall('Any')
- }
+ handle(node: TypeReferenceNode): ts.Expression {
+ const referencedType = node.getTypeName()
+
+ if (Node.isIdentifier(referencedType)) {
+ return ts.factory.createIdentifier(referencedType.getText())
+ }
+ if (Node.isQualifiedName(referencedType)) {
+ return buildNameExpression(referencedType)
+ }
+
+ return makeTypeCall('Any')
+ }Add the helper below the class to build a property access chain for nested qualified names:
function buildNameExpression(name: Node): ts.Expression {
if (Node.isIdentifier(name)) {
return ts.factory.createIdentifier(name.getText())
}
if (Node.isQualifiedName(name)) {
const left = buildNameExpression(name.getLeft())
const right = ts.factory.createIdentifier(name.getRight().getText())
return ts.factory.createPropertyAccessExpression(left, right)
}
return ts.factory.createIdentifier(name.getText())
}🤖 Prompt for AI Agents
In src/handlers/typebox/type-reference-handler.ts around lines 10 to 19, the
handler only converts Identifier type names and falls back to Any for
QualifiedName nodes; update the handler to support QualifiedName chains by
building a property-access expression instead of returning Any. Add a helper
function below the class that recursively converts a Node (Identifier or
QualifiedName) into a ts.Expression by returning an identifier for identifiers
and creating nested ts.factory.createPropertyAccessExpression(left, right) for
qualified names; then change handle(node: TypeReferenceNode) to call that helper
with node.getTypeName() and return its expression when Node.isIdentifier or
Node.isQualifiedName, otherwise keep the existing fallback.
| import { BaseTypeHandler } from '@daxserver/validation-schema-codegen/handlers/typebox/base-type-handler' | ||
| import { makeTypeCall } from '@daxserver/validation-schema-codegen/utils/typebox-codegen-utils' | ||
| import { Node, ts } from 'ts-morph' | ||
|
|
||
| export class TypeofTypeHandler extends BaseTypeHandler { | ||
| canHandle(node: Node): boolean { | ||
| return Node.isTypeQuery(node) | ||
| } | ||
|
|
||
| handle(): ts.Expression { | ||
| // TypeQuery represents 'typeof' expressions in TypeScript | ||
| // For TypeBox, we'll return a String type as typeof returns string literals | ||
| return makeTypeCall('String') | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify dispatcher registration and absence of legacy handlers.
Ensure the new TypeofTypeHandler is registered in the central TypeBoxTypeHandlers and that no conflicting legacy handler tries to process TypeQuery.
🏁 Script executed:
#!/bin/bash
# Verify handler registration and potential duplicates
rg -nP 'new\s+TypeofTypeHandler\(' -C2
fd -a typeof-type-handler.ts
# Ensure no other handler claims TypeQuery
rg -nP 'isTypeQuery\s*\(' -C3Length of output: 1628
Remove legacy TypeQueryHandler to prevent duplicate handling of typeof expressions
Your new TypeofTypeHandler is correctly registered at line 51 of typebox-type-handlers.ts, but the old TypeQueryHandler still claims Node.isTypeQuery(node) (in src/handlers/typebox/type-query-handler.ts), causing two handlers to match the same node type.
Locations needing action:
src/handlers/typebox/typebox-type-handlers.ts(around line 49–51)
• Remove or comment outconst typeQueryHandler = new TypeQueryHandler()
src/handlers/typebox/type-query-handler.ts
• Remove the file or refactor itscanHandlemethod so it no longer returnsNode.isTypeQuery(node)
Once the legacy handler is removed or updated, your TypeofTypeHandler will have exclusive responsibility for typeof queries.
🤖 Prompt for AI Agents
In src/handlers/typebox/typebox-type-handlers.ts around lines 49–51 and
src/handlers/typebox/type-query-handler.ts (entire file), remove the legacy
TypeQueryHandler registration and/or implementation so it no longer claims
TypeScript TypeQuery nodes: delete or comment out the line creating const
typeQueryHandler = new TypeQueryHandler() in typebox-type-handlers.ts and either
remove src/handlers/typebox/type-query-handler.ts or modify its canHandle method
so it does NOT return Node.isTypeQuery(node) (e.g., change to false or narrow to
a different node kind); ensure no other code registers or instantiates the
legacy handler so TypeofTypeHandler is the exclusive handler for
typeof/typequery nodes.
| handle(): ts.Expression { | ||
| // TypeQuery represents 'typeof' expressions in TypeScript | ||
| // For TypeBox, we'll return a String type as typeof returns string literals | ||
| return makeTypeCall('String') | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method signature mismatch and incorrect semantics for TypeQuery (“typeof”).
- The override
handle()is missing thenodeparameter. This will not be type-compatible withBaseTypeHandler.handle(node: Node)and will break the dispatcher call path (handler.handle(node)). - Mapping
typeoftoType.String()is conceptually incorrect.TypeQueryNoderepresents the type of a value (e.g.,typeof foo), not the runtimetypeofoperator’s string result. ReturningStringsilently misrepresents types and will corrupt downstream schemas, especially in patterns likekeyof typeof Enum.
Apply the minimal fix to restore compatibility and avoid incorrect typing until proper resolution is implemented:
- handle(): ts.Expression {
- // TypeQuery represents 'typeof' expressions in TypeScript
- // For TypeBox, we'll return a String type as typeof returns string literals
- return makeTypeCall('String')
- }
+ handle(node: ts.TypeQueryNode): ts.Expression {
+ // TODO: Resolve the referenced declaration and derive its TypeBox schema.
+ // Fallback to Any for now to avoid producing incorrect schemas.
+ return makeTypeCall('Any')
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| handle(): ts.Expression { | |
| // TypeQuery represents 'typeof' expressions in TypeScript | |
| // For TypeBox, we'll return a String type as typeof returns string literals | |
| return makeTypeCall('String') | |
| } | |
| handle(node: ts.TypeQueryNode): ts.Expression { | |
| // TODO: Resolve the referenced declaration and derive its TypeBox schema. | |
| // Fallback to Any for now to avoid producing incorrect schemas. | |
| return makeTypeCall('Any') | |
| } |
🤖 Prompt for AI Agents
In src/handlers/typebox/typeof-type-handler.ts around lines 10 to 14, the handle
method both lacks the required node parameter (breaking the
BaseTypeHandler.handle(node: Node) contract) and incorrectly maps TypeQueryNode
to String; update the method signature to accept the node parameter (e.g.,
handle(node: ts.TypeQueryNode): ts.Expression) to restore compatibility and, to
avoid misrepresenting types, return a neutral placeholder (e.g.,
makeTypeCall('Unknown') or 'Any') instead of 'String' while leaving a TODO
comment to implement proper TypeQuery handling later.
| export const getTypeBoxType = (node?: Node): ts.Expression => { | ||
| if (!node) { | ||
| return makeTypeCall('Any') | ||
| } | ||
| const handler = handlers.getHandler(typeNode) | ||
| if (handler) { | ||
| return handler.handle(typeNode) | ||
|
|
||
| if (!handlers) { | ||
| handlers = new TypeBoxTypeHandlers() | ||
| } | ||
| return makeTypeCall('Any') | ||
|
|
||
| const handler = handlers.getHandler(node) | ||
|
|
||
| return handler ? handler.handle(node) : makeTypeCall('Any') | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Unreachable fallback: getHandler throws, so ?: makeTypeCall('Any') never executes.
TypeBoxTypeHandlers.getHandler() throws when no handler is found; getTypeBoxType therefore never reaches the ternary fallback and will throw instead of degrading to Type.Any as implied elsewhere (docs and parser fallbacks).
export const getTypeBoxType = (node?: Node): ts.Expression => {
if (!node) {
return makeTypeCall('Any')
}
if (!handlers) {
handlers = new TypeBoxTypeHandlers()
}
- const handler = handlers.getHandler(node)
-
- return handler ? handler.handle(node) : makeTypeCall('Any')
+ try {
+ const handler = handlers.getHandler(node)
+ return handler.handle(node)
+ } catch {
+ // Resilient fallback to Any when no handler matches
+ return makeTypeCall('Any')
+ }
}If you prefer not to catch here, alternatively make getHandler return undefined instead of throwing and update callers accordingly.
Run this quick check to confirm current throw behavior:
🏁 Script executed:
#!/bin/bash
rg -nP 'throw new Error\(`No handler found for type' src/handlers/typebox/typebox-type-handlers.ts -C2Length of output: 261
Handle missing handler fallback to Any
The call to handlers.getHandler(node) will always throw when no handler exists (see src/handlers/typebox/typebox-type-handlers.ts:134), so the ternary fallback (?: makeTypeCall('Any')) is effectively dead code. You need to ensure a resilient fallback instead of propagating the exception.
Locations to update:
- src/utils/typebox-call.ts (lines 9–21)
Suggested refactor (wrap in try/catch or adjust getHandler to return undefined):
export const getTypeBoxType = (node?: Node): ts.Expression => {
if (!node) {
return makeTypeCall('Any')
}
if (!handlers) {
handlers = new TypeBoxTypeHandlers()
}
- const handler = handlers.getHandler(node)
-
- return handler ? handler.handle(node) : makeTypeCall('Any')
+ try {
+ const handler = handlers.getHandler(node)
+ return handler.handle(node)
+ } catch {
+ // Fallback to Any when no handler matches
+ return makeTypeCall('Any')
+ }
}Alternatively, modify getHandler to return undefined instead of throwing, and keep the existing conditional.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getTypeBoxType = (node?: Node): ts.Expression => { | |
| if (!node) { | |
| return makeTypeCall('Any') | |
| } | |
| const handler = handlers.getHandler(typeNode) | |
| if (handler) { | |
| return handler.handle(typeNode) | |
| if (!handlers) { | |
| handlers = new TypeBoxTypeHandlers() | |
| } | |
| return makeTypeCall('Any') | |
| const handler = handlers.getHandler(node) | |
| return handler ? handler.handle(node) : makeTypeCall('Any') | |
| } | |
| export const getTypeBoxType = (node?: Node): ts.Expression => { | |
| if (!node) { | |
| return makeTypeCall('Any') | |
| } | |
| if (!handlers) { | |
| handlers = new TypeBoxTypeHandlers() | |
| } | |
| try { | |
| const handler = handlers.getHandler(node) | |
| return handler.handle(node) | |
| } catch { | |
| // Fallback to Any when no handler matches | |
| return makeTypeCall('Any') | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/utils/typebox-call.ts around lines 9 to 21, the call to
handlers.getHandler(node) will throw when no handler exists, so the current
ternary fallback is never reached; wrap the getHandler call in a try/catch
(after ensuring handlers is initialized) and if it throws or returns falsy
return makeTypeCall('Any'); alternatively, update getHandler to return undefined
instead of throwing and keep the existing conditional—either ensure safe handler
retrieval or catch the exception and return makeTypeCall('Any') as the resilient
fallback.
No description provided.