Skip to content

Commit

Permalink
Merge pull request #286 from sharwell/improved-locations
Browse files Browse the repository at this point in the history
Improved DiagnosticResult location support
  • Loading branch information
sharwell authored Mar 29, 2019
2 parents bc41234 + 488d1a4 commit 46c0f70
Show file tree
Hide file tree
Showing 8 changed files with 263 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ private void VerifyDiagnosticResults(IEnumerable<Diagnostic> actualResults, Immu

if (!expected.HasLocation)
{
verifier.Equal(Location.None, actual.Location, $"Expected:\nA project diagnostic with No location\nActual:\n{FormatDiagnostics(analyzers, actual)}");
verifier.Equal(Location.None, actual.Location, $"Expected:\r\nA project diagnostic with No location\r\nActual:\r\n{FormatDiagnostics(analyzers, actual)}");
}
else
{
Expand Down Expand Up @@ -280,39 +280,31 @@ private void VerifyDiagnosticResults(IEnumerable<Diagnostic> actualResults, Immu
/// <param name="expected">The <see cref="FileLinePositionSpan"/> describing the expected location of the
/// diagnostic.</param>
/// <param name="verifier">The verifier to use for test assertions.</param>
private void VerifyDiagnosticLocation(ImmutableArray<DiagnosticAnalyzer> analyzers, Diagnostic diagnostic, Location actual, FileLinePositionSpan expected, IVerifier verifier)
private void VerifyDiagnosticLocation(ImmutableArray<DiagnosticAnalyzer> analyzers, Diagnostic diagnostic, Location actual, DiagnosticLocation expected, IVerifier verifier)
{
var actualSpan = actual.GetLineSpan();

var assert = actualSpan.Path == expected.Path || (actualSpan.Path?.Contains("Test0.") == true && expected.Path.Contains("Test."));
verifier.True(assert, $"Expected diagnostic to be in file \"{expected.Path}\" was actually in file \"{actualSpan.Path}\"\r\n\r\nDiagnostic:\r\n {FormatDiagnostics(analyzers, diagnostic)}\r\n");
var assert = actualSpan.Path == expected.Span.Path || (actualSpan.Path?.Contains("Test0.") == true && expected.Span.Path.Contains("Test."));
verifier.True(assert, $"Expected diagnostic to be in file \"{expected.Span.Path}\" was actually in file \"{actualSpan.Path}\"\r\n\r\nDiagnostic:\r\n {FormatDiagnostics(analyzers, diagnostic)}\r\n");

VerifyLinePosition(analyzers, diagnostic, actualSpan.StartLinePosition, expected.StartLinePosition, "start", verifier);
if (expected.StartLinePosition < expected.EndLinePosition)
VerifyLinePosition(analyzers, diagnostic, actualSpan.StartLinePosition, expected.Span.StartLinePosition, "start", verifier);
if (!expected.Options.HasFlag(DiagnosticLocationOptions.IgnoreLength))
{
VerifyLinePosition(analyzers, diagnostic, actualSpan.EndLinePosition, expected.EndLinePosition, "end", verifier);
VerifyLinePosition(analyzers, diagnostic, actualSpan.EndLinePosition, expected.Span.EndLinePosition, "end", verifier);
}
}

private void VerifyLinePosition(ImmutableArray<DiagnosticAnalyzer> analyzers, Diagnostic diagnostic, LinePosition actualLinePosition, LinePosition expectedLinePosition, string positionText, IVerifier verifier)
{
// Only check the line position if it matters
if (expectedLinePosition.Line > 0)
{
verifier.Equal(
expectedLinePosition.Line,
actualLinePosition.Line,
$"Expected diagnostic to {positionText} on line \"{expectedLinePosition.Line + 1}\" was actually on line \"{actualLinePosition.Line + 1}\"\r\n\r\nDiagnostic:\r\n {FormatDiagnostics(analyzers, diagnostic)}\r\n");
}

// Only check the column position if it matters
if (expectedLinePosition.Character > 0)
{
verifier.Equal(
expectedLinePosition.Character,
actualLinePosition.Character,
$"Expected diagnostic to {positionText} at column \"{expectedLinePosition.Character + 1}\" was actually at column \"{actualLinePosition.Character + 1}\"\r\n\r\nDiagnostic:\r\n {FormatDiagnostics(analyzers, diagnostic)}\r\n");
}
verifier.Equal(
expectedLinePosition.Line,
actualLinePosition.Line,
$"Expected diagnostic to {positionText} on line \"{expectedLinePosition.Line + 1}\" was actually on line \"{actualLinePosition.Line + 1}\"\r\n\r\nDiagnostic:\r\n {FormatDiagnostics(analyzers, diagnostic)}\r\n");

verifier.Equal(
expectedLinePosition.Character,
actualLinePosition.Character,
$"Expected diagnostic to {positionText} at column \"{expectedLinePosition.Character + 1}\" was actually at column \"{actualLinePosition.Character + 1}\"\r\n\r\nDiagnostic:\r\n {FormatDiagnostics(analyzers, diagnostic)}\r\n");
}

/// <summary>
Expand Down Expand Up @@ -428,7 +420,7 @@ private static bool IsSuppressible(ImmutableArray<DiagnosticAnalyzer> analyzers,

private static bool IsInSourceFile(DiagnosticResult result, (string filename, SourceText content)[] sources)
{
return sources.Any(source => source.filename.Equals(result.Spans[0].Path));
return sources.Any(source => source.filename.Equals(result.Spans[0].Span.Path));
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.CodeAnalysis.Testing
{
public readonly struct DiagnosticLocation
{
public DiagnosticLocation(FileLinePositionSpan span, DiagnosticLocationOptions options)
{
Span = span;
Options = options;
}

public FileLinePositionSpan Span { get; }

public DiagnosticLocationOptions Options { get; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.CodeAnalysis.Testing
{
/// <summary>
/// Defines options for interpreting <see cref="DiagnosticLocation"/>.
/// </summary>
public enum DiagnosticLocationOptions
{
/// <summary>
/// The diagnostic location is a simple <see cref="FileLinePositionSpan"/>.
/// </summary>
None = 0,

/// <summary>
/// The diagnostic location is defined as a position instead of a span. The length of the actual diagnostic span
/// should be ignored when comparing results.
/// </summary>
IgnoreLength = 1,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ namespace Microsoft.CodeAnalysis.Testing
/// <summary>
/// Structure that stores information about a <see cref="Diagnostic"/> appearing in a source.
/// </summary>
public struct DiagnosticResult
public readonly struct DiagnosticResult
{
public static readonly DiagnosticResult[] EmptyDiagnosticResults = { };

private static readonly object[] EmptyArguments = new object[0];

private ImmutableArray<FileLinePositionSpan> _spans;
private bool _suppressMessage;
private string _message;
private readonly ImmutableArray<DiagnosticLocation> _spans;
private readonly bool _suppressMessage;
private readonly string _message;

public DiagnosticResult(string id, DiagnosticSeverity severity)
: this()
Expand All @@ -36,9 +36,27 @@ public DiagnosticResult(DiagnosticDescriptor descriptor)
MessageFormat = descriptor.MessageFormat;
}

public ImmutableArray<FileLinePositionSpan> Spans => _spans.IsDefault ? ImmutableArray<FileLinePositionSpan>.Empty : _spans;
private DiagnosticResult(
ImmutableArray<DiagnosticLocation> spans,
bool suppressMessage,
string message,
DiagnosticSeverity severity,
string id,
LocalizableString messageFormat,
object[] messageArguments)
{
_spans = spans;
_suppressMessage = suppressMessage;
_message = message;
Severity = severity;
Id = id;
MessageFormat = messageFormat;
MessageArguments = messageArguments;
}

public DiagnosticSeverity Severity { get; private set; }
public ImmutableArray<DiagnosticLocation> Spans => _spans.IsDefault ? ImmutableArray<DiagnosticLocation>.Empty : _spans;

public DiagnosticSeverity Severity { get; }

public string Id { get; }

Expand All @@ -65,9 +83,9 @@ public string Message
}
}

public LocalizableString MessageFormat { get; private set; }
public LocalizableString MessageFormat { get; }

public object[] MessageArguments { get; private set; }
public object[] MessageArguments { get; }

public bool HasLocation => !Spans.IsEmpty;

Expand All @@ -79,31 +97,62 @@ public static DiagnosticResult CompilerWarning(string identifier)

public DiagnosticResult WithSeverity(DiagnosticSeverity severity)
{
var result = this;
result.Severity = severity;
return result;
return new DiagnosticResult(
spans: _spans,
suppressMessage: _suppressMessage,
message: _message,
severity: severity,
id: Id,
messageFormat: MessageFormat,
messageArguments: MessageArguments);
}

public DiagnosticResult WithArguments(params object[] arguments)
{
var result = this;
result.MessageArguments = arguments;
return result;
return new DiagnosticResult(
spans: _spans,
suppressMessage: _suppressMessage,
message: _message,
severity: Severity,
id: Id,
messageFormat: MessageFormat,
messageArguments: arguments);
}

public DiagnosticResult WithMessage(string message)
{
var result = this;
result._message = message;
result._suppressMessage = message is null;
return result;
return new DiagnosticResult(
spans: _spans,
suppressMessage: message is null,
message: message,
severity: Severity,
id: Id,
messageFormat: MessageFormat,
messageArguments: MessageArguments);
}

public DiagnosticResult WithMessageFormat(LocalizableString messageFormat)
{
var result = this;
result.MessageFormat = messageFormat;
return result;
return new DiagnosticResult(
spans: _spans,
suppressMessage: _suppressMessage,
message: _message,
severity: Severity,
id: Id,
messageFormat: messageFormat,
messageArguments: MessageArguments);
}

public DiagnosticResult WithNoLocation()
{
return new DiagnosticResult(
spans: ImmutableArray<DiagnosticLocation>.Empty,
suppressMessage: _suppressMessage,
message: _message,
severity: Severity,
id: Id,
messageFormat: MessageFormat,
messageArguments: MessageArguments);
}

public DiagnosticResult WithLocation(int line, int column)
Expand All @@ -116,16 +165,16 @@ public DiagnosticResult WithLocation(string path, int line, int column)
=> WithLocation(path, new LinePosition(line - 1, column - 1));

public DiagnosticResult WithLocation(string path, LinePosition location)
=> AppendSpan(new FileLinePositionSpan(path, location, location));
=> AppendSpan(new FileLinePositionSpan(path, location, location), DiagnosticLocationOptions.IgnoreLength);

public DiagnosticResult WithSpan(int startLine, int startColumn, int endLine, int endColumn)
=> WithSpan(path: string.Empty, startLine, startColumn, endLine, endColumn);

public DiagnosticResult WithSpan(string path, int startLine, int startColumn, int endLine, int endColumn)
=> AppendSpan(new FileLinePositionSpan(path, new LinePosition(startLine - 1, startColumn - 1), new LinePosition(endLine - 1, endColumn - 1)));
=> AppendSpan(new FileLinePositionSpan(path, new LinePosition(startLine - 1, startColumn - 1), new LinePosition(endLine - 1, endColumn - 1)), DiagnosticLocationOptions.None);

public DiagnosticResult WithSpan(FileLinePositionSpan span)
=> AppendSpan(span);
=> AppendSpan(span, DiagnosticLocationOptions.None);

public DiagnosticResult WithDefaultPath(string path)
{
Expand All @@ -134,18 +183,23 @@ public DiagnosticResult WithDefaultPath(string path)
return this;
}

var result = this;
var spans = Spans.ToBuilder();
for (var i = 0; i < spans.Count; i++)
{
if (spans[i].Path == string.Empty)
if (spans[i].Span.Path == string.Empty)
{
spans[i] = new FileLinePositionSpan(path, spans[i].Span);
spans[i] = new DiagnosticLocation(new FileLinePositionSpan(path, spans[i].Span.Span), spans[i].Options);
}
}

result._spans = spans.MoveToImmutable();
return result;
return new DiagnosticResult(
spans: spans.MoveToImmutable(),
suppressMessage: _suppressMessage,
message: _message,
severity: Severity,
id: Id,
messageFormat: MessageFormat,
messageArguments: MessageArguments);
}

public DiagnosticResult WithLineOffset(int offset)
Expand All @@ -159,21 +213,32 @@ public DiagnosticResult WithLineOffset(int offset)
var spansBuilder = result.Spans.ToBuilder();
for (var i = 0; i < result.Spans.Length; i++)
{
var newStartLinePosition = new LinePosition(result.Spans[i].StartLinePosition.Line + offset, result.Spans[i].StartLinePosition.Character);
var newEndLinePosition = new LinePosition(result.Spans[i].EndLinePosition.Line + offset, result.Spans[i].EndLinePosition.Character);
var newStartLinePosition = new LinePosition(result.Spans[i].Span.StartLinePosition.Line + offset, result.Spans[i].Span.StartLinePosition.Character);
var newEndLinePosition = new LinePosition(result.Spans[i].Span.EndLinePosition.Line + offset, result.Spans[i].Span.EndLinePosition.Character);

spansBuilder[i] = new FileLinePositionSpan(result.Spans[i].Path, newStartLinePosition, newEndLinePosition);
spansBuilder[i] = new DiagnosticLocation(new FileLinePositionSpan(result.Spans[i].Span.Path, newStartLinePosition, newEndLinePosition), result.Spans[i].Options);
}

result._spans = spansBuilder.MoveToImmutable();
return result;
return new DiagnosticResult(
spans: spansBuilder.MoveToImmutable(),
suppressMessage: _suppressMessage,
message: _message,
severity: Severity,
id: Id,
messageFormat: MessageFormat,
messageArguments: MessageArguments);
}

private DiagnosticResult AppendSpan(FileLinePositionSpan span)
private DiagnosticResult AppendSpan(FileLinePositionSpan span, DiagnosticLocationOptions options)
{
var result = this;
result._spans = Spans.Add(span);
return result;
return new DiagnosticResult(
spans: Spans.Add(new DiagnosticLocation(span, options)),
suppressMessage: _suppressMessage,
message: _message,
severity: Severity,
id: Id,
messageFormat: MessageFormat,
messageArguments: MessageArguments);
}

public override string ToString()
Expand All @@ -182,17 +247,17 @@ public override string ToString()
if (HasLocation)
{
var location = Spans[0];
builder.Append(location.Path == string.Empty ? "?" : location.Path);
builder.Append(location.Span.Path == string.Empty ? "?" : location.Span.Path);
builder.Append("(");
builder.Append(location.StartLinePosition.Line + 1);
builder.Append(location.Span.StartLinePosition.Line + 1);
builder.Append(",");
builder.Append(location.StartLinePosition.Character + 1);
if (location.EndLinePosition != location.StartLinePosition)
builder.Append(location.Span.StartLinePosition.Character + 1);
if (!location.Options.HasFlag(DiagnosticLocationOptions.IgnoreLength))
{
builder.Append(",");
builder.Append(location.EndLinePosition.Line + 1);
builder.Append(location.Span.EndLinePosition.Line + 1);
builder.Append(",");
builder.Append(location.EndLinePosition.Character + 1);
builder.Append(location.Span.EndLinePosition.Character + 1);
}

builder.Append("): ");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ internal static class IEnumerableExtensions
public static DiagnosticResult[] ToOrderedArray(this IEnumerable<DiagnosticResult> diagnosticResults)
{
return diagnosticResults
.OrderBy(diagnosticResult => diagnosticResult.Spans.FirstOrDefault().Path, StringComparer.Ordinal)
.ThenBy(diagnosticResult => diagnosticResult.Spans.FirstOrDefault().Span.Start.Line)
.ThenBy(diagnosticResult => diagnosticResult.Spans.FirstOrDefault().Span.Start.Character)
.ThenBy(diagnosticResult => diagnosticResult.Spans.FirstOrDefault().Span.End.Line)
.ThenBy(diagnosticResult => diagnosticResult.Spans.FirstOrDefault().Span.End.Character)
.OrderBy(diagnosticResult => diagnosticResult.Spans.FirstOrDefault().Span.Path, StringComparer.Ordinal)
.ThenBy(diagnosticResult => diagnosticResult.Spans.FirstOrDefault().Span.Span.Start.Line)
.ThenBy(diagnosticResult => diagnosticResult.Spans.FirstOrDefault().Span.Span.Start.Character)
.ThenBy(diagnosticResult => diagnosticResult.Spans.FirstOrDefault().Span.Span.End.Line)
.ThenBy(diagnosticResult => diagnosticResult.Spans.FirstOrDefault().Span.Span.End.Character)
.ThenBy(diagnosticResult => diagnosticResult.Id, StringComparer.Ordinal)
.ToArray();
}
Expand Down
Loading

0 comments on commit 46c0f70

Please sign in to comment.