Addressed CR feedback.

DanielRosenwasser · DanielRosenwasser · commit 024ca6d6ac9b · 2014-09-05T15:58:22.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -3461,11 +3461,14 @@ module ts {
         }
 
         function checkThisExpression(node: Node): Type {
-            var container = getThisContainerOrArrowFunction(node);
+            // Stop at the first arrow function so that we can
+            // tell whether 'this' needs to be captured.
+            var container = getThisContainer(node, /* includeArrowFunctions */ true);
             var needToCaptureLexicalThis = false;
-            // skip arrow functions
-            while (container.kind === SyntaxKind.ArrowFunction) {
-                container = getThisContainerOrArrowFunction(container);
+
+            // Now skip arrow functions to get the "real" owner of 'this'.
+            if (container.kind === SyntaxKind.ArrowFunction) {
+                container = getThisContainer(container, /* includeArrowFunctions */ false);
                 needToCaptureLexicalThis = true;
             }
 
diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts
@@ -407,13 +407,18 @@ module ts {
         }
     }
 
-    export function getThisContainerOrArrowFunction(node: Node): Node {
+    export function getThisContainer(node: Node, includeArrowFunctions: boolean): Node {
         while (true) {
             node = node.parent;
             if (!node) {
                 return node;
             }
             switch (node.kind) {
+                case SyntaxKind.ArrowFunction:
+                    if (!includeArrowFunctions) {
+                        continue;
+                    }
+                // Fall through
                 case SyntaxKind.FunctionDeclaration:
                 case SyntaxKind.FunctionExpression:
                 case SyntaxKind.ModuleDeclaration:
@@ -424,20 +429,11 @@ module ts {
                 case SyntaxKind.SetAccessor:
                 case SyntaxKind.EnumDeclaration:
                 case SyntaxKind.SourceFile:
-                case SyntaxKind.ArrowFunction:
                     return node;
             }
         }
     }
 
-    export function getThisContainer(node: Node): Node {
-        do {
-            node = getThisContainerOrArrowFunction(node);
-        } while (node.kind === SyntaxKind.ArrowFunction);
-
-        return node;
-    }
-
     export function hasRestParameters(s: SignatureDeclaration): boolean {
         return s.parameters.length > 0 && (s.parameters[s.parameters.length - 1].flags & NodeFlags.Rest) !== 0;
     }
diff --git a/src/services/services.ts b/src/services/services.ts
@@ -2629,7 +2629,7 @@ module ts {
 
             function getReferencesForThisKeyword(thisKeyword: Node, sourceFiles: SourceFile[]) {
                 // Get the owner" of the 'this' keyword.
-                var thisContainer = getThisContainer(thisKeyword);
+                var thisContainer = getThisContainer(thisKeyword, /* includeArrowFunctions */ false);
 
                 var searchSpaceNode: Node;
 
@@ -2645,9 +2645,13 @@ module ts {
                         searchSpaceNode = thisContainer.parent; // should be the owning class
                         staticFlag &= thisContainer.flags
                         break;
+                    case SyntaxKind.SourceFile:
+                        if (isExternalModule(<SourceFile>thisContainer)) {
+                            return undefined;
+                        }
+                    // Fall through
                     case SyntaxKind.FunctionDeclaration:
                     case SyntaxKind.FunctionExpression:
-                    case SyntaxKind.SourceFile:
                         searchSpaceNode = thisContainer;
                         break;
                     default:
@@ -2679,8 +2683,9 @@ module ts {
                             return;
                         }
 
-                        // Get the owner" of the 'this' keyword.
-                        var container = getThisContainer(node);
+                        // Get the owner of the 'this' keyword.
+                        // This *should* be a node that occurs somewhere within searchSpaceNode.
+                        var container = getThisContainer(node, /* includeArrowFunctions */ false);
 
                         switch (container.kind) {
                             case SyntaxKind.Property:
@@ -2702,7 +2707,7 @@ module ts {
                                 break;
                             case SyntaxKind.SourceFile:
                                 // Add all 'this' keywords that belong to the top-level scope.
-                                if (searchSpaceNode.kind === SyntaxKind.SourceFile) {
+                                if (searchSpaceNode.kind === SyntaxKind.SourceFile && !isExternalModule(<SourceFile>searchSpaceNode)) {
                                     result.push(getReferenceEntryFromNode(node));
                                 }
                                 break;
@@ -2970,21 +2975,7 @@ module ts {
                 }
                 else if (parent.kind === SyntaxKind.BinaryExpression && (<BinaryExpression>parent).left === node) {
                     var operator = (<BinaryExpression>parent).operator;
-                    switch (operator) {
-                        case SyntaxKind.AsteriskEqualsToken:
-                        case SyntaxKind.SlashEqualsToken:
-                        case SyntaxKind.PercentEqualsToken:
-                        case SyntaxKind.MinusEqualsToken:
-                        case SyntaxKind.LessThanLessThanEqualsToken:
-                        case SyntaxKind.GreaterThanGreaterThanEqualsToken:
-                        case SyntaxKind.GreaterThanGreaterThanGreaterThanEqualsToken:
-                        case SyntaxKind.BarEqualsToken:
-                        case SyntaxKind.CaretEqualsToken:
-                        case SyntaxKind.AmpersandEqualsToken:
-                        case SyntaxKind.PlusEqualsToken:
-                        case SyntaxKind.EqualsToken:
-                            return true;
-                    }
+                    return SyntaxKind.FirstAssignment <= operator && operator <= SyntaxKind.LastAssignment;
                 }
             }
 

Original file line number	Diff line number	Diff line change
`@@ -407,13 +407,18 @@ module ts {`
`407`	`407`	`}`
`408`	`408`	`}`
`409`	`409`
`410`		`- export function getThisContainerOrArrowFunction(node: Node): Node {`
	`410`	`+ export function getThisContainer(node: Node, includeArrowFunctions: boolean): Node {`
`411`	`411`	`while (true) {`
`412`	`412`	`node = node.parent;`
`413`	`413`	`if (!node) {`
`414`	`414`	`return node;`
`415`	`415`	`}`
`416`	`416`	`switch (node.kind) {`
	`417`	`+ case SyntaxKind.ArrowFunction:`
	`418`	`+ if (!includeArrowFunctions) {`
	`419`	`+ continue;`
	`420`	`+ }`
	`421`	`+ // Fall through`
`417`	`422`	`case SyntaxKind.FunctionDeclaration:`
`418`	`423`	`case SyntaxKind.FunctionExpression:`
`419`	`424`	`case SyntaxKind.ModuleDeclaration:`
`@@ -424,20 +429,11 @@ module ts {`
`424`	`429`	`case SyntaxKind.SetAccessor:`
`425`	`430`	`case SyntaxKind.EnumDeclaration:`
`426`	`431`	`case SyntaxKind.SourceFile:`
`427`		`- case SyntaxKind.ArrowFunction:`
`428`	`432`	`return node;`
`429`	`433`	`}`
`430`	`434`	`}`
`431`	`435`	`}`
`432`	`436`
`433`		`- export function getThisContainer(node: Node): Node {`
`434`		`- do {`
`435`		`- node = getThisContainerOrArrowFunction(node);`
`436`		`- } while (node.kind === SyntaxKind.ArrowFunction);`
`437`		`-`
`438`		`- return node;`
`439`		`- }`
`440`		`-`
`441`	`437`	`export function hasRestParameters(s: SignatureDeclaration): boolean {`
`442`	`438`	`return s.parameters.length > 0 && (s.parameters[s.parameters.length - 1].flags & NodeFlags.Rest) !== 0;`
`443`	`439`	`}`