Code quality improvements

Author: tamasvajk

csharp/extractor/Semmle.Extraction.CSharp.Standalone/AssemblyCache.cs

@@ -16,12 +16,12 @@ internal class AssemblyCache
         /// Locate all reference files and index them.
         /// </summary>
         /// <param name="dirs">Directories to search.</param>
-        /// <param name="progress">Callback for progress.</param>
-        public AssemblyCache(IEnumerable<string> dirs, IProgressMonitor progress)
+        /// <param name="progressMonitor">Callback for progress.</param>
+        public AssemblyCache(IEnumerable<string> dirs, ProgressMonitor progressMonitor)
         {
             foreach (var dir in dirs)
             {
-                progress.FindingFiles(dir);
+                progressMonitor.FindingFiles(dir);
                 AddReferenceDirectory(dir);
             }
             IndexReferences();

csharp/extractor/Semmle.Extraction.CSharp.Standalone/BuildAnalysis.cs

@@ -11,117 +11,87 @@
 
 namespace Semmle.BuildAnalyser
 {
-    /// <summary>
-    /// The output of a build analysis.
-    /// </summary>
-    internal interface IBuildAnalysis
-    {
-        /// <summary>
-        /// Full filepaths of external references.
-        /// </summary>
-        IEnumerable<string> ReferenceFiles { get; }
-
-        /// <summary>
-        /// Full filepaths of C# source files from project files.
-        /// </summary>
-        IEnumerable<string> ProjectSourceFiles { get; }
-
-        /// <summary>
-        /// Full filepaths of C# source files in the filesystem.
-        /// </summary>
-        IEnumerable<string> AllSourceFiles { get; }
-
-        /// <summary>
-        /// The assembly IDs which could not be resolved.
-        /// </summary>
-        IEnumerable<string> UnresolvedReferences { get; }
-
-        /// <summary>
-        /// List of source files referenced by projects but
-        /// which were not found in the filesystem.
-        /// </summary>
-        IEnumerable<string> MissingSourceFiles { get; }
-    }
-
     /// <summary>
     /// Main implementation of the build analysis.
     /// </summary>
-    internal sealed class BuildAnalysis : IBuildAnalysis, IDisposable
+    internal sealed class BuildAnalysis : IDisposable
     {
         private readonly AssemblyCache assemblyCache;
-        private readonly IProgressMonitor progressMonitor;
+        private readonly ProgressMonitor progressMonitor;
         private readonly IDictionary<string, bool> usedReferences = new ConcurrentDictionary<string, bool>();
         private readonly IDictionary<string, bool> sources = new ConcurrentDictionary<string, bool>();
         private readonly IDictionary<string, string> unresolvedReferences = new ConcurrentDictionary<string, string>();
-        private int failedProjects, succeededProjects;
+        private int failedProjects;
+        private int succeededProjects;
         private readonly string[] allSources;
         private int conflictedReferences = 0;
+        private readonly Options options;
+        private readonly DirectoryInfo sourceDir;
 
         /// <summary>
         /// Performs a C# build analysis.
         /// </summary>
         /// <param name="options">Analysis options from the command line.</param>
-        /// <param name="progress">Display of analysis progress.</param>
-        public BuildAnalysis(Options options, IProgressMonitor progress)
+        /// <param name="progressMonitor">Display of analysis progress.</param>
+        public BuildAnalysis(Options options, ProgressMonitor progressMonitor)
         {
             var startTime = DateTime.Now;
 
-            progressMonitor = progress;
-            var sourceDir = new DirectoryInfo(options.SrcDir);
+            this.options = options;
+            this.progressMonitor = progressMonitor;
+            this.sourceDir = new DirectoryInfo(options.SrcDir);
 
-            progressMonitor.FindingFiles(options.SrcDir);
+            this.progressMonitor.FindingFiles(options.SrcDir);
 
-            allSources = sourceDir.GetFiles("*.cs", SearchOption.AllDirectories)
-                .Select(d => d.FullName)
-                .Where(d => !options.ExcludesFile(d))
-                .ToArray();
+            this.allSources = GetFiles("*.cs").ToArray();
+
+            var solutions = options.SolutionFile is not null
+                ? new[] { options.SolutionFile }
+                : GetFiles("*.sln");
 
             var dllDirNames = options.DllDirs.Select(Path.GetFullPath).ToList();
+
+            // Find DLLs in the .Net Framework
+            if (options.ScanNetFrameworkDlls)
+            {
+                var runtimeLocation = Runtime.GetRuntime(options.UseSelfContainedDotnet);
+                progressMonitor.Log(Util.Logging.Severity.Debug, $"Runtime location selected: {runtimeLocation}");
+                dllDirNames.Add(runtimeLocation);
+            }
+
+            if (options.UseMscorlib)
+            {
+                UseReference(typeof(object).Assembly.Location);
+            }
+
             packageDirectory = new TemporaryDirectory(ComputeTempDirectory(sourceDir.FullName));
 
             if (options.UseNuGet)
             {
+                dllDirNames.Add(packageDirectory.DirInfo.FullName);
                 try
                 {
-                    var nuget = new NugetPackages(sourceDir.FullName, packageDirectory);
-                    nuget.InstallPackages(progressMonitor);
+                    var nuget = new NugetPackages(sourceDir.FullName, packageDirectory, progressMonitor);
+                    nuget.InstallPackages();
                 }
                 catch (FileNotFoundException)
                 {
                     progressMonitor.MissingNuGet();
                 }
-            }
-
-            // Find DLLs in the .Net Framework
-            if (options.ScanNetFrameworkDlls)
-            {
-                var runtimeLocation = Runtime.GetRuntime(options.UseSelfContainedDotnet);
-                progressMonitor.Log(Util.Logging.Severity.Debug, $"Runtime location selected: {runtimeLocation}");
-                dllDirNames.Add(runtimeLocation);
-            }
-
-            // TODO: remove the below when the required SDK is installed
-            using (new FileRenamer(sourceDir.GetFiles("global.json", SearchOption.AllDirectories)))
-            {
-                var solutions = options.SolutionFile is not null ?
-                        new[] { options.SolutionFile } :
-                        sourceDir.GetFiles("*.sln", SearchOption.AllDirectories).Select(d => d.FullName);
 
-                if (options.UseNuGet)
+                // TODO: remove the below when the required SDK is installed
+                using (new FileRenamer(sourceDir.GetFiles("global.json", SearchOption.AllDirectories)))
                 {
                     RestoreSolutions(solutions);
                 }
-                dllDirNames.Add(packageDirectory.DirInfo.FullName);
-                assemblyCache = new BuildAnalyser.AssemblyCache(dllDirNames, progress);
-                AnalyseSolutions(solutions);
-
-                foreach (var filename in assemblyCache.AllAssemblies.Select(a => a.Filename))
-                    UseReference(filename);
             }
 
-            if (options.UseMscorlib)
+            assemblyCache = new AssemblyCache(dllDirNames, progressMonitor);
+            AnalyseSolutions(solutions);
+
+            foreach (var filename in assemblyCache.AllAssemblies.Select(a => a.Filename))
             {
-                UseReference(typeof(object).Assembly.Location);
+                UseReference(filename);
             }
 
             ResolveConflicts();
@@ -149,6 +119,13 @@ public BuildAnalysis(Options options, IProgressMonitor progress)
                 DateTime.Now - startTime);
         }
 
+        private IEnumerable<string> GetFiles(string pattern)
+        {
+            return sourceDir.GetFiles(pattern, SearchOption.AllDirectories)
+                .Select(d => d.FullName)
+                .Where(d => !options.ExcludesFile(d));
+        }
+
         /// <summary>
         /// Computes a unique temp directory for the packages associated
         /// with this source tree. Use a SHA1 of the directory name.
@@ -158,9 +135,7 @@ public BuildAnalysis(Options options, IProgressMonitor progress)
         private static string ComputeTempDirectory(string srcDir)
         {
             var bytes = Encoding.Unicode.GetBytes(srcDir);
-
-            using var sha1 = SHA1.Create();
-            var sha = sha1.ComputeHash(bytes);
+            var sha = SHA1.HashData(bytes);
             var sb = new StringBuilder();
             foreach (var b in sha.Take(8))
                 sb.AppendFormat("{0:x2}", b);
@@ -195,12 +170,15 @@ private void ResolveConflicts()
 
             // Pick the highest version for each assembly name
             foreach (var r in sortedReferences)
+            {
                 finalAssemblyList[r.Name] = r;
-
+            }
             // Update the used references list
             usedReferences.Clear();
             foreach (var r in finalAssemblyList.Select(r => r.Value.Filename))
+            {
                 UseReference(r);
+            }
 
             // Report the results
             foreach (var r in sortedReferences)
@@ -278,7 +256,9 @@ private void UnresolvedReference(string id, string projectFile)
         private void AnalyseProjectFiles(IEnumerable<FileInfo> projectFiles)
         {
             foreach (var proj in projectFiles)
+            {
                 AnalyseProject(proj);
+            }
         }
 
         private void AnalyseProject(FileInfo project)
@@ -348,12 +328,12 @@ private void Restore(string projectOrSolution)
             }
         }
 
-        public void RestoreSolutions(IEnumerable<string> solutions)
+        private void RestoreSolutions(IEnumerable<string> solutions)
         {
             Parallel.ForEach(solutions, new ParallelOptions { MaxDegreeOfParallelism = 4 }, Restore);
         }
 
-        public void AnalyseSolutions(IEnumerable<string> solutions)
+        private void AnalyseSolutions(IEnumerable<string> solutions)
         {
             Parallel.ForEach(solutions, new ParallelOptions { MaxDegreeOfParallelism = 4 }, solutionFile =>
             {

csharp/extractor/Semmle.Extraction.CSharp.Standalone/NugetPackages.cs

@@ -17,26 +17,24 @@ internal class NugetPackages
         /// <summary>
         /// Create the package manager for a specified source tree.
         /// </summary>
-        /// <param name="sourceDir">The source directory.</param>
-        public NugetPackages(string sourceDir, TemporaryDirectory packageDirectory)
+        public NugetPackages(string sourceDir, TemporaryDirectory packageDirectory, ProgressMonitor progressMonitor)
         {
             SourceDirectory = sourceDir;
             PackageDirectory = packageDirectory;
+            this.progressMonitor = progressMonitor;
 
             // Expect nuget.exe to be in a `nuget` directory under the directory containing this exe.
             var currentAssembly = System.Reflection.Assembly.GetExecutingAssembly().Location;
-            var directory = Path.GetDirectoryName(currentAssembly);
-            if (directory is null)
-                throw new FileNotFoundException($"Directory path '{currentAssembly}' of current assembly is null");
-
+            var directory = Path.GetDirectoryName(currentAssembly)
+                ?? throw new FileNotFoundException($"Directory path '{currentAssembly}' of current assembly is null");
             nugetExe = Path.Combine(directory, "nuget", "nuget.exe");
 
             if (!File.Exists(nugetExe))
                 throw new FileNotFoundException(string.Format("NuGet could not be found at {0}", nugetExe));
 
-            packages = new DirectoryInfo(SourceDirectory).
-                EnumerateFiles("packages.config", SearchOption.AllDirectories).
-                ToArray();
+            packages = new DirectoryInfo(SourceDirectory)
+                .EnumerateFiles("packages.config", SearchOption.AllDirectories)
+                .ToArray();
         }
 
         // List of package files to download.
@@ -51,11 +49,11 @@ public NugetPackages(string sourceDir, TemporaryDirectory packageDirectory)
         /// Download the packages to the temp folder.
         /// </summary>
         /// <param name="pm">The progress monitor used for reporting errors etc.</param>
-        public void InstallPackages(IProgressMonitor pm)
+        public void InstallPackages()
         {
             foreach (var package in packages)
             {
-                RestoreNugetPackage(package.FullName, pm);
+                RestoreNugetPackage(package.FullName);
             }
         }
 
@@ -80,9 +78,9 @@ public string SourceDirectory
         /// </summary>
         /// <param name="package">The package file.</param>
         /// <param name="pm">Where to log progress/errors.</param>
-        private void RestoreNugetPackage(string package, IProgressMonitor pm)
+        private void RestoreNugetPackage(string package)
         {
-            pm.NugetInstall(package);
+            progressMonitor.NugetInstall(package);
 
             /* Use nuget.exe to install a package.
              * Note that there is a clutch of NuGet assemblies which could be used to
@@ -115,7 +113,7 @@ private void RestoreNugetPackage(string package, IProgressMonitor pm)
 
                 if (p is null)
                 {
-                    pm.FailedNugetCommand(pi.FileName, pi.Arguments, "Couldn't start process.");
+                    progressMonitor.FailedNugetCommand(pi.FileName, pi.Arguments, "Couldn't start process.");
                     return;
                 }
 
@@ -125,16 +123,17 @@ private void RestoreNugetPackage(string package, IProgressMonitor pm)
                 p.WaitForExit();
                 if (p.ExitCode != 0)
                 {
-                    pm.FailedNugetCommand(pi.FileName, pi.Arguments, output + error);
+                    progressMonitor.FailedNugetCommand(pi.FileName, pi.Arguments, output + error);
                 }
             }
             catch (Exception ex)
                 when (ex is System.ComponentModel.Win32Exception || ex is FileNotFoundException)
             {
-                pm.FailedNugetCommand(pi.FileName, pi.Arguments, ex.Message);
+                progressMonitor.FailedNugetCommand(pi.FileName, pi.Arguments, ex.Message);
             }
         }
 
         private readonly string nugetExe;
+        private readonly ProgressMonitor progressMonitor;
     }
 }

csharp/extractor/Semmle.Extraction.CSharp.Standalone/ProgressMonitor.cs

@@ -3,27 +3,7 @@
 
 namespace Semmle.BuildAnalyser
 {
-    /// <summary>
-    /// Callback for various events that may happen during the build analysis.
-    /// </summary>
-    internal interface IProgressMonitor
-    {
-        void FindingFiles(string dir);
-        void UnresolvedReference(string id, string project);
-        void AnalysingSolution(string filename);
-        void FailedProjectFile(string filename, string reason);
-        void FailedNugetCommand(string exe, string args, string message);
-        void NugetInstall(string package);
-        void ResolvedReference(string filename);
-        void Summary(int existingSources, int usedSources, int missingSources, int references, int unresolvedReferences, int resolvedConflicts, int totalProjects, int failedProjects, TimeSpan analysisTime);
-        void Log(Severity severity, string message);
-        void ResolvedConflict(string asm1, string asm2);
-        void MissingProject(string projectFile);
-        void CommandFailed(string exe, string arguments, int exitCode);
-        void MissingNuGet();
-    }
-
-    internal class ProgressMonitor : IProgressMonitor
+    internal class ProgressMonitor
     {
         private readonly ILogger logger;