[NTI] Fix wrong warning about duplicate namespace property definition.

------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=176548527
google · Nov 21, 2017 · 5d347dc · 5d347dc
1 parent 8825406
commit 5d347dc
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 17 deletions.
diff --git a/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java b/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java
@@ -1905,7 +1905,7 @@ else if (NodeUtil.isPrototypeAssignment(getProp)) {
         visitPrototypeAssignment(getProp);
       }
       // "Static" property on constructor
-      else if (isStaticCtorProp(getProp, currentScope)) {
+      else if (isStaticCtorProp(getProp)) {
         visitConstructorPropertyDeclaration(getProp);
       }
       // Namespace property
@@ -1918,17 +1918,12 @@ else if (currentScope.isNamespace(getProp.getFirstChild())) {
       }
     }
 
-    private boolean isStaticCtorProp(Node getProp, NTIScope s) {
+    private boolean isStaticCtorProp(Node getProp) {
       checkArgument(getProp.isGetProp());
       if (!getProp.isQualifiedName()) {
         return false;
       }
-      Node receiverObj = getProp.getFirstChild();
-      if (!s.isLocalFunDef(receiverObj.getQualifiedName())) {
-        return false;
-      }
-      return null != currentScope.getNominalType(
-          QualifiedName.fromNode(receiverObj));
+      return null != currentScope.getNominalType(QualifiedName.fromNode(getProp.getFirstChild()));
     }
 
     /** Compute the declared type for a given scope. */
@@ -2063,13 +2058,18 @@ private void visitConstructorPropertyDeclaration(Node getProp) {
       if (isNamedType(getProp)) {
         return;
       }
-      String ctorName = getProp.getFirstChild().getQualifiedName();
       QualifiedName ctorQname = QualifiedName.fromNode(getProp.getFirstChild());
-      checkState(currentScope.isLocalFunDef(ctorName));
       RawNominalType classType = currentScope.getNominalType(ctorQname);
       String pname = getProp.getLastChild().getString();
       JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(getProp);
-      JSType propDeclType = getDeclaredTypeOfNode(jsdoc, currentScope);
+      JSType propDeclType;
+      if (jsdoc != null && !jsdoc.hasType() && jsdoc.containsFunctionDeclaration()) {
+        FunctionAndSlotType fst =
+            getTypeParser().getFunctionType(jsdoc, pname, getProp, null, null, this.currentScope);
+        propDeclType = getCommonTypes().fromFunctionType(fst.functionType.toFunctionType());
+      } else {
+        propDeclType = getDeclaredTypeOfNode(jsdoc, currentScope);
+      }
       boolean isConst = isConst(getProp);
       if (propDeclType != null || isConst) {
         JSType previousPropType = classType.getCtorPropDeclaredType(pname);
@@ -2164,8 +2164,8 @@ private void visitNamespacePropertyDeclaration(
         defSite.putBooleanProp(Node.ANALYZED_DURING_GTI, true);
         if (ns.hasSubnamespace(new QualifiedName(pname))
             || (ns.hasStaticProp(pname)
-            && previousPropType != null
-            && !suppressDupPropWarning(jsdoc, propDeclType, previousPropType))) {
+                && previousPropType != null
+                && !suppressDupPropWarning(jsdoc, propDeclType, previousPropType))) {
           warnings.add(JSError.make(
               defSite, REDECLARED_PROPERTY, pname, "namespace " + ns));
           defSite.getParent().putBooleanProp(Node.ANALYZED_DURING_GTI, true);
@@ -2713,12 +2713,10 @@ private RawNominalType maybeGetOwnerType(Node funNode, Node parent) {
 
     private boolean isNamedType(Node getProp) {
       JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(getProp);
-      if (jsdoc != null
-          && jsdoc.hasType() && !jsdoc.containsFunctionDeclaration()) {
+      if (jsdoc != null && jsdoc.hasType() && !jsdoc.containsFunctionDeclaration()) {
         return false;
       }
-      return this.currentScope.isNamespace(getProp)
-          || NodeUtil.isTypedefDecl(getProp);
+      return this.currentScope.isNamespace(getProp) || NodeUtil.isTypedefDecl(getProp);
     }
   }
 

diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java
@@ -1702,6 +1702,18 @@ public void testSpecializedFunctions() {
         "var h = f;"));
   }
 
+  public void testGoogNullFunctionDifferentArityNoWarning() {
+    typeCheck(LINE_JOINER.join(
+        CLOSURE_BASE,
+        "/** @const */",
+        "var ns = {};",
+        "/** @constructor */",
+        "ns.Foo = function() {};",
+        "/** @param {!Function} x */",
+        "ns.Foo.fun = goog.nullFunction;",
+        "ns.Foo.fun(function() {});"));
+  }
+
   public void testDifficultObjectSpecialization() {
     typeCheck(LINE_JOINER.join(
         "/** @constructor */",
@@ -15043,6 +15055,18 @@ public void testFunctionNamespacesThatArentProperties() {
         "function f() {",
         "  f.prop = function() {};",
         "}"));
+
+    typeCheck(LINE_JOINER.join(
+        "/** @const */",
+        "var ns = {};",
+        "/** @constructor */",
+        "ns.Foo = function() {};",
+        "function f() {}",
+        "f.prop = 123;",
+        "/** @typedef {function()} */",
+        "var myfun;",
+        "/** @const {myfun} */",
+        "ns.Foo.defaultUrlPolicy_ = f;"));
   }
 
   public void testFunctionNamespacesThatAreProperties() {