-
Notifications
You must be signed in to change notification settings - Fork 214
feat: add ReturnType parser #2317
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
base: next
Are you sure you want to change the base?
Changes from 2 commits
1bc0449
f379eac
af0482e
1008735
7c06489
e188df5
a266867
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
import ts from "typescript"; | ||
import type { Context, NodeParser } from "../NodeParser.js"; | ||
import type { SubNodeParser } from "../SubNodeParser.js"; | ||
import type { BaseType } from "../Type/BaseType.js"; | ||
import { UnknownNodeError } from "../Error/Errors.js"; | ||
import { ObjectType } from "../Type/ObjectType.js"; | ||
Check failure on line 6 in src/NodeParser/ReturnTypeNodeParser.ts
|
||
|
||
export class ReturnTypeNodeParser implements SubNodeParser { | ||
constructor( | ||
private readonly childNodeParser: NodeParser, | ||
private readonly checker: ts.TypeChecker, | ||
) {} | ||
|
||
supportsNode(node: ts.Node): boolean { | ||
if (!ts.isTypeReferenceNode(node)) { | ||
return false; | ||
} | ||
|
||
Check failure on line 18 in src/NodeParser/ReturnTypeNodeParser.ts
|
||
// Check if it's a ReturnType reference | ||
try { | ||
const typeName = ts.isIdentifier(node.typeName) | ||
Check failure on line 21 in src/NodeParser/ReturnTypeNodeParser.ts
|
||
? node.typeName.text | ||
: node.typeName.getText(); | ||
return typeName === 'ReturnType' && node.typeArguments?.length === 1; | ||
Check failure on line 24 in src/NodeParser/ReturnTypeNodeParser.ts
|
||
} catch { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a try catch? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the try/catch. |
||
return false; | ||
} | ||
} | ||
|
||
createType(node: ts.TypeReferenceNode, context: Context): BaseType { | ||
try { | ||
Check failure on line 31 in src/NodeParser/ReturnTypeNodeParser.ts
|
||
if (!node.typeArguments || node.typeArguments.length !== 1) { | ||
throw new UnknownNodeError(node); | ||
} | ||
|
||
const typeArg = node.typeArguments[0]; | ||
|
||
Check failure on line 37 in src/NodeParser/ReturnTypeNodeParser.ts
|
||
// Handle different types of type arguments | ||
if (ts.isTypeQueryNode(typeArg)) { | ||
// Case: ReturnType<typeof functionName> | ||
// Get the symbol for the identifier | ||
const symbol = this.checker.getSymbolAtLocation(typeArg.exprName); | ||
if (!symbol) { | ||
throw new UnknownNodeError(node); | ||
} | ||
|
||
// Get the declarations of the symbol | ||
const declarations = symbol.getDeclarations() || []; | ||
|
||
// Try multiple methods to extract return type | ||
for (const decl of declarations) { | ||
let returnTypeNode: ts.TypeNode | undefined; | ||
|
||
// If declaration is a function/method with explicit return type | ||
if ( | ||
(ts.isFunctionDeclaration(decl) || ts.isMethodDeclaration(decl) || | ||
Check failure on line 56 in src/NodeParser/ReturnTypeNodeParser.ts
|
||
ts.isArrowFunction(decl) || ts.isFunctionExpression(decl)) && | ||
Check failure on line 57 in src/NodeParser/ReturnTypeNodeParser.ts
|
||
decl.type | ||
) { | ||
returnTypeNode = decl.type; | ||
} | ||
Check failure on line 61 in src/NodeParser/ReturnTypeNodeParser.ts
|
||
// If declaration is a variable with function type annotation | ||
else if ( | ||
Check failure on line 63 in src/NodeParser/ReturnTypeNodeParser.ts
|
||
ts.isVariableDeclaration(decl) && | ||
decl.type && | ||
ts.isFunctionTypeNode(decl.type) | ||
) { | ||
returnTypeNode = decl.type.type; | ||
} | ||
|
||
// If we found a return type node, process it | ||
if (returnTypeNode) { | ||
const baseType = this.childNodeParser.createType(returnTypeNode, context); | ||
return baseType; | ||
} | ||
} | ||
|
||
// Fallback to type checking method | ||
const type = this.checker.getTypeOfSymbolAtLocation(symbol, typeArg); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as below. In what cases do we need the fallback? Why can't we use the function parser instead of implementing a lot of custom logic here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing this fallback causes issues with implicit function return types. I added a test for this case.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we reuse some of the existing function parsing logic here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to poke at it in terms of utilizing the FunctionNodeParser but since it's like an abstraction on top of functions I couldn't find a simple way of doing that. I am not fully aware of how this all works, so I do not know how to make it work well enough like this. Also we have it separated as "functions" but this is more of a data structure format (object, number, boolean, string) because of the ReturnType unless it returns a function (which might need to be further tested). I can try to make it better but I am just not sure how to connect them. I added more tests for the different function types though, and also tested methods. |
||
const signatures = type.getCallSignatures(); | ||
|
||
if (signatures.length > 0) { | ||
// Use getReturnType directly from the signature | ||
const returnType = signatures[0].getReturnType(); | ||
|
||
const returnTypeNode = this.checker.typeToTypeNode( | ||
returnType, | ||
undefined, | ||
ts.NodeBuilderFlags.NoTruncation | ||
); | ||
|
||
if (returnTypeNode) { | ||
return this.childNodeParser.createType(returnTypeNode, context); | ||
} | ||
} | ||
} else { | ||
// Case: ReturnType<SomeType["methodName"]> or other complex types | ||
// Get the type directly from TypeScript's type system | ||
const argType = this.checker.getTypeAtLocation(typeArg); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really want to a lid the type checker if possible. What cases fail when we remove this branch? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When removing the else branch completely:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, but what line of the example causes this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test/valid-data/type-return-type-complex/main.ts(15,28) Specifically the node ReturnType If you give me some more context of what to look at, I can take a deeper look to finding a better solution. |
||
|
||
// If it's a function type, get its return type | ||
const signatures = argType.getCallSignatures(); | ||
if (signatures.length > 0) { | ||
const returnType = signatures[0].getReturnType(); | ||
const returnTypeNode = this.checker.typeToTypeNode( | ||
returnType, | ||
undefined, | ||
ts.NodeBuilderFlags.NoTruncation | ||
); | ||
|
||
if (returnTypeNode) { | ||
return this.childNodeParser.createType(returnTypeNode, context); | ||
} | ||
} | ||
|
||
// Final fallback: try to get type directly | ||
const type = this.checker.getTypeAtLocation(typeArg); | ||
const typeNode = this.checker.typeToTypeNode( | ||
type, | ||
undefined, | ||
ts.NodeBuilderFlags.NoTruncation | ||
); | ||
|
||
if (typeNode) { | ||
return this.childNodeParser.createType(typeNode, context); | ||
} | ||
} | ||
|
||
throw new UnknownNodeError(node); | ||
} catch (error) { | ||
throw error; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a noop? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the try/catch |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// Simulated Redux Toolkit scenario | ||
export interface TestState { | ||
counter: number; | ||
name: string; | ||
} | ||
|
||
export function createTestStore() { | ||
return { | ||
getState: () => ({ counter: 0, name: "test" } as TestState), | ||
dispatch: (action: any) => {}, | ||
}; | ||
} | ||
|
||
export type TestAppStore = ReturnType<typeof createTestStore>; | ||
export type TestAppState = ReturnType<TestAppStore["getState"]>; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
{ | ||
"$ref": "#/definitions/TestAppState", | ||
"$schema": "http://json-schema.org/draft-07/schema#", | ||
"definitions": { | ||
"TestAppState": { | ||
"$ref": "#/definitions/TestState" | ||
}, | ||
"TestState": { | ||
"additionalProperties": false, | ||
"properties": { | ||
"counter": { | ||
"type": "number" | ||
}, | ||
"name": { | ||
"type": "string" | ||
} | ||
}, | ||
"required": [ | ||
"counter", | ||
"name" | ||
], | ||
"type": "object" | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export function greet(name: string): { message: string } { | ||
return { message: `Hello, ${name}!` }; | ||
} | ||
|
||
export type Greeting = ReturnType<typeof greet>; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
{ | ||
"$ref": "#/definitions/Greeting", | ||
"$schema": "http://json-schema.org/draft-07/schema#", | ||
"definitions": { | ||
"Greeting": { | ||
"additionalProperties": false, | ||
"properties": { | ||
"message": { | ||
"type": "string" | ||
} | ||
}, | ||
"required": [ | ||
"message" | ||
], | ||
"type": "object" | ||
} | ||
} | ||
} |
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.
Let's remove unnecessary comments that just explain what happens
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.
Removed