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
Load Razor source generators from SDK #9210
Conversation
33da13f
to
77c6172
Compare
analyzers/tests/SonarAnalyzer.TestFramework/Common/SourceGeneratorProvider.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Outdated
Show resolved
Hide resolved
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.
The implementation is sound! The UTs need some work.
analyzers/tests/SonarAnalyzer.TestFramework.Test/Build/ProjectBuilderTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework/Common/SourceGeneratorProvider.cs
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework/Common/SourceGeneratorProvider.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework/Common/SourceGeneratorProvider.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework/Common/SourceGeneratorProvider.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework/Common/SourceGeneratorProvider.cs
Outdated
Show resolved
Hide resolved
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.
Round 2
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework/Common/SourceGeneratorProvider.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/SourceGeneratorProviderTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework/Common/SourceGeneratorProvider.cs
Outdated
Show resolved
Hide resolved
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.
LGTM. Left a few minor improvement suggestions.
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.
The implementation doesn't seem to do what we said.
And the condition causing the coverage problem can be safely removed
analyzers/tests/SonarAnalyzer.TestFramework/Common/SourceGeneratorProvider.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.TestFramework/Common/SourceGeneratorProvider.cs
Outdated
Show resolved
Hide resolved
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.
The implementation is still wrong. It doens't use the latest.
It should use the same as the object assembly, and that will fall to the latest without depending on string-comparison mismatch.
Imagine
8.0.0
8.0.1
8.9.0
8.10.0
While 8.10.0 is the latest that we currently run on, the First
will pick up 8.0.0 instead.
Changing it to Last
doesn't work, because that would pick up 8.9.0 instead.
It needs to be reworked
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.
LGTM
.OrderByDescending(x => Version.Parse(new DirectoryInfo(x).Name)) | ||
.First(); |
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.
What about OrderBy(..).Last()
to have more positive logic?
Quality Gate passed for 'Sonar .NET Java Plugin'Issues Measures |
Quality Gate passed for 'SonarAnalyzer for .NET'Issues Measures |
Fixes #9212