-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[release/6.0] Multi-target Json and Logger Source Generators between Roslyn v3.11 and v4.0 (#58446) #59074
[release/6.0] Multi-target Json and Logger Source Generators between Roslyn v3.11 and v4.0 (#58446) #59074
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<Project> | ||
<Target Name="_{TargetPrefix}GatherAnalyzers"> | ||
|
||
<ItemGroup> | ||
<_{TargetPrefix}Analyzer Include="@(Analyzer)" Condition="'%(Analyzer.NuGetPackageId)' == '{NuGetPackageId}'" /> | ||
</ItemGroup> | ||
</Target> | ||
|
||
<Target Name="_{TargetPrefix}AnalyzerMultiTargeting" | ||
Condition="'$(SupportsRoslynComponentVersioning)' != 'true'" | ||
AfterTargets="ResolvePackageDependenciesForBuild;ResolveNuGetPackageAssets" | ||
DependsOnTargets="_{TargetPrefix}GatherAnalyzers"> | ||
|
||
<ItemGroup> | ||
<!-- Remove our analyzers targeting roslyn4.x --> | ||
<Analyzer Remove="@(_{TargetPrefix}Analyzer)" | ||
Condition="$([System.String]::Copy('%(_{TargetPrefix}Analyzer.Identity)').IndexOf('roslyn4')) >= 0"/> | ||
</ItemGroup> | ||
</Target> | ||
|
||
<Target Name="_{TargetPrefix}RemoveAnalyzers" | ||
Condition="'$({DisableSourceGeneratorPropertyName})' == 'true'" | ||
AfterTargets="ResolvePackageDependenciesForBuild;ResolveNuGetPackageAssets" | ||
DependsOnTargets="_{TargetPrefix}GatherAnalyzers"> | ||
|
||
<!-- Remove all our analyzers --> | ||
<ItemGroup> | ||
<Analyzer Remove="@(_{TargetPrefix}Analyzer)" /> | ||
</ItemGroup> | ||
</Target> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -150,6 +150,41 @@ | |
</ItemGroup> | ||
</Target> | ||
|
||
<PropertyGroup> | ||
<_MultiTargetRoslynComponentTargetsTemplate>$(MSBuildThisFileDirectory)MultiTargetRoslynComponent.targets.template</_MultiTargetRoslynComponentTargetsTemplate> | ||
<MultiTargetRoslynComponentTargetsFileIntermediatePath>$(IntermediateOutputPath)MultiTargetRoslynComponent.targets</MultiTargetRoslynComponentTargetsFileIntermediatePath> | ||
<IncludeMultiTargetRoslynComponentTargets Condition="'$(IncludeMultiTargetRoslynComponentTargets)' == ''">true</IncludeMultiTargetRoslynComponentTargets> | ||
</PropertyGroup> | ||
|
||
<!-- In packages that contain Analyzers, include a .targets file that will select the correct analyzer. --> | ||
<Target Name="IncludeMultiTargetRoslynComponentTargetsInPackage" | ||
AfterTargets="IncludeAnalyzersInPackage" | ||
Condition="'@(AnalyzerReference)' != '' and '$(IncludeMultiTargetRoslynComponentTargets)' == 'true'" | ||
DependsOnTargets="GenerateMultiTargetRoslynComponentTargetsFile"> | ||
<ItemGroup> | ||
<Content Include="$(MultiTargetRoslynComponentTargetsFileIntermediatePath)" | ||
PackagePath="build\$(PackageId).targets" | ||
Pack="True" /> | ||
</ItemGroup> | ||
</Target> | ||
|
||
<Target Name="GenerateMultiTargetRoslynComponentTargetsFile" | ||
Inputs="$(MSBuildProjectFullPath);_MultiTargetRoslynComponentTargetsTemplate" | ||
Outputs="$(MultiTargetRoslynComponentTargetsFileIntermediatePath)"> | ||
<PropertyGroup> | ||
<_MultiTargetRoslynComponentTargetPrefix>$(PackageId.Replace('.', '_'))</_MultiTargetRoslynComponentTargetPrefix> | ||
<_MultiTargetRoslynComponentDisableSourceGeneratorPropertyName>Disable$(PackageId.Replace('.', ''))SourceGenerator</_MultiTargetRoslynComponentDisableSourceGeneratorPropertyName> | ||
<_MultiTargetRoslynComponentDisableSourceGeneratorPropertyName>$(_MultiTargetRoslynComponentDisableSourceGeneratorPropertyName.Replace('Abstractions', ''))</_MultiTargetRoslynComponentDisableSourceGeneratorPropertyName> | ||
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. nit: Not for this PR but maybe 7.0. It seems unusual to special case this centrally. I would have made this property public and let the project define what it wants this to be called. 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. I will make this change in |
||
</PropertyGroup> | ||
|
||
<WriteLinesToFile File="$(MultiTargetRoslynComponentTargetsFileIntermediatePath)" | ||
Lines="$([System.IO.File]::ReadAllText('$(_MultiTargetRoslynComponentTargetsTemplate)') | ||
.Replace('{TargetPrefix}', '$(_MultiTargetRoslynComponentTargetPrefix)') | ||
.Replace('{NuGetPackageId}', '$(PackageId)') | ||
.Replace('{DisableSourceGeneratorPropertyName}', '$(_MultiTargetRoslynComponentDisableSourceGeneratorPropertyName)'))" | ||
Overwrite="true" /> | ||
</Target> | ||
|
||
<!-- Include a netstandard compat error if the project targets both .NETStandard and | ||
.NETCoreApp. This prohibits users to consume packages on an older .NETCoreApp version | ||
than the minimum supported one. --> | ||
|
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.
Not blocking, but one clarification of the way this works is only for this package's analyzers, it does not have any effect if the analyzer from the shared framework would win. It might make sense to put package in the name of the property but that gets a little long. I guess if we document them we should make it clear it only applies to package.
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.
For the analyzers in the shared fx ref-pack, I was relying on dotnet/roslyn#55518 to provide this functionality, if needed.
I wouldn't object to putting
Package
in the property name, if we think that is the best option.DisableSystemTextJsonSourceGenerator
=>DisableSystemTextJsonPackageSourceGenerator
It expresses intent.
Do you have a strong opinion either way?