Fix crash in contextual typing of defaulted element access declarations (#33686)

weswigham · web-flow · commit 9a7fdade0788 · 2019-09-30T14:42:55.000-07:00
diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -2966,7 +2966,7 @@ namespace ts {
             }
             else {
                 const symbol = lookupSymbolForPropertyAccess(node.expression);
-                return symbol && symbol.exports && symbol.exports.get(escapeLeadingUnderscores(getElementOrPropertyAccessName(node)));
+                return symbol && symbol.exports && symbol.exports.get(getElementOrPropertyAccessName(node));
             }
         }
 
@@ -2979,7 +2979,7 @@ namespace ts {
             }
             else {
                 const s = forEachIdentifierInEntityName(e.expression, parent, action);
-                return action(getNameOrArgument(e), s && s.exports && s.exports.get(escapeLeadingUnderscores(getElementOrPropertyAccessName(e))), s);
+                return action(getNameOrArgument(e), s && s.exports && s.exports.get(getElementOrPropertyAccessName(e)), s);
             }
         }
 
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -20940,7 +20940,7 @@ namespace ts {
                         if (!decl) {
                             return false;
                         }
-                        const lhs = binaryExpression.left as PropertyAccessExpression;
+                        const lhs = cast(binaryExpression.left, isAccessExpression);
                         const overallAnnotation = getEffectiveTypeAnnotationNode(decl);
                         if (overallAnnotation) {
                             return getTypeFromTypeNode(overallAnnotation);
@@ -20951,8 +20951,11 @@ namespace ts {
                             if (parentSymbol) {
                                 const annotated = getEffectiveTypeAnnotationNode(parentSymbol.valueDeclaration);
                                 if (annotated) {
-                                    const type = getTypeOfPropertyOfContextualType(getTypeFromTypeNode(annotated), lhs.name.escapedText);
-                                    return type || false;
+                                    const nameStr = getElementOrPropertyAccessName(lhs);
+                                    if (nameStr !== undefined) {
+                                        const type = getTypeOfPropertyOfContextualType(getTypeFromTypeNode(annotated), nameStr);
+                                        return type || false;
+                                    }
                                 }
                                 return false;
                             }
@@ -20972,12 +20975,13 @@ namespace ts {
                         }
                     }
                     if (kind === AssignmentDeclarationKind.ModuleExports) return false;
-                    const thisAccess = binaryExpression.left as PropertyAccessExpression;
+                    const thisAccess = cast(binaryExpression.left, isAccessExpression);
                     if (!isObjectLiteralMethod(getThisContainer(thisAccess.expression, /*includeArrowFunctions*/ false))) {
                         return false;
                     }
                     const thisType = checkThisExpression(thisAccess.expression);
-                    return thisType && getTypeOfPropertyOfContextualType(thisType, thisAccess.name.escapedText) || false;
+                    const nameStr = getElementOrPropertyAccessName(thisAccess);
+                    return nameStr !== undefined && thisType && getTypeOfPropertyOfContextualType(thisType, nameStr) || false;
                 case AssignmentDeclarationKind.ObjectDefinePropertyValue:
                 case AssignmentDeclarationKind.ObjectDefinePropertyExports:
                 case AssignmentDeclarationKind.ObjectDefinePrototypeProperty:
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -2128,18 +2128,21 @@ namespace ts {
     }
 
     /* @internal */
-    export function getElementOrPropertyAccessName(node: LiteralLikeElementAccessExpression | PropertyAccessExpression): string;
-    export function getElementOrPropertyAccessName(node: AccessExpression): string | undefined;
-    export function getElementOrPropertyAccessName(node: AccessExpression): string | undefined {
+    export function getElementOrPropertyAccessName(node: LiteralLikeElementAccessExpression | PropertyAccessExpression): __String;
+    export function getElementOrPropertyAccessName(node: AccessExpression): __String | undefined;
+    export function getElementOrPropertyAccessName(node: AccessExpression): __String | undefined {
         const name = getElementOrPropertyAccessArgumentExpressionOrName(node);
         if (name) {
             if (isIdentifier(name)) {
-                return idText(name);
+                return name.escapedText;
             }
             if (isStringLiteralLike(name) || isNumericLiteral(name)) {
-                return name.text;
+                return escapeLeadingUnderscores(name.text);
             }
         }
+        if (isElementAccessExpression(node) && isWellKnownSymbolSyntactically(node.argumentExpression)) {
+            return getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>node.argumentExpression).name));
+        }
         return undefined;
     }
 
diff --git a/tests/baselines/reference/jsElementAccessNoContextualTypeCrash.errors.txt b/tests/baselines/reference/jsElementAccessNoContextualTypeCrash.errors.txt
@@ -0,0 +1,16 @@
+tests/cases/compiler/jsElementAccessNoContextualTypeCrash.js(2,1): error TS2741: Property 'localize' is missing in type '{}' but required in type 'typeof Common'.
+
+
+==== tests/cases/compiler/jsElementAccessNoContextualTypeCrash.js (1 errors) ====
+    var Common = {};
+    self['Common'] = self['Common'] || {};
+    ~~~~~~~~~~~~~~
+!!! error TS2741: Property 'localize' is missing in type '{}' but required in type 'typeof Common'.
+!!! related TS2728 tests/cases/compiler/jsElementAccessNoContextualTypeCrash.js:7:1: 'localize' is declared here.
+    /**
+     * @param {string} string
+     * @return {string}
+     */
+    Common.localize = function (string) {
+        return string;
+    };
diff --git a/tests/baselines/reference/jsElementAccessNoContextualTypeCrash.symbols b/tests/baselines/reference/jsElementAccessNoContextualTypeCrash.symbols
@@ -0,0 +1,24 @@
+=== tests/cases/compiler/jsElementAccessNoContextualTypeCrash.js ===
+var Common = {};
+>Common : Symbol(Common, Decl(jsElementAccessNoContextualTypeCrash.js, 0, 3), Decl(jsElementAccessNoContextualTypeCrash.js, 1, 38))
+
+self['Common'] = self['Common'] || {};
+>self : Symbol(self, Decl(lib.dom.d.ts, --, --), Decl(jsElementAccessNoContextualTypeCrash.js, 0, 16))
+>'Common' : Symbol(Common, Decl(jsElementAccessNoContextualTypeCrash.js, 0, 3), Decl(jsElementAccessNoContextualTypeCrash.js, 1, 38))
+>self : Symbol(self, Decl(lib.dom.d.ts, --, --), Decl(jsElementAccessNoContextualTypeCrash.js, 0, 16))
+>'Common' : Symbol(Common, Decl(jsElementAccessNoContextualTypeCrash.js, 0, 3), Decl(jsElementAccessNoContextualTypeCrash.js, 1, 38))
+
+/**
+ * @param {string} string
+ * @return {string}
+ */
+Common.localize = function (string) {
+>Common.localize : Symbol(Common.localize, Decl(jsElementAccessNoContextualTypeCrash.js, 1, 38))
+>Common : Symbol(Common, Decl(jsElementAccessNoContextualTypeCrash.js, 0, 3), Decl(jsElementAccessNoContextualTypeCrash.js, 1, 38))
+>localize : Symbol(Common.localize, Decl(jsElementAccessNoContextualTypeCrash.js, 1, 38))
+>string : Symbol(string, Decl(jsElementAccessNoContextualTypeCrash.js, 6, 28))
+
+    return string;
+>string : Symbol(string, Decl(jsElementAccessNoContextualTypeCrash.js, 6, 28))
+
+};
diff --git a/tests/baselines/reference/jsElementAccessNoContextualTypeCrash.types b/tests/baselines/reference/jsElementAccessNoContextualTypeCrash.types
@@ -0,0 +1,32 @@
+=== tests/cases/compiler/jsElementAccessNoContextualTypeCrash.js ===
+var Common = {};
+>Common : typeof Common
+>{} : {}
+
+self['Common'] = self['Common'] || {};
+>self['Common'] = self['Common'] || {} : {}
+>self['Common'] : typeof Common
+>self : Window & typeof globalThis
+>'Common' : "Common"
+>self['Common'] || {} : {}
+>self['Common'] : typeof Common
+>self : Window & typeof globalThis
+>'Common' : "Common"
+>{} : {}
+
+/**
+ * @param {string} string
+ * @return {string}
+ */
+Common.localize = function (string) {
+>Common.localize = function (string) {    return string;} : (string: string) => string
+>Common.localize : (string: string) => string
+>Common : typeof Common
+>localize : (string: string) => string
+>function (string) {    return string;} : (string: string) => string
+>string : string
+
+    return string;
+>string : string
+
+};
diff --git a/tests/cases/compiler/jsElementAccessNoContextualTypeCrash.ts b/tests/cases/compiler/jsElementAccessNoContextualTypeCrash.ts
@@ -0,0 +1,13 @@
+// @checkJs: true
+// @allowJs: true
+// @noEmit: true
+// @filename: jsElementAccessNoContextualTypeCrash.js
+var Common = {};
+self['Common'] = self['Common'] || {};
+/**
+ * @param {string} string
+ * @return {string}
+ */
+Common.localize = function (string) {
+    return string;
+};

Original file line number	Diff line number	Diff line change
`@@ -2966,7 +2966,7 @@ namespace ts {`
`2966`	`2966`	`}`
`2967`	`2967`	`else {`
`2968`	`2968`	`const symbol = lookupSymbolForPropertyAccess(node.expression);`
`2969`		`- return symbol && symbol.exports && symbol.exports.get(escapeLeadingUnderscores(getElementOrPropertyAccessName(node)));`
	`2969`	`+ return symbol && symbol.exports && symbol.exports.get(getElementOrPropertyAccessName(node));`
`2970`	`2970`	`}`
`2971`	`2971`	`}`
`2972`	`2972`
`@@ -2979,7 +2979,7 @@ namespace ts {`
`2979`	`2979`	`}`
`2980`	`2980`	`else {`
`2981`	`2981`	`const s = forEachIdentifierInEntityName(e.expression, parent, action);`
`2982`		`- return action(getNameOrArgument(e), s && s.exports && s.exports.get(escapeLeadingUnderscores(getElementOrPropertyAccessName(e))), s);`
	`2982`	`+ return action(getNameOrArgument(e), s && s.exports && s.exports.get(getElementOrPropertyAccessName(e)), s);`
`2983`	`2983`	`}`
`2984`	`2984`	`}`
`2985`	`2985`
Original file line number	Diff line number	Diff line change
`@@ -2128,18 +2128,21 @@ namespace ts {`
`2128`	`2128`	`}`
`2129`	`2129`
`2130`	`2130`	`/* @internal */`
`2131`		`- export function getElementOrPropertyAccessName(node: LiteralLikeElementAccessExpression \| PropertyAccessExpression): string;`
`2132`		`- export function getElementOrPropertyAccessName(node: AccessExpression): string \| undefined;`
`2133`		`- export function getElementOrPropertyAccessName(node: AccessExpression): string \| undefined {`
	`2131`	`+ export function getElementOrPropertyAccessName(node: LiteralLikeElementAccessExpression \| PropertyAccessExpression): __String;`
	`2132`	`+ export function getElementOrPropertyAccessName(node: AccessExpression): __String \| undefined;`
	`2133`	`+ export function getElementOrPropertyAccessName(node: AccessExpression): __String \| undefined {`
`2134`	`2134`	`const name = getElementOrPropertyAccessArgumentExpressionOrName(node);`
`2135`	`2135`	`if (name) {`
`2136`	`2136`	`if (isIdentifier(name)) {`
`2137`		`- return idText(name);`
	`2137`	`+ return name.escapedText;`
`2138`	`2138`	`}`
`2139`	`2139`	`if (isStringLiteralLike(name) \|\| isNumericLiteral(name)) {`
`2140`		`- return name.text;`
	`2140`	`+ return escapeLeadingUnderscores(name.text);`
`2141`	`2141`	`}`
`2142`	`2142`	`}`
	`2143`	`+ if (isElementAccessExpression(node) && isWellKnownSymbolSyntactically(node.argumentExpression)) {`
	`2144`	`+ return getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>node.argumentExpression).name));`
	`2145`	`+ }`
`2143`	`2146`	`return undefined;`
`2144`	`2147`	`}`
`2145`	`2148`