Fix code review findings

github · Apr 22, 2024 · 05f3c64 · 05f3c64
1 parent 7b5f2c7
commit 05f3c64
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 46 deletions.
diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/Sdk.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/Sdk.cs
@@ -23,7 +23,7 @@ public Sdk(IDotNet dotNet, ILogger logger)
             this.dotNet = dotNet;
             this.logger = logger;
 
-            newestSdkVersion = new Lazy<DotNetVersion?>(GetNewestSdk);
+            newestSdkVersion = new Lazy<DotNetVersion?>(GetNewestSdkVersion);
             cscPath = new Lazy<string?>(GetCscPath);
         }
 
@@ -46,7 +46,7 @@ private static HashSet<DotNetVersion> ParseSdks(IList<string> listed)
             return sdks;
         }
 
-        private DotNetVersion? GetNewestSdk()
+        private DotNetVersion? GetNewestSdkVersion()
         {
             var listed = dotNet.GetListedSdks();
             var sdks = ParseSdks(listed);
@@ -55,14 +55,14 @@ private static HashSet<DotNetVersion> ParseSdks(IList<string> listed)
 
         private string? GetCscPath()
         {
-            var sdk = GetNewestSdk();
-            if (sdk is null)
+            var newestSdkVersion = GetNewestSdkVersion();
+            if (newestSdkVersion is null)
             {
                 logger.LogWarning("No dotnet SDK found.");
                 return null;
             }
 
-            var path = Path.Combine(sdk.FullPath, "Roslyn", "bincore", "csc.dll");
+            var path = Path.Combine(newestSdkVersion.FullPath, "Roslyn", "bincore", "csc.dll");
             logger.LogDebug($"Source generator CSC: '{path}'");
             if (!File.Exists(path))
             {

diff --git a/...Semmle.Extraction.CSharp.DependencyFetching/SourceGenerators/DotnetSourceGeneratorBase.cs b/...Semmle.Extraction.CSharp.DependencyFetching/SourceGenerators/DotnetSourceGeneratorBase.cs
@@ -73,13 +73,7 @@ protected override IEnumerable<string> Run()
                     continue;
                 }
 
-                if (!groupedFiles.TryGetValue(project.File, out var files))
-                {
-                    files = [];
-                    groupedFiles[project.File] = files;
-                }
-
-                files.Add(additionalFile);
+                groupedFiles.AddAnother(project.File, additionalFile);
             }
 
             try

diff --git a/...ncyFetching/SourceGenerators/DotnetSourceGeneratorWrapper/DotnetSourceGeneratorWrapper.cs b/...ncyFetching/SourceGenerators/DotnetSourceGeneratorWrapper/DotnetSourceGeneratorWrapper.cs
@@ -35,23 +35,22 @@ public DotnetSourceGeneratorWrapper(
 
         public IEnumerable<string> RunSourceGenerator(IEnumerable<string> additionalFiles, string csprojFile, IEnumerable<string> references, string targetDir)
         {
-            var name = Guid.NewGuid().ToString("N").ToUpper();
-            var tempPath = FileUtils.GetTemporaryWorkingDirectory(out var shouldCleanUp);
-            var analyzerConfig = Path.Combine(tempPath, $"{name}.txt");
-            var dllPath = Path.Combine(tempPath, $"{name}.dll");
-            var cscArgsPath = Path.Combine(tempPath, $"{name}.rsp");
-            var outputFolder = Path.Combine(targetDir, name);
-            Directory.CreateDirectory(outputFolder);
-
             try
             {
+                var name = Guid.NewGuid().ToString("N").ToUpper();
+                using var tempDir = new TemporaryDirectory(Path.Join(FileUtils.GetTemporaryWorkingDirectory(out _), "source-generator"), "source generator temporary", logger);
+                var analyzerConfigPath = Path.Combine(tempDir.DirInfo.FullName, $"{name}.txt");
+                var dllPath = Path.Combine(tempDir.DirInfo.FullName, $"{name}.dll");
+                var cscArgsPath = Path.Combine(tempDir.DirInfo.FullName, $"{name}.rsp");
+                var outputFolder = Path.Combine(targetDir, name);
+                Directory.CreateDirectory(outputFolder);
                 logger.LogInfo("Producing analyzer config content.");
-                GenerateAnalyzerConfig(additionalFiles, csprojFile, analyzerConfig);
+                GenerateAnalyzerConfig(additionalFiles, csprojFile, analyzerConfigPath);
 
-                logger.LogDebug($"Analyzer config content: {File.ReadAllText(analyzerConfig)}");
+                logger.LogDebug($"Analyzer config content: {File.ReadAllText(analyzerConfigPath)}");
 
                 var args = new StringBuilder();
-                args.Append($"/target:exe /generatedfilesout:\"{outputFolder}\" /out:\"{dllPath}\" /analyzerconfig:\"{analyzerConfig}\" ");
+                args.Append($"/target:exe /generatedfilesout:\"{outputFolder}\" /out:\"{dllPath}\" /analyzerconfig:\"{analyzerConfigPath}\" ");
 
                 foreach (var f in Directory.GetFiles(SourceGeneratorFolder, "*.dll", new EnumerationOptions { RecurseSubdirectories = false, MatchCasing = MatchCasing.CaseInsensitive }))
                 {
@@ -91,27 +90,6 @@ public IEnumerable<string> RunSourceGenerator(IEnumerable<string> additionalFile
                 logger.LogInfo($"Failed to generate source files from {FileType} files: {ex.Message}");
                 return [];
             }
-            finally
-            {
-                if (shouldCleanUp)
-                {
-                    DeleteFile(analyzerConfig);
-                    DeleteFile(dllPath);
-                    DeleteFile(cscArgsPath);
-                }
-            }
-        }
-
-        private void DeleteFile(string path)
-        {
-            try
-            {
-                File.Delete(path);
-            }
-            catch (Exception exc)
-            {
-                logger.LogWarning($"Failed to delete file {path}: {exc}");
-            }
         }
     }
 }
diff --git a/csharp/extractor/Semmle.Util/TemporaryDirectory.cs b/csharp/extractor/Semmle.Util/TemporaryDirectory.cs
@@ -15,9 +15,9 @@ public sealed class TemporaryDirectory : IDisposable
 
         public DirectoryInfo DirInfo { get; }
 
-        public TemporaryDirectory(string name, string userReportedDirectoryPurpose, ILogger logger)
+        public TemporaryDirectory(string path, string userReportedDirectoryPurpose, ILogger logger)
         {
-            DirInfo = new DirectoryInfo(name);
+            DirInfo = new DirectoryInfo(path);
             DirInfo.Create();
             this.userReportedDirectoryPurpose = userReportedDirectoryPurpose;
             this.logger = logger;