feat: extend indexed array access support #5
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds special-case handling for typeof A[number] where A is a readonly tuple of string literals. The handler resolves A from the current source file, extracts a union of literals when applicable, and otherwise falls back to the existing indexed-access handling. Tests add cases for tuple array spread and adjust formatting expectations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant TS as TS Compiler/AST
participant H as IndexedAccessTypeHandler
participant SF as SourceFile Resolver
participant TU as extractTupleUnion
participant TB as TypeBox Generator
Dev->>TS: type A = typeof A[number]
TS->>H: IndexedAccessType(typeof A, number)
rect rgba(230,240,255,0.5)
note right of H: New special-case
H->>SF: Resolve "A" (type alias or var decl)
alt Found tuple reference
SF-->>H: TupleType or readonly TupleType
H->>TU: Extract string literal elements
alt String literals found
TU-->>H: Union<string literals>
H->>TB: Generate Type.Union(Type.Literal(...))
TB-->>H: TypeBox AST
else Not applicable
TU-->>H: null
H->>TB: Fallback to default indexed-access
TB-->>H: TypeBox AST
end
else Not found
SF-->>H: Not found
H->>TB: Fallback to default indexed-access
TB-->>H: TypeBox AST
end
end
H-->>TS: Resulting TypeBox schema
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/handlers/typebox/indexed-access-type-handler.ts (4)
36-58: Resolve the typeof target via the type checker; scanning the source file is brittle.Relying on
sourceFile.getTypeAlias()/getVariableDeclaration()coupled withNode.isIdentifier()misses common cases:
- Imported bindings (e.g.,
import { A } from './a').- Qualified names/namespaces (
typeof NS.A[number]).- Declarations nested in modules or different files.
- Values with only an initializer (no explicit type node), e.g.,
const A = ["a","b","c"] as const.Prefer resolving
exprName.getSymbol()(orgetTypeChecker()lookups) and iterating the symbol’s declarations to find a tuple. This also lets you coverQualifiedNameinstead of onlyIdentifier.If you want a minimal improvement without wiring the checker, at least handle
QualifiedNameand declarations with initializers.I can draft a checker-based resolution that supports identifiers, qualified names, and imports if desired.
41-49: Remove unreachable type-alias branch for typeof.
typeofcannot reference a type alias; it targets value-level symbols. This branch will never fire on valid TS and adds cognitive noise.Apply this diff to simplify:
- // First try to find a type alias declaration - const typeAlias = sourceFile.getTypeAlias(typeName) - if (typeAlias) { - const tupleUnion = this.extractTupleUnion(typeAlias.getTypeNode()) - if (tupleUnion) { - return tupleUnion - } - }
51-57: Handle const-asserted initializers (const A = [...] as const).
getTypeNode()is often undefined for variables that rely on an initializer. You currently miss the commonas constpattern. Either:
- Use the type checker (
variableDeclaration.getType()) and inspect tuple element literal types, or- Inspect the initializer and its
AsExpressionto detect const assertions.Want me to wire a checker-based variant that extracts string literals from
variableDeclaration.getType().getTupleElements()?
37-40: Support qualified names in typeof (e.g., typeof NS.A[number]).You currently only handle
Identifier. Intypeofqueries,exprNamecan be aQualifiedName. Either:
- Resolve via the type checker (recommended), or
- Walk the
QualifiedNameto its rightmost identifier and consult the declaration chain (works for simple namespaces but breaks across modules).Consider adding tests like:
declare namespace NS { export const A: readonly ["a", "b"]; } type T = typeof NS.A[number];and with
exportvariants.tests/handlers/typebox/array-types.test.ts (4)
16-22: Test name/content mismatch; exercise Array explicitly.The test is titled “Array” but the input is
string[]. This duplicates the “string[]” case below and leavesArray<string>untested.Apply this diff:
- const sourceFile = createSourceFile(project, `type A = string[]`) + const sourceFile = createSourceFile(project, `type A = Array<string>`)
37-52: Add a const-asserted initializer variant.To catch the initializer-only pattern the handler currently misses, add:
+ test('array spread via const assertion', () => { + const sourceFile = createSourceFile( + project, + ` + const A = ["a", "b", "c"] as const; + type A = typeof A[number]; + `, + ) + expect(generateFormattedCode(sourceFile)).resolves.toBe( + formatWithPrettier(` + const A = Type.Union([Type.Literal("a"), Type.Literal("b"), Type.Literal("c")]); + + type A = Static<typeof A>; + `), + ) + })
127-133: Exported “Array” test also uses string[]; align the input.Same issue as the non-export block.
Apply this diff:
- const sourceFile = createSourceFile(project, `export type A = string[]`) + const sourceFile = createSourceFile(project, `export type A = Array<string>`)
149-163: Exported array spread works; consider adding namespace/import variants.Great to see the exported path. To harden resolution, please add:
- Namespace-qualified:
export declare namespace NS { export const A: readonly ["a","b","c"]; } export type T = typeof NS.A[number];- Import from another file:
import { A } from './a' as const; export type T = typeof A[number];These will fail today due to identifier-only and same-file lookups.
I can provide fixtures for cross-file tests if helpful.
📜 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 (2)
src/handlers/typebox/indexed-access-type-handler.ts(1 hunks)tests/handlers/typebox/array-types.test.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/handlers/typebox/array-types.test.ts (1)
tests/utils.ts (3)
createSourceFile(8-10)generateFormattedCode(17-28)formatWithPrettier(12-15)
src/handlers/typebox/indexed-access-type-handler.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
🔇 Additional comments (10)
src/handlers/typebox/indexed-access-type-handler.ts (1)
15-21: Good targeted early-exit for typeof A[number].Nice guard that narrows to the exact pattern you want to special‑case before falling back to generic handling.
tests/handlers/typebox/array-types.test.ts (9)
30-35: Duplicate with the previous test after the rename above.Once the previous test uses
Array<string>, this one uniquely coversstring[]. No changes needed here beyond the prior fix.
37-52: Great coverage for typeof A[number] with a readonly tuple.This nails the primary behavior and expected TypeBox emission.
59-79: Union case looks correct and consistent with codegen style.Nothing to change here.
86-106: Intersection emission verified; formatting expectations align with Prettier.Looks good.
115-118: Literal union test is precise and minimal.Clean and readable.
141-146: Exported string[] case is fine once the previous test is corrected.No change needed.
166-190: Exported union generation remains consistent.Looks good.
197-217: Exported intersection generation verified.All good.
222-230: Exported literal union test is clear.No issues.
| private extractTupleUnion(typeNode: Node | undefined): ts.Expression | null { | ||
| if (!typeNode) return null | ||
|
|
||
| let actualTupleType: Node | undefined = typeNode | ||
|
|
||
| // Handle readonly modifier (TypeOperator) | ||
| if (typeNode.isKind(ts.SyntaxKind.TypeOperator)) { | ||
| const typeOperator = typeNode.asKindOrThrow(ts.SyntaxKind.TypeOperator) | ||
| actualTupleType = typeOperator.getTypeNode() | ||
| } | ||
|
|
||
| // Check if it's a tuple type | ||
| if (actualTupleType?.isKind(ts.SyntaxKind.TupleType)) { | ||
| const tupleType = actualTupleType.asKindOrThrow(ts.SyntaxKind.TupleType) | ||
| const elements = tupleType.getElements() | ||
|
|
||
| // Extract literal types from tuple elements | ||
| const literalTypes: ts.Expression[] = [] | ||
| for (const element of elements) { | ||
| if (element.isKind(ts.SyntaxKind.LiteralType)) { | ||
| const literalTypeNode = element.asKindOrThrow(ts.SyntaxKind.LiteralType) | ||
| const literal = literalTypeNode.getLiteral() | ||
|
|
||
| if (literal.isKind(ts.SyntaxKind.StringLiteral)) { | ||
| const stringLiteral = literal.asKindOrThrow(ts.SyntaxKind.StringLiteral) | ||
| const value = stringLiteral.getLiteralValue() | ||
| literalTypes.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(value)])) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Return union of literal types if we found any | ||
| if (literalTypes.length > 0) { | ||
| return makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literalTypes)]) | ||
| } | ||
| } | ||
|
|
||
| return null | ||
| } |
There was a problem hiding this comment.
Incorrect results for mixed-element tuples; return union only when all elements are string literals.
If a tuple mixes string literals with other types (e.g., [1, "a"]), the current logic returns a union of only the string literals and silently drops the rest, producing an unsound schema. You should either:
- Bail out to the default
Indexbehavior unless every element is a string literal, or - Fully cover other literal kinds (number/boolean) and named/optional/rest tuple members.
At minimum, gate on “all elements are string literals.”
Apply this diff to bail unless all elements are string literals and to support named tuple members:
private extractTupleUnion(typeNode: Node | undefined): ts.Expression | null {
if (!typeNode) return null
let actualTupleType: Node | undefined = typeNode
// Handle readonly modifier (TypeOperator)
if (typeNode.isKind(ts.SyntaxKind.TypeOperator)) {
const typeOperator = typeNode.asKindOrThrow(ts.SyntaxKind.TypeOperator)
actualTupleType = typeOperator.getTypeNode()
}
// Check if it's a tuple type
if (actualTupleType?.isKind(ts.SyntaxKind.TupleType)) {
const tupleType = actualTupleType.asKindOrThrow(ts.SyntaxKind.TupleType)
const elements = tupleType.getElements()
- // Extract literal types from tuple elements
- const literalTypes: ts.Expression[] = []
- for (const element of elements) {
- if (element.isKind(ts.SyntaxKind.LiteralType)) {
- const literalTypeNode = element.asKindOrThrow(ts.SyntaxKind.LiteralType)
- const literal = literalTypeNode.getLiteral()
-
- if (literal.isKind(ts.SyntaxKind.StringLiteral)) {
- const stringLiteral = literal.asKindOrThrow(ts.SyntaxKind.StringLiteral)
- const value = stringLiteral.getLiteralValue()
- literalTypes.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(value)]))
- }
- }
- }
-
- // Return union of literal types if we found any
- if (literalTypes.length > 0) {
- return makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literalTypes)])
- }
+ // Extract literal string types from tuple elements.
+ // Bail out if any element is not a string literal.
+ const literalTypes: ts.Expression[] = []
+ for (const element of elements) {
+ // Support named tuple members: [x: "a"]
+ const candidate =
+ element.isKind(ts.SyntaxKind.NamedTupleMember) ? element.getTypeNode() : element
+ if (!candidate || !candidate.isKind(ts.SyntaxKind.LiteralType)) {
+ return null
+ }
+ const literal = candidate.asKindOrThrow(ts.SyntaxKind.LiteralType).getLiteral()
+ if (!literal.isKind(ts.SyntaxKind.StringLiteral)) {
+ return null
+ }
+ const value = literal.asKindOrThrow(ts.SyntaxKind.StringLiteral).getLiteralValue()
+ literalTypes.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(value)]))
+ }
+
+ // Only return a union if every element contributed a string literal
+ if (literalTypes.length === elements.length && literalTypes.length > 0) {
+ return makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literalTypes)])
+ }
}
return null
}📝 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.
| private extractTupleUnion(typeNode: Node | undefined): ts.Expression | null { | |
| if (!typeNode) return null | |
| let actualTupleType: Node | undefined = typeNode | |
| // Handle readonly modifier (TypeOperator) | |
| if (typeNode.isKind(ts.SyntaxKind.TypeOperator)) { | |
| const typeOperator = typeNode.asKindOrThrow(ts.SyntaxKind.TypeOperator) | |
| actualTupleType = typeOperator.getTypeNode() | |
| } | |
| // Check if it's a tuple type | |
| if (actualTupleType?.isKind(ts.SyntaxKind.TupleType)) { | |
| const tupleType = actualTupleType.asKindOrThrow(ts.SyntaxKind.TupleType) | |
| const elements = tupleType.getElements() | |
| // Extract literal types from tuple elements | |
| const literalTypes: ts.Expression[] = [] | |
| for (const element of elements) { | |
| if (element.isKind(ts.SyntaxKind.LiteralType)) { | |
| const literalTypeNode = element.asKindOrThrow(ts.SyntaxKind.LiteralType) | |
| const literal = literalTypeNode.getLiteral() | |
| if (literal.isKind(ts.SyntaxKind.StringLiteral)) { | |
| const stringLiteral = literal.asKindOrThrow(ts.SyntaxKind.StringLiteral) | |
| const value = stringLiteral.getLiteralValue() | |
| literalTypes.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(value)])) | |
| } | |
| } | |
| } | |
| // Return union of literal types if we found any | |
| if (literalTypes.length > 0) { | |
| return makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literalTypes)]) | |
| } | |
| } | |
| return null | |
| } | |
| private extractTupleUnion(typeNode: Node | undefined): ts.Expression | null { | |
| if (!typeNode) return null | |
| let actualTupleType: Node | undefined = typeNode | |
| // Handle readonly modifier (TypeOperator) | |
| if (typeNode.isKind(ts.SyntaxKind.TypeOperator)) { | |
| const typeOperator = typeNode.asKindOrThrow(ts.SyntaxKind.TypeOperator) | |
| actualTupleType = typeOperator.getTypeNode() | |
| } | |
| // Check if it's a tuple type | |
| if (actualTupleType?.isKind(ts.SyntaxKind.TupleType)) { | |
| const tupleType = actualTupleType.asKindOrThrow(ts.SyntaxKind.TupleType) | |
| const elements = tupleType.getElements() | |
| // Extract literal string types from tuple elements. | |
| // Bail out if any element is not a string literal. | |
| const literalTypes: ts.Expression[] = [] | |
| for (const element of elements) { | |
| // Support named tuple members: [x: "a"] | |
| const candidate = | |
| element.isKind(ts.SyntaxKind.NamedTupleMember) ? element.getTypeNode() : element | |
| if (!candidate || !candidate.isKind(ts.SyntaxKind.LiteralType)) { | |
| return null | |
| } | |
| const literal = candidate.asKindOrThrow(ts.SyntaxKind.LiteralType).getLiteral() | |
| if (!literal.isKind(ts.SyntaxKind.StringLiteral)) { | |
| return null | |
| } | |
| const value = literal.asKindOrThrow(ts.SyntaxKind.StringLiteral).getLiteralValue() | |
| literalTypes.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(value)])) | |
| } | |
| // Only return a union if every element contributed a string literal | |
| if (literalTypes.length === elements.length && literalTypes.length > 0) { | |
| return makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literalTypes)]) | |
| } | |
| } | |
| return null | |
| } |
🤖 Prompt for AI Agents
In src/handlers/typebox/indexed-access-type-handler.ts around lines 66–104, the
current extractTupleUnion silently drops non-string-literal elements in mixed
tuples; update it to bail out (return null) unless every tuple element is a
string literal and handle named tuple members by unwrapping their inner type
node before checking. Concretely, iterate elements and for each: if it's a
NamedTupleMember extract its type node, if it's an optional/rest ensure you
examine the underlying type node, then require the node to be a LiteralType
whose literal is a StringLiteral; if any element fails that check return null;
only when all elements are string literals build the Literal calls and return
the Union as before. Ensure no partial unions are produced.
No description provided.