Skip to content

Commit

Permalink
[NTI] Replace PropertyDeclarer with NominalTypeBuilder.
Browse files Browse the repository at this point in the history 
PropertyDeclarer didn't actually work for implementing applySubclassRelationship in NTI. NominalTypeBuilder is a redesign that does work.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=170134043
  • Loading branch information
@shicks @brad4d
shicks authored and brad4d committed Sep 27, 2017
1 parent c44d17c commit 2e55c76
Show file tree
Hide file tree
Showing 17 changed files with 585 additions and 205 deletions.
11 changes: 5 additions & 6 deletions src/com/google/javascript/jscomp/ChromeCodingConvention.java
View file
Expand Up @@ -21,7 +21,7 @@
import com.google.javascript.jscomp.ClosureCodingConvention.AssertInstanceofSpec;
import com.google.javascript.rhino.FunctionTypeI;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.ObjectTypeI;
import com.google.javascript.rhino.NominalTypeBuilder;
import java.util.Collection;

/**
Expand Down Expand Up @@ -53,11 +53,10 @@ public String getSingletonGetterClassName(Node callNode) {
}

@Override
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());
public void applySingletonGetter(NominalTypeBuilder classType, FunctionTypeI getterType) {
Node defSite = ((FunctionTypeI) classType.constructor().toTypeI()).getSource();
classType.constructor().declareProperty("getInstance", getterType, defSite);
classType.constructor().declareProperty("instance_", classType.instance().toTypeI(), defSite);
}

@Override
Expand Down
46 changes: 23 additions & 23 deletions src/com/google/javascript/jscomp/ClosureCodingConvention.java
View file
Expand Up @@ -28,8 +28,7 @@
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.NominalTypeBuilder;
import com.google.javascript.rhino.jstype.FunctionType;
import com.google.javascript.rhino.jstype.JSTypeNative;
import com.google.javascript.rhino.jstype.JSTypeRegistry;
Expand Down Expand Up @@ -74,24 +73,25 @@ public ClosureCodingConvention(CodingConvention wrapped) {
*/
@Override
public void applySubclassRelationship(
PropertyDeclarer declarer,
FunctionTypeI parentCtor,
FunctionTypeI childCtor,
SubclassType type) {
super.applySubclassRelationship(declarer, parentCtor, childCtor, type);
final NominalTypeBuilder parent, final NominalTypeBuilder child, SubclassType type) {
super.applySubclassRelationship(parent, child, type);
if (type == SubclassType.INHERITS) {
declarer.declareProperty(childCtor, "superClass_",
parentCtor.getPrototypeProperty(), childCtor.getSource());
declarer.declareProperty(childCtor.getPrototypeProperty(), "constructor",
// Notice that constructor functions do not need to be covariant
// on the superclass.
// So if G extends F, new G() and new F() can accept completely
// different argument types, but G.prototype.constructor needs
// to be covariant on F.prototype.constructor.
// To get around this, we just turn off type-checking on arguments
// and return types of G.prototype.constructor.
childCtor.toBuilder().withUnknownReturnType().withNoParameters().build(),
childCtor.getSource());
final FunctionTypeI childCtor = (FunctionTypeI) child.constructor().toTypeI();
child.beforeFreeze(new Runnable() {
@Override
public void run() {
child.constructor().declareProperty(
"superClass_", parent.prototype().toTypeI(), childCtor.getSource());
}
}, parent);
// Notice that constructor functions do not need to be covariant on the superclass.
// So if G extends F, new G() and new F() can accept completely different argument
// types, but G.prototype.constructor needs to be covariant on F.prototype.constructor.
// To get around this, we just turn off type-checking on arguments and return types
// of G.prototype.constructor.
FunctionTypeI qmarkCtor =
childCtor.toBuilder().withUnknownReturnType().withNoParameters().build();
child.prototype().declareProperty("constructor", qmarkCtor, childCtor.getSource());
}
}

Expand Down Expand Up @@ -327,10 +327,10 @@ public String getSingletonGetterClassName(Node callNode) {
}

@Override
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());
public void applySingletonGetter(NominalTypeBuilder classType, FunctionTypeI getterType) {
Node defSite = ((FunctionTypeI) classType.constructor().toTypeI()).getSource();
classType.constructor().declareProperty("getInstance", getterType, defSite);
classType.constructor().declareProperty("instance_", classType.instance().toTypeI(), defSite);
}

@Override
Expand Down
13 changes: 3 additions & 10 deletions src/com/google/javascript/jscomp/CodingConvention.java
View file
Expand Up @@ -21,8 +21,7 @@
import com.google.javascript.jscomp.newtypes.JSType;
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.NominalTypeBuilder;
import com.google.javascript.rhino.StaticSourceFile;
import com.google.javascript.rhino.jstype.FunctionType;
import com.google.javascript.rhino.jstype.JSTypeNative;
Expand Down Expand Up @@ -215,10 +214,7 @@ public interface CodingConvention extends Serializable {
* adds properties to the superclass and/or subclass.
*/
public void applySubclassRelationship(
PropertyDeclarer declarer,
FunctionTypeI parentCtor,
FunctionTypeI childCtor,
SubclassType type);
NominalTypeBuilder parent, NominalTypeBuilder child, SubclassType type);

/**
* Function name for abstract methods. An abstract method can be assigned to
Expand Down Expand Up @@ -247,10 +243,7 @@ public void applySubclassRelationship(
* adds properties to the class.
*/
public void applySingletonGetter(
PropertyDeclarer declarer,
FunctionTypeI functionType,
FunctionTypeI getterType,
ObjectTypeI objectType);
NominalTypeBuilder classType, FunctionTypeI getterType);

/**
* @return Whether the function is inlinable by convention.
Expand Down
24 changes: 8 additions & 16 deletions src/com/google/javascript/jscomp/CodingConventions.java
View file
Expand Up @@ -22,8 +22,7 @@
import com.google.errorprone.annotations.Immutable;
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.NominalTypeBuilder;
import com.google.javascript.rhino.StaticSourceFile;
import com.google.javascript.rhino.jstype.FunctionType;
import com.google.javascript.rhino.jstype.JSTypeRegistry;
Expand Down Expand Up @@ -201,11 +200,8 @@ public List<String> identifyTypeDeclarationCall(Node n) {

@Override
public void applySubclassRelationship(
PropertyDeclarer declarer,
FunctionTypeI parentCtor,
FunctionTypeI childCtor,
SubclassType type) {
nextConvention.applySubclassRelationship(declarer, parentCtor, childCtor, type);
NominalTypeBuilder parent, NominalTypeBuilder child, SubclassType type) {
nextConvention.applySubclassRelationship(parent, child, type);
}

@Override
Expand All @@ -219,9 +215,9 @@ public String getSingletonGetterClassName(Node callNode) {
}

@Override
public void applySingletonGetter(PropertyDeclarer declarer,
FunctionTypeI functionType, FunctionTypeI getterType, ObjectTypeI objectType) {
nextConvention.applySingletonGetter(declarer, functionType, getterType, objectType);
public void applySingletonGetter(
NominalTypeBuilder classType, FunctionTypeI getterType) {
nextConvention.applySingletonGetter(classType, getterType);
}

@Override
Expand Down Expand Up @@ -461,10 +457,7 @@ public List<String> identifyTypeDeclarationCall(Node n) {

@Override
public void applySubclassRelationship(
PropertyDeclarer declarer,
FunctionTypeI parentCtor,
FunctionTypeI childCtor,
SubclassType type) {
NominalTypeBuilder parent, NominalTypeBuilder child, SubclassType type) {
// do nothing
}

Expand All @@ -480,8 +473,7 @@ public String getSingletonGetterClassName(Node callNode) {

@Override
public void applySingletonGetter(
PropertyDeclarer declarer, FunctionTypeI functionType,
FunctionTypeI getterType, ObjectTypeI objectType) {
NominalTypeBuilder classType, FunctionTypeI getterType) {
// do nothing.
}

Expand Down
66 changes: 53 additions & 13 deletions src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java
View file
Expand Up @@ -28,6 +28,7 @@
import com.google.common.collect.Multimap;
import com.google.javascript.jscomp.AbstractCompiler.MostRecentTypechecker;
import com.google.javascript.jscomp.CodingConvention.Bind;
import com.google.javascript.jscomp.CodingConvention.SubclassRelationship;
import com.google.javascript.jscomp.NewTypeInference.WarningReporter;
import com.google.javascript.jscomp.NodeTraversal.AbstractShallowCallback;
import com.google.javascript.jscomp.newtypes.Declaration;
Expand All @@ -42,8 +43,8 @@
import com.google.javascript.jscomp.newtypes.JSTypes;
import com.google.javascript.jscomp.newtypes.Namespace;
import com.google.javascript.jscomp.newtypes.NominalType;
import com.google.javascript.jscomp.newtypes.NominalTypeBuilderNti;
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;
Expand All @@ -55,6 +56,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -287,14 +289,15 @@ 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<>();
// Keyed on RawNominalTypes and property names
private HashBasedTable<RawNominalType, String, PropertyDef> propertyDefs =
HashBasedTable.create();
private final NominalTypeBuilderNti.LateProperties lateProps =
new NominalTypeBuilderNti.LateProperties();
private final Set<RawNominalType> inProgressFreezes = new HashSet<>();
private final GlobalTypeInfo globalTypeInfo;

public GlobalTypeInfoCollector(AbstractCompiler compiler) {
Expand Down Expand Up @@ -512,6 +515,8 @@ private void checkAndFreezeNominalType(RawNominalType rawType) {
if (rawType.isFrozen()) {
return;
}
checkState(inProgressFreezes.add(rawType),
"Cycle in freeze order: %s (%s)", rawType, inProgressFreezes);
NominalType superClass = rawType.getSuperClass();
Set<String> nonInheritedPropNames = rawType.getAllNonInheritedProps();
if (superClass != null && !superClass.isFrozen()) {
Expand All @@ -523,6 +528,11 @@ private void checkAndFreezeNominalType(RawNominalType rawType) {
}
}

for (RawNominalType prerequisite : lateProps.prerequisites(rawType)) {
checkAndFreezeNominalType(prerequisite);
}
lateProps.defineProperties(rawType);

Multimap<String, DeclaredFunctionType> propMethodTypesToProcess = LinkedHashMultimap.create();
Multimap<String, JSType> propTypesToProcess = LinkedHashMultimap.create();
// Collect inherited types for extended classes
Expand Down Expand Up @@ -636,7 +646,6 @@ private void checkAndFreezeNominalType(RawNominalType rawType) {
// Warn for a prop declared with @override that isn't overriding anything.
for (String pname : nonInheritedPropNames) {
PropertyDef propDef = propertyDefs.get(rawType, pname);
checkState(propDef != null || rawType.getName().equals(WINDOW_CLASS));
if (propDef != null) {
Node propDefsite = propDef.defSite;
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(propDefsite);
Expand All @@ -654,6 +663,7 @@ private void checkAndFreezeNominalType(RawNominalType rawType) {
literalObj.getRawNominalType().freeze();
}
}
inProgressFreezes.remove(rawType);
}

// TODO(dimvar): the finalization method and this one should be cleaned up;
Expand Down Expand Up @@ -1566,28 +1576,58 @@ private void visitObjectLit(Node objLitNode, Node parent) {

private void visitCall(Node call) {
// Check various coding conventions to see if any additional handling is needed.
SubclassRelationship relationship = convention.getClassesDefinedByCall(call);
if (relationship != null) {
applySubclassRelationship(relationship);
}
String className = convention.getSingletonGetterClassName(call);
if (className != null) {
applySingletonGetter(className);
}
}

private void applySubclassRelationship(SubclassRelationship rel) {
if (rel.superclassName.equals(rel.subclassName)) {
// Note: this is a messed up situation, but it's dealt with elsewhere, so let it go here.
return;
}
RawNominalType superClass = findInScope(rel.superclassName);
RawNominalType subClass = findInScope(rel.subclassName);
if (superClass != null && superClass.getConstructorFunction() != null
&& subClass != null && subClass.getConstructorFunction() != null) {
convention.applySubclassRelationship(
new NominalTypeBuilderNti(lateProps, superClass),
new NominalTypeBuilderNti(lateProps, subClass),
rel.type);
}
}

private void applySingletonGetter(String className) {
QualifiedName qname = QualifiedName.fromQualifiedString(className);
RawNominalType rawType = currentScope.getNominalType(qname);
RawNominalType rawType = findInScope(className);
if (rawType != null) {
JSType instanceType = rawType.getInstanceAsJSType();
FunctionType getInstanceFunType =
new FunctionTypeBuilder(getCommonTypes()).addRetType(instanceType).buildFunction();
JSType getInstanceType = getCommonTypes().fromFunctionType(getInstanceFunType);
JSType getInstanceType =
new FunctionTypeBuilder(getCommonTypes()).addRetType(instanceType).buildType();
convention.applySingletonGetter(
PROPERTY_DECLARER,
getCommonTypes().fromFunctionType(rawType.getConstructorFunction()),
getInstanceType,
instanceType);
new NominalTypeBuilderNti(lateProps, rawType), getInstanceType);
}
}

private RawNominalType findInScope(String qname) {
return currentScope.getNominalType(QualifiedName.fromQualifiedString(qname));
}

/** Returns the type of the constructor for a raw type. Null-safe. */
private JSType getConstructor(RawNominalType rawType) {
if (rawType != null) {
FunctionType ctor = rawType.getConstructorFunction();
if (ctor != null) {
return getCommonTypes().fromFunctionType(ctor);
}
}
return null;
}

private void visitPropertyDeclaration(Node getProp) {
recordPropertyName(getProp.getLastChild());
// Property declaration on THIS; most commonly a class property
Expand Down
18 changes: 11 additions & 7 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
View file
Expand Up @@ -71,9 +71,9 @@
import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.jstype.JSTypeNative;
import com.google.javascript.rhino.jstype.JSTypeRegistry;
import com.google.javascript.rhino.jstype.NominalTypeBuilderOti;
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;
Expand Down Expand Up @@ -150,8 +150,6 @@ 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;
Expand Down Expand Up @@ -1499,8 +1497,12 @@ private void checkForClassDefiningCalls(Node n) {
FunctionType superCtor = superClass.getConstructor();
FunctionType subCtor = subClass.getConstructor();
if (superCtor != null && subCtor != null) {
codingConvention.applySubclassRelationship(
PROPERTY_DECLARER, superCtor, subCtor, relationship.type);
try (NominalTypeBuilderOti.Factory factory = new NominalTypeBuilderOti.Factory()) {
codingConvention.applySubclassRelationship(
factory.builder(superCtor, superClass),
factory.builder(subCtor, subClass),
relationship.type);
}
}
}
}
Expand All @@ -1515,8 +1517,10 @@ private void checkForClassDefiningCalls(Node n) {

if (functionType != null) {
FunctionType getterType = typeRegistry.createFunctionType(objectType);
codingConvention.applySingletonGetter(
PROPERTY_DECLARER, functionType, getterType, objectType);
try (NominalTypeBuilderOti.Factory factory = new NominalTypeBuilderOti.Factory()) {
codingConvention.applySingletonGetter(
factory.builder(functionType, objectType), getterType);
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/com/google/javascript/jscomp/newtypes/FunctionTypeBuilder.java
View file
Expand Up @@ -193,4 +193,8 @@ public FunctionType buildFunction() {
requiredFormals, optionalFormals, restFormals, returnType,
nominalType, receiverType, outerVars, typeParameters, loose, isAbstract);
}

public JSType buildType() {
return this.commonTypes.fromFunctionType(buildFunction());
}
}

0 comments on commit 2e55c76

Please sign in to comment.