diff --git a/src/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindow.java b/src/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindow.java index 9d896b7de0f..197e675c256 100644 --- a/src/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindow.java +++ b/src/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindow.java @@ -31,6 +31,8 @@ */ class DeclaredGlobalExternsOnWindow implements CompilerPass, NodeTraversal.Callback { + private static final String WINDOW_NAME = "window"; + private final AbstractCompiler compiler; private final Set nodes = new LinkedHashSet<>(); @@ -48,23 +50,21 @@ public void process(Node externs, Node root) { } private void addWindowProperties() { - if (!nodes.isEmpty() && windowInExterns) { + if (!nodes.isEmpty()) { for (Node node : nodes) { - addExtern(node); + addExtern(node, windowInExterns); compiler.reportChangeToEnclosingScope(node); } } } - private static void addExtern(Node node) { + private static void addExtern(Node node, boolean defineOnWindow) { String name = node.getString(); JSDocInfo oldJSDocInfo = NodeUtil.getBestJSDocInfo(node); - // TODO(tbreisacher): Consider adding externs to 'this' instead of 'window', - // for environments where the global object is not called 'window.' - Node window = IR.name("window"); + Node globalRef = defineOnWindow ? IR.name(WINDOW_NAME) : IR.thisNode(); Node string = IR.string(name); - Node getprop = IR.getprop(window, string); + Node getprop = IR.getprop(globalRef, string); Node newNode = getprop; if (oldJSDocInfo != null) { @@ -131,7 +131,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { nodes.add(n.getFirstChild()); } else if (n.isVar()) { for (Node c : n.children()) { - if (c.getString().equals("window")) { + if (c.getString().equals(WINDOW_NAME)) { windowInExterns = true; continue; } diff --git a/test/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindowTest.java b/test/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindowTest.java index e95991f6f7b..92f421bb472 100644 --- a/test/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindowTest.java +++ b/test/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindowTest.java @@ -43,9 +43,9 @@ public void testWindowProperty1a() { testExternChanges("var window; var a;", "", "var window;var a;window.a"); } - // No "var window;" so this is a no-op. + // No "var window;" so use "this" instead. public void testWindowProperty1b() { - testExternChanges("var a", "", "var a"); + testExternChanges("var a;", "", "var a;this.a"); } public void testWindowProperty2() { @@ -57,9 +57,9 @@ public void testWindowProperty3a() { "var window;function f(){}window.f;"); } - // No "var window;" so this is a no-op. + // No "var window;" so use "this" instead. public void testWindowProperty3b() { - testExternChanges("function f() {}", "var b", "function f(){}"); + testExternChanges("function f() {}", "var b", "function f(){}this.f"); } public void testWindowProperty4() { @@ -73,9 +73,9 @@ public void testWindowProperty5a() { "var window;var x=function(){};window.x;"); } - // No "var window;" so this is a no-op. + // No "var window;" so use "this" instead. public void testWindowProperty5b() { - testExternChanges("var x = function f() {}", "var b", "var x=function f(){}"); + testExternChanges("var x = function f() {};", "var b", "var x=function f(){};this.x"); } public void testWindowProperty5c() { diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java index c69bd7f7110..6f2e7ef5bd1 100644 --- a/test/com/google/javascript/jscomp/IntegrationTest.java +++ b/test/com/google/javascript/jscomp/IntegrationTest.java @@ -2013,11 +2013,11 @@ public void testClassWithGettersIsRemoved() { // Investigate why are they kept when ran together with other passes. String expected = LINE_JOINER.join( - "('undefined'!=typeof window&&window===this?this:'undefined'!=typeof ", - "global&&null!=global?global:this).", - "c.defineProperties(function() {},", - "{e:{a:!0,b:!0,d:function(){}},", // renamed from init - "f:{a:!0,b:!0,g:function(){}}})"); // renamed from prop + "('undefined'!=typeof window&&window===this?this:'undefined'!=typeof ", + "global&&null!=global?global:this).", + "Object.defineProperties(function() {},", + "{d:{a:!0,b:!0,c:function(){}},", // renamed from init + "e:{a:!0,b:!0,f:function(){}}})"); // renamed from prop options.setLanguageIn(LanguageMode.ECMASCRIPT_2015); options.setLanguageOut(LanguageMode.ECMASCRIPT5); @@ -5141,7 +5141,6 @@ protected CompilerOptions createCompilerOptions() { options.setDevMode(DevMode.EVERY_PASS); options.setCodingConvention(new GoogleCodingConvention()); options.setRenamePrefixNamespaceAssumeCrossModuleNames(true); - options.declaredGlobalExternsOnWindow = false; return options; } } diff --git a/test/com/google/javascript/jscomp/SymbolTableTest.java b/test/com/google/javascript/jscomp/SymbolTableTest.java index 2db86123d27..6c785275a84 100644 --- a/test/com/google/javascript/jscomp/SymbolTableTest.java +++ b/test/com/google/javascript/jscomp/SymbolTableTest.java @@ -89,8 +89,9 @@ public void testGlobalVar() throws Exception { } public void testGlobalThisReferences() throws Exception { - SymbolTable table = - createSymbolTable("var x = this; function f() { return this + this + this; }"); + SymbolTable table = createSymbolTable( + "var x = this; function f() { return this + this + this; }", + /* externsCode= */ ""); Symbol global = getGlobalVar(table, "*global*"); assertNotNull(global); @@ -101,7 +102,7 @@ public void testGlobalThisReferences() throws Exception { public void testGlobalThisReferences2() throws Exception { // Make sure the global this is declared, even if it isn't referenced. - SymbolTable table = createSymbolTable(""); + SymbolTable table = createSymbolTable("", /* externsCode= */ ""); Symbol global = getGlobalVar(table, "*global*"); assertNotNull(global); @@ -111,7 +112,8 @@ public void testGlobalThisReferences2() throws Exception { } public void testGlobalThisReferences3() throws Exception { - SymbolTable table = createSymbolTable("this.foo = {}; this.foo.bar = {};"); + SymbolTable table = + createSymbolTable("this.foo = {}; this.foo.bar = {};", /* externsCode= */ ""); Symbol global = getGlobalVar(table, "*global*"); assertNotNull(global); @@ -343,7 +345,7 @@ public void testGlobalVarInExterns() throws Exception { SymbolTable table = createSymbolTable("customExternFn(1);"); Symbol fn = getGlobalVar(table, "customExternFn"); List refs = table.getReferenceList(fn); - assertThat(refs).hasSize(2); + assertThat(refs).hasSize(3); SymbolScope scope = table.getEnclosingScope(refs.get(0).getNode()); assertTrue(scope.isGlobalScope()); @@ -1184,6 +1186,15 @@ private SymbolTable createSymbolTable(String input) { return assertSymbolTableValid(compiler.buildKnownSymbolTable()); } + private SymbolTable createSymbolTable(String input, String externsCode) { + List inputs = ImmutableList.of(SourceFile.fromCode("in1", input)); + List externs = ImmutableList.of(SourceFile.fromCode("externs1", externsCode)); + + Compiler compiler = new Compiler(new BlackHoleErrorManager()); + compiler.compile(externs, inputs, options); + return assertSymbolTableValid(compiler.buildKnownSymbolTable()); + } + /** * Asserts that the symbol table meets some invariants. Returns the same table for easy chaining. */