Add support for diagnostic hints and track unused substitution keys (#762)

Mpdreamz · web-flow · commit d05612cd7639 · 2025-03-18T06:54:42.000+01:00
* 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.

* update hint errate display
diff --git 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<Diagnostic> _errors = [];
 	private readonly List<Diagnostic> _warnings = [];
+	private readonly List<Diagnostic> _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
@@ -23,19 +23,26 @@ public bool TryGet(string id, [NotNullWhen(true)] out Source? source)
 		return true;
 	}
 
-	public void WriteDiagnosticsToConsole(IReadOnlyCollection<Diagnostic> errors, IReadOnlyCollection<Diagnostic> warnings)
+	public void WriteDiagnosticsToConsole(IReadOnlyCollection<Diagnostic> errors, IReadOnlyCollection<Diagnostic> warnings, List<Diagnostic> 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)
 		{
 			var d = item.Severity switch
 			{
 				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,19 +51,54 @@ public void WriteDiagnosticsToConsole(IReadOnlyCollection<Diagnostic> 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);
 		}
 
 		var totalErrorCount = errors.Count + warnings.Count;
 
 		AnsiConsole.WriteLine();
 		if (totalErrorCount <= 0)
+		{
+			if (hints.Count > 0)
+				DisplayHintsOnly(report, hints);
 			return;
+		}
+		DisplayErrorAndWarningSummary(report, totalErrorCount, limited);
+	}
+
+	private static void DisplayHintsOnly(Report report, List<Diagnostic> 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
@@ -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
@@ -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
@@ -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<Diagnostic> _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<Diagnostic>(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<IDiagnosticsOutput> 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<string> OffendingFiles { get; } = [];
 
+	public HashSet<string> InUseSubstitutionKeys { get; } = [];
+
 	public ConcurrentBag<string> 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<char> key) =>
+		_ = InUseSubstitutionKeys.Add(key.ToString());
 }
diff --git 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<string>(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
@@ -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<char> span,
-		ParserContext context,
-		[NotNullWhen(true)] out string? replacement
-	) =>
-		span.ReplaceSubstitutions([context.Substitutions, context.ContextSubstitutions], out replacement);
-
 	public static bool ReplaceSubstitutions(
 		this ReadOnlySpan<char> span,
 		IReadOnlyDictionary<string, string>? 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<char> span,
 		IReadOnlyDictionary<string, string>[] 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
@@ -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<PageTocItem> 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
diff --git a/tests/Elastic.Markdown.Tests/Interpolation/InterpolationTests.cs b/tests/Elastic.Markdown.Tests/Interpolation/InterpolationTests.cs

Original file line number	Diff line number	Diff line change
`@@ -27,5 +27,7 @@ public void Write(Diagnostic diagnostic)`
`27`	`27`	`githubActions.WriteError(diagnostic.Message, properties);`
`28`	`28`	`if (diagnostic.Severity == Severity.Warning)`
`29`	`29`	`githubActions.WriteWarning(diagnostic.Message, properties);`
	`30`	`+ if (diagnostic.Severity == Severity.Hint)`
	`31`	`+ githubActions.WriteNotice(diagnostic.Message, properties);`
`30`	`32`	`}`
`31`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,15 +16,19 @@ public void Write(Diagnostic diagnostic)`
`16`	`16`	`{`
`17`	`17`	`if (diagnostic.Severity == Severity.Error)`
`18`	`18`	`logger.LogError("{Message}", diagnostic.Message);`
`19`		`- else`
	`19`	`+ else if (diagnostic.Severity == Severity.Warning)`
`20`	`20`	`logger.LogWarning("{Message}", diagnostic.Message);`
	`21`	`+ else`
	`22`	`+ logger.LogInformation("{Message}", diagnostic.Message);`
`21`	`23`	`}`
`22`	`24`	`else`
`23`	`25`	`{`
`24`	`26`	`if (diagnostic.Severity == Severity.Error)`
`25`	`27`	`logger.LogError("{Message} ({File}:{Line})", diagnostic.Message, diagnostic.File, diagnostic.Line ?? 0);`
	`28`	`+ else if (diagnostic.Severity == Severity.Warning)`
	`29`	`+ logger.LogWarning("{Message}", diagnostic.Message);`
`26`	`30`	`else`
`27`		`- logger.LogWarning("{Message} ({File}:{Line})", diagnostic.Message, diagnostic.File, diagnostic.Line ?? 0);`
	`31`	`+ logger.LogInformation("{Message} ({File}:{Line})", diagnostic.Message, diagnostic.File, diagnostic.Line ?? 0);`
`28`	`32`	`}`
`29`	`33`	`}`
`30`	`34`	`}`