Skip to content

Commit

Permalink
fix(compiler): expected type to be "Array", but got "Array" instead (#…
Browse files Browse the repository at this point in the history
…6571)

Fixes #3822 
I considered a few approaches and concluded that the lightest one would be passing the parent type into the validation function, and then using it (if not None) in the diagnostic message.
Updated the validation function calls accordingly.
Currently supported only for the `Equal` and `NotEqual` binary operators

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
tsuf239 authored May 29, 2024
1 parent 79b07d5 commit a79bc5c
Show file tree
Hide file tree
Showing 3 changed files with 282 additions and 16 deletions.
79 changes: 79 additions & 0 deletions examples/tests/invalid/equality.test.w
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// arrays

assert([{a: 1}] == [{a: 1}]);

assert(MutArray<Json>[{a: 1}] == Array<Json>[MutJson{a: 1}]);
// this is fine- we ignore mutable

assert([1,2] == ["a", "b"]);
// ^^^^^^^^^^^^^^^^^^^ Expected type to be "Array<num>", but got "Array<str>" instead

assert(["a", "b"] == [1,2]);
// ^^^^^^^^^^^^^^^^^^^ Expected type to be "Array<str>", but got "Array<num>" instead

assert([{a: 1}] == ["1", "2"]);
//this doesn't throw an error since string is a subtype of json


assert(["1", "2"] == [{a: 1}]);
// ^^^^^^^^^^^^^^^^^^^^^^ Expected type to be "Array<str>", but got "Array<Json>" instead

assert([["1", "2"]] == [[{a: 1}]]);
// ^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected type to be "Array<Array<str>>", but got "Array<Array<Json>>" instead

assert({"a" => ["a"]} == {"a" => [1]});
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected type to be "Map<Array<str>>", but got "Map<Array<num>>" instead

assert([{"a" => [1]}] == [{"a" => ["a"]}]);
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected type to be "Array<Map<Array<num>>>", but got "Array<Map<Array<str>>>" instead

let a1: Array<bool>? = nil;
let a2: Array<str>? = nil;

assert(a1 == a2);
// ^^^^^^^^ Expected type to be "Array<bool>?", but got "Array<str>?" instead

let b1: Array<str> = [];
let b2: Array<str>? = [];

assert(b1 == b2);
// ^^^^^^^^ Expected type to be "Array<str>", but got "Array<str>?" instead

assert([nil] == ["a"]);
// ^^^^^^^^^^^^^^ Expected type to be "Array<nil>", but got "Array<str>" instead

assert(Array<Json>[nil] == ["a"]);
// ^^^ Expected type to be "Json", but got "nil" instead. hint: to allow "nil" assignment use optional type: "Json?"

assert(Array<Json?>[nil] == ["a"]);
// this is ok


let c1: num? = nil;
let c2: str? = nil;
assert([c1] == [c2]);
// ^^^^^^^^^^^^ Expected type to be "Array<num?>", but got "Array<str?>" instead


assert([[1]] == [["1"]]);
// ^^^^^^^^^^^^^^^^ Expected type to be "Array<Array<num>>", but got "Array<Array<str>>" instead

assert([["1"]] == [[1]]);
// ^^^^^^^^^^^^^^^^ Expected type to be "Array<Array<str>>", but got "Array<Array<num>>" instead

let d1 = {"a" => 1};
let d2 = {"b" => "b"};
assert(d1 == d2);
// ^^^^^^^^ Expected type to be "Map<num>", but got "Map<str>" instead

let e1 = MutSet<bool>[true, true, true];
let e2 = Set<num>[1,2,3,3,3,2];

assert(e1 == e2);
// ^^^^^^^^ Expected type to be "MutSet<bool>", but got "Set<num>" instead

let f1 = Set<num>[1,2,3,3,3,2];
let f2 = MutSet<num>[1,2,3,3,3,2];

assert(f1 == f2);
// this is ok
104 changes: 88 additions & 16 deletions libs/wingc/src/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2221,7 +2221,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] });
if let InterpolatedStringPart::Expr(interpolated_expr) = part {
let (exp_type, p) = self.type_check_exp(interpolated_expr, env);
phase = combine_phases(phase, p);
self.validate_type_in(exp_type, &[self.types.stringable()], interpolated_expr);
self.validate_type_in(exp_type, &[self.types.stringable()], interpolated_expr, None, None);
}
});
(self.types.string(), phase)
Expand Down Expand Up @@ -2286,7 +2286,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] });
(self.types.number(), phase)
}
BinaryOperator::Equal | BinaryOperator::NotEqual => {
self.validate_type_binary_equality(rtype, ltype, exp);
self.validate_type_binary_equality(rtype, ltype, exp, None, None);
(self.types.bool(), phase)
}
BinaryOperator::Less | BinaryOperator::LessOrEqual | BinaryOperator::Greater | BinaryOperator::GreaterOrEqual => {
Expand Down Expand Up @@ -3248,27 +3248,53 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] });
actual_type: TypeRef,
expected_type: TypeRef,
span: &impl Spanned,
actual_original_type: Option<TypeRef>,
expected_original_type: Option<TypeRef>,
) -> TypeRef {
if let (
Type::Array(inner_actual) | Type::MutArray(inner_actual),
Type::Array(inner_expected) | Type::MutArray(inner_expected),
) = (&*actual_type, &*expected_type)
{
self.validate_type_binary_equality(*inner_actual, *inner_expected, span)
self.validate_type_binary_equality(
*inner_actual,
*inner_expected,
span,
Some(actual_original_type.unwrap_or(actual_type)),
Some(expected_original_type.unwrap_or(expected_type)),
)
} else if let (
Type::Map(inner_actual) | Type::MutMap(inner_actual),
Type::Map(inner_expected) | Type::MutMap(inner_expected),
) = (&*actual_type, &*expected_type)
{
self.validate_type_binary_equality(*inner_actual, *inner_expected, span)
self.validate_type_binary_equality(
*inner_actual,
*inner_expected,
span,
Some(actual_original_type.unwrap_or(actual_type)),
Some(expected_original_type.unwrap_or(expected_type)),
)
} else if let (
Type::Set(inner_actual) | Type::MutSet(inner_actual),
Type::Set(inner_expected) | Type::MutSet(inner_expected),
) = (&*actual_type, &*expected_type)
{
self.validate_type_binary_equality(*inner_actual, *inner_expected, span)
self.validate_type_binary_equality(
*inner_actual,
*inner_expected,
span,
Some(actual_original_type.unwrap_or(actual_type)),
Some(expected_original_type.unwrap_or(expected_type)),
)
} else {
self.validate_type(actual_type, expected_type, span)
self.validate_nested_type(
actual_type,
expected_type,
span,
actual_original_type,
expected_original_type,
)
}
}

Expand Down Expand Up @@ -3355,13 +3381,45 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] });
///
/// Returns the given type on success, otherwise returns the expected type.
fn validate_type(&mut self, actual_type: TypeRef, expected_type: TypeRef, span: &impl Spanned) -> TypeRef {
self.validate_type_in(actual_type, &[expected_type], span)
self.validate_type_in(actual_type, &[expected_type], span, None, None)
}

/// Validate that the given type is a subtype (or same) as the expected type. If not, add an error
/// to the diagnostics- based on the parent type.
///
/// Returns the given type on success, otherwise returns the expected type.
fn validate_nested_type(
&mut self,
actual_type: TypeRef,
expected_type: TypeRef,
span: &impl Spanned,
actual_original_type: Option<TypeRef>,
expected_original_type: Option<TypeRef>,
) -> TypeRef {
if let Some(expected_original_t) = expected_original_type {
self.validate_type_in(
actual_type,
&[expected_type],
span,
actual_original_type,
Some(&[expected_original_t]),
)
} else {
self.validate_type_in(actual_type, &[expected_type], span, actual_original_type, None)
}
}

/// Validate that the given type is a subtype (or same) as the one of the expected types. If not, add
/// an error to the diagnostics.
/// Returns the given type on success, otherwise returns one of the expected types.
fn validate_type_in(&mut self, actual_type: TypeRef, expected_types: &[TypeRef], span: &impl Spanned) -> TypeRef {
fn validate_type_in(
&mut self,
actual_type: TypeRef,
expected_types: &[TypeRef],
span: &impl Spanned,
actual_original_type: Option<TypeRef>,
expected_original_types: Option<&[TypeRef]>,
) -> TypeRef {
assert!(expected_types.len() > 0);
let first_expected_type = expected_types[0];
let mut return_type = actual_type;
Expand Down Expand Up @@ -3410,18 +3468,20 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] });
}
}

let expected_type_str = if expected_types.len() > 1 {
let expected_types_list = expected_types
let expected_type_origin = expected_original_types.unwrap_or(expected_types);
let expected_type_str = if expected_type_origin.len() > 1 {
let expected_types_list = expected_type_origin
.iter()
.map(|t| format!("{}", t))
.collect::<Vec<String>>()
.join(",");
format!("one of \"{}\"", expected_types_list)
} else {
format!("\"{}\"", first_expected_type)
format!("\"{}\"", expected_type_origin[0])
};

let message = format!("Expected type to be {expected_type_str}, but got \"{return_type}\" instead");
let return_type_str = actual_original_type.unwrap_or(return_type);
let message = format!("Expected type to be {expected_type_str}, but got \"{return_type_str}\" instead");
let mut hints: Vec<String> = vec![];
if return_type.is_nil() && expected_types.len() == 1 {
hints.push(format!(
Expand Down Expand Up @@ -4859,8 +4919,8 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] });
// ```
match kind {
AssignmentKind::AssignIncr => {
self.validate_type_in(exp_type, &[self.types.number(), self.types.string()], value);
self.validate_type_in(var_type, &[self.types.number(), self.types.string()], value);
self.validate_type_in(exp_type, &[self.types.number(), self.types.string()], value, None, None);
self.validate_type_in(var_type, &[self.types.number(), self.types.string()], value, None, None);
}
AssignmentKind::AssignDecr => {
self.validate_type(exp_type, self.types.number(), value);
Expand Down Expand Up @@ -6204,11 +6264,23 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] });
let res = match *instance_type {
// TODO: it might be possible to look at Type::Json's inner data to give a more specific type
Type::Json(_) => {
self.validate_type_in(index_type, &[self.types.number(), self.types.string()], index);
self.validate_type_in(
index_type,
&[self.types.number(), self.types.string()],
index,
None,
None,
);
ResolveReferenceResult::Location(instance_type, self.types.json()) // indexing into a Json object returns a Json object
}
Type::MutJson => {
self.validate_type_in(index_type, &[self.types.number(), self.types.string()], index);
self.validate_type_in(
index_type,
&[self.types.number(), self.types.string()],
index,
None,
None,
);
ResolveReferenceResult::Location(instance_type, self.types.mut_json()) // indexing into a MutJson object returns a MutJson object
}
Type::Array(inner_type) | Type::MutArray(inner_type) => {
Expand Down
115 changes: 115 additions & 0 deletions tools/hangar/__snapshots__/invalid.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,121 @@ error: Property not found



Tests 1 failed (1)
Snapshots 1 skipped
Test Files 1 failed (1)
Duration <DURATION>"
`;

exports[`equality.test.w 1`] = `
"error: Expected type to be \\"Array<num>\\", but got \\"Array<str>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:8:8
|
8 | assert([1,2] == [\\"a\\", \\"b\\"]);
| ^^^^^^^^^^^^^^^^^^^


error: Expected type to be \\"Array<str>\\", but got \\"Array<num>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:11:8
|
11 | assert([\\"a\\", \\"b\\"] == [1,2]);
| ^^^^^^^^^^^^^^^^^^^


error: Expected type to be \\"Array<str>\\", but got \\"Array<Json>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:18:8
|
18 | assert([\\"1\\", \\"2\\"] == [{a: 1}]);
| ^^^^^^^^^^^^^^^^^^^^^^


error: Expected type to be \\"Array<Array<str>>\\", but got \\"Array<Array<Json>>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:21:8
|
21 | assert([[\\"1\\", \\"2\\"]] == [[{a: 1}]]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^


error: Expected type to be \\"Map<Array<str>>\\", but got \\"Map<Array<num>>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:24:8
|
24 | assert({\\"a\\" => [\\"a\\"]} == {\\"a\\" => [1]});
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


error: Expected type to be \\"Array<Map<Array<num>>>\\", but got \\"Array<Map<Array<str>>>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:27:8
|
27 | assert([{\\"a\\" => [1]}] == [{\\"a\\" => [\\"a\\"]}]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


error: Expected type to be \\"Array<bool>?\\", but got \\"Array<str>?\\" instead
--> ../../../examples/tests/invalid/equality.test.w:33:8
|
33 | assert(a1 == a2);
| ^^^^^^^^


error: Expected type to be \\"Array<str>\\", but got \\"Array<str>?\\" instead
--> ../../../examples/tests/invalid/equality.test.w:39:8
|
39 | assert(b1 == b2);
| ^^^^^^^^


error: Expected type to be \\"Array<nil>\\", but got \\"Array<str>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:42:8
|
42 | assert([nil] == [\\"a\\"]);
| ^^^^^^^^^^^^^^


error: Expected type to be \\"Json\\", but got \\"nil\\" instead
--> ../../../examples/tests/invalid/equality.test.w:45:20
|
45 | assert(Array<Json>[nil] == [\\"a\\"]);
| ^^^
|
= hint: to allow \\"nil\\" assignment use optional type: \\"Json?\\"


error: Expected type to be \\"Array<num?>\\", but got \\"Array<str?>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:54:8
|
54 | assert([c1] == [c2]);
| ^^^^^^^^^^^^


error: Expected type to be \\"Array<Array<num>>\\", but got \\"Array<Array<str>>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:58:8
|
58 | assert([[1]] == [[\\"1\\"]]);
| ^^^^^^^^^^^^^^^^


error: Expected type to be \\"Array<Array<str>>\\", but got \\"Array<Array<num>>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:61:8
|
61 | assert([[\\"1\\"]] == [[1]]);
| ^^^^^^^^^^^^^^^^


error: Expected type to be \\"Map<num>\\", but got \\"Map<str>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:66:8
|
66 | assert(d1 == d2);
| ^^^^^^^^


error: Expected type to be \\"MutSet<bool>\\", but got \\"Set<num>\\" instead
--> ../../../examples/tests/invalid/equality.test.w:72:8
|
72 | assert(e1 == e2);
| ^^^^^^^^



Tests 1 failed (1)
Snapshots 1 skipped
Test Files 1 failed (1)
Expand Down

0 comments on commit a79bc5c

Please sign in to comment.