Unit tests for linters sometimes seem to disregard PPDs
3geek14 opened this issue · 8 comments
I've noticed two issues with unit tests I've attempted to write, both of which involve PPDs. I'm willing to believe I'm at fault, but I think there's a bug.
First:
I inherited a custom analyzer that checked if a line was longer than our column limit. This analyzer didn't respect #pragma warning directives, so it was impossible to disable the analyzer for intentionally-too-long lines. After fixing this, I wanted to add a unit test to this analyzer (and our other analyzers) to verify that they respect pragma directives. Here are the two tests I wrote:
[Test]
public async Task IgnoreAfterPragmaDisable() {
const string testCode = @"
using System;
public class Foo {
#pragma warning disable CA0000
public static void ThisIsAVeryLongFunctionNameBecauseIWantToMakeThisLineFarFarTooLong() => Console.WriteLine();
#pragma warning restore CA0000
}
";
await Verifier.VerifyAnalyzerAsync(testCode);
}
[Test]
public async Task ReportAfterPragmaRestore() {
const string testCode = @"
using System;
public class Foo {
#pragma warning disable CA0000
#pragma warning restore CA0000
{|#0: public static void ThisIsAVeryLongFunctionNameBecauseIWantToMakeThisLineFarFarTooLong() => Console.WriteLine();|}
}
";
DiagnosticResult expected =
Verifier.Diagnostic("CA0000").WithLocation(0);
await Verifier.VerifyAnalyzerAsync(testCode, expected);
}
The first test passes, but the second fails. Here's the error message for the second one:
Context: Verifying exclusions in '#pragma warning disable' code
Mismatch between number of diagnostics returned, expected "0" actual "1"
Diagnostics:
// /0/Test0.cs(7,1): warning CA0000: Lines must be no more than 100 characters in length
VerifyCS.Diagnostic().WithSpan(7, 1, 7, 114),
Expected: 0
But was: 1
at Microsoft.CodeAnalysis.Testing.Verifiers.NUnitVerifier.Equal[T](T expected, T actual, String message) in
[stack trace omitted]
Interestingly, if I remove , expected from the final line of test, leaving the test code unchanged, I get a different error message:
Mismatch between number of diagnostics returned, expected "0" actual "1"
Diagnostics:
// /0/Test0.cs(6,1): warning CA0000: Lines must be no more than 100 characters in length
VerifyCS.Diagnostic().WithSpan(6, 1, 6, 114),
Expected: 0
But was: 1
at Microsoft.CodeAnalysis.Testing.Verifiers.NUnitVerifier.Equal[T](T expected, T actual, String message) in
[stack trace omitted]
Somehow, expecting a diagnostic causes the actual diagnostic to change.
Second:
I'm trying to write an analyzer to enforce our indentation rules. There's a bug in my code involving PPDs near the ends of files. Here's a file that my analyzer flags as wrong:
public class Foo {
#region Bar
public int Baz;
#endregion
}
My analyzer reports a diagnostic at the EndOfFileToken. I wanted to debug this issue, so I tossed this small example into a unit test:
[Test]
public async Task PpdAtEof() {
const string testCode = @"
public class Foo {
#region Bar
public int Baz;
#endregion
}
";
await Verifier.VerifyAnalyzerAsync(testCode);
}
I expect this test to fail, because of the bug in my analyzer. However, it passes.
Conclusion:
I've encountered multiple issues when writing unit tests that arise when I'm trying to test how my analyzers behave around PPDs. I don't have a lot of reason to expect them to be related, except that they both involve PPDs.
😕 The term PPD is used several times here, but I don't see a definition for it
It generally should not be necessary to manually test #pragma warning disable for your diagnostic ID. By default, the test framework always tests this case. If you want to write a test for a different case, you would need to pass in SkipSuppressionCheck:
For the second issue, have you verified that the final end-of-line matches?
Sorry, PPD = preprocessor directive.
For the second issue: I tried to verify that the final end-of-lines matched, found that they didn't, and corrected that. Now the test (as expected) catches my bug, and I feel very silly.
For the first one, let me put together a MWE.
Here's an analyzer that does not respect #pragma warning disable. This is roughly what one of our analyzers used to look like.
NamespaceStyleAnalyzer.cs:
using System.Collections.Immutable;
using System.Linq;
using System.Text.RegularExpressions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Text;
namespace Foo;
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class NamespaceStyleAnalyzer : DiagnosticAnalyzer {
public static readonly DiagnosticDescriptor Descriptor =
new("ID0000", "Title", "Message", "Category", DiagnosticSeverity.Warning, true);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
ImmutableArray.Create(Descriptor);
public override void Initialize(AnalysisContext context) {
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterSyntaxTreeAction(HandleSyntaxTree);
}
private static void HandleSyntaxTree(SyntaxTreeAnalysisContext context) {
Regex blockNamespace = new("namespace .* {", RegexOptions.Compiled);
TextLineCollection lines = context.Tree.GetText().Lines;
int count = 0;
for (int lineIndex = 0; lineIndex < lines.Count; ++lineIndex) {
TextLine line = lines[lineIndex];
if (blockNamespace.IsMatch(line.ToString())) {
Location location = Location.Create(
context.Tree.FilePath,
new TextSpan(count + line.Span.Start, count + line.Span.End),
new LinePositionSpan(
new LinePosition(lineIndex, 0),
new LinePosition(lineIndex, line.Span.Length)));
context.ReportDiagnostic(Diagnostic.Create(Descriptor, location));
}
count += line.Span.Length;
}
}
}
Here's the first unit test I wrote when I wanted to fix the analyzer.
NamespaceStyleAnalyzerTest.cs:
using System.Threading.Tasks;
using NUnit.Framework;
using Verifier = Microsoft.CodeAnalysis.CSharp.Testing.NUnit.AnalyzerVerifier<Foo.NamespaceStyleAnalyzer>;
namespace Foo.Tests;
public class NamespaceStyleAnalyzerTests {
[Test]
public async Task IgnoreAfterPragmaDisable() {
const string testCode = @"
#pragma warning disable ID0000
namespace Bar {
public class Baz { }
}
#pragma warning restore ID0000
";
await Verifier.VerifyAnalyzerAsync(testCode);
}
}
This test fails:
Mismatch between number of diagnostics returned, expected "0" actual "1"
Diagnostics:
// /0/Test0.cs(3,35): warning ID0000: Message
new DiagnosticResult(NamespaceStyleAnalyzer.ID0000).WithSpan("/0/Test0.cs", 3, 1, 3, 16),
The fix for the analyzer was pretty simple: rather than six lines for a Location.Create(), I used context.Tree.GetLocation(line.Span). The new function was simply:
private static void HandleSyntaxTree(SyntaxTreeAnalysisContext context) {
Regex blockNamespace = new("namespace .* {", RegexOptions.Compiled);
TextLineCollection lines = context.Tree.GetText().Lines;
foreach (TextLine line in lines.Where(line => blockNamespace.IsMatch(line.ToString()))) {
context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Tree.GetLocation(line.Span)));
}
}
Now it passed the test, so I decided to write another test, resulting in the weird behaviour of my first example in the original post.
Because I know that we can write analyzers which pass our other unit tests but which don't respect #pragma warning disable, I want to write unit tests to ensure that we don't write analyzers in that way. It sounds like I'm supposed to use a SkipSuppressionCheck to do that? Can you point me to an example of passing in a test behaviour?
What version of the test framework are you using? I'm curious if the newest releases would have caught this without writing a new test.
Ah, I looked and the validation I was thinking of only applied to code fixes. If you do have a code fix for this diagnostic, it may have flagged it.
Can you point me to an example of passing in a test behaviour?
await new VerifyCS.Test
{
TestState =
{
Sources =
{
"""
sourcue
""",
},
},
FixedState =
{
Sources =
{
"""
sourcue
""",
},
},
TestBehaviors = TestBehaviors.SkipSuppressionCheck,
}.RunAsync();
I'm not sure which package you want the version of, so here are NuGet packages I'm currently including, with the latest version (if not what I'm already using) included in parentheses.
- Microsoft.CodeAnalysis.Analyzers 3.3.4
- Microsoft.CodeAnalysis.CSharp 4.7.0 (4.10.0)
- Microsoft.CodeAnalysis.CSharp.Analyzer.Testing.NUnit 1.1.1
- Microsoft.NET.Test.Sdk 16.10.0 (17.10.0)
- NETStandard.Library 2.0.3
- NUnit 3.14.0 (4.1.0)
- NUnit3TestAdapter 4.5.0