Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AD0001 on S1186: NullReferenceException for top-level methods #7050

Closed
jnyrup opened this issue Apr 10, 2023 · 3 comments · Fixed by #7264
Closed

Fix AD0001 on S1186: NullReferenceException for top-level methods #7050

jnyrup opened this issue Apr 10, 2023 · 3 comments · Fixed by #7264
Assignees
Labels
Area: C# C# rules related issues. Type: Bug Exceptions and blocking issues during analysis.
Milestone

Comments

@jnyrup
Copy link

jnyrup commented Apr 10, 2023

Description

When applying EmptyMethodCodeFix to an empty top-level method (available from C# 9.0) the analyzer throws an exception (AD0001):

Stack Trace: 
EmptyMethodCodeFix.RegisterCodeFixesAsync(SyntaxNode root, CodeFixContext context) line 51
SonarCodeFix.RegisterCodeFixesAsync(CodeFixContext context) line 45
--- End of inner exception stack trace ---
Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
Task.Wait()
CodeFixVerifier.ActionToApply(CodeFixProvider codeFix, Document document, Diagnostic diagnostic) line 94
<>c__DisplayClass6_0.<VerifyWhileDocumentChanges>b__1(Diagnostic x) line 56
SelectManySingleSelectorIterator`2.MoveNext()
Enumerable.TryGetFirst[TSource](IEnumerable`1 source, Boolean& found)
Enumerable.FirstOrDefault[TSource](IEnumerable`1 source)
CodeFixVerifier.VerifyWhileDocumentChanges(ParseOptions parseOptions, String pathToExpected) line 54
Verifier.VerifyCodeFix() line 117
VerifierBuilderExtensions.VerifyCodeFix(VerifierBuilder builder) line 161

Repro steps

In Visual Studio put the cursor on EmptyTopLevelMethod in the snippet below, press ctrl + dot and see the analyzer except.

void EmptyTopLevelMethod() { }

Note

The RegisterCodeFixesAsync throws a NullReferenceException, after it unsuccessfully attempts to find the method declaration node:
syntaxNode.FirstAncestorOrSelf<MethodDeclarationSyntax>();
A top-level method is always considered a local function (LocalFunctionStatementSyntax) rather than a MethodDeclarationSyntax.
This can be reproduced in a unit test by adding a code fix test for an empty top-level method.

Another problem - caused by the same issue - is when a local function is inside another method:

public class MyClass
{
    public void Method()
    {
        Console.WriteLine();

        void LocalFunction()
        {
        }
    }
}

If we apply the Throw fix for LocalFunction, then it will remove the code of the outer method (as well as the local function) and replace it with a throw statement:

public class MyClass
{
    public void Method()
    {
        throw new NotSupportedException();
    }
}

Summa summarum, additional test cases are needed for local functions in general, not just for top-level methods when code fixes are involved.

Related information

  • C#/VB.NET Plugins version
    • SonarLint 6.14.1.66430
  • Visual Studio version:
    • Version 17.5.4
  • Operating System:
    • Windows 10
@zsolt-kolbay-sonarsource
Copy link
Contributor

Hi @jnyrup. Thank you for reporting this error. So far I've been unable to replicate it using SonarLint 6.14.1.66430 and Visual Studio 2022 17.6.0 Preview 2.0. Could you provide the code of the class - if it isn't too long or confidential - that contains the _ method?

@jnyrup
Copy link
Author

jnyrup commented Apr 11, 2023

It's actually the only code 😅

ConsoleApp1.zip

@zsolt-kolbay-sonarsource
Copy link
Contributor

zsolt-kolbay-sonarsource commented Apr 12, 2023

Thank you. I was able to reproduce it with the sample. The issue is related to top-level methods. I've updated the description.
Confirmed as a bug.

@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource added Type: Bug Exceptions and blocking issues during analysis. Area: C# C# rules related issues. Area: C#9 labels Apr 12, 2023
@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource changed the title NullReferenceException from EmptyMethodCodeFix Fix AD0001 on S1186: NullReferenceException for top-level methods Apr 12, 2023
@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource removed their assignment Apr 12, 2023
@csaba-sagi-sonarsource csaba-sagi-sonarsource added this to the 9.2 milestone May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Type: Bug Exceptions and blocking issues during analysis.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants