Skip to content

Commit

Permalink
Automated g4 rollback of changelist 207912024.
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

Broke project using pattern (/** @type {?} */) (this.prop) = ...

*** Original change description ***

Start typechecking destructuring patterns in TypeCheck

Right now this recalculates what values are being assigned to a lvalue in a destructuring pattern from the rhs value. We should consider modifying this in the future to avoid repeating work done in TypeInference.

Still doesn't handle computed properties or default values in destructuring patterns.

***

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=207934939
  • Loading branch information
@lauraharker @tjgq
lauraharker authored and tjgq committed Aug 10, 2018
1 parent 48eb424 commit a0e22cc
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 210 deletions.
193 changes: 86 additions & 107 deletions src/com/google/javascript/jscomp/TypeCheck.java
View file
Expand Up @@ -883,20 +883,12 @@ public void visit(NodeTraversal t, Node n, Node parent) {
case FOR: case FOR:
case TEMPLATELIT_SUB: case TEMPLATELIT_SUB:
case REST: case REST:
case DESTRUCTURING_LHS:
case ARRAY_PATTERN:
case OBJECT_PATTERN:
typeable = false; typeable = false;
break; break;


case DEFAULT_VALUE: case DEFAULT_VALUE:
checkCanAssignToWithScope( checkCanAssignToWithScope(
t, t, n, n.getFirstChild(), getJSType(n.getSecondChild()), "default value has wrong type");
n,
n.getFirstChild(),
getJSType(n.getSecondChild()),
/* info= */ null,
"default value has wrong type");
typeable = false; typeable = false;
break; break;


Expand Down Expand Up @@ -1033,52 +1025,7 @@ private void visitAssign(NodeTraversal t, Node assign) {
Node lvalue = assign.getFirstChild(); Node lvalue = assign.getFirstChild();
Node rvalue = assign.getLastChild(); Node rvalue = assign.getLastChild();


JSType rightType = getJSType(rvalue); // Check property sets to 'object.property' when 'object' is known.
checkCanAssignToWithScope(t, assign, lvalue, rightType, info, "assignment");
ensureTyped(assign, rightType);
}

/**
* Checks that we can assign the given right type to the given lvalue or destructuring pattern.
*
* <p>See {@link #checkCanAssignToNameGetpropOrGetelem(NodeTraversal, Node, Node, JSType,
* JSDocInfo, String)} for more details
*
* @param nodeToWarn A node to report type mismatch warnings on
* @param lvalue The lvalue to which we're assigning or a destructuring pattern
* @param rightType The type we're assigning to the lvalue
* @param msg A message to report along with any type mismatch warnings
*/
private void checkCanAssignToWithScope(
NodeTraversal t, Node nodeToWarn, Node lvalue, JSType rightType, JSDocInfo info, String msg) {
if (lvalue.isDestructuringPattern()) {
for (Node child : lvalue.children()) {
DestructuredTarget target = DestructuredTarget.createTarget(typeRegistry, rightType, child);
// TODO(b/77597706): this is not very efficient because it re-infers the types below,
// which we already did once in TypeInference. don't repeat the work.
checkCanAssignToWithScope(
t, nodeToWarn, target.getNode(), target.inferType(), /* info= */ null, msg);
}
} else {
checkCanAssignToNameGetpropOrGetelem(t, nodeToWarn, lvalue, rightType, info, msg);
}
}

/**
* Checks that we can assign the given right type to the given lvalue.
*
* <p>If the lvalue is a qualified name, and has a declared type in the given scope, uses the
* declared type of the qualified name instead of the type on the node.
*
* @param nodeToWarn A node to report type mismatch warnings on
* @param lvalue The lvalue to which we're assigning - a NAME, GETELEM, or GETPROP
* @param rightType The type we're assigning to the lvalue
* @param msg A message to report along with any type mismatch warnings
*/
private void checkCanAssignToNameGetpropOrGetelem(
NodeTraversal t, Node nodeToWarn, Node lvalue, JSType rightType, JSDocInfo info, String msg) {
checkArgument(lvalue.isName() || lvalue.isGetProp() || lvalue.isGetElem(), lvalue);

if (lvalue.isGetProp()) { if (lvalue.isGetProp()) {
Node object = lvalue.getFirstChild(); Node object = lvalue.getFirstChild();
JSType objectJsType = getJSType(object); JSType objectJsType = getJSType(object);
Expand All @@ -1090,19 +1037,19 @@ private void checkCanAssignToNameGetpropOrGetelem(
if (object.isGetProp()) { if (object.isGetProp()) {
JSType jsType = getJSType(object.getFirstChild()); JSType jsType = getJSType(object.getFirstChild());
if (jsType.isInterface() && object.getLastChild().getString().equals("prototype")) { if (jsType.isInterface() && object.getLastChild().getString().equals("prototype")) {
visitInterfacePropertyAssignment(t, object, lvalue); visitInterfacePropertyAssignment(t, object, rvalue);
} }
} }


checkEnumAlias(t, info, rightType, nodeToWarn); checkEnumAlias(t, info, rvalue);
checkPropCreation(t, lvalue); checkPropCreation(t, lvalue);


// Prototype assignments are special, because they actually affect // Prototype assignments are special, because they actually affect
// the definition of a class. These are mostly validated // the definition of a class. These are mostly validated
// during TypedScopeCreator, and we only look for the "dumb" cases here. // during TypedScopeCreator, and we only look for the "dumb" cases here.
// object.prototype = ...; // object.prototype = ...;
if (pname.equals("prototype")) { if (pname.equals("prototype")) {
validator.expectCanAssignToPrototype(t, objectJsType, nodeToWarn, rightType); validator.expectCanAssignToPrototype(t, objectJsType, rvalue, getJSType(rvalue));
return; return;
} }


Expand All @@ -1112,8 +1059,7 @@ private void checkCanAssignToNameGetpropOrGetelem(
ObjectType objectCastType = ObjectType.cast(objectJsType.restrictByNotNullOrUndefined()); ObjectType objectCastType = ObjectType.cast(objectJsType.restrictByNotNullOrUndefined());
JSType expectedPropertyType = getPropertyTypeIfDeclared(objectCastType, pname); JSType expectedPropertyType = getPropertyTypeIfDeclared(objectCastType, pname);


checkPropertyInheritanceOnGetpropAssign( checkPropertyInheritanceOnGetpropAssign(t, assign, object, pname, info, expectedPropertyType);
t, nodeToWarn, object, pname, info, expectedPropertyType);


// If we successfully found a non-unknown declared type, validate the assignment and don't do // If we successfully found a non-unknown declared type, validate the assignment and don't do
// any further checks. // any further checks.
Expand All @@ -1122,12 +1068,30 @@ private void checkCanAssignToNameGetpropOrGetelem(
// assignments to it. // assignments to it.
if (!propertyIsImplicitCast(objectCastType, pname)) { if (!propertyIsImplicitCast(objectCastType, pname)) {
validator.expectCanAssignToPropertyOf( validator.expectCanAssignToPropertyOf(
t, nodeToWarn, rightType, expectedPropertyType, object, pname); t, assign, getJSType(rvalue), expectedPropertyType, object, pname);
} }
return; return;
} }
} }


JSType rightType = getJSType(rvalue);
checkCanAssignToWithScope(t, assign, lvalue, rightType, "assignment");
ensureTyped(assign, rightType);
}

/**
* Checks that we can assign the given right type to the given lvalue.
*
* <p>If the lvalue is a qualified name, and has a declared type in the given scope, uses the
* declared type of the qualified name instead of the type on the node.
*
* @param assign A node to report warnings on for this assignment
* @param lvalue The lvalue to which we're assigning
* @param rightType The type we're assigning to the lvalue
* @param msg A message to report along with any type mismatch warnings
*/
private void checkCanAssignToWithScope(
NodeTraversal t, Node assign, Node lvalue, JSType rightType, String msg) {


// Check qualified name sets to 'object' and 'object.property'. // Check qualified name sets to 'object' and 'object.property'.
// This can sometimes handle cases when the type of 'object' is not known. // This can sometimes handle cases when the type of 'object' is not known.
Expand Down Expand Up @@ -1155,7 +1119,7 @@ private void checkCanAssignToNameGetpropOrGetelem(
} }
} // Fall through case for arbitrary LHS and arbitrary RHS. } // Fall through case for arbitrary LHS and arbitrary RHS.


validator.expectCanAssignTo(t, nodeToWarn, rightType, leftType, msg); validator.expectCanAssignTo(t, assign, rightType, leftType, msg);
} }


private void checkPropCreation(NodeTraversal t, Node lvalue) { private void checkPropCreation(NodeTraversal t, Node lvalue) {
Expand Down Expand Up @@ -1571,20 +1535,13 @@ static JSType getObjectLitKeyTypeFromValueType(Node key, JSType valueType) {
} }


/** /**
* Visits an lvalue node for cases such as * Visits an ASSIGN node for cases such as
* *
* <pre> * <pre>
* interface.prototype.property = ...; * interface.prototype.property = ...;
* </pre> * </pre>
*/ */
private void visitInterfacePropertyAssignment(NodeTraversal t, Node object, Node lvalue) { private void visitInterfacePropertyAssignment(NodeTraversal t, Node object, Node rvalue) {
if (!lvalue.getParent().isAssign()) {
// assignments to interface properties cannot be in destructuring patterns or for-of loops
reportInvalidInterfaceMemberDeclaration(t, object);
return;
}
Node assign = lvalue.getParent();
Node rvalue = assign.getSecondChild();
JSType rvalueType = getJSType(rvalue); JSType rvalueType = getJSType(rvalue);


// Only 2 values are allowed for interface methods: // Only 2 values are allowed for interface methods:
Expand Down Expand Up @@ -1674,17 +1631,42 @@ private void checkForOfTypes(NodeTraversal t, Node forOf) {
// e.g. get "x" given the VAR in "for (var x of arr) {" // e.g. get "x" given the VAR in "for (var x of arr) {"
lhs = lhs.getFirstChild(); lhs = lhs.getFirstChild();
} }
if (lhs.isDestructuringLhs()) {
// e.g. get `[x, y]` given the VAR in `for (var [x, y] of arr) {` // Do some additional checks for property accesses in the initializer
lhs = lhs.getFirstChild(); // e.g. check `obj.foo` in `for (obj.foo of someIterable) {`
if (lhs.isGetProp()) {
Node object = lhs.getFirstChild();
// Don't allow assigning to an interface member
if (object.isGetProp() && object.getLastChild().getString().equals("prototype")) {
JSType jsType = getJSType(object.getFirstChild());
if (jsType.isInterface()) {
reportInvalidInterfaceMemberDeclaration(t, object);
}
}
checkPropCreation(t, lhs);

JSType objectJsType = getJSType(object);
Node property = lhs.getLastChild();
String pname = property.getString();

ObjectType objectCastType = ObjectType.cast(objectJsType.restrictByNotNullOrUndefined());
JSType expectedPropertyType = getPropertyTypeIfDeclared(objectCastType, pname);
if (!expectedPropertyType.isUnknownType()) {
// Note: if the property has @implicitCast at its declaration, we don't check any
// assignments to it.
if (!propertyIsImplicitCast(objectCastType, pname)) {
validator.expectCanAssignToPropertyOf(
t, iterable, actualType, expectedPropertyType, object, pname);
}
return;
}
} }


checkCanAssignToWithScope( checkCanAssignToWithScope(
t, t,
forOf, forOf,
lhs, lhs,
actualType, actualType,
lhs.getJSDocInfo(),
"declared type of for-of loop variable does not match inferred type"); "declared type of for-of loop variable does not match inferred type");
} }


Expand Down Expand Up @@ -1894,27 +1876,27 @@ private void visitVar(NodeTraversal t, Node n) {
// on the NAME node. We probably want to wait for the parser // on the NAME node. We probably want to wait for the parser
// merge to fix this. // merge to fix this.
JSDocInfo varInfo = n.hasOneChild() ? n.getJSDocInfo() : null; JSDocInfo varInfo = n.hasOneChild() ? n.getJSDocInfo() : null;
for (Node child : n.children()) { for (Node name : n.children()) {
if (child.isName()) { Node value = name.getFirstChild();
Node value = child.getFirstChild(); // A null var would indicate a bug in the scope creation logic.

TypedVar var = t.getTypedScope().getVar(name.getString());
if (value != null) {
JSType valueType = getJSType(value);
JSDocInfo info = child.getJSDocInfo();
if (info == null) {
info = varInfo;
}


checkEnumAlias(t, info, valueType, value); if (value != null) {
checkCanAssignToWithScope(t, value, child, valueType, info, "initializing variable");
}
} else {
checkState(child.isDestructuringLhs(), child);
Node name = child.getFirstChild();
Node value = child.getSecondChild();
JSType valueType = getJSType(value); JSType valueType = getJSType(value);
checkCanAssignToWithScope( JSType nameType = var.getType();
t, child, name, valueType, /* info= */ null, "initializing variable"); nameType = (nameType == null) ? getNativeType(UNKNOWN_TYPE) : nameType;
JSDocInfo info = name.getJSDocInfo();
if (info == null) {
info = varInfo;
}

checkEnumAlias(t, info, value);
if (var.isTypeInferred()) {
ensureTyped(name, valueType);
} else {
validator.expectCanAssignTo(
t, value, valueType, nameType, "initializing variable");
}
} }
} }
} }
Expand Down Expand Up @@ -2623,37 +2605,34 @@ private void visitBinaryOperator(Token op, NodeTraversal t, Node n) {
} }


/** /**
* Checks enum aliases. * <p>Checks enum aliases.
*
* <p>We verify that the enum element type of the enum used for initialization is a subtype of the
* enum element type of the enum the value is being copied in.
* *
* <p>Example: * <p>We verify that the enum element type of the enum used
* for initialization is a subtype of the enum element type of
* the enum the value is being copied in.</p>
* *
* <p>Example:</p>
* <pre>var myEnum = myOtherEnum;</pre> * <pre>var myEnum = myOtherEnum;</pre>
* *
* <p>Enum aliases are irregular, so we need special code for this :( * <p>Enum aliases are irregular, so we need special code for this :(</p>
* *
* @param valueType the type of the value used for initialization of the enum * @param value the value used for initialization of the enum
* @param nodeToWarn the node on which to issue warnings on
*/ */
private void checkEnumAlias( private void checkEnumAlias(
NodeTraversal t, JSDocInfo declInfo, JSType valueType, Node nodeToWarn) { NodeTraversal t, JSDocInfo declInfo, Node value) {
if (declInfo == null || !declInfo.hasEnumParameterType()) { if (declInfo == null || !declInfo.hasEnumParameterType()) {
return; return;
} }


JSType valueType = getJSType(value);
if (!valueType.isEnumType()) { if (!valueType.isEnumType()) {
return; return;
} }


EnumType valueEnumType = valueType.toMaybeEnumType(); EnumType valueEnumType = valueType.toMaybeEnumType();
JSType valueEnumPrimitiveType = JSType valueEnumPrimitiveType =
valueEnumType.getElementsType().getPrimitiveType(); valueEnumType.getElementsType().getPrimitiveType();
validator.expectCanAssignTo( validator.expectCanAssignTo(t, value, valueEnumPrimitiveType,
t,
nodeToWarn,
valueEnumPrimitiveType,
declInfo.getEnumParameterType().evaluate(t.getTypedScope(), typeRegistry), declInfo.getEnumParameterType().evaluate(t.getTypedScope(), typeRegistry),
"incompatible enum element types"); "incompatible enum element types");
} }
Expand Down

0 comments on commit a0e22cc

Please sign in to comment.