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

Report deprecation analysis warning for MsBuild 14/15 #6710

Merged
merged 8 commits into from Feb 23, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions analyzers/src/SonarAnalyzer.CFG/Helpers/RoslynHelper.cs
Expand Up @@ -20,9 +20,9 @@

namespace SonarAnalyzer.CFG.Helpers
{
internal static class RoslynHelper
public static class RoslynHelper
{
private const int MinimalSupportedMajorVersion = 3;
public const int MinimalSupportedMajorVersion = 3;

public static bool IsRoslynCfgSupported(int minimalVersion = MinimalSupportedMajorVersion) =>
typeof(SemanticModel).Assembly.GetName().Version.Major >= minimalVersion;
Expand Down
@@ -0,0 +1,24 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class AnalysisWarningAnalyzer : AnalysisWarningAnalyzerBase { }
@@ -0,0 +1,61 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.IO;
using SonarAnalyzer.CFG.Helpers;

namespace SonarAnalyzer.Rules;

public abstract class AnalysisWarningAnalyzerBase : UtilityAnalyzerBase
{
private const string DiagnosticId = "S9999-warning";
private const string Title = "Analysis Warning generator";

private static readonly object FileWriteLock = new();

protected virtual int MinimalSupportedRoslynVersion { get; } = RoslynHelper.MinimalSupportedMajorVersion; // For testing
costin-zaharia-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

protected AnalysisWarningAnalyzerBase() : base(DiagnosticId, Title) { }

protected sealed override void Initialize(SonarAnalysisContext context) =>
context.RegisterCompilationAction(c =>
{
ReadParameters(c);
if (IsAnalyzerEnabled && !RoslynHelper.IsRoslynCfgSupported(MinimalSupportedRoslynVersion)) // MsBuild 15 is bound with Roslyn 2.x, where Roslyn CFG is not available.
costin-zaharia-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
{
costin-zaharia-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
// This can be removed after we bump Microsoft.CodeAnalysis references to 3.0 or higher.
costin-zaharia-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
var path = Path.GetFullPath(Path.Combine(OutPath, "../../AnalysisWarnings.MsBuild.json"));
lock (FileWriteLock)
{
if (!File.Exists(path))
{
try
{
File.WriteAllText(path, """[{"text": "Analysis using MsBuild 14 and 15 build tools is deprecated. Please update your pipeline to MsBuild 16 or higher."}]""");
}
catch
{
// Nothing to do here. Two compilations running on two different processes are unlikely to lock each other out on a small file write.
}
}
}
}
});
}
@@ -0,0 +1,24 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.VisualBasic;

[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
public class AnalysisWarningAnalyzer : AnalysisWarningAnalyzerBase { }
@@ -0,0 +1,111 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.IO;
using SonarAnalyzer.CFG.Helpers;
using SonarAnalyzer.Common;
using SonarAnalyzer.Rules;
using CS = SonarAnalyzer.Rules.CSharp;
using VB = SonarAnalyzer.Rules.VisualBasic;

namespace SonarAnalyzer.UnitTest.Rules;

[TestClass]
public class AnalysisWarningAnalyzerTest
{
public TestContext TestContext { get; set; }

[DataTestMethod]
[DataRow(LanguageNames.CSharp, true)]
[DataRow(LanguageNames.CSharp, false)]
[DataRow(LanguageNames.VisualBasic, true)]
[DataRow(LanguageNames.VisualBasic, false)]
public void SupportedRoslyn(string languageName, bool isAnalyzerEnabled)
{
var expectedPath = ExecuteAnalyzer(languageName, isAnalyzerEnabled, RoslynHelper.MinimalSupportedMajorVersion); // Using production value that is lower than our UT Roslyn version
File.Exists(expectedPath).Should().BeFalse("Analysis warning file should not be generated.");
}

[DataTestMethod]
[DataRow(LanguageNames.CSharp)]
[DataRow(LanguageNames.VisualBasic)]
public void OldRoslyn(string languageName)
{
var expectedPath = ExecuteAnalyzer(languageName, true, 1000); // Requiring too high Roslyn version => we're under unsupported scenario
File.Exists(expectedPath).Should().BeTrue();
File.ReadAllText(expectedPath).Should().Be("""[{"text": "Analysis using MsBuild 14 and 15 build tools is deprecated. Please update your pipeline to MsBuild 16 or higher."}]""");

// Lock file and run it for 2nd time
using var lockedFile = new FileStream(expectedPath, FileMode.Open, FileAccess.Write, FileShare.None);
ExecuteAnalyzer(languageName, true, 1000).Should().Be(expectedPath, "path should be reused and analyzer should not fail");
}

[DataTestMethod]
[DataRow(LanguageNames.CSharp)]
[DataRow(LanguageNames.VisualBasic)]
public void FileExceptions_AreIgnored(string languageName)
{
// This will not create the output directory, causing an exception in the File.WriteAllText(...)
var expectedPath = ExecuteAnalyzer(languageName, true, 1000, false); // Requiring too high Roslyn version => we're under unsupported scenario
File.Exists(expectedPath).Should().BeFalse();
}

private string ExecuteAnalyzer(string languageName, bool isAnalyzerEnabled, int minimalSupportedRoslynVersion, bool createDirectory = true)
{
var language = AnalyzerLanguage.FromName(languageName);
var outPath = TestHelper.TestPath(TestContext, @".sonarqube\out");
if (createDirectory)
{
Directory.CreateDirectory(outPath);
}
UtilityAnalyzerBase analyzer = language.LanguageName switch
{
LanguageNames.CSharp => new TestAnalysisWarningAnalyzer_CS(isAnalyzerEnabled, minimalSupportedRoslynVersion, outPath),
LanguageNames.VisualBasic => new TestAnalysisWarningAnalyzer_VB(isAnalyzerEnabled, minimalSupportedRoslynVersion, outPath),
_ => throw new UnexpectedLanguageException(language)
};
new VerifierBuilder().AddAnalyzer(() => analyzer).AddSnippet(string.Empty).VerifyNoIssueReported(); // Nothing to analyze, just make it run
return Path.Combine(outPath, "AnalysisWarnings.MsBuild.json");
}

private sealed class TestAnalysisWarningAnalyzer_CS : CS.AnalysisWarningAnalyzer
{
protected override int MinimalSupportedRoslynVersion { get; }

public TestAnalysisWarningAnalyzer_CS(bool isAnalyzerEnabled, int minimalSupportedRoslynVersion, string outPath)
{
IsAnalyzerEnabled = isAnalyzerEnabled;
MinimalSupportedRoslynVersion = minimalSupportedRoslynVersion;
OutPath = Path.GetFullPath(Path.Combine(outPath, "0", "output-language"));
costin-zaharia-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}
}

private sealed class TestAnalysisWarningAnalyzer_VB : VB.AnalysisWarningAnalyzer
{
protected override int MinimalSupportedRoslynVersion { get; }

public TestAnalysisWarningAnalyzer_VB(bool isAnalyzerEnabled, int minimalSupportedRoslynVersion, string outPath)
{
IsAnalyzerEnabled = isAnalyzerEnabled;
MinimalSupportedRoslynVersion = minimalSupportedRoslynVersion;
OutPath = outPath;
}
}
}
Expand Up @@ -19,7 +19,6 @@
*/

using System.IO;
using System.Runtime.CompilerServices;
using SonarAnalyzer.Common;
using SonarAnalyzer.Protobuf;
using SonarAnalyzer.Rules;
Expand Down
Expand Up @@ -18,11 +18,9 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Runtime.CompilerServices;
using SonarAnalyzer.Common;
using SonarAnalyzer.Protobuf;
using SonarAnalyzer.Rules;
using SonarAnalyzer.UnitTest.Common;
using SonarAnalyzer.UnitTest.Helpers;
using CS = SonarAnalyzer.Rules.CSharp;
using VB = SonarAnalyzer.Rules.VisualBasic;
Expand Down
Expand Up @@ -19,7 +19,6 @@
*/

using System.IO;
using System.Runtime.CompilerServices;
using SonarAnalyzer.Common;
using SonarAnalyzer.Protobuf;
using SonarAnalyzer.Rules;
Expand Down
Expand Up @@ -19,7 +19,6 @@
*/

using System.IO;
using System.Runtime.CompilerServices;
using SonarAnalyzer.Common;
using SonarAnalyzer.Protobuf;
using SonarAnalyzer.Rules;
Expand Down
8 changes: 8 additions & 0 deletions its/projects/Roslyn.1.3.1/OldRoslyn.cs
@@ -0,0 +1,8 @@

costin-zaharia-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
namespace Roslyn
{
public class OldRoslyn
{
// Nothing to see here
}
}
11 changes: 11 additions & 0 deletions its/projects/Roslyn.1.3.1/Roslyn.1.3.1.csproj
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net452</TargetFramework>
</PropertyGroup>

<ItemGroup>
<!-- This will enforce using Roslyn 1.3.1 -->
Copy link
Contributor

Choose a reason for hiding this comment

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

[education] how does this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

<PackageReference Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform" Version="1.0.8" />
antonioaversa marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>
</Project>
25 changes: 25 additions & 0 deletions its/projects/Roslyn.1.3.1/Roslyn.1.3.1.sln
@@ -0,0 +1,25 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 17.3.32811.315
MinimumVisualStudioVersion = 10.0.40219.1
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Roslyn.1.3.1", "Roslyn.1.3.1.csproj", "{151E9036-31F1-481F-BBCE-6C663F21EB31}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Release|Any CPU = Release|Any CPU
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{151E9036-31F1-481F-BBCE-6C663F21EB31}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{151E9036-31F1-481F-BBCE-6C663F21EB31}.Debug|Any CPU.Build.0 = Debug|Any CPU
{151E9036-31F1-481F-BBCE-6C663F21EB31}.Release|Any CPU.ActiveCfg = Release|Any CPU
{151E9036-31F1-481F-BBCE-6C663F21EB31}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {0E15ED11-FF3B-4CCE-8940-EFE27FCC42FD}
EndGlobalSection
EndGlobal
21 changes: 21 additions & 0 deletions its/projects/Roslyn.1.3.1/packages.lock.json
@@ -0,0 +1,21 @@
{
"version": 1,
"dependencies": {
".NETFramework,Version=v4.5.2": {
"Microsoft.CodeDom.Providers.DotNetCompilerPlatform": {
"type": "Direct",
"requested": "[1.0.8, )",
"resolved": "1.0.8",
"contentHash": "Mrl8R7fzrb4/TxsuqVeXxyuAp7j81MJTCHLWxsMAVeuPqjZ7Y/Zg52q0yAPfmrvA2Nx/gkAsWoed/rSToJuLFg==",
"dependencies": {
"Microsoft.Net.Compilers": "1.3.2"
}
},
"Microsoft.Net.Compilers": {
"type": "Transitive",
"resolved": "1.3.2",
"contentHash": "usbCzvNYmSWTdlsJM8FYm1NgGPdM+njnBzerAn3xKGy4/prztHcofq7CMiO7Lb0AK71jXKUzmUuW9Fc0cIzdbQ=="
}
}
}
}
19 changes: 13 additions & 6 deletions its/src/test/java/com/sonar/it/csharp/AnalysisWarningsTest.java
Expand Up @@ -20,19 +20,17 @@
package com.sonar.it.csharp;

import com.sonar.it.shared.TestUtils;
import com.sonar.it.shared.Tests;
import com.sonar.orchestrator.build.BuildResult;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.sonarqube.ws.Ce;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;

import static com.sonar.it.shared.Tests.ORCHESTRATOR;
import static com.sonar.it.csharp.Tests.ORCHESTRATOR;
import static org.assertj.core.api.Assertions.assertThat;

public class AnalysisWarningsTest {
Expand Down Expand Up @@ -60,4 +58,13 @@ public void analysisWarningsImport() throws IOException {
assertThat(task.getStatus()).isEqualTo(Ce.TaskStatus.SUCCESS);
assertThat(task.getWarningsList()).containsExactly("First message", "Second message");
}

@Test
public void analysisWarnings_OldRoslyn() throws IOException {
BuildResult buildResult = Tests.analyzeProject(temp, "Roslyn.1.3.1", null);
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we actually have integration tests for MSB14 and MSB15?

I guess we only have tests with different MSB versions on sonar-security...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

it should be easy to install the build tools (there's no need for a full VS instance) https://community.chocolatey.org/packages/visualstudio2017buildtools


Ce.Task task = TestUtils.getAnalysisWarningsTask(ORCHESTRATOR, buildResult);
assertThat(task.getStatus()).isEqualTo(Ce.TaskStatus.SUCCESS);
assertThat(task.getWarningsList()).containsExactly("Analysis using MsBuild 14 and 15 build tools is deprecated. Please update your pipeline to MsBuild 16 or higher.");
}
}