From a79bc5cf2c4a74b35095377d820744f3034f0184 Mon Sep 17 00:00:00 2001 From: Tsuf Cohen <39455181+tsuf239@users.noreply.github.com> Date: Wed, 29 May 2024 15:15:58 +0300 Subject: [PATCH] fix(compiler): expected type to be "Array", but got "Array" instead (#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)*. --- examples/tests/invalid/equality.test.w | 79 ++++++++++++++ libs/wingc/src/type_check.rs | 104 ++++++++++++++++--- tools/hangar/__snapshots__/invalid.ts.snap | 115 +++++++++++++++++++++ 3 files changed, 282 insertions(+), 16 deletions(-) create mode 100644 examples/tests/invalid/equality.test.w diff --git a/examples/tests/invalid/equality.test.w b/examples/tests/invalid/equality.test.w new file mode 100644 index 00000000000..97b04961144 --- /dev/null +++ b/examples/tests/invalid/equality.test.w @@ -0,0 +1,79 @@ +// arrays + +assert([{a: 1}] == [{a: 1}]); + +assert(MutArray[{a: 1}] == Array[MutJson{a: 1}]); +// this is fine- we ignore mutable + +assert([1,2] == ["a", "b"]); +// ^^^^^^^^^^^^^^^^^^^ Expected type to be "Array", but got "Array" instead + +assert(["a", "b"] == [1,2]); +// ^^^^^^^^^^^^^^^^^^^ Expected type to be "Array", but got "Array" 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", but got "Array" instead + +assert([["1", "2"]] == [[{a: 1}]]); +// ^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected type to be "Array>", but got "Array>" instead + +assert({"a" => ["a"]} == {"a" => [1]}); +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected type to be "Map>", but got "Map>" instead + +assert([{"a" => [1]}] == [{"a" => ["a"]}]); +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected type to be "Array>>", but got "Array>>" instead + +let a1: Array? = nil; +let a2: Array? = nil; + +assert(a1 == a2); +// ^^^^^^^^ Expected type to be "Array?", but got "Array?" instead + +let b1: Array = []; +let b2: Array? = []; + +assert(b1 == b2); +// ^^^^^^^^ Expected type to be "Array", but got "Array?" instead + +assert([nil] == ["a"]); +// ^^^^^^^^^^^^^^ Expected type to be "Array", but got "Array" instead + +assert(Array[nil] == ["a"]); +// ^^^ Expected type to be "Json", but got "nil" instead. hint: to allow "nil" assignment use optional type: "Json?" + +assert(Array[nil] == ["a"]); +// this is ok + + +let c1: num? = nil; +let c2: str? = nil; +assert([c1] == [c2]); +// ^^^^^^^^^^^^ Expected type to be "Array", but got "Array" instead + + +assert([[1]] == [["1"]]); +// ^^^^^^^^^^^^^^^^ Expected type to be "Array>", but got "Array>" instead + +assert([["1"]] == [[1]]); +// ^^^^^^^^^^^^^^^^ Expected type to be "Array>", but got "Array>" instead + +let d1 = {"a" => 1}; +let d2 = {"b" => "b"}; +assert(d1 == d2); +// ^^^^^^^^ Expected type to be "Map", but got "Map" instead + +let e1 = MutSet[true, true, true]; +let e2 = Set[1,2,3,3,3,2]; + +assert(e1 == e2); +// ^^^^^^^^ Expected type to be "MutSet", but got "Set" instead + +let f1 = Set[1,2,3,3,3,2]; +let f2 = MutSet[1,2,3,3,3,2]; + +assert(f1 == f2); +// this is ok \ No newline at end of file diff --git a/libs/wingc/src/type_check.rs b/libs/wingc/src/type_check.rs index 358af534d53..1df2e4fc6d7 100644 --- a/libs/wingc/src/type_check.rs +++ b/libs/wingc/src/type_check.rs @@ -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) @@ -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 => { @@ -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, + expected_original_type: Option, ) -> 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, + ) } } @@ -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, + expected_original_type: Option, + ) -> 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, + expected_original_types: Option<&[TypeRef]>, + ) -> TypeRef { assert!(expected_types.len() > 0); let first_expected_type = expected_types[0]; let mut return_type = actual_type; @@ -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::>() .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 = vec![]; if return_type.is_nil() && expected_types.len() == 1 { hints.push(format!( @@ -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); @@ -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) => { diff --git a/tools/hangar/__snapshots__/invalid.ts.snap b/tools/hangar/__snapshots__/invalid.ts.snap index 8a87b3637c7..5707b478135 100644 --- a/tools/hangar/__snapshots__/invalid.ts.snap +++ b/tools/hangar/__snapshots__/invalid.ts.snap @@ -1479,6 +1479,121 @@ error: Property not found +Tests 1 failed (1) +Snapshots 1 skipped +Test Files 1 failed (1) +Duration " +`; + +exports[`equality.test.w 1`] = ` +"error: Expected type to be \\"Array\\", but got \\"Array\\" instead + --> ../../../examples/tests/invalid/equality.test.w:8:8 + | +8 | assert([1,2] == [\\"a\\", \\"b\\"]); + | ^^^^^^^^^^^^^^^^^^^ + + +error: Expected type to be \\"Array\\", but got \\"Array\\" instead + --> ../../../examples/tests/invalid/equality.test.w:11:8 + | +11 | assert([\\"a\\", \\"b\\"] == [1,2]); + | ^^^^^^^^^^^^^^^^^^^ + + +error: Expected type to be \\"Array\\", but got \\"Array\\" instead + --> ../../../examples/tests/invalid/equality.test.w:18:8 + | +18 | assert([\\"1\\", \\"2\\"] == [{a: 1}]); + | ^^^^^^^^^^^^^^^^^^^^^^ + + +error: Expected type to be \\"Array>\\", but got \\"Array>\\" instead + --> ../../../examples/tests/invalid/equality.test.w:21:8 + | +21 | assert([[\\"1\\", \\"2\\"]] == [[{a: 1}]]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + + +error: Expected type to be \\"Map>\\", but got \\"Map>\\" instead + --> ../../../examples/tests/invalid/equality.test.w:24:8 + | +24 | assert({\\"a\\" => [\\"a\\"]} == {\\"a\\" => [1]}); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + +error: Expected type to be \\"Array>>\\", but got \\"Array>>\\" instead + --> ../../../examples/tests/invalid/equality.test.w:27:8 + | +27 | assert([{\\"a\\" => [1]}] == [{\\"a\\" => [\\"a\\"]}]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + +error: Expected type to be \\"Array?\\", but got \\"Array?\\" instead + --> ../../../examples/tests/invalid/equality.test.w:33:8 + | +33 | assert(a1 == a2); + | ^^^^^^^^ + + +error: Expected type to be \\"Array\\", but got \\"Array?\\" instead + --> ../../../examples/tests/invalid/equality.test.w:39:8 + | +39 | assert(b1 == b2); + | ^^^^^^^^ + + +error: Expected type to be \\"Array\\", but got \\"Array\\" 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[nil] == [\\"a\\"]); + | ^^^ + | + = hint: to allow \\"nil\\" assignment use optional type: \\"Json?\\" + + +error: Expected type to be \\"Array\\", but got \\"Array\\" instead + --> ../../../examples/tests/invalid/equality.test.w:54:8 + | +54 | assert([c1] == [c2]); + | ^^^^^^^^^^^^ + + +error: Expected type to be \\"Array>\\", but got \\"Array>\\" instead + --> ../../../examples/tests/invalid/equality.test.w:58:8 + | +58 | assert([[1]] == [[\\"1\\"]]); + | ^^^^^^^^^^^^^^^^ + + +error: Expected type to be \\"Array>\\", but got \\"Array>\\" instead + --> ../../../examples/tests/invalid/equality.test.w:61:8 + | +61 | assert([[\\"1\\"]] == [[1]]); + | ^^^^^^^^^^^^^^^^ + + +error: Expected type to be \\"Map\\", but got \\"Map\\" instead + --> ../../../examples/tests/invalid/equality.test.w:66:8 + | +66 | assert(d1 == d2); + | ^^^^^^^^ + + +error: Expected type to be \\"MutSet\\", but got \\"Set\\" 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)