From ae3904b10bbe5782333a1701fae622144b59802d Mon Sep 17 00:00:00 2001 From: Mostafa Mohammed Date: Wed, 18 Dec 2024 14:46:07 +0100 Subject: [PATCH] SLVS-1710 Fix clang-cl compiler path we add a new fallback way to get the compiler path correctly. This fixes the `clang-cl` support, which is currently broken because the `ClCompilerPath` property we rely on evaluates to `/clang-cl.exe`. The new fallback method searches in the `ExecutablePath` path list for the `ClToolExe` executable, which gives the correct answer in the case of `clang-cl.exe` Co-authored-by: Georgii Borovinskikh Co-authored-by: Michael Jabbour <117195239+michael-jabbour-sonarsource@users.noreply.github.com> --- .../VcxProject/FileConfigProviderTests.cs | 14 +- .../CFamily/VcxProject/FileConfigTests.cs | 138 ++++++++++++++++-- .../CFamily/VcxProject/FileConfig.cs | 109 ++++++++++++-- .../CFamily/VcxProject/FileConfigProvider.cs | 5 +- 4 files changed, 236 insertions(+), 30 deletions(-) diff --git a/src/Integration.Vsix.UnitTests/CFamily/VcxProject/FileConfigProviderTests.cs b/src/Integration.Vsix.UnitTests/CFamily/VcxProject/FileConfigProviderTests.cs index ce3e36f637..38d25e8b8c 100644 --- a/src/Integration.Vsix.UnitTests/CFamily/VcxProject/FileConfigProviderTests.cs +++ b/src/Integration.Vsix.UnitTests/CFamily/VcxProject/FileConfigProviderTests.cs @@ -28,6 +28,8 @@ using SonarLint.VisualStudio.Infrastructure.VS; using SonarLint.VisualStudio.Integration.Vsix.CFamily.VcxProject; using static SonarLint.VisualStudio.Integration.Vsix.CFamily.UnitTests.CFamilyTestUtility; +using System.IO.Abstractions; +using SonarLint.VisualStudio.Core.SystemAbstractions; namespace SonarLint.VisualStudio.Integration.UnitTests.CFamily.VcxProject { @@ -40,11 +42,21 @@ public class FileConfigProviderTests private DTE2 dte; private IVsUIServiceOperation uiServiceOperation; private FileConfigProvider testSubject; + private const string ClFilePath = "C:\\path\\cl.exe"; + + private static IFileSystemService CreateFileSystemWithExistingFile(string fullPath) + { + var fileSystem = Substitute.For(); + fileSystem.File.Exists(fullPath).Returns(true); + return fileSystem; + } + private static IFileSystemService CreateFileSystemWithClCompiler() => CreateFileSystemWithExistingFile(ClFilePath); [TestMethod] public void MefCtor_CheckIsExported() => MefTestHelpers.CheckTypeCanBeImported( MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport()); @@ -60,7 +72,7 @@ public void TestInitialize() dte = Substitute.For(); uiServiceOperation = CreateDefaultUiServiceOperation(dte); - testSubject = new FileConfigProvider(uiServiceOperation, fileInSolutionIndicator, logger, new NoOpThreadHandler()); + testSubject = new FileConfigProvider(uiServiceOperation, fileInSolutionIndicator, CreateFileSystemWithClCompiler(), logger, new NoOpThreadHandler()); } private static IFileInSolutionIndicator CreateDefaultFileInSolutionIndicator() diff --git a/src/Integration.Vsix.UnitTests/CFamily/VcxProject/FileConfigTests.cs b/src/Integration.Vsix.UnitTests/CFamily/VcxProject/FileConfigTests.cs index 9263622f27..acf2963cac 100644 --- a/src/Integration.Vsix.UnitTests/CFamily/VcxProject/FileConfigTests.cs +++ b/src/Integration.Vsix.UnitTests/CFamily/VcxProject/FileConfigTests.cs @@ -18,14 +18,10 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.Collections.Generic; +using System.IO.Abstractions; using EnvDTE; -using FluentAssertions; -using Microsoft.VisualStudio.TestTools.UnitTesting; using Microsoft.VisualStudio.VCProjectEngine; -using Moq; using SonarLint.VisualStudio.Integration.Vsix.CFamily.VcxProject; -using SonarLint.VisualStudio.TestInfrastructure; using static SonarLint.VisualStudio.Integration.Vsix.CFamily.UnitTests.CFamilyTestUtility; namespace SonarLint.VisualStudio.Integration.UnitTests.CFamily.VcxProject @@ -34,6 +30,25 @@ namespace SonarLint.VisualStudio.Integration.UnitTests.CFamily.VcxProject public class FileConfigTests { private readonly TestLogger testLogger = new TestLogger(); + private const string ClFilePath = "C:\\path\\cl.exe"; + private const string ClangClFilePath = "C:\\path\\clang-cl.exe"; + + private static IFileSystem CreateFileSystemWithNoFiles() + { + var fileSystem = Substitute.For(); + fileSystem.File.Exists(Arg.Any()).Returns(false); + return fileSystem; + } + + private static IFileSystem CreateFileSystemWithExistingFile(string fullPath) + { + var fileSystem = Substitute.For(); + fileSystem.File.Exists(fullPath).Returns(true); + return fileSystem; + } + + private static IFileSystem CreateFileSystemWithClCompiler() => CreateFileSystemWithExistingFile(ClFilePath); + private static IFileSystem CreateFileSystemWithClangClCompiler() => CreateFileSystemWithExistingFile(ClangClFilePath); [TestMethod] public void TryGet_NoVCProject_ReturnsNull() @@ -45,7 +60,7 @@ public void TryGet_NoVCProject_ReturnsNull() dteProjectItemMock.Setup(x => x.Object).Returns(Mock.Of()); dteProjectItemMock.Setup(x => x.ContainingProject).Returns(dteProjectMock.Object); - FileConfig.TryGet(testLogger, dteProjectItemMock.Object, "c:\\path") + FileConfig.TryGet(testLogger, dteProjectItemMock.Object, "c:\\path", CreateFileSystemWithClCompiler()) .Should().BeNull(); } @@ -59,7 +74,7 @@ public void TryGet_NoVCFile_ReturnsNull() dteProjectItemMock.Setup(x => x.Object).Returns(null); dteProjectItemMock.Setup(x => x.ContainingProject).Returns(dteProjectMock.Object); - FileConfig.TryGet(testLogger, dteProjectItemMock.Object, "c:\\path") + FileConfig.TryGet(testLogger, dteProjectItemMock.Object, "c:\\path", CreateFileSystemWithClCompiler()) .Should().BeNull(); } @@ -71,7 +86,7 @@ public void TryGet_UnsupportedItemType_ReturnsNull() var projectItemMock = CreateMockProjectItem("c:\\foo\\xxx.vcxproj", projectItemConfig); // Act - var fileConfig = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp"); + var fileConfig = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithClCompiler()); // Assert fileConfig.Should().BeNull(); @@ -86,7 +101,7 @@ public void TryGet_UnsupportedConfigurationType_ReturnsNull() var projectItemMock = CreateMockProjectItem("c:\\foo\\xxx.vcxproj", projectItemConfig); // Act - var fileConfig = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp"); + var fileConfig = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithClCompiler()); // Assert fileConfig.Should().BeNull(); @@ -101,7 +116,7 @@ public void TryGet_UnsupportedCustomBuild_ReturnsNull() var projectItemMock = CreateMockProjectItem("c:\\foo\\xxx.vcxproj", projectItemConfig); // Act - var fileConfig = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp"); + var fileConfig = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithClCompiler()); // Assert fileConfig.Should().BeNull(); @@ -134,7 +149,7 @@ public void TryGet_Full_Cmd() var projectItemMock = CreateMockProjectItem("c:\\foo\\xxx.vcxproj", projectItemConfig); // Act - var request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp"); + var request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithClCompiler()); // Assert request.Should().NotBeNull(); @@ -170,7 +185,7 @@ public void TryGet_HeaderFileOptions_ReturnsValidConfig() var projectItemMock = CreateMockProjectItem("c:\\foo\\xxx.vcxproj", projectItemConfig); // Act - var request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.h"); + var request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.h", CreateFileSystemWithClCompiler()); // Assert request.Should().NotBeNull(); @@ -183,7 +198,7 @@ public void TryGet_HeaderFileOptions_ReturnsValidConfig() projectItemConfig.FileConfigProperties["ForcedIncludeFiles"] = "FHeader.h"; // Act - request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.h"); + request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.h", CreateFileSystemWithClCompiler()); // Assert Assert.AreEqual("\"C:\\path\\cl.exe\" /FI\"FHeader.h\" /Yu\"pch.h\" /EHsc /RTCu /TC \"c:\\dummy\\file.h\"", request.CDCommand); @@ -194,7 +209,7 @@ public void TryGet_HeaderFileOptions_ReturnsValidConfig() projectItemConfig.FileConfigProperties["CompileAs"] = "CompileAsCpp"; // Act - request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.h"); + request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.h", CreateFileSystemWithClCompiler()); // Assert Assert.AreEqual("\"C:\\path\\cl.exe\" /FI\"FHeader.h\" /Yu\"pch.h\" /EHsc /RTCu /TP \"c:\\dummy\\file.h\"", request.CDCommand); @@ -220,7 +235,7 @@ public void TryGet_CompilerName_VS2017() var projectItemMock = CreateMockProjectItem("c:\\foo\\xxx.vcxproj", projectItemConfig); // Act - var request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp"); + var request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithExistingFile("C:\\path\\x86\\cl.exe")); // Assert request.Should().NotBeNull(); @@ -230,12 +245,103 @@ public void TryGet_CompilerName_VS2017() projectItemConfig.PlatformName = "x64"; projectItemMock = CreateMockProjectItem("c:\\foo\\xxx.vcxproj", projectItemConfig); // Act - request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp"); + request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithExistingFile("C:\\path\\x64\\cl.exe")); // Assert request.Should().NotBeNull(); Assert.IsTrue(request.CDCommand.StartsWith("\"C:\\path\\x64\\cl.exe\"")); } + [TestMethod] + public void TryGet_CompilerName_ClangCL() + { + // Arrange + var projectItemConfig = new ProjectItemConfig + { + ProjectConfigProperties = new Dictionary + { + ["ClCompilerPath"] = null, + ["IncludePath"] = "C:\\path\\includeDir1;C:\\path\\includeDir2;C:\\path\\includeDir3;", + ["ExecutablePath"] = "C:\\path\\no-compiler\\;C:\\path", + ["CLToolExe"] = "clang-cl.exe", + ["VC_ExecutablePath_x86"] = "C:\\path\\x86", + ["VC_ExecutablePath_x64"] = "C:\\path\\x64", + } + }; + + var projectItemMock = CreateMockProjectItem("c:\\foo\\xxx.vcxproj", projectItemConfig); + + // Act + var request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithClangClCompiler()); + + // Assert + request.Should().NotBeNull(); + Assert.IsTrue(request.CDCommand.StartsWith("\"C:\\path\\clang-cl.exe\"")); + + // Arrange + projectItemConfig.ProjectConfigProperties["ClCompilerPath"] = "\\clang-cl.exe"; + + // Act + request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithClangClCompiler()); + + // Assert + request.Should().NotBeNull(); + Assert.IsTrue(request.CDCommand.StartsWith("\"C:\\path\\clang-cl.exe\"")); + + // Act + request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithNoFiles()); + + // Assert + request.Should().BeNull(); + testLogger.AssertOutputStringExists("Compiler is not supported."); + + // Arrange + projectItemConfig.ProjectConfigProperties["ClToolExe"] = null; + + // Act + request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithExistingFile("C:\\path\\x86\\cl.exe")); + + // Assert + request.Should().NotBeNull(); + Assert.IsTrue(request.CDCommand.StartsWith("\"C:\\path\\x86\\cl.exe\"")); + + // Arrange + projectItemConfig.ProjectConfigProperties["ClToolExe"] = null; + + // Act + request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithNoFiles()); + + // Assert + request.Should().BeNull(); + testLogger.AssertOutputStringExists("Compiler is not supported."); + } + + [TestMethod] + public void TryGet_CompilerName_CL_No_ClCompilerPath_NoCLToolExe() + { + // Arrange + var projectItemConfig = new ProjectItemConfig + { + ProjectConfigProperties = new Dictionary + { + ["ClCompilerPath"] = null, + ["IncludePath"] = "C:\\path\\includeDir1;C:\\path\\includeDir2;C:\\path\\includeDir3;", + ["ExecutablePath"] = "C:\\path\\no-compiler\\;C:\\path", + ["CLToolExe"] = null, + ["VC_ExecutablePath_x86"] = "C:\\path\\x86", + ["VC_ExecutablePath_x64"] = "C:\\path\\x64", + } + }; + + var projectItemMock = CreateMockProjectItem("c:\\foo\\xxx.vcxproj", projectItemConfig); + + // Act + var request = FileConfig.TryGet(testLogger, projectItemMock.Object, "c:\\dummy\\file.cpp", CreateFileSystemWithClCompiler()); + + // Assert + request.Should().NotBeNull(); + Assert.IsTrue(request.CDCommand.StartsWith("\"C:\\path\\cl.exe\"")); + } + } } diff --git a/src/Integration.Vsix/CFamily/VcxProject/FileConfig.cs b/src/Integration.Vsix/CFamily/VcxProject/FileConfig.cs index 8de56fbc91..a808bf24d7 100644 --- a/src/Integration.Vsix/CFamily/VcxProject/FileConfig.cs +++ b/src/Integration.Vsix/CFamily/VcxProject/FileConfig.cs @@ -20,6 +20,7 @@ using System.Diagnostics.CodeAnalysis; using System.IO; +using System.IO.Abstractions; using EnvDTE; using Microsoft.VisualStudio.VCProjectEngine; using SonarLint.VisualStudio.Core; @@ -29,7 +30,7 @@ namespace SonarLint.VisualStudio.Integration.Vsix.CFamily.VcxProject internal class FileConfig : IFileConfig { [ExcludeFromCodeCoverage] - public static FileConfig TryGet(ILogger logger, ProjectItem dteProjectItem, string absoluteFilePath) + public static FileConfig TryGet(ILogger logger, ProjectItem dteProjectItem, string absoluteFilePath, IFileSystem fileSystem) { if (!(dteProjectItem.ContainingProject.Object is VCProject vcProject) || !(dteProjectItem.Object is VCFile vcFile)) @@ -49,19 +50,11 @@ public static FileConfig TryGet(ILogger logger, ProjectItem dteProjectItem, stri bool isHeaderFile = vcFile.ItemType == "ClInclude"; CmdBuilder cmdBuilder = new CmdBuilder(isHeaderFile); - var compilerPath = vcConfig.GetEvaluatedPropertyValue("ClCompilerPath"); - if (string.IsNullOrEmpty(compilerPath)) + if (!GetCompilerPath(logger, vcConfig, fileSystem, out var compilerPath)) { - // in case ClCompilerPath is not available on VS2017 toolchains - var platform = ((VCPlatform)vcConfig.Platform).Name.Contains("64") ? "x64" : "x86"; - var exeVar = "VC_ExecutablePath_" + platform; - compilerPath = Path.Combine(vcConfig.GetEvaluatedPropertyValue(exeVar), "cl.exe"); - if (string.IsNullOrEmpty(compilerPath)) - { - logger.WriteLine("Compiler is not supported. \"ClCompilerPath\" and \"VC_ExecutablePath\" were not found."); - return null; - } + return null; } + logger.WriteLine(compilerPath); // command: add compiler cmdBuilder.AddCompiler(compilerPath); @@ -82,6 +75,98 @@ public static FileConfig TryGet(ILogger logger, ProjectItem dteProjectItem, stri }; } + private static bool TryGetCompilerPathFromClCompilerPath(ILogger logger, VCConfiguration vcConfig, IFileSystem fileSystem, out string compilerPath) + { + compilerPath = vcConfig.GetEvaluatedPropertyValue("ClCompilerPath"); + if (string.IsNullOrEmpty(compilerPath)) + { + logger.WriteLine("\"ClCompilerPath\" was not found."); + return false; + } + + if (!fileSystem.File.Exists(compilerPath)) + { + logger.WriteLine($"Compiler path (based on \"ClCompilerPath\") \"{compilerPath}\" does not exist."); + return false; + } + + return true; + } + + private static bool TryGetCompilerPathFromExecutablePath(ILogger logger, VCConfiguration vcConfig, IFileSystem fileSystem, out string compilerPath) + { + compilerPath = default; + var executablePath = vcConfig.GetEvaluatedPropertyValue("ExecutablePath"); + if (string.IsNullOrEmpty(executablePath)) + { + logger.WriteLine("\"ExecutablePath\" was not found."); + return false; + } + + var toolExe = vcConfig.GetEvaluatedPropertyValue("CLToolExe"); + if (string.IsNullOrEmpty(toolExe)) + { + // VS2022 sets "CLToolExe" only when clang-cl is chosen as the toolset. + logger.WriteLine("\"CLToolExe\" was not found, falling back to cl.exe."); + toolExe = "cl.exe"; + } + + foreach (var path in executablePath.Split(';')) + { + compilerPath = Path.Combine(path, toolExe); + if (fileSystem.File.Exists(compilerPath)) + { + return true; + } + else + { + logger.WriteLine($"Compiler path (based on \"ExecutablePath\") \"{compilerPath}\" does not exist."); + } + } + + return false; + } + + private static bool TryGetCompilerPathFromVCExecutablePath(ILogger logger, VCConfiguration vcConfig, IFileSystem fileSystem, out string compilerPath) + { + var platform = ((VCPlatform)vcConfig.Platform).Name.Contains("64") ? "x64" : "x86"; + var exeVar = "VC_ExecutablePath_" + platform; + compilerPath = Path.Combine(vcConfig.GetEvaluatedPropertyValue(exeVar), "cl.exe"); + if (fileSystem.File.Exists(compilerPath)) + { + return true; + } + else + { + logger.WriteLine($"Compiler path (based on \"{exeVar}\") \"{compilerPath}\" does not exist."); + return false; + } + } + + private static bool GetCompilerPath(ILogger logger, VCConfiguration vcConfig, IFileSystem fileSystem, out string compilerPath) + { + if (TryGetCompilerPathFromClCompilerPath(logger, vcConfig, fileSystem, out compilerPath)) + { + return true; + } + + // Fallback to ExecutablePath and CLToolExe + if (TryGetCompilerPathFromExecutablePath(logger, vcConfig, fileSystem, out compilerPath)) + { + return true; + } + + // Fallback to VC_ExecutablePath, which is used to be used in VS2017 toolchains + // In case ClCompilerPath isn't available, and ExecutablePath matching fails + if (TryGetCompilerPathFromVCExecutablePath(logger, vcConfig, fileSystem, out compilerPath)) + { + return true; + } + + logger.WriteLine("Compiler is not supported."); + return false; + } + private static IVCRulePropertyStorage GetVcFileSettings(ILogger logger, string absoluteFilePath, VCConfiguration vcConfig, VCFile vcFile) { var projectKind = vcConfig.ConfigurationType; diff --git a/src/Integration.Vsix/CFamily/VcxProject/FileConfigProvider.cs b/src/Integration.Vsix/CFamily/VcxProject/FileConfigProvider.cs index efdafb4959..cc62dca69f 100644 --- a/src/Integration.Vsix/CFamily/VcxProject/FileConfigProvider.cs +++ b/src/Integration.Vsix/CFamily/VcxProject/FileConfigProvider.cs @@ -27,6 +27,8 @@ using SonarLint.VisualStudio.CFamily.Analysis; using SonarLint.VisualStudio.Infrastructure.VS; using SonarLint.VisualStudio.Integration.Helpers; +using System.IO.Abstractions; +using SonarLint.VisualStudio.Core.SystemAbstractions; namespace SonarLint.VisualStudio.Integration.Vsix.CFamily.VcxProject { @@ -41,6 +43,7 @@ internal interface IFileConfigProvider internal class FileConfigProvider( IVsUIServiceOperation uiServiceOperation, IFileInSolutionIndicator fileInSolutionIndicator, + IFileSystemService fileSystem, ILogger logger, IThreadHandling threadHandling) : IFileConfigProvider { @@ -75,7 +78,7 @@ private FileConfig GetInternal(string analyzedFilePath, DTE2 dte, ILogger analys // Note: if the C++ tools are not installed then it's likely an exception will be thrown when // the framework tries to JIT-compile the TryGet method (since it won't be able to find the MS.VS.VCProjectEngine // types). - return FileConfig.TryGet(analysisLogger, projectItem, analyzedFilePath); + return FileConfig.TryGet(analysisLogger, projectItem, analyzedFilePath, fileSystem); } catch (Exception ex) when (!Microsoft.VisualStudio.ErrorHandler.IsCriticalException(ex)) {