SLVS-1697 Integrate Reproducer command with SLCore analysis (#5905)

[SLVS-1697](https://sonarsource.atlassian.net/browse/SLVS-1697) [SLVS-1697]: https://sonarsource.atlassian.net/browse/SLVS-1697?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
SonarSource · Dec 23, 2024 · 5874542 · 5874542
1 parent ddbf1ba
commit 5874542
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 111 deletions.
diff --git a/...Family/Analysis/CFamilyAnalyzerOptions.cs → src/Core/Analysis/CFamilyAnalyzerOptions.cs b/...Family/Analysis/CFamilyAnalyzerOptions.cs → src/Core/Analysis/CFamilyAnalyzerOptions.cs
@@ -22,10 +22,14 @@
 
 namespace SonarLint.VisualStudio.CFamily.Analysis
 {
-    public class CFamilyAnalyzerOptions : IAnalyzerOptions
+    public interface ICFamilyAnalyzerOptions : IAnalyzerOptions
+    {
+        bool CreateReproducer { get; set; }
+    }
+
+    public class CFamilyAnalyzerOptions : ICFamilyAnalyzerOptions
     {
         public bool CreateReproducer { get; set; }
-        public bool CreatePreCompiledHeaders { get; set; }
         public bool IsOnOpen { get; set; }
     }
 }
diff --git a/src/Integration.Vsix.UnitTests/CFamily/CFamilyReproducerCommandTests.cs b/src/Integration.Vsix.UnitTests/CFamily/CFamilyReproducerCommandTests.cs
@@ -167,8 +167,9 @@ public void Execute_ReanalysisTriggered()
 
             // Assert
             logger.AssertOutputStringExists(CFamilyStrings.ReproCmd_ExecutingReproducer);
-            actualOptions.Should().BeOfType<CFamilyAnalyzerOptions>();
-            ((CFamilyAnalyzerOptions)actualOptions).CreateReproducer.Should().BeTrue();
+            var cFamilyAnalyzerOptions = actualOptions.Should().BeAssignableTo<ICFamilyAnalyzerOptions>().Subject;
+            cFamilyAnalyzerOptions.CreateReproducer.Should().BeTrue();
+            cFamilyAnalyzerOptions.IsOnOpen.Should().BeFalse();
             actualFilePaths.Should().BeEquivalentTo(ValidTextDocument.FilePath);
         }
 
@@ -213,12 +214,12 @@ public void Execute_CriticalErrorNotSuppressed()
             docLocatorMock.Setup(x => x.FindActiveDocument()).Throws(new StackOverflowException("exception xxx"));
             Action act = () => testSubject.Invoke();
 
-            // Act 
+            // Act
             act.Should().ThrowExactly<StackOverflowException>().And.Message.Should().Be("exception xxx");
             logger.AssertPartialOutputStringDoesNotExist("exception xxx");
         }
 
-        private static MenuCommand CreateCFamilyReproducerCommand(IActiveDocumentLocator documentLocator, 
+        private static MenuCommand CreateCFamilyReproducerCommand(IActiveDocumentLocator documentLocator,
             ISonarLanguageRecognizer languageRecognizer, IAnalysisRequester analysisRequester, ILogger logger)
         {
             var dummyMenuService = new DummyMenuCommandService();

diff --git a/src/Integration.Vsix.UnitTests/CFamily/VcxProject/FileConfigProviderTests.cs b/src/Integration.Vsix.UnitTests/CFamily/VcxProject/FileConfigProviderTests.cs
@@ -24,11 +24,9 @@
 using NSubstitute.ExceptionExtensions;
 using NSubstitute.ReturnsExtensions;
 using SonarLint.VisualStudio.Core;
-using SonarLint.VisualStudio.CFamily.Analysis;
 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
@@ -94,29 +92,18 @@ public void Get_FailsToRetrieveProjectItem_NonCriticalException_ExceptionCaughtA
         {
             dte.Solution.ThrowsForAnyArgs<NotImplementedException>();
 
-            var result = testSubject.Get(SourceFilePath, new CFamilyAnalyzerOptions());
+            var result = testSubject.Get(SourceFilePath);
 
             result.Should().BeNull();
             logger.AssertPartialOutputStringExists(nameof(NotImplementedException), SourceFilePath);
         }
 
-        [TestMethod]
-        public void Get_FailsToRetrieveProjectItem_NonCriticalException_Pch_ExceptionCaughtNotLoggedAndNullReturned()
-        {
-            dte.Solution.ThrowsForAnyArgs<NotImplementedException>();
-
-            var result = testSubject.Get(SourceFilePath, new CFamilyAnalyzerOptions{CreatePreCompiledHeaders = true});
-
-            result.Should().BeNull();
-            logger.AssertNoOutputMessages();
-        }
-
         [TestMethod]
         public void Get_FailsToRetrieveProjectItem_CriticalException_ExceptionThrown()
         {
             dte.Solution.ThrowsForAnyArgs<DivideByZeroException>();
 
-            Action act = () => testSubject.Get(SourceFilePath, new CFamilyAnalyzerOptions());
+            Action act = () => testSubject.Get(SourceFilePath);
 
             act.Should().Throw<DivideByZeroException>();
         }
@@ -126,7 +113,7 @@ public void Get_NoProjectItem_ReturnsNull()
         {
             dte.Solution.FindProjectItem(SourceFilePath).ReturnsNull();
 
-            testSubject.Get(SourceFilePath, null)
+            testSubject.Get(SourceFilePath)
                 .Should().BeNull();
 
             Received.InOrder(() =>
@@ -143,7 +130,7 @@ public void Get_ProjectItemNotInSolution_ReturnsNull()
             dte.Solution.FindProjectItem(SourceFilePath).Returns(mockProjectItem.Object);
             fileInSolutionIndicator.IsFileInSolution(mockProjectItem.Object).Returns(false);
 
-            testSubject.Get(SourceFilePath, null)
+            testSubject.Get(SourceFilePath)
                 .Should().BeNull();
 
             Received.InOrder(() =>
@@ -160,7 +147,7 @@ public void Get_SuccessfulConfig_ConfigReturned()
             var mockProjectItem = CreateMockProjectItem(SourceFilePath);
             dte.Solution.FindProjectItem(SourceFilePath).Returns(mockProjectItem.Object);
 
-            testSubject.Get(SourceFilePath, null)
+            testSubject.Get(SourceFilePath)
                 .Should().NotBeNull();
         }
     }

diff --git a/src/Integration.Vsix.UnitTests/CFamily/VcxProject/VCXCompilationDatabaseProviderTests.cs b/src/Integration.Vsix.UnitTests/CFamily/VcxProject/VCXCompilationDatabaseProviderTests.cs
@@ -66,7 +66,7 @@ public void MefCtor_CheckIsExported()
     [TestMethod]
     public void CreateOrNull_NoFileConfig_ReturnsNull()
     {
-        fileConfigProvider.Get(SourceFilePath, default).ReturnsNull();
+        fileConfigProvider.Get(SourceFilePath).ReturnsNull();
         var testSubject = new VCXCompilationDatabaseProvider(
             storage,
             envVarProvider,
@@ -82,7 +82,7 @@ public void CreateOrNull_NoFileConfig_ReturnsNull()
     public void CreateOrNull_FileConfig_CantStore_ReturnsNull()
     {
         var fileConfig = GetFileConfig();
-        fileConfigProvider.Get(SourceFilePath, default).Returns(fileConfig);
+        fileConfigProvider.Get(SourceFilePath).Returns(fileConfig);
         storage.CreateDatabase(default, default, default, default).ReturnsNullForAnyArgs();
         var testSubject = new VCXCompilationDatabaseProvider(
             storage,
@@ -99,7 +99,7 @@ public void CreateOrNull_FileConfig_CantStore_ReturnsNull()
     public void CreateOrNull_FileConfig_StoresAndReturnsHandle()
     {
         var fileConfig = GetFileConfig();
-        fileConfigProvider.Get(SourceFilePath, default).Returns(fileConfig);
+        fileConfigProvider.Get(SourceFilePath).Returns(fileConfig);
         var compilationDatabaseHandle = Substitute.For<ICompilationDatabaseHandle>();
         storage.CreateDatabase(CDFile, CDDirectory, CDCommand, Arg.Any<IEnumerable<string>>()).Returns(compilationDatabaseHandle);
         var testSubject = new VCXCompilationDatabaseProvider(
@@ -115,7 +115,7 @@ public void CreateOrNull_FileConfig_StoresAndReturnsHandle()
     public void CreateOrNull_NoEnvIncludeInFileConfig_UsesStatic()
     {
         var fileConfig = GetFileConfig(null);
-        fileConfigProvider.Get(SourceFilePath, default).Returns(fileConfig);
+        fileConfigProvider.Get(SourceFilePath).Returns(fileConfig);
         envVarProvider.GetAll().Returns([("Var1", "Value1"), ("INCLUDE", "static"), ("Var2", "Value2")]);
         var testSubject = new VCXCompilationDatabaseProvider(
             storage,
@@ -132,7 +132,7 @@ public void CreateOrNull_NoEnvIncludeInFileConfig_UsesStatic()
     public void CreateOrNull_FileConfigHasEnvInclude_UsesDynamic()
     {
         var fileConfig = GetFileConfig(EnvInclude);
-        fileConfigProvider.Get(SourceFilePath, default).Returns(fileConfig);
+        fileConfigProvider.Get(SourceFilePath).Returns(fileConfig);
         envVarProvider.GetAll().Returns([("Var1", "Value1"), ("INCLUDE", "static"), ("Var2", "Value2")]);
         var testSubject = new VCXCompilationDatabaseProvider(
             storage,
@@ -150,7 +150,7 @@ public void CreateOrNull_FileConfigHasEnvInclude_UsesDynamic()
     public void CreateOrNull_NoStaticInclude_UsesDynamic()
     {
         var fileConfig = GetFileConfig(EnvInclude);
-        fileConfigProvider.Get(SourceFilePath, default).Returns(fileConfig);
+        fileConfigProvider.Get(SourceFilePath).Returns(fileConfig);
         envVarProvider.GetAll().Returns([("Var1", "Value1"), ("Var2", "Value2")]);
         var testSubject = new VCXCompilationDatabaseProvider(
             storage,
@@ -168,7 +168,7 @@ public void CreateOrNull_NoStaticInclude_UsesDynamic()
     public void CreateOrNull_StaticEnvVarsAreCached()
     {
         var fileConfig = GetFileConfig();
-        fileConfigProvider.Get(SourceFilePath, default).Returns(fileConfig);
+        fileConfigProvider.Get(SourceFilePath).Returns(fileConfig);
         envVarProvider.GetAll().Returns([("Var1", "Value1"), ("Var2", "Value2")]);
         var testSubject = new VCXCompilationDatabaseProvider(
             storage,
@@ -187,7 +187,7 @@ public void CreateOrNull_StaticEnvVarsAreCached()
     public void CreateOrNull_EnvVarsContainHeaderPropertyForHeaderFiles()
     {
         var fileConfig = GetFileConfig(EnvInclude, true);
-        fileConfigProvider.Get(SourceFilePath, default).Returns(fileConfig);
+        fileConfigProvider.Get(SourceFilePath).Returns(fileConfig);
         envVarProvider.GetAll().Returns([("Var1", "Value1"), ("Var2", "Value2")]);
         var testSubject = new VCXCompilationDatabaseProvider(
             storage,

diff --git a/src/Integration.Vsix/CFamily/CFamilyReproducerCommand.cs b/src/Integration.Vsix/CFamily/CFamilyReproducerCommand.cs
@@ -164,6 +164,7 @@ private void TriggerReproducer()
             {
                 var options = new CFamilyAnalyzerOptions
                 {
+                    IsOnOpen = false,
                     CreateReproducer = true
                 };
 

diff --git a/src/Integration.Vsix/CFamily/VcxProject/FileConfigProvider.cs b/src/Integration.Vsix/CFamily/VcxProject/FileConfigProvider.cs
@@ -18,105 +18,64 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-using System;
 using System.ComponentModel.Composition;
-using EnvDTE;
 using EnvDTE80;
 using Microsoft.VisualStudio.Shell.Interop;
 using SonarLint.VisualStudio.Core;
-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
+namespace SonarLint.VisualStudio.Integration.Vsix.CFamily.VcxProject;
+
+internal interface IFileConfigProvider
+{
+    IFileConfig Get(string analyzedFilePath);
+}
+
+[Export(typeof(IFileConfigProvider))]
+[PartCreationPolicy(CreationPolicy.Shared)]
+[method: ImportingConstructor]
+internal class FileConfigProvider(
+    IVsUIServiceOperation uiServiceOperation,
+    IFileInSolutionIndicator fileInSolutionIndicator,
+    IFileSystemService fileSystem,
+    ILogger logger,
+    IThreadHandling threadHandling) : IFileConfigProvider
 {
-    internal interface IFileConfigProvider
+
+    public IFileConfig Get(string analyzedFilePath)
     {
-        IFileConfig Get(string analyzedFilePath, CFamilyAnalyzerOptions analyzerOptions);
+        return uiServiceOperation.Execute<SDTE, DTE2, IFileConfig>(dte =>
+            GetInternal(analyzedFilePath, dte));
     }
 
-    [Export(typeof(IFileConfigProvider))]
-    [PartCreationPolicy(CreationPolicy.Shared)]
-    [method: ImportingConstructor]
-    internal class FileConfigProvider(
-        IVsUIServiceOperation uiServiceOperation,
-        IFileInSolutionIndicator fileInSolutionIndicator,
-        IFileSystemService fileSystem,
-        ILogger logger,
-        IThreadHandling threadHandling) : IFileConfigProvider
+    private FileConfig GetInternal(string analyzedFilePath, DTE2 dte)
     {
-        private static readonly NoOpLogger noOpLogger = new NoOpLogger();
-
-        public IFileConfig Get(string analyzedFilePath, CFamilyAnalyzerOptions analyzerOptions)
-        {
-            var analysisLogger = GetAnalysisLogger(analyzerOptions);
+        threadHandling.ThrowIfNotOnUIThread();
 
-            return uiServiceOperation.Execute<SDTE, DTE2, IFileConfig>(dte =>
-                GetInternal(analyzedFilePath, dte, analysisLogger));
-        }
-
-        private FileConfig GetInternal(string analyzedFilePath, DTE2 dte, ILogger analysisLogger)
+        try
         {
-            threadHandling.ThrowIfNotOnUIThread();
+            var projectItem = dte.Solution.FindProjectItem(analyzedFilePath);
 
-            try
+            if (projectItem == null)
             {
-                var projectItem = dte.Solution.FindProjectItem(analyzedFilePath);
-
-                if (projectItem == null)
-                {
-                    return null;
-                }
-
-                if (!fileInSolutionIndicator.IsFileInSolution(projectItem))
-                {
-                    analysisLogger.LogVerbose($"[VCX:FileConfigProvider] The file is not part of a VCX project. File: {analyzedFilePath}");
-                    return null;
-                }
-                // 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, fileSystem);
-            }
-            catch (Exception ex) when (!Microsoft.VisualStudio.ErrorHandler.IsCriticalException(ex))
-            {
-                analysisLogger.WriteLine(CFamilyStrings.ERROR_CreatingConfig, analyzedFilePath, ex);
                 return null;
             }
-        }
 
-        private ILogger GetAnalysisLogger(CFamilyAnalyzerOptions analyzerOptions)
-        {
-            if (analyzerOptions is CFamilyAnalyzerOptions cFamilyAnalyzerOptions &&
-                cFamilyAnalyzerOptions.CreatePreCompiledHeaders)
+            if (!fileInSolutionIndicator.IsFileInSolution(projectItem))
             {
-                // In case the requeset is coming from PCH generation, we don't log failures.
-                // This is to avoid redundant messages while navigating unsupported files.
-                return noOpLogger;
+                logger.LogVerbose($"[VCX:FileConfigProvider] The file is not part of a VCX project. File: {analyzedFilePath}");
+                return null;
             }
-
-            return logger;
+            // 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(logger, projectItem, analyzedFilePath, fileSystem);
         }
-
-
-        private class NoOpLogger : ILogger
+        catch (Exception ex) when (!Microsoft.VisualStudio.ErrorHandler.IsCriticalException(ex))
         {
-            public void WriteLine(string message)
-            {
-                // no-op
-            }
-
-            public void WriteLine(string messageFormat, params object[] args)
-            {
-                // no-op
-            }
-
-            public void LogVerbose(string message, params object[] args)
-            {
-                // no-op
-            }
+            logger.WriteLine(CFamilyStrings.ERROR_CreatingConfig, analyzedFilePath, ex);
+            return null;
         }
     }
 }
diff --git a/src/Integration.Vsix/CFamily/VcxProject/VCXCompilationDatabaseProvider.cs b/src/Integration.Vsix/CFamily/VcxProject/VCXCompilationDatabaseProvider.cs
@@ -51,7 +51,7 @@ public VCXCompilationDatabaseProvider(
     }
 
     public ICompilationDatabaseHandle CreateOrNull(string filePath) =>
-        fileConfigProvider.Get(filePath, null) is { } fileConfig
+        fileConfigProvider.Get(filePath) is { } fileConfig
             ? storage.CreateDatabase(fileConfig.CDFile, fileConfig.CDDirectory, fileConfig.CDCommand, GetEnvironmentEntries(fileConfig).Select(x => x.FormattedEntry))
             : null;
-Original file line number
+Diff line change
@@ Expand Up / @@ -164,6 +164,7 @@ private void TriggerReproducer() @@
                 {
                     var options = new CFamilyAnalyzerOptions
                     {
+                        IsOnOpen = false,
                         CreateReproducer = true
                     };
@@ Expand Down @@