fix(46402): create valid property keys/jsx attribute names (microsoft#46716)

Author: a-tarasyuk

src/compiler/checker.ts

@@ -410,6 +410,7 @@ namespace ts {
                 const location = getParseTreeNode(locationIn);
                 return location ? getTypeOfSymbolAtLocation(symbol, location) : errorType;
             },
+            getTypeOfSymbol,
             getSymbolsOfParameterPropertyDeclaration: (parameterIn, parameterName) => {
                 const parameter = getParseTreeNode(parameterIn, isParameter);
                 if (parameter === undefined) return Debug.fail("Cannot get symbols of a synthetic parameter that cannot be resolved to a parse-tree node.");
@@ -6261,7 +6262,7 @@ namespace ts {
                 }
                 const rawName = unescapeLeadingUnderscores(symbol.escapedName);
                 const stringNamed = !!length(symbol.declarations) && every(symbol.declarations, isStringNamed);
-                return createPropertyNameNodeForIdentifierOrLiteral(rawName, stringNamed, singleQuote);
+                return createPropertyNameNodeForIdentifierOrLiteral(rawName, getEmitScriptTarget(compilerOptions), singleQuote, stringNamed);
             }
 
             // See getNameForSymbolFromNameType for a stringy equivalent
@@ -6276,20 +6277,14 @@ namespace ts {
                         if (isNumericLiteralName(name) && startsWith(name, "-")) {
                             return factory.createComputedPropertyName(factory.createNumericLiteral(+name));
                         }
-                        return createPropertyNameNodeForIdentifierOrLiteral(name);
+                        return createPropertyNameNodeForIdentifierOrLiteral(name, getEmitScriptTarget(compilerOptions));
                     }
                     if (nameType.flags & TypeFlags.UniqueESSymbol) {
                         return factory.createComputedPropertyName(symbolToExpression((nameType as UniqueESSymbolType).symbol, context, SymbolFlags.Value));
                     }
                 }
             }
 
-            function createPropertyNameNodeForIdentifierOrLiteral(name: string, stringNamed?: boolean, singleQuote?: boolean) {
-                return isIdentifierText(name, getEmitScriptTarget(compilerOptions)) ? factory.createIdentifier(name) :
-                    !stringNamed && isNumericLiteralName(name) && +name >= 0 ? factory.createNumericLiteral(+name) :
-                    factory.createStringLiteral(name, !!singleQuote);
-            }
-
             function cloneNodeBuilderContext(context: NodeBuilderContext): NodeBuilderContext {
                 const initial: NodeBuilderContext = { ...context };
                 // Make type parameters created within this context not consume the name outside this context
@@ -27050,31 +27045,6 @@ namespace ts {
             return isTypeAssignableToKind(checkComputedPropertyName(name), TypeFlags.NumberLike);
         }
 
-        function isNumericLiteralName(name: string | __String) {
-            // The intent of numeric names is that
-            //     - they are names with text in a numeric form, and that
-            //     - setting properties/indexing with them is always equivalent to doing so with the numeric literal 'numLit',
-            //         acquired by applying the abstract 'ToNumber' operation on the name's text.
-            //
-            // The subtlety is in the latter portion, as we cannot reliably say that anything that looks like a numeric literal is a numeric name.
-            // In fact, it is the case that the text of the name must be equal to 'ToString(numLit)' for this to hold.
-            //
-            // Consider the property name '"0xF00D"'. When one indexes with '0xF00D', they are actually indexing with the value of 'ToString(0xF00D)'
-            // according to the ECMAScript specification, so it is actually as if the user indexed with the string '"61453"'.
-            // Thus, the text of all numeric literals equivalent to '61543' such as '0xF00D', '0xf00D', '0170015', etc. are not valid numeric names
-            // because their 'ToString' representation is not equal to their original text.
-            // This is motivated by ECMA-262 sections 9.3.1, 9.8.1, 11.1.5, and 11.2.1.
-            //
-            // Here, we test whether 'ToString(ToNumber(name))' is exactly equal to 'name'.
-            // The '+' prefix operator is equivalent here to applying the abstract ToNumber operation.
-            // Applying the 'toString()' method on a number gives us the abstract ToString operation on a number.
-            //
-            // Note that this accepts the values 'Infinity', '-Infinity', and 'NaN', and that this is intentional.
-            // This is desired behavior, because when indexing with them as numeric entities, you are indexing
-            // with the strings '"Infinity"', '"-Infinity"', and '"NaN"' respectively.
-            return (+name).toString() === name;
-        }
-
         function checkComputedPropertyName(node: ComputedPropertyName): Type {
             const links = getNodeLinks(node.expression);
             if (!links.resolvedType) {

src/compiler/types.ts

@@ -4153,6 +4153,7 @@ namespace ts {
 
     export interface TypeChecker {
         getTypeOfSymbolAtLocation(symbol: Symbol, node: Node): Type;
+        /* @internal */ getTypeOfSymbol(symbol: Symbol): Type;
         getDeclaredTypeOfSymbol(symbol: Symbol): Type;
         getPropertiesOfType(type: Type): Symbol[];
         getPropertyOfType(type: Type, propertyName: string): Symbol | undefined;

src/compiler/utilities.ts

@@ -7413,7 +7413,38 @@ namespace ts {
     }
 
     export function escapeSnippetText(text: string): string {
-        return text.replace(/\$/gm, "\\$");
+        return text.replace(/\$/gm, () => "\\$");
+    }
+
+    export function isNumericLiteralName(name: string | __String) {
+        // The intent of numeric names is that
+        //     - they are names with text in a numeric form, and that
+        //     - setting properties/indexing with them is always equivalent to doing so with the numeric literal 'numLit',
+        //         acquired by applying the abstract 'ToNumber' operation on the name's text.
+        //
+        // The subtlety is in the latter portion, as we cannot reliably say that anything that looks like a numeric literal is a numeric name.
+        // In fact, it is the case that the text of the name must be equal to 'ToString(numLit)' for this to hold.
+        //
+        // Consider the property name '"0xF00D"'. When one indexes with '0xF00D', they are actually indexing with the value of 'ToString(0xF00D)'
+        // according to the ECMAScript specification, so it is actually as if the user indexed with the string '"61453"'.
+        // Thus, the text of all numeric literals equivalent to '61543' such as '0xF00D', '0xf00D', '0170015', etc. are not valid numeric names
+        // because their 'ToString' representation is not equal to their original text.
+        // This is motivated by ECMA-262 sections 9.3.1, 9.8.1, 11.1.5, and 11.2.1.
+        //
+        // Here, we test whether 'ToString(ToNumber(name))' is exactly equal to 'name'.
+        // The '+' prefix operator is equivalent here to applying the abstract ToNumber operation.
+        // Applying the 'toString()' method on a number gives us the abstract ToString operation on a number.
+        //
+        // Note that this accepts the values 'Infinity', '-Infinity', and 'NaN', and that this is intentional.
+        // This is desired behavior, because when indexing with them as numeric entities, you are indexing
+        // with the strings '"Infinity"', '"-Infinity"', and '"NaN"' respectively.
+        return (+name).toString() === name;
+    }
+
+    export function createPropertyNameNodeForIdentifierOrLiteral(name: string, target: ScriptTarget, singleQuote?: boolean, stringNamed?: boolean) {
+        return isIdentifierText(name, target) ? factory.createIdentifier(name) :
+            !stringNamed && isNumericLiteralName(name) && +name >= 0 ? factory.createNumericLiteral(+name) :
+            factory.createStringLiteral(name, !!singleQuote);
     }
 
     export function isThisTypeParameter(type: Type): boolean {

src/services/codefixes/fixAddMissingMember.ts

@@ -183,7 +183,8 @@ namespace ts.codefix {
         }
 
         if (isIdentifier(token) && isJsxOpeningLikeElement(token.parent)) {
-            const attributes = getUnmatchedAttributes(checker, token.parent);
+            const target = getEmitScriptTarget(program.getCompilerOptions());
+            const attributes = getUnmatchedAttributes(checker, target, token.parent);
             if (!length(attributes)) return undefined;
             return { kind: InfoKind.JsxAttributes, token, attributes, parentDeclaration: token.parent };
         }
@@ -465,8 +466,12 @@ namespace ts.codefix {
         const jsxAttributesNode = info.parentDeclaration.attributes;
         const hasSpreadAttribute = some(jsxAttributesNode.properties, isJsxSpreadAttribute);
         const attrs = map(info.attributes, attr => {
-            const value = attr.valueDeclaration ? tryGetValueFromType(context, checker, importAdder, quotePreference, checker.getTypeAtLocation(attr.valueDeclaration)) : createUndefined();
-            return factory.createJsxAttribute(factory.createIdentifier(attr.name), factory.createJsxExpression(/*dotDotDotToken*/ undefined, value));
+            const value = tryGetValueFromType(context, checker, importAdder, quotePreference, checker.getTypeOfSymbol(attr));
+            const name = factory.createIdentifier(attr.name);
+            const jsxAttribute = factory.createJsxAttribute(name, factory.createJsxExpression(/*dotDotDotToken*/ undefined, value));
+            // formattingScanner requires the Identifier to have a context for scanning attributes with "-" (data-foo).
+            setParent(name, jsxAttribute);
+            return jsxAttribute;
         });
         const jsxAttributes = factory.createJsxAttributes(hasSpreadAttribute ? [...attrs, ...jsxAttributesNode.properties] : [...jsxAttributesNode.properties, ...attrs]);
         const options = { prefix: jsxAttributesNode.pos === jsxAttributesNode.end ? " " : undefined };
@@ -476,10 +481,11 @@ namespace ts.codefix {
     function addObjectLiteralProperties(changes: textChanges.ChangeTracker, context: CodeFixContextBase, info: ObjectLiteralInfo) {
         const importAdder = createImportAdder(context.sourceFile, context.program, context.preferences, context.host);
         const quotePreference = getQuotePreference(context.sourceFile, context.preferences);
+        const target = getEmitScriptTarget(context.program.getCompilerOptions());
         const checker = context.program.getTypeChecker();
         const props = map(info.properties, prop => {
-            const initializer = prop.valueDeclaration ? tryGetValueFromType(context, checker, importAdder, quotePreference, checker.getTypeAtLocation(prop.valueDeclaration)) : createUndefined();
-            return factory.createPropertyAssignment(prop.name, initializer);
+            const initializer = tryGetValueFromType(context, checker, importAdder, quotePreference, checker.getTypeOfSymbol(prop));
+            return factory.createPropertyAssignment(createPropertyNameNodeForIdentifierOrLiteral(prop.name, target, quotePreference === QuotePreference.Single), initializer);
         });
         const options = {
             leadingTriviaOption: textChanges.LeadingTriviaOption.Exclude,
@@ -571,7 +577,7 @@ namespace ts.codefix {
             ((getObjectFlags(type) & ObjectFlags.ObjectLiteral) || (type.symbol && tryCast(singleOrUndefined(type.symbol.declarations), isTypeLiteralNode)));
     }
 
-    function getUnmatchedAttributes(checker: TypeChecker, source: JsxOpeningLikeElement) {
+    function getUnmatchedAttributes(checker: TypeChecker, target: ScriptTarget, source: JsxOpeningLikeElement) {
         const attrsType = checker.getContextualType(source.attributes);
         if (attrsType === undefined) return emptyArray;
 
@@ -591,6 +597,6 @@ namespace ts.codefix {
             }
         }
         return filter(targetProps, targetProp =>
-            !((targetProp.flags & SymbolFlags.Optional || getCheckFlags(targetProp) & CheckFlags.Partial) || seenNames.has(targetProp.escapedName)));
+            isIdentifierText(targetProp.name, target, LanguageVariant.JSX) && !((targetProp.flags & SymbolFlags.Optional || getCheckFlags(targetProp) & CheckFlags.Partial) || seenNames.has(targetProp.escapedName)));
     }
 }

tests/cases/fourslash/codeFixAddMissingAttributes10.ts

@@ -0,0 +1,18 @@
+/// <reference path='fourslash.ts' />
+
+// @jsx: preserve
+// @filename: foo.tsx
+
+////type A = 'a' | 'b' | 'c' | 'd' | 'e';
+////type B = 1 | 2 | 3;
+////type C = '@' | '!';
+////type D = `${A}${Uppercase<A>}${B}${C}`;
+
+////const A = (props: { [K in D]: K }) =>
+////   <div {...props}></div>;
+////
+////const Bar = () =>
+////   [|<A></A>|]
+
+verify.not.codeFixAvailable("fixMissingAttributes");
+

tests/cases/fourslash/codeFixAddMissingAttributes8.ts

@@ -0,0 +1,20 @@
+/// <reference path='fourslash.ts' />
+
+// @jsx: preserve
+// @filename: foo.tsx
+
+////type A = 'a' | 'b';
+////type B = 'd' | 'c';
+////type C = `${A}${B}`;
+
+////const A = (props: { [K in C]: K }) =>
+////   <div {...props}></div>;
+////
+////const Bar = () =>
+////   [|<A></A>|]
+
+verify.codeFix({
+    index: 0,
+    description: ts.Diagnostics.Add_missing_attributes.message,
+    newRangeContent: `<A ad={"ad"} ac={"ac"} bd={"bd"} bc={"bc"}></A>`
+});

tests/cases/fourslash/codeFixAddMissingAttributes9.ts

@@ -0,0 +1,20 @@
+/// <reference path='fourslash.ts' />
+
+// @jsx: preserve
+// @filename: foo.tsx
+
+////type A = 'a-' | 'b-';
+////type B = 'd' | 'c';
+////type C = `${A}${B}`;
+
+////const A = (props: { [K in C]: K }) =>
+////   <div {...props}></div>;
+////
+////const Bar = () =>
+////   [|<A></A>|]
+
+verify.codeFix({
+    index: 0,
+    description: ts.Diagnostics.Add_missing_attributes.message,
+    newRangeContent: `<A a-d={"a-d"} a-c={"a-c"} b-d={"b-d"} b-c={"b-c"}></A>`
+});

tests/cases/fourslash/codeFixAddMissingProperties15.ts

@@ -0,0 +1,17 @@
+/// <reference path='fourslash.ts' />
+
+////interface Foo {
+////    1: number;
+////    2: number;
+////}
+////[|const foo: Foo = {}|]
+
+verify.codeFix({
+    index: 0,
+    description: ts.Diagnostics.Add_missing_properties.message,
+    newRangeContent:
+`const foo: Foo = {
+    1: 0,
+    2: 0
+}`
+});