diff --git a/src/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindow.java b/src/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindow.java index 197e675c256..9d896b7de0f 100644 --- a/src/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindow.java +++ b/src/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindow.java @@ -31,8 +31,6 @@ */ class DeclaredGlobalExternsOnWindow implements CompilerPass, NodeTraversal.Callback { - private static final String WINDOW_NAME = "window"; - private final AbstractCompiler compiler; private final Set nodes = new LinkedHashSet<>(); @@ -50,21 +48,23 @@ public void process(Node externs, Node root) { } private void addWindowProperties() { - if (!nodes.isEmpty()) { + if (!nodes.isEmpty() && windowInExterns) { for (Node node : nodes) { - addExtern(node, windowInExterns); + addExtern(node); compiler.reportChangeToEnclosingScope(node); } } } - private static void addExtern(Node node, boolean defineOnWindow) { + private static void addExtern(Node node) { String name = node.getString(); JSDocInfo oldJSDocInfo = NodeUtil.getBestJSDocInfo(node); - Node globalRef = defineOnWindow ? IR.name(WINDOW_NAME) : IR.thisNode(); + // 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 string = IR.string(name); - Node getprop = IR.getprop(globalRef, string); + Node getprop = IR.getprop(window, 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_NAME)) { + if (c.getString().equals("window")) { windowInExterns = true; continue; } diff --git a/test/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindowTest.java b/test/com/google/javascript/jscomp/DeclaredGlobalExternsOnWindowTest.java index 92f421bb472..e95991f6f7b 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 use "this" instead. + // No "var window;" so this is a no-op. public void testWindowProperty1b() { - testExternChanges("var a;", "", "var a;this.a"); + testExternChanges("var a", "", "var a"); } public void testWindowProperty2() { @@ -57,9 +57,9 @@ public void testWindowProperty3a() { "var window;function f(){}window.f;"); } - // No "var window;" so use "this" instead. + // No "var window;" so this is a no-op. public void testWindowProperty3b() { - testExternChanges("function f() {}", "var b", "function f(){}this.f"); + testExternChanges("function f() {}", "var b", "function f(){}"); } public void testWindowProperty4() { @@ -73,9 +73,9 @@ public void testWindowProperty5a() { "var window;var x=function(){};window.x;"); } - // No "var window;" so use "this" instead. + // No "var window;" so this is a no-op. public void testWindowProperty5b() { - testExternChanges("var x = function f() {};", "var b", "var x=function f(){};this.x"); + testExternChanges("var x = function f() {}", "var b", "var x=function f(){}"); } public void testWindowProperty5c() { diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java index 6f2e7ef5bd1..c69bd7f7110 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).", - "Object.defineProperties(function() {},", - "{d:{a:!0,b:!0,c:function(){}},", // renamed from init - "e:{a:!0,b:!0,f:function(){}}})"); // renamed from prop + "('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 options.setLanguageIn(LanguageMode.ECMASCRIPT_2015); options.setLanguageOut(LanguageMode.ECMASCRIPT5); @@ -5141,6 +5141,7 @@ 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 6c785275a84..2db86123d27 100644 --- a/test/com/google/javascript/jscomp/SymbolTableTest.java +++ b/test/com/google/javascript/jscomp/SymbolTableTest.java @@ -89,9 +89,8 @@ public void testGlobalVar() throws Exception { } public void testGlobalThisReferences() throws Exception { - SymbolTable table = createSymbolTable( - "var x = this; function f() { return this + this + this; }", - /* externsCode= */ ""); + SymbolTable table = + createSymbolTable("var x = this; function f() { return this + this + this; }"); Symbol global = getGlobalVar(table, "*global*"); assertNotNull(global); @@ -102,7 +101,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("", /* externsCode= */ ""); + SymbolTable table = createSymbolTable(""); Symbol global = getGlobalVar(table, "*global*"); assertNotNull(global); @@ -112,8 +111,7 @@ public void testGlobalThisReferences2() throws Exception { } public void testGlobalThisReferences3() throws Exception { - SymbolTable table = - createSymbolTable("this.foo = {}; this.foo.bar = {};", /* externsCode= */ ""); + SymbolTable table = createSymbolTable("this.foo = {}; this.foo.bar = {};"); Symbol global = getGlobalVar(table, "*global*"); assertNotNull(global); @@ -345,7 +343,7 @@ public void testGlobalVarInExterns() throws Exception { SymbolTable table = createSymbolTable("customExternFn(1);"); Symbol fn = getGlobalVar(table, "customExternFn"); List refs = table.getReferenceList(fn); - assertThat(refs).hasSize(3); + assertThat(refs).hasSize(2); SymbolScope scope = table.getEnclosingScope(refs.get(0).getNode()); assertTrue(scope.isGlobalScope()); @@ -1186,15 +1184,6 @@ 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. */