Skip to content

Commit

Permalink
[NTI] Add an "unresolved type" to the type system.
Browse files Browse the repository at this point in the history
This type is useful for per-library type checking. The semantics is that this type is neither a subtype nor a supertype of any other type. In effect, if you try to use it, you get a warning.

To avoid flowing this type around and having to change all type operations to recognize it, if an expression evaluates to this type, we warn and return ? instead.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=172934055
  • Loading branch information
dimvar authored and lauraharker committed Oct 20, 2017
1 parent 55702a8 commit 2403a08
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 45 deletions.
3 changes: 0 additions & 3 deletions src/com/google/javascript/jscomp/Compiler.java
Expand Up @@ -1543,9 +1543,6 @@ public JSTypeRegistry getTypeRegistry() {


@Override @Override
void forwardDeclareType(String typeName) { void forwardDeclareType(String typeName) {
if (options.dropForwardDeclarations()) {
return;
}
forwardDeclaredTypes.add(typeName); forwardDeclaredTypes.add(typeName);
} }


Expand Down
4 changes: 0 additions & 4 deletions src/com/google/javascript/jscomp/CompilerOptions.java
Expand Up @@ -195,10 +195,6 @@ public boolean allowIjsInputs() {
return incrementalCheckMode != IncrementalCheckMode.OFF; return incrementalCheckMode != IncrementalCheckMode.OFF;
} }


public boolean dropForwardDeclarations() {
return incrementalCheckMode == IncrementalCheckMode.CHECK_IJS && useNewTypeInference;
}

private Config.JsDocParsing parseJsDocDocumentation = Config.JsDocParsing.TYPES_ONLY; private Config.JsDocParsing parseJsDocDocumentation = Config.JsDocParsing.TYPES_ONLY;


private boolean printExterns; private boolean printExterns;
Expand Down
3 changes: 2 additions & 1 deletion src/com/google/javascript/jscomp/GlobalTypeInfo.java
Expand Up @@ -103,7 +103,8 @@ public class GlobalTypeInfo implements TypeIRegistry {
this.getCommonTypes(), this.getCommonTypes(),
compiler.getCodingConvention(), compiler.getCodingConvention(),
this.varNameGen, this.varNameGen,
new RecordPropertyCallBack()); new RecordPropertyCallBack(),
compiler.getOptions().inIncrementalCheckMode());
} }


class RecordPropertyCallBack implements Function<Node, Void>, Serializable { class RecordPropertyCallBack implements Function<Node, Void>, Serializable {
Expand Down
28 changes: 19 additions & 9 deletions src/com/google/javascript/jscomp/NewTypeInference.java
Expand Up @@ -350,6 +350,10 @@ final class NewTypeInference implements CompilerPass {
"JSC_NTI_YIELD_ALL_EXPECTS_ITERABLE", "JSC_NTI_YIELD_ALL_EXPECTS_ITERABLE",
"Expression yield* expects an iterable, found type {0}."); "Expression yield* expects an iterable, found type {0}.");


static final DiagnosticType CANNOT_USE_UNRESOLVED_TYPE = DiagnosticType.warning(
"JSC_CANNOT_USE_UNRESOLVED_TYPE",
"Cannot use unresolved type {0}. Please include the type definition in your application.");

static final DiagnosticGroup COMPATIBLE_DIAGNOSTICS = new DiagnosticGroup( static final DiagnosticGroup COMPATIBLE_DIAGNOSTICS = new DiagnosticGroup(
ABSTRACT_SUPER_METHOD_NOT_CALLABLE, ABSTRACT_SUPER_METHOD_NOT_CALLABLE,
CANNOT_BIND_CTOR, CANNOT_BIND_CTOR,
Expand Down Expand Up @@ -388,14 +392,15 @@ final class NewTypeInference implements CompilerPass {
INVALID_DECLARED_RETURN_TYPE_OF_GENERATOR_FUNCTION); INVALID_DECLARED_RETURN_TYPE_OF_GENERATOR_FUNCTION);


// TODO(dimvar): Check for which of these warnings it makes sense to keep // TODO(dimvar): Check for which of these warnings it makes sense to keep
// going after warning, eg, for NOT_UNIQUE_INSTANTIATION, we must instantiate // going after warning, e.g., for NOT_UNIQUE_INSTANTIATION, we must instantiate
// to the join of the types. // to the join of the types.
static final DiagnosticGroup NEW_DIAGNOSTICS = static final DiagnosticGroup NEW_DIAGNOSTICS =
new DiagnosticGroup( new DiagnosticGroup(
ADDING_PROPERTY_TO_NON_OBJECT, ADDING_PROPERTY_TO_NON_OBJECT,
ASSERT_FALSE, ASSERT_FALSE,
BOTTOM_INDEX_TYPE, BOTTOM_INDEX_TYPE,
BOTTOM_PROP, BOTTOM_PROP,
CANNOT_USE_UNRESOLVED_TYPE,
CROSS_SCOPE_GOTCHA, CROSS_SCOPE_GOTCHA,
FORIN_EXPECTS_OBJECT, FORIN_EXPECTS_OBJECT,
INCOMPATIBLE_STRICT_COMPARISON, INCOMPATIBLE_STRICT_COMPARISON,
Expand Down Expand Up @@ -457,6 +462,8 @@ void add(JSError warning) {
private final boolean joinTypesWhenInstantiatingGenerics; private final boolean joinTypesWhenInstantiatingGenerics;
private final boolean allowPropertyOnSubtypes; private final boolean allowPropertyOnSubtypes;
private final boolean areTypeVariablesUnknown; private final boolean areTypeVariablesUnknown;
// Used in per-library type checking
private final boolean warnForUnresolvedTypes;


// Used only for development // Used only for development
private static boolean showDebuggingPrints = false; private static boolean showDebuggingPrints = false;
Expand Down Expand Up @@ -518,6 +525,7 @@ void add(JSError warning) {
this.joinTypesWhenInstantiatingGenerics = inCompatibilityMode; this.joinTypesWhenInstantiatingGenerics = inCompatibilityMode;
this.allowPropertyOnSubtypes = inCompatibilityMode; this.allowPropertyOnSubtypes = inCompatibilityMode;
this.areTypeVariablesUnknown = inCompatibilityMode; this.areTypeVariablesUnknown = inCompatibilityMode;
this.warnForUnresolvedTypes = compiler.getOptions().inIncrementalCheckMode();
} }


@VisibleForTesting // Only used from tests @VisibleForTesting // Only used from tests
Expand Down Expand Up @@ -1640,18 +1648,20 @@ private EnvTypePair analyzeExprFwd(
default: default:
throw new RuntimeException("Unhandled expression type: " + expr.getToken()); throw new RuntimeException("Unhandled expression type: " + expr.getToken());
} }
mayWarnAboutUnknownType(expr, resultPair.type); JSType resultType = resultPair.type;
// TODO(dimvar): We attach the type to every expression because that's also mayWarnAboutUnknownType(expr, resultType);
// what the old type inference does. if (resultType.isUnresolved()) {
// But maybe most of these types are never looked at. if (this.warnForUnresolvedTypes) {
// See if the passes that use types only need types on a small subset of the warnings.add(JSError.make(expr, CANNOT_USE_UNRESOLVED_TYPE, resultType.toString()));
// nodes, and only store these types to save mem. }
maybeSetTypeI(expr, resultPair.type); resultPair.type = UNKNOWN;
}
maybeSetTypeI(expr, resultType);
if (this.currentScope.isFunction()) { if (this.currentScope.isFunction()) {
// In global scope, the env is too big and produces too much output // In global scope, the env is too big and produces too much output
println("AnalyzeExprFWD: ", expr, println("AnalyzeExprFWD: ", expr,
" ::reqtype: ", requiredType, " ::spectype: ", specializedType, " ::reqtype: ", requiredType, " ::spectype: ", specializedType,
" ::resulttype: ", resultPair.type); " ::resulttype: ", resultType);
} }
return resultPair; return resultPair;
} }
Expand Down
31 changes: 23 additions & 8 deletions src/com/google/javascript/jscomp/newtypes/JSType.java
Expand Up @@ -78,6 +78,14 @@ public abstract class JSType implements TypeI, FunctionTypeI, ObjectTypeI {
private static final int TRUTHY_MASK = 0x200; private static final int TRUTHY_MASK = 0x200;
private static final int FALSY_MASK = 0x400; private static final int FALSY_MASK = 0x400;
// Room to grow. // Room to grow.

/**
* The unresolved type is useful for per-library type checking. This type is neither a subtype
* nor a supertype of any other type. In effect, if you try to use it, you get a warning.
* To avoid flowing this type around and having to change all type operations to recognize it,
* if an expression evaluates to this type, we warn and return ? instead.
*/
private static final int UNRESOLVED_MASK = 0x6fffffff;
private static final int UNKNOWN_MASK = 0x7fffffff; // @type {?} private static final int UNKNOWN_MASK = 0x7fffffff; // @type {?}
private static final int TOP_MASK = 0xffffffff; // @type {*} private static final int TOP_MASK = 0xffffffff; // @type {*}


Expand Down Expand Up @@ -169,6 +177,8 @@ static JSType makeMaskType(JSTypes commonTypes, int mask) {
return commonTypes.FALSY; return commonTypes.FALSY;
case UNKNOWN_MASK: case UNKNOWN_MASK:
return commonTypes.UNKNOWN; return commonTypes.UNKNOWN;
case UNRESOLVED_MASK:
return commonTypes.UNRESOLVED;
case TOP_MASK: case TOP_MASK:
return commonTypes.TOP; return commonTypes.TOP;
case BOOLEAN_MASK: case BOOLEAN_MASK:
Expand Down Expand Up @@ -260,6 +270,7 @@ static Map<String, JSType> createScalars(JSTypes commonTypes) {
types.put("TRUTHY", new MaskType(commonTypes, TRUTHY_MASK)); types.put("TRUTHY", new MaskType(commonTypes, TRUTHY_MASK));
types.put("UNDEFINED", new MaskType(commonTypes, UNDEFINED_MASK)); types.put("UNDEFINED", new MaskType(commonTypes, UNDEFINED_MASK));
types.put("UNKNOWN", new MaskType(commonTypes, UNKNOWN_MASK)); types.put("UNKNOWN", new MaskType(commonTypes, UNKNOWN_MASK));
types.put("UNRESOLVED", new MaskType(commonTypes, UNRESOLVED_MASK));


types.put("UNDEFINED_OR_BOOLEAN", new MaskType(commonTypes, UNDEFINED_OR_BOOLEAN_MASK)); types.put("UNDEFINED_OR_BOOLEAN", new MaskType(commonTypes, UNDEFINED_OR_BOOLEAN_MASK));
types.put("UNDEFINED_OR_NUMBER", new MaskType(commonTypes, UNDEFINED_OR_NUMBER_MASK)); types.put("UNDEFINED_OR_NUMBER", new MaskType(commonTypes, UNDEFINED_OR_NUMBER_MASK));
Expand Down Expand Up @@ -611,12 +622,15 @@ public static JSType join(JSType lhs, JSType rhs) {
checkNotNull(lhs); checkNotNull(lhs);
checkNotNull(rhs); checkNotNull(rhs);
JSTypes commonTypes = lhs.commonTypes; JSTypes commonTypes = lhs.commonTypes;
if (lhs.isTop() || rhs.isTop()) { if (lhs.isUnresolved() || rhs.isUnresolved()) {
return commonTypes.TOP; return commonTypes.UNRESOLVED;
} }
if (lhs.isUnknown() || rhs.isUnknown()) { if (lhs.isUnknown() || rhs.isUnknown()) {
return commonTypes.UNKNOWN; return commonTypes.UNKNOWN;
} }
if (lhs.isTop() || rhs.isTop()) {
return commonTypes.TOP;
}
if (lhs.isBottom()) { if (lhs.isBottom()) {
return rhs; return rhs;
} }
Expand Down Expand Up @@ -1257,6 +1271,9 @@ private boolean isSubtypeOfHelper(
if (isUnknown() || other.isUnknown() || other.isTop()) { if (isUnknown() || other.isUnknown() || other.isTop()) {
return true; return true;
} }
if (other.isUnresolved()) {
return false;
}
if (isTheTruthyType()) { if (isTheTruthyType()) {
return !other.makeTruthy().isBottom(); return !other.makeTruthy().isBottom();
} }
Expand Down Expand Up @@ -1327,7 +1344,7 @@ public final JSType removeType(JSType other) {
!other.isTop() && !other.isUnknown() !other.isTop() && !other.isUnknown()
&& (otherMask & TYPEVAR_MASK) == 0 && (otherMask & ENUM_MASK) == 0, && (otherMask & TYPEVAR_MASK) == 0 && (otherMask & ENUM_MASK) == 0,
"Requested invalid type to remove: %s", other); "Requested invalid type to remove: %s", other);
if (isUnknown()) { if (isUnknown() || isUnresolved()) {
return this; return this;
} }
if (isTop()) { if (isTop()) {
Expand Down Expand Up @@ -1621,6 +1638,8 @@ StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
return builder.append("*"); return builder.append("*");
case UNKNOWN_MASK: case UNKNOWN_MASK:
return builder.append("?"); return builder.append("?");
case UNRESOLVED_MASK:
return builder.append("unresolved");
default: default:
if (isUnion) { if (isUnion) {
builder.append("("); builder.append("(");
Expand Down Expand Up @@ -1758,11 +1777,7 @@ public final boolean isSomeUnknownType() {


@Override @Override
public final boolean isUnresolved() { public final boolean isUnresolved() {
// TODO(aravindpg): This is purely a stub to ensure we never get into a codepath that return getMask() == UNRESOLVED_MASK;
// depends on us being an unresolved type. We currently do not mark unresolved types as such
// in NTI since the main use-case (warning for unfulfilled forward declares) can be
// handled differently (by warning after GTI), so we don't want to change the type system.
return false;
} }


@Override @Override
Expand Down
Expand Up @@ -210,17 +210,21 @@ public final class JSTypeCreatorFromJSDoc implements Serializable {
private final Set<JSError> warnings = new LinkedHashSet<>(); private final Set<JSError> warnings = new LinkedHashSet<>();
// Unknown type names indexed by JSDoc AST node at which they were found. // Unknown type names indexed by JSDoc AST node at which they were found.
private final Map<Node, String> unknownTypeNames = new LinkedHashMap<>(); private final Map<Node, String> unknownTypeNames = new LinkedHashMap<>();
// If true, unresolved forward declares become UNRESOLVED, otherwise UNKNOWN.
private final boolean createUnresolvedTypes;


public JSTypeCreatorFromJSDoc(JSTypes commonTypes, public JSTypeCreatorFromJSDoc(JSTypes commonTypes,
CodingConvention convention, UniqueNameGenerator nameGen, CodingConvention convention, UniqueNameGenerator nameGen,
Function<Node, Void> recordPropertyName) { Function<Node, Void> recordPropertyName,
boolean createUnresolvedTypes) {
checkNotNull(commonTypes); checkNotNull(commonTypes);
this.commonTypes = commonTypes; this.commonTypes = commonTypes;
this.qmarkFunctionDeclared = new FunctionAndSlotType( this.qmarkFunctionDeclared = new FunctionAndSlotType(
null, DeclaredFunctionType.qmarkFunctionDeclaration(commonTypes)); null, DeclaredFunctionType.qmarkFunctionDeclaration(commonTypes));
this.convention = convention; this.convention = convention;
this.nameGen = nameGen; this.nameGen = nameGen;
this.recordPropertyName = recordPropertyName; this.recordPropertyName = recordPropertyName;
this.createUnresolvedTypes = createUnresolvedTypes;
} }


private final FunctionAndSlotType qmarkFunctionDeclared; private final FunctionAndSlotType qmarkFunctionDeclared;
Expand Down Expand Up @@ -468,7 +472,7 @@ private JSType lookupTypeByName(String name, Node n,
return getNominalTypeHelper(decl.getNominal(), n, registry, outerTypeParameters); return getNominalTypeHelper(decl.getNominal(), n, registry, outerTypeParameters);
} }
// Forward-declared type // Forward-declared type
return this.commonTypes.UNKNOWN; return this.createUnresolvedTypes ? this.commonTypes.UNRESOLVED : this.commonTypes.UNKNOWN;
} }


private JSType getTypedefType(Typedef td, DeclaredTypeRegistry registry) { private JSType getTypedefType(Typedef td, DeclaredTypeRegistry registry) {
Expand Down
3 changes: 3 additions & 0 deletions src/com/google/javascript/jscomp/newtypes/JSTypes.java
Expand Up @@ -91,6 +91,8 @@ public final class JSTypes implements Serializable {
public final JSType UNDEFINED; public final JSType UNDEFINED;
@SuppressWarnings("ConstantField") @SuppressWarnings("ConstantField")
public final JSType UNKNOWN; public final JSType UNKNOWN;
@SuppressWarnings("ConstantField")
public final JSType UNRESOLVED;


private ObjectType topObjectType; private ObjectType topObjectType;
@SuppressWarnings("ConstantField") @SuppressWarnings("ConstantField")
Expand Down Expand Up @@ -188,6 +190,7 @@ private JSTypes(boolean inCompatibilityMode) {
this.TRUTHY = checkNotNull(types.get("TRUTHY")); this.TRUTHY = checkNotNull(types.get("TRUTHY"));
this.UNDEFINED = checkNotNull(types.get("UNDEFINED")); this.UNDEFINED = checkNotNull(types.get("UNDEFINED"));
this.UNKNOWN = checkNotNull(types.get("UNKNOWN")); this.UNKNOWN = checkNotNull(types.get("UNKNOWN"));
this.UNRESOLVED = checkNotNull(types.get("UNRESOLVED"));


this.UNDEFINED_OR_BOOLEAN = checkNotNull(types.get("UNDEFINED_OR_BOOLEAN")); this.UNDEFINED_OR_BOOLEAN = checkNotNull(types.get("UNDEFINED_OR_BOOLEAN"));
this.UNDEFINED_OR_NUMBER = checkNotNull(types.get("UNDEFINED_OR_NUMBER")); this.UNDEFINED_OR_NUMBER = checkNotNull(types.get("UNDEFINED_OR_NUMBER"));
Expand Down
Expand Up @@ -1516,7 +1516,7 @@ public void testCustomBanUnresolvedType() {
// addresses this use case. // addresses this use case.
this.mode = TypeInferenceMode.OTI_ONLY; this.mode = TypeInferenceMode.OTI_ONLY;
testWarning( testWarning(
"goog.forwardDeclare('Foo');" + "/** @param {Foo} a */ function f(a) {a.foo()};", "goog.forwardDeclare('Foo'); /** @param {Foo} a */ function f(a) {a.foo()};",
CheckConformance.CONFORMANCE_VIOLATION, CheckConformance.CONFORMANCE_VIOLATION,
"Violation: BanUnresolvedType Message"); "Violation: BanUnresolvedType Message");


Expand Down

0 comments on commit 2403a08

Please sign in to comment.