Skip to content

Commit 6fc0584

Browse files
a-tarasyuka-tarasyuk
authored
fix(46402): create valid property keys/jsx attribute names (microsoft#46716)
1 parent 484f141 commit 6fc0584

9 files changed

+290
-41
lines changed

‎src/compiler/checker.ts

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ namespace ts {
410410
const location = getParseTreeNode(locationIn);
411411
return location ? getTypeOfSymbolAtLocation(symbol, location) : errorType;
412412
},
413+
getTypeOfSymbol,
413414
getSymbolsOfParameterPropertyDeclaration: (parameterIn, parameterName) => {
414415
const parameter = getParseTreeNode(parameterIn, isParameter);
415416
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 {
62616262
}
62626263
const rawName = unescapeLeadingUnderscores(symbol.escapedName);
62636264
const stringNamed = !!length(symbol.declarations) && every(symbol.declarations, isStringNamed);
6264-
return createPropertyNameNodeForIdentifierOrLiteral(rawName, stringNamed, singleQuote);
6265+
return createPropertyNameNodeForIdentifierOrLiteral(rawName, getEmitScriptTarget(compilerOptions), singleQuote, stringNamed);
62656266
}
62666267

62676268
// See getNameForSymbolFromNameType for a stringy equivalent
@@ -6276,20 +6277,14 @@ namespace ts {
62766277
if (isNumericLiteralName(name) && startsWith(name, "-")) {
62776278
return factory.createComputedPropertyName(factory.createNumericLiteral(+name));
62786279
}
6279-
return createPropertyNameNodeForIdentifierOrLiteral(name);
6280+
return createPropertyNameNodeForIdentifierOrLiteral(name, getEmitScriptTarget(compilerOptions));
62806281
}
62816282
if (nameType.flags & TypeFlags.UniqueESSymbol) {
62826283
return factory.createComputedPropertyName(symbolToExpression((nameType as UniqueESSymbolType).symbol, context, SymbolFlags.Value));
62836284
}
62846285
}
62856286
}
62866287

6287-
function createPropertyNameNodeForIdentifierOrLiteral(name: string, stringNamed?: boolean, singleQuote?: boolean) {
6288-
return isIdentifierText(name, getEmitScriptTarget(compilerOptions)) ? factory.createIdentifier(name) :
6289-
!stringNamed && isNumericLiteralName(name) && +name >= 0 ? factory.createNumericLiteral(+name) :
6290-
factory.createStringLiteral(name, !!singleQuote);
6291-
}
6292-
62936288
function cloneNodeBuilderContext(context: NodeBuilderContext): NodeBuilderContext {
62946289
const initial: NodeBuilderContext = { ...context };
62956290
// Make type parameters created within this context not consume the name outside this context
@@ -27050,31 +27045,6 @@ namespace ts {
2705027045
return isTypeAssignableToKind(checkComputedPropertyName(name), TypeFlags.NumberLike);
2705127046
}
2705227047

27053-
function isNumericLiteralName(name: string | __String) {
27054-
// The intent of numeric names is that
27055-
// - they are names with text in a numeric form, and that
27056-
// - setting properties/indexing with them is always equivalent to doing so with the numeric literal 'numLit',
27057-
// acquired by applying the abstract 'ToNumber' operation on the name's text.
27058-
//
27059-
// The subtlety is in the latter portion, as we cannot reliably say that anything that looks like a numeric literal is a numeric name.
27060-
// In fact, it is the case that the text of the name must be equal to 'ToString(numLit)' for this to hold.
27061-
//
27062-
// Consider the property name '"0xF00D"'. When one indexes with '0xF00D', they are actually indexing with the value of 'ToString(0xF00D)'
27063-
// according to the ECMAScript specification, so it is actually as if the user indexed with the string '"61453"'.
27064-
// Thus, the text of all numeric literals equivalent to '61543' such as '0xF00D', '0xf00D', '0170015', etc. are not valid numeric names
27065-
// because their 'ToString' representation is not equal to their original text.
27066-
// This is motivated by ECMA-262 sections 9.3.1, 9.8.1, 11.1.5, and 11.2.1.
27067-
//
27068-
// Here, we test whether 'ToString(ToNumber(name))' is exactly equal to 'name'.
27069-
// The '+' prefix operator is equivalent here to applying the abstract ToNumber operation.
27070-
// Applying the 'toString()' method on a number gives us the abstract ToString operation on a number.
27071-
//
27072-
// Note that this accepts the values 'Infinity', '-Infinity', and 'NaN', and that this is intentional.
27073-
// This is desired behavior, because when indexing with them as numeric entities, you are indexing
27074-
// with the strings '"Infinity"', '"-Infinity"', and '"NaN"' respectively.
27075-
return (+name).toString() === name;
27076-
}
27077-
2707827048
function checkComputedPropertyName(node: ComputedPropertyName): Type {
2707927049
const links = getNodeLinks(node.expression);
2708027050
if (!links.resolvedType) {

‎src/compiler/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4153,6 +4153,7 @@ namespace ts {
41534153

41544154
export interface TypeChecker {
41554155
getTypeOfSymbolAtLocation(symbol: Symbol, node: Node): Type;
4156+
/* @internal */ getTypeOfSymbol(symbol: Symbol): Type;
41564157
getDeclaredTypeOfSymbol(symbol: Symbol): Type;
41574158
getPropertiesOfType(type: Type): Symbol[];
41584159
getPropertyOfType(type: Type, propertyName: string): Symbol | undefined;

‎src/compiler/utilities.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7413,7 +7413,38 @@ namespace ts {
74137413
}
74147414

74157415
export function escapeSnippetText(text: string): string {
7416-
return text.replace(/\$/gm, "\\$");
7416+
return text.replace(/\$/gm, () => "\\$");
7417+
}
7418+
7419+
export function isNumericLiteralName(name: string | __String) {
7420+
// The intent of numeric names is that
7421+
// - they are names with text in a numeric form, and that
7422+
// - setting properties/indexing with them is always equivalent to doing so with the numeric literal 'numLit',
7423+
// acquired by applying the abstract 'ToNumber' operation on the name's text.
7424+
//
7425+
// The subtlety is in the latter portion, as we cannot reliably say that anything that looks like a numeric literal is a numeric name.
7426+
// In fact, it is the case that the text of the name must be equal to 'ToString(numLit)' for this to hold.
7427+
//
7428+
// Consider the property name '"0xF00D"'. When one indexes with '0xF00D', they are actually indexing with the value of 'ToString(0xF00D)'
7429+
// according to the ECMAScript specification, so it is actually as if the user indexed with the string '"61453"'.
7430+
// Thus, the text of all numeric literals equivalent to '61543' such as '0xF00D', '0xf00D', '0170015', etc. are not valid numeric names
7431+
// because their 'ToString' representation is not equal to their original text.
7432+
// This is motivated by ECMA-262 sections 9.3.1, 9.8.1, 11.1.5, and 11.2.1.
7433+
//
7434+
// Here, we test whether 'ToString(ToNumber(name))' is exactly equal to 'name'.
7435+
// The '+' prefix operator is equivalent here to applying the abstract ToNumber operation.
7436+
// Applying the 'toString()' method on a number gives us the abstract ToString operation on a number.
7437+
//
7438+
// Note that this accepts the values 'Infinity', '-Infinity', and 'NaN', and that this is intentional.
7439+
// This is desired behavior, because when indexing with them as numeric entities, you are indexing
7440+
// with the strings '"Infinity"', '"-Infinity"', and '"NaN"' respectively.
7441+
return (+name).toString() === name;
7442+
}
7443+
7444+
export function createPropertyNameNodeForIdentifierOrLiteral(name: string, target: ScriptTarget, singleQuote?: boolean, stringNamed?: boolean) {
7445+
return isIdentifierText(name, target) ? factory.createIdentifier(name) :
7446+
!stringNamed && isNumericLiteralName(name) && +name >= 0 ? factory.createNumericLiteral(+name) :
7447+
factory.createStringLiteral(name, !!singleQuote);
74177448
}
74187449

74197450
export function isThisTypeParameter(type: Type): boolean {

‎src/services/codefixes/fixAddMissingMember.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,8 @@ namespace ts.codefix {
183183
}
184184

185185
if (isIdentifier(token) && isJsxOpeningLikeElement(token.parent)) {
186-
const attributes = getUnmatchedAttributes(checker, token.parent);
186+
const target = getEmitScriptTarget(program.getCompilerOptions());
187+
const attributes = getUnmatchedAttributes(checker, target, token.parent);
187188
if (!length(attributes)) return undefined;
188189
return { kind: InfoKind.JsxAttributes, token, attributes, parentDeclaration: token.parent };
189190
}
@@ -465,8 +466,12 @@ namespace ts.codefix {
465466
const jsxAttributesNode = info.parentDeclaration.attributes;
466467
const hasSpreadAttribute = some(jsxAttributesNode.properties, isJsxSpreadAttribute);
467468
const attrs = map(info.attributes, attr => {
468-
const value = attr.valueDeclaration ? tryGetValueFromType(context, checker, importAdder, quotePreference, checker.getTypeAtLocation(attr.valueDeclaration)) : createUndefined();
469-
return factory.createJsxAttribute(factory.createIdentifier(attr.name), factory.createJsxExpression(/*dotDotDotToken*/ undefined, value));
469+
const value = tryGetValueFromType(context, checker, importAdder, quotePreference, checker.getTypeOfSymbol(attr));
470+
const name = factory.createIdentifier(attr.name);
471+
const jsxAttribute = factory.createJsxAttribute(name, factory.createJsxExpression(/*dotDotDotToken*/ undefined, value));
472+
// formattingScanner requires the Identifier to have a context for scanning attributes with "-" (data-foo).
473+
setParent(name, jsxAttribute);
474+
return jsxAttribute;
470475
});
471476
const jsxAttributes = factory.createJsxAttributes(hasSpreadAttribute ? [...attrs, ...jsxAttributesNode.properties] : [...jsxAttributesNode.properties, ...attrs]);
472477
const options = { prefix: jsxAttributesNode.pos === jsxAttributesNode.end ? " " : undefined };
@@ -476,10 +481,11 @@ namespace ts.codefix {
476481
function addObjectLiteralProperties(changes: textChanges.ChangeTracker, context: CodeFixContextBase, info: ObjectLiteralInfo) {
477482
const importAdder = createImportAdder(context.sourceFile, context.program, context.preferences, context.host);
478483
const quotePreference = getQuotePreference(context.sourceFile, context.preferences);
484+
const target = getEmitScriptTarget(context.program.getCompilerOptions());
479485
const checker = context.program.getTypeChecker();
480486
const props = map(info.properties, prop => {
481-
const initializer = prop.valueDeclaration ? tryGetValueFromType(context, checker, importAdder, quotePreference, checker.getTypeAtLocation(prop.valueDeclaration)) : createUndefined();
482-
return factory.createPropertyAssignment(prop.name, initializer);
487+
const initializer = tryGetValueFromType(context, checker, importAdder, quotePreference, checker.getTypeOfSymbol(prop));
488+
return factory.createPropertyAssignment(createPropertyNameNodeForIdentifierOrLiteral(prop.name, target, quotePreference === QuotePreference.Single), initializer);
483489
});
484490
const options = {
485491
leadingTriviaOption: textChanges.LeadingTriviaOption.Exclude,
@@ -571,7 +577,7 @@ namespace ts.codefix {
571577
((getObjectFlags(type) & ObjectFlags.ObjectLiteral) || (type.symbol && tryCast(singleOrUndefined(type.symbol.declarations), isTypeLiteralNode)));
572578
}
573579

574-
function getUnmatchedAttributes(checker: TypeChecker, source: JsxOpeningLikeElement) {
580+
function getUnmatchedAttributes(checker: TypeChecker, target: ScriptTarget, source: JsxOpeningLikeElement) {
575581
const attrsType = checker.getContextualType(source.attributes);
576582
if (attrsType === undefined) return emptyArray;
577583

@@ -591,6 +597,6 @@ namespace ts.codefix {
591597
}
592598
}
593599
return filter(targetProps, targetProp =>
594-
!((targetProp.flags & SymbolFlags.Optional || getCheckFlags(targetProp) & CheckFlags.Partial) || seenNames.has(targetProp.escapedName)));
600+
isIdentifierText(targetProp.name, target, LanguageVariant.JSX) && !((targetProp.flags & SymbolFlags.Optional || getCheckFlags(targetProp) & CheckFlags.Partial) || seenNames.has(targetProp.escapedName)));
595601
}
596602
}

‎tests/cases/fourslash/codeFixAddMissingAttributes10.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @jsx: preserve
4+
// @filename: foo.tsx
5+
6+
////type A = 'a' | 'b' | 'c' | 'd' | 'e';
7+
////type B = 1 | 2 | 3;
8+
////type C = '@' | '!';
9+
////type D = `${A}${Uppercase<A>}${B}${C}`;
10+
11+
////const A = (props: { [K in D]: K }) =>
12+
//// <div {...props}></div>;
13+
////
14+
////const Bar = () =>
15+
//// [|<A></A>|]
16+
17+
verify.not.codeFixAvailable("fixMissingAttributes");
18+

‎tests/cases/fourslash/codeFixAddMissingAttributes8.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @jsx: preserve
4+
// @filename: foo.tsx
5+
6+
////type A = 'a' | 'b';
7+
////type B = 'd' | 'c';
8+
////type C = `${A}${B}`;
9+
10+
////const A = (props: { [K in C]: K }) =>
11+
//// <div {...props}></div>;
12+
////
13+
////const Bar = () =>
14+
//// [|<A></A>|]
15+
16+
verify.codeFix({
17+
index: 0,
18+
description: ts.Diagnostics.Add_missing_attributes.message,
19+
newRangeContent: `<A ad={"ad"} ac={"ac"} bd={"bd"} bc={"bc"}></A>`
20+
});

‎tests/cases/fourslash/codeFixAddMissingAttributes9.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @jsx: preserve
4+
// @filename: foo.tsx
5+
6+
////type A = 'a-' | 'b-';
7+
////type B = 'd' | 'c';
8+
////type C = `${A}${B}`;
9+
10+
////const A = (props: { [K in C]: K }) =>
11+
//// <div {...props}></div>;
12+
////
13+
////const Bar = () =>
14+
//// [|<A></A>|]
15+
16+
verify.codeFix({
17+
index: 0,
18+
description: ts.Diagnostics.Add_missing_attributes.message,
19+
newRangeContent: `<A a-d={"a-d"} a-c={"a-c"} b-d={"b-d"} b-c={"b-c"}></A>`
20+
});

‎tests/cases/fourslash/codeFixAddMissingProperties15.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////interface Foo {
4+
//// 1: number;
5+
//// 2: number;
6+
////}
7+
////[|const foo: Foo = {}|]
8+
9+
verify.codeFix({
10+
index: 0,
11+
description: ts.Diagnostics.Add_missing_properties.message,
12+
newRangeContent:
13+
`const foo: Foo = {
14+
1: 0,
15+
2: 0
16+
}`
17+
});

0 commit comments

Comments
 (0)