From cef88f83f73dfb01180cd5f15105fb53d984cc73 Mon Sep 17 00:00:00 2001 From: Larry Golding Date: Mon, 31 Aug 2020 09:00:16 -0700 Subject: [PATCH] Rebase absolute URIs relative to the closest enclosing repo root. (#2047) --- src/Sarif/ExecutionEnvironment.cs | 35 ---- src/Sarif/ExternalProcess.cs | 4 +- src/Sarif/GitHelper.cs | 192 ++++++++++++++++++ src/Sarif/GitInformation.cs | 180 ---------------- src/Sarif/IExecutionEnvironment.cs | 35 ---- src/Sarif/SdkResources.Designer.cs | 9 + src/Sarif/SdkResources.resx | 3 + .../Visitors/InsertOptionalDataVisitor.cs | 163 ++++++++++++--- .../GitInformationTests.cs | 34 ---- src/Test.UnitTests.Sarif/GitHelperTests.cs | 121 +++++++++++ .../GitInformationTests.cs | 42 ---- .../CoreTests-Relative_All.sarif | 11 +- ...s-Relative_VersionControlInformation.sarif | 3 + .../InsertOptionalDataVisitorTests.cs | 134 +++++++++++- 14 files changed, 598 insertions(+), 368 deletions(-) delete mode 100644 src/Sarif/ExecutionEnvironment.cs create mode 100644 src/Sarif/GitHelper.cs delete mode 100644 src/Sarif/GitInformation.cs delete mode 100644 src/Sarif/IExecutionEnvironment.cs delete mode 100644 src/Test.FunctionalTests.Sarif/GitInformationTests.cs create mode 100644 src/Test.UnitTests.Sarif/GitHelperTests.cs delete mode 100644 src/Test.UnitTests.Sarif/GitInformationTests.cs diff --git a/src/Sarif/ExecutionEnvironment.cs b/src/Sarif/ExecutionEnvironment.cs deleted file mode 100644 index f8090a513..000000000 --- a/src/Sarif/ExecutionEnvironment.cs +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; - -namespace Microsoft.CodeAnalysis.Sarif -{ - /// - /// A wrapper class for accessing the execution environment. - /// - /// - /// Clients should use this class rather than directly using the System.Environment - /// class, so they can mock the IExecutionEnvironment interface in unit tests. - /// - public class ExecutionEnvironment : IExecutionEnvironment - { - /// - /// Replaces the name of each environment variable embedded in the specified string with - /// the string equivalent of the value of the variable, then returns the resulting string. - /// - /// - /// A string containing the names of zero or more environment variables. Each environment - /// variable is quoted with the percent sign character (%). - /// - /// - /// A string with each environment variable replaced by its value. - /// - public string ExpandEnvironmentVariables(string name) => Environment.ExpandEnvironmentVariables(name); - - /// - /// Gets the fully qualified path of the current working directory. - /// - public string CurrentDirectory => Environment.CurrentDirectory; - } -} diff --git a/src/Sarif/ExternalProcess.cs b/src/Sarif/ExternalProcess.cs index e8d617610..dc742c820 100644 --- a/src/Sarif/ExternalProcess.cs +++ b/src/Sarif/ExternalProcess.cs @@ -17,7 +17,7 @@ public class ExternalProcess public ExternalProcess( string workingDirectory, - string fileName, + string exePath, string arguments, IConsoleCapture stdOut = null, int[] acceptableReturnCodes = null) @@ -26,7 +26,7 @@ public ExternalProcess( ProcessStartInfo psi = new ProcessStartInfo(); - psi.FileName = fileName; + psi.FileName = exePath; psi.Arguments = arguments; psi.WorkingDirectory = workingDirectory; diff --git a/src/Sarif/GitHelper.cs b/src/Sarif/GitHelper.cs new file mode 100644 index 000000000..647935d28 --- /dev/null +++ b/src/Sarif/GitHelper.cs @@ -0,0 +1,192 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; + +namespace Microsoft.CodeAnalysis.Sarif +{ + public class GitHelper : IDisposable + { + public static readonly GitHelper Default = new GitHelper(); + + public delegate string ProcessRunner(string workingDirectory, string exePath, string arguments); + + private static readonly ProcessRunner DefaultProcessRunner = + (string workingDirectory, string exePath, string arguments) + => new ExternalProcess(workingDirectory, exePath, arguments, stdOut: null, acceptableReturnCodes: null).StdOut.Text; + + private readonly IFileSystem fileSystem; + private readonly ProcessRunner processRunner; + private readonly ReaderWriterLockSlim cacheLock = new ReaderWriterLockSlim(); + + internal static readonly string s_expectedGitExePath = Environment.ExpandEnvironmentVariables(@"%ProgramFiles%\Git\cmd\git.exe"); + + // A cache that maps directory names to the root directory of the repository, if any, that + // contains them. + // + // The case-insensitive key comparison is correct on Windows systems and wrong on Linux/MacOS. + // This is a general problem in the SDK. See https://github.com/microsoft/sarif-sdk/issues/1736, + // "File path comparisons are not file-system-appropriate". + // + // The cache is internal rather than private so that tests can verify that the cache is + // being populated appropriately. It's not so easy to verify that it's being _used_ + // appropriately. + internal readonly Dictionary directoryToRepoRootPathDictionary = new Dictionary(StringComparer.OrdinalIgnoreCase); + + public bool IsRepositoryRoot(string repositoryPath) => this.fileSystem.DirectoryExists(Path.Combine(repositoryPath, ".git")); + + public GitHelper(IFileSystem fileSystem = null, ProcessRunner processRunner = null) + { + this.fileSystem = fileSystem ?? new FileSystem(); + this.processRunner = processRunner ?? DefaultProcessRunner; + + GitExePath = GetGitExePath(); + } + + public string GitExePath { get; } + + public Uri GetRemoteUri(string repoPath) + { + string uriText = GetSimpleGitCommandOutput( + repoPath, + args: $"remote get-url origin"); + + return uriText == null + ? null + : new Uri(uriText, UriKind.Absolute); + } + + public string GetCurrentCommit(string repoPath) + { + return GetSimpleGitCommandOutput( + repoPath, + args: "rev-parse --verify HEAD"); + } + + public void Checkout(string repoPath, string commitSha) + { + GetSimpleGitCommandOutput( + repoPath, + args: $"checkout {commitSha}"); + } + + private string GetGitExePath() + => this.fileSystem.FileExists(s_expectedGitExePath) ? s_expectedGitExePath : null; + + public string GetCurrentBranch(string repoPath) + { + return GetSimpleGitCommandOutput( + repoPath, + args: "rev-parse --abbrev-ref HEAD"); + } + + private string GetSimpleGitCommandOutput(string repositoryPath, string args) + { + string gitPath = this.GitExePath; + + if (gitPath == null || !IsRepositoryRoot(repositoryPath)) + { + return null; + } + + string stdOut = this.processRunner( + workingDirectory: repositoryPath, + exePath: gitPath, + arguments: args); + + return TrimNewlines(stdOut); + } + + private static string TrimNewlines(string text) => text + .Replace("\r", string.Empty) + .Replace("\n", string.Empty); + + public string GetRepositoryRoot(string path, bool useCache = true) + { + // The "default" instance won't let you use the cache, to prevent independent users + // from interfering with each other. + if (useCache && object.ReferenceEquals(this, Default)) + { + throw new ArgumentException(SdkResources.GitHelperDefaultInstanceDoesNotPermitCaching, nameof(useCache)); + } + + if (this.fileSystem.FileExists(path)) + { + path = Path.GetDirectoryName(path); + } + + string repoRootPath; + if (useCache) + { + cacheLock.EnterReadLock(); + try + { + if (directoryToRepoRootPathDictionary.TryGetValue(path, out repoRootPath)) + { + return repoRootPath; + } + } + finally + { + cacheLock.ExitReadLock(); + } + } + + repoRootPath = path; + while (!string.IsNullOrEmpty(repoRootPath) && !IsRepositoryRoot(repoRootPath)) + { + repoRootPath = Path.GetDirectoryName(repoRootPath); + } + + // It's important to terminate with a slash because the value returned from this method + // will be used to create an absolute URI on which MakeUriRelative will be called. For + // example, suppose this method returns @"C:\\dev\sarif-sdk\". The caller will use it + // to create an absolute URI (call it repoRootUri) "file:///C:/dev/sarif-sdk/". The + // caller will use this URI to "rebase" another URI (call it artifactUri) such as + // "file:///C:/dev/sarif-sdk/src/Sarif". The caller will do that by calling + // repoRootUri.MakeRelativeUri(artifactUri). It turns out that unless repoRootUri + // ends with a slash, this call will return "sarif-sdk/src/Sarif" rather than the + // expected (at least for me) "src/Sarif". + if (repoRootPath != null && !repoRootPath.EndsWith(@"\")) { repoRootPath += @"\"; } + + if (useCache) + { + cacheLock.EnterWriteLock(); + try + { + // Add whatever we found to the cache, even if it was null (in which case we now know + // that this path isn't under source control). + directoryToRepoRootPathDictionary.Add(path, repoRootPath); + } + finally + { + cacheLock.ExitWriteLock(); + } + } + + return repoRootPath; + } + + /// + /// Dispose pattern as required by style + /// + /// Set to true to actually dispose + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + cacheLock.Dispose(); + } + } + + /// + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + } +} diff --git a/src/Sarif/GitInformation.cs b/src/Sarif/GitInformation.cs deleted file mode 100644 index 57aa301a6..000000000 --- a/src/Sarif/GitInformation.cs +++ /dev/null @@ -1,180 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; -using System.Collections.Generic; -using System.IO; -using System.Threading; - -namespace Microsoft.CodeAnalysis.Sarif -{ - internal class GitInformation : IDisposable - { - private readonly IFileSystem fileSystem; - private readonly ReaderWriterLockSlim cacheLock = new ReaderWriterLockSlim(); - private readonly Dictionary repoRoots = new Dictionary(StringComparer.OrdinalIgnoreCase); - - public bool IsRepositoryRoot(string repositoryPath) => this.fileSystem.DirectoryExists(Path.Combine(repositoryPath, ".git")); - - public GitInformation(IFileSystem fileSystem = null) - { - this.fileSystem = fileSystem ?? new FileSystem(); - GitExePath = GetGitExePath(); - } - - public string GitExePath { get; } - - public Uri GetRemoteUri(string repoPath) - { - string uriText = GetSimpleGitCommandOutput( - repoPath, - args: $"remote get-url origin"); - - return uriText == null - ? null - : new Uri(uriText, UriKind.Absolute); - } - - public string GetCurrentCommit(string repoPath) - { - return GetSimpleGitCommandOutput( - repoPath, - args: "rev-parse --verify HEAD"); - } - - public void Checkout(string repoPath, string commitSha) - { - GetSimpleGitCommandOutput( - repoPath, - args: $"checkout {commitSha}"); - } - - private string GetGitExePath() - { - string gitPath = Environment.ExpandEnvironmentVariables(@"%ProgramFiles%\Git\cmd\git.exe"); - return (!string.IsNullOrEmpty(gitPath) && this.fileSystem.FileExists(gitPath)) ? gitPath : null; - } - - public string GetCurrentBranch(string repoPath) - { - return GetSimpleGitCommandOutput( - repoPath, - args: "rev-parse --abbrev-ref HEAD"); - } - - private string GetSimpleGitCommandOutput(string repositoryPath, string args) - { - string gitPath = this.GitExePath; - - if (gitPath == null || !IsRepositoryRoot(repositoryPath)) - { - return null; - } - - var gitCommand = new ExternalProcess( - workingDirectory: repositoryPath, - fileName: gitPath, - arguments: args); - - return TrimNewlines(gitCommand.StdOut.Text); - } - - private static string TrimNewlines(string text) => text - .Replace("\r", string.Empty) - .Replace("\n", string.Empty); - - public VersionControlDetails GetVersionControlDetails(string repositoryPath, bool crawlParentDirectories) - { - VersionControlDetails value; - - cacheLock.EnterReadLock(); - try - { - if (!crawlParentDirectories) - { - if (repoRoots.TryGetValue(repositoryPath, out value)) - { - return value; - } - } - else - { - foreach (KeyValuePair kp in repoRoots) - { - if (repositoryPath.StartsWith(kp.Key, StringComparison.OrdinalIgnoreCase) && ((repositoryPath.Length == kp.Key.Length) || (repositoryPath[kp.Key.Length] == '\\'))) - { - return kp.Value; - } - } - } - } - finally - { - cacheLock.ExitReadLock(); - } - - repositoryPath = GetRepositoryRoot(repositoryPath); - - if (string.IsNullOrEmpty(repositoryPath)) - { - return null; - } - - Uri repoRemoteUri = GetRemoteUri(repositoryPath); - value = (repoRemoteUri is null) - ? null - : new VersionControlDetails - { - RepositoryUri = repoRemoteUri, - RevisionId = GetCurrentCommit(repositoryPath), - Branch = GetCurrentBranch(repositoryPath), - MappedTo = new ArtifactLocation { Uri = new Uri(repositoryPath, UriKind.Absolute) }, - }; - - cacheLock.EnterWriteLock(); - try - { - if (!repoRoots.ContainsKey(repositoryPath)) - { - repoRoots.Add(repositoryPath, value); - } - } - finally - { - cacheLock.ExitWriteLock(); - } - - return value; - } - - // Internal rather than private for unit-testability. - internal string GetRepositoryRoot(string path) - { - while (!string.IsNullOrEmpty(path) && !IsRepositoryRoot(path)) - { - path = Path.GetDirectoryName(path); - } - - return path; - } - - /// - /// Dispose pattern as required by style - /// - /// Set to true to actually dispose - protected virtual void Dispose(bool disposing) - { - if (disposing) - { - cacheLock.Dispose(); - } - } - - /// - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - } -} diff --git a/src/Sarif/IExecutionEnvironment.cs b/src/Sarif/IExecutionEnvironment.cs deleted file mode 100644 index 3ebe9867b..000000000 --- a/src/Sarif/IExecutionEnvironment.cs +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -namespace Microsoft.CodeAnalysis.Sarif -{ - /// - /// An interface for accessing the execution environment. - /// - /// - /// Clients wishing to access aspects of the execution environment such as environment - /// variables should instantiate an ExecutionEnvironment object rather than directly using - /// the System.Environment class, so they can mock the IExecutionEnvironment interface in - /// unit tests. - /// - public interface IExecutionEnvironment - { - /// - /// Replaces the name of each environment variable embedded in the specified string with - /// the string equivalent of the value of the variable, then returns the resulting string. - /// - /// - /// A string containing the names of zero or more environment variables. Each environment - /// variable is quoted with the percent sign character (%). - /// - /// - /// A string with each environment variable replaced by its value. - /// - string ExpandEnvironmentVariables(string name); - - /// - /// Gets the fully qualified path of the current working directory. - /// - string CurrentDirectory { get; } - } -} diff --git a/src/Sarif/SdkResources.Designer.cs b/src/Sarif/SdkResources.Designer.cs index 9f8d39b53..a6d9298f2 100644 --- a/src/Sarif/SdkResources.Designer.cs +++ b/src/Sarif/SdkResources.Designer.cs @@ -249,6 +249,15 @@ public static string ERR999_UnhandledEngineException { } } + /// + /// Looks up a localized string similar to When using the static 'Default' instance of GitHelper, you must pass the argument useCache: false to GetRepositoryRoot, which degrades performance. If the performance is not acceptable, create a separate GitHelper instance. You need not pass useCache: true because that is the default.. + /// + public static string GitHelperDefaultInstanceDoesNotPermitCaching { + get { + return ResourceManager.GetString("GitHelperDefaultInstanceDoesNotPermitCaching", resourceCulture); + } + } + /// /// Looks up a localized string similar to Element expected to be located under a different parent element.. /// diff --git a/src/Sarif/SdkResources.resx b/src/Sarif/SdkResources.resx index 1f87d639d..a6bfa828b 100644 --- a/src/Sarif/SdkResources.resx +++ b/src/Sarif/SdkResources.resx @@ -252,4 +252,7 @@ Cannot provide version control information because the current directory '{0}' is not under a Git repository root directory. + + When using the static 'Default' instance of GitHelper, you must pass the argument useCache: false to GetRepositoryRoot, which degrades performance. If the performance is not acceptable, create a separate GitHelper instance. You need not pass useCache: true because that is the default. + \ No newline at end of file diff --git a/src/Sarif/Visitors/InsertOptionalDataVisitor.cs b/src/Sarif/Visitors/InsertOptionalDataVisitor.cs index c833baa2a..eb5356fe0 100644 --- a/src/Sarif/Visitors/InsertOptionalDataVisitor.cs +++ b/src/Sarif/Visitors/InsertOptionalDataVisitor.cs @@ -10,10 +10,13 @@ namespace Microsoft.CodeAnalysis.Sarif.Visitors { public class InsertOptionalDataVisitor : SarifRewritingVisitor { - internal IFileSystem s_fileSystem = new FileSystem(); - internal IExecutionEnvironment s_executionEnvironment = new ExecutionEnvironment(); + private readonly IFileSystem _fileSystem; + private readonly GitHelper.ProcessRunner _processRunner; private Run _run; + private HashSet _repoRootUris; + private GitHelper _gitHelper; + private int _ruleIndex; private FileRegionsCache _fileRegionsCache; private readonly OptionallyEmittedData _dataToInsert; @@ -25,8 +28,15 @@ public InsertOptionalDataVisitor(OptionallyEmittedData dataToInsert, Run run) _run = run ?? throw new ArgumentNullException(nameof(run)); } - public InsertOptionalDataVisitor(OptionallyEmittedData dataToInsert, IDictionary originalUriBaseIds = null) + public InsertOptionalDataVisitor( + OptionallyEmittedData dataToInsert, + IDictionary originalUriBaseIds = null, + IFileSystem fileSystem = null, + GitHelper.ProcessRunner processRunner = null) { + _fileSystem = fileSystem ?? new FileSystem(); + _processRunner = processRunner; + _dataToInsert = dataToInsert; _originalUriBaseIds = originalUriBaseIds; _ruleIndex = -1; @@ -35,6 +45,8 @@ public InsertOptionalDataVisitor(OptionallyEmittedData dataToInsert, IDictionary public override Run VisitRun(Run node) { _run = node; + _gitHelper = new GitHelper(_fileSystem, _processRunner); + _repoRootUris = new HashSet(); if (_originalUriBaseIds != null) { @@ -46,11 +58,6 @@ public override Run VisitRun(Run node) } } - if (_run.VersionControlProvenance == null && _dataToInsert.HasFlag(OptionallyEmittedData.VersionControlInformation)) - { - InsertVersionControlInformation(); - } - if (node == null) { return null; } bool scrapeFileReferences = _dataToInsert.HasFlag(OptionallyEmittedData.Hashes) || @@ -65,29 +72,13 @@ public override Run VisitRun(Run node) Run visited = base.VisitRun(node); - return visited; - } - - private void InsertVersionControlInformation() - { - var gitInformation = new GitInformation(s_fileSystem); - string currentDirectory = s_executionEnvironment.CurrentDirectory; - - VersionControlDetails versionControlDetails = - gitInformation.GetVersionControlDetails(currentDirectory, crawlParentDirectories: true); - if (versionControlDetails == null) + // After all the ArtifactLocations have been visited, + if (_run.VersionControlProvenance == null && _dataToInsert.HasFlag(OptionallyEmittedData.VersionControlInformation)) { - throw new InvalidOperationException( - string.Format( - CultureInfo.CurrentCulture, - SdkResources.CannotProvideVersionControlInformation, - currentDirectory)); + _run.VersionControlProvenance = CreateVersionControlProvenance(); } - _run.VersionControlProvenance = new List - { - versionControlDetails - }; + return visited; } public override PhysicalLocation VisitPhysicalLocation(PhysicalLocation node) @@ -195,6 +186,16 @@ public override Artifact VisitArtifact(Artifact node) return base.VisitArtifact(node); } + public override ArtifactLocation VisitArtifactLocation(ArtifactLocation node) + { + if (_dataToInsert.HasFlag(OptionallyEmittedData.VersionControlInformation)) + { + node = ExpressRelativeToRepoRoot(node); + } + + return base.VisitArtifactLocation(node); + } + public override Result VisitResult(Result node) { _ruleIndex = node.RuleIndex; @@ -236,5 +237,111 @@ public override Message VisitMessage(Message node) } return base.VisitMessage(node); } + + private List CreateVersionControlProvenance() + { + var versionControlProvenance = new List(); + + foreach (Uri repoRootUri in _repoRootUris) + { + string repoRootPath = repoRootUri.LocalPath; + Uri repoRemoteUri = _gitHelper.GetRemoteUri(repoRootPath); + if (repoRemoteUri != null) + { + versionControlProvenance.Add( + new VersionControlDetails + { + RepositoryUri = repoRemoteUri, + RevisionId = _gitHelper.GetCurrentCommit(repoRootPath), + Branch = _gitHelper.GetCurrentBranch(repoRootPath), + MappedTo = new ArtifactLocation { Uri = repoRootUri } + }); + } + } + + return versionControlProvenance; + } + + private ArtifactLocation ExpressRelativeToRepoRoot(ArtifactLocation node) + { + Uri uri = node.Uri; + if (uri == null && node.Index >= 0 && _run.Artifacts?.Count > node.Index) + { + uri = _run.Artifacts[node.Index].Location.Uri; + } + + if (uri != null && uri.IsAbsoluteUri && uri.IsFile) + { + string repoRootPath = _gitHelper.GetRepositoryRoot(uri.LocalPath); + if (repoRootPath != null) + { + var repoRootUri = new Uri(repoRootPath, UriKind.Absolute); + _repoRootUris.Add(repoRootUri); + + Uri repoRelativeUri = repoRootUri.MakeRelativeUri(uri); + node.Uri = repoRelativeUri; + node.UriBaseId = GetUriBaseIdForRepoRoot(repoRootUri); + } + } + + return node; + } + + private string GetUriBaseIdForRepoRoot(Uri repoRootUri) + { + // Is there already an entry in OriginalUriBaseIds for this repo? + if (_run.OriginalUriBaseIds != null) + { + foreach (KeyValuePair pair in _run.OriginalUriBaseIds) + { + if (pair.Value.Uri == repoRootUri) + { + // Yes, so return it. + return pair.Key; + } + } + } + + // No, so add one. + if (_run.OriginalUriBaseIds == null) + { + _run.OriginalUriBaseIds = new Dictionary(); + } + + string uriBaseId = GetNextRepoRootUriBaseId(); + _run.OriginalUriBaseIds.Add( + uriBaseId, + new ArtifactLocation + { + Uri = repoRootUri + }); + + return uriBaseId; + } + + private string GetNextRepoRootUriBaseId() + { + ICollection originalUriBaseIdSymbols = _run.OriginalUriBaseIds.Keys; + + for (int i = 0; ; i++) + { + string uriBaseId = GetUriBaseId(i); + if (!originalUriBaseIdSymbols.Contains(uriBaseId)) + { + return uriBaseId; + } + } + } + + private const string RepoRootUriBaseIdStem = "REPO_ROOT"; + + // When there is only one repo root (the usual case), the uriBaseId is "REPO_ROOT" (unless + // that symbol is already in use in originalUriBaseIds. The second and subsequent uriBaseIds + // are REPO_ROOT_2, _3, etc. (again, skipping over any that are in use). We never assign + // REPO_ROOT_1 (although of course it might exist in originalUriBaseIds). + internal static string GetUriBaseId(int i) + => i == 0 + ? RepoRootUriBaseIdStem + : $"{RepoRootUriBaseIdStem}_{i + 1}"; } } diff --git a/src/Test.FunctionalTests.Sarif/GitInformationTests.cs b/src/Test.FunctionalTests.Sarif/GitInformationTests.cs deleted file mode 100644 index 0c200538a..000000000 --- a/src/Test.FunctionalTests.Sarif/GitInformationTests.cs +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; -using FluentAssertions; -using Xunit; - -namespace Microsoft.CodeAnalysis.Sarif -{ - public class GitInformationTests - { - [Fact] - public void GetVersionControlDetails_ReturnsExpectedInformation() - { - var gitInformation = new GitInformation(); - - // This test assumes that the directory in which the tests are run lies under the local - // repo root, for example, \bld\bin\AnyCPU_Release\Test.FunctionalTests.Sarif\netcoreapp2.1. - string localRepoRoot = gitInformation.GetRepositoryRoot(Environment.CurrentDirectory); - - VersionControlDetails versionControlDetails = - gitInformation.GetVersionControlDetails(Environment.CurrentDirectory, crawlParentDirectories: true); - - // We don't check for "microsoft/sarif-sdk" so that the test will pass in forks of the - // original repo. - versionControlDetails.RepositoryUri.OriginalString - .Should().StartWith("https://github.com/") - .And.EndWith("/sarif-sdk"); - - versionControlDetails.MappedTo.Uri.OriginalString.Should().Be(localRepoRoot); - versionControlDetails.MappedTo.UriBaseId.Should().BeNull(); - } - } -} diff --git a/src/Test.UnitTests.Sarif/GitHelperTests.cs b/src/Test.UnitTests.Sarif/GitHelperTests.cs new file mode 100644 index 000000000..fe02d3a63 --- /dev/null +++ b/src/Test.UnitTests.Sarif/GitHelperTests.cs @@ -0,0 +1,121 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using FluentAssertions; +using Moq; +using Xunit; + +namespace Microsoft.CodeAnalysis.Sarif +{ + public class GitHelperTests + { + [Fact] + public void GetRepositoryRoot_WhenDotGitIsAbsent_ReturnsNull() + { + var mockFileSystem = new Mock(); + mockFileSystem.Setup(x => x.DirectoryExists(It.IsAny())).Returns(false); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src\Sarif")).Returns(true); + + var gitHelper = new GitHelper(mockFileSystem.Object); + + gitHelper.GetRepositoryRoot(@"C:\dev\sarif-sdk\src\Sarif").Should().BeNull(); + } + + [Fact] + public void GetRepositoryRoot_WhenDotGitIsPresent_ReturnsTheDirectortyContainingDotGit() + { + var mockFileSystem = new Mock(); + mockFileSystem.Setup(x => x.DirectoryExists(It.IsAny())).Returns(false); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\.git")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src\Sarif")).Returns(true); + + var gitHelper = new GitHelper(mockFileSystem.Object); + + gitHelper.GetRepositoryRoot(@"C:\dev\sarif-sdk\src\Sarif").Should().Be(@"C:\dev\sarif-sdk\"); + } + + [Fact] + public void GetRepositoryRoot_ByDefault_PopulatesTheDirectoryToRepoRootCache() + { + var mockFileSystem = new Mock(); + + mockFileSystem.Setup(x => x.DirectoryExists(It.IsAny())).Returns(false); + + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\.git")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src\Sarif")).Returns(true); + + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\docs")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\docs\public")).Returns(true); + + var gitHelper = new GitHelper(mockFileSystem.Object); + + string topLevelDirectoryRoot = gitHelper.GetRepositoryRoot(@"C:\dev\sarif-sdk"); + string topLevelDirectoryRootAgain = gitHelper.GetRepositoryRoot(@"C:\dev\sarif-sdk"); + string subdirectoryRoot = gitHelper.GetRepositoryRoot(@"C:\dev\sarif-sdk\src\Sarif"); + string nonSourceControlledRoot = gitHelper.GetRepositoryRoot(@"C:\docs\public"); + + // Verify that the API returns the correct results whether or not the cache is in use. + topLevelDirectoryRoot.Should().Be(@"C:\dev\sarif-sdk\"); + topLevelDirectoryRootAgain.Should().Be(topLevelDirectoryRoot); + subdirectoryRoot.Should().Be(topLevelDirectoryRoot); + nonSourceControlledRoot.Should().BeNull(); + + gitHelper.directoryToRepoRootPathDictionary.Count.Should().Be(3); + gitHelper.directoryToRepoRootPathDictionary[@"C:\dev\sarif-sdk\src\Sarif"].Should().Be(@"C:\dev\sarif-sdk\"); + gitHelper.directoryToRepoRootPathDictionary[@"C:\dev\sarif-sdk"].Should().Be(@"C:\dev\sarif-sdk\"); + gitHelper.directoryToRepoRootPathDictionary[@"C:\docs\public"].Should().BeNull(); + } + + [Fact] + public void GetRepositoryRoot_WhenCachingIsDisabled_DoesNotPopulateTheDirectoryToRepoRootCache() + { + var mockFileSystem = new Mock(); + + mockFileSystem.Setup(x => x.DirectoryExists(It.IsAny())).Returns(false); + + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\.git")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src\Sarif")).Returns(true); + + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\docs")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@"C:\docs\public")).Returns(true); + + var gitHelper = new GitHelper(mockFileSystem.Object); + + // Verify that the API returns the correct results whether or not the cache is in use. + string topLevelDirectoryRoot = gitHelper.GetRepositoryRoot(@"C:\dev\sarif-sdk", useCache: false); + string topLevelDirectoryRootAgain = gitHelper.GetRepositoryRoot(@"C:\dev\sarif-sdk", useCache: false); + string subdirectoryRoot = gitHelper.GetRepositoryRoot(@"C:\dev\sarif-sdk\src\Sarif", useCache: false); + string nonSourceControlledRoot = gitHelper.GetRepositoryRoot(@"C:\docs\public", useCache: false); + + gitHelper.directoryToRepoRootPathDictionary.Count.Should().Be(0); + } + + [Fact] + public void GetRepositoryRoot_WhenCalledOnTheDefaultInstanceWithNoParameters_Throws() + { + Action action = () => GitHelper.Default.GetRepositoryRoot(@"C:\dev"); + + action.Should().Throw().WithMessage($"{SdkResources.GitHelperDefaultInstanceDoesNotPermitCaching}*"); + } + + [Fact] + public void GetRepositoryRoot_WhenCalledOnTheDefaultInstanceWithCachingDisabled_DoesNotThrow() + { + Action action = () => GitHelper.Default.GetRepositoryRoot(@"C:\dev", useCache: false); + + action.Should().NotThrow(); + } + } +} diff --git a/src/Test.UnitTests.Sarif/GitInformationTests.cs b/src/Test.UnitTests.Sarif/GitInformationTests.cs deleted file mode 100644 index 576f4a809..000000000 --- a/src/Test.UnitTests.Sarif/GitInformationTests.cs +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using FluentAssertions; -using Moq; -using Xunit; - -namespace Microsoft.CodeAnalysis.Sarif -{ - public class GitInformationTests - { - [Fact] - public void GetRepositoryRoot_WhenDotGitIsAbsent_ReturnsNull() - { - var mockFileSystem = new Mock(); - mockFileSystem.Setup(x => x.DirectoryExists(It.IsAny())).Returns(false); - mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev")).Returns(true); - mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk")).Returns(true); - mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src")).Returns(true); - mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src\Sarif")).Returns(true); - - var gitInformation = new GitInformation(mockFileSystem.Object); - - gitInformation.GetRepositoryRoot(@"C:\dev\sarif-sdk\src\Sarif").Should().BeNull(); - } - - [Fact] - public void GetRepositoryRoot_WhenDotGitIsPresent_ReturnsTheDirectortyContainingDotGit() - { - var mockFileSystem = new Mock(); - mockFileSystem.Setup(x => x.DirectoryExists(It.IsAny())).Returns(false); - mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk")).Returns(true); - mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\.git")).Returns(true); - mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src")).Returns(true); - mockFileSystem.Setup(x => x.DirectoryExists(@"C:\dev\sarif-sdk\src\Sarif")).Returns(true); - - var gitInformation = new GitInformation(mockFileSystem.Object); - - gitInformation.GetRepositoryRoot(@"C:\dev\sarif-sdk\src\Sarif").Should().Be(@"C:\dev\sarif-sdk"); - } - } -} diff --git a/src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_All.sarif b/src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_All.sarif index 51571765b..777cd34a0 100644 --- a/src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_All.sarif +++ b/src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_All.sarif @@ -68,14 +68,16 @@ { "repositoryUri": "https://REMOTE_URI", "mappedTo": { - "uri": "file:///REPLACED_AT_TEST_RUNTIME", - "index": 1 + "uri": "file:///REPLACED_AT_TEST_RUNTIME" } } ], "originalUriBaseIds": { "TESTROOT": { "uri": "file:///REPLACED_AT_TEST_RUNTIME/src/Test.UnitTests.Sarif/TestData/" + }, + "REPO_ROOT": { + "uri": "file:///REPLACED_AT_TEST_RUNTIME" } }, "artifacts": [ @@ -92,11 +94,6 @@ "sha-1": "91655EA8262D81C262A8687E9667AEFF7432906A", "sha-256": "1BDE85DC91168DAD541E776BB0437AC8A22D2959351A0640F2757D72AEE60C8A" } - }, - { - "location": { - "uri": "file:///REPLACED_AT_TEST_RUNTIME" - } } ], "results": [ diff --git a/src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_VersionControlInformation.sarif b/src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_VersionControlInformation.sarif index 1a046ab30..aa8c206c4 100644 --- a/src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_VersionControlInformation.sarif +++ b/src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_VersionControlInformation.sarif @@ -73,6 +73,9 @@ "originalUriBaseIds": { "TESTROOT": { "uri": "file:///REPLACED_AT_TEST_RUNTIME/src/Test.UnitTests.Sarif/TestData/" + }, + "REPO_ROOT": { + "uri": "file:///REPLACED_AT_TEST_RUNTIME" } }, "artifacts": [ diff --git a/src/Test.UnitTests.Sarif/Visitors/InsertOptionalDataVisitorTests.cs b/src/Test.UnitTests.Sarif/Visitors/InsertOptionalDataVisitorTests.cs index 330122772..265f31c68 100644 --- a/src/Test.UnitTests.Sarif/Visitors/InsertOptionalDataVisitorTests.cs +++ b/src/Test.UnitTests.Sarif/Visitors/InsertOptionalDataVisitorTests.cs @@ -10,6 +10,7 @@ using Microsoft.CodeAnalysis.Sarif.Writers; using Microsoft.CodeAnalysis.Test.Utilities.Sarif; +using Moq; using Newtonsoft.Json; using Xunit; using Xunit.Abstractions; @@ -46,6 +47,7 @@ protected override string ConstructTestOutputFromInputResource(string inputResou string enlistmentRoot = currentDirectory .Substring(0, currentDirectory.IndexOf(@"\bld\")) .Replace('\\', '/'); + if (!enlistmentRoot.EndsWith("/")) { enlistmentRoot += "/"; } if (inputResourceName == "Inputs.CoreTests-Relative.sarif") { @@ -59,8 +61,20 @@ protected override string ConstructTestOutputFromInputResource(string inputResou var visitor = new InsertOptionalDataVisitor(_currentOptionallyEmittedData); visitor.Visit(actualLog.Runs[0]); - // Restore the remanufactured URI so that file diffing matches + // Restore the remanufactured URI so that file diffing succeeds. actualLog.Runs[0].OriginalUriBaseIds["TESTROOT"] = new ArtifactLocation { Uri = originalUri }; + + // In some of the tests, the visitor added an originalUriBaseId for the repo root. + // Adjust that one, too. + string repoRootUriBaseId = InsertOptionalDataVisitor.GetUriBaseId(0); + if (actualLog.Runs[0].OriginalUriBaseIds.TryGetValue(repoRootUriBaseId, out ArtifactLocation artifactLocation)) + { + Uri repoRootUri = artifactLocation.Uri; + string repoRootString = repoRootUri.ToString(); + repoRootString = repoRootString.Replace(enlistmentRoot, EnlistmentRoot); + + actualLog.Runs[0].OriginalUriBaseIds[repoRootUriBaseId] = new ArtifactLocation { Uri = new Uri(repoRootString, UriKind.Absolute) }; + } } else if (inputResourceName == "Inputs.CoreTests-Absolute.sarif") { @@ -116,8 +130,8 @@ protected override string ConstructTestOutputFromInputResource(string inputResou } // Verify and replace the remote repo URI, because it would be different in a fork. - var gitInformation = new GitInformation(); - Uri remoteUri = gitInformation.GetRemoteUri(enlistmentRoot); + var gitHelper = new GitHelper(); + Uri remoteUri = gitHelper.GetRemoteUri(enlistmentRoot); versionControlDetails.RepositoryUri.Should().Be(remoteUri); versionControlDetails.RepositoryUri = new Uri("https://REMOTE_URI"); @@ -224,6 +238,117 @@ public void InsertOptionalDataVisitor_ContextRegionSnippets_DoesNotFail_TopLevel OptionallyEmittedData.ContextRegionSnippets); } + [Fact] + public void InsertOptionalDataVisitor_SkipsExistingRepoRootSymbolsAndHandlesMultipleRoots() + { + const string ParentRepoRoot = @"C:\repo\"; + const string ParentRepoBranch = "users/mary/my-feature"; + const string ParentRepoCommit = "11111"; + + const string SubmoduleRepoRoot = @"C:\repo\submodule\"; + const string SubmoduleBranch = "main"; + const string SubmoduleCommit = "22222"; + + var mockFileSystem = new Mock(); + + mockFileSystem.Setup(x => x.DirectoryExists(It.IsAny())).Returns(false); + mockFileSystem.Setup(x => x.FileExists(It.IsAny())).Returns(false); + + mockFileSystem.Setup(x => x.DirectoryExists(@$"{ParentRepoRoot}.git")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@$"C:\repo")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@$"{ParentRepoRoot}src")).Returns(true); + mockFileSystem.Setup(x => x.FileExists(@$"{ParentRepoRoot}src\File.cs")).Returns(true); + + mockFileSystem.Setup(x => x.DirectoryExists($@"{SubmoduleRepoRoot}.git")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@$"C:\repo\submodule")).Returns(true); + mockFileSystem.Setup(x => x.DirectoryExists(@$"{SubmoduleRepoRoot}src")).Returns(true); + mockFileSystem.Setup(x => x.FileExists(@$"{SubmoduleRepoRoot}src\File2.cs")).Returns(true); + + mockFileSystem.Setup(x => x.FileExists(GitHelper.s_expectedGitExePath)).Returns(true); + + var mockProcessRunner = new Mock(); + + mockProcessRunner.Setup(x => x(ParentRepoRoot, GitHelper.s_expectedGitExePath, "remote get-url origin")).Returns(ParentRepoRoot); + mockProcessRunner.Setup(x => x(ParentRepoRoot, GitHelper.s_expectedGitExePath, "rev-parse --abbrev-ref HEAD")).Returns(ParentRepoBranch); + mockProcessRunner.Setup(x => x(ParentRepoRoot, GitHelper.s_expectedGitExePath, "rev-parse --verify HEAD")).Returns(ParentRepoCommit); + + mockProcessRunner.Setup(x => x(SubmoduleRepoRoot, GitHelper.s_expectedGitExePath, "remote get-url origin")).Returns(SubmoduleRepoRoot); + mockProcessRunner.Setup(x => x(SubmoduleRepoRoot, GitHelper.s_expectedGitExePath, "rev-parse --abbrev-ref HEAD")).Returns(SubmoduleBranch); + mockProcessRunner.Setup(x => x(SubmoduleRepoRoot, GitHelper.s_expectedGitExePath, "rev-parse --verify HEAD")).Returns(SubmoduleCommit); + + var run = new Run + { + OriginalUriBaseIds = new Dictionary + { + // Called "REPO_ROOT" but doesn't actually point to a repo. + ["REPO_ROOT"] = new ArtifactLocation + { + Uri = new Uri(@"C:\dir1\dir2", UriKind.Absolute) + } + }, + Results = new List + { + new Result + { + Locations = new List + { + new Location + { + PhysicalLocation = new PhysicalLocation + { + ArtifactLocation = new ArtifactLocation + { + // The visitor will encounter this file and notice that it + // is under a repo root. It will invent a uriBaseId symbol + // for this repo. Since REPO_ROOT is already taken, it will + // choose REPO_ROOT_2 + Uri = new Uri(@$"{ParentRepoRoot}src\File.cs", UriKind.Absolute) + } + } + }, + new Location + { + PhysicalLocation = new PhysicalLocation + { + ArtifactLocation = new ArtifactLocation + { + Uri = new Uri(@$"{SubmoduleRepoRoot}src\File2.cs", UriKind.Absolute) + } + } + } + } + } + } + }; + + var visitor = new InsertOptionalDataVisitor( + OptionallyEmittedData.VersionControlInformation, + originalUriBaseIds: null, + fileSystem: mockFileSystem.Object, + processRunner: mockProcessRunner.Object); + visitor.Visit(run); + + run.VersionControlProvenance[0].MappedTo.Uri.LocalPath.Should().Be(ParentRepoRoot); + run.VersionControlProvenance[0].Branch.Should().Be(ParentRepoBranch); + run.VersionControlProvenance[0].RevisionId.Should().Be(ParentRepoCommit); + + run.VersionControlProvenance[1].MappedTo.Uri.LocalPath.Should().Be(SubmoduleRepoRoot); + run.VersionControlProvenance[1].Branch.Should().Be(SubmoduleBranch); + run.VersionControlProvenance[1].RevisionId.Should().Be(SubmoduleCommit); + + run.OriginalUriBaseIds["REPO_ROOT_2"].Uri.LocalPath.Should().Be(ParentRepoRoot); + + IList resultLocations = run.Results[0].Locations; + + ArtifactLocation resultArtifactLocation = resultLocations[0].PhysicalLocation.ArtifactLocation; + resultArtifactLocation.Uri.OriginalString.Should().Be("src/File.cs"); + resultArtifactLocation.UriBaseId.Should().Be("REPO_ROOT_2"); + + resultArtifactLocation = resultLocations[1].PhysicalLocation.ArtifactLocation; + resultArtifactLocation.Uri.OriginalString.Should().Be("src/File2.cs"); + resultArtifactLocation.UriBaseId.Should().Be("REPO_ROOT_3"); + } + [Fact] public void InsertOptionalDataVisitor_CanVisitIndividualResultsInASuppliedRun() { @@ -543,11 +668,10 @@ public void InsertOptionalDataVisitor_ResolvesOriginalUriBaseIds() string inputFileName = "InsertOptionalDataVisitor.txt"; string testDirectory = GetTestDirectory("InsertOptionalDataVisitor") + @"\"; string uriBaseId = "TEST_DIR"; - string fileKey = "#" + uriBaseId + "#" + inputFileName; IDictionary originalUriBaseIds = new Dictionary { { uriBaseId, new ArtifactLocation { Uri = new Uri(testDirectory, UriKind.Absolute) } } }; - Run run = new Run() + var run = new Run() { DefaultEncoding = "UTF-8", OriginalUriBaseIds = null,