-
Notifications
You must be signed in to change notification settings - Fork 256
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
Source generator samples #511
Conversation
@chsienki I suggest we keep this in its own branch until the feature is complete. Then we merge this into master once its done. We can also work on the testing portion of this in that branch |
I would prefer they be in This is a samples section and these are samples of our work in progress. The compiler changes are in |
I would be fine with a readme explaining that these are a work-in-progress. I am just concerned that we would have a bunch of shipped samples next to some in-progress ones with no differentiation |
Added a readme to explain they're in progress. Happy to play with the wording to get something we all like. |
|
||
<ItemGroup> | ||
<!--<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="2.9.8" />--> | ||
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.6.0-3.20207.2" PrivateAssets="all" /> |
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.
This currently emits a NuGet warning about depending on Microsoft.CodeAnalysis.Analyzers, picking 3.0.0-beta2.final.
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.
Fixed.
The install.ps1 are for installing the analyzers as a package, but we don't support it for generators yet. Removed to clear up confusion. |
// begin creating the source we'll inject into the users compilation | ||
StringBuilder sourceBuilder = new StringBuilder(@" | ||
using System; | ||
namespace HelloWorldGenerator |
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.
Might be better to have this be HelloWorldGenerated
to distinguish from the name of the generator
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.
Updated.
In the README there should be instructions on how to install the sample generators into your own project. Since there's no example here showing consumption, folks will definitely want the prescriptive guidance. |
- Add demo consumption project - Update readme - Lock samples to correct .net version via global.json
@cartermp I Added a demo project that consumes the samples, and allows a 'one click' run experience to see what they do, and how to consume them. I updated the readme to include how to build and consume in your own project. I added a solution in the SourceGenerators dir so you can just open the generators sample + demo project without anything else. |
|
||
Building the samples | ||
----- | ||
Open `SourceGenerators.sln` in Visual Studio or run `dotnet build` from the `\SourceGenerators` directory. |
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.
I don't see SourceGenerators.sln
in this PR or repo. Did you mean to open Samples.sln
?
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.
Yes, it might help if I added that file to the repo huh?
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "SourceGenerators", "SourceGenerators", "{14D18F51-6B59-49D5-9AB7-08B38417A459}" | ||
EndProject | ||
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SourceGeneratorSamples", "samples\CSharp\SourceGenerators\SourceGeneratorSamples\SourceGeneratorSamples.csproj", "{2ADE5CFA-5DF4-44A9-BD67-E884BCFBA045}" | ||
EndProject |
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.
This appears to be missing the consumption project.
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.
Why did you need to downgrade the demo? This is now using a non-supported |
Yeah, it wasn't able to build netcoreapp3.1 Looks like that just pushed the problem further though, because it's now using a compiler that doesn't support SourceGenerators. I think I'll just remove them from the main 'shipping' |
@chsienki what was the original failure about? Do we need to install a runtime for .NET 3.1 or update the sdk in the global.json file? |
@jmarolf https://dev.azure.com/dnceng/public/_build/results?buildId=619533&view=logs&j=a846d25a-e32c-5640-1b53-e815fab94407&t=aafa82ad-f831-5b30-c619-9b12d27e8ee8 I think we'll need to update the global.json. I added one for the Wasn't sure if we'd want the shipping samples in the repo depening on a pre-release SDK? |
ah change the global.json from this {
"tools": {
"dotnet": "3.0.101",
"vs": {
"version": "16.3"
},
"xcopy-msbuild": "16.3.0-alpha"
},
"msbuild-sdks": {
"Microsoft.DotNet.Arcade.Sdk": "5.0.0-beta.20117.3"
}
} to this {
"tools": {
"dotnet": "3.1.101",
"vs": {
"version": "16.3"
},
"xcopy-msbuild": "16.3.0-alpha"
},
"msbuild-sdks": {
"Microsoft.DotNet.Arcade.Sdk": "5.0.0-beta.20117.3"
}
} |
That won't fix it though, because 3.1.101 doesn't include the source generator support. So we need to either make the whole repo depend on 3.1.300 or remove them from the main samples solution. |
ah, ok. I am fine with that, There are lots of repos that depend on un-released versions of the SDK |
Remove special source generators one
fix reference in release
🎉 |
Adds a sample generator project which contains three example generators:
INotifyPropertyChanged
support, and shows how to use theSyntaxReceiver
functionality.These are still a work in progress, and we expect them to change as we evolve the feature. In the long run these are expected to represent both simple how-to's and best practices with generators (we'll probably update the SG cookbook in the roslyn repo to start pointing here for example).
The project doesn't have tests yet. I suspect we'll want to build out some tooling like we provide for analyzers for testing generators.