Add dot expressions to type checker
Currently the only type that permits field retrieval is `Any`. The resulting type of the retrieval is also `Any`.
To support type checking more complex assignment targets, including subscript expressions and tuple assignment in follow-up CLs, the `visit(AssignmentStatement)` override is factored out into an `assign(Expression lhs, StarlarkType rhsType)` helper.
Added an `assertContainsErrors()` helper to the unit test, to allow checking for more than one error in a single test case code snippet. This gets more use in tests introduced in follow-up CLs.
Work toward #28037.
PiperOrigin-RevId: 847446752
Change-Id: I01f033e14aa2126b3df99311fb5b4c72d5082280
diff --git a/src/main/java/net/starlark/java/syntax/TypeChecker.java b/src/main/java/net/starlark/java/syntax/TypeChecker.java
index cfe1a34..70e288b 100644
--- a/src/main/java/net/starlark/java/syntax/TypeChecker.java
+++ b/src/main/java/net/starlark/java/syntax/TypeChecker.java
@@ -92,15 +92,64 @@
case FLOAT_LITERAL -> {
return Types.FLOAT;
}
+ case DOT -> {
+ // TODO: #27370 - Add support for field retrieval on types besides Any.
+ var dot = (DotExpression) expr;
+ StarlarkType objType = infer(dot.getObject());
+ if (objType.equals(Types.ANY)) {
+ return Types.ANY;
+ } else {
+ errorf(
+ dot.getDotLocation(),
+ "'%s' of type '%s' does not have field '%s'",
+ dot.getObject(),
+ objType,
+ dot.getField().getName());
+ return Types.ANY;
+ }
+ }
default -> {
- // TODO: #28037 - support binaryop, call, cast, comprehension, conditional, dict_expr, dot,
- // index, lambda, list, and unaryop expressions.
+ // TODO: #28037 - support binaryop, call, cast, comprehension, conditional, dict_expr,
+ // index, lambda, list, slice, and unaryop expressions.
throw new UnsupportedOperationException(
String.format("cannot typecheck %s expression", expr.kind()));
}
}
}
+ /**
+ * Recursively typechecks the assignment of type {@code rhsType} to the target expression {@code
+ * lhs}.
+ *
+ * <p>The asymmetry of the parameter types comes from the fact that this helper recursively
+ * decomposes the LHS syntactically, whereas the RHS has already been fully evaluated to a type.
+ * For instance, {@code x, y = (1, 2)} and {@code x, y = my_pair} both trigger the same behavior
+ * in this method. Decomposing the LHS syntactically rather than by type is what allows {@code (x,
+ * y) = [1, 2]} to succeed, even though assignment of a list to a tuple type is illegal (as in
+ * {@code t : Tuple[int, int] = [1, 2]}).
+ */
+ private void assign(Expression lhs, StarlarkType rhsType) {
+ // infer() handles Identifier and DotExpression. The type for evaluating these expressions in a
+ // read context is the same as its type for assignment purposes.
+ StarlarkType lhsType = infer(lhs);
+
+ if (lhs.kind() == Expression.Kind.LIST_EXPR) {
+ // TODO: #28037 - support LHSs containing multiple targets (list expression), field
+ // assignments, and subscript assignments.
+ throw new UnsupportedOperationException(
+ "cannot typecheck assignment statements with multiple targets on the LHS");
+ }
+
+ if (!StarlarkType.assignableFrom(lhsType, rhsType)) {
+ errorf(
+ lhs.getStartLocation(),
+ "cannot assign type '%s' to '%s' of type '%s'",
+ rhsType,
+ lhs,
+ lhsType);
+ }
+ }
+
// Expressions should only be visited via infer(), not the visit() dispatch mechanism.
// Override visit(Identifier) as a poison pill.
@Override
@@ -116,21 +165,13 @@
// TODO: #28037 - support this by validating that `lhs <op> rhs` would type check
throw new UnsupportedOperationException("cannot typecheck augmented assignment statements");
}
- if (!(assignment.getLHS() instanceof Identifier id)) {
- // TODO: #28037 - support LHSs containing multiple targets (list expression), field
- // assignments, and subscript assignments.
- throw new UnsupportedOperationException(
- "cannot typecheck assignment statements with anything besides a single identifier on the"
- + " LHS");
- }
- var lhsType = getType(id);
+
// TODO: #27370 - Do bidirectional inference, passing down information about the expected type
// from the LHS to the infer() call here, e.g. to construct the type of `[1, 2, 3]` as list[int]
// instead of list[object].
var rhsType = infer(assignment.getRHS());
- if (!StarlarkType.assignableFrom(lhsType, rhsType)) {
- errorf(assignment, "cannot assign type '%s' to '%s'", rhsType, lhsType);
- }
+
+ assign(assignment.getLHS(), rhsType);
}
@Override
diff --git a/src/test/java/net/starlark/java/syntax/TypeCheckerTest.java b/src/test/java/net/starlark/java/syntax/TypeCheckerTest.java
index e285285..5ac0a83 100644
--- a/src/test/java/net/starlark/java/syntax/TypeCheckerTest.java
+++ b/src/test/java/net/starlark/java/syntax/TypeCheckerTest.java
@@ -78,6 +78,13 @@
assertContainsError(file.errors(), expectedError);
}
+ /** Asserts that the given file has at least the specified errors. */
+ private static void assertContainsErrors(StarlarkFile file, String... errors) throws Exception {
+ for (var err : errors) {
+ assertContainsError(file.errors(), err);
+ }
+ }
+
@Test
public void assignment_simple() throws Exception {
assertValid(
@@ -87,7 +94,7 @@
""");
assertInvalid(
- ":2:1: cannot assign type 'str' to 'int'",
+ ":2:1: cannot assign type 'str' to 'x' of type 'int'",
"""
x: int
x = "abc"
@@ -97,7 +104,7 @@
@Test
public void canLookupIdentifierTypeOnRhs() throws Exception {
assertInvalid(
- ":3:1: cannot assign type 'bool' to 'int'",
+ ":3:1: cannot assign type 'bool' to 'x' of type 'int'",
"""
x: int
y: bool
@@ -107,21 +114,18 @@
@Test
public void primitiveTypesAreInferredFromLiteralsAndAreIncompatible() throws Exception {
- assertInvalid(
- ":1:1: cannot assign type 'str' to 'bool'",
- """
- x: bool = 'abc'
- """);
- assertInvalid(
- ":1:1: cannot assign type 'int' to 'float'",
- """
- x: float = 123
- """);
- assertInvalid(
- ":1:1: cannot assign type 'float' to 'None'",
- """
- x: None = 1.0
- """);
+ var file =
+ typecheckFilePossiblyFailing(
+ """
+ x: bool = 'abc'
+ y: float = 123
+ z: None = 1.0
+ """);
+ assertContainsErrors(
+ file,
+ ":1:1: cannot assign type 'str' to 'x' of type 'bool'",
+ ":2:1: cannot assign type 'int' to 'y' of type 'float'",
+ ":3:1: cannot assign type 'float' to 'z' of type 'None'");
}
// TODO: #27728 - We should add a test that the types of universals, and in particular the
@@ -173,4 +177,38 @@
""");
// TODO: #28037 - Check break/continue, once we support for and def statements
}
+
+ @Test
+ public void dotExpression() throws Exception {
+ // TODO: #27370 - Make this more interesting when we support struct types.
+ assertValid(
+ """
+ o: Any
+ x: int
+ x = o.f # Any.f ==> Any
+ """);
+
+ assertInvalid(
+ ":2:2: 'x' of type 'int' does not have field 'f'",
+ """
+ x: int
+ x.f
+ """);
+ }
+
+ @Test
+ public void assignment_dot() throws Exception {
+ assertValid(
+ """
+ x: Any
+ x.f = 123
+ """);
+
+ assertInvalid(
+ ":2:2: 'x' of type 'str' does not have field 'f'",
+ """
+ x: str
+ x.f = 123
+ """);
+ }
}
