Reparse non-identifier jsdoc names where possible, error otherwise

_packages/native-preview/src/enums/nodeFlags.enum.ts

@@ -30,6 +30,7 @@ export enum NodeFlags {
     JsonFile = 1 << 25,
     PossiblyContainsDeprecatedTag = 1 << 26,
     Unreachable = 1 << 27,
+    ReparserTransformedLiteral = 1 << 28,
     BlockScoped = Let | Const | Using,
     Constant = Const | Using,
     AwaitUsing = Const | Using,

_packages/native-preview/src/enums/nodeFlags.ts

@@ -30,6 +30,7 @@ export var NodeFlags: any;
     NodeFlags[NodeFlags["JsonFile"] = 33554432] = "JsonFile";
     NodeFlags[NodeFlags["PossiblyContainsDeprecatedTag"] = 67108864] = "PossiblyContainsDeprecatedTag";
     NodeFlags[NodeFlags["Unreachable"] = 134217728] = "Unreachable";
+    NodeFlags[NodeFlags["ReparserTransformedLiteral"] = 268435456] = "ReparserTransformedLiteral";
     NodeFlags[NodeFlags["BlockScoped"] = 7] = "BlockScoped";
     NodeFlags[NodeFlags["Constant"] = 6] = "Constant";
     NodeFlags[NodeFlags["AwaitUsing"] = 6] = "AwaitUsing";

internal/ast/nodeflags.go

@@ -44,6 +44,7 @@ const (
 	NodeFlagsJsonFile                      NodeFlags = 1 << 25 // If node was parsed in a Json
 	NodeFlagsPossiblyContainsDeprecatedTag NodeFlags = 1 << 26 // Set during parse if comment text contains '@deprecated'; must confirm via JSDoc lookup
 	NodeFlagsUnreachable                   NodeFlags = 1 << 27 // If node is unreachable according to the binder
+	NodeFlagsReparserTransformedLiteral    NodeFlags = 1 << 28 // If node was transformed during parsing, making its' naive text source not match the AST
 
 	NodeFlagsBlockScoped = NodeFlagsLet | NodeFlagsConst | NodeFlagsUsing
 	NodeFlagsConstant    = NodeFlagsConst | NodeFlagsUsing

internal/parser/reparser.go

@@ -1,8 +1,13 @@
 package parser
 
 import (
+	"strconv"
+	"strings"
+
 	"github.com/microsoft/typescript-go/internal/ast"
 	"github.com/microsoft/typescript-go/internal/core"
+	"github.com/microsoft/typescript-go/internal/diagnostics"
+	"github.com/microsoft/typescript-go/internal/scanner"
 )
 
 func (p *Parser) finishReparsedNode(node *ast.Node, locationNode *ast.Node) {
@@ -26,6 +31,20 @@ func (p *Parser) addDeepCloneReparse(node *ast.Node) *ast.Node {
 	return clone
 }
 
+func (p *Parser) addTransformedReparse(newNode *ast.Node, old *ast.Node) *ast.Node {
+	p.finishReparsedNode(newNode, old)
+	newNode.Flags |= ast.NodeFlagsReparserTransformedLiteral
+	p.reparsedClones = append(p.reparsedClones, newNode)
+	return newNode
+}
+
+func (p *Parser) checkNonIdentifierName(name *ast.Node) *ast.Node {
+	if ast.IsIdentifier(name) && !scanner.IsValidIdentifier(name.AsIdentifier().Text) {
+		p.parseErrorAtRange(name.Loc, diagnostics.Identifier_expected)
+	}
+	return name
+}
+
 // Hosted tags find a host and add their children to the correct location under the host.
 // Unhosted tags add synthetic nodes to the reparse list.
 func (p *Parser) reparseTags(parent *ast.Node, jsDoc []*ast.Node) {
@@ -51,7 +70,7 @@ func (p *Parser) reparseUnhosted(tag *ast.Node, parent *ast.Node, jsDoc *ast.Nod
 		if typeExpression == nil {
 			break
 		}
-		typeAlias := p.factory.NewJSTypeAliasDeclaration(nil, p.addDeepCloneReparse(tag.AsJSDocTypedefTag().Name()), nil, nil)
+		typeAlias := p.factory.NewJSTypeAliasDeclaration(nil, p.addDeepCloneReparse(p.checkNonIdentifierName(tag.AsJSDocTypedefTag().Name())), nil, nil)
 		typeAlias.AsTypeAliasDeclaration().TypeParameters = p.gatherTypeParameters(jsDoc, tag)
 		var t *ast.Node
 		switch typeExpression.Kind {
@@ -107,9 +126,9 @@ func (p *Parser) reparseJSDocSignature(jsSignature *ast.Node, fun *ast.Node, jsD
 	clonedModifiers := p.factory.DeepCloneReparseModifiers(modifiers)
 	switch fun.Kind {
 	case ast.KindFunctionDeclaration:
-		signature = p.factory.NewFunctionDeclaration(clonedModifiers, nil, p.factory.DeepCloneReparse(fun.Name()), nil, nil, nil, nil, nil)
+		signature = p.factory.NewFunctionDeclaration(clonedModifiers, nil, p.factory.DeepCloneReparse(p.checkNonIdentifierName(fun.Name())), nil, nil, nil, nil, nil)
 	case ast.KindMethodDeclaration:
-		signature = p.factory.NewMethodDeclaration(clonedModifiers, nil, p.factory.DeepCloneReparse(fun.Name()), nil, nil, nil, nil, nil, nil)
+		signature = p.factory.NewMethodDeclaration(clonedModifiers, nil, p.factory.DeepCloneReparse(p.checkNonIdentifierName(fun.Name())), nil, nil, nil, nil, nil, nil)
 	case ast.KindConstructor:
 		signature = p.factory.NewConstructorDeclaration(clonedModifiers, nil, nil, nil, nil, nil)
 	case ast.KindJSDocCallbackTag:
@@ -122,7 +141,7 @@ func (p *Parser) reparseJSDocSignature(jsSignature *ast.Node, fun *ast.Node, jsD
 		signature.FunctionLikeData().TypeParameters = p.gatherTypeParameters(jsDoc, tag)
 	}
 	parameters := p.nodeSliceArena.NewSlice(0)
-	for _, param := range jsSignature.Parameters() {
+	for pi, param := range jsSignature.Parameters() {
 		var parameter *ast.Node
 		if param.Kind == ast.KindJSDocThisTag {
 			thisTag := param.AsJSDocThisTag()
@@ -155,8 +174,33 @@ func (p *Parser) reparseJSDocSignature(jsSignature *ast.Node, fun *ast.Node, jsD
 					paramType = p.reparseJSDocTypeLiteral(jsparam.TypeExpression.Type())
 				}
 			}
-
-			parameter = p.factory.NewParameterDeclaration(nil, dotDotDotToken, p.addDeepCloneReparse(jsparam.Name()), p.makeQuestionIfOptional(jsparam), paramType, nil)
+			name := jsparam.Name()
+			if ast.IsIdentifier(name) && !scanner.IsValidIdentifier(name.AsIdentifier().Text) {
+				// drop invalid chars for _, if empty, write _0, etc., so we have a valid param name to emit later
+				result := strings.Builder{}
+				for i, ch := range name.AsIdentifier().Text {
+					if i == 0 {
+						if !scanner.IsIdentifierStart(ch) {
+							result.WriteRune('_')
+						} else {
+							result.WriteRune(ch)
+						}
+						continue
+					} else if !scanner.IsIdentifierPart(ch) {
+						result.WriteRune('_')
+					} else {
+						result.WriteRune(ch)
+					}
+				}
+				if result.Len() == 0 {
+					result.WriteRune('_')
+					result.WriteString(strconv.Itoa(pi))
+				}
+				name = p.addTransformedReparse(p.factory.NewIdentifier(result.String()), name)
+			} else {
+				name = p.addDeepCloneReparse(name)
+			}
+			parameter = p.factory.NewParameterDeclaration(nil, dotDotDotToken, name, p.makeQuestionIfOptional(jsparam), paramType, nil)
 		}
 		p.finishReparsedNode(parameter, param)
 		parameters = append(parameters, parameter)
@@ -192,7 +236,12 @@ func (p *Parser) reparseJSDocTypeLiteral(t *ast.TypeNode) *ast.Node {
 			if name.Kind == ast.KindQualifiedName {
 				name = name.AsQualifiedName().Right
 			}
-			property := p.factory.NewPropertySignatureDeclaration(nil, p.addDeepCloneReparse(name), p.makeQuestionIfOptional(jsprop), nil, nil)
+			if ast.IsIdentifier(name) && !scanner.IsValidIdentifier(name.AsIdentifier().Text) {
+				name = p.addTransformedReparse(p.factory.NewStringLiteral(name.AsIdentifier().Text, ast.TokenFlagsNone), name)
+			} else {
+				name = p.addDeepCloneReparse(name)
+			}
+			property := p.factory.NewPropertySignatureDeclaration(nil, name, p.makeQuestionIfOptional(jsprop), nil, nil)
 			if jsprop.TypeExpression != nil {
 				property.AsPropertySignatureDeclaration().Type = p.reparseJSDocTypeLiteral(jsprop.TypeExpression.Type())
 			}
@@ -258,7 +307,7 @@ func (p *Parser) gatherTypeParameters(j *ast.Node, tagWithTypeParameters *ast.No
 			if constraint != nil && firstTypeParameter {
 				reparse = p.factory.NewTypeParameterDeclaration(
 					p.factory.DeepCloneReparseModifiers(tp.Modifiers()),
-					p.addDeepCloneReparse(tp.Name()),
+					p.addDeepCloneReparse(p.checkNonIdentifierName(tp.Name())),
 					p.addDeepCloneReparse(constraint.Type()),
 					nil, // expression
 					p.addDeepCloneReparse(tp.AsTypeParameterDeclaration().DefaultType),

internal/scanner/utilities.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/microsoft/typescript-go/internal/ast"
 	"github.com/microsoft/typescript-go/internal/core"
+	"github.com/microsoft/typescript-go/internal/debug"
 )
 
 const (
@@ -70,6 +71,21 @@ func GetTextOfNodeFromSourceText(sourceText string, node *ast.Node, includeTrivi
 		pos = SkipTrivia(sourceText, pos)
 	}
 	text := sourceText[pos:node.End()]
+	if node.Flags&ast.NodeFlagsReparserTransformedLiteral != 0 {
+		// This is similar to `getLiteralTextOfNode` in the printer, but without the context of an `emitContext` to provide overrides
+		if ast.IsStringLiteral(node) {
+			if node.AsStringLiteral().TokenFlags&ast.TokenFlagsSingleQuote != 0 {
+				return "'" + text + "'"
+			}
+			return "\"" + text + "\""
+		} else if ast.IsIdentifier(node) {
+			return node.Text()
+		}
+		// Only the above node kinds are currently transformed into one another by the reparser, requiring the textual remapping.
+		// (Any reamppings done by emit transforms are handled by `getLiteralTextOfNode` in the printer)
+		// Fail on any other kinds.
+		debug.FailBadSyntaxKind(node, "Unexpected reparser-transformed node kind")
+	}
 	// if (isJSDocTypeExpressionOrChild(node)) {
 	//     // strip space + asterisk at line start
 	//     text = text.split(/\r\n|\n|\r/).map(line => line.replace(/^\s*\*/, "").trimStart()).join("\n");

testdata/baselines/reference/compiler/jsdocNonIdentifierPropertiesAndParams.js

@@ -0,0 +1,50 @@
+//// [tests/cases/compiler/jsdocNonIdentifierPropertiesAndParams.ts] ////
+
+//// [index.js]
+/**
+ * @typedef {Object} ButtonProps
+ * @property {string} label The button label
+ * @property {string | null | undefined} [data-test-name] Test automation attribute
+ * @property {string | null | undefined} [aria-label] Accessibility label
+ */
+
+/**
+ * @param {ButtonProps} props
+ * @returns {ButtonProps}
+ */
+export function Button(props) {
+    return { ...props }
+}
+
+/**
+ * @callback ButtonPropsCallback
+ * @param {ButtonProps} [props-like]
+ * @returns {ButtonProps}
+ */
+
+
+
+
+//// [index.d.ts]
+/**
+ * @typedef {Object} ButtonProps
+ * @property {string} label The button label
+ * @property {string | null | undefined} [data-test-name] Test automation attribute
+ * @property {string | null | undefined} [aria-label] Accessibility label
+ */
+export type ButtonProps = {
+    label: string;
+    "data-test-name"?: string | null | undefined;
+    "aria-label"?: string | null | undefined;
+};
+/**
+ * @param {ButtonProps} props
+ * @returns {ButtonProps}
+ */
+export declare function Button(props: ButtonProps): ButtonProps;
+export type ButtonPropsCallback = (props_like?: ButtonProps) => ButtonProps;
+/**
+ * @callback ButtonPropsCallback
+ * @param {ButtonProps} [props-like]
+ * @returns {ButtonProps}
+ */

testdata/baselines/reference/compiler/jsdocNonIdentifierPropertiesAndParams.symbols

@@ -0,0 +1,28 @@
+//// [tests/cases/compiler/jsdocNonIdentifierPropertiesAndParams.ts] ////
+
+=== index.js ===
+/**
+ * @typedef {Object} ButtonProps
+ * @property {string} label The button label
+ * @property {string | null | undefined} [data-test-name] Test automation attribute
+ * @property {string | null | undefined} [aria-label] Accessibility label
+ */
+
+/**
+ * @param {ButtonProps} props
+ * @returns {ButtonProps}
+ */
+export function Button(props) {
+>Button : Symbol(Button, Decl(index.js, 0, 0))
+>props : Symbol(props, Decl(index.js, 11, 23))
+
+    return { ...props }
+>props : Symbol(props, Decl(index.js, 11, 23))
+}
+
+/**
+ * @callback ButtonPropsCallback
+ * @param {ButtonProps} [props-like]
+ * @returns {ButtonProps}
+ */
+

testdata/baselines/reference/compiler/jsdocNonIdentifierPropertiesAndParams.types

@@ -0,0 +1,29 @@
+//// [tests/cases/compiler/jsdocNonIdentifierPropertiesAndParams.ts] ////
+
+=== index.js ===
+/**
+ * @typedef {Object} ButtonProps
+ * @property {string} label The button label
+ * @property {string | null | undefined} [data-test-name] Test automation attribute
+ * @property {string | null | undefined} [aria-label] Accessibility label
+ */
+
+/**
+ * @param {ButtonProps} props
+ * @returns {ButtonProps}
+ */
+export function Button(props) {
+>Button : (props: ButtonProps) => ButtonProps
+>props : ButtonProps
+
+    return { ...props }
+>{ ...props } : { label: string; "data-test-name"?: string | null | undefined; "aria-label"?: string | null | undefined; }
+>props : ButtonProps
+}
+
+/**
+ * @callback ButtonPropsCallback
+ * @param {ButtonProps} [props-like]
+ * @returns {ButtonProps}
+ */
+

testdata/baselines/reference/submodule/compiler/jsEnumCrossFileExport.errors.txt

@@ -1,10 +1,11 @@
 enumDef.js(14,21): error TS1003: Identifier expected.
+enumDef.js(14,21): error TS1003: Identifier expected.
 index.js(4,17): error TS2702: 'Host' only refers to a type, but is being used as a namespace here.
 index.js(13,11): error TS2702: 'Host' only refers to a type, but is being used as a namespace here.
 index.js(18,11): error TS2702: 'Host' only refers to a type, but is being used as a namespace here.
 
 
-==== enumDef.js (1 errors) ====
+==== enumDef.js (2 errors) ====
     var Host = {};
     Host.UserMetrics = {};
     /** @enum {number} */
@@ -20,6 +21,8 @@ index.js(18,11): error TS2702: 'Host' only refers to a type, but is being used a
     /**
      * @typedef {string}
 
+!!! error TS1003: Identifier expected.
+
      */
 
 !!! error TS1003: Identifier expected.

testdata/baselines/reference/submodule/compiler/jsEnumCrossFileExport.errors.txt.diff

@@ -3,12 +3,13 @@
 @@= skipped -0, +0 lines =@@
 -<no content>
 +enumDef.js(14,21): error TS1003: Identifier expected.
++enumDef.js(14,21): error TS1003: Identifier expected.
 +index.js(4,17): error TS2702: 'Host' only refers to a type, but is being used as a namespace here.
 +index.js(13,11): error TS2702: 'Host' only refers to a type, but is being used as a namespace here.
 +index.js(18,11): error TS2702: 'Host' only refers to a type, but is being used as a namespace here.
 +
 +
-+==== enumDef.js (1 errors) ====
++==== enumDef.js (2 errors) ====
 +    var Host = {};
 +    Host.UserMetrics = {};
 +    /** @enum {number} */
@@ -24,6 +25,8 @@
 +    /**
 +     * @typedef {string}
 +                        
++!!! error TS1003: Identifier expected.
++                        
 +     */
 +    
 +!!! error TS1003: Identifier expected.

testdata/baselines/reference/submodule/compiler/jsdocTypedefNoCrash.errors.txt

@@ -0,0 +1,11 @@
+export.js(3,6): error TS1003: Identifier expected.
+
+
+==== export.js (1 errors) ====
+    /**
+     * @typedef {{
+     * }}
+
+!!! error TS1003: Identifier expected.
+     */
+    export const foo = 5;

testdata/baselines/reference/submodule/compiler/jsdocTypedefNoCrash.errors.txt.diff

@@ -0,0 +1,15 @@
+--- old.jsdocTypedefNoCrash.errors.txt
++++ new.jsdocTypedefNoCrash.errors.txt
+@@= skipped -0, +0 lines =@@
+-<no content>
++export.js(3,6): error TS1003: Identifier expected.
++
++
++==== export.js (1 errors) ====
++    /**
++     * @typedef {{
++     * }}
++         
++!!! error TS1003: Identifier expected.
++     */
++    export const foo = 5;

testdata/baselines/reference/submodule/compiler/jsdocTypedefNoCrash2.errors.txt

@@ -1,12 +1,15 @@
 export.js(1,13): error TS8008: Type aliases can only be used in TypeScript files.
+export.js(4,6): error TS1003: Identifier expected.
 
 
-==== export.js (1 errors) ====
+==== export.js (2 errors) ====
     export type foo = 5;
                 ~~~
 !!! error TS8008: Type aliases can only be used in TypeScript files.
     /**
      * @typedef {{
      * }}
+
+!!! error TS1003: Identifier expected.
      */
     export const foo = 5;