Report deprecation analysis warning for MsBuild 14/15 #6710
Conversation
pavel-mikula-sonarsource
force-pushed
the
Pavel/Deprecation
branch
from
84e15ac
to
bb11181
Compare
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/AnalysisWarningAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/AnalysisWarningAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/Utilities/AnalysisWarningAnalyzerTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/Utilities/AnalysisWarningAnalyzerTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/Utilities/AnalysisWarningAnalyzerTest.cs
Outdated
Show resolved
Hide resolved
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <!-- This will enforce using Roslyn 1.3.1 --> |
There was a problem hiding this comment.
[education] how does this work?
There was a problem hiding this comment.
It has some ugly targets that copy specific Roslyn compiler, kill existing processes and enforce different version to be used. I don't know exact details.
The project is stolen from .NET ITs where I used it to ensure proper integration between the old and the new SE engines.
https://github.com/SonarSource/sonar-dotnet/tree/master/analyzers/its/sources/Roslyn.1.3.1
|
|
||
| @Test | ||
| public void analysisWarnings_OldRoslyn() throws IOException { | ||
| BuildResult buildResult = Tests.analyzeProject(temp, "Roslyn.1.3.1", null); |
There was a problem hiding this comment.
shouldn't we actually have integration tests for MSB14 and MSB15?
I guess we only have tests with different MSB versions on sonar-security...
There was a problem hiding this comment.
MsBuild 14 is bound with Roslyn 1.x
MsBuild 15 is bound with Roslyn 2.x
So while we could do the integration testing like that, it's not worth the effort to build the infra for it.
Effectively, once we bump the Roslyn version, users on the old Roslyn will have trouble. And it's a good thing that all of them will already have the message.
Comments:
- We can't determine MsBuild version from within the analyzer. S4NET would have to do that. Yet not all users on SQ 10.0 will be forced to use the latest scanner.
- To align the message and the code, we should warn them about using old Roslyn. The problem with that is that users have no idea what Roslyn they use and how they are related to the build tools versions. So they wouldn't understand the problem.
- Combination of MsBuild 17 and DoetNetCompilerPlatform v1.x and v2.x package (or any other custom build solutions enforcing old Roslyn) will produce a message that is confusing for users. Yet valid to show, as they will have braking changes once we bump the version.
There was a problem hiding this comment.
did you test manually with MSB 15?
because you say that MSB15 is bound to Roslyn 2.x, but the IT is using Roslyn 1.x
or am I missing smth?
There was a problem hiding this comment.
I cannot create IT for it, because a higher version of the package is giving Roslyn 4.4.0 😒
I plan to validate on Peach, I don't have MsBuild 15 locally.
Reference: https://learn.microsoft.com/en-us/visualstudio/extensibility/roslyn-version-support?view=vs-2022
There was a problem hiding this comment.
it should be easy to install the build tools (there's no need for a full VS instance) https://community.chocolatey.org/packages/visualstudio2017buildtools
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/AnalysisWarningAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/AnalysisWarningAnalyzerBase.cs
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/AnalysisWarningAnalyzerBase.cs
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/AnalysisWarningAnalyzerBase.cs
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/Utilities/AnalysisWarningAnalyzerTest.cs
Outdated
Show resolved
Hide resolved
|
Kudos, SonarCloud Quality Gate passed!
|
|
SonarCloud Quality Gate failed. |
Fixes #6678
MsBuild 14 is bound with Roslyn 1.x
MsBuild 15 is bound with Roslyn 2.x
We're fine with MsBuild 16+ and Roslyn 3.x+
https://learn.microsoft.com/en-us/visualstudio/extensibility/roslyn-version-support?view=vs-2022
While it is possible to have MsBuild 16+ and use Roslyn 1.x with it (see the trick in ITs), it's a corner case that we'll ignore as we can't detect MsBuild version from within Roslyn without help of Scanner for .NET.
The analyzer complains when old Roslyn is used and that's also correct behavior. As bumping Roslyn dependency will make it stop working on that Roslyn, no matter what MsBuild is used.
How this works:
SQ plugin (the java part) already has a mechanism, that picks up all JSON files from predefined location with predefined name. And will report them as analysis warnings in UI. This is used for scanner and autoscan (other applications dropping files around to report issues).
So all we need to do is to drop one file per analysis (per solution) onto a disk.
Sample file: https://github.com/SonarSource/sonar-dotnet/blob/master/sonar-dotnet-shared-library/src/test/resources/analysisWarnings/AnalysisWarnings.AutoScan.json