From fb3f02407c1ea4a5796db5824a4197cfc0a4063f Mon Sep 17 00:00:00 2001 From: Marcio Cruz de Almeida <67694075+marciocadev@users.noreply.github.com> Date: Wed, 23 Aug 2023 16:39:01 -0300 Subject: [PATCH] fix(compiler): comparison between mutable and immutable objects (#3870) Closes #2867 ## Checklist - [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted) - [ ] 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/valid/deep_equality.w | 15 +++--- libs/wingc/src/type_check.rs | 35 +++++++++++++- .../valid/deep_equality.w_compile_tf-aws.md | 48 +++++++++---------- 3 files changed, 65 insertions(+), 33 deletions(-) diff --git a/examples/tests/valid/deep_equality.w b/examples/tests/valid/deep_equality.w index 5f63bd6b0cf..36ad386055a 100644 --- a/examples/tests/valid/deep_equality.w +++ b/examples/tests/valid/deep_equality.w @@ -48,13 +48,12 @@ let setC = { 4,5,6 }; test "Set types with the same value" { assert(setA == setA); - // TODO https://github.com/winglang/wing/issues/2867 - assert(setA == setB.copy()); + assert(setA == setB); } test "Set types with different values" { assert(setA != setC); - assert(!(setA != setB.copy())); + assert(!(setA != setB)); } //----------------------------------------------------------------------------- @@ -66,12 +65,12 @@ let mapC = { "c" => 10, "b" => 2 }; test "Map with the same value" { assert(mapA == mapA); - assert(mapA == mapB.copy()); + assert(mapA == mapB); } test "Map with different values" { assert(mapA != mapC); - assert(!(mapA != mapB.copy())); + assert(!(mapA != mapB)); } //----------------------------------------------------------------------------- @@ -83,12 +82,12 @@ let arrayC: Array = [4,5,6]; test "Array with the same value" { assert(arrayA == arrayA); - assert(arrayA == arrayB.copy()); + assert(arrayA == arrayB); } test "Array with different values" { assert(arrayA != arrayC); - assert(!(arrayA != arrayB.copy())); + assert(!(arrayA != arrayB)); } //----------------------------------------------------------------------------- @@ -111,4 +110,4 @@ test "Struct with the same value" { test "Struct with different values" { assert(cat1 != cat3); assert(!(cat1 != cat2)); -} \ No newline at end of file +} diff --git a/libs/wingc/src/type_check.rs b/libs/wingc/src/type_check.rs index 10fcd015c75..bff3b08d9f9 100644 --- a/libs/wingc/src/type_check.rs +++ b/libs/wingc/src/type_check.rs @@ -1793,7 +1793,7 @@ impl<'a> TypeChecker<'a> { (self.types.number(), Phase::Independent) } BinaryOperator::Equal | BinaryOperator::NotEqual => { - self.validate_type(rtype, ltype, exp); + self.validate_type_binary_equality(rtype, ltype, exp); (self.types.bool(), Phase::Independent) } BinaryOperator::Less @@ -2584,6 +2584,39 @@ impl<'a> TypeChecker<'a> { } } + /// Validate that the given type is a subtype (or same) as the expected type while allowing + /// collection types to have different mutability (e.g. Array and MutArray). + /// + /// Returns the given type on success, otherwise returns the expected type. + fn validate_type_binary_equality( + &mut self, + actual_type: TypeRef, + expected_type: TypeRef, + span: &impl Spanned, + ) -> 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) + } 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) + } 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) + } else { + self.validate_type_in(actual_type, &[expected_type], span) + } + } + /// Validate that the given type is a subtype (or same) as the expected type. If not, add an error /// to the diagnostics. /// diff --git a/tools/hangar/__snapshots__/test_corpus/valid/deep_equality.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/deep_equality.w_compile_tf-aws.md index 1857c237048..2de13cd6cde 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/deep_equality.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/deep_equality.w_compile_tf-aws.md @@ -55,7 +55,7 @@ module.exports = function({ $numA, $numB, $strA, $strB }) { ## inflight.$Closure10-1.js ```js -module.exports = function({ $_____arrayB__, $arrayA, $arrayC }) { +module.exports = function({ $arrayA, $arrayB, $arrayC }) { class $Closure10 { constructor({ }) { const $obj = (...args) => this.handle(...args); @@ -64,7 +64,7 @@ module.exports = function({ $_____arrayB__, $arrayA, $arrayC }) { } async handle() { {((cond) => {if (!cond) throw new Error("assertion failed: arrayA != arrayC")})((((a,b) => { try { return require('assert').notDeepStrictEqual(a,b) === undefined; } catch { return false; } })($arrayA,$arrayC)))}; - {((cond) => {if (!cond) throw new Error("assertion failed: !(arrayA != arrayB.copy())")})((!(((a,b) => { try { return require('assert').notDeepStrictEqual(a,b) === undefined; } catch { return false; } })($arrayA,$_____arrayB__))))}; + {((cond) => {if (!cond) throw new Error("assertion failed: !(arrayA != arrayB)")})((!(((a,b) => { try { return require('assert').notDeepStrictEqual(a,b) === undefined; } catch { return false; } })($arrayA,$arrayB))))}; } } return $Closure10; @@ -169,7 +169,7 @@ module.exports = function({ $jsonA, $jsonB, $jsonC }) { ## inflight.$Closure5-1.js ```js -module.exports = function({ $new_Set_setB_, $setA }) { +module.exports = function({ $setA, $setB }) { class $Closure5 { constructor({ }) { const $obj = (...args) => this.handle(...args); @@ -178,7 +178,7 @@ module.exports = function({ $new_Set_setB_, $setA }) { } async handle() { {((cond) => {if (!cond) throw new Error("assertion failed: setA == setA")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })($setA,$setA)))}; - {((cond) => {if (!cond) throw new Error("assertion failed: setA == setB.copy()")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })($setA,$new_Set_setB_)))}; + {((cond) => {if (!cond) throw new Error("assertion failed: setA == setB")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })($setA,$setB)))}; } } return $Closure5; @@ -188,7 +188,7 @@ module.exports = function({ $new_Set_setB_, $setA }) { ## inflight.$Closure6-1.js ```js -module.exports = function({ $new_Set_setB_, $setA, $setC }) { +module.exports = function({ $setA, $setB, $setC }) { class $Closure6 { constructor({ }) { const $obj = (...args) => this.handle(...args); @@ -197,7 +197,7 @@ module.exports = function({ $new_Set_setB_, $setA, $setC }) { } async handle() { {((cond) => {if (!cond) throw new Error("assertion failed: setA != setC")})((((a,b) => { try { return require('assert').notDeepStrictEqual(a,b) === undefined; } catch { return false; } })($setA,$setC)))}; - {((cond) => {if (!cond) throw new Error("assertion failed: !(setA != setB.copy())")})((!(((a,b) => { try { return require('assert').notDeepStrictEqual(a,b) === undefined; } catch { return false; } })($setA,$new_Set_setB_))))}; + {((cond) => {if (!cond) throw new Error("assertion failed: !(setA != setB)")})((!(((a,b) => { try { return require('assert').notDeepStrictEqual(a,b) === undefined; } catch { return false; } })($setA,$setB))))}; } } return $Closure6; @@ -207,7 +207,7 @@ module.exports = function({ $new_Set_setB_, $setA, $setC }) { ## inflight.$Closure7-1.js ```js -module.exports = function({ $______mapB___, $mapA }) { +module.exports = function({ $mapA, $mapB }) { class $Closure7 { constructor({ }) { const $obj = (...args) => this.handle(...args); @@ -216,7 +216,7 @@ module.exports = function({ $______mapB___, $mapA }) { } async handle() { {((cond) => {if (!cond) throw new Error("assertion failed: mapA == mapA")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })($mapA,$mapA)))}; - {((cond) => {if (!cond) throw new Error("assertion failed: mapA == mapB.copy()")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })($mapA,$______mapB___)))}; + {((cond) => {if (!cond) throw new Error("assertion failed: mapA == mapB")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })($mapA,$mapB)))}; } } return $Closure7; @@ -226,7 +226,7 @@ module.exports = function({ $______mapB___, $mapA }) { ## inflight.$Closure8-1.js ```js -module.exports = function({ $______mapB___, $mapA, $mapC }) { +module.exports = function({ $mapA, $mapB, $mapC }) { class $Closure8 { constructor({ }) { const $obj = (...args) => this.handle(...args); @@ -235,7 +235,7 @@ module.exports = function({ $______mapB___, $mapA, $mapC }) { } async handle() { {((cond) => {if (!cond) throw new Error("assertion failed: mapA != mapC")})((((a,b) => { try { return require('assert').notDeepStrictEqual(a,b) === undefined; } catch { return false; } })($mapA,$mapC)))}; - {((cond) => {if (!cond) throw new Error("assertion failed: !(mapA != mapB.copy())")})((!(((a,b) => { try { return require('assert').notDeepStrictEqual(a,b) === undefined; } catch { return false; } })($mapA,$______mapB___))))}; + {((cond) => {if (!cond) throw new Error("assertion failed: !(mapA != mapB)")})((!(((a,b) => { try { return require('assert').notDeepStrictEqual(a,b) === undefined; } catch { return false; } })($mapA,$mapB))))}; } } return $Closure8; @@ -245,7 +245,7 @@ module.exports = function({ $______mapB___, $mapA, $mapC }) { ## inflight.$Closure9-1.js ```js -module.exports = function({ $_____arrayB__, $arrayA }) { +module.exports = function({ $arrayA, $arrayB }) { class $Closure9 { constructor({ }) { const $obj = (...args) => this.handle(...args); @@ -254,7 +254,7 @@ module.exports = function({ $_____arrayB__, $arrayA }) { } async handle() { {((cond) => {if (!cond) throw new Error("assertion failed: arrayA == arrayA")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })($arrayA,$arrayA)))}; - {((cond) => {if (!cond) throw new Error("assertion failed: arrayA == arrayB.copy()")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })($arrayA,$_____arrayB__)))}; + {((cond) => {if (!cond) throw new Error("assertion failed: arrayA == arrayB")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })($arrayA,$arrayB)))}; } } return $Closure9; @@ -1269,8 +1269,8 @@ class $Root extends $stdlib.std.Resource { static _toInflightType(context) { return $stdlib.core.NodeJsCode.fromInline(` require("./inflight.$Closure5-1.js")({ - $new_Set_setB_: ${context._lift(new Set(setB))}, $setA: ${context._lift(setA)}, + $setB: ${context._lift(setB)}, }) `); } @@ -1287,8 +1287,8 @@ class $Root extends $stdlib.std.Resource { } _registerBind(host, ops) { if (ops.includes("handle")) { - $Closure5._registerBindObject(new Set(setB), host, []); $Closure5._registerBindObject(setA, host, []); + $Closure5._registerBindObject(setB, host, []); } super._registerBind(host, ops); } @@ -1302,8 +1302,8 @@ class $Root extends $stdlib.std.Resource { static _toInflightType(context) { return $stdlib.core.NodeJsCode.fromInline(` require("./inflight.$Closure6-1.js")({ - $new_Set_setB_: ${context._lift(new Set(setB))}, $setA: ${context._lift(setA)}, + $setB: ${context._lift(setB)}, $setC: ${context._lift(setC)}, }) `); @@ -1321,8 +1321,8 @@ class $Root extends $stdlib.std.Resource { } _registerBind(host, ops) { if (ops.includes("handle")) { - $Closure6._registerBindObject(new Set(setB), host, []); $Closure6._registerBindObject(setA, host, []); + $Closure6._registerBindObject(setB, host, []); $Closure6._registerBindObject(setC, host, []); } super._registerBind(host, ops); @@ -1337,8 +1337,8 @@ class $Root extends $stdlib.std.Resource { static _toInflightType(context) { return $stdlib.core.NodeJsCode.fromInline(` require("./inflight.$Closure7-1.js")({ - $______mapB___: ${context._lift(({...(mapB)}))}, $mapA: ${context._lift(mapA)}, + $mapB: ${context._lift(mapB)}, }) `); } @@ -1355,8 +1355,8 @@ class $Root extends $stdlib.std.Resource { } _registerBind(host, ops) { if (ops.includes("handle")) { - $Closure7._registerBindObject(({...(mapB)}), host, []); $Closure7._registerBindObject(mapA, host, []); + $Closure7._registerBindObject(mapB, host, []); } super._registerBind(host, ops); } @@ -1370,8 +1370,8 @@ class $Root extends $stdlib.std.Resource { static _toInflightType(context) { return $stdlib.core.NodeJsCode.fromInline(` require("./inflight.$Closure8-1.js")({ - $______mapB___: ${context._lift(({...(mapB)}))}, $mapA: ${context._lift(mapA)}, + $mapB: ${context._lift(mapB)}, $mapC: ${context._lift(mapC)}, }) `); @@ -1389,8 +1389,8 @@ class $Root extends $stdlib.std.Resource { } _registerBind(host, ops) { if (ops.includes("handle")) { - $Closure8._registerBindObject(({...(mapB)}), host, []); $Closure8._registerBindObject(mapA, host, []); + $Closure8._registerBindObject(mapB, host, []); $Closure8._registerBindObject(mapC, host, []); } super._registerBind(host, ops); @@ -1405,8 +1405,8 @@ class $Root extends $stdlib.std.Resource { static _toInflightType(context) { return $stdlib.core.NodeJsCode.fromInline(` require("./inflight.$Closure9-1.js")({ - $_____arrayB__: ${context._lift([...(arrayB)])}, $arrayA: ${context._lift(arrayA)}, + $arrayB: ${context._lift(arrayB)}, }) `); } @@ -1423,8 +1423,8 @@ class $Root extends $stdlib.std.Resource { } _registerBind(host, ops) { if (ops.includes("handle")) { - $Closure9._registerBindObject([...(arrayB)], host, []); $Closure9._registerBindObject(arrayA, host, []); + $Closure9._registerBindObject(arrayB, host, []); } super._registerBind(host, ops); } @@ -1438,8 +1438,8 @@ class $Root extends $stdlib.std.Resource { static _toInflightType(context) { return $stdlib.core.NodeJsCode.fromInline(` require("./inflight.$Closure10-1.js")({ - $_____arrayB__: ${context._lift([...(arrayB)])}, $arrayA: ${context._lift(arrayA)}, + $arrayB: ${context._lift(arrayB)}, $arrayC: ${context._lift(arrayC)}, }) `); @@ -1457,8 +1457,8 @@ class $Root extends $stdlib.std.Resource { } _registerBind(host, ops) { if (ops.includes("handle")) { - $Closure10._registerBindObject([...(arrayB)], host, []); $Closure10._registerBindObject(arrayA, host, []); + $Closure10._registerBindObject(arrayB, host, []); $Closure10._registerBindObject(arrayC, host, []); } super._registerBind(host, ops);