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..74962613c 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,11 +51,20 @@ 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); + if (item.Severity == Severity.Hint) + d = d.WithColor(Color.Yellow).WithCategory("Hint"); + _ = report.AddDiagnostic(d); } @@ -56,7 +72,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