Skip to content
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

Merged
merged 18 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore

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:

  • Generator tests, which use CSharpGeneratorDriver.Create(generator).RunGeneratorsAndUpdateCompilation(..); and Verify; the RunGeneratorsAndUpdateCompilation(..) call to verify that both the original and generated code compile correctly, and Verify to store the generated output and require confirmation of any changes to the generated output.
  • Functional tests, which test the behavior of the generated code.
    • In these functional tests, I do not use EmitCompilerGeneratedFiles, since any generated code I may wish to review is already available via the generated tests.

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.

Copy link
Contributor Author

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 😅

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, though TopLevel is probably unnecessary; adding DynamicGeneration is the question under concern here.

Copy link
Contributor Author

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

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 correct launchSettings.json. Adding a new project entirely seems unnecessary.

Copy link
Contributor Author

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...

Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ obj
*.DotSettings.user
.vs
.DS_Store

Generator.Equals.Tests/Generated/**
82 changes: 82 additions & 0 deletions Generator.Equals.DynamicGenerationTests/Base_Assertions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
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()
Copy link
Owner

Choose a reason for hiding this comment

The 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
{
SyntaxKind = SyntaxKind.ClassDeclaration,
TypeName = "MyType",
BaseTypeName = "BaseType",
IsSealed = true,
ContainingSymbols = containingSymbols,
AttributesMetadata = attributesMetadata,
ExplicitMode = false,
IgnoreInheritedMembers = false,
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">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you using an alias for this reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of PolySharp in Generator.Equals - it generates the StringSyntaxAttribute, but internal. InternalsVisibleTo makes the build fail because of ambiguity.

<Aliases>genEquals</Aliases>
</ProjectReference>
</ItemGroup>


<ItemGroup>
<Using Include="Xunit" />
</ItemGroup>

</Project>
4 changes: 4 additions & 0 deletions Generator.Equals.DynamicGenerationTests/Globals.cs
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;
10 changes: 10 additions & 0 deletions Generator.Equals.DynamicGenerationTests/SourceText.cs
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;
}
14 changes: 13 additions & 1 deletion Generator.Equals.Tests/Generator.Equals.Tests.csproj
diegofrata marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,19 @@
<NoWarn>NU1701</NoWarn>
<Nullable>enable</Nullable>
</PropertyGroup>


<!-- For debugging -->
JKamsker marked this conversation as resolved.
Show resolved Hide resolved
<!--
<PropertyGroup>
<EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles>
<CompilerGeneratedFilesOutputPath>$(MSBuildProjectDirectory)\Generated</CompilerGeneratedFilesOutputPath>
</PropertyGroup>
<ItemGroup>
<Compile Remove="Generated\**" />
<None Include="Generated\**" />
</ItemGroup>
-->

<ItemGroup>

<PackageReference Include="Microsoft.Bcl.HashCode" Version="1.1.1" Condition="'$(TargetFramework.TrimEnd(`0123456789`))' == 'net'" />
Expand Down
2 changes: 1 addition & 1 deletion Generator.Equals.Tests/Structs/UnorderedEquality.Sample.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ public partial struct Sample
[UnorderedEquality] public List<int>? Properties { get; set; }
}
}
}
}
10 changes: 8 additions & 2 deletions Generator.Equals.sln
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Generator.Equals.Tests", "G
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Generator.Equals.SnapshotTests", "Generator.Equals.SnapshotTests\Generator.Equals.SnapshotTests.csproj", "{F11B4229-E761-4135-B758-E61AC4FF5303}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Generator.Equals.Tests.TopLevel", "Generator.Equals.Tests.TopLevel\Generator.Equals.Tests.TopLevel.csproj", "{9A48A8A6-4213-4CBB-B184-FE1E50F3721E}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Generator.Equals.Tests.TopLevel", "Generator.Equals.Tests.TopLevel\Generator.Equals.Tests.TopLevel.csproj", "{9A48A8A6-4213-4CBB-B184-FE1E50F3721E}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Generator.Equals.Runtime", "Generator.Equals.Runtime\Generator.Equals.Runtime.csproj", "{2AD99F42-5E2C-451A-97EE-59C64EC8B270}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Generator.Equals.Runtime", "Generator.Equals.Runtime\Generator.Equals.Runtime.csproj", "{2AD99F42-5E2C-451A-97EE-59C64EC8B270}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Generator.Equals.Tests.DynamicGeneration", "Generator.Equals.DynamicGenerationTests\Generator.Equals.Tests.DynamicGeneration.csproj", "{20F96A29-3BC9-4115-B99D-1E6D45C6340F}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Expand Down Expand Up @@ -39,6 +41,10 @@ Global
{2AD99F42-5E2C-451A-97EE-59C64EC8B270}.Debug|Any CPU.Build.0 = Debug|Any CPU
{2AD99F42-5E2C-451A-97EE-59C64EC8B270}.Release|Any CPU.ActiveCfg = Release|Any CPU
{2AD99F42-5E2C-451A-97EE-59C64EC8B270}.Release|Any CPU.Build.0 = Release|Any CPU
{20F96A29-3BC9-4115-B99D-1E6D45C6340F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{20F96A29-3BC9-4115-B99D-1E6D45C6340F}.Debug|Any CPU.Build.0 = Debug|Any CPU
{20F96A29-3BC9-4115-B99D-1E6D45C6340F}.Release|Any CPU.ActiveCfg = Release|Any CPU
{20F96A29-3BC9-4115-B99D-1E6D45C6340F}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down
43 changes: 0 additions & 43 deletions Generator.Equals/AttributesMetadata.cs

This file was deleted.

69 changes: 16 additions & 53 deletions Generator.Equals/ContainingTypesBuilder.cs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opening this file in VS 2022 comes up with 9 warnings. These should all be addressed.

Original file line number Diff line number Diff line change
@@ -1,88 +1,51 @@
using System;
using System.CodeDom.Compiler;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Text;
using Microsoft.CodeAnalysis;

using Generator.Equals.Models;

using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace Generator.Equals
{
static class ContainingTypesBuilder
internal static class ContainingTypesBuilder
{
static IEnumerable<INamespaceOrTypeSymbol> ContainingNamespaceAndTypes(ISymbol symbol, bool includeSelf)
{
foreach (var item in AllContainingNamespacesAndTypes(symbol, includeSelf))
{
yield return item;

if (item.IsNamespace)
yield break;
}
}

static IEnumerable<INamespaceOrTypeSymbol> AllContainingNamespacesAndTypes(ISymbol symbol, bool includeSelf)
public static string Build(ImmutableArray<ContainingSymbol> containingSymbols, Action<IndentedTextWriter> content)
{
if (includeSelf && symbol is INamespaceOrTypeSymbol self)
yield return self;
using var buffer = new StringWriter(new StringBuilder(capacity: 4096));
using var writer = new IndentedTextWriter(buffer);

while (true)
foreach (var parentSymbol in containingSymbols.Reverse())
{
symbol = symbol.ContainingSymbol;

if (!(symbol is INamespaceOrTypeSymbol namespaceOrTypeSymbol))
yield break;

yield return namespaceOrTypeSymbol;
}
}

public static string Build(ISymbol symbol, Action<IndentedTextWriter> content, bool includeSelf = false)
{
var buffer = new StringWriter(new StringBuilder(capacity: 4096));
var writer = new IndentedTextWriter(buffer);
var symbols = ContainingNamespaceAndTypes(symbol, includeSelf).ToList();

for (var i = symbols.Count - 1; i >= 0; i--)
{
var s = symbols[i];

if (s.IsNamespace)
if (parentSymbol is NamespaceContainingSymbol namespaceSymbol)
{
writer.WriteLine();
writer.WriteLine(EqualityGeneratorBase.EnableNullableContext);
writer.WriteLine(EqualityGeneratorBase.SuppressObsoleteWarningsPragma);
writer.WriteLine(EqualityGeneratorBase.SuppressTypeConflictsWarningsPragma);
writer.WriteLine();

var namespaceName = s.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat.WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted));

if (!string.IsNullOrEmpty(namespaceName))
if (!string.IsNullOrEmpty(namespaceSymbol.Name))
{
writer.WriteLine($"namespace {namespaceName}");
writer.WriteLine($"namespace {namespaceSymbol.Name}");
writer.AppendOpenBracket();
}
}
else
else if (parentSymbol is TypeContainingSymbol typeContainingSymbol)
{
var typeDeclarationSyntax = s.DeclaringSyntaxReferences
.Select(x => x.GetSyntax())
.OfType<TypeDeclarationSyntax>()
.First();

var keyword = typeDeclarationSyntax.Kind() switch
var keyword = typeContainingSymbol.Kind switch

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(SyntaxKind)9068 can be updated to SyntaxKind.RecordStructDeclaration

{
SyntaxKind.ClassDeclaration => "class",
SyntaxKind.RecordDeclaration => "record",
(SyntaxKind)9068 => "record struct", // RecordStructDeclaration
SyntaxKind.RecordStructDeclaration => "record struct",
SyntaxKind.StructDeclaration => "struct",
var x => throw new ArgumentOutOfRangeException($"Syntax kind {x} not supported")
};

var typeName = s.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat);
writer.WriteLine($"partial {keyword} {typeName}");
writer.WriteLine($"partial {keyword} {parentSymbol.Name}");
writer.AppendOpenBracket();
}
}
Expand Down
Loading