-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Correctly support incremental source generation #62
Changes from 4 commits
17ca7b0
ceb45e2
5797558
cdff4c9
d12bf22
018b0d8
afd36b0
762c681
1eae2be
60fc464
cacd1dd
75f67a8
7e0e70c
927e2a7
0df78df
6d11303
5cadbe9
df2966c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,3 +5,5 @@ obj | |
*.DotSettings.user | ||
.vs | ||
.DS_Store | ||
|
||
Generator.Equals.Tests/Generated/** |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
extern alias genEquals; | ||
using System.Collections.Immutable; | ||
using Microsoft.CodeAnalysis.CSharp; | ||
|
||
namespace Generator.Equals.Tests.DynamicGeneration; | ||
|
||
public class Base_Assertions | ||
{ | ||
// Check if immutable arrays are comparable | ||
[Fact] | ||
public void Immutable_Arrays_Eqatable() | ||
JKamsker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
var a = ImmutableArray.Create(1, 2, 3); | ||
var b = ImmutableArray.Create(1, 2, 3); | ||
Assert.Equal(a, b); | ||
} | ||
|
||
[Fact] | ||
public void Immutable_Arrays_Comparable() | ||
{ | ||
var a = ImmutableArray.Create(1, 2, 3); | ||
var b = ImmutableArray.Create(1, 2, 3); | ||
Assert.True(a.SequenceEqual(b)); | ||
} | ||
|
||
// Check if equality type model is comparable by value | ||
[Fact] | ||
public void EqualityTypeModel_Eqatable() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: typo |
||
{ | ||
var a = CreateEqualityTypeModelMock(); | ||
var b = CreateEqualityTypeModelMock(); | ||
Assert.Equal(a, b); | ||
} | ||
|
||
[Fact] | ||
public void AttributesMetadata_Equatable() | ||
{ | ||
var a = AttributesMetadata.CreateDefault(); | ||
var b = AttributesMetadata.CreateDefault(); | ||
Assert.True(a.Equals(b)); | ||
} | ||
|
||
internal static EqualityTypeModel CreateEqualityTypeModelMock() | ||
{ | ||
var containingSymbols = ImmutableArray.Create<ContainingSymbol>( | ||
new NamespaceContainingSymbol { Name = "Namespace1" }, | ||
new TypeContainingSymbol { Name = "ContainingType", Kind = null } | ||
); | ||
|
||
var attributesMetadata = AttributesMetadata.Instance; | ||
|
||
var equalityMemberModels = ImmutableArray.Create( | ||
new EqualityMemberModel | ||
{ | ||
PropertyName = "Property1", | ||
TypeName = "int", | ||
EqualityType = EqualityType.DefaultEquality | ||
}, | ||
new EqualityMemberModel | ||
{ | ||
PropertyName = "Property2", | ||
TypeName = "string", | ||
EqualityType = EqualityType.StringEquality, | ||
StringComparer = attributesMetadata.StringComparisonLookup[4] | ||
} | ||
); | ||
|
||
return new EqualityTypeModel( | ||
typeName: "MyType", | ||
containingSymbols: containingSymbols, | ||
attributesMetadata: attributesMetadata, | ||
explicitMode: false, | ||
ignoreInheritedMembers: false | ||
) | ||
{ | ||
SyntaxKind = SyntaxKind.ClassDeclaration, | ||
BaseTypeName = "BaseType", | ||
IsSealed = true, | ||
BuildEqualityModels = equalityMemberModels, | ||
Fullname = "Namespace1.MyType" | ||
}; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>net8.0</TargetFramework> | ||
<ImplicitUsings>enable</ImplicitUsings> | ||
<Nullable>enable</Nullable> | ||
|
||
<IsPackable>false</IsPackable> | ||
<IsTestProject>true</IsTestProject> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="coverlet.collector" Version="6.0.0" /> | ||
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" /> | ||
<PackageReference Include="xunit" Version="2.5.3" /> | ||
<PackageReference Include="xunit.runner.visualstudio" Version="2.5.3" /> | ||
<PackageReference Include="SourceGeneratorTestHelpers" Version="8.1.0" /> | ||
|
||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\Generator.Equals.Runtime\Generator.Equals.Runtime.csproj" /> | ||
<ProjectReference Include="..\Generator.Equals\Generator.Equals.csproj"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you using an alias for this reference? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of |
||
<Aliases>genEquals</Aliases> | ||
</ProjectReference> | ||
</ItemGroup> | ||
|
||
|
||
<ItemGroup> | ||
<Using Include="Xunit" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
extern alias genEquals; | ||
|
||
global using genEquals::Generator.Equals.Models; | ||
global using genEquals::Generator.Equals; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
extern alias genEquals; | ||
|
||
using genEquals::System.Diagnostics.CodeAnalysis; | ||
|
||
namespace Generator.Equals.DynamicGenerationTests; | ||
|
||
internal static class SourceText | ||
{ | ||
public static string CSharp([StringSyntax("c#-test")] string source) => source; | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename this file. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,216 @@ | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.CSharp; | ||
|
||
using SourceGeneratorTestHelpers; | ||
|
||
namespace Generator.Equals.DynamicGenerationTests; | ||
|
||
public class UnitTest1 | ||
{ | ||
public static readonly List<PortableExecutableReference> References2 = | ||
AppDomain.CurrentDomain.GetAssemblies() | ||
.Where(_ => !_.IsDynamic && !string.IsNullOrWhiteSpace(_.Location)) | ||
.Select(_ => MetadataReference.CreateFromFile(_.Location)) | ||
.Concat(new[] | ||
{ | ||
// add your app/lib specifics, e.g.: | ||
MetadataReference.CreateFromFile(typeof(EquatableAttribute).Assembly.Location), | ||
}) | ||
JKamsker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.ToList(); | ||
|
||
[Fact] | ||
public void Test1() | ||
{ | ||
var input = SourceText.CSharp( | ||
""" | ||
using System; | ||
using Generator.Equals; | ||
|
||
namespace Tests; | ||
|
||
[Equatable] | ||
public partial record MyRecord( | ||
[property: OrderedEquality] string[] Fruits, | ||
[property: StringEquality(StringComparison.OrdinalIgnoreCase)] string Fruit | ||
); | ||
|
||
""" | ||
); | ||
|
||
var result = IncrementalGenerator.Run<EqualsGenerator> | ||
( | ||
input, | ||
new CSharpParseOptions() | ||
{ | ||
}, | ||
References2 | ||
); | ||
|
||
var gensource = result.Results | ||
.SelectMany(x => x.GeneratedSources) | ||
.Select(x => x.SourceText) | ||
.ToList() | ||
; | ||
|
||
Assert.NotNull(gensource); | ||
} | ||
|
||
[Fact] | ||
public void Test2_Nested() | ||
{ | ||
var input = SourceText.CSharp( | ||
""" | ||
using System; | ||
using System.Collections.Generic; | ||
using Generator.Equals; | ||
|
||
|
||
namespace Generator.Equals.Tests.Records | ||
{ | ||
public partial class CustomEquality | ||
{ | ||
[Equatable] | ||
public partial record Sample | ||
{ | ||
public Sample(string name1, string name2, string name3) | ||
{ | ||
Name1 = name1; | ||
Name2 = name2; | ||
Name3 = name3; | ||
} | ||
|
||
[CustomEquality(typeof(Comparer1))] public string Name1 { get; } | ||
[CustomEquality(typeof(Comparer2), nameof(Comparer2.Instance))] public string Name2 { get; } | ||
[CustomEquality(typeof(LengthEqualityComparer))] public string Name3 { get; } | ||
} | ||
|
||
class Comparer1 | ||
{ | ||
public static readonly LengthEqualityComparer Default = new(); | ||
} | ||
|
||
class Comparer2 | ||
{ | ||
public static readonly LengthEqualityComparer Instance = new(); | ||
} | ||
|
||
class LengthEqualityComparer : IEqualityComparer<string> | ||
{ | ||
public bool Equals(string? x, string? y) => x?.Length == y?.Length; | ||
|
||
public int GetHashCode(string obj) => obj.Length.GetHashCode(); | ||
} | ||
|
||
} | ||
} | ||
""" | ||
); | ||
|
||
var result = IncrementalGenerator.Run<EqualsGenerator> | ||
( | ||
input, | ||
new CSharpParseOptions() | ||
{ | ||
}, | ||
References2 | ||
); | ||
|
||
var gensource = result.Results | ||
.SelectMany(x => x.GeneratedSources) | ||
.Select(x => x.SourceText) | ||
.ToList() | ||
; | ||
|
||
Assert.NotNull(gensource); | ||
} | ||
|
||
[Fact] | ||
public void Test3_Struct_UnorderedEquality() | ||
{ | ||
var input = SourceText.CSharp( | ||
""" | ||
using System.Collections.Generic; | ||
|
||
namespace Generator.Equals.Tests.Structs | ||
{ | ||
public partial class UnorderedEquality | ||
{ | ||
[Equatable] | ||
public partial struct Sample | ||
{ | ||
[UnorderedEquality] public List<int>? Properties { get; set; } | ||
} | ||
} | ||
} | ||
""" | ||
); | ||
|
||
var result = IncrementalGenerator.Run<EqualsGenerator> | ||
( | ||
input, | ||
new CSharpParseOptions() | ||
{ | ||
}, | ||
References2 | ||
); | ||
|
||
var gensource = result.Results | ||
.SelectMany(x => x.GeneratedSources) | ||
.Select(x => x.SourceText) | ||
.ToList() | ||
; | ||
|
||
Assert.NotNull(gensource); | ||
} | ||
|
||
[Fact] | ||
public void Test3_Struct_ExplicitMode() | ||
{ | ||
var input = SourceText.CSharp( | ||
""" | ||
namespace Generator.Equals.Tests.Structs | ||
{ | ||
public partial class ExplicitMode | ||
{ | ||
[Equatable(Explicit = true)] | ||
public partial struct Sample | ||
{ | ||
bool _flag; | ||
|
||
public Sample(string name, int age, bool flag) | ||
{ | ||
_flag = flag; | ||
Name = name; | ||
Age = age; | ||
} | ||
|
||
public string Name { get; } | ||
|
||
[DefaultEquality] | ||
public int Age { get; } | ||
} | ||
} | ||
} | ||
""" | ||
); | ||
|
||
var result = IncrementalGenerator.Run<EqualsGenerator> | ||
( | ||
input, | ||
new CSharpParseOptions() | ||
{ | ||
}, | ||
References2 | ||
); | ||
|
||
var gensource = result.Results | ||
.SelectMany(x => x.GeneratedSources) | ||
.Select(x => x.SourceText) | ||
.ToList() | ||
; | ||
|
||
Assert.NotNull(gensource); | ||
} | ||
|
||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a personal preference, I do two things with regards to generated code:
CSharpGeneratorDriver.Create(generator).RunGeneratorsAndUpdateCompilation(..);
andVerify
; theRunGeneratorsAndUpdateCompilation(..)
call to verify that both the original and generated code compile correctly, andVerify
to store the generated output and require confirmation of any changes to the generated output.EmitCompilerGeneratedFiles
, since any generated code I may wish to review is already available via the generated tests.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening the code in VS fully, I see that there are now four test projects. Only two are necessary, imo, using the above process.
It appears that SnapshotTests has the first bullet point, and Tests contain the functional tests. TopLevel is probably unnecessary, and I am uncertain what DynamicGeneration is supposed to add over the existing ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea i generally think the sln would require some form of restructuring. Ideally following your format (from eg Immediate.Validators).
But i would like to move that topic to another pr, since there is already another pr. Changing the folder structure would prolly require me to do everything in that other pr again 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You still did not answer why you added
DynamicGeneration
. The existing layout was fine, thoughTopLevel
is probably unnecessary; addingDynamicGeneration
is the question under concern here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea i wanted to generate and debug the generator without having to debug visual studio itslef.
In the other pr i need it to verify that the issue has been fixed https://github.com/diegofrata/Generator.Equals/blob/25d158fe365fc8ca43e08498ae6c95cf72d94142/Generator.Equals.DynamicGenerationTests/Issues/Issue-60-StringEquality-Enumerables.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: You do that by using
<IsRoslynComponent />
and the correctlaunchSettings.json
. Adding a new project entirely seems unnecessary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea removed the proj for now...