Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
8 changes: 6 additions & 2 deletions src/Elastic.Documentation.Tooling/Diagnostics/Log.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
75 changes: 44 additions & 31 deletions src/Elastic.Markdown/Diagnostics/DiagnosticsChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand All @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -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) { }
Expand All @@ -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());
}
20 changes: 20 additions & 0 deletions src/Elastic.Markdown/DocumentationGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ public async Task GenerateAll(Cancel ctx)

await ProcessDocumentationFiles(offendingFiles, outputSeenChanges, ctx);

HintUnusedSubstitutionKeys();

await ExtractEmbeddedStaticResources(ctx);

_logger.LogInformation($"Completing diagnostics channel");
Expand Down Expand Up @@ -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");
Expand Down
19 changes: 8 additions & 11 deletions src/Elastic.Markdown/Helpers/Interpolation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System.Diagnostics.CodeAnalysis;
using System.Text.RegularExpressions;
using Elastic.Markdown.Diagnostics;
using Elastic.Markdown.Myst;

namespace Elastic.Markdown.Helpers;
Expand All @@ -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
)
{
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/Elastic.Markdown/IO/MarkdownFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
Loading
Loading