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

Add C# Source Generator #1726

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

valchetski
Copy link

@valchetski valchetski commented Aug 29, 2024

What's in this PR

This change aims to add a C# Source generator that automatically creates classes based on JSON schema. The idea is to make code generation more straightforward for generic scenarios. I've tried that approach on my project and it worked quite well. I thought it'd be good to make it a part of the library. This Source Generator has few configurable properties and can be extended in the future if needed.

Current approach

To generate C# classes you need to create a separate console project that will generate a .cs file and put it somewhere in your Solution. Then you need to manually run that console application to generate/regenerate the models. Here's example

New (additional) approach

Add the following elements in the .csproj file of your project where the generated .cs file will exist:

  <ItemGroup>
    <PackageReference Include="NJsonSchema.SourceGenerators.CSharp" />
  </ItemGroup>

  <ItemGroup>
    <AdditionalFiles 
        Include="schema.json" 
        NJsonSchema_GenerateOptionalPropertiesAsNullable="true" 
        NJsonSchema_Namespace="NJsonSchema.Demo.Generated" 
        NJsonSchema_TypeNameHint="PersonGenerated" 
        NJsonSchema_FileName="Person.g.cs" />
  </ItemGroup>

After that Source Generator will automatically create a .cs file during compilation. No additional console project or manual job is needed.

Concerns

I have some questions/concerns regarding the implementation and have put them in the comments. We can start from this main one.

Also added some explanatory comments about why something was implemented that way. As tooling for the Source Generators is not the best for now and there is a lot of msbuild magic in the .csproj files.

@@ -14,9 +14,12 @@
<PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
<PackageVersion Include="NodaTime" Version="3.1.9" />
<PackageVersion Include="NSwag.Core.Yaml" Version="14.0.0-preview011" />
<PackageVersion Include="Parlot" Version="0.0.25.0" />
Copy link
Author

Choose a reason for hiding this comment

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

Source generators do not support transient dependencies: dotnet/sdk#17775
Because of that need to explicitly install some transient packages.
For example, it'll be used here to include Parlot into NuGet package https://github.com/RicoSuter/NJsonSchema/pull/1726/files#diff-ead3877feb09b2619c61d0feb8b85e0c85c627756febe0dda0829dafe13d5325R39

</ItemGroup>

<ItemGroup>
<None Update="Tests\**\*.json" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>

<ItemGroup>
<AdditionalFiles
Copy link
Author

Choose a reason for hiding this comment

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

In this project you can see how Source Generator actually works

</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\NJsonSchema.CodeGeneration.CSharp\NJsonSchema.CodeGeneration.CSharp.csproj" PrivateAssets="All">
Copy link
Author

@valchetski valchetski Aug 29, 2024

Choose a reason for hiding this comment

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

With PrivateAssets="All" it will not add NJsonSchema.CodeGeneration.CSharp as a dependency of NJsonSchema.SourceGenerators.CSharp NuGet package as I assume it's not needed. Code from NJsonSchema.SourceGenerators.CSharp should be enough to generate C# classes. For more advanced scenarios NJsonSchema.CodeGeneration.CSharp can be installed explicitly in client's projects.

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" PrivateAssets="all" />
<PackageReference Include="Newtonsoft.Json" GeneratePathProperty="true" />
Copy link
Author

Choose a reason for hiding this comment

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

Deliberately didn't add PrivateAssets="all" for Newtonsoft.Json so it will be included as NuGet dependency. In the generated code Newtonsoft attributes will be used. That way client don't need to install Newtonsoft.Json in their project explicitly.

@@ -0,0 +1,74 @@
<Project Sdk="Microsoft.NET.Sdk">
Copy link
Author

@valchetski valchetski Aug 29, 2024

Choose a reason for hiding this comment

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

My main concern is whether we should create a separate project for the source generator or add them to the existing NJsonSchema.CodeGeneration.CSharp. Frankly speaking, I didn't try the second approach at all, but in theory, it should work.
Below are my thoughts.

Add Source Generator to new NJsonSchema.SourceGenerators.CSharp

Pros:

  • Clients will install that package only if they know they need to use a source generator.
  • Easier to maintain as source generator projects require additional configuration (IsRoslynComponent, exclude NU5128 warning, etc.)

Cons:

  • The names of the NuGet packages could be confusing. NJsonSchema.CodeGeneration.CSharp and NJsonSchema.SourceGenerators.CSharp may sound a bit synonimical.

Add Source Generator to existing NJsonSchema.CodeGeneration.CSharp

Pros:

  • We don't introduce a new NuGet package.
  • By installing NJsonSchema.CodeGeneration.CSharp client can use either classes from the library or/and Source Generator.

Cons:

  • It'll automatically install the source generator when the client does a package upgrade. (However, the source generator will not do much without additional configuration. Not sure if this is an issue).
  • The size of the NuGet package will slightly increase as it'll include DLLs for the source generator.
  • The project might be harder to maintain as it will be a mix of regular code and code that will go to the source generator.

@valchetski valchetski marked this pull request as ready for review August 29, 2024 15:49
@HavenDV
Copy link

HavenDV commented Sep 25, 2024

You can check my implementation of Source Generator for NSwag to see how to support transient packages automatically https://github.com/HavenDV/H.NSwag.Generator

@valchetski
Copy link
Author

You can check my implementation of Source Generator for NSwag to see how to support transient packages automatically https://github.com/HavenDV/H.NSwag.Generator

Thank you @HavenDV. Amazing library 🙂 However, I have some issues with project references. I asked about them here: HavenDV/H.Generators.Extensions#15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants