From 68627c4667b8f8aaefefa7b0219274a099082dc0 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 17 Mar 2025 21:50:36 +0100 Subject: [PATCH 1/2] Add support for diagnostic hints and track unused substitution keys Introduced a "Hint" severity level to diagnostics for improvement suggestions. Enhanced substitution tracking to collect unused keys and provide hints, optimizing substitution usage. Updated console and GitHub outputs to display diagnostic hints alongside errors and warnings for better feedback. --- .../Console/ConsoleDiagnosticsCollector.cs | 11 ++- .../Console/ErrataFileSourceRepository.cs | 49 ++++++++++-- .../Console/GithubAnnotationOutput.cs | 2 + .../Diagnostics/Log.cs | 8 +- .../Diagnostics/DiagnosticsChannel.cs | 75 +++++++++++-------- .../DocumentationGenerator.cs | 20 +++++ src/Elastic.Markdown/Helpers/Interpolation.cs | 19 ++--- src/Elastic.Markdown/IO/MarkdownFile.cs | 6 +- .../CodeBlocks/EnhancedCodeBlockParser.cs | 4 +- .../Interpolation/InterpolationTests.cs | 4 +- 10 files changed, 138 insertions(+), 60 deletions(-) diff --git a/src/Elastic.Documentation.Tooling/Diagnostics/Console/ConsoleDiagnosticsCollector.cs b/src/Elastic.Documentation.Tooling/Diagnostics/Console/ConsoleDiagnosticsCollector.cs index f863a1163..d40e285b8 100644 --- a/src/Elastic.Documentation.Tooling/Diagnostics/Console/ConsoleDiagnosticsCollector.cs +++ b/src/Elastic.Documentation.Tooling/Diagnostics/Console/ConsoleDiagnosticsCollector.cs @@ -16,13 +16,16 @@ public class ConsoleDiagnosticsCollector(ILoggerFactory loggerFactory, ICoreServ { private readonly List _errors = []; private readonly List _warnings = []; + private readonly List _hints = []; protected override void HandleItem(Diagnostic diagnostic) { - if (diagnostic.Severity == Severity.Warning) + if (diagnostic.Severity == Severity.Error) + _errors.Add(diagnostic); + else if (diagnostic.Severity == Severity.Warning) _warnings.Add(diagnostic); else - _errors.Add(diagnostic); + _hints.Add(diagnostic); } private bool _stopped; @@ -32,10 +35,10 @@ public override async Task StopAsync(Cancel cancellationToken) return; _stopped = true; var repository = new ErrataFileSourceRepository(); - repository.WriteDiagnosticsToConsole(_errors, _warnings); + repository.WriteDiagnosticsToConsole(_errors, _warnings, _hints); AnsiConsole.WriteLine(); - AnsiConsole.Write(new Markup($" [bold red]{Errors} Errors[/] / [bold blue]{Warnings} Warnings[/]")); + AnsiConsole.Write(new Markup($" [bold red]{Errors} Errors[/] / [bold blue]{Warnings} Warnings[/] / [bold yellow]{Hints} Hints[/]")); AnsiConsole.WriteLine(); AnsiConsole.WriteLine(); diff --git a/src/Elastic.Documentation.Tooling/Diagnostics/Console/ErrataFileSourceRepository.cs b/src/Elastic.Documentation.Tooling/Diagnostics/Console/ErrataFileSourceRepository.cs index 13c3f614b..695c5aa3e 100644 --- a/src/Elastic.Documentation.Tooling/Diagnostics/Console/ErrataFileSourceRepository.cs +++ b/src/Elastic.Documentation.Tooling/Diagnostics/Console/ErrataFileSourceRepository.cs @@ -23,12 +23,18 @@ public bool TryGet(string id, [NotNullWhen(true)] out Source? source) return true; } - public void WriteDiagnosticsToConsole(IReadOnlyCollection errors, IReadOnlyCollection warnings) + public void WriteDiagnosticsToConsole(IReadOnlyCollection errors, IReadOnlyCollection warnings, List hints) { var report = new Report(this); - var limitedErrors = errors.Take(100).ToArray(); - var limitedWarnings = warnings.Take(100 - limitedErrors.Length); - var limited = limitedWarnings.Concat(limitedErrors).ToArray(); + var limited = errors + .Concat(warnings) + .OrderBy(d => d.Severity switch { Severity.Error => 0, Severity.Warning => 1, Severity.Hint => 2, _ => 3 }) + .Take(100) + .ToArray(); + + // show hints if we don't have plenty of errors/warnings to show + if (limited.Length < 100) + limited = limited.Concat(hints).Take(100).ToArray(); foreach (var item in limited) { @@ -36,6 +42,7 @@ public void WriteDiagnosticsToConsole(IReadOnlyCollection errors, IR { Severity.Error => Errata.Diagnostic.Error(item.Message), Severity.Warning => Errata.Diagnostic.Warning(item.Message), + Severity.Hint => Errata.Diagnostic.Info(item.Message), _ => Errata.Diagnostic.Info(item.Message) }; if (item is { Line: not null, Column: not null }) @@ -44,7 +51,13 @@ public void WriteDiagnosticsToConsole(IReadOnlyCollection errors, IR d = d.WithLabel(new Label(item.File, location, "") .WithLength(item.Length == null ? 1 : Math.Clamp(item.Length.Value, 1, item.Length.Value + 3)) .WithPriority(1) - .WithColor(item.Severity == Severity.Error ? Color.Red : Color.Blue)); + .WithColor(item.Severity switch + { + Severity.Error => Color.Red, + Severity.Warning => Color.Blue, + Severity.Hint => Color.Yellow, + _ => Color.Blue + })); } else d = d.WithNote(item.File); @@ -56,7 +69,33 @@ public void WriteDiagnosticsToConsole(IReadOnlyCollection errors, IR AnsiConsole.WriteLine(); if (totalErrorCount <= 0) + { + if (hints.Count > 0) + DisplayHintsOnly(report, hints); return; + } + DisplayErrorAndWarningSummary(report, totalErrorCount, limited); + } + + private static void DisplayHintsOnly(Report report, List hints) + { + AnsiConsole.Write(new Markup($" [bold]The following improvement hints found in the documentation[/]")); + AnsiConsole.WriteLine(); + AnsiConsole.WriteLine(); + // Render the report + report.Render(AnsiConsole.Console); + + AnsiConsole.WriteLine(); + AnsiConsole.WriteLine(); + + if (hints.Count >= 100) + AnsiConsole.Write(new Markup($" [bold]Only shown the first [yellow]{100}[/] hints out of [yellow]{hints.Count}[/][/]")); + + AnsiConsole.WriteLine(); + } + + private static void DisplayErrorAndWarningSummary(Report report, int totalErrorCount, Diagnostic[] limited) + { AnsiConsole.Write(new Markup($" [bold]The following errors and warnings were found in the documentation[/]")); AnsiConsole.WriteLine(); AnsiConsole.WriteLine(); diff --git a/src/Elastic.Documentation.Tooling/Diagnostics/Console/GithubAnnotationOutput.cs b/src/Elastic.Documentation.Tooling/Diagnostics/Console/GithubAnnotationOutput.cs index 891b0f2c7..cfa66d913 100644 --- a/src/Elastic.Documentation.Tooling/Diagnostics/Console/GithubAnnotationOutput.cs +++ b/src/Elastic.Documentation.Tooling/Diagnostics/Console/GithubAnnotationOutput.cs @@ -27,5 +27,7 @@ public void Write(Diagnostic diagnostic) githubActions.WriteError(diagnostic.Message, properties); if (diagnostic.Severity == Severity.Warning) githubActions.WriteWarning(diagnostic.Message, properties); + if (diagnostic.Severity == Severity.Hint) + githubActions.WriteNotice(diagnostic.Message, properties); } } diff --git a/src/Elastic.Documentation.Tooling/Diagnostics/Log.cs b/src/Elastic.Documentation.Tooling/Diagnostics/Log.cs index 7e6da01c4..2334e5412 100644 --- a/src/Elastic.Documentation.Tooling/Diagnostics/Log.cs +++ b/src/Elastic.Documentation.Tooling/Diagnostics/Log.cs @@ -16,15 +16,19 @@ public void Write(Diagnostic diagnostic) { if (diagnostic.Severity == Severity.Error) logger.LogError("{Message}", diagnostic.Message); - else + else if (diagnostic.Severity == Severity.Warning) logger.LogWarning("{Message}", diagnostic.Message); + else + logger.LogInformation("{Message}", diagnostic.Message); } else { if (diagnostic.Severity == Severity.Error) logger.LogError("{Message} ({File}:{Line})", diagnostic.Message, diagnostic.File, diagnostic.Line ?? 0); + else if (diagnostic.Severity == Severity.Warning) + logger.LogWarning("{Message}", diagnostic.Message); else - logger.LogWarning("{Message} ({File}:{Line})", diagnostic.Message, diagnostic.File, diagnostic.Line ?? 0); + logger.LogInformation("{Message} ({File}:{Line})", diagnostic.Message, diagnostic.File, diagnostic.Line ?? 0); } } } diff --git a/src/Elastic.Markdown/Diagnostics/DiagnosticsChannel.cs b/src/Elastic.Markdown/Diagnostics/DiagnosticsChannel.cs index 96f86bf69..7a67e643c 100644 --- a/src/Elastic.Markdown/Diagnostics/DiagnosticsChannel.cs +++ b/src/Elastic.Markdown/Diagnostics/DiagnosticsChannel.cs @@ -8,6 +8,23 @@ namespace Elastic.Markdown.Diagnostics; +public enum Severity +{ + Error, + Warning, + Hint +} + +public readonly record struct Diagnostic +{ + public Severity Severity { get; init; } + public int? Line { get; init; } + public int? Column { get; init; } + public int? Length { get; init; } + public string File { get; init; } + public string Message { get; init; } +} + public sealed class DiagnosticsChannel : IDisposable { private readonly Channel _channel; @@ -18,7 +35,11 @@ public sealed class DiagnosticsChannel : IDisposable public DiagnosticsChannel() { - var options = new UnboundedChannelOptions { SingleReader = true, SingleWriter = false }; + var options = new UnboundedChannelOptions + { + SingleReader = true, + SingleWriter = false + }; _ctxSource = new CancellationTokenSource(); _channel = Channel.CreateUnbounded(options); } @@ -43,18 +64,6 @@ public void Write(Diagnostic diagnostic) public void Dispose() => _ctxSource.Dispose(); } -public enum Severity { Error, Warning } - -public readonly record struct Diagnostic -{ - public Severity Severity { get; init; } - public int? Line { get; init; } - public int? Column { get; init; } - public int? Length { get; init; } - public string File { get; init; } - public string Message { get; init; } -} - public interface IDiagnosticsOutput { void Write(Diagnostic diagnostic); @@ -67,13 +76,17 @@ public class DiagnosticsCollector(IReadOnlyCollection output private int _errors; private int _warnings; + private int _hints; public int Warnings => _warnings; public int Errors => _errors; + public int Hints => _hints; private Task? _started; public HashSet OffendingFiles { get; } = []; + public HashSet InUseSubstitutionKeys { get; } = []; + public ConcurrentBag CrossLinks { get; } = []; public Task StartAsync(Cancel cancellationToken) @@ -119,6 +132,8 @@ private void IncrementSeverityCount(Diagnostic item) _ = Interlocked.Increment(ref _errors); else if (item.Severity == Severity.Warning) _ = Interlocked.Increment(ref _warnings); + else if (item.Severity == Severity.Hint) + _ = Interlocked.Increment(ref _hints); } protected virtual void HandleItem(Diagnostic diagnostic) { } @@ -132,35 +147,33 @@ public virtual async Task StopAsync(Cancel cancellationToken) public void EmitCrossLink(string link) => CrossLinks.Add(link); - public void EmitError(string file, string message, Exception? e = null) - { - var d = new Diagnostic + private void Emit(Severity severity, string file, string message) => + Channel.Write(new Diagnostic { - Severity = Severity.Error, + Severity = severity, File = file, Message = message - + (e != null ? Environment.NewLine + e : string.Empty) - + (e?.InnerException != null ? Environment.NewLine + e.InnerException : string.Empty), + }); - }; - Channel.Write(d); - } - - public void EmitWarning(string file, string message) + public void EmitError(string file, string message, Exception? e = null) { - var d = new Diagnostic - { - Severity = Severity.Warning, - File = file, - Message = message, - }; - Channel.Write(d); + message = message + + (e != null ? Environment.NewLine + e : string.Empty) + + (e?.InnerException != null ? Environment.NewLine + e.InnerException : string.Empty); + Emit(Severity.Error, file, message); } + public void EmitWarning(string file, string message) => Emit(Severity.Warning, file, message); + + public void EmitHint(string file, string message) => Emit(Severity.Hint, file, message); + public async ValueTask DisposeAsync() { Channel.TryComplete(); await StopAsync(CancellationToken.None); GC.SuppressFinalize(this); } + + public void CollectUsedSubstitutionKey(ReadOnlySpan key) => + _ = InUseSubstitutionKeys.Add(key.ToString()); } diff --git a/src/Elastic.Markdown/DocumentationGenerator.cs b/src/Elastic.Markdown/DocumentationGenerator.cs index 0a5dfacf2..b818b03ab 100644 --- a/src/Elastic.Markdown/DocumentationGenerator.cs +++ b/src/Elastic.Markdown/DocumentationGenerator.cs @@ -87,6 +87,8 @@ public async Task GenerateAll(Cancel ctx) await ProcessDocumentationFiles(offendingFiles, outputSeenChanges, ctx); + HintUnusedSubstitutionKeys(); + await ExtractEmbeddedStaticResources(ctx); _logger.LogInformation($"Completing diagnostics channel"); @@ -135,6 +137,24 @@ await Parallel.ForEachAsync(DocumentationSet.Files, ctx, async (file, token) => _logger.LogInformation("-> Processed {ProcessedFileCount}/{TotalFileCount} files", processedFileCount, totalFileCount); } + private void HintUnusedSubstitutionKeys() + { + var definedKeys = new HashSet(Context.Configuration.Substitutions.Keys.ToArray()); + var keysNotInUse = definedKeys.Except(Context.Collector.InUseSubstitutionKeys).ToArray(); + // If we have less than 20 unused keys emit them separately + // Otherwise emit one hint with all of them for brevity + if (keysNotInUse.Length >= 20) + { + var keys = string.Join(", ", keysNotInUse); + Context.Collector.EmitHint(Context.ConfigurationPath.FullName, $"The following keys: '{keys}' are not used in any file"); + } + else + { + foreach (var key in keysNotInUse) + Context.Collector.EmitHint(Context.ConfigurationPath.FullName, $"Substitution key '{key}' is not used in any file"); + } + } + private async Task ExtractEmbeddedStaticResources(Cancel ctx) { _logger.LogInformation($"Copying static files to output directory"); diff --git a/src/Elastic.Markdown/Helpers/Interpolation.cs b/src/Elastic.Markdown/Helpers/Interpolation.cs index c55eeba22..ef11e9bf1 100644 --- a/src/Elastic.Markdown/Helpers/Interpolation.cs +++ b/src/Elastic.Markdown/Helpers/Interpolation.cs @@ -4,6 +4,7 @@ using System.Diagnostics.CodeAnalysis; using System.Text.RegularExpressions; +using Elastic.Markdown.Diagnostics; using Elastic.Markdown.Myst; namespace Elastic.Markdown.Helpers; @@ -22,32 +23,26 @@ ParserContext context ) { var span = input.AsSpan(); - return span.ReplaceSubstitutions([context.Substitutions, context.ContextSubstitutions], out var replacement) + return span.ReplaceSubstitutions([context.Substitutions, context.ContextSubstitutions], context.Build.Collector, out var replacement) ? replacement : input; } - - public static bool ReplaceSubstitutions( - this ReadOnlySpan span, - ParserContext context, - [NotNullWhen(true)] out string? replacement - ) => - span.ReplaceSubstitutions([context.Substitutions, context.ContextSubstitutions], out replacement); - public static bool ReplaceSubstitutions( this ReadOnlySpan span, IReadOnlyDictionary? properties, + DiagnosticsCollector? collector, [NotNullWhen(true)] out string? replacement ) { replacement = null; return properties is not null && properties.Count != 0 && - span.IndexOf("}}") >= 0 && span.ReplaceSubstitutions([properties], out replacement); + span.IndexOf("}}") >= 0 && span.ReplaceSubstitutions([properties], collector, out replacement); } - public static bool ReplaceSubstitutions( + private static bool ReplaceSubstitutions( this ReadOnlySpan span, IReadOnlyDictionary[] properties, + DiagnosticsCollector? collector, [NotNullWhen(true)] out string? replacement ) { @@ -78,6 +73,8 @@ public static bool ReplaceSubstitutions( if (!lookup.TryGetValue(key, out var value)) continue; + collector?.CollectUsedSubstitutionKey(key); + replacement ??= span.ToString(); replacement = replacement.Replace(spanMatch.ToString(), value); replaced = true; diff --git a/src/Elastic.Markdown/IO/MarkdownFile.cs b/src/Elastic.Markdown/IO/MarkdownFile.cs index 548e0e297..fa3ed2437 100644 --- a/src/Elastic.Markdown/IO/MarkdownFile.cs +++ b/src/Elastic.Markdown/IO/MarkdownFile.cs @@ -210,7 +210,7 @@ protected void ReadDocumentInstructions(MarkdownDocument document) if (!string.IsNullOrEmpty(NavigationTitle)) { - if (NavigationTitle.AsSpan().ReplaceSubstitutions(subs, out var replacement)) + if (NavigationTitle.AsSpan().ReplaceSubstitutions(subs, Collector, out var replacement)) NavigationTitle = replacement; } @@ -219,7 +219,7 @@ protected void ReadDocumentInstructions(MarkdownDocument document) Title = RelativePath; Collector.EmitWarning(FilePath, "Document has no title, using file name as title."); } - else if (Title.AsSpan().ReplaceSubstitutions(subs, out var replacement)) + else if (Title.AsSpan().ReplaceSubstitutions(subs, Collector, out var replacement)) Title = replacement; var toc = GetAnchors(_set, MarkdownParser, YamlFrontMatter, document, subs, out var anchors); @@ -272,7 +272,7 @@ public static List GetAnchors( .Concat(includedTocs) .Select(toc => subs.Count == 0 ? toc - : toc.Heading.AsSpan().ReplaceSubstitutions(subs, out var r) + : toc.Heading.AsSpan().ReplaceSubstitutions(subs, set.Build.Collector, out var r) ? toc with { Heading = r } : toc) .ToList(); diff --git a/src/Elastic.Markdown/Myst/CodeBlocks/EnhancedCodeBlockParser.cs b/src/Elastic.Markdown/Myst/CodeBlocks/EnhancedCodeBlockParser.cs index 9c021021b..e42b4a676 100644 --- a/src/Elastic.Markdown/Myst/CodeBlocks/EnhancedCodeBlockParser.cs +++ b/src/Elastic.Markdown/Myst/CodeBlocks/EnhancedCodeBlockParser.cs @@ -177,14 +177,14 @@ private static void ProcessCodeBlock( var span = line.Slice.AsSpan(); if (codeBlockArgs.UseSubstitutions) { - if (span.ReplaceSubstitutions(context.YamlFrontMatter?.Properties, out var frontMatterReplacement)) + if (span.ReplaceSubstitutions(context.YamlFrontMatter?.Properties, context.Build.Collector, out var frontMatterReplacement)) { var s = new StringSlice(frontMatterReplacement); lines.Lines[index] = new StringLine(ref s); span = lines.Lines[index].Slice.AsSpan(); } - if (span.ReplaceSubstitutions(context.Substitutions, out var globalReplacement)) + if (span.ReplaceSubstitutions(context.Substitutions, context.Build.Collector, out var globalReplacement)) { var s = new StringSlice(globalReplacement); lines.Lines[index] = new StringLine(ref s); diff --git a/tests/Elastic.Markdown.Tests/Interpolation/InterpolationTests.cs b/tests/Elastic.Markdown.Tests/Interpolation/InterpolationTests.cs index 6fbd864ad..c0f0303d2 100644 --- a/tests/Elastic.Markdown.Tests/Interpolation/InterpolationTests.cs +++ b/tests/Elastic.Markdown.Tests/Interpolation/InterpolationTests.cs @@ -14,7 +14,7 @@ public void ReplacesVariables() { var span = "My text {{with-variables}} {{not-defined}}".AsSpan(); var replacements = new Dictionary { { "with-variables", "With Variables" } }; - var replaced = span.ReplaceSubstitutions(replacements, out var replacement); + var replaced = span.ReplaceSubstitutions(replacements, null, out var replacement); replaced.Should().BeTrue(); replacement.Should().Be("My text With Variables {{not-defined}}"); @@ -25,7 +25,7 @@ public void OnlyReplacesDefinedVariables() { var span = "My text {{not-defined}}".AsSpan(); var replacements = new Dictionary { { "with-variables", "With Variables" } }; - var replaced = span.ReplaceSubstitutions(replacements, out var replacement); + var replaced = span.ReplaceSubstitutions(replacements, null, out var replacement); replaced.Should().BeFalse(); // no need to allocate replacement we can continue with span From 1cd336fbc0bc1bc21a89aea42b29abf8a48fc2e6 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 17 Mar 2025 21:55:34 +0100 Subject: [PATCH 2/2] update hint errate display --- .../Diagnostics/Console/ErrataFileSourceRepository.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Elastic.Documentation.Tooling/Diagnostics/Console/ErrataFileSourceRepository.cs b/src/Elastic.Documentation.Tooling/Diagnostics/Console/ErrataFileSourceRepository.cs index 695c5aa3e..74962613c 100644 --- a/src/Elastic.Documentation.Tooling/Diagnostics/Console/ErrataFileSourceRepository.cs +++ b/src/Elastic.Documentation.Tooling/Diagnostics/Console/ErrataFileSourceRepository.cs @@ -62,6 +62,9 @@ public void WriteDiagnosticsToConsole(IReadOnlyCollection errors, IR else d = d.WithNote(item.File); + if (item.Severity == Severity.Hint) + d = d.WithColor(Color.Yellow).WithCategory("Hint"); + _ = report.AddDiagnostic(d); }