Skip to content

Commit fa8336e

Browse files
a-tarasyuka-tarasyuk
committed
fix(46402): create valid property keys/jsx attribute names
1 parent 2d4b243 commit fa8336e

9 files changed

+290
-41
lines changed

Diff for: ‎src/compiler/checker.ts

+3-33
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.");
@@ -6241,7 +6242,7 @@ namespace ts {
62416242
}
62426243
const rawName = unescapeLeadingUnderscores(symbol.escapedName);
62436244
const stringNamed = !!length(symbol.declarations) && every(symbol.declarations, isStringNamed);
6244-
return createPropertyNameNodeForIdentifierOrLiteral(rawName, stringNamed, singleQuote);
6245+
return createPropertyNameNodeForIdentifierOrLiteral(rawName, getEmitScriptTarget(compilerOptions), singleQuote, stringNamed);
62456246
}
62466247

62476248
// See getNameForSymbolFromNameType for a stringy equivalent
@@ -6256,20 +6257,14 @@ namespace ts {
62566257
if (isNumericLiteralName(name) && startsWith(name, "-")) {
62576258
return factory.createComputedPropertyName(factory.createNumericLiteral(+name));
62586259
}
6259-
return createPropertyNameNodeForIdentifierOrLiteral(name);
6260+
return createPropertyNameNodeForIdentifierOrLiteral(name, getEmitScriptTarget(compilerOptions));
62606261
}
62616262
if (nameType.flags & TypeFlags.UniqueESSymbol) {
62626263
return factory.createComputedPropertyName(symbolToExpression((nameType as UniqueESSymbolType).symbol, context, SymbolFlags.Value));
62636264
}
62646265
}
62656266
}
62666267

6267-
function createPropertyNameNodeForIdentifierOrLiteral(name: string, stringNamed?: boolean, singleQuote?: boolean) {
6268-
return isIdentifierText(name, getEmitScriptTarget(compilerOptions)) ? factory.createIdentifier(name) :
6269-
!stringNamed && isNumericLiteralName(name) && +name >= 0 ? factory.createNumericLiteral(+name) :
6270-
factory.createStringLiteral(name, !!singleQuote);
6271-
}
6272-
62736268
function cloneNodeBuilderContext(context: NodeBuilderContext): NodeBuilderContext {
62746269
const initial: NodeBuilderContext = { ...context };
62756270
// Make type parameters created within this context not consume the name outside this context
@@ -26950,31 +26945,6 @@ namespace ts {
2695026945
return isTypeAssignableToKind(checkComputedPropertyName(name), TypeFlags.NumberLike);
2695126946
}
2695226947

26953-
function isNumericLiteralName(name: string | __String) {
26954-
// The intent of numeric names is that
26955-
// - they are names with text in a numeric form, and that
26956-
// - setting properties/indexing with them is always equivalent to doing so with the numeric literal 'numLit',
26957-
// acquired by applying the abstract 'ToNumber' operation on the name's text.
26958-
//
26959-
// The subtlety is in the latter portion, as we cannot reliably say that anything that looks like a numeric literal is a numeric name.
26960-
// In fact, it is the case that the text of the name must be equal to 'ToString(numLit)' for this to hold.
26961-
//
26962-
// Consider the property name '"0xF00D"'. When one indexes with '0xF00D', they are actually indexing with the value of 'ToString(0xF00D)'
26963-
// according to the ECMAScript specification, so it is actually as if the user indexed with the string '"61453"'.
26964-
// Thus, the text of all numeric literals equivalent to '61543' such as '0xF00D', '0xf00D', '0170015', etc. are not valid numeric names
26965-
// because their 'ToString' representation is not equal to their original text.
26966-
// This is motivated by ECMA-262 sections 9.3.1, 9.8.1, 11.1.5, and 11.2.1.
26967-
//
26968-
// Here, we test whether 'ToString(ToNumber(name))' is exactly equal to 'name'.
26969-
// The '+' prefix operator is equivalent here to applying the abstract ToNumber operation.
26970-
// Applying the 'toString()' method on a number gives us the abstract ToString operation on a number.
26971-
//
26972-
// Note that this accepts the values 'Infinity', '-Infinity', and 'NaN', and that this is intentional.
26973-
// This is desired behavior, because when indexing with them as numeric entities, you are indexing
26974-
// with the strings '"Infinity"', '"-Infinity"', and '"NaN"' respectively.
26975-
return (+name).toString() === name;
26976-
}
26977-
2697826948
function checkComputedPropertyName(node: ComputedPropertyName): Type {
2697926949
const links = getNodeLinks(node.expression);
2698026950
if (!links.resolvedType) {

Diff for: ‎src/compiler/types.ts

+1
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;

Diff for: ‎src/compiler/utilities.ts

+32-1
Original file line numberDiff line numberDiff line change
@@ -7441,6 +7441,37 @@ namespace ts {
74417441
}
74427442

74437443
export function escapeSnippetText(text: string): string {
7444-
return text.replace(/\$/gm, "\\$");
7444+
return text.replace(/\$/gm, () => "\\$");
7445+
}
7446+
7447+
export function isNumericLiteralName(name: string | __String) {
7448+
// The intent of numeric names is that
7449+
// - they are names with text in a numeric form, and that
7450+
// - setting properties/indexing with them is always equivalent to doing so with the numeric literal 'numLit',
7451+
// acquired by applying the abstract 'ToNumber' operation on the name's text.
7452+
//
7453+
// The subtlety is in the latter portion, as we cannot reliably say that anything that looks like a numeric literal is a numeric name.
7454+
// In fact, it is the case that the text of the name must be equal to 'ToString(numLit)' for this to hold.
7455+
//
7456+
// Consider the property name '"0xF00D"'. When one indexes with '0xF00D', they are actually indexing with the value of 'ToString(0xF00D)'
7457+
// according to the ECMAScript specification, so it is actually as if the user indexed with the string '"61453"'.
7458+
// Thus, the text of all numeric literals equivalent to '61543' such as '0xF00D', '0xf00D', '0170015', etc. are not valid numeric names
7459+
// because their 'ToString' representation is not equal to their original text.
7460+
// This is motivated by ECMA-262 sections 9.3.1, 9.8.1, 11.1.5, and 11.2.1.
7461+
//
7462+
// Here, we test whether 'ToString(ToNumber(name))' is exactly equal to 'name'.
7463+
// The '+' prefix operator is equivalent here to applying the abstract ToNumber operation.
7464+
// Applying the 'toString()' method on a number gives us the abstract ToString operation on a number.
7465+
//
7466+
// Note that this accepts the values 'Infinity', '-Infinity', and 'NaN', and that this is intentional.
7467+
// This is desired behavior, because when indexing with them as numeric entities, you are indexing
7468+
// with the strings '"Infinity"', '"-Infinity"', and '"NaN"' respectively.
7469+
return (+name).toString() === name;
7470+
}
7471+
7472+
export function createPropertyNameNodeForIdentifierOrLiteral(name: string, target: ScriptTarget, singleQuote?: boolean, stringNamed?: boolean) {
7473+
return isIdentifierText(name, target) ? factory.createIdentifier(name) :
7474+
!stringNamed && isNumericLiteralName(name) && +name >= 0 ? factory.createNumericLiteral(+name) :
7475+
factory.createStringLiteral(name, !!singleQuote);
74457476
}
74467477
}

Diff for: ‎src/services/codefixes/fixAddMissingMember.ts

+13-7
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
}

Diff for: ‎tests/cases/fourslash/codeFixAddMissingAttributes10.ts

+18
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+

Diff for: ‎tests/cases/fourslash/codeFixAddMissingAttributes8.ts

+20
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+
});

Diff for: ‎tests/cases/fourslash/codeFixAddMissingAttributes9.ts

+20
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+
});

Diff for: ‎tests/cases/fourslash/codeFixAddMissingProperties15.ts

+17
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)