Unit tests for linters sometimes seem to disregard PPDs

[3geek14]

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.

[sharwell]

😕 The term PPD is used several times here, but I don't see a definition for it

[sharwell]

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:

roslyn-sdk/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/TestBehaviors.cs

Lines 34 to 38 in 0a17426

	/// <summary>
	/// Skip a verification check that diagnostics will not be reported if a <c>#pragma warning disable</c> appears
	/// at the beginning of the file.
	/// </summary>
	SkipSuppressionCheck = 0x02,

For the second issue, have you verified that the final end-of-line matches?

[3geek14]

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.

[3geek14]

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?

[sharwell]

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.

[sharwell]

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.

[sharwell]

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();

[3geek14]

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