sonar-dotnet icon indicating copy to clipboard operation
sonar-dotnet copied to clipboard

DiagnosticVerifier fails to validate issue location on first line

Open zsolt-kolbay-sonarsource opened this issue 1 year ago • 2 comments

When a test file has an expected issue in the first line of the code with an exact location then the DiagnosticVerifier will throw an exception, showing the wrong column number.

Repro

e.g. delete the first empty line - which was added as a workaround - from ClassShouldNotBeEmpty.CSharp10.cs and run the UTs:

record class EmptyRecordClass1();        // Noncompliant {{Remove this empty record, write its code or make it an "interface".}}
//           ^^^^^^^^^^^^^^^^^
record class EmptyRecordClass2() { };    // Noncompliant

record struct EmptyRecordStruct1();      // Compliant - this rule only deals with classes
record struct EmptyRecordStruct2() { };

record class NotEmptyRecordClass1(int RecordMember);
record class NotEmptyRecordClass2()
{
    int RecordMember => 42;
};

The test will fail:

Message: 
Test method SonarAnalyzer.UnitTest.Rules.ClassShouldNotBeEmptyTest.ClassShouldNotBeEmpty_CSharp10 threw exception: 
SonarAnalyzer.UnitTest.TestFramework.UnexpectedDiagnosticException: CSharp10: Expected primary issue on line 1 to start on column 13 but got column 61.

  Stack Trace: 
Test Case line 1
DiagnosticVerifier.VerifyIssue(String languageVersion, ICollection`1 expectedIssues, Func`2 issueFilter, Location location, String message, String extraInfo, Boolean isPrimary, String primaryIssueId) line 285
DiagnosticVerifier.VerifyPrimaryIssue(String languageVersion, IList`1 expectedIssues, Func`2 issueFilter, Location location, String message, String extraInfo) line 239
DiagnosticVerifier.CompareActualToExpected(String languageVersion, Diagnostic[] diagnostics, FileIssueLocations[] expectedIssuesPerFile, Boolean compareIdToMessage) line 163
DiagnosticVerifier.Verify(Compilation compilation, DiagnosticAnalyzer[] diagnosticAnalyzers, CompilationErrorBehavior checkMode, IEnumerable`1 sources, String sonarProjectConfigPath, String[] onlyDiagnostics) line 77
DiagnosticVerifier.Verify(Compilation compilation, DiagnosticAnalyzer[] diagnosticAnalyzers, CompilationErrorBehavior checkMode, String sonarProjectConfigPath, String[] onlyDiagnostics) line 56
Verifier.Verify() line 94
VerifierBuilderExtensions.Verify(VerifierBuilder builder) line 155
ClassShouldNotBeEmptyTest.ClassShouldNotBeEmpty_CSharp10() line 62
RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

Workaround

Add an empty line before the first line.

Note

Most likely the verifier generates some boilerplate code (maybe a namespace declaration), which causes this difference.

This is by design. While we can have issues on 1st line, we cannot assert the location there.

This is due to the way how concurrent analysis is validated. Because a namespace is included into the 1st line: https://github.com/SonarSource/sonar-dotnet/blob/59ce4768fac3956df149d3ca2bbf5359678f9ef0/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/Verifier.cs#L180-L191

As an action for this, we should update the docs here, to be explicit about this limitation: https://github.com/SonarSource/sonar-dotnet/blob/master/docs/verifier-syntax.md

And maybe even add specific validation for this and throw an exception in https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/IssueLocationCollector.cs to make our live easier.

see also #6211