Open in IDE: various bug fixes

src/Core/IFolderWorkspaceService.cs

@@ -39,6 +39,6 @@ public interface IFolderWorkspaceService
         /// <summary>
         /// Returns the list of files in a folder view
         /// </summary>
-        IEnumerable<string> ListFiles();
+        IReadOnlyCollection<string> ListFiles();
     }
 }

src/Core/ISolutionWorkspaceService.cs

@@ -33,6 +33,6 @@ public interface ISolutionWorkspaceService
         /// List Files in a solution
         /// </summary>
         /// <returns>Returns empty if it's not in solution Mode</returns>
-        IEnumerable<string> ListFiles();
+        IReadOnlyCollection<string> ListFiles();
     }
 }

src/Infrastructure.VS/FolderWorkspaceService.cs

@@ -63,13 +63,13 @@ public string FindRootDirectory()
             return solutionInfoProvider.GetSolutionDirectory();
         }
 
-        public IEnumerable<string> ListFiles()
+        public IReadOnlyCollection<string> ListFiles()
         {
             var root = FindRootDirectory();
 
-            return root is not null ?
-                fileSystem.Directory.EnumerateFiles(root, "*", SearchOption.AllDirectories).Where(x => !x.Contains("\\node_modules\\")) :
-                Array.Empty<string>();
+            return root is not null 
+                ? fileSystem.Directory.EnumerateFiles(root, "*", SearchOption.AllDirectories).Where(x => !x.Contains("\\node_modules\\")).ToList() 
+                : [];
         }
     }
 }

src/Integration.UnitTests/SLCore/SLCoreConstantsProviderTests.cs

@@ -93,7 +93,7 @@ public void ClientConstants_VsOperationCalledOnlyOnce()
         [TestMethod]
         public void FeatureFlags_ShouldBeExpected()
         {
-            var expectedFeatureFlags = new FeatureFlagsDto(true, true, false, true, false, false, true, false);
+            var expectedFeatureFlags = new FeatureFlagsDto(true, true, true, true, false, false, true, false);
             var result = testSubject.FeatureFlags;
 
             result.Should().BeEquivalentTo(expectedFeatureFlags);

src/Integration/LocalServices/SolutionWorkspaceService.cs

@@ -55,7 +55,7 @@ internal SolutionWorkspaceService(ISolutionInfoProvider solutionInfoProvider, IS
         public bool IsSolutionWorkSpace() => !solutionInfoProvider.IsFolderWorkspace();
 
         [ExcludeFromCodeCoverage]
-        public IEnumerable<string> ListFiles()
+        public IReadOnlyCollection<string> ListFiles()
         {
             if (!IsSolutionWorkSpace()) { return Array.Empty<string>(); }
 
@@ -65,17 +65,18 @@ public IEnumerable<string> ListFiles()
         }
 
         [ExcludeFromCodeCoverage]
-        private IEnumerable<string> GetAllFilesInSolution(IVsSolution solution)
+        private IReadOnlyCollection<string> GetAllFilesInSolution(IVsSolution solution)
         {
-            IEnumerable<string> result = null;
+            IReadOnlyCollection<string> result = null;
             threadHandling.RunOnUIThread(() => result = GetLoadedProjects(solution)
                 .SelectMany(AllItemsInProject)
                 .Where(x => x != null)
                 .Where(x => x.Contains("\\"))
                 .Where(x => !x.EndsWith("\\"))
                 .Where(x => !x.Contains("\\.nuget\\"))
-                .Where(x => !x.Contains("\\node_modules\\"))); // move filtering closer to path extraction to avoid processing unnecessary items)
-
+                .Where(x => !x.Contains("\\node_modules\\"))
+                .ToHashSet(StringComparer.InvariantCultureIgnoreCase)); // move filtering closer to path extraction to avoid processing unnecessary items)
+
             return result;
         }
 

src/Integration/SLCore/SLCoreConstantsProvider.cs

@@ -52,7 +52,7 @@ public SLCoreConstantsProvider(IVsUIServiceOperation vsUiServiceOperation)
 
         public ClientConstantsDto ClientConstants => new ClientConstantsDto(ideName.Value, $"SonarLint Visual Studio/{VersionHelper.SonarLintVersion}", Process.GetCurrentProcess().Id);
 
-        public FeatureFlagsDto FeatureFlags => new FeatureFlagsDto(true, true, false, true, false, false, true, false);
+        public FeatureFlagsDto FeatureFlags => new FeatureFlagsDto(true, true, true, true, false, false, true, false);
 
         //We do not support telemetry now
         public TelemetryClientConstantAttributesDto TelemetryConstants => new TelemetryClientConstantAttributesDto("SLVS_SHOULD_NOT_SEND_TELEMETRY", default, default, default, default);

src/IssueViz.Security/OpenInIdeHotspots/HotspotDetailsDtoToHotspotConverter.cs

@@ -50,7 +50,7 @@ public IAnalysisIssueBase Convert(HotspotDetailsDto hotspotDetailsDto, string ro
         return new Hotspot(hotspotDetailsDto.key,
             hotspotDetailsDto.ideFilePath,
             new AnalysisIssueLocation(hotspotDetailsDto.message,
-                Path.Combine(rootPath, hotspotDetailsDto.ideFilePath),
+                Path.Combine(rootPath, hotspotDetailsDto.ideFilePath), //todo ideFilePath we get from the server is wrong https://sonarsource.atlassian.net/browse/SLCORE-783
                 GetTextRange(hotspotDetailsDto)),
             GetHotspotRule(hotspotDetailsDto),
             null);

src/IssueViz.UnitTests/OpenInIDE/IssueDetailDtoToAnalysisIssueConverterTests.cs

@@ -175,7 +175,9 @@ public void Convert_RuleDescriptionContextSetToDefault()
     [TestMethod]
     public void Convert_FlowsPreservedWithPathTranslation()
     {
-        var testSubject = new IssueDetailDtoToAnalysisIssueConverter(Substitute.For<IChecksumCalculator>());
+        var checksumCalculator = Substitute.For<IChecksumCalculator>();
+        checksumCalculator.Calculate(Arg.Any<string>()).Returns(info => "hash of " + info.Arg<string>());
+        var testSubject = new IssueDetailDtoToAnalysisIssueConverter(checksumCalculator);
 
         var issue = testSubject.Convert(new IssueDetailDto("key",
                 "rule",
@@ -190,12 +192,12 @@ public void Convert_FlowsPreservedWithPathTranslation()
                 {
                     new FlowDto(new List<LocationDto>
                     {
-                        new LocationDto(new TextRangeWithHashDto(1, 11, 111, 1111, "11111"), "message1", "flow\\path\\1"),
-                        new LocationDto(new TextRangeWithHashDto(2, 22, 222, 2222, "22222"), "message2", "flow\\path\\2"),
+                        new LocationDto(new TextRangeDto(1, 11, 111, 1111), "message1", "flow\\path\\1",  "11111"),
+                        new LocationDto(new TextRangeDto(2, 22, 222, 2222), "message2", "flow\\path\\2", "22222"),
                     }),
                     new FlowDto(new List<LocationDto>
                     {
-                        new LocationDto(new TextRangeWithHashDto(3, 33, 333, 3333, "33333"), "message3", "flow\\path\\3")
+                        new LocationDto(new TextRangeDto(3, 33, 333, 3333), "message3", "flow\\path\\3", "33333")
                     })
                 },
                 new TextRangeDto(1, 2, 3, 4)),
@@ -205,12 +207,12 @@ public void Convert_FlowsPreservedWithPathTranslation()
         {
             new AnalysisIssueFlow(new List<IAnalysisIssueLocation>
             {
-                new AnalysisIssueLocation("message1", "some\\path\\flow\\path\\1", new TextRange(1, 111, 11, 1111, "11111")),
-                new AnalysisIssueLocation("message2", "some\\path\\flow\\path\\2", new TextRange(2, 222, 22, 2222, "22222"))
+                new AnalysisIssueLocation("message1", "some\\path\\flow\\path\\1", new TextRange(1, 111, 11, 1111, "hash of 11111")),
+                new AnalysisIssueLocation("message2", "some\\path\\flow\\path\\2", new TextRange(2, 222, 22, 2222, "hash of 22222"))
             }),
             new AnalysisIssueFlow(new List<IAnalysisIssueLocation>
             {
-                new AnalysisIssueLocation("message3", "some\\path\\flow\\path\\3", new TextRange(3, 333, 33, 3333, "33333"))
+                new AnalysisIssueLocation("message3", "some\\path\\flow\\path\\3", new TextRange(3, 333, 33, 3333, "hash of 33333"))
             })
         });
     }

src/IssueViz.UnitTests/OpenInIDE/OpenInIdeHandlerImplementationTests.cs

@@ -182,7 +182,7 @@ public void ShowIssue_UnableToOpenFile_ShowsMessageBox()
         analysisIssueVisualization.CurrentFilePath.Returns(issueFilePath);
         var dtoConverter = Substitute.For<IOpenInIdeIssueToAnalysisIssueConverter<IOpenInIdeIssue>>();
         var testSubject = CreateTestSubject(out var logger, out _, out var configScopeValidator, out var converter,
-            out var messageBox, out var toolWindowService, out _, out var navigator, out var issueSelectionService);
+            out var messageBox, out _, out _, out var navigator, out var issueSelectionService);
         var visualizationProcessor = SetUpProcessor(analysisIssueVisualization);
         SetUpValidConfigScope(configScopeValidator, configurationScope, configurationScopeRoot);
         SetUpIssueConversion(converter, issue, configurationScopeRoot, dtoConverter, analysisIssueVisualization);
@@ -193,7 +193,6 @@ public void ShowIssue_UnableToOpenFile_ShowsMessageBox()
         navigator.Received().TryNavigatePartial(analysisIssueVisualization);
         messageBox.Received().UnableToOpenFile(issueFilePath);
         logger.AssertPartialOutputStringExists("[Open in IDE] Could not find the location at");
-        toolWindowService.DidNotReceiveWithAnyArgs().Show(default);
         visualizationProcessor.Received().HandleConvertedIssue(analysisIssueVisualization);
         issueSelectionService.Received().SelectedIssue = analysisIssueVisualization;
     }

src/IssueViz.UnitTests/OpenInIDE/OpenInIdeMessageBoxTests.cs

@@ -18,6 +18,7 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
+using System;
 using System.Windows;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using NSubstitute;
@@ -34,7 +35,8 @@ public class OpenInIdeMessageBoxTests
     public void MefCtor_CheckIsExported()
     {
         MefTestHelpers.CheckTypeCanBeImported<OpenInIdeMessageBox, IOpenInIdeMessageBox>(
-            MefTestHelpers.CreateExport<IMessageBox>());
+            MefTestHelpers.CreateExport<IMessageBox>(),
+            MefTestHelpers.CreateExport<IThreadHandling>());
     }
 
     [TestMethod]
@@ -48,32 +50,48 @@ public void UnableToLocateIssue_ShowsMessageBox()
     {
         var messageBox = Substitute.For<IMessageBox>();
         var filePath = "file/path/123";
-        new OpenInIdeMessageBox(messageBox).UnableToLocateIssue(filePath);
+        var threadHandling = SetUpThreadHandling();
+        new OpenInIdeMessageBox(messageBox, threadHandling).UnableToLocateIssue(filePath);
 
         VerifyMessageBox(messageBox, $"Could not locate issue. Ensure the file (file/path/123) has not been modified");
+        VerifyRanOnUIThread(threadHandling);
     }
 
     [TestMethod]
     public void UnableToOpenFile_ShowsMessageBox()
     {
         var messageBox = Substitute.For<IMessageBox>();
         var filePath = "file/path/123";
-        new OpenInIdeMessageBox(messageBox).UnableToOpenFile(filePath);
+        var threadHandling = SetUpThreadHandling();
+        new OpenInIdeMessageBox(messageBox, threadHandling).UnableToOpenFile(filePath);
 
         VerifyMessageBox(messageBox, $"Could not open File: file/path/123");
+        VerifyRanOnUIThread(threadHandling);
     }
 
     [TestMethod]
     public void InvalidConfiguration_ShowsMessageBox()
     {
         var messageBox = Substitute.For<IMessageBox>();
         var reason = "reason 123";
-        new OpenInIdeMessageBox(messageBox).InvalidRequest(reason);
+        var threadHandling = SetUpThreadHandling();
+        new OpenInIdeMessageBox(messageBox, threadHandling).InvalidRequest(reason);
 
-        VerifyMessageBox(messageBox, string.Format(OpenInIdeResources.MessageBox_InvalidConfiguration, reason));
+
         VerifyMessageBox(messageBox, $"Unable to process Open in IDE request. Reason: reason 123");
+        VerifyRanOnUIThread(threadHandling);
     }
 
+    private IThreadHandling SetUpThreadHandling()
+    {
+        var threadHandling = Substitute.For<IThreadHandling>();
+        threadHandling.When(x => x.RunOnUIThread(Arg.Any<Action>())).Do(call => call.Arg<Action>()());
+        return threadHandling;
+    }
+
+    private void VerifyRanOnUIThread(IThreadHandling threadHandling) =>
+        threadHandling.ReceivedWithAnyArgs().RunOnUIThread(default);
+
     private void VerifyMessageBox(IMessageBox messageBox, string message) =>
         messageBox.Received(1).Show(message, OpenInIdeResources.MessageBox_Caption, MessageBoxButton.OK,
             MessageBoxImage.Warning);

src/IssueViz/OpenInIde/IssueDetailDtoToAnalysisIssueConverter.cs

@@ -50,7 +50,7 @@ public IAnalysisIssueBase Convert(IssueDetailDto issueDetailDto, string rootPath
         return new ServerIssue(
             issueDetailDto.ruleKey,
             new AnalysisIssueLocation(issueDetailDto.message,
-                Path.Combine(rootPath, issueDetailDto.ideFilePath),
+                Path.Combine(rootPath, issueDetailDto.ideFilePath),  //todo ideFilePath we get from the server is wrong https://sonarsource.atlassian.net/browse/SLCORE-783
                 new TextRange(issueDetailDto.textRange.startLine,
                     issueDetailDto.textRange.endLine,
                     issueDetailDto.textRange.startLineOffset,
@@ -62,12 +62,12 @@ public IAnalysisIssueBase Convert(IssueDetailDto issueDetailDto, string rootPath
                     new AnalysisIssueFlow(flowDto.locations
                         .Select(locationDto =>
                             new AnalysisIssueLocation(locationDto.message,
-                                Path.Combine(rootPath, locationDto.filePath),
+                                Path.Combine(rootPath, locationDto.ideFilePath),
                                 new TextRange(locationDto.textRange.startLine,
                                     locationDto.textRange.endLine,
                                     locationDto.textRange.startLineOffset,
                                     locationDto.textRange.endLineOffset,
-                                    locationDto.textRange.hash)))
+                                    checksumCalculator.Calculate(locationDto.codeSnippet))))
                         .ToList()))
                 .ToList());
     }

src/IssueViz/OpenInIde/OpenInIdeHandlerImplementation.cs

@@ -92,12 +92,13 @@ public void ShowIssue<T>(T issueDetails,
     private void ShowIssueInternal<T>(T issueDetails, string issueConfigurationScope, IOpenInIdeIssueToAnalysisIssueConverter<T> converter, Guid toolWindowId, IOpenInIdeVisualizationProcessor visualizationProcessor) where T : IOpenInIdeIssue
     {
         logger.WriteLine(OpenInIdeResources.ProcessingRequest, issueConfigurationScope,
-            issueDetails.Key, issueDetails.Type);
+            issueDetails?.Key, issueDetails?.Type);
 
         ideWindowService.BringToFront();
 
-        if (!ValidateConfiguration(issueConfigurationScope, out var configurationScopeRoot, out var failureReason)
-            || !ValidateOpenInIdeIssue(issueDetails, converter, configurationScopeRoot, out var visualization, out failureReason))
+        if (!ValidateIssueNotNull(issueDetails, out var failureReason)
+            || !ValidateConfiguration(issueConfigurationScope, out var configurationScopeRoot, out failureReason)
+            || !ValidateIssueIsConvertible(issueDetails, converter, configurationScopeRoot, out var visualization, out failureReason))
         {
             messageBox.InvalidRequest(failureReason);
             return;
@@ -108,50 +109,55 @@ private void ShowIssueInternal<T>(T issueDetails, string issueConfigurationScope
             visualization = visualizationProcessor.HandleConvertedIssue(visualization) ;
         }
         issueSelectionService.SelectedIssue = visualization;
-
+
+        toolWindowService.Show(toolWindowId);
+
         var navigationResult = navigator.TryNavigatePartial(visualization);
-        HandleNavigationResult(navigationResult, toolWindowId, visualization);
+        if (navigationResult == NavigationResult.OpenedLocation)
+        {
+            logger.WriteLine(string.Format(OpenInIdeResources.DoneProcessingRequest, issueDetails?.Key));
+        }
+        else
+        {
+            HandleIncompleteNavigation(visualization, navigationResult);
+        }
+    }
+
+    private bool ValidateIssueNotNull<T>(T issueDetails, out string failureReason) where T : IOpenInIdeIssue
+    {
+        if (issueDetails is { Key: not null })
+        {
+            failureReason = default;
+            return true;
+        }
+
+        failureReason = OpenInIdeResources.ValidationReason_MalformedRequest;
+        return false;
+
     }
 
     private bool ValidateConfiguration(string issueConfigurationScope, out string configurationScopeRoot, out string failureReason)
     {
         return openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(issueConfigurationScope, out configurationScopeRoot, out failureReason);
     }
 
-    private bool ValidateOpenInIdeIssue<T>(T issueDetails,
+    private bool ValidateIssueIsConvertible<T>(T issueDetails,
         IOpenInIdeIssueToAnalysisIssueConverter<T> converter,
         string configurationScopeRoot,
         out IAnalysisIssueVisualization visualization,
         out string failureReason) where T : IOpenInIdeIssue
     {
-        failureReason = default;
-
         if (converterImplementation.TryConvert(issueDetails, configurationScopeRoot, converter, out visualization))
         {
+            failureReason = default;
             return true;
         }
 
         failureReason = OpenInIdeResources.ValidationReason_UnableToConvertIssue;
         return false;
-
-    }
-
-    private void HandleNavigationResult(NavigationResult navigationResult,
-        Guid toolWindowId,
-        IAnalysisIssueVisualization visualization)
-    {
-        if (navigationResult == NavigationResult.OpenedLocation)
-        {
-            toolWindowService.Show(toolWindowId);
-        }
-        else
-        {
-            HandleIncompleteNavigation(toolWindowId, visualization, navigationResult);
-        }
     }
 
-    private void HandleIncompleteNavigation(Guid toolWindowId, IAnalysisIssueVisualization visualization,
-        NavigationResult navigationResult)
+    private void HandleIncompleteNavigation(IAnalysisIssueVisualization visualization, NavigationResult navigationResult)
     {
         logger.WriteLine(OpenInIdeResources.IssueLocationNotFound, visualization.CurrentFilePath,
             visualization.Location?.TextRange?.StartLine, visualization.Location?.TextRange?.StartLineOffset);
@@ -162,7 +168,6 @@ private void HandleIncompleteNavigation(Guid toolWindowId, IAnalysisIssueVisuali
                 messageBox.UnableToOpenFile(visualization.CurrentFilePath);
                 return;
             case NavigationResult.OpenedFile:
-                toolWindowService.Show(toolWindowId);
                 messageBox.UnableToLocateIssue(visualization.CurrentFilePath);
                 return;
             default:

src/IssueViz/OpenInIde/OpenInIdeMessageBox.cs

@@ -21,6 +21,7 @@
 using System.ComponentModel.Composition;
 using System.Windows;
 using SonarLint.VisualStudio.Core;
+using SonarLint.VisualStudio.Infrastructure.VS;
 
 namespace SonarLint.VisualStudio.IssueVisualization.OpenInIde;
 
@@ -36,11 +37,13 @@ internal interface IOpenInIdeMessageBox
 internal class OpenInIdeMessageBox : IOpenInIdeMessageBox
 {
     private readonly IMessageBox messageBox;
+    private readonly IThreadHandling threadHandling;
 
     [ImportingConstructor]
-    public OpenInIdeMessageBox(IMessageBox messageBox)
+    public OpenInIdeMessageBox(IMessageBox messageBox, IThreadHandling threadHandling)
     {
         this.messageBox = messageBox;
+        this.threadHandling = threadHandling;
     }
 
     public void UnableToLocateIssue(string filePath) => 
@@ -52,6 +55,6 @@ public void UnableToOpenFile(string filePath) =>
     public void InvalidRequest(string reason) => 
         Show(string.Format(OpenInIdeResources.MessageBox_InvalidConfiguration, reason));
 
-    private void Show(string message) =>
-        messageBox.Show(message, OpenInIdeResources.MessageBox_Caption, MessageBoxButton.OK, MessageBoxImage.Warning);
+    private void Show(string message) => threadHandling.RunOnUIThread(() =>
+        messageBox.Show(message, OpenInIdeResources.MessageBox_Caption, MessageBoxButton.OK, MessageBoxImage.Warning));
 }