From c9000d615e02a1ca90ea34f72855c5c70681a006 Mon Sep 17 00:00:00 2001 From: dimvar Date: Wed, 1 Nov 2017 16:00:02 -0700 Subject: [PATCH] [NTI] Change when to copy properties from window to Window.prototype, to handle cases where Window is extended (e.g., happens in tests when people use fakes) ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=174250192 --- .../jscomp/GlobalTypeInfoCollector.java | 34 ++++++++----- .../javascript/jscomp/newtypes/Namespace.java | 4 +- .../jscomp/newtypes/RawNominalType.java | 2 +- .../jscomp/NewTypeInferenceTest.java | 51 ++++++++++++++++++- 4 files changed, 75 insertions(+), 16 deletions(-) diff --git a/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java b/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java index 44f2a951824..009ae94ad7c 100644 --- a/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java +++ b/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java @@ -299,6 +299,7 @@ public class GlobalTypeInfoCollector implements CompilerPass { private final GlobalTypeInfo globalTypeInfo; private final SimpleInference simpleInference; private final OrderedExterns orderedExterns; + private RawNominalType window; public GlobalTypeInfoCollector(AbstractCompiler compiler) { this.warnings = new WarningReporter(compiler); @@ -368,30 +369,28 @@ public void process(Node externs, Node root) { // (7) Adjust types of properties based on inheritance information. // Report errors in the inheritance chain. Do Window last. - RawNominalType win = null; + Collection windows = new ArrayList<>(); for (Map.Entry entry : nominaltypesByNode.entrySet()) { RawNominalType rawType = entry.getValue(); - if (rawType.getName().equals(WINDOW_CLASS) && entry.getKey().isFromExterns()) { - win = rawType; + if (this.window != null && rawType.hasAncestorClass(this.window)) { + windows.add(rawType); continue; } checkAndFreezeNominalType(rawType); } JSType globalThisType; - if (win != null) { + if (this.window != null) { // Copy properties from window to Window.prototype, because sometimes // people pass window around rather than using it directly. - // Copying the properties is correct only when there is a single object - // of type Window in the program. But in very rare cases, people subclass Window. - // Then, win is frozen here and we don't copy the properties. - // Window has been subclassed iff it is already frozen here. Namespace winNs = getGlobalScope().getNamespace(WINDOW_INSTANCE); - if (winNs != null && !win.isFrozen()) { - winNs.copyWindowProperties(getCommonTypes(), win); + if (winNs != null) { + winNs.copyWindowProperties(getCommonTypes(), this.window); + } + for (RawNominalType rawType : windows) { + checkAndFreezeNominalType(rawType); } - checkAndFreezeNominalType(win); // Type the global THIS as window - globalThisType = win.getInstanceAsJSType(); + globalThisType = this.window.getInstanceAsJSType(); } else { // Type the global THIS as a loose object globalThisType = getCommonTypes().getTopObject().withLoose(); @@ -447,6 +446,14 @@ public void visit(NodeTraversal t, Node n, Node parent) { this.compiler.setExternProperties(ImmutableSet.copyOf(getExternPropertyNames())); } + private void setWindow(RawNominalType rawType) { + this.window = rawType; + } + + private static boolean isWindowRawType(RawNominalType rawType) { + return rawType.getName().equals(WINDOW_CLASS) && rawType.getDefSite().isFromExterns(); + } + private RawNominalType dummyRawTypeForMissingExterns(String name) { Node defSite = NodeUtil.emptyFunction(); defSite.setStaticSourceFile(new SimpleSourceFile("", true)); @@ -1302,6 +1309,9 @@ private void maybeRecordNominalType( rawType = RawNominalType.makeNominalInterface( getCommonTypes(), defSite, qname, typeParameters, objKind); } + if (isWindowRawType(rawType)) { + setWindow(rawType); + } nominaltypesByNode.put(defSite, rawType); if (isRedeclaration) { return; diff --git a/src/com/google/javascript/jscomp/newtypes/Namespace.java b/src/com/google/javascript/jscomp/newtypes/Namespace.java index 2fd829e8632..1af0f32cd89 100644 --- a/src/com/google/javascript/jscomp/newtypes/Namespace.java +++ b/src/com/google/javascript/jscomp/newtypes/Namespace.java @@ -271,8 +271,8 @@ public final void copyWindowProperties(JSTypes commonTypes, RawNominalType win) win.addProtoProperty(entry.getKey(), null, ns.toJSType(), true); } for (Map.Entry entry : this.otherProps.entrySet()) { - win.addProtoProperty( - entry.getKey(), null, entry.getValue().getType(), true); + Property p = entry.getValue(); + win.addProtoProperty(entry.getKey(), null, p.getType(), p.isConstant()); } } diff --git a/src/com/google/javascript/jscomp/newtypes/RawNominalType.java b/src/com/google/javascript/jscomp/newtypes/RawNominalType.java index 9e2cf369f7d..242f0da679f 100644 --- a/src/com/google/javascript/jscomp/newtypes/RawNominalType.java +++ b/src/com/google/javascript/jscomp/newtypes/RawNominalType.java @@ -247,7 +247,7 @@ public void setCtorFunction(FunctionType ctorFn) { this.ctorFn = ctorFn; } - boolean hasAncestorClass(RawNominalType ancestor) { + public boolean hasAncestorClass(RawNominalType ancestor) { checkState(ancestor.isClass()); if (this == ancestor) { return true; diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java index df5c874ac8f..93a1f9d5670 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java @@ -20504,7 +20504,8 @@ public void testPromoteTrueAndFalseToBooleanWhenInstantiatingGenerics() { "x(true, false);")); } - public void testExtendWindowDontCrash() { + public void testFakeWindows() { + // Only the globals in externs are copied over to Window.prototype typeCheck(LINE_JOINER.join( "/** @type {number} */", "var globalvar;", @@ -20518,6 +20519,54 @@ public void testExtendWindowDontCrash() { "}", "f(new FakeWindow);"), NewTypeInference.INEXISTENT_PROPERTY); + + typeCheckCustomExterns( + LINE_JOINER.join( + DEFAULT_EXTERNS, + "/** @type {number} */", + "var foobar;"), + LINE_JOINER.join( + "/**", + " * @constructor", + " * @extends {Window}", + " */", + "function FakeWindow() {}", + "function f(/** !Window */ w) {", + " var /** string */ s = w.foobar;", + "}"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + + typeCheckCustomExterns( + LINE_JOINER.join( + DEFAULT_EXTERNS, + "/** @type {number} */", + "var foobar;"), + LINE_JOINER.join( + "/**", + " * @constructor", + " * @extends {Window}", + " */", + "function FakeWindow() {}", + "function f(/** !FakeWindow */ w) {", + " var /** string */ s = w.foobar;", + "}"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + + typeCheckCustomExterns( + LINE_JOINER.join( + DEFAULT_EXTERNS, + "/** @type {number} */", + "var foobar;"), + LINE_JOINER.join( + "/**", + " * @constructor", + " * @extends {Window}", + " */", + "function FakeWindow() {}", + "function f(/** !FakeWindow */ w) {", + " w.foobar = 'asdf';", + "}"), + NewTypeInference.MISTYPED_ASSIGN_RHS); } public void testInferUndeclaredPrototypeProperty() {