From 68e6d7a3ff169a5dd16ecb61f4f2b36023e32043 Mon Sep 17 00:00:00 2001 From: beppemarazzi Date: Fri, 3 Jan 2020 17:08:45 +0100 Subject: [PATCH 1/6] fix equals implementation when comparing different types derived from the same base --- .../Given_GeneratedEquality.DerivedTypes.cs | 59 +++++++++++++++++++ src/Uno.CodeGen/EqualityGenerator.cs | 1 + 2 files changed, 60 insertions(+) create mode 100644 src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs diff --git a/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs b/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs new file mode 100644 index 0000000..898ab74 --- /dev/null +++ b/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs @@ -0,0 +1,59 @@ +// ****************************************************************** +// Copyright � 2015-2018 nventive inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// ****************************************************************** +using FluentAssertions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Uno.Equality; + +namespace Uno.CodeGen.Tests +{ + public partial class Given_GeneratedEquality + { + [TestMethod] + public void Equality_WhenUsingDifferentDerivedTypes() + { + var d1 = new MyDerived1.Builder { a = 10, b = 15, }.ToImmutable(); + var d2 = new MyDerived2.Builder { a = 10, c = 25, }.ToImmutable(); + var d3 = new MyDerived3.Builder { a = 10, b = 15, c=25 }.ToImmutable(); + + (d1 == d2).Should().BeFalse(); + d1.Equals(d2).Should().BeFalse(); + d2.Equals(d1).Should().BeFalse(); + (d1==d3).Should().BeFalse(); + d1.Equals(d3).Should().BeFalse(); + d3.Equals(d1).Should().BeFalse(); + } + + } + + [GeneratedImmutable] + internal partial class MyBase + { + public int a { get; } + } + internal partial class MyDerived1 : MyBase + { + public int b { get; } + } + internal partial class MyDerived2 : MyBase + { + public int c { get; } + } + internal partial class MyDerived3 : MyDerived1 + { + public int c { get; } + } +} diff --git a/src/Uno.CodeGen/EqualityGenerator.cs b/src/Uno.CodeGen/EqualityGenerator.cs index f91c0f0..b464c3f 100644 --- a/src/Uno.CodeGen/EqualityGenerator.cs +++ b/src/Uno.CodeGen/EqualityGenerator.cs @@ -254,6 +254,7 @@ namespace Uno builder.AppendLineInvariant("// private method doing the real .Equals() job"); using (builder.BlockInvariant($"private bool InnerEquals({symbolNameWithGenerics} other)")) { + builder.AppendLineInvariant("if (other.GetType() != GetType()) return false;"); builder.AppendLineInvariant("if (other.GetHashCode() != GetHashCode()) return false;"); var baseCall = baseTypeInfo.baseOverridesEquals From c23f832f47ebf74f0d4757cd1a4641f21e5281b5 Mon Sep 17 00:00:00 2001 From: beppemarazzi Date: Fri, 3 Jan 2020 18:20:36 +0100 Subject: [PATCH 2/6] better test assertions --- .../Given_GeneratedEquality.DerivedTypes.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs b/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs index 898ab74..ff5a616 100644 --- a/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs +++ b/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs @@ -29,12 +29,18 @@ namespace Uno.CodeGen.Tests var d2 = new MyDerived2.Builder { a = 10, c = 25, }.ToImmutable(); var d3 = new MyDerived3.Builder { a = 10, b = 15, c=25 }.ToImmutable(); - (d1 == d2).Should().BeFalse(); - d1.Equals(d2).Should().BeFalse(); - d2.Equals(d1).Should().BeFalse(); - (d1==d3).Should().BeFalse(); - d1.Equals(d3).Should().BeFalse(); - d3.Equals(d1).Should().BeFalse(); + (d1 == d2).Should().BeFalse("d1 == d2"); + (d2 == d1).Should().BeFalse("d2 == d1"); + (d1 != d2).Should().BeTrue("d1 != d2"); + (d2 != d1).Should().BeTrue("d2 != d1"); + d1.Should().NotBe(d2, "d1.Equals(d2)"); + d2.Should().NotBe(d1, "d2.Equals(d1)"); + (d1 == d3).Should().BeFalse("d1 == d3"); + (d3 == d1).Should().BeFalse("d3 == d1"); + (d1 != d3).Should().BeTrue("d1 == d3"); + (d3 != d1).Should().BeTrue("d3 == d1"); + d1.Should().NotBe(d3, "d1.Equals(d3)"); + d3.Should().NotBe(d1, "d3.Equals(d1)"); } } From 123b757cd9d15aca2b95b57e90a0c6e45f2092f6 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Fri, 3 Jan 2020 13:23:30 -0500 Subject: [PATCH 3/6] Run tests in CI --- .vsts-ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.vsts-ci.yml b/.vsts-ci.yml index 1a260b7..77dd4cc 100644 --- a/.vsts-ci.yml +++ b/.vsts-ci.yml @@ -7,6 +7,8 @@ phases: - powershell: './build/build.ps1 -script build/build.cake' + - task: VSTest@2 + - task: CopyFiles@2 inputs: SourceFolder: $(Build.SourcesDirectory)/build From d404b7144e6a560f68028f7bd404dd6ca4e7f7cf Mon Sep 17 00:00:00 2001 From: beppemarazzi Date: Sat, 4 Jan 2020 09:47:25 +0100 Subject: [PATCH 4/6] add test to verify if base.Equals is called --- .../ConcreteExternalClass.cs | 6 ++++++ .../Given_GeneratedEquality.DerivedTypes.cs | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/Uno.CodeGen.Tests.ExternalClasses/ConcreteExternalClass.cs b/src/Uno.CodeGen.Tests.ExternalClasses/ConcreteExternalClass.cs index 9b70cb2..eb92e94 100644 --- a/src/Uno.CodeGen.Tests.ExternalClasses/ConcreteExternalClass.cs +++ b/src/Uno.CodeGen.Tests.ExternalClasses/ConcreteExternalClass.cs @@ -22,4 +22,10 @@ namespace Uno.CodeGen.Tests.ExternalClasses [EqualityHash] public string Id { get; } } + + [GeneratedImmutable] + public partial class ConcreteExternalClassNoHash + { + public string Id { get; } + } } diff --git a/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs b/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs index ff5a616..f21d393 100644 --- a/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs +++ b/src/Uno.CodeGen.Tests/Given_GeneratedEquality.DerivedTypes.cs @@ -43,6 +43,19 @@ namespace Uno.CodeGen.Tests d3.Should().NotBe(d1, "d3.Equals(d1)"); } + [TestMethod] + public void Equality_WhenUsingDerivedFromExternal() + { + var d1 = new MyADerived.Builder { Id = "d1", a = 15, }.ToImmutable(); + var d2 = new MyADerived.Builder { Id = "d2", a = 15, }.ToImmutable(); + + (d1 == d2).Should().BeFalse("d1 == d2"); + (d2 == d1).Should().BeFalse("d2 == d1"); + (d1 != d2).Should().BeTrue("d1 != d2"); + (d2 != d1).Should().BeTrue("d2 != d1"); + d1.Should().NotBe(d2, "d1.Equals(d2)"); + d2.Should().NotBe(d1, "d2.Equals(d1)"); + } } [GeneratedImmutable] @@ -62,4 +75,9 @@ namespace Uno.CodeGen.Tests { public int c { get; } } + + internal partial class MyADerived : Uno.CodeGen.Tests.ExternalClasses.ConcreteExternalClassNoHash + { + public int a { get; } + } } From 6ea2ca183e8239be6194e732306ac2a724cd5190 Mon Sep 17 00:00:00 2001 From: beppemarazzi Date: Sat, 4 Jan 2020 10:22:20 +0100 Subject: [PATCH 5/6] fix base.Equals not called when base is in external assembly --- src/Uno.CodeGen/EqualityGenerator.cs | 40 +++++++++++++++++----------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/Uno.CodeGen/EqualityGenerator.cs b/src/Uno.CodeGen/EqualityGenerator.cs index b464c3f..46a0198 100644 --- a/src/Uno.CodeGen/EqualityGenerator.cs +++ b/src/Uno.CodeGen/EqualityGenerator.cs @@ -99,28 +99,36 @@ namespace Uno private string _currentType = "unknown"; + private INamedTypeSymbol GetTypeSymbol(string name) + { + var s = _context.Compilation.GetTypeByMetadataName(name); + if (s == null) + throw new InvalidOperationException($"Invalid type symbol '{name}'"); + return s; + } + /// public override void Execute(SourceGeneratorContext context) { _context = context; _logger = context.GetLogger(); - _objectSymbol = context.Compilation.GetTypeByMetadataName("System.Object"); - _valueTypeSymbol = context.Compilation.GetTypeByMetadataName("System.ValueType"); - _boolSymbol = context.Compilation.GetTypeByMetadataName("System.Bool"); - _intSymbol = context.Compilation.GetTypeByMetadataName("System.Int32"); - _enumSymbol = context.Compilation.GetTypeByMetadataName("System.Enum"); - _arraySymbol = context.Compilation.GetTypeByMetadataName("System.Array"); - _collectionSymbol = context.Compilation.GetTypeByMetadataName("System.Collections.ICollection"); - _iEquatableSymbol = context.Compilation.GetTypeByMetadataName("System.IEquatable`1"); - _iKeyEquatableSymbol = context.Compilation.GetTypeByMetadataName("Uno.Equality.IKeyEquatable"); - _iKeyEquatableGenericSymbol = context.Compilation.GetTypeByMetadataName("Uno.Equality.IKeyEquatable`1"); - _generatedEqualityAttributeSymbol = context.Compilation.GetTypeByMetadataName("Uno.GeneratedEqualityAttribute"); - _ignoreForEqualityAttributeSymbol = context.Compilation.GetTypeByMetadataName("Uno.EqualityIgnoreAttribute"); - _equalityHashAttributeSymbol = context.Compilation.GetTypeByMetadataName("Uno.EqualityHashAttribute"); - _equalityKeyAttributeSymbol = context.Compilation.GetTypeByMetadataName("Uno.EqualityKeyAttribute"); - _equalityComparerOptionsAttributeSymbol = context.Compilation.GetTypeByMetadataName("Uno.Equality.EqualityComparerOptionsAttribute"); - _dataAnnonationsKeyAttributeSymbol = context.Compilation.GetTypeByMetadataName("System.ComponentModel.DataAnnotations.KeyAttribute"); + _objectSymbol = GetTypeSymbol("System.Object"); + _valueTypeSymbol = GetTypeSymbol("System.ValueType"); + _boolSymbol = GetTypeSymbol("System.Boolean"); + _intSymbol = GetTypeSymbol("System.Int32"); + _enumSymbol = GetTypeSymbol("System.Enum"); + _arraySymbol = GetTypeSymbol("System.Array"); + _collectionSymbol = GetTypeSymbol("System.Collections.ICollection"); + _iEquatableSymbol = GetTypeSymbol("System.IEquatable`1"); + _iKeyEquatableSymbol = GetTypeSymbol("Uno.Equality.IKeyEquatable"); + _iKeyEquatableGenericSymbol = GetTypeSymbol("Uno.Equality.IKeyEquatable`1"); + _generatedEqualityAttributeSymbol = GetTypeSymbol("Uno.GeneratedEqualityAttribute"); + _ignoreForEqualityAttributeSymbol = GetTypeSymbol("Uno.EqualityIgnoreAttribute"); + _equalityHashAttributeSymbol = GetTypeSymbol("Uno.EqualityHashAttribute"); + _equalityKeyAttributeSymbol = GetTypeSymbol("Uno.EqualityKeyAttribute"); + _equalityComparerOptionsAttributeSymbol = GetTypeSymbol("Uno.Equality.EqualityComparerOptionsAttribute"); + _dataAnnonationsKeyAttributeSymbol = GetTypeSymbol("System.ComponentModel.DataAnnotations.KeyAttribute"); _isPureAttributePresent = context.Compilation.GetTypeByMetadataName("System.Diagnostics.Contracts.Pure") != null; _generateKeyEqualityCode = _iKeyEquatableSymbol != null; From dc3d7304b4fd29db2e31b9d74dbd72a378b7b97d Mon Sep 17 00:00:00 2001 From: beppemarazzi Date: Sat, 4 Jan 2020 11:16:21 +0100 Subject: [PATCH 6/6] Uno.Equality.IKeyEquatable is not mandatory --- src/Uno.CodeGen/EqualityGenerator.cs | 34 ++++++++++++++-------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Uno.CodeGen/EqualityGenerator.cs b/src/Uno.CodeGen/EqualityGenerator.cs index 46a0198..2b35c48 100644 --- a/src/Uno.CodeGen/EqualityGenerator.cs +++ b/src/Uno.CodeGen/EqualityGenerator.cs @@ -99,7 +99,7 @@ namespace Uno private string _currentType = "unknown"; - private INamedTypeSymbol GetTypeSymbol(string name) + private INamedTypeSymbol GetMandatoryTypeSymbol(string name) { var s = _context.Compilation.GetTypeByMetadataName(name); if (s == null) @@ -113,22 +113,22 @@ namespace Uno _context = context; _logger = context.GetLogger(); - _objectSymbol = GetTypeSymbol("System.Object"); - _valueTypeSymbol = GetTypeSymbol("System.ValueType"); - _boolSymbol = GetTypeSymbol("System.Boolean"); - _intSymbol = GetTypeSymbol("System.Int32"); - _enumSymbol = GetTypeSymbol("System.Enum"); - _arraySymbol = GetTypeSymbol("System.Array"); - _collectionSymbol = GetTypeSymbol("System.Collections.ICollection"); - _iEquatableSymbol = GetTypeSymbol("System.IEquatable`1"); - _iKeyEquatableSymbol = GetTypeSymbol("Uno.Equality.IKeyEquatable"); - _iKeyEquatableGenericSymbol = GetTypeSymbol("Uno.Equality.IKeyEquatable`1"); - _generatedEqualityAttributeSymbol = GetTypeSymbol("Uno.GeneratedEqualityAttribute"); - _ignoreForEqualityAttributeSymbol = GetTypeSymbol("Uno.EqualityIgnoreAttribute"); - _equalityHashAttributeSymbol = GetTypeSymbol("Uno.EqualityHashAttribute"); - _equalityKeyAttributeSymbol = GetTypeSymbol("Uno.EqualityKeyAttribute"); - _equalityComparerOptionsAttributeSymbol = GetTypeSymbol("Uno.Equality.EqualityComparerOptionsAttribute"); - _dataAnnonationsKeyAttributeSymbol = GetTypeSymbol("System.ComponentModel.DataAnnotations.KeyAttribute"); + _objectSymbol = GetMandatoryTypeSymbol("System.Object"); + _valueTypeSymbol = GetMandatoryTypeSymbol("System.ValueType"); + _boolSymbol = GetMandatoryTypeSymbol("System.Boolean"); + _intSymbol = GetMandatoryTypeSymbol("System.Int32"); + _enumSymbol = GetMandatoryTypeSymbol("System.Enum"); + _arraySymbol = GetMandatoryTypeSymbol("System.Array"); + _collectionSymbol = GetMandatoryTypeSymbol("System.Collections.ICollection"); + _iEquatableSymbol = GetMandatoryTypeSymbol("System.IEquatable`1"); + _iKeyEquatableSymbol = _context.Compilation.GetTypeByMetadataName("Uno.Equality.IKeyEquatable"); + _iKeyEquatableGenericSymbol = _context.Compilation.GetTypeByMetadataName("Uno.Equality.IKeyEquatable`1"); + _generatedEqualityAttributeSymbol = GetMandatoryTypeSymbol("Uno.GeneratedEqualityAttribute"); + _ignoreForEqualityAttributeSymbol = GetMandatoryTypeSymbol("Uno.EqualityIgnoreAttribute"); + _equalityHashAttributeSymbol = GetMandatoryTypeSymbol("Uno.EqualityHashAttribute"); + _equalityKeyAttributeSymbol = GetMandatoryTypeSymbol("Uno.EqualityKeyAttribute"); + _equalityComparerOptionsAttributeSymbol = GetMandatoryTypeSymbol("Uno.Equality.EqualityComparerOptionsAttribute"); + _dataAnnonationsKeyAttributeSymbol = GetMandatoryTypeSymbol("System.ComponentModel.DataAnnotations.KeyAttribute"); _isPureAttributePresent = context.Compilation.GetTypeByMetadataName("System.Diagnostics.Contracts.Pure") != null; _generateKeyEqualityCode = _iKeyEquatableSymbol != null;