C#: Re-factor FileContent to make it unit-testable and make an initializer class.

Author: michaelnebel

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

@@ -17,17 +17,17 @@ namespace Semmle.BuildAnalyser
     internal partial class FileContent
     {
         private readonly ProgressMonitor progressMonitor;
-        private readonly TemporaryDirectory packageDirectory;
+        private readonly IUnsafeFileReader unsafeFileReader;
         private readonly Func<IEnumerable<string>> getFiles;
+        private readonly Func<HashSet<string>> getAlreadyDownloadedPackages;
         private readonly HashSet<string> notYetDownloadedPackages = new HashSet<string>();
-
-        private bool IsInitialized { get; set; } = false;
+        private Initializer Initialize { get; init; }
 
         public HashSet<string> NotYetDownloadedPackages
         {
             get
             {
-                Initialize();
+                Initialize.Run();
                 return notYetDownloadedPackages;
             }
         }
@@ -45,18 +45,30 @@ public bool UseAspNetDlls
         {
             get
             {
-                Initialize();
+                Initialize.Run();
                 return useAspNetDlls;
             }
         }
 
-        public FileContent(TemporaryDirectory packageDirectory, ProgressMonitor progressMonitor, Func<IEnumerable<string>> getFiles)
+        internal FileContent(Func<HashSet<string>> getAlreadyDownloadedPackages,
+            ProgressMonitor progressMonitor,
+            Func<IEnumerable<string>> getFiles,
+            IUnsafeFileReader unsafeFileReader)
         {
+            this.getAlreadyDownloadedPackages = getAlreadyDownloadedPackages;
             this.progressMonitor = progressMonitor;
-            this.packageDirectory = packageDirectory;
             this.getFiles = getFiles;
+            this.unsafeFileReader = unsafeFileReader;
+            Initialize = new Initializer(DoInitialize);
         }
 
+
+        public FileContent(TemporaryDirectory packageDirectory, ProgressMonitor progressMonitor, Func<IEnumerable<string>> getFiles) : this(() => Directory.GetDirectories(packageDirectory.DirInfo.FullName)
+                .Select(d => Path.GetFileName(d)
+                .ToLowerInvariant())
+                .ToHashSet(), progressMonitor, getFiles, new UnsafeFileReader())
+        { }
+
         private static string GetGroup(ReadOnlySpan<char> input, ValueMatch valueMatch, string groupPrefix)
         {
             var match = input.Slice(valueMatch.Index, valueMatch.Length);
@@ -87,21 +99,14 @@ private static bool IsGroupMatch(ReadOnlySpan<char> line, Regex regex, string gr
             return false;
         }
 
-        private void Initialize()
+        private void DoInitialize()
         {
-            if (IsInitialized)
-            {
-                return;
-            }
-
-            var alreadyDownloadedPackages = Directory.GetDirectories(packageDirectory.DirInfo.FullName).Select(d => Path.GetFileName(d).ToLowerInvariant()).ToHashSet();
+            var alreadyDownloadedPackages = getAlreadyDownloadedPackages();
             foreach (var file in getFiles())
             {
                 try
                 {
-                    using var sr = new StreamReader(file);
-                    ReadOnlySpan<char> line;
-                    while ((line = sr.ReadLine()) != null)
+                    foreach (ReadOnlySpan<char> line in unsafeFileReader.ReadLines(file))
                     {
 
                         // Find the not yet downloaded packages.
@@ -122,15 +127,13 @@ private void Initialize()
                                 IsGroupMatch(line, ProjectSdk(), "Sdk", "Microsoft.NET.Sdk.Web") ||
                                 IsGroupMatch(line, FrameworkReference(), "Include", "Microsoft.AspNetCore.App");
                         }
-
                     }
                 }
                 catch (Exception ex)
                 {
                     progressMonitor.FailedToReadFile(file, ex);
                 }
             }
-            IsInitialized = true;
         }
 
         [GeneratedRegex("<PackageReference.*\\sInclude=\"(.*?)\".*/?>", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.Singleline)]
@@ -142,4 +145,22 @@ private void Initialize()
         [GeneratedRegex("<(.*\\s)?Project.*\\sSdk=\"(.*?)\".*/?>", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.Singleline)]
         private static partial Regex ProjectSdk();
     }
+}
+
+internal interface IUnsafeFileReader
+{
+    IEnumerable<string> ReadLines(string file);
+}
+
+internal class UnsafeFileReader : IUnsafeFileReader
+{
+    public IEnumerable<string> ReadLines(string file)
+    {
+        using var sr = new StreamReader(file);
+        string? line;
+        while ((line = sr.ReadLine()) != null)
+        {
+            yield return line;
+        }
+    }
 }

csharp/extractor/Semmle.Util/Initializer.cs

@@ -0,0 +1,28 @@
+using System;
+
+namespace Semmle.Util
+{
+    /// <summary>
+    /// An instance of this class is used to ensure that the provided
+    /// action is executed only once and on the first call to `Run`.
+    /// It is thread-safe.
+    /// </summary>
+    public class Initializer
+    {
+        private readonly Lazy<bool> doInit;
+
+        public Initializer(Action action)
+        {
+            doInit = new Lazy<bool>(() =>
+            {
+                action();
+                return true;
+            });
+        }
+
+        public void Run()
+        {
+            var _ = doInit.Value;
+        }
+    }
+}