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

Support for nuget package with multiple Roslyn version analyzers #616

Merged
merged 11 commits into from
Mar 3, 2024

Conversation

hadashiA
Copy link
Contributor

To address the #615, I added a process to prevent the same analyzer from being enabled more than once.

It works as follows.

  • If analyzers/dotnet subfolders are not split into versions, then it remains the same.
  • If not, only newer versions that can be supported will be given the RoslynAnalyzer label.

src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs Outdated Show resolved Hide resolved
src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs Outdated Show resolved Hide resolved
if (!enabledRoslynVersions.Contains(assetRoslynVersion) ||
string.CompareOrdinal(assetRoslynVersion, enabledRoslynVersions.Max()) < 0)
{
AssetDatabase.SetLabels(plugin, new[] { ProcessedLabel });
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of setting and then removing the label, just save in some bool variable if RoslynAnalyzer label should be added and then in the end just SetLabels once to proper value depending on that bool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I fixed.

@hadashiA hadashiA requested a review from igor84 January 25, 2024 06:38
@hadashiA hadashiA requested a review from JoC0de February 1, 2024 01:11
Copy link
Collaborator

@JoC0de JoC0de left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.
I still don't understand why Unity doesn't handle this by itself but at least we can fix it 😃

src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs Outdated Show resolved Hide resolved
src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs Outdated Show resolved Hide resolved
@hadashiA hadashiA requested a review from JoC0de February 14, 2024 06:55
… add unit test for package with multiple roslyn analyzers
@JoC0de JoC0de merged commit 0218352 into GlitchEnzo:master Mar 3, 2024
8 checks passed
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.

Unintended behavior when there are analyzers for multiple Roslyn versions in one nuget package
4 participants