Skip to content

Commit

Permalink
Add validation for additional unnecessary locations
Browse files Browse the repository at this point in the history
  • Loading branch information
@sharwell
sharwell committed Aug 22, 2024
1 parent dda9795 commit 515a3a5
Show file tree
Hide file tree
Showing 6 changed files with 228 additions and 9 deletions.
57 changes: 51 additions & 6 deletions src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;
using DiffPlex;
Expand All @@ -34,6 +35,7 @@ public abstract class AnalyzerTest<TVerifier>
where TVerifier : IVerifier, new()
{
private static readonly ConditionalWeakTable<Diagnostic, object> NonLocalDiagnostics = new ConditionalWeakTable<Diagnostic, object>();
private static readonly Regex EncodedIndicesSyntax = new Regex(@"^\s*\[\s*((?<Index>[0-9]+)\s*(,\s*(?<Index>[0-9]+)\s*)*)?\]\s*$", RegexOptions.ExplicitCapture | RegexOptions.Compiled);

/// <summary>
/// Gets the default verifier for the test.
Expand Down Expand Up @@ -558,16 +560,43 @@ private void VerifyDiagnosticResults(IEnumerable<(Project project, Diagnostic di
else
{
VerifyDiagnosticLocation(analyzers, actual.diagnostic, expected, actual.diagnostic.Location, expected.Spans[0], verifier);
int[] unnecessaryIndices = { };
if (actual.diagnostic.Properties.TryGetValue(WellKnownDiagnosticTags.Unnecessary, out var encodedUnnecessaryLocations))
{
verifier.True(actual.diagnostic.Descriptor.CustomTags.Contains(WellKnownDiagnosticTags.Unnecessary), "Diagnostic reported extended unnecessary locations, but the descriptor is not marked as unnecessary code.");
var match = EncodedIndicesSyntax.Match(encodedUnnecessaryLocations);
verifier.True(match.Success, $"Expected encoded unnecessary locations to be a valid JSON array of non-negative integers: {encodedUnnecessaryLocations}");
unnecessaryIndices = match.Groups["Index"].Captures.OfType<Capture>().Select(capture => int.Parse(capture.Value)).ToArray();
verifier.NotEmpty(nameof(unnecessaryIndices), unnecessaryIndices);
foreach (var index in unnecessaryIndices)
{
if (index < 0 || index >= actual.diagnostic.AdditionalLocations.Count)
{
verifier.Fail($"All unnecessary indices in the diagnostic must be valid indices in AdditionalLocations [0-{actual.diagnostic.AdditionalLocations.Count}): {encodedUnnecessaryLocations}");
}
}
}

if (!expected.Options.HasFlag(DiagnosticOptions.IgnoreAdditionalLocations))
{
var additionalLocations = actual.diagnostic.AdditionalLocations.ToArray();

message = FormatVerifierMessage(analyzers, actual.diagnostic, expected, $"Expected {expected.Spans.Length - 1} additional locations but got {additionalLocations.Length} for Diagnostic:");
verifier.Equal(expected.Spans.Length - 1, additionalLocations.Length, message);

for (var j = 0; j < additionalLocations.Length; ++j)
for (var j = 0; j < additionalLocations.Length; j++)
{
VerifyDiagnosticLocation(analyzers, actual.diagnostic, expected, additionalLocations[j], expected.Spans[j + 1], verifier);
var isActualUnnecessary = unnecessaryIndices.Contains(j);
var isExpectedUnnecessary = expected.Spans[j + 1].Options.HasFlag(DiagnosticLocationOptions.UnnecessaryCode);
if (isExpectedUnnecessary)
{
verifier.True(isActualUnnecessary, $"Expected diagnostic additional location index \"{j}\" to be marked unnecessary, but was not.");
}
else
{
verifier.False(isActualUnnecessary, $"Expected diagnostic additional location index \"{j}\" to not be marked unnecessary, but was instead marked unnecessary.");
}
}
}
}
Expand Down Expand Up @@ -855,10 +884,23 @@ private static string FormatDiagnostics(ImmutableArray<DiagnosticAnalyzer> analy
}
else
{
AppendLocation(diagnostics[i].Location);
foreach (var additionalLocation in diagnostics[i].AdditionalLocations)
// The unnecessary code designator is ignored for the primary diagnostic location.
AppendLocation(diagnostics[i].Location, isUnnecessary: false);

int[] unnecessaryIndices = { };
if (diagnostics[i].Properties.TryGetValue(WellKnownDiagnosticTags.Unnecessary, out var encodedUnnecessaryLocations))
{
var match = EncodedIndicesSyntax.Match(encodedUnnecessaryLocations);
if (match.Success)
{
unnecessaryIndices = match.Groups["Index"].Captures.OfType<Capture>().Select(capture => int.Parse(capture.Value)).ToArray();
}
}

for (var j = 0; j < diagnostics[i].AdditionalLocations.Count; j++)
{
AppendLocation(additionalLocation);
var additionalLocation = diagnostics[i].AdditionalLocations[j];
AppendLocation(additionalLocation, isUnnecessary: unnecessaryIndices.Contains(j));
}
}

Expand All @@ -881,13 +923,16 @@ private static string FormatDiagnostics(ImmutableArray<DiagnosticAnalyzer> analy
return builder.ToString();

// Local functions
void AppendLocation(Location location)
void AppendLocation(Location location, bool isUnnecessary)
{
var lineSpan = location.GetLineSpan();
var pathString = location.IsInSource && lineSpan.Path == defaultFilePath ? string.Empty : $"\"{lineSpan.Path}\", ";
var linePosition = lineSpan.StartLinePosition;
var endLinePosition = lineSpan.EndLinePosition;
builder.Append($".WithSpan({pathString}{linePosition.Line + 1}, {linePosition.Character + 1}, {endLinePosition.Line + 1}, {endLinePosition.Character + 1})");
var unnecessaryArgument = isUnnecessary
? $", {nameof(DiagnosticLocationOptions)}.{nameof(DiagnosticLocationOptions.UnnecessaryCode)}"
: string.Empty;
builder.Append($".WithSpan({pathString}{linePosition.Line + 1}, {linePosition.Character + 1}, {endLinePosition.Line + 1}, {endLinePosition.Character + 1}{unnecessaryArgument})");
}
}

Expand Down
5 changes: 5 additions & 0 deletions ...CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticLocationOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,10 @@ public enum DiagnosticLocationOptions
/// </list>
/// </summary>
InterpretAsMarkupKey = 2,

/// <summary>
/// The diagnostic location is marked as unnecessary code.
/// </summary>
UnnecessaryCode = 4,
}
}
1 change: 1 addition & 0 deletions ...soft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/PublicAPI.Unshipped.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions
Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions.IgnoreLength = 1 -> Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions
Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions.InterpretAsMarkupKey = 2 -> Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions
Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions.None = 0 -> Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions
Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions.UnnecessaryCode = 4 -> Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions
Microsoft.CodeAnalysis.Testing.DiagnosticOptions
Microsoft.CodeAnalysis.Testing.DiagnosticOptions.IgnoreAdditionalLocations = 1 -> Microsoft.CodeAnalysis.Testing.DiagnosticOptions
Microsoft.CodeAnalysis.Testing.DiagnosticOptions.IgnoreSeverity = 2 -> Microsoft.CodeAnalysis.Testing.DiagnosticOptions
Expand Down
163 changes: 163 additions & 0 deletions ...ysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/DiagnosticLocationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,120 @@ Expected diagnostic to start at column "18" was actually at column "17"
new DefaultVerifier().EqualOrDiff(expected, exception.Message);
}

[Fact]
public async Task TestAdditionalUnnecessaryLocations()
{
await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>
{
TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
ExpectedDiagnostics =
{
Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>().WithLocation(0).WithLocation(1, DiagnosticLocationOptions.UnnecessaryCode),
},
}.RunAsync();
}

[Fact]
public async Task TestAdditionalUnnecessaryLocationsIgnored()
{
await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>
{
TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
ExpectedDiagnostics =
{
Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>().WithLocation(0).WithOptions(DiagnosticOptions.IgnoreAdditionalLocations),
},
}.RunAsync();
}

[Fact]
public async Task TestAdditionalUnnecessaryLocationRequiresExpectedMarkedUnnecessary()
{
var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
{
await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>
{
TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
ExpectedDiagnostics =
{
Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>().WithLocation(0).WithLocation(1),
},
}.RunAsync();
});

var expected =
"""
Expected diagnostic additional location index "0" to not be marked unnecessary, but was instead marked unnecessary.
""".ReplaceLineEndings();
new DefaultVerifier().EqualOrDiff(expected, exception.Message);
}

[Fact]
public async Task TestAdditionalUnnecessaryLocationRequiresDescriptorMarkedUnnecessary()
{
var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
{
await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryButDescriptorNotAnalyzer>
{
TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
ExpectedDiagnostics =
{
Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryButDescriptorNotAnalyzer>().WithLocation(0).WithLocation(1, DiagnosticLocationOptions.UnnecessaryCode),
},
}.RunAsync();
});

var expected =
"""
Diagnostic reported extended unnecessary locations, but the descriptor is not marked as unnecessary code.
""".ReplaceLineEndings();
new DefaultVerifier().EqualOrDiff(expected, exception.Message);
}

[Fact]
public async Task TestAdditionalUnnecessaryLocationNotJsonArray()
{
var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
{
await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryNotJsonArrayAnalyzer>
{
TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
ExpectedDiagnostics =
{
Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryNotJsonArrayAnalyzer>().WithLocation(0).WithLocation(1, DiagnosticLocationOptions.UnnecessaryCode),
},
}.RunAsync();
});

var expected =
"""
Expected encoded unnecessary locations to be a valid JSON array of non-negative integers: Text
""".ReplaceLineEndings();
new DefaultVerifier().EqualOrDiff(expected, exception.Message);
}

[Fact]
public async Task TestAdditionalUnnecessaryLocationIndexOutOfRange()
{
var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
{
await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryIndexOutOfRangeAnalyzer>
{
TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
ExpectedDiagnostics =
{
Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryIndexOutOfRangeAnalyzer>().WithLocation(0).WithLocation(1, DiagnosticLocationOptions.UnnecessaryCode),
},
}.RunAsync();
});

var expected =
"""
All unnecessary indices in the diagnostic must be valid indices in AdditionalLocations [0-1): [1]
""".ReplaceLineEndings();
new DefaultVerifier().EqualOrDiff(expected, exception.Message);
}

[DiagnosticAnalyzer(LanguageNames.CSharp)]
private class HighlightBracePositionAnalyzer : AbstractHighlightBracesAnalyzer
{
Expand All @@ -264,6 +378,55 @@ protected override Diagnostic CreateDiagnostic(SyntaxToken token)
}
}

[DiagnosticAnalyzer(LanguageNames.CSharp)]
private class HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer : AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer
{
}

[DiagnosticAnalyzer(LanguageNames.CSharp)]
private class HighlightBraceSpanWithEndMarkedUnnecessaryNotJsonArrayAnalyzer : AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer
{
protected override string UnnecessaryLocationsValue => "Text";
}

[DiagnosticAnalyzer(LanguageNames.CSharp)]
private class HighlightBraceSpanWithEndMarkedUnnecessaryIndexOutOfRangeAnalyzer : AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer
{
protected override string UnnecessaryLocationsValue => "[1]";
}

[DiagnosticAnalyzer(LanguageNames.CSharp)]
private class HighlightBraceSpanWithEndMarkedUnnecessaryButDescriptorNotAnalyzer : AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer
{
public HighlightBraceSpanWithEndMarkedUnnecessaryButDescriptorNotAnalyzer()
: base(customTags: new string[0])
{
}
}

private abstract class AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer : AbstractHighlightBracesAnalyzer
{
public AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer(string[]? customTags = null)
: base(customTags: customTags ?? new[] { WellKnownDiagnosticTags.Unnecessary })
{
}

protected virtual string UnnecessaryLocationsValue => "[0]";

protected override Diagnostic CreateDiagnostic(SyntaxToken token)
{
var endLocation = token.Parent switch
{
CSharp.Syntax.ClassDeclarationSyntax classDeclaration => classDeclaration.CloseBraceToken.GetLocation(),
_ => throw new NotSupportedException(),
};

var additionalLocations = new[] { endLocation };
var properties = ImmutableDictionary.Create<string, string?>().Add(WellKnownDiagnosticTags.Unnecessary, UnnecessaryLocationsValue);
return CodeAnalysis.Diagnostic.Create(Descriptor, token.GetLocation(), additionalLocations, properties);
}
}

[DiagnosticAnalyzer(LanguageNames.CSharp)]
private class ReportCompilationDiagnosticAnalyzer : DiagnosticAnalyzer
{
Expand Down
4 changes: 2 additions & 2 deletions ...Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/AbstractHighlightBracesAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ namespace Microsoft.CodeAnalysis.Testing.TestAnalyzers
{
public abstract class AbstractHighlightBracesAnalyzer : AbstractHighlightTokensAnalyzer
{
protected AbstractHighlightBracesAnalyzer(string id = "Brace")
: base(id, (int)CSharpSyntaxKind.OpenBraceToken, (int)VisualBasicSyntaxKind.OpenBraceToken)
protected AbstractHighlightBracesAnalyzer(string id = "Brace", string[]? customTags = null)
: base(id, customTags ?? new string[0], (int)CSharpSyntaxKind.OpenBraceToken, (int)VisualBasicSyntaxKind.OpenBraceToken)
{
}
}
Expand Down
7 changes: 6 additions & 1 deletion ...Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/AbstractHighlightTokensAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@ namespace Microsoft.CodeAnalysis.Testing.TestAnalyzers
public abstract class AbstractHighlightTokensAnalyzer : DiagnosticAnalyzer
{
protected AbstractHighlightTokensAnalyzer(string id, params int[] tokenKinds)
: this(id, customTags: new string[0], tokenKinds)
{
Descriptor = new DiagnosticDescriptor(id, "title", "message", "category", DiagnosticSeverity.Warning, isEnabledByDefault: true);
}

protected AbstractHighlightTokensAnalyzer(string id, string[] customTags, params int[] tokenKinds)
{
Descriptor = new DiagnosticDescriptor(id, "title", "message", "category", DiagnosticSeverity.Warning, isEnabledByDefault: true, customTags: customTags);
Tokens = ImmutableHashSet.CreateRange(tokenKinds);
}

Expand Down

0 comments on commit 515a3a5

Please sign in to comment.