From ed522c718992a57faa055fff033ac4238dcd2ec0 Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:52:51 -0700 Subject: [PATCH 1/2] always add file name to error --- .../src/DocfxVerifier/PathVerifier.cs | 41 +++++++++++++++-- .../RedirectTargetVerifier.cs | 44 +++++++++++++++++-- .../GitHub.UnitTests/PathVerifierTests.cs | 2 +- .../RedirectTargetVerifierTests.cs | 6 ++- 4 files changed, 84 insertions(+), 9 deletions(-) diff --git a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs index 74ae3593..5cad2e57 100644 --- a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs +++ b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs @@ -268,9 +268,44 @@ int GetLineNumber(long tokenStartIndex) } private static Task WriteErrorAsync(TextWriter writer, string filePath, int? lineNumber, string message) - => lineNumber.HasValue - ? writer.WriteLineAsync($"::error file={filePath},line={lineNumber.Value}::{message}") - : writer.WriteLineAsync($"::error file={filePath}::{message}"); + { + string annotationFilePath = GetAnnotationFilePath(filePath); + string escapedMessage = EscapeCommandData(message); + return lineNumber.HasValue + ? writer.WriteLineAsync($"::error file={annotationFilePath},line={lineNumber.Value}::{escapedMessage}") + : writer.WriteLineAsync($"::error file={annotationFilePath}::{escapedMessage}"); + } + + private static string GetAnnotationFilePath(string filePath) + { + string normalizedPath = NormalizePath(filePath); + if (!Path.IsPathRooted(filePath)) + { + return EscapeCommandProperty(normalizedPath); + } + + string repositoryRoot = Path.GetFullPath(Directory.GetCurrentDirectory()); + string fullPath = Path.GetFullPath(filePath); + string relativePath = NormalizePath(Path.GetRelativePath(repositoryRoot, fullPath)); + bool isUnderRepository = !relativePath.Equals("..", StringComparison.Ordinal) + && !relativePath.StartsWith("../", StringComparison.Ordinal); + string pathForAnnotation = isUnderRepository ? relativePath : normalizedPath; + return EscapeCommandProperty(pathForAnnotation); + } + + private static string EscapeCommandProperty(string value) + => value + .Replace("%", "%25", StringComparison.Ordinal) + .Replace("\r", "%0D", StringComparison.Ordinal) + .Replace("\n", "%0A", StringComparison.Ordinal) + .Replace(":", "%3A", StringComparison.Ordinal) + .Replace(",", "%2C", StringComparison.Ordinal); + + private static string EscapeCommandData(string value) + => value + .Replace("%", "%25", StringComparison.Ordinal) + .Replace("\r", "%0D", StringComparison.Ordinal) + .Replace("\n", "%0A", StringComparison.Ordinal); private readonly record struct ValidationError(int? LineNumber, string Path); diff --git a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs index c028e54e..a725b914 100644 --- a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs +++ b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs @@ -241,7 +241,45 @@ int GetLineNumber(long tokenStartIndex) } private static Task WriteErrorAsync(TextWriter writer, string filePath, int? lineNumber, string message) - => lineNumber.HasValue - ? writer.WriteLineAsync($"::error file={filePath},line={lineNumber.Value}::{message}") - : writer.WriteLineAsync($"::error file={filePath}::{message}"); + { + string annotationFilePath = GetAnnotationFilePath(filePath); + string escapedMessage = EscapeCommandData(message); + return lineNumber.HasValue + ? writer.WriteLineAsync($"::error file={annotationFilePath},line={lineNumber.Value}::{escapedMessage}") + : writer.WriteLineAsync($"::error file={annotationFilePath}::{escapedMessage}"); + } + + private static string GetAnnotationFilePath(string filePath) + { + string normalizedPath = NormalizePath(filePath); + if (!Path.IsPathRooted(filePath)) + { + return EscapeCommandProperty(normalizedPath); + } + + string repositoryRoot = Path.GetFullPath(Directory.GetCurrentDirectory()); + string fullPath = Path.GetFullPath(filePath); + string relativePath = NormalizePath(Path.GetRelativePath(repositoryRoot, fullPath)); + bool isUnderRepository = !relativePath.Equals("..", StringComparison.Ordinal) + && !relativePath.StartsWith("../", StringComparison.Ordinal); + string pathForAnnotation = isUnderRepository ? relativePath : normalizedPath; + return EscapeCommandProperty(pathForAnnotation); + } + + private static string EscapeCommandProperty(string value) + => value + .Replace("%", "%25", StringComparison.Ordinal) + .Replace("\r", "%0D", StringComparison.Ordinal) + .Replace("\n", "%0A", StringComparison.Ordinal) + .Replace(":", "%3A", StringComparison.Ordinal) + .Replace(",", "%2C", StringComparison.Ordinal); + + private static string EscapeCommandData(string value) + => value + .Replace("%", "%25", StringComparison.Ordinal) + .Replace("\r", "%0D", StringComparison.Ordinal) + .Replace("\n", "%0A", StringComparison.Ordinal); + + private static string NormalizePath(string path) + => path.Replace('\\', '/'); } diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs index f4a0a3df..39d0b920 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs +++ b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs @@ -201,7 +201,7 @@ await File.WriteAllTextAsync("docfx.json", """ Assert.False(result); Assert.Contains("Invalid path 'missing/path/**/**.{md,yml}'.", output, StringComparison.Ordinal); - Assert.Contains(",line=5::Invalid path 'missing/path/**/**.{md,yml}'.", output, StringComparison.Ordinal); + Assert.Contains("::error file=docfx.json,line=5::Invalid path 'missing/path/**/**.{md,yml}'.", output, StringComparison.Ordinal); } finally { diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs index d3f44842..38149906 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs +++ b/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs @@ -68,8 +68,10 @@ public async Task WriteResultsAsyncReturnsFalseFor404Url() _ => Task.FromResult(HttpStatusCode.NotFound)); Assert.False(result); - Assert.Contains("returns 404", writer.ToString(), StringComparison.Ordinal); - Assert.Contains(",line=5::Redirect target returns 404", writer.ToString(), StringComparison.Ordinal); + string output = writer.ToString(); + Assert.Contains("returns 404", output, StringComparison.Ordinal); + Assert.Contains("::error file=", output, StringComparison.Ordinal); + Assert.Contains(",line=5::Redirect target returns 404", output, StringComparison.Ordinal); } finally { From 28ec26d5704ec3290100dab8a70f504fd309524a Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Fri, 11 Sep 2026 13:10:09 -0700 Subject: [PATCH 2/2] respond to feedback --- .../src/DocfxVerifier/PathVerifier.cs | 12 +++- .../RedirectTargetVerifier.cs | 6 +- .../GitHub.UnitTests/PathVerifierTests.cs | 55 +++++++++++++++++++ .../RedirectTargetVerifierTests.cs | 19 ++++++- 4 files changed, 88 insertions(+), 4 deletions(-) diff --git a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs index 5cad2e57..25105e07 100644 --- a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs +++ b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs @@ -29,13 +29,21 @@ public static async Task WriteResultsAsync(TextWriter writer, string? conf configurationPath ??= FindDocfxConfigurationPath(); if (configurationPath is null) { - await writer.WriteLineAsync("::error::Unable to find docfx.json in the repository root or its immediate subdirectories."); + await WriteErrorAsync( + writer, + "docfx.json", + lineNumber: null, + "Unable to find docfx.json in the repository root or its immediate subdirectories."); return false; } if (!File.Exists(configurationPath)) { - await writer.WriteLineAsync($"::error::docfx.json file '{configurationPath}' does not exist."); + await WriteErrorAsync( + writer, + configurationPath, + lineNumber: null, + $"docfx.json file '{configurationPath}' does not exist."); return false; } diff --git a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs index a725b914..6a3817c3 100644 --- a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs +++ b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs @@ -31,7 +31,11 @@ internal static async Task WriteResultsAsync( if (!File.Exists(redirectionFilePath)) { - await writer.WriteLineAsync($"::error::Redirection file '{redirectionFilePath}' does not exist."); + await WriteErrorAsync( + writer, + redirectionFilePath, + lineNumber: null, + $"Redirection file '{redirectionFilePath}' does not exist."); return false; } diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs index 39d0b920..5982854f 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs +++ b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs @@ -345,6 +345,61 @@ await File.WriteAllTextAsync(modifiedDocfxPath, """ } } + [Fact] + public async Task WriteResultsAsyncReturnsFalseForMissingSpecifiedDocfxPathWithFileAnnotation() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + string missingDocfxPath = Path.Combine("missing-docs", "docfx.json"); + using var writer = new StringWriter(); + + bool result = await PathVerifier.WriteResultsAsync(writer, missingDocfxPath); + + string output = writer.ToString(); + Assert.False(result); + Assert.Contains("docfx.json file", output, StringComparison.Ordinal); + Assert.Contains("file=missing-docs/docfx.json", output, StringComparison.Ordinal); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + + [Fact] + public async Task WriteResultsAsyncReturnsFalseWhenDocfxNotFoundWithFileAnnotation() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + using var writer = new StringWriter(); + + bool result = await PathVerifier.WriteResultsAsync(writer); + + string output = writer.ToString(); + Assert.False(result); + Assert.Contains("Unable to find docfx.json", output, StringComparison.Ordinal); + Assert.Contains("file=docfx.json", output, StringComparison.Ordinal); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + [Fact] public async Task WriteResultsAsyncAllowsTrailingCommas() { diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs index 38149906..3f2ecd97 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs +++ b/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs @@ -70,7 +70,7 @@ public async Task WriteResultsAsyncReturnsFalseFor404Url() Assert.False(result); string output = writer.ToString(); Assert.Contains("returns 404", output, StringComparison.Ordinal); - Assert.Contains("::error file=", output, StringComparison.Ordinal); + Assert.Contains(Path.GetFileName(redirectionFilePath), output, StringComparison.Ordinal); Assert.Contains(",line=5::Redirect target returns 404", output, StringComparison.Ordinal); } finally @@ -102,6 +102,23 @@ public async Task WriteResultsAsyncReturnsFalseWhenLearnUrlCannotBeVerified() } } + [Fact] + public async Task WriteResultsAsyncReturnsFalseForMissingRedirectionFileWithFileAnnotation() + { + string redirectionFilePath = Path.Combine(Path.GetTempPath(), $"redirect-missing-{Guid.NewGuid():N}.json"); + using var writer = new StringWriter(); + + bool result = await RedirectTargetVerifier.WriteResultsAsync( + writer, + redirectionFilePath, + _ => Task.FromResult(HttpStatusCode.OK)); + + string output = writer.ToString(); + Assert.False(result); + Assert.Contains("Redirection file", output, StringComparison.Ordinal); + Assert.Contains(Path.GetFileName(redirectionFilePath), output, StringComparison.Ordinal); + } + private static async Task CreateRedirectionFileAsync(string redirectUrl) { string content = $$"""