Skip to content

Commit

Permalink
[NTI] Finish support for abstract classes and methods.
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=139624919
  • Loading branch information
@dimvar @blickly
dimvar authored and blickly committed Nov 19, 2016
1 parent 7ef9044 commit 1ab5aca
Show file tree
Hide file tree
Showing 9 changed files with 278 additions and 29 deletions.
2 changes: 2 additions & 0 deletions src/com/google/javascript/jscomp/DiagnosticGroups.java
View file
Expand Up @@ -313,6 +313,7 @@ public DiagnosticGroup forName(String name) {
JSTypeCreatorFromJSDoc.IMPLEMENTS_WITHOUT_CONSTRUCTOR, JSTypeCreatorFromJSDoc.IMPLEMENTS_WITHOUT_CONSTRUCTOR,
JSTypeCreatorFromJSDoc.INHERITANCE_CYCLE, JSTypeCreatorFromJSDoc.INHERITANCE_CYCLE,
JSTypeCreatorFromJSDoc.UNION_IS_UNINHABITABLE, JSTypeCreatorFromJSDoc.UNION_IS_UNINHABITABLE,
GlobalTypeInfo.ABSTRACT_METHOD_IN_CONCRETE_CLASS,
GlobalTypeInfo.ANONYMOUS_NOMINAL_TYPE, GlobalTypeInfo.ANONYMOUS_NOMINAL_TYPE,
GlobalTypeInfo.CANNOT_INIT_TYPEDEF, GlobalTypeInfo.CANNOT_INIT_TYPEDEF,
GlobalTypeInfo.CANNOT_OVERRIDE_FINAL_METHOD, GlobalTypeInfo.CANNOT_OVERRIDE_FINAL_METHOD,
Expand All @@ -336,6 +337,7 @@ public DiagnosticGroup forName(String name) {
GlobalTypeInfo.SUPER_INTERFACES_HAVE_INCOMPATIBLE_PROPERTIES, GlobalTypeInfo.SUPER_INTERFACES_HAVE_INCOMPATIBLE_PROPERTIES,
GlobalTypeInfo.UNKNOWN_OVERRIDE, GlobalTypeInfo.UNKNOWN_OVERRIDE,
GlobalTypeInfo.UNRECOGNIZED_TYPE_NAME, GlobalTypeInfo.UNRECOGNIZED_TYPE_NAME,
NewTypeInference.ABSTRACT_METHOD_NOT_CALLABLE,
NewTypeInference.ASSERT_FALSE, NewTypeInference.ASSERT_FALSE,
NewTypeInference.CANNOT_BIND_CTOR, NewTypeInference.CANNOT_BIND_CTOR,
NewTypeInference.CONST_REASSIGNED, NewTypeInference.CONST_REASSIGNED,
Expand Down
40 changes: 38 additions & 2 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
View file
Expand Up @@ -224,7 +224,24 @@ class GlobalTypeInfo implements CompilerPass, TypeIRegistry {
"JSC_NTI_INTERFACE_METHOD_NOT_EMPTY", "JSC_NTI_INTERFACE_METHOD_NOT_EMPTY",
"interface member functions must have an empty body"); "interface member functions must have an empty body");


static final DiagnosticType ABSTRACT_METHOD_IN_CONCRETE_CLASS =
DiagnosticType.warning(
"JSC_NTI_ABSTRACT_METHOD_IN_CONCRETE_CLASS",
"Abstract methods can only appear in abstract classes. "
+ "Please declare class {0} as @abstract");

static final DiagnosticType ABSTRACT_METHOD_IN_INTERFACE =
DiagnosticType.warning(
"JSC_NTI_ABSTRACT_METHOD_IN_INTERFACE",
"Abstract methods cannot appear in interfaces");

static final DiagnosticType ABSTRACT_METHOD_NOT_IMPLEMENTED_IN_CONCRETE_CLASS =
DiagnosticType.warning(
"JSC_NTI_ABSTRACT_METHOD_NOT_IMPLEMENTED_IN_CONCRETE_CLASS",
"Abstract method {0} from superclass {1} not implemented");

static final DiagnosticGroup COMPATIBLE_DIAGNOSTICS = new DiagnosticGroup( static final DiagnosticGroup COMPATIBLE_DIAGNOSTICS = new DiagnosticGroup(
ABSTRACT_METHOD_IN_CONCRETE_CLASS,
CANNOT_OVERRIDE_FINAL_METHOD, CANNOT_OVERRIDE_FINAL_METHOD,
DICT_WITHOUT_CTOR, DICT_WITHOUT_CTOR,
DUPLICATE_PROP_IN_ENUM, DUPLICATE_PROP_IN_ENUM,
Expand All @@ -246,6 +263,8 @@ class GlobalTypeInfo implements CompilerPass, TypeIRegistry {
WRONG_PARAMETER_COUNT); WRONG_PARAMETER_COUNT);


static final DiagnosticGroup NEW_DIAGNOSTICS = new DiagnosticGroup( static final DiagnosticGroup NEW_DIAGNOSTICS = new DiagnosticGroup(
ABSTRACT_METHOD_IN_INTERFACE,
ABSTRACT_METHOD_NOT_IMPLEMENTED_IN_CONCRETE_CLASS,
ANONYMOUS_NOMINAL_TYPE, ANONYMOUS_NOMINAL_TYPE,
CANNOT_ADD_PROPERTIES_TO_TYPEDEF, CANNOT_ADD_PROPERTIES_TO_TYPEDEF,
CANNOT_INIT_TYPEDEF, CANNOT_INIT_TYPEDEF,
Expand Down Expand Up @@ -546,6 +565,14 @@ private void checkAndFinalizeNominalType(RawNominalType rawType) {
Preconditions.checkState(superClass.isFinalized()); Preconditions.checkState(superClass.isFinalized());
// TODO(blickly): Can we optimize this to skip unnecessary iterations? // TODO(blickly): Can we optimize this to skip unnecessary iterations?
for (String pname : superClass.getAllPropsOfClass()) { for (String pname : superClass.getAllPropsOfClass()) {
if (superClass.isAbstractClass()
&& superClass.hasAbstractMethod(pname)
&& !rawType.isAbstractClass()
&& !rawType.mayHaveOwnProp(pname)) {
warnings.add(JSError.make(
rawType.getDefSite(), ABSTRACT_METHOD_NOT_IMPLEMENTED_IN_CONCRETE_CLASS,
pname, superClass.getName()));
}
nonInheritedPropNames.remove(pname); nonInheritedPropNames.remove(pname);
checkSuperProperty(rawType, superClass, pname, checkSuperProperty(rawType, superClass, pname,
propMethodTypesToProcess, propTypesToProcess); propMethodTypesToProcess, propTypesToProcess);
Expand Down Expand Up @@ -1128,7 +1155,7 @@ private void maybeRecordNominalType(
? ObjectKind.STRUCT : (fnDoc.makesDicts() ? ObjectKind.DICT : ObjectKind.UNRESTRICTED); ? ObjectKind.STRUCT : (fnDoc.makesDicts() ? ObjectKind.DICT : ObjectKind.UNRESTRICTED);
if (fnDoc.isConstructor()) { if (fnDoc.isConstructor()) {
rawType = RawNominalType.makeClass( rawType = RawNominalType.makeClass(
commonTypes, defSite, qname, typeParameters, objKind); commonTypes, defSite, qname, typeParameters, objKind, fnDoc.isAbstract());
} else if (fnDoc.usesImplicitMatch()) { } else if (fnDoc.usesImplicitMatch()) {
rawType = RawNominalType.makeStructuralInterface( rawType = RawNominalType.makeStructuralInterface(
commonTypes, defSite, qname, typeParameters, objKind); commonTypes, defSite, qname, typeParameters, objKind);
Expand Down Expand Up @@ -1175,7 +1202,7 @@ private void maybeRecordBuiltinType(
// Create a separate raw type for object literals // Create a separate raw type for object literals
RawNominalType objLitRawType = RawNominalType.makeClass( RawNominalType objLitRawType = RawNominalType.makeClass(
commonTypes, rawType.getDefSite(), JSTypes.OBJLIT_CLASS_NAME, commonTypes, rawType.getDefSite(), JSTypes.OBJLIT_CLASS_NAME,
ImmutableList.<String>of(), ObjectKind.UNRESTRICTED); ImmutableList.<String>of(), ObjectKind.UNRESTRICTED, false);
objLitRawType.addSuperClass(rawType.getAsNominalType()); objLitRawType.addSuperClass(rawType.getAsNominalType());
commonTypes.setLiteralObjNominalType(objLitRawType); commonTypes.setLiteralObjNominalType(objLitRawType);
break; break;
Expand Down Expand Up @@ -2410,6 +2437,15 @@ private void mayAddPropToPrototype(
} }
propertyDefs.put(rawType, pname, new PropertyDef(defSite, methodType, methodScope)); propertyDefs.put(rawType, pname, new PropertyDef(defSite, methodType, methodScope));


// Warn for abstract methods not in abstract classes
if (methodType != null && methodType.isAbstract() && !rawType.isAbstractClass()) {
if (rawType.isClass()) {
warnings.add(JSError.make(defSite, ABSTRACT_METHOD_IN_CONCRETE_CLASS, rawType.getName()));
} else if (rawType.isInterface()) {
warnings.add(JSError.make(defSite, ABSTRACT_METHOD_IN_INTERFACE));
}
}

// Add the property to the class with the appropriate type. // Add the property to the class with the appropriate type.
boolean isConst = isConst(defSite); boolean isConst = isConst(defSite);
if (propDeclType != null || isConst) { if (propDeclType != null || isConst) {
Expand Down
33 changes: 22 additions & 11 deletions src/com/google/javascript/jscomp/NewTypeInference.java
View file
Expand Up @@ -170,9 +170,9 @@ final class NewTypeInference implements CompilerPass {
"JSC_NTI_NOT_A_CONSTRUCTOR", "JSC_NTI_NOT_A_CONSTRUCTOR",
"Expected a constructor but found type {0}."); "Expected a constructor but found type {0}.");


static final DiagnosticType INSTANTIATE_ABSTRACT_CLASS = static final DiagnosticType CANNOT_INSTANTIATE_ABSTRACT_CLASS =
DiagnosticType.warning( DiagnosticType.warning(
"JSC_NTI_INSTANTIATE_ABSTRACT_CLASS", "JSC_NTI_CANNOT_INSTANTIATE_ABSTRACT_CLASS",
"Cannot instantiate abstract class {0}."); "Cannot instantiate abstract class {0}.");


static final DiagnosticType UNDEFINED_SUPER_CLASS = static final DiagnosticType UNDEFINED_SUPER_CLASS =
Expand Down Expand Up @@ -292,6 +292,11 @@ final class NewTypeInference implements CompilerPass {
+ "left : {0}\n" + "left : {0}\n"
+ "right: {1}"); + "right: {1}");


static final DiagnosticType ABSTRACT_METHOD_NOT_CALLABLE =
DiagnosticType.warning(
"JSC_NTI_ABSTRACT_METHOD_NOT_CALLABLE",
"Abstract method {0} cannot be called");

// Not part of ALL_DIAGNOSTICS because it should not be enabled with // Not part of ALL_DIAGNOSTICS because it should not be enabled with
// --jscomp_error=newCheckTypes. It should only be enabled explicitly. // --jscomp_error=newCheckTypes. It should only be enabled explicitly.


Expand All @@ -306,8 +311,10 @@ final class NewTypeInference implements CompilerPass {
"This {0} expression has the unknown type."); "This {0} expression has the unknown type.");


static final DiagnosticGroup COMPATIBLE_DIAGNOSTICS = new DiagnosticGroup( static final DiagnosticGroup COMPATIBLE_DIAGNOSTICS = new DiagnosticGroup(
ABSTRACT_METHOD_NOT_CALLABLE,
ASSERT_FALSE, ASSERT_FALSE,
CANNOT_BIND_CTOR, CANNOT_BIND_CTOR,
CANNOT_INSTANTIATE_ABSTRACT_CLASS,
CONST_PROPERTY_DELETED, CONST_PROPERTY_DELETED,
CONST_PROPERTY_REASSIGNED, CONST_PROPERTY_REASSIGNED,
CONST_REASSIGNED, CONST_REASSIGNED,
Expand All @@ -321,7 +328,6 @@ final class NewTypeInference implements CompilerPass {
ILLEGAL_PROPERTY_CREATION, ILLEGAL_PROPERTY_CREATION,
IN_USED_WITH_STRUCT, IN_USED_WITH_STRUCT,
INEXISTENT_PROPERTY, INEXISTENT_PROPERTY,
INSTANTIATE_ABSTRACT_CLASS,
INVALID_ARGUMENT_TYPE, INVALID_ARGUMENT_TYPE,
INVALID_CAST, INVALID_CAST,
INVALID_INDEX_TYPE, INVALID_INDEX_TYPE,
Expand Down Expand Up @@ -1928,8 +1934,8 @@ private EnvTypePair analyzeCallNewFwd(
if (!funType.isSomeConstructorOrInterface() || funType.isInterfaceDefinition()) { if (!funType.isSomeConstructorOrInterface() || funType.isInterfaceDefinition()) {
warnings.add(JSError.make(expr, NOT_A_CONSTRUCTOR, funType.toString())); warnings.add(JSError.make(expr, NOT_A_CONSTRUCTOR, funType.toString()));
return analyzeCallNodeArgsFwdWhenError(expr, envAfterCallee); return analyzeCallNodeArgsFwdWhenError(expr, envAfterCallee);
} else if (funType.isAbstract()) { } else if (funType.isConstructorOfAbstractClass()) {
warnings.add(JSError.make(expr, INSTANTIATE_ABSTRACT_CLASS, funType.toString())); warnings.add(JSError.make(expr, CANNOT_INSTANTIATE_ABSTRACT_CLASS, funType.toString()));
return analyzeCallNodeArgsFwdWhenError(expr, envAfterCallee); return analyzeCallNodeArgsFwdWhenError(expr, envAfterCallee);
} }
} }
Expand Down Expand Up @@ -3107,13 +3113,18 @@ private EnvTypePair analyzePropAccessFwd(Node receiver, String pname,
return new EnvTypePair(pair.env, requiredType); return new EnvTypePair(pair.env, requiredType);
} }
FunctionType ft = recvType.getFunTypeIfSingletonObj(); FunctionType ft = recvType.getFunTypeIfSingletonObj();
if (ft != null && pname.equals("call")) { if (ft != null && (pname.equals("call") || pname.equals("apply"))) {
return new EnvTypePair(pair.env, if (ft.isAbstract()) {
commonTypes.fromFunctionType(ft.transformByCallProperty())); // We don't check if the parent of the property access is a call node.
} // This catches calls that are a few nodes away, and also warns on .call/.apply
if (ft != null && pname.equals("apply")) { // accesses that do not result in calls (these should be very rare).
String funName = receiver.isQualifiedName() ? receiver.getQualifiedName() : "";
warnings.add(JSError.make(propAccessNode, ABSTRACT_METHOD_NOT_CALLABLE, funName));
}
return new EnvTypePair(pair.env, return new EnvTypePair(pair.env,
commonTypes.fromFunctionType(ft.transformByApplyProperty())); pname.equals("call")
? commonTypes.fromFunctionType(ft.transformByCallProperty())
: commonTypes.fromFunctionType(ft.transformByApplyProperty()));
} }
if (this.convention.isSuperClassReference(pname)) { if (this.convention.isSuperClassReference(pname)) {
if (ft != null && ft.isUniqueConstructor()) { if (ft != null && ft.isUniqueConstructor()) {
Expand Down
5 changes: 5 additions & 0 deletions src/com/google/javascript/jscomp/newtypes/FunctionType.java
View file
Expand Up @@ -142,6 +142,11 @@ public boolean isAbstract() {
return isAbstract; return isAbstract;
} }


public boolean isConstructorOfAbstractClass() {
return isUniqueConstructor()
&& this.nominalType.getNominalTypeIfSingletonObj().isAbstractClass();
}

static FunctionType normalized( static FunctionType normalized(
JSTypes commonTypes, JSTypes commonTypes,
List<JSType> requiredFormals, List<JSType> requiredFormals,
Expand Down
5 changes: 4 additions & 1 deletion src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java
View file
Expand Up @@ -856,7 +856,10 @@ private DeclaredFunctionType getFunTypeFromTypicalFunctionJsdoc(
getThisOrNewType(thisRoot.getFirstChild(), registry, typeParameters)); getThisOrNewType(thisRoot.getFirstChild(), registry, typeParameters));
} }


builder.addAbstract(jsdoc.isAbstract()); if (!jsdoc.isConstructor()) {
builder.addAbstract(jsdoc.isAbstract());
}

return builder.buildDeclaration(); return builder.buildDeclaration();
} }


Expand Down
8 changes: 8 additions & 0 deletions src/com/google/javascript/jscomp/newtypes/NominalType.java
View file
Expand Up @@ -248,6 +248,10 @@ public boolean isClass() {
return this.rawType.isClass(); return this.rawType.isClass();
} }


public boolean isAbstractClass() {
return this.rawType.isAbstractClass();
}

public boolean isInterface() { public boolean isInterface() {
return this.rawType.isInterface(); return this.rawType.isInterface();
} }
Expand Down Expand Up @@ -355,6 +359,10 @@ boolean mayHaveProp(String pname) {
return this.rawType.mayHaveProp(pname); return this.rawType.mayHaveProp(pname);
} }


public boolean hasAbstractMethod(String pname) {
return this.rawType.hasAbstractMethod(pname);
}

boolean isSubtypeOf(NominalType other, SubtypeCache subSuperMap) { boolean isSubtypeOf(NominalType other, SubtypeCache subSuperMap) {
return isNominalSubtypeOf(other) return isNominalSubtypeOf(other)
|| other.isStructuralInterface() && isStructuralSubtypeOf(other, subSuperMap); || other.isStructuralInterface() && isStructuralSubtypeOf(other, subSuperMap);
Expand Down
26 changes: 19 additions & 7 deletions src/com/google/javascript/jscomp/newtypes/RawNominalType.java
View file
Expand Up @@ -60,6 +60,7 @@ public final class RawNominalType extends Namespace {
private final Set<RawNominalType> subtypes = new LinkedHashSet<>(); private final Set<RawNominalType> subtypes = new LinkedHashSet<>();
private ImmutableSet<NominalType> interfaces = null; private ImmutableSet<NominalType> interfaces = null;
private final Kind kind; private final Kind kind;
private final boolean isAbstractClass;
// Used in GlobalTypeInfo to find type mismatches in the inheritance chain. // Used in GlobalTypeInfo to find type mismatches in the inheritance chain.
private ImmutableSet<String> allProps = null; private ImmutableSet<String> allProps = null;
// In GlobalTypeInfo, we request (wrapped) RawNominalTypes in various // In GlobalTypeInfo, we request (wrapped) RawNominalTypes in various
Expand All @@ -80,8 +81,8 @@ private enum Kind {
} }


private RawNominalType( private RawNominalType(
JSTypes commonTypes, Node defSite, JSTypes commonTypes, Node defSite, String name,
String name, ImmutableList<String> typeParameters, Kind kind, ObjectKind objectKind) { ImmutableList<String> typeParameters, Kind kind, ObjectKind objectKind, boolean isAbstract) {
super(commonTypes, name, defSite); super(commonTypes, name, defSite);
Preconditions.checkNotNull(objectKind); Preconditions.checkNotNull(objectKind);
Preconditions.checkState(isValidDefsite(defSite), "Invalid defsite %s", defSite); Preconditions.checkState(isValidDefsite(defSite), "Invalid defsite %s", defSite);
Expand All @@ -96,6 +97,7 @@ private RawNominalType(
this.kind = isBuiltinHelper(name, "IObject", defSite) ? Kind.RECORD : kind; this.kind = isBuiltinHelper(name, "IObject", defSite) ? Kind.RECORD : kind;
this.objectKind = isBuiltinHelper(name, "IObject", defSite) this.objectKind = isBuiltinHelper(name, "IObject", defSite)
? ObjectKind.UNRESTRICTED : objectKind; ? ObjectKind.UNRESTRICTED : objectKind;
this.isAbstractClass = isAbstract;
this.wrappedAsNominal = new NominalType(ImmutableMap.<String, JSType>of(), this); this.wrappedAsNominal = new NominalType(ImmutableMap.<String, JSType>of(), this);
ObjectType objInstance; ObjectType objInstance;


Expand Down Expand Up @@ -128,10 +130,10 @@ private static boolean isValidDefsite(Node defSite) {
return false; return false;
} }


public static RawNominalType makeClass(JSTypes commonTypes, public static RawNominalType makeClass(JSTypes commonTypes, Node defSite, String name,
Node defSite, String name, ImmutableList<String> typeParameters, ObjectKind objKind) { ImmutableList<String> typeParameters, ObjectKind objKind, boolean isAbstract) {
return new RawNominalType( return new RawNominalType(
commonTypes, defSite, name, typeParameters, Kind.CLASS, objKind); commonTypes, defSite, name, typeParameters, Kind.CLASS, objKind, isAbstract);
} }


public static RawNominalType makeNominalInterface(JSTypes commonTypes, public static RawNominalType makeNominalInterface(JSTypes commonTypes,
Expand All @@ -140,7 +142,7 @@ public static RawNominalType makeNominalInterface(JSTypes commonTypes,
objKind = ObjectKind.UNRESTRICTED; objKind = ObjectKind.UNRESTRICTED;
} }
return new RawNominalType( return new RawNominalType(
commonTypes, defSite, name, typeParameters, Kind.INTERFACE, objKind); commonTypes, defSite, name, typeParameters, Kind.INTERFACE, objKind, false);
} }


public static RawNominalType makeStructuralInterface(JSTypes commonTypes, public static RawNominalType makeStructuralInterface(JSTypes commonTypes,
Expand All @@ -149,7 +151,7 @@ public static RawNominalType makeStructuralInterface(JSTypes commonTypes,
objKind = ObjectKind.UNRESTRICTED; objKind = ObjectKind.UNRESTRICTED;
} }
return new RawNominalType( return new RawNominalType(
commonTypes, defSite, name, typeParameters, Kind.RECORD, objKind); commonTypes, defSite, name, typeParameters, Kind.RECORD, objKind, false);
} }


JSTypes getCommonTypes() { JSTypes getCommonTypes() {
Expand Down Expand Up @@ -196,6 +198,10 @@ public boolean isDict() {
return this.objectKind.isDict(); return this.objectKind.isDict();
} }


public boolean isAbstractClass() {
return this.isAbstractClass;
}

public boolean isFinalized() { public boolean isFinalized() {
return this.isFinalized; return this.isFinalized;
} }
Expand Down Expand Up @@ -373,6 +379,12 @@ public JSType getProtoPropDeclaredType(String pname) {
return null; return null;
} }


public boolean hasAbstractMethod(String pname) {
Property p = getPropFromClass(pname);
JSType ptype = p == null ? null : p.getType();
return ptype != null && ptype.isFunctionType() && ptype.getFunType().isAbstract();
}

private Property getPropFromClass(String pname) { private Property getPropFromClass(String pname) {
Preconditions.checkState(isClass()); Preconditions.checkState(isClass());
Property p = getOwnProp(pname); Property p = getOwnProp(pname);
Expand Down

0 comments on commit 1ab5aca

Please sign in to comment.