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

Providing a Source Generator #277

Open
shuebner opened this issue Aug 8, 2021 · 23 comments
Open

Providing a Source Generator #277

shuebner opened this issue Aug 8, 2021 · 23 comments

Comments

@shuebner
Copy link

shuebner commented Aug 8, 2021

Hi Michael,
have you considered publishing a source generator to go along with your XmlSchemaClassGenerator?
I have created one as a proof-of-concept in my own repo.
Apart from the annoying System.ComponentModel.Annotations issue, it went as smoothly as building a source generator can get considering the current documentation and tooling ;-).

@mganss
Copy link
Owner

mganss commented Aug 9, 2021

Hey Sven, aweome work! Also thanks for the kind words in your blog article. TBH I hadn't thought of doing a source generator but it's a great idea. Would you be interested in contributing code to this project?

@shuebner
Copy link
Author

shuebner commented Aug 9, 2021

Hi Michael,
I would love to contribute. What would the requirements for the generator be?

We could allow the same configuration options as the console application, since those options are already given by string values and are thus easy to port to csproj attributes.
We could scope all options to the consuming project, not to the xsd AdditionalFile, except maybe the --namespace option, which could (additionally?) be file-scoped.
Can you think of anything else?

What would make the generator implementation easier is using System.ComponentModel.Annotations package version 4.4.0 instead of 5.0.0 in the actual XmlSchemaClassGenerator. This way we can avoid adding the dll manually in the build process (which would pretty much mean adding it to the repo). The issue is discussed at length here. It seems to boil down to the lib folder assembly version being different than the ref folder assembly version in every nuget package above 4.4.0 (and yes, they did this even with the 6.0.0 preview package). I can see the XmlSchemaClassGenerator code only uses the annotation attributes as Type and I did not get any errors or warnings when changing the package version. For fun, I decompiled both and looked at the differences. I also checked out the dotnet runtime repo and had a quick blame. There are some new features, like new properties RangeAttribute.ParseLimitsInInvariantCulture and they annotated the attributes for nullability. You be the judge if we could live with losing that.

Looking into the future with people using the source generator on big and complex schema files (as I maybe will on my current assignment), we may need to benchmark and possibly optimize XmlSchemaClassGenerator for speed more. I don't know if speed was ever a concern. But one thing after the other ;-)

@mganss
Copy link
Owner

mganss commented Aug 11, 2021

OK, I've pinned System.ComponentModel.Annotations to 4.4.0. We don't actually depend on runtime functionality of the attributes, we use them only as statically typed references in constructing the CodeDOM. If worse comes to worst, I think we can even replace all references with strings and remove the NuGet package dependency.

Regarding the configuration options, your suggestions look good to me. As you have indicated, the namespace mapping should additionally be scoped to the .xsd files to enable the scenario described here: https://github.com/mganss/XmlSchemaClassGenerator#mapping-xsd-files-to-c-namespaces

Speed was never a concern. The biggest problem in this area is probably network access when big graphs of schema files are downloaded. If all necessary schema files are locally available, I doubt that performance will be an issue even for interactive scenarios in VS.

@shuebner
Copy link
Author

Alright, I will see how much time I have this week. I will create a PR for discussing the details when I have a first draft. Do I have to fork or are you going to add me as contributor?

@mganss
Copy link
Owner

mganss commented Aug 11, 2021

Let's try doing this on a PR basis at first. If it turns out be too complicated, I'll add you as a collaborator.

@sergeiwork
Copy link

Just to comment on this. Last version doesn't work with Linq2db because linq2db requires System.ComponentModel.Annotations >= 4.7.0

@mganss
Copy link
Owner

mganss commented Aug 24, 2021

@sergeiwork So you're using the library version of XmlSchemaClassGenerator in a project with linq2db?

@sergeiwork
Copy link

Yes I'm using nuget package and linq2db

@mganss
Copy link
Owner

mganss commented Aug 25, 2021

OK I've removed the package reference altogether and replaced the type references with hard coded strings 🤷🏻

@sergeiwork
Copy link

Thank you! That was really quick =)

@TFTomSun
Copy link

TFTomSun commented Mar 29, 2022

I am interested in source generator support as well. @mganss what's the status of this work? I saw that there's a draft of a pull request. Will it be merged?

@shuebner
Copy link
Author

The spirits of the past come to haunt me 😉. I said I would do it and then did not. I am sorry. The PR should work in principle. There are just some options missing for the generator to be useful in practice, such as setting an output filename or folder and potentially other options that are also provided by the CLI.

It also should be lifted to Roslyn 4.x and maybe use incremental source generation properly to not slow down the development experience.

@mganss
Copy link
Owner

mganss commented Apr 4, 2022

@shuebner Can you implement the changes you mentioned?

@shuebner
Copy link
Author

shuebner commented Apr 5, 2022

@mganss I am capable but on a very tight schedule. I will see what I can do but do not expect anything within less than two weeks. I hope it will not take another 8 months ;-)

@shuebner
Copy link
Author

Alright. I rebased my code and made some changes (in particular all settings are now scoped to an additional file aka a schema file).

It works for me in the dotnet CLI with .NET 6.0.201, but not in VS 2022 for some reason.
Your appveyor build also fails with the same message as VS: MethodMissing on AdditionalFiles.

I will investigate why this is.

@shuebner
Copy link
Author

shuebner commented May 3, 2022

I just investigated the issue and the reason for the deviating behavior seems to be that VS 2022 (17.1.5) has -- at source generator runtime -- System.Collections.Immutable 5.0.0 loaded, whereas building via command line has System.Collections.Immutable 6.0.0 loaded.

This leads to the MethodMissingException on GeneratorExecutionContext.AdditionalFiles because in the return type ImmutableArray<AdditionalText> the assembly version of AdditionalText is the same, but the assembly version of ImmutableArray is different.

Because XmlSchemaClassGenerator and the source generator are compiled against System.Collections.Immutable 6.0.0, the command line workflow works and the VS workflow does not.

If this were an executable we would maybe employ a binding redirect or something similar.
If XmlSchemaClassGenerator referenced System.Collections.Immutable 5.0.0, it would also work in both workflows.

I try to make it work as is because really it should be possible to have a source generator built on top of a library without changing the library's dependency as long as something like a binding redirect would work at runtime as well.

@shuebner
Copy link
Author

shuebner commented May 3, 2022

For the time being I updated the PR to a working state by changing XmlSchemaClassGenerator's dependency on System.Collections.Immutable from 6.0.0 to 5.0.0.

The appveyor build still fails, now for a different reason that is not directly apparent from the error message. If someone can see the entire stacktrace or tell me how to reproduce it locally, please do so.

@shuebner
Copy link
Author

shuebner commented May 3, 2022

OK, this was actually quite easy. The appveyor build uses the 5.0.407 SDK, which apparently has no binding redirects for the compiler assemblies. Locally reproducing the issue with a global.json and 5.0.100 SDK, the generator run fails to load Microsoft.CodeAnalysis, Version=3.10.0..

This error does not occur with the .NET 6 SDK.

Using the 4.x compiler assemblies will probably solve the issue. I will update the PR once more.

@shuebner
Copy link
Author

shuebner commented May 3, 2022

OK, it seems that .NET 5 SDK has a problem with source generators in general. Tons of people are reporting different dll loading problems. That is probably why they added the proper redirects in the .NET 6 SDK.

@mganss I will not try to fix this. .NET 6 is the LTS version anyway. I suggest moving forward to using .NET 6 SDK in your appveyor build

@mganss
Copy link
Owner

mganss commented May 3, 2022

@shuebner Thanks, I've updated the AppVeyor build to VS 2022. Please rebase your PR.

@shuebner
Copy link
Author

shuebner commented May 4, 2022

@mganss done. In the source generator test project you can see the generator in action.

To be able to support the source generator with the current VS 2022, XmlSchemaClassGenerator would have to use System.Collections.Immutable 5.0.0 (instead of 6.0.0). I think there is no way around that.

Now that there is a PoC, we can think about where the generator would live.

TL;DR: I suggest it live in this repo.

In principle, it could live in its own repo, pulling the dll for XmlSchemaClassGenerator via the library nuget package.
But, seeing the different issues regarding dependencies (first Annotations, now Immutable), this approach would lead to future issues being discovered late (when there is already a new nuget package of XmlSchemaClassGenerator).

If the source generator lives in this repo and its build and test run along with XmlSchemaClassGenerator's existing automated build chain, those issues would be discovered early enough to still make design changes or at least trigger discussions.

Next step would be to play around with it in a real-life setting. I will do that with my current project, in which I need to build XML files for 4+ different versions of the same set of schema files.
After the bugs are fixed and missing options are added for that, we could publish a first release version of the generator.

@mganss
Copy link
Owner

mganss commented May 4, 2022

@shuebner Sounds good. Of course the source generator could live in this repo.

@pfeigl
Copy link

pfeigl commented Oct 27, 2023

Do you guys plan to finish work on this? This would be such a great feature for people still beeing forced to work with XML

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

No branches or pull requests

5 participants