Skip to content

Commit

Permalink
Merge pull request #614 from FirelyTeam/613-fix-integration-test---sy…
Browse files Browse the repository at this point in the history
…stemargumentexception-cannot-generate-a-hash-code-for-valuetuple3-arg_paramname_name

613 Fix CqlComparer for tuples
  • Loading branch information
baseTwo authored Dec 10, 2024
2 parents 8315c1a + d837ca7 commit 6ebc3ea
Show file tree
Hide file tree
Showing 35 changed files with 438 additions and 314 deletions.
2 changes: 1 addition & 1 deletion Cql/CodeGeneration.NET/CSharpLibrarySetToStreamsWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ private void WriteCqlTupleMetadataProperties(
foreach (var (propertyName, signature) in tupleMetadataBuilder.GetAllTupleMetadataPropertySignatures())
{
var types = string.Join(", ", signature.Select(t => $"typeof({_typeToCSharpConverter.ToCSharp(t.Type)})"));
var names = string.Join(", ", signature.Select(t => t.Name.QuoteString()));
var names = string.Join(", ", signature.Select(t => t.PropName.QuoteString()));
writer.WriteLine(indentLevel, $"private static CqlTupleMetadata {propertyName} = new(");
writer.WriteLine(indentLevel+1, $"[{types}],");
writer.WriteLine(indentLevel+1, $"[{names}]);");
Expand Down
2 changes: 1 addition & 1 deletion Cql/CodeGeneration.NET/ExpressionToCSharpConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public Context WithOverride(Func<int, int>? indentFn = null, Func<bool, bool>? u

public Context WithDoUseIndent() => WithOverride(useIndentFn: _ => true);

public string GetTupleMetadataPropertyName(IReadOnlyCollection<(string Name, Type Type)> tupleProperties) =>
public string GetTupleMetadataPropertyName(IReadOnlyCollection<(Type Type, string Name)> tupleProperties) =>
_tupleMetadataBuilder.GetTupleMetadataPropertyName(tupleProperties);
}

Expand Down
15 changes: 7 additions & 8 deletions Cql/CodeGeneration.NET/TupleMetadataBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,22 @@
using System;
using System.Collections.Generic;
using Hl7.Cql.Abstractions.Infrastructure;
using Hl7.Cql.Runtime;
using Hl7.Cql.Primitives;

namespace Hl7.Cql.CodeGeneration.NET;

internal class TupleMetadataBuilder
{
private readonly Dictionary<string, IReadOnlyCollection<(string Name, Type Type)>> _signaturesByHash = new();
private readonly Dictionary<string, IReadOnlyCollection<(Type Type, string PropName)>> _signaturesByHash = new();

public string GetTupleMetadataPropertyName(
IReadOnlyCollection<(string Name, Type Type)> tupleItemsSignature)
IReadOnlyCollection<(Type Type, string PropName)> tupleProps)
{
var propName = CqlTupleMetadata.BuildSignatureHashString(tupleItemsSignature);
_signaturesByHash[propName] = tupleItemsSignature;
var propName = CqlTupleMetadata.BuildSignatureHashString(tupleProps, CqlTupleMetadata.PropertyPrefix);
_signaturesByHash[propName] = tupleProps;
return propName;
}

public IReadOnlyCollection<(string PropertyName, IReadOnlyCollection<(string Name, Type Type)> Signature)> GetAllTupleMetadataPropertySignatures() =>
_signaturesByHash
.SelectToArray(kv => (kv.Key, kv.Value));
public IReadOnlyCollection<(string PropertyName, IReadOnlyCollection<(Type Type, string PropName)> Signature)> GetAllTupleMetadataPropertySignatures() =>
_signaturesByHash.SelectToArray(kv => (kv.Key, kv.Value));
}
13 changes: 5 additions & 8 deletions Cql/CodeGeneration.NET/TypeToCSharpConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Collections.Generic;
using System.Linq;
using Hl7.Cql.Abstractions.Infrastructure;
using Hl7.Cql.Primitives;

namespace Hl7.Cql.CodeGeneration.NET;

Expand All @@ -35,18 +36,14 @@ private TextWriterFormattableString FormatTypeNameAsTuple(ITypeNameCSharpFormatC
return formatTypeNameAsTuple;
}

public IEnumerable<(string Name, Type Type)> GetTupleProperties(Type type)
public IEnumerable<(Type Type, string Name)> GetTupleProperties(Type type)
{
var length = type.GetProperties().Length;
for (var i = 0; i < length; i++)
{
var prop = type.GetProperties()[i];
yield return (prop.Name, prop.PropertyType);
}
var properties = type.GetProperties();
return properties.Select(p => (p.PropertyType, p.Name));
}

public bool ShouldUseTupleType(Type type) =>
_useCSharpValueTuples && type.Name.StartsWith("Tuple_"); // REVIEW: This is a heuristic, and may not be correct in all cases.
_useCSharpValueTuples && type.IsTupleBaseType();

public string ToCSharp(Type type)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
using Hl7.Cql.Compiler.Infrastructure;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace CoreTests.Infrastructure;
namespace CoreTests.Abstractions;

[TestClass]
[TestCategory("UnitTest")]
Expand Down Expand Up @@ -83,7 +83,7 @@ public void TypeToCSharpString_ShouldReturnCorrectResults()

Assert.AreEqual(
expected: "System.Collections.Generic.IDictionary<, >",
actual: typeof(IDictionary<,>).ToCSharpString(typeFormatterOptions: new(NoGenericTypeParameterNames:true)));
actual: typeof(IDictionary<,>).ToCSharpString(typeFormatterOptions: new(NoGenericTypeParameterNames: true)));

Assert.AreEqual(
expected: "IDictionary<TKey, TValue>",
Expand All @@ -98,13 +98,13 @@ public void TypeToCSharpString_ShouldReturnCorrectResults()
typeFormatterOptions: new(
NoNamespaces: false,
UseKeywords: true,
GenericArgumentTokens: CSharpTokens.GenericArguments with { Separator = ","})));
GenericArgumentTokens: CSharpTokens.GenericArguments with { Separator = "," })));

Assert.AreEqual(
expected: "CoreTests.Infrastructure.EmptyStruct+Nested1+Nested2",
expected: "CoreTests.Abstractions.EmptyStruct+Nested1+Nested2",
actual: typeof(EmptyStruct.Nested1.Nested2).ToCSharpString(
typeFormatterOptions: new(
NestedTypeSeparator:"+")));
NestedTypeSeparator: "+")));

Assert.AreEqual(
expected: "System.Nullable<int>",
Expand All @@ -123,7 +123,7 @@ public void TypeToCSharpString_ShouldReturnCorrectResults()
[TestMethod]
public void MethodToCSharpString_GenericGenericDefinition_ShouldReturnCorrectResults()
{
MethodInfo m = ReflectionUtility.GenericMethodDefinitionOf(fnToMethodCall: () => default(TypeExtensionsTests.INonGenericInterface)!.GenericMethod<decimal,int,TypeExtensionsTests.MyDerivedClass,char>(default!, default!, default!));
MethodInfo m = ReflectionUtility.GenericMethodDefinitionOf(fnToMethodCall: () => default(TypeExtensionsTests.INonGenericInterface)!.GenericMethod<decimal, int, TypeExtensionsTests.MyDerivedClass, char>(default!, default!, default!));

Assert.AreEqual(
expected: "IList<T> GenericMethod<T1, T2, T3, T>(T1 a, T2[] b, IEnumerable<T3>[] c)",
Expand Down Expand Up @@ -153,12 +153,12 @@ public void MethodToCSharpString_NonGeneric_ShouldReturnCorrectResults()
tw = new TestTextWriter(new StringWriter());
var methodCSharpFormat = new MethodCSharpFormat(
MethodFormat: method => $"function {method.Name}{method.GenericArguments}{method.Parameters}: {method.ReturnType};",
ParameterFormat: new (
ParameterFormat: new(
ParameterFormat: parameter => $"{parameter.Name}: {parameter.Type}",
TypeFormat: new(
UseKeywords:true,
NoNamespaces:true)),
ParameterTokens: CSharpTokens.Parameters with { Separator = "; "}
UseKeywords: true,
NoNamespaces: true)),
ParameterTokens: CSharpTokens.Parameters with { Separator = "; " }
);
methodCSharpFormat.WriteTo(m, tw);
Assert.AreEqual(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
#pragma warning disable CS8500 // This takes the address of, gets the size of, or declares a pointer to a managed type

namespace CoreTests.Infrastructure;
namespace CoreTests.Abstractions;


[TestClass]
Expand Down Expand Up @@ -329,12 +329,12 @@ IList<T> GenericMethod<T1, T2, T3, T>(
T1 a,
T2[] b,
IEnumerable<T3>[] c)
where T1: struct, IComparable
where T2: notnull, new()
where T3: class, new();
where T1 : struct, IComparable
where T2 : notnull, new()
where T3 : class, new();
}

public abstract class MyGenericClassBase<T> : IGenericInterface<T>
public abstract class MyGenericClassBase<T> : IGenericInterface<T>
{
public void Method(T value)
{
Expand All @@ -359,9 +359,9 @@ public readonly record struct Nested1
{
public readonly record struct Nested2 { }

public readonly record struct GenericNested2<T1,T2> { }
public readonly record struct GenericNested2<T1, T2> { }

public delegate TOut NestedFunc<in TIn, out TOut>(TIn input)
where TIn: notnull;
where TIn : notnull;
};
}
1 change: 0 additions & 1 deletion Cql/CoreTests/Fhir/DataSourceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ public void FiltersOnDefaultProp()
}

[TestMethod]
[Ignore("Will be fixed in PR 614")]
public void FiltersOnSpecificProp()
{
var dr = buildDataSource();
Expand Down
65 changes: 65 additions & 0 deletions Cql/CoreTests/Primitives/TypeExtensionsTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
using System;
using Hl7.Cql.Primitives;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace CoreTests.Primitives;

[TestClass]
public class TypeExtensionsTests
{
[TestMethod]
public void IsCqlInterval_ShouldReturnTrueAndSetElementType_WhenTypeIsCqlInterval()
{
// Arrange
Type type = typeof(CqlInterval<int>);
Type elementType;

// Act
bool result = type.IsCqlInterval(out elementType);

// Assert
Assert.IsTrue(result);
Assert.AreEqual(typeof(int), elementType);
}

[TestMethod]
public void IsCqlInterval_ShouldReturnFalseAndSetElementTypeToNull_WhenTypeIsNotCqlInterval()
{
// Arrange
Type type = typeof(string);
Type elementType;

// Act
bool result = type.IsCqlInterval(out elementType);

// Assert
Assert.IsFalse(result);
Assert.IsNull(elementType);
}

[TestMethod]
public void IsCqlValueTuple_ShouldReturnTrue_WhenTypeIsCqlValueTuple()
{
// Arrange
Type type = typeof(ValueTuple<CqlTupleMetadata, int>);

// Act
bool result = type.IsCqlValueTuple();

// Assert
Assert.IsTrue(result);
}

[TestMethod]
public void IsCqlValueTuple_ShouldReturnFalse_WhenTypeIsNotCqlValueTuple()
{
// Arrange
Type type = typeof(string);

// Act
bool result = type.IsCqlValueTuple();

// Assert
Assert.IsFalse(result);
}
}
2 changes: 1 addition & 1 deletion Cql/CoreTests/Tuples/CqlTupleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
using System.Text.Json;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Hl7.Cql.Fhir;
using Hl7.Cql.Runtime;
using Hl7.Cql.Runtime.Serialization;
using System.IO;
using System.Runtime.Loader;
using Hl7.Cql.Packaging;
using Hl7.Cql.Primitives;

namespace CoreTests.Tuples;

Expand Down
8 changes: 6 additions & 2 deletions Cql/Cql.Abstractions/Cql.Abstractions.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\Iso8601\Iso8601.csproj" />
<InternalsVisibleTo Include="Hl7.Cql.Conversion" Key="$(LibraryPKHash)" />
<ProjectReference Include="..\Iso8601\Iso8601.csproj" />
</ItemGroup>

<ItemGroup>
<InternalsVisibleTo Include="Hl7.Cql.Conversion" Key="$(LibraryPKHash)" />
<InternalsVisibleTo Include="Hl7.Cql.Packager" Key="$(LibraryPKHash)" />
<InternalsVisibleTo Include="Hl7.Cql.MultiPackager" Key="$(LibraryPKHash)" />
<InternalsVisibleTo Include="Hl7.Cql.Fhir" Key="$(LibraryPKHash)" />
<InternalsVisibleTo Include="Hl7.Cql.Model" Key="$(LibraryPKHash)" />
<InternalsVisibleTo Include="Hl7.Cql.Runtime" Key="$(LibraryPKHash)" />
<InternalsVisibleTo Include="Hl7.Cql.Primitives" Key="$(LibraryPKHash)" />
<InternalsVisibleTo Include="Hl7.Cql.Compiler" Key="$(LibraryPKHash)" />
<InternalsVisibleTo Include="Hl7.Cql.Operators" Key="$(LibraryPKHash)" />
<InternalsVisibleTo Include="Hl7.Cql.Comparers" Key="$(LibraryPKHash)" />
Expand Down
1 change: 0 additions & 1 deletion Cql/Cql.Abstractions/Infrastructure/TypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
Expand Down
Loading

0 comments on commit 6ebc3ea

Please sign in to comment.