Merge pull request #15424 from tamasvajk/standalone/logging

C#: Improve log messages in buildless mode + some cleanup/refactoring

Author: tamasvajk

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

@@ -1,6 +1,7 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using Semmle.Util.Logging;
 
 namespace Semmle.Extraction.CSharp.DependencyFetching
 {
@@ -18,25 +19,26 @@ internal class AssemblyCache
         /// Paths to search. Directories are searched recursively. Files are added directly to the
         /// assembly cache.
         /// </param>
-        /// <param name="progressMonitor">Callback for progress.</param>
-        public AssemblyCache(IEnumerable<string> paths, IEnumerable<string> frameworkPaths, ProgressMonitor progressMonitor)
+        /// <param name="logger">Callback for progress.</param>
+        public AssemblyCache(IEnumerable<string> paths, IEnumerable<string> frameworkPaths, ILogger logger)
         {
+            this.logger = logger;
             foreach (var path in paths)
             {
                 if (File.Exists(path))
                 {
-                    pendingDllsToIndex.Enqueue(path);
+                    dllsToIndex.Add(path);
                     continue;
                 }
 
                 if (Directory.Exists(path))
                 {
-                    progressMonitor.FindingFiles(path);
+                    logger.LogInfo($"Finding reference DLLs in {path}...");
                     AddReferenceDirectory(path);
                 }
                 else
                 {
-                    progressMonitor.LogInfo("AssemblyCache: Path not found: " + path);
+                    logger.LogInfo("AssemblyCache: Path not found: " + path);
                 }
             }
             IndexReferences(frameworkPaths);
@@ -52,7 +54,7 @@ private void AddReferenceDirectory(string dir)
         {
             foreach (var dll in new DirectoryInfo(dir).EnumerateFiles("*.dll", SearchOption.AllDirectories))
             {
-                pendingDllsToIndex.Enqueue(dll.FullName);
+                dllsToIndex.Add(dll.FullName);
             }
         }
 
@@ -62,12 +64,16 @@ private void AddReferenceDirectory(string dir)
         /// </summary>
         private void IndexReferences(IEnumerable<string> frameworkPaths)
         {
+            logger.LogInfo($"Indexing {dllsToIndex.Count} assemblies...");
+
             // Read all of the files
-            foreach (var filename in pendingDllsToIndex)
+            foreach (var filename in dllsToIndex)
             {
                 IndexReference(filename);
             }
 
+            logger.LogInfo($"Read {assemblyInfoByFileName.Count} assembly infos");
+
             foreach (var info in assemblyInfoByFileName.Values
                 .OrderBy(info => info.Name)
                 .OrderAssemblyInfosByPreference(frameworkPaths))
@@ -83,25 +89,16 @@ private void IndexReference(string filename)
         {
             try
             {
+                logger.LogDebug($"Reading assembly info from {filename}");
                 var info = AssemblyInfo.ReadFromFile(filename);
                 assemblyInfoByFileName[filename] = info;
             }
             catch (AssemblyLoadException)
             {
-                failedAssemblyInfoFileNames.Add(filename);
+                logger.LogInfo($"Couldn't read assembly info from {filename}");
             }
         }
 
-        /// <summary>
-        /// The number of DLLs which are assemblies.
-        /// </summary>
-        public int AssemblyCount => assemblyInfoByFileName.Count;
-
-        /// <summary>
-        /// The number of DLLs which weren't assemblies. (E.g. C++).
-        /// </summary>
-        public int NonAssemblyCount => failedAssemblyInfoFileNames.Count;
-
         /// <summary>
         /// Given an assembly id, determine its full info.
         /// </summary>
@@ -113,8 +110,7 @@ public AssemblyInfo ResolveReference(string id)
             if (failedAssemblyInfoIds.Contains(id))
                 throw new AssemblyLoadException();
 
-            string assemblyName;
-            (id, assemblyName) = AssemblyInfo.ComputeSanitizedAssemblyInfo(id);
+            (id, var assemblyName) = AssemblyInfo.ComputeSanitizedAssemblyInfo(id);
 
             // Look up the id in our references map.
             if (assemblyInfoById.TryGetValue(id, out var result))
@@ -164,17 +160,15 @@ public AssemblyInfo GetAssemblyInfo(string filepath)
             throw new AssemblyLoadException();
         }
 
-        private readonly Queue<string> pendingDllsToIndex = new Queue<string>();
+        private readonly List<string> dllsToIndex = new List<string>();
 
         private readonly Dictionary<string, AssemblyInfo> assemblyInfoByFileName = new Dictionary<string, AssemblyInfo>();
 
-        // List of DLLs which are not assemblies.
-        // We probably don't need to keep this
-        private readonly List<string> failedAssemblyInfoFileNames = new List<string>();
-
         // Map from assembly id (in various formats) to the full info.
         private readonly Dictionary<string, AssemblyInfo> assemblyInfoById = new Dictionary<string, AssemblyInfo>();
 
         private readonly HashSet<string> failedAssemblyInfoIds = new HashSet<string>();
+
+        private readonly ILogger logger;
     }
 }

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyInfo.cs

@@ -120,21 +120,6 @@ private AssemblyInfo(string filename, string name, Version version, string cultu
             NetCoreVersion = netCoreVersion;
         }
 
-        /// <summary>
-        /// Get AssemblyInfo from a loaded Assembly.
-        /// </summary>
-        /// <param name="assembly">The assembly.</param>
-        /// <returns>Info about the assembly.</returns>
-        public static AssemblyInfo MakeFromAssembly(Assembly assembly)
-        {
-            if (assembly.FullName is null)
-            {
-                throw new InvalidOperationException("Assembly with empty full name is not expected.");
-            }
-
-            return new AssemblyInfo(assembly.FullName, assembly.Location);
-        }
-
         /// <summary>
         /// Returns the id and name of the assembly that would be created from the received id.
         /// </summary>

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/Assets.cs

@@ -5,6 +5,7 @@
 using System.Linq;
 using Newtonsoft.Json.Linq;
 using Semmle.Util;
+using Semmle.Util.Logging;
 
 namespace Semmle.Extraction.CSharp.DependencyFetching
 {
@@ -13,11 +14,11 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
     /// </summary>
     internal class Assets
     {
-        private readonly ProgressMonitor progressMonitor;
+        private readonly ILogger logger;
 
-        internal Assets(ProgressMonitor progressMonitor)
+        internal Assets(ILogger logger)
         {
-            this.progressMonitor = progressMonitor;
+            this.logger = logger;
         }
 
         /// <summary>
@@ -35,7 +36,7 @@ private record class ReferenceInfo(string? Type, Dictionary<string, object>? Com
 
         /// <summary>
         /// Add the package dependencies from the assets file to dependencies.
-        /// 
+        ///
         /// Parse a part of the JSon assets file and add the paths
         /// to the dependencies required for compilation (and collect
         /// information about used packages).
@@ -60,7 +61,7 @@ private record class ReferenceInfo(string? Type, Dictionary<string, object>? Com
         ///     }
         ///   }
         /// }
-        /// 
+        ///
         /// Adds the following dependencies
         ///   Paths: {
         ///     "castle.core/4.4.1/lib/netstandard1.5/Castle.Core.dll",
@@ -85,7 +86,7 @@ private void AddPackageDependencies(JObject json, DependencyContainer dependenci
 
             if (references is null)
             {
-                progressMonitor.LogDebug("No references found in the targets section in the assets file.");
+                logger.LogDebug("No references found in the targets section in the assets file.");
                 return;
             }
 
@@ -157,7 +158,7 @@ private void AddFrameworkDependencies(JObject json, DependencyContainer dependen
 
             if (frameworks is null)
             {
-                progressMonitor.LogDebug("No framework section in assets.json.");
+                logger.LogDebug("No framework section in assets.json.");
                 return;
             }
 
@@ -171,7 +172,7 @@ private void AddFrameworkDependencies(JObject json, DependencyContainer dependen
 
             if (references is null)
             {
-                progressMonitor.LogDebug("No framework references in assets.json.");
+                logger.LogDebug("No framework references in assets.json.");
                 return;
             }
 
@@ -196,12 +197,12 @@ public bool TryParse(string json, DependencyContainer dependencies)
             }
             catch (Exception e)
             {
-                progressMonitor.LogDebug($"Failed to parse assets file (unexpected error): {e.Message}");
+                logger.LogDebug($"Failed to parse assets file (unexpected error): {e.Message}");
                 return false;
             }
         }
 
-        private static bool TryReadAllText(string path, ProgressMonitor progressMonitor, [NotNullWhen(returnValue: true)] out string? content)
+        private static bool TryReadAllText(string path, ILogger logger, [NotNullWhen(returnValue: true)] out string? content)
         {
             try
             {
@@ -210,19 +211,19 @@ private static bool TryReadAllText(string path, ProgressMonitor progressMonitor,
             }
             catch (Exception e)
             {
-                progressMonitor.LogInfo($"Failed to read assets file '{path}': {e.Message}");
+                logger.LogInfo($"Failed to read assets file '{path}': {e.Message}");
                 content = null;
                 return false;
             }
         }
 
-        public static DependencyContainer GetCompilationDependencies(ProgressMonitor progressMonitor, IEnumerable<string> assets)
+        public static DependencyContainer GetCompilationDependencies(ILogger logger, IEnumerable<string> assets)
         {
-            var parser = new Assets(progressMonitor);
+            var parser = new Assets(logger);
             var dependencies = new DependencyContainer();
             assets.ForEach(asset =>
             {
-                if (TryReadAllText(asset, progressMonitor, out var json))
+                if (TryReadAllText(asset, logger, out var json))
                 {
                     parser.TryParse(json, dependencies);
                 }