Address review comments

Author: hvitved

csharp/extractor/Semmle.Extraction.CSharp.DependencyStubGenerator/Program.cs

@@ -2,7 +2,7 @@
 using Semmle.Extraction.CSharp.StubGenerator;
 using Semmle.Util.Logging;
 
-var logger = new ConsoleLogger(Verbosity.Info);
+var logger = new ConsoleLogger(Verbosity.Info, logThreadId: false);
 using var dependencyManager = new DependencyManager(".", DependencyOptions.Default, logger);
 StubGenerator.GenerateStubs(logger, dependencyManager.ReferenceFiles, "codeql_csharp_stubs");
 

csharp/extractor/Semmle.Extraction.CSharp.StubGenerator/StubGenerator.cs

@@ -41,10 +41,10 @@ public static void GenerateStubs(ILogger logger, IEnumerable<string> referencesP
             references.Select(tuple => tuple.Item1),
             new CSharpCompilationOptions(OutputKind.ConsoleApplication, allowUnsafe: true));
 
-        var referenceStubTasks = references.Select(@ref => (Action)(() => StubReference(compilation, outputPath, @ref.Reference, @ref.Path)));
-        Parallel.Invoke(
-            new ParallelOptions { MaxDegreeOfParallelism = threads },
-            referenceStubTasks.ToArray());
+        Parallel.ForEach(references, new ParallelOptions { MaxDegreeOfParallelism = threads }, @ref =>
+        {
+            StubReference(logger, compilation, outputPath, @ref.Reference, @ref.Path);
+        });
 
         stopWatch.Stop();
         logger.Log(Severity.Info, $"Stub generation took {stopWatch.Elapsed}.");
@@ -59,25 +59,24 @@ private static IEnumerable<Action> GetResolvedReferenceTasks(IEnumerable<string>
         });
     }
 
-    private static void StubReference(CSharpCompilation compilation, string outputPath, MetadataReference reference, string path)
+    private static void StubReference(ILogger logger, CSharpCompilation compilation, string outputPath, MetadataReference reference, string path)
     {
-        if (compilation.GetAssemblyOrModuleSymbol(reference) is IAssemblySymbol assembly)
-        {
-            var logger = new ConsoleLogger(Verbosity.Info);
-            using var fileStream = new FileStream(FileUtils.NestPaths(logger, outputPath, path.Replace(".dll", ".cs")), FileMode.Create, FileAccess.Write);
-            using var writer = new StreamWriter(fileStream, new UTF8Encoding(false));
+        if (compilation.GetAssemblyOrModuleSymbol(reference) is not IAssemblySymbol assembly)
+            return;
+
+        using var fileStream = new FileStream(FileUtils.NestPaths(logger, outputPath, path.Replace(".dll", ".cs")), FileMode.Create, FileAccess.Write);
+        using var writer = new StreamWriter(fileStream, new UTF8Encoding(false));
 
-            writer.WriteLine("// This file contains auto-generated code.");
-            writer.WriteLine($"// Generated from `{assembly.Identity}`.");
+        writer.WriteLine("// This file contains auto-generated code.");
+        writer.WriteLine($"// Generated from `{assembly.Identity}`.");
 
-            var visitor = new StubVisitor(assembly, writer);
+        var visitor = new StubVisitor(assembly, writer);
 
-            visitor.StubAttributes(assembly.GetAttributes(), "assembly: ");
+        visitor.StubAttributes(assembly.GetAttributes(), "assembly: ");
 
-            foreach (var module in assembly.Modules)
-            {
-                module.GlobalNamespace.Accept(new StubVisitor(assembly, writer));
-            }
+        foreach (var module in assembly.Modules)
+        {
+            module.GlobalNamespace.Accept(visitor);
         }
     }
 }

csharp/extractor/Semmle.Extraction.CSharp.StubGenerator/StubVisitor.cs

@@ -36,7 +36,7 @@ private bool IsRelevantNamedType(INamedTypeSymbol symbol) =>
         IsRelevantBaseType(symbol) &&
         SymbolEqualityComparer.Default.Equals(symbol.ContainingAssembly, assembly);
 
-    private bool IsRelevantNamespace(INamespaceSymbol symbol) => isRelevantNamespace[symbol];
+    private bool IsRelevantNamespace(INamespaceSymbol symbol) => isRelevantNamespace.Invoke(symbol);
 
     private void StubExplicitInterface(ISymbol symbol, ISymbol? explicitInterfaceSymbol, bool writeName = true)
     {
@@ -109,7 +109,7 @@ private void StubAccessibility(Accessibility accessibility)
             case Accessibility.Internal:
                 stubWriter.Write("internal ");
                 break;
-            case Accessibility.ProtectedAndInternal or Accessibility.ProtectedOrInternal:
+            case Accessibility.ProtectedAndInternal:
                 stubWriter.Write("protected internal ");
                 break;
             default:
@@ -739,7 +739,7 @@ public override void VisitNamedType(INamedTypeSymbol symbol)
         else
         {
             var seenCtor = false;
-            foreach (var childSymbol in symbol.GetMembers())
+            foreach (var childSymbol in symbol.GetMembers().OrderBy(m => m.GetName()))
             {
                 seenCtor |= childSymbol is IMethodSymbol method && method.MethodKind == MethodKind.Constructor;
                 childSymbol.Accept(this);
@@ -768,7 +768,7 @@ public override void VisitNamespace(INamespaceSymbol symbol)
         if (!isGlobal)
             stubWriter.WriteLine($"namespace {symbol.Name} {{");
 
-        foreach (var childSymbol in symbol.GetMembers())
+        foreach (var childSymbol in symbol.GetMembers().OrderBy(m => m.GetName()))
         {
             childSymbol.Accept(this);
         }

csharp/extractor/Semmle.Util/MemoizedFunc.cs

@@ -14,17 +14,14 @@ public MemoizedFunc(Func<T1, T2> f)
         this.f = f;
     }
 
-    public T2 this[T1 s]
+    public T2 Invoke(T1 s)
     {
-        get
+        if (!cache.TryGetValue(s, out var t))
         {
-            if (!cache.TryGetValue(s, out var t))
-            {
-                t = f(s);
-                cache[s] = t;
-            }
-            return t;
+            t = f(s);
+            cache[s] = t;
         }
+        return t;
     }
 }
 
@@ -38,11 +35,5 @@ public ConcurrentMemoizedFunc(Func<T1, T2> f)
         this.f = f;
     }
 
-    public T2 this[T1 s]
-    {
-        get
-        {
-            return cache.GetOrAdd(s, f);
-        }
-    }
+    public T2 Invoke(T1 s) => cache.GetOrAdd(s, f);
 }