From bd99aec6cf0b1b4b6d6ea440cda695d943c8c0af Mon Sep 17 00:00:00 2001 From: Gaoyang Date: Wed, 30 Sep 2026 17:48:29 +0800 Subject: [PATCH 01/10] fix: handle char literals, escaped triple quotes and operator dashes in language rules Treat '"' as a character literal in Rust, Scala, Haskell, F# and OCaml, honor backslash escapes in Dart, Groovy and Starlark triple-quoted strings, and stop Haskell operators such as --> and |-- from being classified as line comments. --- src/Sloc.Core/Languages/LanguageDefinition.cs | 21 +++++++ src/Sloc.Core/Languages/LanguageRegistry.cs | 22 ++++--- src/Sloc.Core/LineClassifier.cs | 58 ++++++++++++++++++ .../LineClassifierLanguageTests.cs | 60 +++++++++++++++++++ 4 files changed, 154 insertions(+), 7 deletions(-) diff --git a/src/Sloc.Core/Languages/LanguageDefinition.cs b/src/Sloc.Core/Languages/LanguageDefinition.cs index 3cd203f..bc72171 100644 --- a/src/Sloc.Core/Languages/LanguageDefinition.cs +++ b/src/Sloc.Core/Languages/LanguageDefinition.cs @@ -161,6 +161,27 @@ public bool QuoteDigitSeparators get; init; } + /// + /// Whether a " written as the character literal '"' (or '\"') is a + /// character rather than the start of a string, for languages whose ' is not a + /// string delimiter (Rust, Scala, Haskell, F#, OCaml). + /// + public bool DoubleQuoteCharLiteral + { + get; init; + } + + /// + /// Whether a line-comment token made of a repeated + /// character (Haskell's --) only starts a comment when the run of that character is + /// not adjacent to a symbol character, so operators such as --> and |-- + /// are not comments. + /// + public bool SymbolAwareLineComments + { + get; init; + } + /// /// String prefixes (e.g. Python's r and u) that may directly precede a /// doc-comment literal without making it an diff --git a/src/Sloc.Core/Languages/LanguageRegistry.cs b/src/Sloc.Core/Languages/LanguageRegistry.cs index 189aff8..20dba4b 100644 --- a/src/Sloc.Core/Languages/LanguageRegistry.cs +++ b/src/Sloc.Core/Languages/LanguageRegistry.cs @@ -114,6 +114,8 @@ private static IReadOnlyList CreateLanguages() var pyTripleSingle = new StringLiteral("'''", Multiline: true, IsDocComment: true); var rawTripleDouble = new StringLiteral("\"\"\"", Multiline: true, AllowEscape: false); var rawTripleSingle = new StringLiteral("'''", Multiline: true, AllowEscape: false); + // Dart, Groovy, and Starlark treat a backslash as an escape in triple-single-quoted strings too. + var escapedTripleSingle = rawTripleSingle with { AllowEscape = true }; // Unlike rawTripleDouble, these languages' """-strings support a backslash escape // to embed the closing delimiter (e.g. Swift's \""", TOML's \""" before the closing // quotes, GraphQL block strings' documented \""" escape, Elixir heredocs, Dart @@ -274,6 +276,7 @@ private static IReadOnlyList CreateLanguages() // Single quotes denote lifetimes as well as char literals in Rust, so they // are not treated as string delimiters here. StringLiterals = [rustRawStringOneHash, rustRawStringNoHash, doubleQuote], + DoubleQuoteCharLiteral = true, ComplexityKeywords = rustComplexity }, new() @@ -307,7 +310,8 @@ private static IReadOnlyList CreateLanguages() BlockCommentExceptions = ["(*)"], // Single quotes appear in generic type parameters (e.g. 'a), so only // double quotes are treated as string delimiters. - StringLiterals = [doubleQuote] + StringLiterals = [doubleQuote], + DoubleQuoteCharLiteral = true }, new() { @@ -403,7 +407,7 @@ private static IReadOnlyList CreateLanguages() Extensions = [".dart"], LineCommentTokens = ["//"], BlockComments = [nestedCStyleBlock], - StringLiterals = [escapedTripleDouble, rawTripleSingle, doubleQuote, singleQuote] + StringLiterals = [escapedTripleDouble, escapedTripleSingle, doubleQuote, singleQuote] }, new() { @@ -413,7 +417,8 @@ private static IReadOnlyList CreateLanguages() BlockComments = [nestedCStyleBlock], // Single quotes denote char literals and symbols (e.g. 'sym), so only // double and triple-double quotes are treated as string delimiters. - StringLiterals = [rawTripleDouble, doubleQuote] + StringLiterals = [rawTripleDouble, doubleQuote], + DoubleQuoteCharLiteral = true }, new() { @@ -469,7 +474,9 @@ .. new[] Extensions = [".hs"], LineCommentTokens = ["--"], BlockComments = [new BlockComment("{-", "-}", AllowNested: true)], - StringLiterals = [doubleQuote] + StringLiterals = [doubleQuote], + DoubleQuoteCharLiteral = true, + SymbolAwareLineComments = true }, new() { @@ -535,7 +542,7 @@ .. new[] Extensions = [".groovy", ".gradle"], LineCommentTokens = ["//"], BlockComments = [cStyleBlock], - StringLiterals = [rawTripleDouble, rawTripleSingle, doubleQuote, singleQuote] + StringLiterals = [escapedTripleDouble, escapedTripleSingle, doubleQuote, singleQuote] }, new() { @@ -654,7 +661,8 @@ .. new[] Name = "OCaml", Extensions = [".ml", ".mli"], BlockComments = [new BlockComment("(*", "*)", AllowNested: true)], - StringLiterals = [doubleQuote] + StringLiterals = [doubleQuote], + DoubleQuoteCharLiteral = true }, new() { @@ -694,7 +702,7 @@ .. new[] // "build"/"workspace" are common names for unrelated scripts and directories. CaseSensitiveFilenames = true, LineCommentTokens = ["#"], - StringLiterals = [rawTripleDouble, rawTripleSingle, doubleQuote, singleQuote] + StringLiterals = [escapedTripleDouble, escapedTripleSingle, doubleQuote, singleQuote] }, new() { diff --git a/src/Sloc.Core/LineClassifier.cs b/src/Sloc.Core/LineClassifier.cs index 7998f40..b09c4e2 100644 --- a/src/Sloc.Core/LineClassifier.cs +++ b/src/Sloc.Core/LineClassifier.cs @@ -631,12 +631,47 @@ private bool MatchesLineComment(ReadOnlySpan line, int index) continue; } + if (_language.SymbolAwareLineComments && IsSymbolOperatorAdjacent(line, index, token)) + { + continue; + } + return true; } return false; } + /// + /// Whether the run of 's first character at + /// is glued to a symbol character, making it part of an operator (Haskell's --> + /// or |--) rather than a line comment. + /// + /// The line being classified. + /// The index where the line-comment token matched. + /// The matched line-comment token. + private static bool IsSymbolOperatorAdjacent(ReadOnlySpan line, int index, string token) + { + if (index > 0 && IsOperatorSymbol(line[index - 1])) + { + return true; + } + + var end = index + token.Length; + while (end < line.Length && line[end] == token[0]) + { + end++; + } + + return end < line.Length && IsOperatorSymbol(line[end]); + } + + /// + /// Whether is one of Haskell's ASCII operator symbol characters. + /// + /// The character to test. + private static bool IsOperatorSymbol(char c) => "!#$%&*+./<=>?@\\^|-~:".Contains(c); + /// /// Returns the length of the block-comment /// exception token at , or 0 when none matches there. @@ -760,6 +795,23 @@ private static bool IsInNumericLiteral(ReadOnlySpan line, int index) return start < index && char.IsAsciiDigit(line[start]); } + /// + /// Whether the " at is the content of the character + /// literal '"' or '\"'. + /// + /// The line being classified. + /// The index of the ". + private static bool IsQuotedDoubleQuoteChar(ReadOnlySpan line, int index) + { + if (line[index] != '"' || index + 1 >= line.Length || line[index + 1] != '\'') + { + return false; + } + + return (index >= 1 && line[index - 1] == '\'') + || (index >= 2 && line[index - 1] == '\\' && line[index - 2] == '\''); + } + private bool TryMatchStringOpen(ReadOnlySpan line, int index, [NotNullWhen(true)] out StringLiteral? literal) { if (_language.QuoteDigitSeparators && line[index] == '\'' && IsInNumericLiteral(line, index)) @@ -768,6 +820,12 @@ private bool TryMatchStringOpen(ReadOnlySpan line, int index, [NotNullWhen return false; } + if (_language.DoubleQuoteCharLiteral && IsQuotedDoubleQuoteChar(line, index)) + { + literal = null; + return false; + } + if (_language.StringLiteralsByFirstChar.TryGetValue(line[index], out var candidates)) { foreach (var candidate in candidates) diff --git a/tests/Sloc.Core.Tests/LineClassifierLanguageTests.cs b/tests/Sloc.Core.Tests/LineClassifierLanguageTests.cs index 383946f..54bd8cc 100644 --- a/tests/Sloc.Core.Tests/LineClassifierLanguageTests.cs +++ b/tests/Sloc.Core.Tests/LineClassifierLanguageTests.cs @@ -476,6 +476,66 @@ public void Classify_NixLineComment_ReturnsComment() Assert.Equal(LineKind.Comment, classifier.Classify("# a nix comment")); } + /// + /// Verifies that a backslash-escaped closing delimiter inside a triple-quoted string does + /// not end it early in Dart, Groovy, and Starlark, so a following comment stays a comment. + /// + /// The file extension selecting the language. + /// The language's line-comment token. + /// A line holding a triple-quoted string with an escaped delimiter. + [Theory] + [InlineData(".groovy", "//", "def s = \"\"\"a \\\"\"\" b\"\"\"")] + [InlineData(".groovy", "//", "def s = '''a \\''' b'''")] + [InlineData(".dart", "//", "var s = '''a \\''' b''';")] + [InlineData(".bzl", "#", "s = \"\"\"a \\\"\"\" b\"\"\"")] + public void Classify_TripleQuotedStringWithEscapedDelimiter_DoesNotLeakIntoNextLine(string extension, string commentToken, string line) + { + var classifier = new LineClassifier(Resolve(extension)); + + Assert.Equal(LineKind.Code, classifier.Classify(line)); + Assert.Equal(LineKind.Comment, classifier.Classify($"{commentToken} real comment")); + } + + /// + /// Verifies that the character literal '"' does not open a string, so a block + /// comment opened later on the same line is still recognized. + /// + /// The file extension selecting the language. + /// A line with a '"' literal followed by an open block comment. + [Theory] + [InlineData(".rs", "let q = '\"'; /* note")] + [InlineData(".rs", "let q = '\\\"'; /* note")] + [InlineData(".scala", "val q = '\"'; /* note")] + [InlineData(".ml", "let q = '\"'; (* note")] + [InlineData(".fs", "let q = '\"'; (* note")] + public void Classify_DoubleQuoteCharLiteral_DoesNotOpenString(string extension, string line) + { + var classifier = new LineClassifier(Resolve(extension)); + + Assert.Equal(LineKind.Code, classifier.Classify(line)); + Assert.Equal(LineKind.Comment, classifier.Classify("still comment")); + } + + /// + /// Verifies that Haskell's -- only starts a comment when it is not part of a longer + /// operator such as --> or |--. + /// + /// The line to classify. + /// The expected classification. + [Theory] + [InlineData("-- a comment", LineKind.Comment)] + [InlineData("--- a comment", LineKind.Comment)] + [InlineData("-- | haddock", LineKind.Comment)] + [InlineData(" --> next", LineKind.Code)] + [InlineData(" |-- next", LineKind.Code)] + [InlineData("x = 1 -- trailing", LineKind.Code)] + public void Classify_HaskellDashes_OnlyPlainRunsAreComments(string line, LineKind expected) + { + var classifier = new LineClassifier(Resolve(".hs")); + + Assert.Equal(expected, classifier.Classify(line)); + } + private static LanguageDefinition Resolve(string extension) { LanguageRegistry.TryGetByExtension(extension, out var language); From aa1977d8cd59a22e14ad1e090e874827902a0e34 Mon Sep 17 00:00:00 2001 From: Gaoyang Date: Wed, 30 Sep 2026 17:48:31 +0800 Subject: [PATCH 02/10] fix: accept blanks between the [attr] prefix and the macro name Git allows blanks after [attr], so the name is the next token rather than a separate attribute state on a bogus pattern. --- src/Sloc.Core/Scanning/GitAttributesRules.cs | 10 ++++++++++ tests/Sloc.Core.Tests/GitAttributesRulesTests.cs | 15 +++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/Sloc.Core/Scanning/GitAttributesRules.cs b/src/Sloc.Core/Scanning/GitAttributesRules.cs index 387dd2d..a67d159 100644 --- a/src/Sloc.Core/Scanning/GitAttributesRules.cs +++ b/src/Sloc.Core/Scanning/GitAttributesRules.cs @@ -588,6 +588,16 @@ public static bool TryParse(string rawLine, out AttributeLine line) var tokenEnd = FindBlank(rawLine, start); pattern = rawLine[start..tokenEnd]; statesStart = tokenEnd; + + // Git allows blanks between the "[attr]" prefix and the macro name, so the name is + // the next token rather than a separate attribute state. + if (pattern == MacroPrefix) + { + var nameStart = SkipBlanks(rawLine, tokenEnd); + var nameEnd = FindBlank(rawLine, nameStart); + pattern = MacroPrefix + rawLine[nameStart..nameEnd]; + statesStart = nameEnd; + } } string? macroName = null; diff --git a/tests/Sloc.Core.Tests/GitAttributesRulesTests.cs b/tests/Sloc.Core.Tests/GitAttributesRulesTests.cs index 1023be2..cc86122 100644 --- a/tests/Sloc.Core.Tests/GitAttributesRulesTests.cs +++ b/tests/Sloc.Core.Tests/GitAttributesRulesTests.cs @@ -161,6 +161,21 @@ public void IsVendoredOrGenerated_TopLevelMacro_ExpandsWhenSet(string assignment Assert.False(rules.IsVendoredOrGenerated("src/app.js")); } + /// + /// Verifies that blanks between the [attr] prefix and the macro name are accepted. + /// + [Fact] + public void IsVendoredOrGenerated_MacroWithBlankAfterPrefix_ExpandsMacro() + { + var rules = GitAttributesRules.FromLines(string.Empty, + [ + "[attr] thirdparty linguist-vendored", + "v/** thirdparty" + ]); + + Assert.True(rules.IsVendoredOrGenerated("v/lib.js")); + } + /// /// Verifies that a later macro definition replaces an earlier one. /// From befabaffa1a22891225d84e3d61a77b1502478f3 Mon Sep 17 00:00:00 2001 From: Gaoyang Date: Wed, 30 Sep 2026 17:48:32 +0800 Subject: [PATCH 03/10] fix: treat an empty gitdir pointer as an unusable .git file A .git file containing only 'gitdir:' made Path.GetFullPath throw instead of being ignored. --- src/Sloc.Core/Scanning/GitWorkTree.cs | 17 +++++++++++++++-- tests/Sloc.Core.Tests/GitWorkTreeTests.cs | 13 +++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/Sloc.Core/Scanning/GitWorkTree.cs b/src/Sloc.Core/Scanning/GitWorkTree.cs index 6eeb142..ca22338 100644 --- a/src/Sloc.Core/Scanning/GitWorkTree.cs +++ b/src/Sloc.Core/Scanning/GitWorkTree.cs @@ -131,8 +131,21 @@ public string RelativeDirectory(string directory) return null; } - var gitDirectory = Path.GetFullPath(line[Prefix.Length..].Trim(), workTreeRoot); - return Directory.Exists(gitDirectory) ? gitDirectory : null; + var target = line[Prefix.Length..].Trim(); + if (target.Length == 0) + { + return null; + } + + try + { + var gitDirectory = Path.GetFullPath(target, workTreeRoot); + return Directory.Exists(gitDirectory) ? gitDirectory : null; + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + return null; + } } /// diff --git a/tests/Sloc.Core.Tests/GitWorkTreeTests.cs b/tests/Sloc.Core.Tests/GitWorkTreeTests.cs index 3104293..b8d6315 100644 --- a/tests/Sloc.Core.Tests/GitWorkTreeTests.cs +++ b/tests/Sloc.Core.Tests/GitWorkTreeTests.cs @@ -74,6 +74,19 @@ public void Find_GitFileWithCommonDir_ReturnsMainRepositoryDirectory() Assert.Equal(Path.Combine(mainGit, "info", "exclude"), workTree.InfoExcludePath); } + /// + /// Verifies that a .git file with an empty gitdir: is treated as unusable + /// instead of throwing. + /// + [Fact] + public void Find_EmptyGitdirTarget_ReturnsNull() + { + var sub = Directory.CreateDirectory(Path.Combine(_root, "sub")).FullName; + File.WriteAllText(Path.Combine(sub, ".git"), "gitdir:\n"); + + Assert.Null(GitWorkTree.Find(sub)); + } + /// /// Verifies that a .git file pointing nowhere stops the search instead of /// continuing to a repository further up. From 9e564432085489be822f1d6e1d5f0b19cba99e85 Mon Sep 17 00:00:00 2001 From: Gaoyang Date: Wed, 30 Sep 2026 17:48:33 +0800 Subject: [PATCH 04/10] fix: resolve rev:./path tree-ish from the hint directory and harden extraction Run the rev-parse in the hint directory so ./ and ../ resolve the way the tree prefix bookkeeping assumes, kill the cat-file process when the temp file cannot be created, and expose DeletePendingTempRoot so an interrupt can remove a partially extracted snapshot. --- src/Sloc.Core/GitSnapshotExtractor.cs | 68 +++++++++++++++---- .../GitSnapshotExtractorTests.cs | 34 ++++++++++ 2 files changed, 87 insertions(+), 15 deletions(-) diff --git a/src/Sloc.Core/GitSnapshotExtractor.cs b/src/Sloc.Core/GitSnapshotExtractor.cs index 8480e08..5ec3db8 100644 --- a/src/Sloc.Core/GitSnapshotExtractor.cs +++ b/src/Sloc.Core/GitSnapshotExtractor.cs @@ -85,6 +85,37 @@ public sealed class GitSnapshotExtractor private static readonly SearchValues InvalidFileNameChars = SearchValues.Create([.. Path.GetInvalidFileNameChars(), '/', '\\']); + /// + /// The temporary directory of an call that has not returned yet, so + /// can remove it when the process is interrupted. + /// + private string? _pendingTempRoot; + + /// + /// Deletes the temporary directory of an call that is still running, + /// ignoring errors. Meant for an interrupt handler, since Ctrl+C ends the process without + /// running the catch block that normally cleans up. Does nothing when no extraction + /// is in progress, including once has returned and the caller owns + /// (and must dispose) the resulting . + /// + public void DeletePendingTempRoot() + { + var tempRoot = Interlocked.Exchange(ref _pendingTempRoot, null); + if (tempRoot is null) + { + return; + } + + try + { + Directory.Delete(tempRoot, recursive: true); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // Best-effort cleanup while the process is shutting down. + } + } + /// /// Extracts every blob reachable from in the git /// repository containing into a new temporary @@ -144,7 +175,9 @@ public GitSnapshot Extract(string repoPathHint, string commitHash, CancellationT // Resolve the revision on its own before peeling it to a tree: appending "^{tree}" // directly would make git read a "rev:path" tree-ish (e.g. "HEAD:src") as the path // "src^{tree}". The resolved object is a full hash, so the peel below is unambiguous. - var objectHash = RunGit(repoRoot, ["rev-parse", "--verify", "--quiet", commitHash]).Trim(); + // Run from hintDirectory: git resolves a "rev:./path" or "rev:../path" relative to the working + // directory, which is what TreePathPrefix below assumes; root-relative paths are unaffected. + var objectHash = RunGit(hintDirectory, ["rev-parse", "--verify", "--quiet", commitHash]).Trim(); var treeHash = RunGit(repoRoot, ["rev-parse", "--verify", "--quiet", $"{objectHash}^{{tree}}"]).Trim(); // Trailing slash trimmed so a subdirectory match is "prefix" or "prefix/...", never // "prefix/" (an empty result means repoPathHint was the repo root itself). @@ -157,6 +190,7 @@ public GitSnapshot Extract(string repoPathHint, string commitHash, CancellationT var tempRoot = Path.Combine(Path.GetTempPath(), "sloc-git-" + Guid.NewGuid().ToString("N")); Directory.CreateDirectory(tempRoot); + Volatile.Write(ref _pendingTempRoot, tempRoot); try { @@ -175,10 +209,15 @@ public GitSnapshot Extract(string repoPathHint, string commitHash, CancellationT var filtered = FindFilteredBlobs(repoRoot, treePrefix, blobsToExtract, cancellationToken); var files = ExtractBlobs(repoRoot, tempRoot, treePrefix, blobsToExtract, filtered, skipped, cancellationToken); - return new GitSnapshot(tempRoot, relativePrefix, files, skipped); + var snapshot = new GitSnapshot(tempRoot, relativePrefix, files, skipped); + + // From here on the caller owns the directory through the snapshot. + Volatile.Write(ref _pendingTempRoot, null); + return snapshot; } catch { + Volatile.Write(ref _pendingTempRoot, null); try { Directory.Delete(tempRoot, recursive: true); @@ -609,20 +648,19 @@ internal static bool IsCheckoutPath(string repositoryPath) => using var process = StartGit(repoRoot, ["cat-file", "--filters", $"--path={repositoryPath}", hash], redirectInput: false); var stderrTask = process.StandardError.ReadToEndAsync(cancellationToken); string tempPath; - using (var fileStream = allocator.Create(gitPath, out tempPath)) + try { - try - { - process.StandardOutput.BaseStream.CopyToAsync(fileStream, cancellationToken).GetAwaiter().GetResult(); - } - catch - { - // A filter such as Git LFS can block on a download, and disposing the process - // on any other failure (e.g. a disk-full IOException) only releases the handle - // without stopping it; don't leave it running either way. - process.Kill(entireProcessTree: true); - throw; - } + using var fileStream = allocator.Create(gitPath, out tempPath); + process.StandardOutput.BaseStream.CopyToAsync(fileStream, cancellationToken).GetAwaiter().GetResult(); + } + catch + { + // A filter such as Git LFS can block on a download, and disposing the process + // on any other failure (e.g. a disk-full IOException, including one raised while + // creating the temp file) only releases the handle without stopping it; don't + // leave it running either way. + process.Kill(entireProcessTree: true); + throw; } string stderr; diff --git a/tests/Sloc.Core.Tests/GitSnapshotExtractorTests.cs b/tests/Sloc.Core.Tests/GitSnapshotExtractorTests.cs index 4cfc386..71521cb 100644 --- a/tests/Sloc.Core.Tests/GitSnapshotExtractorTests.cs +++ b/tests/Sloc.Core.Tests/GitSnapshotExtractorTests.cs @@ -122,6 +122,40 @@ public void Extract_RevPathTreeish_ExtractsThatSubtree() Assert.Equal("print(1)", File.ReadAllText(file.TempPath)); } + /// + /// Verifies that leaves a finished + /// extraction alone, since the returned snapshot owns its directory from then on. + /// + [Fact] + public void DeletePendingTempRoot_AfterExtractReturned_KeepsSnapshotDirectory() + { + Write("a.cs", "int x;"); + Commit("first"); + + using var snapshot = _extractor.Extract(_root, "HEAD", TestContext.Current.CancellationToken); + _extractor.DeletePendingTempRoot(); + + Assert.True(Directory.Exists(snapshot.TempRoot)); + } + + /// + /// Verifies that a ./-relative rev:path is resolved against the directory the + /// caller passed as the repo path (as the tree-prefix bookkeeping assumes), not the + /// repository root. + /// + [Fact] + public void Extract_RevPathRelativeToSubdirectory_ResolvesAgainstThatDirectory() + { + Write("cur/dir/a/x.cs", "int x;"); + Write("a/y.py", "print(1)"); + Commit("first"); + + using var snapshot = _extractor.Extract(Path.Combine(_root, "cur", "dir"), "HEAD:./a", TestContext.Current.CancellationToken); + + var file = Assert.Single(snapshot.Files); + Assert.Equal("x.cs", file.GitPath); + } + /// /// Verifies that a rev:path naming a blob rather than a tree is still rejected. /// From ade4092acb6cc95950f1c11c79fdc8e18dfc9b8b Mon Sep 17 00:00:00 2001 From: Gaoyang Date: Wed, 30 Sep 2026 17:48:34 +0800 Subject: [PATCH 05/10] fix: neutralize a leading carriage return in CSV fields --- src/Sloc.Cli/Output/CsvRenderer.cs | 4 ++-- tests/Sloc.Cli.Tests/CsvRendererTests.cs | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Sloc.Cli/Output/CsvRenderer.cs b/src/Sloc.Cli/Output/CsvRenderer.cs index 4624351..dac57ff 100644 --- a/src/Sloc.Cli/Output/CsvRenderer.cs +++ b/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) { diff --git a/tests/Sloc.Cli.Tests/CsvRendererTests.cs b/tests/Sloc.Cli.Tests/CsvRendererTests.cs index 1f7c9c4..1755aae 100644 --- a/tests/Sloc.Cli.Tests/CsvRendererTests.cs +++ b/tests/Sloc.Cli.Tests/CsvRendererTests.cs @@ -112,6 +112,21 @@ public void Render_ByFile_NeutralizesFormulaTriggerPaths(string maliciousPath) Assert.StartsWith("'" + maliciousPath[0], lines[1]); } + /// + /// Verifies that a path starting with a carriage return is neutralized too; the field is + /// also quoted because it contains a line break. + /// + [Fact] + public void Render_ByFile_NeutralizesLeadingCarriageReturn() + { + var summary = BuildSummary("\r=1+1"); + using var writer = new StringWriter(); + + new CsvRenderer(writer).Render(summary, byFile: true, noHealth: true, noComplexity: true); + + Assert.Contains("\"'\r=1+1\"", writer.ToString()); + } + /// /// Verifies that a skip-reason starting with a formula-trigger character is also /// neutralized, not just file paths. From f9ef0028e6882ee0f3f8a9620e9d795af7eeec82 Mon Sep 17 00:00:00 2001 From: Gaoyang Date: Wed, 30 Sep 2026 17:48:36 +0800 Subject: [PATCH 06/10] fix: round JSON percentages half away from zero Math.Round used banker's rounding, so 6.25 became 6.2 while the table showed 6.3. --- src/Sloc.Cli/Output/JsonRenderer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sloc.Cli/Output/JsonRenderer.cs b/src/Sloc.Cli/Output/JsonRenderer.cs index 020de4b..04751b4 100644 --- a/src/Sloc.Cli/Output/JsonRenderer.cs +++ b/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); } /// From 39db8f46ff96a552019f5b523e5dfe574fd5698e Mon Sep 17 00:00:00 2001 From: Gaoyang Date: Wed, 30 Sep 2026 17:48:37 +0800 Subject: [PATCH 07/10] refactor: use the Lock type for the live aggregator gate --- src/Sloc.Cli/Analysis/LiveAggregator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sloc.Cli/Analysis/LiveAggregator.cs b/src/Sloc.Cli/Analysis/LiveAggregator.cs index fabcce8..62c239c 100644 --- a/src/Sloc.Cli/Analysis/LiveAggregator.cs +++ b/src/Sloc.Cli/Analysis/LiveAggregator.cs @@ -13,7 +13,7 @@ internal sealed class LiveAggregator(LanguageSort sortBy, int? top, bool unique { private readonly Dictionary _byLanguage = new(StringComparer.OrdinalIgnoreCase); private readonly HashSet _seenHashes = []; - private readonly object _gate = new(); + private readonly Lock _gate = new(); private int _processed; private int _files; From 261afa7720785412c5d207361d5d24a8f43d1958 Mon Sep 17 00:00:00 2001 From: Gaoyang Date: Wed, 30 Sep 2026 17:48:38 +0800 Subject: [PATCH 08/10] fix: stop repeatable options from swallowing the path argument Drop AllowMultipleArgumentsPerToken so 'sloc -e "*.md" src' analyzes src instead of treating it as another exclude glob. Move command construction into SlocCommandFactory so parsing is testable, validate --sort, guard against a missing console when setting the output encoding, and fix the --sort/--output help text. --- src/Sloc.Cli/Parsing/SlocCommandFactory.cs | 339 ++++++++++++++++++ src/Sloc.Cli/Program.cs | 324 +---------------- .../Sloc.Cli.Tests/SlocCommandFactoryTests.cs | 127 +++++++ 3 files changed, 477 insertions(+), 313 deletions(-) create mode 100644 src/Sloc.Cli/Parsing/SlocCommandFactory.cs create mode 100644 tests/Sloc.Cli.Tests/SlocCommandFactoryTests.cs diff --git a/src/Sloc.Cli/Parsing/SlocCommandFactory.cs b/src/Sloc.Cli/Parsing/SlocCommandFactory.cs new file mode 100644 index 0000000..57d9083 --- /dev/null +++ b/src/Sloc.Cli/Parsing/SlocCommandFactory.cs @@ -0,0 +1,339 @@ +using Sloc.Cli.Analysis; +using Sloc.Core.Languages; +using Sloc.Core.Models; +using System.CommandLine; +using System.CommandLine.Help; + +namespace Sloc.Cli.Parsing; + +/// +/// Builds the sloc command line: its arguments and options, the validation of their +/// combinations, and the mapping of a parsed command line to . +/// Kept apart from Program.cs so parsing behavior can be tested without running an analysis. +/// +internal static class SlocCommandFactory +{ + /// + /// Creates the root command. Its action validates the parsed arguments and passes the + /// resulting to . + /// + /// + /// Runs the analysis for the validated options and returns the process exit code. + /// + /// The configured root command. + public static RootCommand Create(Func execute) + { + ArgumentNullException.ThrowIfNull(execute); + + var pathArgument = new Argument("path") + { + Description = "The file or directory to analyze. Defaults to the current directory. Running sloc with no arguments at all shows this help instead.", + DefaultValueFactory = _ => "." + }; + + var listFileOption = new Option("--list-file") + { + Description = "Analyze exactly the files listed (one path per line) in this file instead of scanning a directory. Use '-' to read the list from stdin. Ignores 'path'." + }; + + var gitHashOption = new Option("--git-hash") + { + Description = "Analyze the repository tree as of this commit/tree-ish, without checking it out. " + + "'path' is used as the repo root to query. Requires git on PATH. Mutually exclusive with --list-file." + }; + + var compareToOption = new Option("--compare-to") + { + Description = "Diff the current analysis against 'path' as of this other commit/tree-ish, without checking it out or saving a baseline file first. " + + "Requires git on PATH. Combine with --git-hash to diff two commits directly. " + + "Mutually exclusive with --baseline, --watch, and --list-file; output is Table or Json only, same as --baseline." + }; + + var includeOption = new Option("--include", "-i") + { + Description = "Glob pattern of files to include. Can be specified multiple times.", + }; + + var excludeOption = new Option("--exclude", "-e") + { + Description = "Glob pattern of files to exclude. Can be specified multiple times.", + }; + + var excludeDirOption = new Option("--exclude-dir") + { + Description = "Directory name to exclude at any depth (e.g. \"vendor\"). " + + "Shorthand for --exclude \"**//**\". Can be specified multiple times.", + }; + + var supportedLangNames = string.Join(", ", LanguageRegistry.Languages + .Select(language => language.Name) + .OrderBy(name => name, StringComparer.OrdinalIgnoreCase)); + + var includeLangOption = new Option("--include-lang") + { + Description = $"Language name to include, matched case-insensitively. Can be specified multiple times. Supported: {supportedLangNames}.", + }; + + var excludeLangOption = new Option("--exclude-lang") + { + Description = $"Language name to exclude, matched case-insensitively. Can be specified multiple times. Supported: {supportedLangNames}.", + }; + + var formatOption = new Option("--format", "-f") + { + Description = "Output format: Table, Json, Html, Csv, or Markdown. Inferred from the --output extension when omitted." + }; + + var noRecursiveOption = new Option("--no-recursive") + { + Description = "Do not descend into subdirectories." + }; + + var noHealthOption = new Option("--no-health") + { + Description = "Hide the Comment Health column and percentage breakdowns." + }; + + var noComplexityOption = new Option("--no-complexity") + { + Description = "Hide the Complexity column/field." + }; + + var byFileOption = new Option("--by-file") + { + Description = "Show a per-file breakdown in addition to the language summary." + }; + + var detailedOption = new Option("--detailed") + { + Description = "In Json, Html, and Markdown output, include both the language summary and the per-file breakdown. In Csv output, selects the language summary only." + }; + + var pagedOption = new Option("--paged", "-p") + { + Description = "Enable pagination when displaying the per-file breakdown." + }; + + var allOption = new Option("--all") + { + Description = "Include files whose extension maps to no known language." + }; + + var outputOption = new Option("--output", "-o") + { + Description = "Output file path for Json, Html, Csv, and Markdown formats. Use '-' to write to stdout." + }; + + var quietOption = new Option("--quiet", "-q") + { + Description = "Suppress the banner, progress UI, and 'Saved to' message." + }; + + var noProgressOption = new Option("--no-progress") + { + Description = "Suppress the live table and progress bar." + }; + + var watchOption = new Option("--watch", "-w") + { + Description = "Watch the path for file changes and re-run the analysis, refreshing the table. Table format only; press Ctrl+C to stop." + }; + + var minCommentPctOption = new Option("--min-comment-pct") + { + Description = "Fail (exit code 2) if the overall comment percentage is below this value." + }; + + var jobsOption = new Option("--jobs", "-j") + { + Description = "Maximum number of files to analyze in parallel. Defaults to the processor count (also used for 0 or less); use 1 for sequential." + }; + + var noGitignoreOption = new Option("--no-gitignore") + { + Description = "Do not honor .gitignore files (they are respected by default), and scan " + + "nested repositories such as submodules, which are otherwise skipped inside a repository " + + "(--compare-to always skips them)." + }; + + var noGitAttributesOption = new Option("--no-gitattributes") + { + Description = "Do not exclude files marked linguist-vendored or linguist-generated in " + + ".gitattributes (excluded by default)." + }; + + var followSymlinksOption = new Option("--follow-symlinks") + { + Description = "Include symlinked/junctioned directories and symlinked files instead of skipping them. " + + "A directory symlink that loops directly back to one of its own ancestors is still skipped." + }; + + var baselineOption = new Option("--baseline") + { + Description = "Compare against a previously saved JSON report and show the line-count diff." + }; + + var sortOption = new Option("--sort") + { + Description = "Order the language summary by: Total (default), Code, Comment, Blank, Files, Name, or CommentPct." + }; + + var topOption = new Option("--top") + { + Description = "Show only the top N languages in the summary." + }; + + var noUpdateCheckOption = new Option("--no-update-check") + { + Description = "Do not check GitHub for a newer release (checked by default, with a 2 second timeout)." + }; + + var uniqueOption = new Option("--unique") + { + Description = "Count each distinct file content only once; later byte-identical duplicates are reported as skipped." + }; + + var rootCommand = new RootCommand("Sloc - counts code, comment, and blank lines in source files.") + { + pathArgument, + listFileOption, + gitHashOption, + compareToOption, + includeOption, + excludeOption, + excludeDirOption, + includeLangOption, + excludeLangOption, + formatOption, + noRecursiveOption, + byFileOption, + detailedOption, + pagedOption, + allOption, + outputOption, + noHealthOption, + noComplexityOption, + quietOption, + noProgressOption, + watchOption, + minCommentPctOption, + jobsOption, + noGitignoreOption, + noGitAttributesOption, + followSymlinksOption, + baselineOption, + sortOption, + topOption, + noUpdateCheckOption, + uniqueOption + }; + + var helpOpt = rootCommand.Options.OfType().FirstOrDefault(); + helpOpt?.Description = "Show help and usage information."; + + var versionOpt = rootCommand.Options.OfType().FirstOrDefault(); + versionOpt?.Description = "Show version information."; + + rootCommand.SetAction(parseResult => + { + var outputFile = parseResult.GetValue(outputOption); + var explicitFormat = parseResult.GetValue(formatOption); + + var format = FormatResolver.Resolve(explicitFormat, outputFile); + + // Table output never writes a file, so an --output path would be silently discarded. + // Warn rather than fail so scripts relying on the exit code are unaffected. + if (FormatResolver.OutputIgnoredForTable(format, outputFile)) + { + Console.Error.WriteLine( + "sloc: --output is ignored for Table format; pass -f json|html|csv|markdown or use a recognized file extension."); + } + + var minCommentPct = parseResult.GetValue(minCommentPctOption); + if (!CliArgumentValidation.IsValidMinCommentPct(minCommentPct)) + { + Console.Error.WriteLine("sloc: --min-comment-pct must be between 0 and 100."); + return ExitCode.Error; + } + + var top = parseResult.GetValue(topOption); + if (!CliArgumentValidation.IsValidTop(top)) + { + Console.Error.WriteLine("sloc: --top must be 1 or greater."); + return ExitCode.Error; + } + + if (!Enum.IsDefined(parseResult.GetValue(sortOption))) + { + Console.Error.WriteLine("sloc: --sort must be Total, Code, Comment, Blank, Files, Name, or CommentPct."); + return ExitCode.Error; + } + + var listFile = parseResult.GetValue(listFileOption); + var gitHash = parseResult.GetValue(gitHashOption); + if (listFile is not null && gitHash is not null) + { + Console.Error.WriteLine("sloc: --git-hash and --list-file cannot be used together."); + return ExitCode.Error; + } + + var watch = parseResult.GetValue(watchOption); + var baselinePath = parseResult.GetValue(baselineOption); + var watchError = CliArgumentValidation.ValidateWatch(watch, listFile, gitHash, format, baselinePath); + if (watchError is not null) + { + Console.Error.WriteLine(watchError); + return ExitCode.Error; + } + + var compareTo = parseResult.GetValue(compareToOption); + var compareToError = CliArgumentValidation.ValidateCompareTo(compareTo, baselinePath, watch, listFile); + if (compareToError is not null) + { + Console.Error.WriteLine(compareToError); + return ExitCode.Error; + } + + var options = new AnalyzeOptions + { + Path = parseResult.GetValue(pathArgument) ?? ".", + ListFile = listFile, + GitHash = gitHash, + CompareTo = compareTo, + Includes = parseResult.GetValue(includeOption) ?? [], + Excludes = [ + .. parseResult.GetValue(excludeOption) ?? [], + .. (parseResult.GetValue(excludeDirOption) ?? []).Select(name => $"**/{name}/**") + ], + IncludeLangs = parseResult.GetValue(includeLangOption) ?? [], + ExcludeLangs = parseResult.GetValue(excludeLangOption) ?? [], + Format = format, + NoRecursive = parseResult.GetValue(noRecursiveOption), + ByFile = parseResult.GetValue(byFileOption), + Detailed = parseResult.GetValue(detailedOption), + Paged = parseResult.GetValue(pagedOption), + IncludeUnknown = parseResult.GetValue(allOption), + OutputFile = outputFile, + NoHealth = parseResult.GetValue(noHealthOption), + NoComplexity = parseResult.GetValue(noComplexityOption), + Quiet = parseResult.GetValue(quietOption), + NoProgress = parseResult.GetValue(noProgressOption), + MinCommentPct = minCommentPct, + Jobs = parseResult.GetValue(jobsOption), + RespectGitignore = !parseResult.GetValue(noGitignoreOption), + RespectGitAttributes = !parseResult.GetValue(noGitAttributesOption), + FollowSymlinks = parseResult.GetValue(followSymlinksOption), + BaselinePath = baselinePath, + Sort = parseResult.GetValue(sortOption), + Top = top, + NoUpdateCheck = parseResult.GetValue(noUpdateCheckOption), + Unique = parseResult.GetValue(uniqueOption), + Watch = watch + }; + + return execute(options); + }); + + return rootCommand; + } +} diff --git a/src/Sloc.Cli/Program.cs b/src/Sloc.Cli/Program.cs index 25b2393..891809d 100644 --- a/src/Sloc.Cli/Program.cs +++ b/src/Sloc.Cli/Program.cs @@ -1,327 +1,25 @@ using Sloc.Cli.Analysis; using Sloc.Cli.Parsing; -using Sloc.Core.Languages; -using Sloc.Core.Models; -using System.CommandLine; -using System.CommandLine.Help; using System.Globalization; // No BOM: matches the encoding AnalyzeHandler.WriteToFile uses for "-o ", so piped or // redirected stdout output (the default, or "-o -") parses the same way file output does for // a strict JSON/CSV consumer (e.g. jq). -Console.OutputEncoding = new System.Text.UTF8Encoding(encoderShouldEmitUTF8Identifier: false); +try +{ + Console.OutputEncoding = new System.Text.UTF8Encoding(encoderShouldEmitUTF8Identifier: false); +} +catch (IOException) +{ + // No console is attached (e.g. a detached or service process); the encoding only matters + // for a console, so keep the default rather than crashing before any work starts. +} // Render all numbers with the invariant culture so report output (thousands separators, // percentages) is deterministic regardless of the host's regional settings. CultureInfo.CurrentCulture = CultureInfo.InvariantCulture; CultureInfo.DefaultThreadCurrentCulture = CultureInfo.InvariantCulture; -var pathArgument = new Argument("path") -{ - Description = "The file or directory to analyze. Defaults to the current directory. Running sloc with no arguments at all shows this help instead.", - DefaultValueFactory = _ => "." -}; - -var listFileOption = new Option("--list-file") -{ - Description = "Analyze exactly the files listed (one path per line) in this file instead of scanning a directory. Use '-' to read the list from stdin. Ignores 'path'." -}; - -var gitHashOption = new Option("--git-hash") -{ - Description = "Analyze the repository tree as of this commit/tree-ish, without checking it out. " - + "'path' is used as the repo root to query. Requires git on PATH. Mutually exclusive with --list-file." -}; - -var compareToOption = new Option("--compare-to") -{ - Description = "Diff the current analysis against 'path' as of this other commit/tree-ish, without checking it out or saving a baseline file first. " - + "Requires git on PATH. Combine with --git-hash to diff two commits directly. " - + "Mutually exclusive with --baseline, --watch, and --list-file; output is Table or Json only, same as --baseline." -}; - -var includeOption = new Option("--include", "-i") -{ - Description = "Glob pattern of files to include. Can be specified multiple times.", - AllowMultipleArgumentsPerToken = true -}; - -var excludeOption = new Option("--exclude", "-e") -{ - Description = "Glob pattern of files to exclude. Can be specified multiple times.", - AllowMultipleArgumentsPerToken = true -}; - -var excludeDirOption = new Option("--exclude-dir") -{ - Description = "Directory name to exclude at any depth (e.g. \"vendor\"). " - + "Shorthand for --exclude \"**//**\". Can be specified multiple times.", - AllowMultipleArgumentsPerToken = true -}; - -var supportedLangNames = string.Join(", ", LanguageRegistry.Languages - .Select(language => language.Name) - .OrderBy(name => name, StringComparer.OrdinalIgnoreCase)); - -var includeLangOption = new Option("--include-lang") -{ - Description = $"Language name to include, matched case-insensitively. Can be specified multiple times. Supported: {supportedLangNames}.", - AllowMultipleArgumentsPerToken = true -}; - -var excludeLangOption = new Option("--exclude-lang") -{ - Description = $"Language name to exclude, matched case-insensitively. Can be specified multiple times. Supported: {supportedLangNames}.", - AllowMultipleArgumentsPerToken = true -}; - -var formatOption = new Option("--format", "-f") -{ - Description = "Output format: Table, Json, Html, Csv, or Markdown. Inferred from the --output extension when omitted." -}; - -var noRecursiveOption = new Option("--no-recursive") -{ - Description = "Do not descend into subdirectories." -}; - -var noHealthOption = new Option("--no-health") -{ - Description = "Hide the Comment Health column and percentage breakdowns." -}; - -var noComplexityOption = new Option("--no-complexity") -{ - Description = "Hide the Complexity column/field." -}; - -var byFileOption = new Option("--by-file") -{ - Description = "Show a per-file breakdown in addition to the language summary." -}; - -var detailedOption = new Option("--detailed") -{ - Description = "In Json, Html, and Markdown output, include both the language summary and the per-file breakdown. In Csv output, selects the language summary only." -}; - -var pagedOption = new Option("--paged", "-p") -{ - Description = "Enable pagination when displaying the per-file breakdown." -}; - -var allOption = new Option("--all") -{ - Description = "Include files whose extension maps to no known language." -}; - -var outputOption = new Option("--output", "-o") -{ - Description = "Output file path for Json and Html formats. Use '-' to write to stdout." -}; - -var quietOption = new Option("--quiet", "-q") -{ - Description = "Suppress the banner, progress UI, and 'Saved to' message." -}; - -var noProgressOption = new Option("--no-progress") -{ - Description = "Suppress the live table and progress bar." -}; - -var watchOption = new Option("--watch", "-w") -{ - Description = "Watch the path for file changes and re-run the analysis, refreshing the table. Table format only; press Ctrl+C to stop." -}; - -var minCommentPctOption = new Option("--min-comment-pct") -{ - Description = "Fail (exit code 2) if the overall comment percentage is below this value." -}; - -var jobsOption = new Option("--jobs", "-j") -{ - Description = "Maximum number of files to analyze in parallel. Defaults to the processor count; use 1 for sequential." -}; - -var noGitignoreOption = new Option("--no-gitignore") -{ - Description = "Do not honor .gitignore files (they are respected by default), and scan " - + "nested repositories such as submodules, which are otherwise skipped inside a repository " - + "(--compare-to always skips them)." -}; - -var noGitAttributesOption = new Option("--no-gitattributes") -{ - Description = "Do not exclude files marked linguist-vendored or linguist-generated in " - + ".gitattributes (excluded by default)." -}; - -var followSymlinksOption = new Option("--follow-symlinks") -{ - Description = "Include symlinked/junctioned directories and symlinked files instead of skipping them. " - + "A directory symlink that loops directly back to one of its own ancestors is still skipped." -}; - -var baselineOption = new Option("--baseline") -{ - Description = "Compare against a previously saved JSON report and show the line-count diff." -}; - -var sortOption = new Option("--sort") -{ - Description = "Order the language summary by: Total (default), Code, Comment, Blank, Files, or Name." -}; - -var topOption = new Option("--top") -{ - Description = "Show only the top N languages in the summary." -}; - -var noUpdateCheckOption = new Option("--no-update-check") -{ - Description = "Do not check GitHub for a newer release (checked by default, with a 2 second timeout)." -}; - -var uniqueOption = new Option("--unique") -{ - Description = "Count each distinct file content only once; later byte-identical duplicates are reported as skipped." -}; - -var rootCommand = new RootCommand("Sloc - counts code, comment, and blank lines in source files.") -{ - pathArgument, - listFileOption, - gitHashOption, - compareToOption, - includeOption, - excludeOption, - excludeDirOption, - includeLangOption, - excludeLangOption, - formatOption, - noRecursiveOption, - byFileOption, - detailedOption, - pagedOption, - allOption, - outputOption, - noHealthOption, - noComplexityOption, - quietOption, - noProgressOption, - watchOption, - minCommentPctOption, - jobsOption, - noGitignoreOption, - noGitAttributesOption, - followSymlinksOption, - baselineOption, - sortOption, - topOption, - noUpdateCheckOption, - uniqueOption -}; - -var helpOpt = rootCommand.Options.OfType().FirstOrDefault(); -helpOpt?.Description = "Show help and usage information."; - -var versionOpt = rootCommand.Options.OfType().FirstOrDefault(); -versionOpt?.Description = "Show version information."; - -rootCommand.SetAction(parseResult => -{ - var outputFile = parseResult.GetValue(outputOption); - var explicitFormat = parseResult.GetValue(formatOption); - - var format = FormatResolver.Resolve(explicitFormat, outputFile); - - // Table output never writes a file, so an --output path would be silently discarded. - // Warn rather than fail so scripts relying on the exit code are unaffected. - if (FormatResolver.OutputIgnoredForTable(format, outputFile)) - { - Console.Error.WriteLine( - "sloc: --output is ignored for Table format; pass -f json|html|csv|markdown or use a recognized file extension."); - } - - var minCommentPct = parseResult.GetValue(minCommentPctOption); - if (!CliArgumentValidation.IsValidMinCommentPct(minCommentPct)) - { - Console.Error.WriteLine("sloc: --min-comment-pct must be between 0 and 100."); - return ExitCode.Error; - } - - var top = parseResult.GetValue(topOption); - if (!CliArgumentValidation.IsValidTop(top)) - { - Console.Error.WriteLine("sloc: --top must be 1 or greater."); - return ExitCode.Error; - } - - var listFile = parseResult.GetValue(listFileOption); - var gitHash = parseResult.GetValue(gitHashOption); - if (listFile is not null && gitHash is not null) - { - Console.Error.WriteLine("sloc: --git-hash and --list-file cannot be used together."); - return ExitCode.Error; - } - - var watch = parseResult.GetValue(watchOption); - var baselinePath = parseResult.GetValue(baselineOption); - var watchError = CliArgumentValidation.ValidateWatch(watch, listFile, gitHash, format, baselinePath); - if (watchError is not null) - { - Console.Error.WriteLine(watchError); - return ExitCode.Error; - } - - var compareTo = parseResult.GetValue(compareToOption); - var compareToError = CliArgumentValidation.ValidateCompareTo(compareTo, baselinePath, watch, listFile); - if (compareToError is not null) - { - Console.Error.WriteLine(compareToError); - return ExitCode.Error; - } - - var options = new AnalyzeOptions - { - Path = parseResult.GetValue(pathArgument) ?? ".", - ListFile = listFile, - GitHash = gitHash, - CompareTo = compareTo, - Includes = parseResult.GetValue(includeOption) ?? [], - Excludes = [ - .. parseResult.GetValue(excludeOption) ?? [], - .. (parseResult.GetValue(excludeDirOption) ?? []).Select(name => $"**/{name}/**") - ], - IncludeLangs = parseResult.GetValue(includeLangOption) ?? [], - ExcludeLangs = parseResult.GetValue(excludeLangOption) ?? [], - Format = format, - NoRecursive = parseResult.GetValue(noRecursiveOption), - ByFile = parseResult.GetValue(byFileOption), - Detailed = parseResult.GetValue(detailedOption), - Paged = parseResult.GetValue(pagedOption), - IncludeUnknown = parseResult.GetValue(allOption), - OutputFile = outputFile, - NoHealth = parseResult.GetValue(noHealthOption), - NoComplexity = parseResult.GetValue(noComplexityOption), - Quiet = parseResult.GetValue(quietOption), - NoProgress = parseResult.GetValue(noProgressOption), - MinCommentPct = minCommentPct, - Jobs = parseResult.GetValue(jobsOption), - RespectGitignore = !parseResult.GetValue(noGitignoreOption), - RespectGitAttributes = !parseResult.GetValue(noGitAttributesOption), - FollowSymlinks = parseResult.GetValue(followSymlinksOption), - BaselinePath = baselinePath, - Sort = parseResult.GetValue(sortOption), - Top = top, - NoUpdateCheck = parseResult.GetValue(noUpdateCheckOption), - Unique = parseResult.GetValue(uniqueOption), - Watch = watch - }; - - return new AnalyzeHandler().Execute(options); -}); +var rootCommand = SlocCommandFactory.Create(options => new AnalyzeHandler().Execute(options)); -return CommandInvoker.Invoke(rootCommand.Parse(args.Length == 0 ? ["--help"] : args), Console.Error); \ No newline at end of file +return CommandInvoker.Invoke(rootCommand.Parse(args.Length == 0 ? ["--help"] : args), Console.Error); diff --git a/tests/Sloc.Cli.Tests/SlocCommandFactoryTests.cs b/tests/Sloc.Cli.Tests/SlocCommandFactoryTests.cs new file mode 100644 index 0000000..bcb8913 --- /dev/null +++ b/tests/Sloc.Cli.Tests/SlocCommandFactoryTests.cs @@ -0,0 +1,127 @@ +using Sloc.Cli.Analysis; +using Sloc.Cli.Parsing; + +namespace Sloc.Cli.Tests; + +/// +/// Contains tests for , pinning how command lines map to +/// without running an analysis. +/// +public sealed class SlocCommandFactoryTests +{ + /// + /// Verifies that a repeatable option followed by a bare token leaves that token to the + /// path argument instead of swallowing it as another value, so + /// sloc -e "*.md" src analyzes src rather than the current directory. + /// + /// The command-line arguments. + /// The path the run should analyze. + [Theory] + [InlineData(new[] { "-e", "*.md", "src" }, "src")] + [InlineData(new[] { "--exclude", "*.md", "src" }, "src")] + [InlineData(new[] { "-i", "**/*.cs", "src" }, "src")] + [InlineData(new[] { "--exclude-dir", "vendor", "./src" }, "./src")] + [InlineData(new[] { "--include-lang", "C#", "src" }, "src")] + [InlineData(new[] { "--exclude-lang", "Markdown", "src" }, "src")] + public void Create_RepeatableOptionThenBareToken_TokenIsPath(string[] args, string expectedPath) + { + var (options, exitCode) = Run(args); + + Assert.Equal(ExitCode.Success, exitCode); + Assert.NotNull(options); + Assert.Equal(expectedPath, options.Path); + } + + /// + /// Verifies that repeatable options take exactly one value per occurrence, so a second + /// bare token after -e is the path and not a further exclude glob. + /// + [Fact] + public void Create_ExcludeFollowedByTwoTokens_SecondTokenIsPath() + { + var (options, _) = Run(["-e", "a", "b"]); + + Assert.NotNull(options); + Assert.Equal(["a"], options.Excludes); + Assert.Equal("b", options.Path); + } + + /// + /// Verifies that repeating an option accumulates its values, and that + /// --exclude-dir names are expanded to **/name/** globs after --exclude. + /// + [Fact] + public void Create_RepeatedOptions_AccumulateValues() + { + var (options, _) = Run(["-e", "a", "-e", "b", "--exclude-dir", "vendor", "-i", "x", "-i", "y", "src"]); + + Assert.NotNull(options); + Assert.Equal(["a", "b", "**/vendor/**"], options.Excludes); + Assert.Equal(["x", "y"], options.Includes); + Assert.Equal("src", options.Path); + } + + /// + /// Verifies that the path defaults to the current directory when omitted. + /// + [Fact] + public void Create_NoPath_DefaultsToCurrentDirectory() + { + var (options, _) = Run(["-e", "*.md"]); + + Assert.NotNull(options); + Assert.Equal(".", options.Path); + } + + /// + /// Verifies that a non-positive --jobs is accepted and passed through, since the + /// handler treats it as "use the processor count". + /// + /// The --jobs argument text. + /// The parsed job count. + [Theory] + [InlineData("0", 0)] + [InlineData("-1", -1)] + [InlineData("4", 4)] + public void Create_Jobs_AcceptsNonPositiveValues(string jobs, int expected) + { + var (options, exitCode) = Run(["--jobs", jobs]); + + Assert.Equal(ExitCode.Success, exitCode); + Assert.NotNull(options); + Assert.Equal(expected, options.Jobs); + } + + /// + /// Verifies that an out-of-range --min-comment-pct is rejected before the analysis + /// runs, with the path-error exit code. + /// + [Fact] + public void Create_MinCommentPctOutOfRange_ReturnsErrorWithoutExecuting() + { + var (options, exitCode) = Run(["--min-comment-pct", "101"]); + + Assert.Equal(ExitCode.Error, exitCode); + Assert.Null(options); + } + + /// + /// Parses against a command whose action records the options it + /// receives instead of analyzing anything. + /// + /// The command-line arguments. + private static (AnalyzeOptions? Options, int ExitCode) Run(string[] args) + { + AnalyzeOptions? captured = null; + var command = SlocCommandFactory.Create(options => + { + captured = options; + return ExitCode.Success; + }); + using var error = new StringWriter(); + + var exitCode = CommandInvoker.Invoke(command.Parse(args), error); + + return (captured, exitCode); + } +} From 8982df06e6f6e758600bc66913da4d4c668aaf32 Mon Sep 17 00:00:00 2001 From: Gaoyang Date: Wed, 30 Sep 2026 17:48:49 +0800 Subject: [PATCH 09/10] fix: make the comment threshold gate report precisely and hide temp paths in skip reasons Warn when no files were analyzed instead of silently passing, print the values with enough precision that 4.96% is not reported as '5.0% below 5.0%', and rewrite temporary snapshot paths embedded in skip reasons back to git paths. --- .../Analysis/AnalyzeHandler.Support.cs | 26 ++++++++-- .../AnalyzeHandlerWatchTests.cs | 50 +++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/Sloc.Cli/Analysis/AnalyzeHandler.Support.cs b/src/Sloc.Cli/Analysis/AnalyzeHandler.Support.cs index d1c6fcf..c80bd67 100644 --- a/src/Sloc.Cli/Analysis/AnalyzeHandler.Support.cs +++ b/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 RemapGitPaths(List 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) /// The summary to check the comment percentage of. 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; } diff --git a/tests/Sloc.Cli.Tests/AnalyzeHandlerWatchTests.cs b/tests/Sloc.Cli.Tests/AnalyzeHandlerWatchTests.cs index b241f9b..7a954c0 100644 --- a/tests/Sloc.Cli.Tests/AnalyzeHandlerWatchTests.cs +++ b/tests/Sloc.Cli.Tests/AnalyzeHandlerWatchTests.cs @@ -44,6 +44,56 @@ public void WatchExitCode_MeetsCommentThreshold_ReturnsSuccess() Assert.Equal(ExitCode.Success, exitCode); } + /// + /// Verifies that a percentage which rounds up to the threshold at one decimal is not + /// reported as "5.0% is below the required 5.0%". + /// + [Fact] + public void WatchExitCode_BelowThresholdByFraction_MessageShowsDistinctValues() + { + var summary = new AnalysisSummary([ + new FileAnalysis { Path = "a.cs", Language = "C#", Code = 2500, Comment = 130, Blank = 0 } + ]); + var original = Console.Error; + using var writer = new StringWriter(); + Console.SetError(writer); + int exitCode; + try + { + exitCode = AnalyzeHandler.WatchExitCode(new AnalyzeOptions { Path = ".", MinCommentPct = 5 }, summary); + } + finally + { + Console.SetError(original); + } + + Assert.Equal(ExitCode.ThresholdNotMet, exitCode); + Assert.DoesNotContain("5.0% is below the required 5.0%", writer.ToString()); + } + + /// + /// Verifies that an empty result set warns on stderr instead of silently passing the gate. + /// + [Fact] + public void WatchExitCode_NoFilesAnalyzed_WarnsAndSucceeds() + { + var original = Console.Error; + using var writer = new StringWriter(); + Console.SetError(writer); + int exitCode; + try + { + exitCode = AnalyzeHandler.WatchExitCode(new AnalyzeOptions { Path = ".", MinCommentPct = 50 }, new AnalysisSummary([])); + } + finally + { + Console.SetError(original); + } + + Assert.Equal(ExitCode.Success, exitCode); + Assert.Contains("no files were analyzed", writer.ToString()); + } + /// /// Verifies that no threshold was ever configured, or the watch loop ended before /// completing a single pass, still yields success rather than throwing. From b3f774498fee50956613a11bc962ba8692c4ab4d Mon Sep 17 00:00:00 2001 From: Gaoyang Date: Wed, 30 Sep 2026 17:48:50 +0800 Subject: [PATCH 10/10] fix: remove snapshot temp directories on Ctrl+C and report invalid paths as errors Register cleanups for --git-hash and --compare-to extractions so an interrupt deletes the temp directory, reject --watch when stdout is redirected and keep the final table on exit, and map empty or malformed --baseline and --list-file paths to the path-error exit code instead of an unhandled exception. --- src/Sloc.Cli/Analysis/AnalyzeHandler.cs | 159 +++++++++++++++++++- src/Sloc.Cli/Output/DiffRenderer.cs | 2 +- tests/Sloc.Cli.Tests/AnalyzeHandlerTests.cs | 64 ++++++++ 3 files changed, 219 insertions(+), 6 deletions(-) diff --git a/src/Sloc.Cli/Analysis/AnalyzeHandler.cs b/src/Sloc.Cli/Analysis/AnalyzeHandler.cs index 01bdf51..da832ce 100644 --- a/src/Sloc.Cli/Analysis/AnalyzeHandler.cs +++ b/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(); + /// + /// Guards , which the Ctrl+C handler reads from another thread. + /// + private readonly Lock _cleanupLock = new(); + + /// + /// Deleters for the git snapshot temp directories that still exist (or are being + /// extracted), so an interrupt (which skips finally/using) can remove them + /// before the process exits. + /// + private readonly List _interruptCleanups = []; + /// /// Runs the analysis described by . /// @@ -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; + } + } + + /// + /// 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. + /// + /// The event source. + /// The event data. + private void OnInterrupt(object? sender, ConsoleCancelEventArgs e) + { + Action[] cleanups; + lock (_cleanupLock) + { + cleanups = [.. _interruptCleanups]; + } + + foreach (var cleanup in cleanups) + { + cleanup(); + } + } + + /// + /// Registers to run if the user interrupts the process. + /// + /// The temp-directory deleter to run on interrupt. + private void TrackCleanup(Action cleanup) + { + lock (_cleanupLock) + { + _interruptCleanups.Add(cleanup); + } + } + + /// + /// Stops tracking once its directory has been handled normally. + /// + /// The deleter previously passed to . + private void UntrackCleanup(Action cleanup) + { + lock (_cleanupLock) + { + _interruptCleanups.Remove(cleanup); + } + } + + /// + /// Extracts 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 once it has disposed the snapshot. + /// + /// The path inside the repository to query. + /// The commit/tree-ish to extract. + /// The snapshot and the interrupt cleanup registered for it. + 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); + } + } + + /// + /// Runs a single (non-watch) analysis using the already-built . + /// + /// The parsed options. + /// The run context built by . + 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 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) /// The parsed options. 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); + } + } + /// + /// Scans and analyzes an extracted for + /// . + /// + /// The extracted snapshot of the compared ref. + /// The path passed as path. + /// The scan options to apply to the extracted snapshot. + /// The parsed options. + private AnalysisSummary AnalyzeSnapshot(GitSnapshot snapshot, string repoPath, ScanOptions scanOptions, AnalyzeOptions options) + { var isFilePath = File.Exists(repoPath); var filesInScope = snapshot.Files; IEnumerable 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 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) diff --git a/src/Sloc.Cli/Output/DiffRenderer.cs b/src/Sloc.Cli/Output/DiffRenderer.cs index d61c225..296ef19 100644 --- a/src/Sloc.Cli/Output/DiffRenderer.cs +++ b/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}"); } diff --git a/tests/Sloc.Cli.Tests/AnalyzeHandlerTests.cs b/tests/Sloc.Cli.Tests/AnalyzeHandlerTests.cs index 0faf767..b79691f 100644 --- a/tests/Sloc.Cli.Tests/AnalyzeHandlerTests.cs +++ b/tests/Sloc.Cli.Tests/AnalyzeHandlerTests.cs @@ -96,6 +96,45 @@ public void Execute_BaselineWithUnsupportedFormatAndOutputFile_ReturnsErrorAndDo Assert.False(File.Exists(reportPath)); } + /// + /// Verifies that an empty --baseline path is reported as a path error (exit 1) + /// rather than crashing with an unhandled . + /// + [Fact] + public void Execute_EmptyBaselinePath_ReturnsError() + { + File.WriteAllText(Path.Combine(_root, "a.cs"), "int x = 1;\n"); + + var exitCode = 0; + CaptureStdout(() => exitCode = new AnalyzeHandler().Execute(new AnalyzeOptions + { + Path = _root, + Format = OutputFormat.Json, + BaselinePath = "", + Quiet = true + })); + + Assert.Equal(ExitCode.Error, exitCode); + } + + /// + /// Verifies that an empty --list-file path is reported as a path error (exit 1). + /// + [Fact] + public void Execute_EmptyListFilePath_ReturnsError() + { + var exitCode = 0; + CaptureStdout(() => exitCode = new AnalyzeHandler().Execute(new AnalyzeOptions + { + Path = _root, + Format = OutputFormat.Json, + ListFile = "", + Quiet = true + })); + + Assert.Equal(ExitCode.Error, exitCode); + } + /// /// Verifies that the comment-percentage threshold gate returns the threshold exit /// code when the comment percentage is below the requested minimum. @@ -567,6 +606,31 @@ public void Execute_ParallelAndSequential_ProduceIdenticalOutput() Assert.Equal(NormalizeGeneratedAt(sequential), NormalizeGeneratedAt(parallel)); } + /// + /// Verifies that a non-positive --jobs value falls back to the processor count + /// instead of failing, as documents. + /// + /// The non-positive job count. + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void Execute_NonPositiveJobs_FallsBackToAutomatic(int jobs) + { + File.WriteAllText(Path.Combine(_root, "a.cs"), "int x = 1;\n"); + + var exitCode = 0; + var stdout = CaptureStdout(() => exitCode = new AnalyzeHandler().Execute(new AnalyzeOptions + { + Path = _root, + Format = OutputFormat.Json, + Quiet = true, + Jobs = jobs + })); + + Assert.Equal(ExitCode.Success, exitCode); + Assert.Contains("\"fileCount\": 1", stdout); + } + private static string NormalizeGeneratedAt(string json) => Regex.Replace(json, "\"generatedAt\": \"[^\"]*\"", "\"generatedAt\": \"\"");