[NTI] Refactor CodingConvention.addSingletonGetter to share the same …

…implementation for both OTI and NTI.

The main change is to introduce an ObjectTypeI.PropertyDeclarer interface that exposes a declareProperty method to add a property to a not-yet-frozen type.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=168755535

src/com/google/javascript/jscomp/ChromeCodingConvention.java

@@ -19,9 +19,9 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.annotations.Immutable;
 import com.google.javascript.jscomp.ClosureCodingConvention.AssertInstanceofSpec;
+import com.google.javascript.rhino.FunctionTypeI;
 import com.google.javascript.rhino.Node;
-import com.google.javascript.rhino.jstype.FunctionType;
-import com.google.javascript.rhino.jstype.ObjectType;
+import com.google.javascript.rhino.ObjectTypeI;
 import java.util.Collection;
 
 /**
@@ -53,13 +53,11 @@ public String getSingletonGetterClassName(Node callNode) {
   }
 
   @Override
-  public void applySingletonGetterOld(FunctionType functionType,
-      FunctionType getterType, ObjectType objectType) {
-    super.applySingletonGetterOld(functionType, getterType, objectType);
-    functionType.defineDeclaredProperty("getInstance", getterType,
-        functionType.getSource());
-    functionType.defineDeclaredProperty("instance_", objectType,
-        functionType.getSource());
+  public void applySingletonGetter(ObjectTypeI.PropertyDeclarer declarer,
+      FunctionTypeI functionType, FunctionTypeI getterType, ObjectTypeI objectType) {
+    super.applySingletonGetter(declarer, functionType, getterType, objectType);
+    declarer.declareProperty(functionType, "getInstance", getterType, functionType.getSource());
+    declarer.declareProperty(functionType, "instance_", objectType, functionType.getSource());
   }
 
   @Override

src/com/google/javascript/jscomp/ClosureCodingConvention.java

@@ -25,13 +25,14 @@
 import com.google.javascript.jscomp.newtypes.DeclaredTypeRegistry;
 import com.google.javascript.jscomp.newtypes.JSType;
 import com.google.javascript.jscomp.newtypes.QualifiedName;
-import com.google.javascript.jscomp.newtypes.RawNominalType;
+import com.google.javascript.rhino.FunctionTypeI;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.ObjectTypeI;
+import com.google.javascript.rhino.ObjectTypeI.PropertyDeclarer;
 import com.google.javascript.rhino.jstype.FunctionType;
 import com.google.javascript.rhino.jstype.JSTypeNative;
 import com.google.javascript.rhino.jstype.JSTypeRegistry;
-import com.google.javascript.rhino.jstype.ObjectType;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -323,19 +324,10 @@ public String getSingletonGetterClassName(Node callNode) {
   }
 
   @Override
-  public void applySingletonGetterOld(FunctionType functionType,
-      FunctionType getterType, ObjectType objectType) {
-    functionType.defineDeclaredProperty("getInstance", getterType,
-        functionType.getSource());
-    functionType.defineDeclaredProperty("instance_", objectType,
-        functionType.getSource());
-  }
-
-  @Override
-  public void applySingletonGetterNew(
-      RawNominalType rawType, JSType getInstanceType, JSType instanceType) {
-    rawType.addCtorProperty("getInstance", null, getInstanceType, true);
-    rawType.addCtorProperty("instance_", null, instanceType, true);
+  public void applySingletonGetter(PropertyDeclarer declarer,
+      FunctionTypeI functionType, FunctionTypeI getterType, ObjectTypeI objectType) {
+    declarer.declareProperty(functionType, "getInstance", getterType, functionType.getSource());
+    declarer.declareProperty(functionType, "instance_", objectType, functionType.getSource());
   }
 
   @Override

src/com/google/javascript/jscomp/CodingConvention.java

@@ -19,8 +19,10 @@
 import com.google.errorprone.annotations.Immutable;
 import com.google.javascript.jscomp.newtypes.DeclaredTypeRegistry;
 import com.google.javascript.jscomp.newtypes.JSType;
-import com.google.javascript.jscomp.newtypes.RawNominalType;
+import com.google.javascript.rhino.FunctionTypeI;
 import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.ObjectTypeI;
+import com.google.javascript.rhino.ObjectTypeI.PropertyDeclarer;
 import com.google.javascript.rhino.StaticSourceFile;
 import com.google.javascript.rhino.jstype.FunctionType;
 import com.google.javascript.rhino.jstype.JSTypeNative;
@@ -241,11 +243,11 @@ public void applySubclassRelationship(FunctionType parentCtor,
    * In many JS libraries, the function that adds a singleton getter to a class
    * adds properties to the class.
    */
-  public void applySingletonGetterOld(FunctionType functionType,
-      FunctionType getterType, ObjectType objectType);
-
-  public void applySingletonGetterNew(
-      RawNominalType rawType, JSType getInstanceType, JSType instanceType);
+  public void applySingletonGetter(
+      PropertyDeclarer declarer,
+      FunctionTypeI functionType,
+      FunctionTypeI getterType,
+      ObjectTypeI objectType);
 
   /**
    * @return Whether the function is inlinable by convention.

src/com/google/javascript/jscomp/CodingConventions.java

@@ -20,9 +20,10 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.errorprone.annotations.Immutable;
-import com.google.javascript.jscomp.newtypes.JSType;
-import com.google.javascript.jscomp.newtypes.RawNominalType;
+import com.google.javascript.rhino.FunctionTypeI;
 import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.ObjectTypeI;
+import com.google.javascript.rhino.ObjectTypeI.PropertyDeclarer;
 import com.google.javascript.rhino.StaticSourceFile;
 import com.google.javascript.rhino.jstype.FunctionType;
 import com.google.javascript.rhino.jstype.JSTypeRegistry;
@@ -216,16 +217,9 @@ public String getSingletonGetterClassName(Node callNode) {
     }
 
     @Override
-    public void applySingletonGetterOld(FunctionType functionType,
-        FunctionType getterType, ObjectType objectType) {
-      nextConvention.applySingletonGetterOld(
-          functionType, getterType, objectType);
-    }
-
-    @Override
-    public void applySingletonGetterNew(
-        RawNominalType rawType, JSType getInstanceType, JSType instanceType) {
-      nextConvention.applySingletonGetterNew(rawType, getInstanceType, instanceType);
+    public void applySingletonGetter(PropertyDeclarer declarer,
+        FunctionTypeI functionType, FunctionTypeI getterType, ObjectTypeI objectType) {
+      nextConvention.applySingletonGetter(declarer, functionType, getterType, objectType);
     }
 
     @Override
@@ -480,14 +474,9 @@ public String getSingletonGetterClassName(Node callNode) {
     }
 
     @Override
-    public void applySingletonGetterOld(FunctionType functionType,
-        FunctionType getterType, ObjectType objectType) {
-      // do nothing.
-    }
-
-    @Override
-    public void applySingletonGetterNew(
-        RawNominalType rawType, JSType getInstanceType, JSType instanceType) {
+    public void applySingletonGetter(
+        PropertyDeclarer declarer, FunctionTypeI functionType,
+        FunctionTypeI getterType, ObjectTypeI objectType) {
       // do nothing.
     }
 

src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java

@@ -43,6 +43,7 @@
 import com.google.javascript.jscomp.newtypes.Namespace;
 import com.google.javascript.jscomp.newtypes.NominalType;
 import com.google.javascript.jscomp.newtypes.ObjectKind;
+import com.google.javascript.jscomp.newtypes.PropertyDeclarer;
 import com.google.javascript.jscomp.newtypes.QualifiedName;
 import com.google.javascript.jscomp.newtypes.RawNominalType;
 import com.google.javascript.jscomp.newtypes.Typedef;
@@ -286,6 +287,8 @@ public class GlobalTypeInfoCollector implements CompilerPass {
   private static final String WINDOW_INSTANCE = "window";
   private static final String WINDOW_CLASS = "Window";
 
+  private static final PropertyDeclarer PROPERTY_DECLARER = new PropertyDeclarer();
+
   private DefaultNameGenerator funNameGen;
   // Only for original definitions, not for aliased constructors
   private Map<Node, RawNominalType> nominaltypesByNode = new LinkedHashMap<>();
@@ -1562,18 +1565,26 @@ private void visitObjectLit(Node objLitNode, Node parent) {
     }
 
     private void visitCall(Node call) {
+      // Check various coding conventions to see if any additional handling is needed.
       String className = convention.getSingletonGetterClassName(call);
-      if (className == null) {
-        return;
+      if (className != null) {
+        applySingletonGetter(className);
       }
+    }
+
+    private void applySingletonGetter(String className) {
       QualifiedName qname = QualifiedName.fromQualifiedString(className);
       RawNominalType rawType = currentScope.getNominalType(qname);
       if (rawType != null) {
         JSType instanceType = rawType.getInstanceAsJSType();
         FunctionType getInstanceFunType =
             new FunctionTypeBuilder(getCommonTypes()).addRetType(instanceType).buildFunction();
         JSType getInstanceType = getCommonTypes().fromFunctionType(getInstanceFunType);
-        convention.applySingletonGetterNew(rawType, getInstanceType, instanceType);
+        convention.applySingletonGetter(
+            PROPERTY_DECLARER,
+            getCommonTypes().fromFunctionType(rawType.getConstructorFunction()),
+            getInstanceType,
+            instanceType);
       }
     }
 

src/com/google/javascript/jscomp/TypedScopeCreator.java

@@ -73,6 +73,7 @@
 import com.google.javascript.rhino.jstype.JSTypeRegistry;
 import com.google.javascript.rhino.jstype.ObjectType;
 import com.google.javascript.rhino.jstype.Property;
+import com.google.javascript.rhino.jstype.PropertyDeclarer;
 import com.google.javascript.rhino.jstype.TemplateType;
 import com.google.javascript.rhino.jstype.TemplateTypeMap;
 import com.google.javascript.rhino.jstype.TemplateTypeMapReplacer;
@@ -149,6 +150,8 @@ final class TypedScopeCreator implements ScopeCreator {
       UNKNOWN_LENDS,
       LENDS_ON_NON_OBJECT);
 
+  private static final PropertyDeclarer PROPERTY_DECLARER = new PropertyDeclarer();
+
   private final AbstractCompiler compiler;
   private final ErrorReporter typeParsingErrorReporter;
   private final TypeValidator validator;
@@ -1514,7 +1517,8 @@ private void checkForClassDefiningCalls(Node n) {
 
           if (functionType != null) {
             FunctionType getterType = typeRegistry.createFunctionType(objectType);
-            codingConvention.applySingletonGetterOld(functionType, getterType, objectType);
+            codingConvention.applySingletonGetter(
+                PROPERTY_DECLARER, functionType, getterType, objectType);
           }
         }
       }

src/com/google/javascript/jscomp/newtypes/PropertyDeclarer.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright 2017 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp.newtypes;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.javascript.rhino.FunctionTypeI;
+import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.ObjectTypeI;
+import com.google.javascript.rhino.TypeI;
+
+/**
+ * NTI implementation of ObjectTypeI.PropertyDeclarer.
+ */
+public final class PropertyDeclarer implements ObjectTypeI.PropertyDeclarer {
+  @Override
+  public void declareProperty(ObjectTypeI object, String property, TypeI type, Node defSite) {
+    checkArgument(object instanceof JSType);
+    checkArgument(type instanceof JSType);
+    FunctionTypeI owner = object.getOwnerFunction();
+    if (owner != null) {
+      object = owner;
+    }
+    boolean ctor = object.isConstructor();
+    if (ctor) {
+      object = ((FunctionTypeI) object).getInstanceType();
+    }
+    NominalType nt = ((JSType) object).getNominalTypeIfSingletonObj();
+    checkArgument(nt != null);
+    RawNominalType rt = nt.getRawNominalType();
+    if (owner != null) {
+      rt.addProtoProperty(property, defSite, (JSType) type, true);
+    } else if (ctor) {
+      rt.addCtorProperty(property, defSite, (JSType) type, true);
+    } else {
+      rt.addInstanceProperty(property, defSite, (JSType) type, true);
+    }
+  }
+}

src/com/google/javascript/rhino/FunctionTypeI.java

@@ -46,7 +46,7 @@
  * @author blickly@google.com (Ben Lickly)
  * @author dimvar@google.com (Dimitris Vardoulakis)
  */
-public interface FunctionTypeI extends TypeI {
+public interface FunctionTypeI extends ObjectTypeI {
   /**
    * Creates a new function type B based on the original function type A.
    * Takes the receiver type (this type) of A and makes it the first

src/com/google/javascript/rhino/ObjectTypeI.java

@@ -176,4 +176,10 @@ public interface ObjectTypeI extends TypeI {
    * Returns a set of properties defined or inferred on this type or any of its supertypes.
    */
   Set<String> getPropertyNames();
+
+  /** Interface for declaring properties on object types while they are being built. */
+  interface PropertyDeclarer {
+    /** Declares the given property on the given object type. */
+    void declareProperty(ObjectTypeI object, String property, TypeI type, Node defSite);
+  }
 }

src/com/google/javascript/rhino/jstype/PropertyDeclarer.java

@@ -0,0 +1,58 @@
+/*
+ *
+ * ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Rhino code, released
+ * May 6, 1999.
+ *
+ * The Initial Developer of the Original Code is
+ * Netscape Communications Corporation.
+ * Portions created by the Initial Developer are Copyright (C) 1997-1999
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Bob Jervis
+ *   Google Inc.
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * the GNU General Public License Version 2 or later (the "GPL"), in which
+ * case the provisions of the GPL are applicable instead of those above. If
+ * you wish to allow use of your version of this file only under the terms of
+ * the GPL and not to allow others to use your version of this file under the
+ * MPL, indicate your decision by deleting the provisions above and replacing
+ * them with the notice and other provisions required by the GPL. If you do
+ * not delete the provisions above, a recipient may use your version of this
+ * file under either the MPL or the GPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+package com.google.javascript.rhino.jstype;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.ObjectTypeI;
+import com.google.javascript.rhino.TypeI;
+
+/**
+ * OTI implementation of ObjectTypeI.PropertyDeclarer.
+ */
+public final class PropertyDeclarer implements ObjectTypeI.PropertyDeclarer {
+  @Override
+  public void declareProperty(ObjectTypeI object, String property, TypeI type, Node defSite) {
+    checkArgument(object instanceof ObjectType);
+    checkArgument(type instanceof JSType);
+    ((ObjectType) object).defineDeclaredProperty(property, (JSType) type, defSite);
+  }
+}