fix: address bugs found in a full-codebase audit

src/Sloc.Cli/Analysis/AnalyzeHandler.Support.cs

@@ -3,6 +3,7 @@
 using Sloc.Core.Models;
 using Sloc.Core.Scanning;
 using Spectre.Console;
+using System.Globalization;
 using System.Reflection;
 
 namespace Sloc.Cli.Analysis;
@@ -298,9 +299,12 @@ private static List<SkippedEntry> RemapGitPaths(List<SkippedEntry> skipped, Dict
 
             if (gitPathByTempPath.TryGetValue(entry.Path, out var gitPath))
             {
+                // An IO error message quotes the temp path it failed on ("Access to the path
+                // '...' is denied"), so rewrite that too.
                 entry = entry with
                 {
-                    Path = gitPath
+                    Path = gitPath,
+                    Reason = entry.Reason.Replace(entry.Path, gitPath, StringComparison.Ordinal)
                 };
             }
 
@@ -429,10 +433,26 @@ private static string ResolveFullPath(string path)
     /// <param name="summary">The summary to check the comment percentage of.</param>
     private static int ThresholdResult(AnalyzeOptions options, AnalysisSummary summary)
     {
-        if (options.MinCommentPct is { } min && summary.FileCount > 0 && summary.CommentPct < min)
+        if (options.MinCommentPct is not { } min)
+        {
+            return ExitCode.Success;
+        }
+
+        if (summary.FileCount == 0)
+        {
+            // There is no percentage to judge, but a silent pass would let a mistyped glob or
+            // path slip through a CI gate unnoticed.
+            Console.Error.WriteLine("sloc: warning: no files were analyzed, so --min-comment-pct was not evaluated.");
+            return ExitCode.Success;
+        }
+
+        if (summary.CommentPct < min)
         {
+            // Up to three decimals so a value that rounds up to the threshold (4.96 -> "5.0")
+            // is not reported as "5.0% is below 5.0%".
             Console.Error.WriteLine(
-                $"sloc: comment percentage {summary.CommentPct:F1}% is below the required {min:F1}%.");
+                $"sloc: comment percentage {summary.CommentPct.ToString("0.0##", CultureInfo.InvariantCulture)}% "
+                + $"is below the required {min.ToString("0.0##", CultureInfo.InvariantCulture)}%.");
             return ExitCode.ThresholdNotMet;
         }
 

src/Sloc.Cli/Analysis/AnalyzeHandler.cs

@@ -23,6 +23,18 @@ public sealed partial class AnalyzeHandler
     private readonly FileAnalyzer _analyzer = new();
     private readonly DirectoryScanner _scanner = new();
 
+    /// <summary>
+    /// Guards <see cref="_interruptCleanups"/>, which the Ctrl+C handler reads from another thread.
+    /// </summary>
+    private readonly Lock _cleanupLock = new();
+
+    /// <summary>
+    /// Deleters for the git snapshot temp directories that still exist (or are being
+    /// extracted), so an interrupt (which skips <c>finally</c>/<c>using</c>) can remove them
+    /// before the process exits.
+    /// </summary>
+    private readonly List<Action> _interruptCleanups = [];
+
     /// <summary>
     /// Runs the analysis described by <paramref name="options"/>.
     /// </summary>
@@ -41,12 +53,105 @@ public int Execute(AnalyzeOptions options)
             return ExecuteWatch(options, ctx.ScanOptions, ctx.TableRenderer, ctx.SourcePath);
         }
 
+        // A one-shot run has no cancellation of its own: Ctrl+C terminates the process without
+        // running finally/using blocks, which would leave extracted snapshots in the temp directory.
+        Console.CancelKeyPress += OnInterrupt;
+        try
+        {
+            return ExecuteOnce(options, ctx);
+        }
+        finally
+        {
+            Console.CancelKeyPress -= OnInterrupt;
+        }
+    }
+
+    /// <summary>
+    /// Deletes any git snapshot temp directories still on disk when the user interrupts the
+    /// run. Does not cancel the event, so the process still terminates as usual.
+    /// </summary>
+    /// <param name="sender">The event source.</param>
+    /// <param name="e">The event data.</param>
+    private void OnInterrupt(object? sender, ConsoleCancelEventArgs e)
+    {
+        Action[] cleanups;
+        lock (_cleanupLock)
+        {
+            cleanups = [.. _interruptCleanups];
+        }
+
+        foreach (var cleanup in cleanups)
+        {
+            cleanup();
+        }
+    }
+
+    /// <summary>
+    /// Registers <paramref name="cleanup"/> to run if the user interrupts the process.
+    /// </summary>
+    /// <param name="cleanup">The temp-directory deleter to run on interrupt.</param>
+    private void TrackCleanup(Action cleanup)
+    {
+        lock (_cleanupLock)
+        {
+            _interruptCleanups.Add(cleanup);
+        }
+    }
+
+    /// <summary>
+    /// Stops tracking <paramref name="cleanup"/> once its directory has been handled normally.
+    /// </summary>
+    /// <param name="cleanup">The deleter previously passed to <see cref="TrackCleanup"/>.</param>
+    private void UntrackCleanup(Action cleanup)
+    {
+        lock (_cleanupLock)
+        {
+            _interruptCleanups.Remove(cleanup);
+        }
+    }
+
+    /// <summary>
+    /// Extracts <paramref name="gitRef"/> with an interrupt cleanup registered for the whole
+    /// extraction, so Ctrl+C mid-extraction also removes the partly written temp directory.
+    /// The finished snapshot's own cleanup is registered before the extractor's is removed, so
+    /// the directory is never uncovered in between. The caller must pass the returned cleanup
+    /// to <see cref="UntrackCleanup"/> once it has disposed the snapshot.
+    /// </summary>
+    /// <param name="repoPath">The path inside the repository to query.</param>
+    /// <param name="gitRef">The commit/tree-ish to extract.</param>
+    /// <returns>The snapshot and the interrupt cleanup registered for it.</returns>
+    private (GitSnapshot Snapshot, Action Cleanup) ExtractTracked(string repoPath, string gitRef)
+    {
+        var extractor = new GitSnapshotExtractor();
+        Action extractorCleanup = extractor.DeletePendingTempRoot;
+        TrackCleanup(extractorCleanup);
+        try
+        {
+            var snapshot = extractor.Extract(repoPath, gitRef);
+            Action snapshotCleanup = snapshot.Dispose;
+            TrackCleanup(snapshotCleanup);
+            return (snapshot, snapshotCleanup);
+        }
+        finally
+        {
+            UntrackCleanup(extractorCleanup);
+        }
+    }
+
+    /// <summary>
+    /// Runs a single (non-watch) analysis using the already-built <paramref name="ctx"/>.
+    /// </summary>
+    /// <param name="options">The parsed options.</param>
+    /// <param name="ctx">The run context built by <see cref="BuildRunContext"/>.</param>
+    private int ExecuteOnce(AnalyzeOptions options, RunContext ctx)
+    {
         GitSnapshot? gitSnapshot = null;
+        Action? snapshotCleanup = null;
         if (options.GitHash is { } gitHash)
         {
             try
             {
-                gitSnapshot = new GitSnapshotExtractor().Extract(options.Path, gitHash);
+                (gitSnapshot, snapshotCleanup) = ExtractTracked(options.Path, gitHash);
             }
             catch (Exception ex) when (ex is GitSnapshotException or IOException or UnauthorizedAccessException)
             {
@@ -149,7 +254,11 @@ AnalysisSummary Summarize(List<FileAnalysis> results)
         }
         finally
         {
-            gitSnapshot?.Dispose();
+            if (gitSnapshot is not null && snapshotCleanup is not null)
+            {
+                gitSnapshot.Dispose();
+                UntrackCleanup(snapshotCleanup);
+            }
         }
     }
 
@@ -247,8 +356,28 @@ void AnalyzeAt(int i)
     /// <param name="options">The parsed options.</param>
     private AnalysisSummary AnalyzeGitRef(string repoPath, string gitRef, ScanOptions scanOptions, AnalyzeOptions options)
     {
-        using var snapshot = new GitSnapshotExtractor().Extract(repoPath, gitRef);
+        var (snapshot, cleanup) = ExtractTracked(repoPath, gitRef);
+        try
+        {
+            return AnalyzeSnapshot(snapshot, repoPath, scanOptions, options);
+        }
+        finally
+        {
+            snapshot.Dispose();
+            UntrackCleanup(cleanup);
+        }
+    }
 
+    /// <summary>
+    /// Scans and analyzes an extracted <paramref name="snapshot"/> for
+    /// <see cref="AnalyzeGitRef"/>.
+    /// </summary>
+    /// <param name="snapshot">The extracted snapshot of the compared ref.</param>
+    /// <param name="repoPath">The path passed as <c>path</c>.</param>
+    /// <param name="scanOptions">The scan options to apply to the extracted snapshot.</param>
+    /// <param name="options">The parsed options.</param>
+    private AnalysisSummary AnalyzeSnapshot(GitSnapshot snapshot, string repoPath, ScanOptions scanOptions, AnalyzeOptions options)
+    {
         var isFilePath = File.Exists(repoPath);
         var filesInScope = snapshot.Files;
         IEnumerable<SkippedEntry> skippedInScope = snapshot.Skipped;
@@ -424,6 +553,14 @@ private int ExecuteWatch(AnalyzeOptions options, ScanOptions scanOptions, TableR
             return ExitCode.Error;
         }
 
+        // The live table is redrawn in place with cursor movement, which Spectre.Console cannot
+        // do (and may throw on) when stdout is piped or redirected.
+        if (Console.IsOutputRedirected)
+        {
+            Console.Error.WriteLine("sloc: --watch requires an interactive terminal; stdout is redirected.");
+            return ExitCode.Error;
+        }
+
         if (!options.Quiet)
         {
             var version = typeof(AnalyzeHandler).Assembly
@@ -519,7 +656,7 @@ void OnChange(object sender, FileSystemEventArgs e)
             }
 
             AnsiConsole.Live(tableRenderer.BuildLanguageTable(lastSummary, noHealth: options.NoHealth, noComplexity: options.NoComplexity))
-                .AutoClear(true)
+                .AutoClear(false)
                 .Start(ctx =>
                 {
                     ctx.Refresh();
@@ -686,7 +823,19 @@ private ScanResult RunScan(GitSnapshot? gitSnapshot, AnalyzeOptions options, Sca
 
         if (options.ListFile is { } listFile)
         {
-            return _scanner.ScanFiles(ReadListFile(listFile), scanOptions);
+            IEnumerable<string> entries;
+            try
+            {
+                entries = ReadListFile(listFile);
+            }
+            catch (Exception ex) when (ex is ArgumentException or NotSupportedException)
+            {
+                // An empty or malformed path is a user input error, reported like any other
+                // unreadable list file rather than as an unexpected failure.
+                throw new IOException($"Invalid --list-file path '{listFile}': {ex.Message}", ex);
+            }
+
+            return _scanner.ScanFiles(entries, scanOptions);
         }
 
         if (!showProgress)

src/Sloc.Cli/Analysis/LiveAggregator.cs

@@ -13,7 +13,7 @@ internal sealed class LiveAggregator(LanguageSort sortBy, int? top, bool unique
 {
     private readonly Dictionary<string, Counts> _byLanguage = new(StringComparer.OrdinalIgnoreCase);
     private readonly HashSet<string> _seenHashes = [];
-    private readonly object _gate = new();
+    private readonly Lock _gate = new();
     private int _processed;
     private int _files;
 

src/Sloc.Cli/Output/CsvRenderer.cs

@@ -56,10 +56,10 @@ private static string ComplexityCell(int? complexity) =>
         complexity?.ToString() ?? string.Empty;
 
     // Spreadsheet apps (Excel, Google Sheets, LibreOffice) treat a cell starting with
-    // =, +, -, @, or tab as a formula. A file path or skip-reason with such a leading
+    // =, +, -, @, tab, or carriage return as a formula. A file path or skip-reason with such a leading
     // character would otherwise execute as a formula when the CSV is opened; prefixing
     // it with a quote neutralizes that without changing the visible text.
-    private static readonly char[] FormulaTriggers = ['=', '+', '-', '@', '\t'];
+    private static readonly char[] FormulaTriggers = ['=', '+', '-', '@', '\t', '\r'];
 
     private static string Escape(string field)
     {

src/Sloc.Cli/Output/DiffRenderer.cs

@@ -33,7 +33,7 @@ public static JsonReport Load(string path)
         {
             json = File.ReadAllText(path);
         }
-        catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
+        catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or ArgumentException or NotSupportedException)
         {
             throw new InvalidOperationException($"Could not read baseline '{path}': {ex.Message}");
         }

src/Sloc.Cli/Output/JsonRenderer.cs

@@ -96,7 +96,7 @@ public void Render(AnalysisSummary summary, bool byFile, bool noHealth, bool det
         noHealth || health == CommentHealthLevel.NotApplicable ? null : health.ToString();
 
     private static double Pct(int count, int total) =>
-            total == 0 ? 0.0 : Math.Round((double)count / total * 100, 1);
+            total == 0 ? 0.0 : Math.Round((double)count / total * 100, 1, MidpointRounding.AwayFromZero);
 }
 
 /// <summary>