From 9fbe67be99945527f0237671b44091c440e187c0 Mon Sep 17 00:00:00 2001 From: costin-zaharia-sonarsource Date: Tue, 5 Nov 2024 15:03:49 +0100 Subject: [PATCH 1/3] SCAN4NET-158 Make file indexing more resilient --- .../AdditionalFilesServiceTest.cs | 63 ++++++++++++++++++- .../PropertiesFileGeneratorTests.cs | 2 +- .../AdditionalFilesService.cs | 27 ++++++-- .../PropertiesFileGenerator.cs | 2 +- .../Resources.Designer.cs | 19 +++++- src/SonarScanner.MSBuild.Shim/Resources.resx | 3 + 6 files changed, 107 insertions(+), 9 deletions(-) diff --git a/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs b/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs index 486273686..fe5b1ac2d 100644 --- a/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs +++ b/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs @@ -23,6 +23,7 @@ using FluentAssertions; using Microsoft.VisualStudio.TestTools.UnitTesting; using NSubstitute; +using NSubstitute.ExceptionExtensions; using SonarScanner.MSBuild.Common; using TestUtilities; @@ -43,7 +44,7 @@ public AdditionalFilesServiceTest() wrapper .EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories) .Returns([]); - sut = new(wrapper); + sut = new(wrapper, logger); } [TestMethod] @@ -344,4 +345,64 @@ public void AdditionalFiles_ExtensionsFound_MultipleProperties_NoAdditionalParam "file12.test.TSx"); logger.AssertNoWarningsLogged(); } + + [TestMethod] + public void AdditionalFiles_DirectoryAccessFail() + { + var directoryWrapper = Substitute.For(); + directoryWrapper.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories).Throws(_ => new DirectoryNotFoundException("Error message")); + var analysisConfig = new AnalysisConfig + { + ScanAllAnalysis = true, + LocalSettings = [], + ServerSettings = [new("sonar.typescript.file.suffixes", ".ts,.tsx")] + }; + + var sut = new AdditionalFilesService(directoryWrapper, logger); + var files = sut.AdditionalFiles(analysisConfig, ProjectBaseDir); + + files.Sources.Select(x => x.Name).Should().BeEmpty(); + files.Tests.Select(x => x.Name).Should().BeEmpty(); + logger.DebugMessages.Should().BeEquivalentTo( + $"Reading directories from: '{ProjectBaseDir}'.", + $"Reading files from: '{ProjectBaseDir}'.", + "HResult: -2147024893, Exception: Error message", + $"Found 0 files in: '{ProjectBaseDir}'."); + logger.AssertWarningLogged($"Failed to get directories from: '{ProjectBaseDir}'."); + } + + [TestMethod] + public void AdditionalFiles_FileAccessFail() + { + var firstDirectory = new DirectoryInfo(Path.Combine(ProjectBaseDir.FullName, "first directory")); + var secondDirectory = new DirectoryInfo(Path.Combine(ProjectBaseDir.FullName, "second directory")); + var directoryWrapper = Substitute.For(); + directoryWrapper.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories).Returns([firstDirectory, secondDirectory]); + directoryWrapper.EnumerateFiles(ProjectBaseDir, "*", SearchOption.TopDirectoryOnly).Returns([new FileInfo("file in base dir.ts")]); + directoryWrapper.EnumerateFiles(firstDirectory, "*", SearchOption.TopDirectoryOnly).Throws(_ => new PathTooLongException("Error message")); + directoryWrapper.EnumerateFiles(secondDirectory, "*", SearchOption.TopDirectoryOnly).Returns([new FileInfo("file in second dir.ts")]); + var analysisConfig = new AnalysisConfig + { + ScanAllAnalysis = true, + LocalSettings = [], + ServerSettings = [new("sonar.typescript.file.suffixes", ".ts,.tsx")] + }; + + var sut = new AdditionalFilesService(directoryWrapper, logger); + var files = sut.AdditionalFiles(analysisConfig, ProjectBaseDir); + + files.Sources.Select(x => x.Name).Should().BeEquivalentTo("file in base dir.ts", "file in second dir.ts"); + files.Tests.Select(x => x.Name).Should().BeEmpty(); + + logger.DebugMessages.Should().BeEquivalentTo( + @"Reading directories from: 'C:\dev'.", + @"Found 2 directories in: 'C:\dev'.", + @"Reading files from: 'C:\dev\first directory'.", + @"HResult: -2147024690, Exception: Error message", + @"Reading files from: 'C:\dev\second directory'.", + @"Found 1 files in: 'C:\dev\second directory'.", + @"Reading files from: 'C:\dev'.", + @"Found 1 files in: 'C:\dev'."); + logger.AssertWarningLogged(@"Failed to get files from: 'C:\dev\first directory'."); + } } diff --git a/Tests/SonarScanner.MSBuild.Shim.Test/PropertiesFileGeneratorTests.cs b/Tests/SonarScanner.MSBuild.Shim.Test/PropertiesFileGeneratorTests.cs index ca0592b59..7af1bfaea 100644 --- a/Tests/SonarScanner.MSBuild.Shim.Test/PropertiesFileGeneratorTests.cs +++ b/Tests/SonarScanner.MSBuild.Shim.Test/PropertiesFileGeneratorTests.cs @@ -1546,7 +1546,7 @@ private PropertiesFileGenerator CreateSut( { sarifFixer ??= new RoslynV1SarifFixer(logger); runtimeInformationWrapper ??= new RuntimeInformationWrapper(); - additionalFileService ??= new AdditionalFilesService(DirectoryWrapper.Instance); + additionalFileService ??= new AdditionalFilesService(DirectoryWrapper.Instance, logger); return new(analysisConfig, logger, sarifFixer, runtimeInformationWrapper, additionalFileService); } } diff --git a/src/SonarScanner.MSBuild.Shim/AdditionalFilesService.cs b/src/SonarScanner.MSBuild.Shim/AdditionalFilesService.cs index a8dd8efd8..6b7a2039b 100644 --- a/src/SonarScanner.MSBuild.Shim/AdditionalFilesService.cs +++ b/src/SonarScanner.MSBuild.Shim/AdditionalFilesService.cs @@ -31,8 +31,9 @@ namespace SonarScanner.MSBuild.Shim; // https://github.com/SonarSource/sonar-scanner-engine/blob/0d222f01c0b3a15e95c5c7d335d29c40ddf5d628/sonarcloud/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/ProjectFilePreprocessor.java#L96 // and // https://github.com/SonarSource/sonar-scanner-engine/blob/0d222f01c0b3a15e95c5c7d335d29c40ddf5d628/sonarcloud/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/LanguageDetection.java#L70 -public class AdditionalFilesService(IDirectoryWrapper directoryWrapper) : IAdditionalFilesService +public class AdditionalFilesService(IDirectoryWrapper directoryWrapper, ILogger logger) : IAdditionalFilesService { + private const string SearchPatternAll = "*"; private static readonly char[] Comma = [',']; private static readonly IReadOnlyList ExcludedDirectories = @@ -87,15 +88,31 @@ public AdditionalFiles AdditionalFiles(AnalysisConfig analysisConfig, DirectoryI : PartitionAdditionalFiles(GetAllFiles(extensions, projectBaseDir), analysisConfig); } - private FileInfo[] GetAllFiles(IEnumerable extensions, DirectoryInfo projectBaseDir) => - directoryWrapper - .EnumerateDirectories(projectBaseDir, "*", SearchOption.AllDirectories) + private FileInfo[] GetAllFiles(IReadOnlyList extensions, DirectoryInfo projectBaseDir) => + CallDirectoryQuerySafe(projectBaseDir, "directories", () => directoryWrapper.EnumerateDirectories(projectBaseDir, SearchPatternAll, SearchOption.AllDirectories)) .Concat([projectBaseDir]) // also include the root directory .Where(x => !IsExcludedDirectory(x)) - .SelectMany(x => directoryWrapper.EnumerateFiles(x, "*", SearchOption.TopDirectoryOnly)) + .SelectMany(x => CallDirectoryQuerySafe(x, "files", () => directoryWrapper.EnumerateFiles(x, SearchPatternAll, SearchOption.TopDirectoryOnly))) .Where(x => !IsExcludedFile(x) && extensions.Any(e => x.Name.EndsWith(e, StringComparison.OrdinalIgnoreCase) && !x.Name.Equals(e, StringComparison.OrdinalIgnoreCase))) .ToArray(); + private IReadOnlyList CallDirectoryQuerySafe(DirectoryInfo path, string entryType, Func> query) + { + try + { + logger.LogDebug("Reading {0} from: '{1}'.", entryType, path.FullName); + var result = query().ToArray(); + logger.LogDebug("Found {0} {1} in: '{2}'.", result.Length, entryType, path.FullName); + return result; + } + catch (Exception exception) + { + logger.LogWarning(Resources.WARN_DirectoryGetContentFailure, entryType, path.FullName); + logger.LogDebug("HResult: {0}, Exception: {1}", exception.HResult, exception.Message, exception); + } + return Array.Empty(); + } + private static bool IsExcludedDirectory(DirectoryInfo directory) => ExcludedDirectories.Any(x => Array.Exists( directory.FullName.Split(Path.DirectorySeparatorChar), // split it so that we also exclude subdirectories like .sonarqube/conf. diff --git a/src/SonarScanner.MSBuild.Shim/PropertiesFileGenerator.cs b/src/SonarScanner.MSBuild.Shim/PropertiesFileGenerator.cs index 3d5498664..b0c628083 100644 --- a/src/SonarScanner.MSBuild.Shim/PropertiesFileGenerator.cs +++ b/src/SonarScanner.MSBuild.Shim/PropertiesFileGenerator.cs @@ -51,7 +51,7 @@ public class PropertiesFileGenerator : IPropertiesFileGenerator private readonly StringComparison pathComparison; public PropertiesFileGenerator(AnalysisConfig analysisConfig, ILogger logger) - : this(analysisConfig, logger, new RoslynV1SarifFixer(logger), new RuntimeInformationWrapper(), new AdditionalFilesService(DirectoryWrapper.Instance)) + : this(analysisConfig, logger, new RoslynV1SarifFixer(logger), new RuntimeInformationWrapper(), new AdditionalFilesService(DirectoryWrapper.Instance, logger)) { } diff --git a/src/SonarScanner.MSBuild.Shim/Resources.Designer.cs b/src/SonarScanner.MSBuild.Shim/Resources.Designer.cs index e056d6a78..72a8bc03b 100644 --- a/src/SonarScanner.MSBuild.Shim/Resources.Designer.cs +++ b/src/SonarScanner.MSBuild.Shim/Resources.Designer.cs @@ -1,7 +1,6 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.42000 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -401,6 +400,15 @@ internal static string REPORT_TestProjectsTitle { } } + /// + /// Looks up a localized string similar to Failed to get {0} from: '{1}'.. + /// + internal static string WARN_DirectoryGetContentFailure { + get { + return ResourceManager.GetString("WARN_DirectoryGetContentFailure", resourceCulture); + } + } + /// /// Looks up a localized string similar to Directory '{0}' is not located under the base directory '{1}' and will not be analyzed.. /// @@ -410,6 +418,15 @@ internal static string WARN_DirectoryIsOutsideBaseDir { } } + /// + /// Looks up a localized string similar to The support for multi-language analysis may not function correctly if {0} is set. If this is the case, please explicitly set "sonar.scanner.scanAll=false" to disable the multi-language analysis.. + /// + internal static string WARN_DisableScanAllAnalysisWhenProvidingParameters { + get { + return ResourceManager.GetString("WARN_DisableScanAllAnalysisWhenProvidingParameters", resourceCulture); + } + } + /// /// Looks up a localized string similar to Duplicate ProjectGuid: "{0}". The project will not be analyzed. Project file: "{1}". /// diff --git a/src/SonarScanner.MSBuild.Shim/Resources.resx b/src/SonarScanner.MSBuild.Shim/Resources.resx index e96ef2d66..8a588fa81 100644 --- a/src/SonarScanner.MSBuild.Shim/Resources.resx +++ b/src/SonarScanner.MSBuild.Shim/Resources.resx @@ -261,6 +261,9 @@ Possible causes: Directory '{0}' is not located under the base directory '{1}' and will not be analyzed. + + Failed to get {0} from: '{1}'. + Using working directory as project base directory: '{0}'. From a40ee0f84dad22136d9463958e5a6f6a83f120f9 Mon Sep 17 00:00:00 2001 From: costin-zaharia-sonarsource Date: Thu, 7 Nov 2024 10:13:05 +0100 Subject: [PATCH 2/3] Address comments --- .../AdditionalFilesServiceTest.cs | 45 ++++++++++--------- .../AdditionalFilesService.cs | 2 +- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs b/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs index fe5b1ac2d..6428cb3e1 100644 --- a/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs +++ b/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs @@ -361,14 +361,15 @@ public void AdditionalFiles_DirectoryAccessFail() var sut = new AdditionalFilesService(directoryWrapper, logger); var files = sut.AdditionalFiles(analysisConfig, ProjectBaseDir); - files.Sources.Select(x => x.Name).Should().BeEmpty(); - files.Tests.Select(x => x.Name).Should().BeEmpty(); - logger.DebugMessages.Should().BeEquivalentTo( - $"Reading directories from: '{ProjectBaseDir}'.", - $"Reading files from: '{ProjectBaseDir}'.", - "HResult: -2147024893, Exception: Error message", - $"Found 0 files in: '{ProjectBaseDir}'."); - logger.AssertWarningLogged($"Failed to get directories from: '{ProjectBaseDir}'."); + files.Sources.Should().BeEmpty(); + files.Tests.Should().BeEmpty(); + directoryWrapper.Received(1).EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories); + logger.DebugMessages[0].Should().Be($"Reading directories from: '{ProjectBaseDir}'."); + logger.DebugMessages[1].Should().MatchEquivalentOf(@"HResult: -2147024893, Exception: System.IO.DirectoryNotFoundException: Error message + at NSubstitute.ExceptionExtensions.ExceptionExtensions.<>c__DisplayClass2_0.b__0(CallInfo ci) *"); + logger.DebugMessages[2].Should().Be($"Reading files from: '{ProjectBaseDir}'."); + logger.DebugMessages[3].Should().Be($"Found 0 files in: '{ProjectBaseDir}'."); + logger.AssertSingleWarningExists($"Failed to get directories from: '{ProjectBaseDir}'."); } [TestMethod] @@ -392,17 +393,21 @@ public void AdditionalFiles_FileAccessFail() var files = sut.AdditionalFiles(analysisConfig, ProjectBaseDir); files.Sources.Select(x => x.Name).Should().BeEquivalentTo("file in base dir.ts", "file in second dir.ts"); - files.Tests.Select(x => x.Name).Should().BeEmpty(); - - logger.DebugMessages.Should().BeEquivalentTo( - @"Reading directories from: 'C:\dev'.", - @"Found 2 directories in: 'C:\dev'.", - @"Reading files from: 'C:\dev\first directory'.", - @"HResult: -2147024690, Exception: Error message", - @"Reading files from: 'C:\dev\second directory'.", - @"Found 1 files in: 'C:\dev\second directory'.", - @"Reading files from: 'C:\dev'.", - @"Found 1 files in: 'C:\dev'."); - logger.AssertWarningLogged(@"Failed to get files from: 'C:\dev\first directory'."); + files.Tests.Should().BeEmpty(); + directoryWrapper.Received(1).EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories); + directoryWrapper.Received(3).EnumerateFiles(Arg.Any(), "*", SearchOption.TopDirectoryOnly); + + logger.DebugMessages.Should().HaveCount(8); + logger.DebugMessages[0].Should().Be(@"Reading directories from: 'C:\dev'."); + logger.DebugMessages[1].Should().Be(@"Found 2 directories in: 'C:\dev'."); + logger.DebugMessages[2].Should().Be(@"Reading files from: 'C:\dev\first directory'."); + logger.DebugMessages[3].Should().MatchEquivalentOf(@"HResult: -2147024690, Exception: System.IO.PathTooLongException: Error message + at NSubstitute.ExceptionExtensions.ExceptionExtensions.<>c__DisplayClass2_0.b__0(CallInfo ci) *"); + logger.DebugMessages[4].Should().Be(@"Reading files from: 'C:\dev\second directory'."); + logger.DebugMessages[5].Should().Be(@"Found 1 files in: 'C:\dev\second directory'."); + logger.DebugMessages[6].Should().Be(@"Reading files from: 'C:\dev'."); + logger.DebugMessages[7].Should().Be(@"Found 1 files in: 'C:\dev'."); + + logger.AssertSingleWarningExists(@"Failed to get files from: 'C:\dev\first directory'."); } } diff --git a/src/SonarScanner.MSBuild.Shim/AdditionalFilesService.cs b/src/SonarScanner.MSBuild.Shim/AdditionalFilesService.cs index 6b7a2039b..30cf8af47 100644 --- a/src/SonarScanner.MSBuild.Shim/AdditionalFilesService.cs +++ b/src/SonarScanner.MSBuild.Shim/AdditionalFilesService.cs @@ -108,7 +108,7 @@ private IReadOnlyList CallDirectoryQuerySafe(DirectoryInfo path, string en catch (Exception exception) { logger.LogWarning(Resources.WARN_DirectoryGetContentFailure, entryType, path.FullName); - logger.LogDebug("HResult: {0}, Exception: {1}", exception.HResult, exception.Message, exception); + logger.LogDebug("HResult: {0}, Exception: {1}", exception.HResult, exception); } return Array.Empty(); } From 7b1891e2a65b6fd8e04038acd8f1c1c99e291760 Mon Sep 17 00:00:00 2001 From: costin-zaharia-sonarsource Date: Thu, 7 Nov 2024 14:03:10 +0100 Subject: [PATCH 3/3] Use class based sut --- .../AdditionalFilesServiceTest.cs | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs b/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs index 6428cb3e1..74db9ae34 100644 --- a/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs +++ b/Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs @@ -349,8 +349,7 @@ public void AdditionalFiles_ExtensionsFound_MultipleProperties_NoAdditionalParam [TestMethod] public void AdditionalFiles_DirectoryAccessFail() { - var directoryWrapper = Substitute.For(); - directoryWrapper.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories).Throws(_ => new DirectoryNotFoundException("Error message")); + wrapper.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories).Throws(_ => new DirectoryNotFoundException("Error message")); var analysisConfig = new AnalysisConfig { ScanAllAnalysis = true, @@ -358,12 +357,11 @@ public void AdditionalFiles_DirectoryAccessFail() ServerSettings = [new("sonar.typescript.file.suffixes", ".ts,.tsx")] }; - var sut = new AdditionalFilesService(directoryWrapper, logger); var files = sut.AdditionalFiles(analysisConfig, ProjectBaseDir); files.Sources.Should().BeEmpty(); files.Tests.Should().BeEmpty(); - directoryWrapper.Received(1).EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories); + wrapper.Received(1).EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories); logger.DebugMessages[0].Should().Be($"Reading directories from: '{ProjectBaseDir}'."); logger.DebugMessages[1].Should().MatchEquivalentOf(@"HResult: -2147024893, Exception: System.IO.DirectoryNotFoundException: Error message at NSubstitute.ExceptionExtensions.ExceptionExtensions.<>c__DisplayClass2_0.b__0(CallInfo ci) *"); @@ -377,11 +375,10 @@ public void AdditionalFiles_FileAccessFail() { var firstDirectory = new DirectoryInfo(Path.Combine(ProjectBaseDir.FullName, "first directory")); var secondDirectory = new DirectoryInfo(Path.Combine(ProjectBaseDir.FullName, "second directory")); - var directoryWrapper = Substitute.For(); - directoryWrapper.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories).Returns([firstDirectory, secondDirectory]); - directoryWrapper.EnumerateFiles(ProjectBaseDir, "*", SearchOption.TopDirectoryOnly).Returns([new FileInfo("file in base dir.ts")]); - directoryWrapper.EnumerateFiles(firstDirectory, "*", SearchOption.TopDirectoryOnly).Throws(_ => new PathTooLongException("Error message")); - directoryWrapper.EnumerateFiles(secondDirectory, "*", SearchOption.TopDirectoryOnly).Returns([new FileInfo("file in second dir.ts")]); + wrapper.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories).Returns([firstDirectory, secondDirectory]); + wrapper.EnumerateFiles(ProjectBaseDir, "*", SearchOption.TopDirectoryOnly).Returns([new FileInfo("file in base dir.ts")]); + wrapper.EnumerateFiles(firstDirectory, "*", SearchOption.TopDirectoryOnly).Throws(_ => new PathTooLongException("Error message")); + wrapper.EnumerateFiles(secondDirectory, "*", SearchOption.TopDirectoryOnly).Returns([new FileInfo("file in second dir.ts")]); var analysisConfig = new AnalysisConfig { ScanAllAnalysis = true, @@ -389,13 +386,12 @@ public void AdditionalFiles_FileAccessFail() ServerSettings = [new("sonar.typescript.file.suffixes", ".ts,.tsx")] }; - var sut = new AdditionalFilesService(directoryWrapper, logger); var files = sut.AdditionalFiles(analysisConfig, ProjectBaseDir); files.Sources.Select(x => x.Name).Should().BeEquivalentTo("file in base dir.ts", "file in second dir.ts"); files.Tests.Should().BeEmpty(); - directoryWrapper.Received(1).EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories); - directoryWrapper.Received(3).EnumerateFiles(Arg.Any(), "*", SearchOption.TopDirectoryOnly); + wrapper.Received(1).EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories); + wrapper.Received(3).EnumerateFiles(Arg.Any(), "*", SearchOption.TopDirectoryOnly); logger.DebugMessages.Should().HaveCount(8); logger.DebugMessages[0].Should().Be(@"Reading directories from: 'C:\dev'.");