diff --git a/src/com/google/javascript/jscomp/TypeCheck.java b/src/com/google/javascript/jscomp/TypeCheck.java index c1db771ec4f..e29fe8db3da 100644 --- a/src/com/google/javascript/jscomp/TypeCheck.java +++ b/src/com/google/javascript/jscomp/TypeCheck.java @@ -883,20 +883,12 @@ public void visit(NodeTraversal t, Node n, Node parent) { case FOR: case TEMPLATELIT_SUB: case REST: - case DESTRUCTURING_LHS: - case ARRAY_PATTERN: - case OBJECT_PATTERN: typeable = false; break; case DEFAULT_VALUE: checkCanAssignToWithScope( - t, - n, - n.getFirstChild(), - getJSType(n.getSecondChild()), - /* info= */ null, - "default value has wrong type"); + t, n, n.getFirstChild(), getJSType(n.getSecondChild()), "default value has wrong type"); typeable = false; break; @@ -1033,52 +1025,7 @@ private void visitAssign(NodeTraversal t, Node assign) { Node lvalue = assign.getFirstChild(); Node rvalue = assign.getLastChild(); - JSType rightType = getJSType(rvalue); - 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. - * - *

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. - * - *

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); - + // Check property sets to 'object.property' when 'object' is known. if (lvalue.isGetProp()) { Node object = lvalue.getFirstChild(); JSType objectJsType = getJSType(object); @@ -1090,11 +1037,11 @@ private void checkCanAssignToNameGetpropOrGetelem( if (object.isGetProp()) { JSType jsType = getJSType(object.getFirstChild()); 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); // Prototype assignments are special, because they actually affect @@ -1102,7 +1049,7 @@ private void checkCanAssignToNameGetpropOrGetelem( // during TypedScopeCreator, and we only look for the "dumb" cases here. // object.prototype = ...; if (pname.equals("prototype")) { - validator.expectCanAssignToPrototype(t, objectJsType, nodeToWarn, rightType); + validator.expectCanAssignToPrototype(t, objectJsType, rvalue, getJSType(rvalue)); return; } @@ -1112,8 +1059,7 @@ private void checkCanAssignToNameGetpropOrGetelem( ObjectType objectCastType = ObjectType.cast(objectJsType.restrictByNotNullOrUndefined()); JSType expectedPropertyType = getPropertyTypeIfDeclared(objectCastType, pname); - checkPropertyInheritanceOnGetpropAssign( - t, nodeToWarn, object, pname, info, expectedPropertyType); + checkPropertyInheritanceOnGetpropAssign(t, assign, object, pname, info, expectedPropertyType); // If we successfully found a non-unknown declared type, validate the assignment and don't do // any further checks. @@ -1122,12 +1068,30 @@ private void checkCanAssignToNameGetpropOrGetelem( // assignments to it. if (!propertyIsImplicitCast(objectCastType, pname)) { validator.expectCanAssignToPropertyOf( - t, nodeToWarn, rightType, expectedPropertyType, object, pname); + t, assign, getJSType(rvalue), expectedPropertyType, object, pname); } 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. + * + *

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'. // This can sometimes handle cases when the type of 'object' is not known. @@ -1155,7 +1119,7 @@ private void checkCanAssignToNameGetpropOrGetelem( } } // 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) { @@ -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 * *

    * interface.prototype.property = ...;
    * 
*/ - private void visitInterfacePropertyAssignment(NodeTraversal t, Node object, Node lvalue) { - 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(); + private void visitInterfacePropertyAssignment(NodeTraversal t, Node object, Node rvalue) { JSType rvalueType = getJSType(rvalue); // Only 2 values are allowed for interface methods: @@ -1674,9 +1631,35 @@ private void checkForOfTypes(NodeTraversal t, Node forOf) { // e.g. get "x" given the VAR in "for (var x of arr) {" lhs = lhs.getFirstChild(); } - if (lhs.isDestructuringLhs()) { - // e.g. get `[x, y]` given the VAR in `for (var [x, y] of arr) {` - lhs = lhs.getFirstChild(); + + // Do some additional checks for property accesses in the initializer + // 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( @@ -1684,7 +1667,6 @@ private void checkForOfTypes(NodeTraversal t, Node forOf) { forOf, lhs, actualType, - lhs.getJSDocInfo(), "declared type of for-of loop variable does not match inferred type"); } @@ -1894,27 +1876,27 @@ private void visitVar(NodeTraversal t, Node n) { // on the NAME node. We probably want to wait for the parser // merge to fix this. JSDocInfo varInfo = n.hasOneChild() ? n.getJSDocInfo() : null; - for (Node child : n.children()) { - if (child.isName()) { - Node value = child.getFirstChild(); - - if (value != null) { - JSType valueType = getJSType(value); - JSDocInfo info = child.getJSDocInfo(); - if (info == null) { - info = varInfo; - } + for (Node name : n.children()) { + Node value = name.getFirstChild(); + // A null var would indicate a bug in the scope creation logic. + TypedVar var = t.getTypedScope().getVar(name.getString()); - checkEnumAlias(t, info, valueType, value); - checkCanAssignToWithScope(t, value, child, valueType, info, "initializing variable"); - } - } else { - checkState(child.isDestructuringLhs(), child); - Node name = child.getFirstChild(); - Node value = child.getSecondChild(); + if (value != null) { JSType valueType = getJSType(value); - checkCanAssignToWithScope( - t, child, name, valueType, /* info= */ null, "initializing variable"); + JSType nameType = var.getType(); + 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"); + } } } } @@ -2623,26 +2605,26 @@ private void visitBinaryOperator(Token op, NodeTraversal t, Node n) { } /** - * Checks enum aliases. - * - *

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. + *

Checks enum aliases. * - *

Example: + *

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.

* + *

Example:

*
var myEnum = myOtherEnum;
* - *

Enum aliases are irregular, so we need special code for this :( + *

Enum aliases are irregular, so we need special code for this :(

* - * @param valueType the type of the value used for initialization of the enum - * @param nodeToWarn the node on which to issue warnings on + * @param value the value used for initialization of the enum */ private void checkEnumAlias( - NodeTraversal t, JSDocInfo declInfo, JSType valueType, Node nodeToWarn) { + NodeTraversal t, JSDocInfo declInfo, Node value) { if (declInfo == null || !declInfo.hasEnumParameterType()) { return; } + JSType valueType = getJSType(value); if (!valueType.isEnumType()) { return; } @@ -2650,10 +2632,7 @@ private void checkEnumAlias( EnumType valueEnumType = valueType.toMaybeEnumType(); JSType valueEnumPrimitiveType = valueEnumType.getElementsType().getPrimitiveType(); - validator.expectCanAssignTo( - t, - nodeToWarn, - valueEnumPrimitiveType, + validator.expectCanAssignTo(t, value, valueEnumPrimitiveType, declInfo.getEnumParameterType().evaluate(t.getTypedScope(), typeRegistry), "incompatible enum element types"); } diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java index 99e0fdd1bf9..d625c12cdef 100644 --- a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java @@ -3854,107 +3854,4 @@ public void testDefaultParameterInIifeWithInferredType() { "found : (number|string)", "required: string")); } - - public void testBasicArrayPatternDeclaration() { - testTypes( - lines( - "function f(/** !Iterable */ numbers) {", - " const [/** string */ str] = numbers;", - "}"), - lines( - "initializing variable", // - "found : number", - "required: string")); - } - - public void testNestedDestructuringPatternDeclaration() { - testTypes( - lines( - "function f(/** !Iterable<{x: number}> */ xNumberObjs) {", - " const [{/** string */ x}] = xNumberObjs;", - "}"), - lines( - "initializing variable", // - "found : number", - "required: string")); - } - - public void testBasicArrayPatternAssign() { - testTypes( - lines( - "function f(/** !Iterable */ numbers) {", - " var /** string */ str;", - " [str] = numbers;", - "}"), - lines( - "assignment", // - "found : number", - "required: string")); - } - - public void testNestedDestructuringPatternAssign() { - testTypes( - lines( - "function f(/** !Iterable<{x: number}> */ xNumberObjs) {", - " var /** string */ x;", - " [{x}] = xNumberObjs;", - "}"), - lines( - "assignment", // - "found : number", - "required: string")); - } - - public void testValidArrayPatternInForOfInitializer() { - testTypes( - lines( - "function f(/** !Iterable> */ numberLists) {", - " for (const [/** number */ x, /** number */ y] of numberLists) {}", - "}")); - } - - public void testArrayPatternInForOfInitializerWithTypeMismatch() { - testTypes( - lines( - "function f(/** !Iterable> */ numberLists) {", - " for (const [/** number */ x, /** string */ y] of numberLists) {}", - "}"), - lines( - "declared type of for-of loop variable does not match inferred type", - "found : number", - "required: string")); - } - - public void testCannotAliasEnumThroughDestructuring() { - testTypesWithExterns( - new TestExternsBuilder().addArray().build(), - lines( - "/** @enum {number} */ const THINGS = {THING1: 1, THING2: 2};", - // TODO(lharker): warn for putting @enum here - "/** @enum */ const [OTHERTHINGS] = [THINGS];")); - } - - public void testArrayPatternAssign_badInterfacePropertyCreation() { - testTypes( - "/** @interface */ function Foo() {}; [Foo.prototype.bar] = [];", - "interface members can only be " - + "empty property declarations, empty functions, or goog.abstractMethod"); - } - - public void testArrayPatternAssign_badPropertyAssignment() { - testTypes( - lines( - "/** @param {!Iterable} numbers */", - "function f(numbers) {", - " const /** {a: string} */ obj = {a: 'foo'};", - " [obj.a] = numbers;", - "}"), - lines( - "assignment to property a of obj", // - "found : number", - "required: string")); - } - - // TODO(b/77597706): add tests for default values and computed properties in destructuring, - // and cases where the rvalue is not iterable/an object }