JSDoc to Type Annotations Codefix And @typedef #50644
Labels
Bug
A bug in TypeScript
Domain: Quick Fixes
Editor-provided fixes, often called code actions.
Effort: Moderate
Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual".
Help Wanted
You can do this
Milestone
Comments
|
It’s intentional that JSDoc types are only resolvable in JS files. So to clarify, you’re suggesting an additional codefix/refactor to convert |
|
Currently the annotateWithTypeFromJSDoc codefix in the service leaves you with unresolved references. Why suggestion is to make a codefix/refactor for |
DanielRosenwasser
added
Bug
|
This may not be super easy, but ideally beneath each |
|
We don’t. I really thought we did. |
|
I can try and take a stab at this. Should this be a refactoring or a codefix? |
|
I think a fix would be ok since the typedefs aren’t going to do anything in .ts files... using a codefix implies issuing a suggestion diagnostic, and we try to avoid doing that unless we think something is actually wrong. Having a |
chadhietala
added a commit
to chadhietala/TypeScript
that referenced
this issue
Sep 27, 2022
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
index 7a8f3192d9..12a8ab221d 100644
--- a/src/compiler/diagnosticMessages.json
+++ b/src/compiler/diagnosticMessages.json
@@ -5146,7 +5146,7 @@
"category": "Error",
"code": 6258
},
- "Found 1 error in {1}": {
+ "Found 1 error in {1}": {
"category": "Message",
"code": 6259
},
@@ -6028,9 +6028,9 @@
"category": "Message",
"code": 6930
},
- "List of file name suffixes to search when resolving a module." : {
- "category": "Error",
- "code": 6931
+ "List of file name suffixes to search when resolving a module.": {
+ "category": "Error",
+ "code": 6931
},
"Variable '{0}' implicitly has an '{1}' type.": {
@@ -7368,7 +7368,6 @@
"category": "Message",
"code": 95175
},
-
"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
"category": "Error",
"code": 18004
@@ -7524,5 +7523,13 @@
"The value '{0}' cannot be used here.": {
"category": "Error",
"code": 18050
+ },
+ "Convert @typedef to type": {
+ "category": "Message",
+ "code": 18051
+ },
+ "Convert all @typedefs to types": {
+ "category": "Message",
+ "code": 18051
}
}
diff --git a/src/services/codefixes/convertTypedefToType.ts b/src/services/codefixes/convertTypedefToType.ts
new file mode 100644
index 0000000000..fbb8d53354
--- /dev/null
+++ b/src/services/codefixes/convertTypedefToType.ts
@@ -0,0 +1,165 @@
+namespace ts.codefix {
+ const fixId = "convertTypedefToType";
+ registerCodeFix({
+ fixIds: [fixId],
+ errorCodes: [Diagnostics.Convert_typedef_to_type.code],
+ getCodeActions(context) {
+ debugger;
+ const checker = context.program.getTypeChecker();
+ const info = getInfo(
+ context.sourceFile,
+ context.span.start,
+ checker
+ );
+ const changes = textChanges.ChangeTracker.with(context, (t) => {
+ const node = getTokenAtPosition(
+ context.sourceFile,
+ context.span.start
+ );
+
+ if (containsTypeDefTag(node)) {
+ fixSingleTypeDef(t, node, context, info?.typeNode);
+ }
+ });
+
+ if (changes.length > 0) {
+ return [
+ createCodeFixAction(
+ fixId,
+ changes,
+ Diagnostics.Convert_typedef_to_type,
+ fixId,
+ Diagnostics.Convert_all_typedefs_to_types
+ ),
+ ];
+ }
+ },
+ getAllCodeActions() {
+ debugger;
+ return { changes: [] };
+ },
+ });
+
+ function fixSingleTypeDef(
+ changes: textChanges.ChangeTracker,
+ typeDefNode: JSDocTypedefTag | undefined,
+ context: CodeFixContextBase,
+ typeNode?: TypeNode
+ ) {
+ if (!typeDefNode?.name || !typeDefNode?.fullName || !typeNode) {
+ return undefined;
+ }
+
+ const comment = typeDefNode.parent;
+
+ changes.replaceNode(
+ context.sourceFile,
+ comment,
+ factory.createTypeAliasDeclaration(
+ [],
+ typeDefNode.fullName.getFullText(),
+ getTypeParameters(typeDefNode),
+ typeNode
+ )
+ // factory.createTypeParameterDeclaration()
+ );
+ }
+
+ function getInfo(
+ sourceFile: SourceFile,
+ pos: number,
+ checker: TypeChecker
+ ): { readonly typeNode: TypeNode; readonly type: Type } | undefined {
+ const decl = findAncestor(
+ getTokenAtPosition(sourceFile, pos),
+ isTypeContainer
+ );
+ const typeNode = decl && decl.type;
+ return (
+ typeNode && {
+ typeNode,
+ type: checker.getTypeFromTypeNode(typeNode),
+ }
+ );
+ }
+
+ type TypeContainer =
+ | AsExpression
+ | CallSignatureDeclaration
+ | ConstructSignatureDeclaration
+ | FunctionDeclaration
+ | GetAccessorDeclaration
+ | IndexSignatureDeclaration
+ | MappedTypeNode
+ | MethodDeclaration
+ | MethodSignature
+ | ParameterDeclaration
+ | PropertyDeclaration
+ | PropertySignature
+ | SetAccessorDeclaration
+ | TypeAliasDeclaration
+ | TypeAssertion
+ | VariableDeclaration;
+ function isTypeContainer(node: Node): node is TypeContainer {
+ // NOTE: Some locations are not handled yet:
+ // MappedTypeNode.typeParameters and SignatureDeclaration.typeParameters, as well as CallExpression.typeArguments
+ switch (node.kind) {
+ case SyntaxKind.AsExpression:
+ case SyntaxKind.CallSignature:
+ case SyntaxKind.ConstructSignature:
+ case SyntaxKind.FunctionDeclaration:
+ case SyntaxKind.GetAccessor:
+ case SyntaxKind.IndexSignature:
+ case SyntaxKind.MappedType:
+ case SyntaxKind.MethodDeclaration:
+ case SyntaxKind.MethodSignature:
+ case SyntaxKind.Parameter:
+ case SyntaxKind.PropertyDeclaration:
+ case SyntaxKind.PropertySignature:
+ case SyntaxKind.SetAccessor:
+ case SyntaxKind.TypeAliasDeclaration:
+ case SyntaxKind.TypeAssertionExpression:
+ case SyntaxKind.VariableDeclaration:
+ return true;
+ default:
+ return false;
+ }
+ }
+
+ function getTypeParameters(node: JSDocTypedefTag) {
+ if (node.typeExpression && isJSDocTypeLiteral(node.typeExpression)) {
+ node.typeExpression.jsDocPropertyTags?.map((tag) => {
+ return factory.createTypeParameterDeclaration(
+ [],
+ tag.name.getFullText()
+ );
+ });
+ }
+
+ return [];
+ }
+
+ export function _containsJSDocTypedef(node: Node): node is HasJSDoc {
+ if (hasJSDocNodes(node)) {
+ const jsDocNodes = node.jsDoc || [];
+ return jsDocNodes.some((node) => {
+ const tags = node.tags || [];
+ return tags.some((tag) => isJSDocTypedefTag(tag));
+ });
+ }
+ return false;
+ }
+
+ export function getJSDocTypedefNode(node: HasJSDoc): JSDocTypedefTag {
+ const jsDocNodes = node.jsDoc || [];
+
+ return flatMap(jsDocNodes, (node) => {
+ const tags = node.tags || [];
+ return tags.filter((tag) => isJSDocTypedefTag(tag));
+ })[0] as unknown as JSDocTypedefTag;
+ }
+
+ export function containsTypeDefTag(node: Node): node is JSDocTypedefTag {
+ return isInJSDoc(node) && isJSDocTypedefTag(node);
+ }
+}
diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts
index 7be5d210b3..b1db35818e 100644
--- a/src/services/suggestionDiagnostics.ts
+++ b/src/services/suggestionDiagnostics.ts
@@ -54,6 +54,11 @@ namespace ts {
}
}
+ if (codefix._containsJSDocTypedef(node)) {
+ const jsdocTypedefNode = codefix.getJSDocTypedefNode(node);
+ diags.push(createDiagnosticForNode(jsdocTypedefNode.name || jsdocTypedefNode, Diagnostics.Convert_typedef_to_type));
+ }
+
if (codefix.parameterShouldGetTypeFromJSDoc(node)) {
diags.push(createDiagnosticForNode(node.name || node, Diagnostics.JSDoc_types_may_be_moved_to_TypeScript_types));
}
diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json
index 2237163ebb..7eacc161c4 100644
--- a/src/services/tsconfig.json
+++ b/src/services/tsconfig.json
@@ -116,6 +116,7 @@
"codefixes/convertConstToLet.ts",
"codefixes/fixExpectedComma.ts",
"codefixes/fixAddVoidToPromise.ts",
+ "codefixes/convertTypedefToType.ts",
"refactors/convertExport.ts",
"refactors/convertImport.ts",
"refactors/convertToOptionalChainExpression.ts",
diff --git a/tests/cases/fourslash/codeFixConvertTypedefToType.ts b/tests/cases/fourslash/codeFixConvertTypedefToType.ts
new file mode 100644
index 0000000000..e22f5da83c
--- /dev/null
+++ b/tests/cases/fourslash/codeFixConvertTypedefToType.ts
@@ -0,0 +1,14 @@
+/// <reference path='fourslash.ts' />
+
+/////**
+//// * @typedef {Object} Foo
+//// * @Property {Number} bar
+//// */
+
+verify.codeFix({
+ description: ts.Diagnostics.Convert_typedef_to_type.message,
+ errorCode: ts.Diagnostics.Convert_typedef_to_type.code,
+ newFileContent: `type Foo = {
+ bar: number
+};`,
+});
This was referenced Nov 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Suggestion
Currently the JSDoc to TS types codefix does not
🔍 Search Terms
Found #19285 but after trying out the code action in the editor it doesn't seem like
✅ Viability Checklist
My suggestion meets these guidelines:
⭐ Suggestion
Currently the
annotateWithTypeFromJSDoccodefix does a great job at handling many cases for converting JSDoc to types. However it seems that it does not follow type defs that have been introduced via@typedef, leaving you will unresolved references. Since the compiler is able to emit type aliases from@typedefreferences it seems like there is some missing functionality in this codefix or that a code fix could be introduced for@typedefreferences in TS files to convert to a type alias.📃 Motivating Example
Before:
After:
💻 Use Cases
I think the uses cases are covered in #48982, but in general we are trying to convert millions of lines of JS to TS, being able to leverage as much of the static information we already in files to produce valid types would be very helpful.
The text was updated successfully, but these errors were encountered: