Skip to content

Commit

Permalink
Automated rollback of changelist 149580744.
Browse files Browse the repository at this point in the history 
*** Original change description ***

[NTI] Move code that registers mismatches from TypeValidator to TypeMismatch, and convert it to TypeI.

This is needed so that we start registering mismatches from the new type inference.
Also, clean up some methods with overlapping functionality that check whether some type is a variant of a built-in object type.

***

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=149597271
  • Loading branch information
@dimvar
dimvar committed Mar 9, 2017
1 parent 070d55b commit 93c3d90
Show file tree
Hide file tree
Showing 15 changed files with 110 additions and 179 deletions.
4 changes: 2 additions & 2 deletions src/com/google/javascript/jscomp/AccessControlUtils.java
View file
Expand Up @@ -60,7 +60,7 @@ static Visibility getEffectiveNameVisibility(Node name, Var var,
Visibility defaultVisibilityForFile = Visibility defaultVisibilityForFile =
fileVisibilityMap.get(var.getSourceFile()); fileVisibilityMap.get(var.getSourceFile());
TypeI type = name.getTypeI(); TypeI type = name.getTypeI();
boolean createdFromGoogProvide = (type != null && type.isLiteralObject()); boolean createdFromGoogProvide = (type != null && type.isInstanceofObject());
// Ignore @fileoverview visibility when computing the effective visibility // Ignore @fileoverview visibility when computing the effective visibility
// for names created by goog.provide. // for names created by goog.provide.
// //
Expand Down Expand Up @@ -205,7 +205,7 @@ private static Visibility getEffectiveVisibilityForNonOverriddenProperty(
raw = objectType.getOwnPropertyJSDocInfo(propertyName).getVisibility(); raw = objectType.getOwnPropertyJSDocInfo(propertyName).getVisibility();
} }
TypeI type = getprop.getTypeI(); TypeI type = getprop.getTypeI();
boolean createdFromGoogProvide = (type != null && type.isLiteralObject()); boolean createdFromGoogProvide = (type != null && type.isInstanceofObject());
// Ignore @fileoverview visibility when computing the effective visibility // Ignore @fileoverview visibility when computing the effective visibility
// for properties created by goog.provide. // for properties created by goog.provide.
// //
Expand Down
9 changes: 1 addition & 8 deletions src/com/google/javascript/jscomp/Compiler.java
View file
Expand Up @@ -1433,14 +1433,7 @@ TypeValidator getTypeValidator() {


@Override @Override
Iterable<TypeMismatch> getTypeMismatches() { Iterable<TypeMismatch> getTypeMismatches() {
switch (this.mostRecentTypechecker) { return getTypeValidator().getMismatches();
case OTI:
return getTypeValidator().getMismatches();
case NTI:
return getSymbolTable().getMismatches();
default:
throw new RuntimeException("Can't ask for type mismatches before type checking.");
}
} }


@Override @Override
Expand Down
6 changes: 0 additions & 6 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
View file
Expand Up @@ -282,7 +282,6 @@ class GlobalTypeInfo implements CompilerPass, TypeIRegistry {
private final List<NTIScope> scopes = new ArrayList<>(); private final List<NTIScope> scopes = new ArrayList<>();
private NTIScope globalScope; private NTIScope globalScope;
private WarningReporter warnings; private WarningReporter warnings;
private final List<TypeMismatch> mismatches;
private final JSTypeCreatorFromJSDoc typeParser; private final JSTypeCreatorFromJSDoc typeParser;
private final AbstractCompiler compiler; private final AbstractCompiler compiler;
private final CodingConvention convention; private final CodingConvention convention;
Expand Down Expand Up @@ -326,7 +325,6 @@ class GlobalTypeInfo implements CompilerPass, TypeIRegistry {
compiler.getOptions().disables(DiagnosticGroups.NEW_CHECK_TYPES_EXTRA_CHECKS); compiler.getOptions().disables(DiagnosticGroups.NEW_CHECK_TYPES_EXTRA_CHECKS);


this.warnings = new WarningReporter(compiler); this.warnings = new WarningReporter(compiler);
this.mismatches = new ArrayList<>();
this.compiler = compiler; this.compiler = compiler;
this.unknownTypeNames = unknownTypeNames; this.unknownTypeNames = unknownTypeNames;
this.convention = compiler.getCodingConvention(); this.convention = compiler.getCodingConvention();
Expand Down Expand Up @@ -358,10 +356,6 @@ JSTypes getCommonTypes() {
return this.commonTypes; return this.commonTypes;
} }


List<TypeMismatch> getMismatches() {
return this.mismatches;
}

JSType getCastType(Node n) { JSType getCastType(Node n) {
JSType t = castTypes.get(n); JSType t = castTypes.get(n);
Preconditions.checkNotNull(t); Preconditions.checkNotNull(t);
Expand Down
60 changes: 0 additions & 60 deletions src/com/google/javascript/jscomp/TypeMismatch.java
View file
Expand Up @@ -15,11 +15,7 @@
*/ */
package com.google.javascript.jscomp; package com.google.javascript.jscomp;


import com.google.javascript.rhino.FunctionTypeI;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.TypeI; import com.google.javascript.rhino.TypeI;
import java.util.Iterator;
import java.util.List;
import java.util.Objects; import java.util.Objects;


/** /**
Expand All @@ -45,62 +41,6 @@ class TypeMismatch {
this.src = src; this.src = src;
} }


static void registerIfMismatch(
List<TypeMismatch> mismatches, List<TypeMismatch> implicitInterfaceUses,
TypeI found, TypeI required, JSError error) {
if (found != null && required != null && !found.isSubtypeWithoutStructuralTyping(required)) {
registerMismatch(mismatches, implicitInterfaceUses, found, required, error);
}
}

static void registerMismatch(
List<TypeMismatch> mismatches, List<TypeMismatch> implicitInterfaceUses,
TypeI found, TypeI required, JSError error) {
// Don't register a mismatch for differences in null or undefined or if the
// code didn't downcast.
found = found.restrictByNotNullOrUndefined();
required = required.restrictByNotNullOrUndefined();

if (found.isSubtypeOf(required) || required.isSubtypeOf(found)) {
boolean strictMismatch =
!found.isSubtypeWithoutStructuralTyping(required)
&& !required.isSubtypeWithoutStructuralTyping(found);
if (strictMismatch) {
implicitInterfaceUses.add(new TypeMismatch(found, required, error));
}
return;
}

mismatches.add(new TypeMismatch(found, required, error));

if (found.isFunctionType() && required.isFunctionType()) {
FunctionTypeI fnTypeA = found.toMaybeFunctionType();
FunctionTypeI fnTypeB = required.toMaybeFunctionType();
Iterator<TypeI> paramItA = fnTypeA.getParameterTypes().iterator();
Iterator<TypeI> paramItB = fnTypeB.getParameterTypes().iterator();
while (paramItA.hasNext() && paramItB.hasNext()) {
TypeMismatch.registerIfMismatch(
mismatches, implicitInterfaceUses, paramItA.next(), paramItB.next(), error);
}
TypeMismatch.registerIfMismatch(
mismatches, implicitInterfaceUses,
fnTypeA.getReturnType(), fnTypeB.getReturnType(), error);
}
}

static void recordImplicitUseOfNativeObject(
List<TypeMismatch> mismatches, Node src, TypeI sourceType, TypeI targetType) {
sourceType = sourceType.restrictByNotNullOrUndefined();
targetType = targetType.restrictByNotNullOrUndefined();
if (sourceType.isInstanceofObject() && !targetType.isInstanceofObject()) {
// We don't report a type error, but we still need to construct a JSError,
// for people who enable the invalidation diagnostics in DisambiguateProperties.
String msg = "Implicit use of Object type: " + sourceType + " as type: " + targetType;
JSError err = JSError.make(src, TypeValidator.TYPE_MISMATCH_WARNING, msg);
mismatches.add(new TypeMismatch(sourceType, targetType, err));
}
}

@Override public boolean equals(Object object) { @Override public boolean equals(Object object) {
if (object instanceof TypeMismatch) { if (object instanceof TypeMismatch) {
TypeMismatch that = (TypeMismatch) object; TypeMismatch that = (TypeMismatch) object;
Expand Down
100 changes: 79 additions & 21 deletions src/com/google/javascript/jscomp/TypeValidator.java
View file
Expand Up @@ -44,6 +44,7 @@
import com.google.javascript.rhino.jstype.UnknownType; import com.google.javascript.rhino.jstype.UnknownType;
import java.text.MessageFormat; import java.text.MessageFormat;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Iterator;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
Expand Down Expand Up @@ -364,7 +365,7 @@ void expectSwitchMatchesCase(NodeTraversal t, Node n, JSType switchType,
|| !caseType.autoboxesTo() || !caseType.autoboxesTo()
.isSubtypeWithoutStructuralTyping(switchType))) { .isSubtypeWithoutStructuralTyping(switchType))) {
recordImplicitInterfaceUses(n, caseType, switchType); recordImplicitInterfaceUses(n, caseType, switchType);
TypeMismatch.recordImplicitUseOfNativeObject(this.mismatches, n, caseType, switchType); recordImplicitUseOfNativeObject(n, caseType, switchType);
} }
} }


Expand Down Expand Up @@ -449,9 +450,10 @@ boolean expectCanAssignToPropertyOf(NodeTraversal t, Node n, JSType rightType,
typeRegistry.getReadableTypeName(owner), typeRegistry.getReadableTypeName(owner),
rightType, leftType); rightType, leftType);
return false; return false;
} else if (!leftType.isNoType() && !rightType.isSubtypeWithoutStructuralTyping(leftType)){ } else if (!leftType.isNoType()
&& !rightType.isSubtypeWithoutStructuralTyping(leftType)){
recordImplicitInterfaceUses(n, rightType, leftType); recordImplicitInterfaceUses(n, rightType, leftType);
TypeMismatch.recordImplicitUseOfNativeObject(this.mismatches, n, rightType, leftType); recordImplicitUseOfNativeObject(n, rightType, leftType);
} }
return true; return true;
} }
Expand All @@ -474,7 +476,7 @@ boolean expectCanAssignTo(NodeTraversal t, Node n, JSType rightType,
return false; return false;
} else if (!rightType.isSubtypeWithoutStructuralTyping(leftType)) { } else if (!rightType.isSubtypeWithoutStructuralTyping(leftType)) {
recordImplicitInterfaceUses(n, rightType, leftType); recordImplicitInterfaceUses(n, rightType, leftType);
TypeMismatch.recordImplicitUseOfNativeObject(this.mismatches, n, rightType, leftType); recordImplicitUseOfNativeObject(n, rightType, leftType);
} }
return true; return true;
} }
Expand All @@ -500,7 +502,7 @@ void expectArgumentMatchesParameter(NodeTraversal t, Node n, JSType argType,
argType, paramType); argType, paramType);
} else if (!argType.isSubtypeWithoutStructuralTyping(paramType)){ } else if (!argType.isSubtypeWithoutStructuralTyping(paramType)){
recordImplicitInterfaceUses(n, argType, paramType); recordImplicitInterfaceUses(n, argType, paramType);
TypeMismatch.recordImplicitUseOfNativeObject(this.mismatches, n, argType, paramType); recordImplicitUseOfNativeObject(n, argType, paramType);
} }
} }


Expand All @@ -526,9 +528,8 @@ void expectSuperType(NodeTraversal t, Node n, ObjectType superObject,
!(superObject instanceof UnknownType) && !(superObject instanceof UnknownType) &&
!declaredSuper.isEquivalentTo(superObject)) { !declaredSuper.isEquivalentTo(superObject)) {
if (declaredSuper.isEquivalentTo(getNativeType(OBJECT_TYPE))) { if (declaredSuper.isEquivalentTo(getNativeType(OBJECT_TYPE))) {
TypeMismatch.registerMismatch(this.mismatches, this.implicitInterfaceUses, registerMismatch(superObject, declaredSuper, report(
superObject, declaredSuper, t.makeError(n, MISSING_EXTENDS_TAG_WARNING, subObject.toString())));
report(t.makeError(n, MISSING_EXTENDS_TAG_WARNING, subObject.toString())));
} else { } else {
mismatch(n, "mismatch in declaration of superclass type", mismatch(n, "mismatch in declaration of superclass type",
superObject, declaredSuper); superObject, declaredSuper);
Expand All @@ -552,9 +553,8 @@ void expectSuperType(NodeTraversal t, Node n, ObjectType superObject,
*/ */
void expectCanCast(NodeTraversal t, Node n, JSType targetType, JSType sourceType) { void expectCanCast(NodeTraversal t, Node n, JSType targetType, JSType sourceType) {
if (!sourceType.canCastTo(targetType)) { if (!sourceType.canCastTo(targetType)) {
TypeMismatch.registerMismatch( registerMismatch(sourceType, targetType, report(t.makeError(n, INVALID_CAST,
this.mismatches, this.implicitInterfaceUses, sourceType, targetType, sourceType.toString(), targetType.toString())));
report(t.makeError(n, INVALID_CAST, sourceType.toString(), targetType.toString())));
} else if (!sourceType.isSubtypeWithoutStructuralTyping(targetType)){ } else if (!sourceType.isSubtypeWithoutStructuralTyping(targetType)){
recordImplicitInterfaceUses(n, sourceType, targetType); recordImplicitInterfaceUses(n, sourceType, targetType);
} }
Expand Down Expand Up @@ -670,9 +670,7 @@ private void expectInterfaceProperty(NodeTraversal t, Node n,
// Not implemented // Not implemented
String sourceName = n.getSourceFileName(); String sourceName = n.getSourceFileName();
sourceName = nullToEmpty(sourceName); sourceName = nullToEmpty(sourceName);
TypeMismatch.registerMismatch( registerMismatch(
this.mismatches,
this.implicitInterfaceUses,
instance, instance,
implementedInterface, implementedInterface,
report( report(
Expand Down Expand Up @@ -711,8 +709,7 @@ private void expectInterfaceProperty(NodeTraversal t, Node n,
HIDDEN_INTERFACE_PROPERTY_MISMATCH, prop, HIDDEN_INTERFACE_PROPERTY_MISMATCH, prop,
constructor.getTopMostDefiningType(prop).toString(), constructor.getTopMostDefiningType(prop).toString(),
required.toString(), found.toString()); required.toString(), found.toString());
TypeMismatch.registerMismatch( registerMismatch(found, required, err);
this.mismatches, this.implicitInterfaceUses, found, required, err);
report(err); report(err);
} }
} }
Expand Down Expand Up @@ -753,9 +750,7 @@ void expectAbstractMethodsImplemented(Node n, FunctionType ctorType) {
if (abstractMethod == null || abstractMethod.isAbstract()) { if (abstractMethod == null || abstractMethod.isAbstract()) {
String sourceName = n.getSourceFileName(); String sourceName = n.getSourceFileName();
sourceName = nullToEmpty(sourceName); sourceName = nullToEmpty(sourceName);
TypeMismatch.registerMismatch( registerMismatch(
this.mismatches,
this.implicitInterfaceUses,
instance, instance,
superType, superType,
report( report(
Expand Down Expand Up @@ -783,8 +778,7 @@ private void mismatch(Node n, String msg, JSType found, JSType required) {
if (!found.isSubtype(required, this.subtypingMode)) { if (!found.isSubtype(required, this.subtypingMode)) {
JSError err = JSError.make( JSError err = JSError.make(
n, TYPE_MISMATCH_WARNING, formatFoundRequired(msg, found, required)); n, TYPE_MISMATCH_WARNING, formatFoundRequired(msg, found, required));
TypeMismatch.registerMismatch( registerMismatch(found, required, err);
this.mismatches, this.implicitInterfaceUses, found, required, err);
report(err); report(err);
} }
} }
Expand Down Expand Up @@ -814,6 +808,70 @@ private void recordImplicitInterfaceUses(Node src, JSType sourceType, JSType tar
} }
} }


// NOTE(dimvar): declaring this here instead of JSType, because there is another
// method there that behaves differently :-/
private static boolean isInstanceOfObject(JSType t) {
if (t.isObject() && !t.isUnionType()) {
ObjectType proto = t.toObjectType().getImplicitPrototype();
return proto != null && proto.isNativeObjectType();
}
return false;
}

private void recordImplicitUseOfNativeObject(Node src, JSType sourceType, JSType targetType) {
sourceType = sourceType.restrictByNotNullOrUndefined();
targetType = targetType.restrictByNotNullOrUndefined();
if (isInstanceOfObject(sourceType) && !isInstanceOfObject(targetType)) {
// We don't report a type error, but we still need to construct a JSError,
// for people who enable the invalidation diagnostics in DisambiguateProperties.
String msg = "Implicit use of Object type: " + sourceType + " as type: " + targetType;
JSError err = JSError.make(src, TYPE_MISMATCH_WARNING, msg);
mismatches.add(new TypeMismatch(sourceType, targetType, err));
}
}

private void registerMismatch(JSType found, JSType required, JSError error) {
// Don't register a mismatch for differences in null or undefined or if the
// code didn't downcast.
found = found.restrictByNotNullOrUndefined();
required = required.restrictByNotNullOrUndefined();

if (found.isSubtype(required) || required.isSubtype(found)) {
boolean strictMismatch =
!found.isSubtypeWithoutStructuralTyping(required)
&& !required.isSubtypeWithoutStructuralTyping(found);
if (strictMismatch) {
implicitInterfaceUses.add(new TypeMismatch(found, required, error));
}
return;
}

mismatches.add(new TypeMismatch(found, required, error));

if (found.isFunctionType() &&
required.isFunctionType()) {
FunctionType fnTypeA = found.toMaybeFunctionType();
FunctionType fnTypeB = required.toMaybeFunctionType();
Iterator<Node> paramItA = fnTypeA.getParameters().iterator();
Iterator<Node> paramItB = fnTypeB.getParameters().iterator();
while (paramItA.hasNext() && paramItB.hasNext()) {
registerIfMismatch(paramItA.next().getJSType(),
paramItB.next().getJSType(), error);
}

registerIfMismatch(
fnTypeA.getReturnType(), fnTypeB.getReturnType(), error);
}
}

private void registerIfMismatch(
JSType found, JSType required, JSError error) {
if (found != null && required != null &&
!found.isSubtypeWithoutStructuralTyping(required)) {
registerMismatch(found, required, error);
}
}

/** /**
* Formats a found/required error message. * Formats a found/required error message.
*/ */
Expand Down
13 changes: 0 additions & 13 deletions src/com/google/javascript/jscomp/newtypes/FunctionType.java
View file
Expand Up @@ -22,8 +22,6 @@
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Multimap; import com.google.common.collect.Multimap;
import com.google.javascript.rhino.TypeI;
import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
Expand Down Expand Up @@ -769,17 +767,6 @@ public List<String> getTypeParameters() {
return typeParameters; return typeParameters;
} }


List<TypeI> getParameterTypes() {
int howmanyTypes = getMaxArityWithoutRestFormals() + (hasRestFormals() ? 1 : 0);
ArrayList<TypeI> types = new ArrayList<>(howmanyTypes);
types.addAll(this.requiredFormals);
types.addAll(this.optionalFormals);
if (hasRestFormals()) {
types.add(this.restFormals);
}
return types;
}

boolean unifyWithSubtype(FunctionType other, List<String> typeParameters, boolean unifyWithSubtype(FunctionType other, List<String> typeParameters,
Multimap<String, JSType> typeMultimap, SubtypeCache subSuperMap) { Multimap<String, JSType> typeMultimap, SubtypeCache subSuperMap) {
Preconditions.checkState(this.typeParameters.isEmpty(), Preconditions.checkState(this.typeParameters.isEmpty(),
Expand Down
21 changes: 1 addition & 20 deletions src/com/google/javascript/jscomp/newtypes/JSType.java
View file
Expand Up @@ -1853,18 +1853,9 @@ public boolean isUnknownObject() {
return isSingletonObj() && getNominalTypeIfSingletonObj().isBuiltinObject(); return isSingletonObj() && getNominalTypeIfSingletonObj().isBuiltinObject();
} }


@Override
public boolean isLiteralObject() {
return isSingletonObj() && getNominalTypeIfSingletonObj().isLiteralObject();
}

@Override @Override
public boolean isInstanceofObject() { public boolean isInstanceofObject() {
if (isSingletonObj()) { return isSingletonObj() && getNominalTypeIfSingletonObj().isLiteralObject();
NominalType nt = getNominalTypeIfSingletonObj();
return nt.isLiteralObject() || nt.isBuiltinObject();
}
return false;
} }


public boolean mayContainUnknownObject() { public boolean mayContainUnknownObject() {
Expand Down Expand Up @@ -2024,16 +2015,6 @@ public FunctionTypeI getOwnerFunction() {
} }
return null; return null;
} }

@Override
public boolean isSubtypeWithoutStructuralTyping(TypeI other) {
throw new UnsupportedOperationException();
}

@Override
public Iterable<TypeI> getParameterTypes() {
return Preconditions.checkNotNull(getFunType()).getParameterTypes();
}
} }


final class UnionType extends JSType { final class UnionType extends JSType {
Expand Down
2 changes: 0 additions & 2 deletions src/com/google/javascript/rhino/FunctionTypeI.java
View file
Expand Up @@ -86,6 +86,4 @@ public interface FunctionTypeI extends TypeI {
Collection<ObjectTypeI> getAncestorInterfaces(); Collection<ObjectTypeI> getAncestorInterfaces();


ObjectTypeI getPrototypeProperty(); ObjectTypeI getPrototypeProperty();

Iterable<TypeI> getParameterTypes();
} }

0 comments on commit 93c3d90

Please sign in to comment.