search

dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.08k stars 4.04k forks source link

NullRef in GenerateVariable.CSharpGenerateVariableCodeFixProvider.GetCodeActionsAsync #75204

Closed KirillOsenkov closed 1 month ago

KirillOsenkov commented 1 month ago

Saw this in the wild in someone's logs, no line number or repro unfortunately, but perhaps simple eyeballing or adding nullable could fix this.

System.NullReferenceException: Object reference not set to an instance of an object.
   at Task<ImmutableArray<CodeAction>> Microsoft.CodeAnalysis.CSharp.GenerateVariable.CSharpGenerateVariableCodeFixProvider.GetCodeActionsAsync(Document document, SyntaxNode node, CancellationToken cancellationToken)
   at async Task Microsoft.CodeAnalysis.CodeFixes.GenerateMember.AbstractGenerateMemberCodeFixProvider.RegisterCodeFixesAsync(CodeFixContext context)
   at void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.Start<TStateMachine>(ref TStateMachine stateMachine)
   at Task Microsoft.CodeAnalysis.CodeFixes.GenerateMember.AbstractGenerateMemberCodeFixProvider.RegisterCodeFixesAsync(CodeFixContext context)
   at async Task<ImmutableArray<CodeFix>> Microsoft.CodeAnalysis.CodeFixes.CodeFixService.GetCodeFixesAsync(TextDocument document, TextSpan span, CodeFixProvider fixer, CodeChangeProviderMetadata fixerMetadata, CodeActionOptionsProvider fallbackOptions, ImmutableArray<Diagnostic> diagnostics, Dictionary<Diagnostic, PooledHashSet<string>> uniqueDiagosticToEquivalenceKeysMap, Dictionary<(Diagnostic diagnostic, string equivalenceKey)> diagnosticAndEquivalenceKeyToFixersMap, CancellationToken cancellationToken) in C:/Roslyn/src/LanguageServer/Protocol/Features/CodeFixes/CodeFixService.cs:line 651
   at void System.Runtime.CompilerServices.AsyncTaskMethodBuilder<TResult>.Start<TStateMachine>(ref TStateMachine stateMachine)
   at Task<ImmutableArray<CodeFix>> Microsoft.CodeAnalysis.CodeFixes.CodeFixService.GetCodeFixesAsync(TextDocument document, TextSpan span, CodeFixProvider fixer, CodeChangeProviderMetadata fixerMetadata, CodeActionOptionsProvider fallbackOptions, ImmutableArray<Diagnostic> diagnostics, Dictionary<Diagnostic, PooledHashSet<string>> uniqueDiagosticToEquivalenceKeysMap, Dictionary<(Diagnostic diagnostic, string equivalenceKey)> diagnosticAndEquivalenceKeyToFixersMap, CancellationToken cancellationToken)
   at Task<ImmutableArray<CodeFix>> Microsoft.CodeAnalysis.CodeFixes.CodeFixService+<>c__DisplayClass24_2.<StreamFixesAsync>b__4(?)+(ImmutableArray<Diagnostic> dxs) => { } in C:/Roslyn/src/LanguageServer/Protocol/Features/CodeFixes/CodeFixService.cs:line 553
   at Task<CodeFixCollection> Microsoft.CodeAnalysis.CodeFixes.CodeFixService.TryGetFixesOrConfigurationsAsync<TCodeFixProvider>(TextDocument textDocument, TextSpan fixesSpan, IEnumerable<DiagnosticData> diagnosticsWithSameSpan, bool fixAllForInSpan, TCodeFixProvider fixer, Func<Diagnostic, bool> hasFix, Func<ImmutableArray<Diagnostic>, Task<ImmutableArray<CodeFix>>> getFixes, CodeActionOptionsProvider fallbackOptions, CancellationToken cancellationToken)+(CancellationToken _) => { } in C:/Roslyn/src/LanguageServer/Protocol/Features/CodeFixes/CodeFixService.cs:line 760
   at async Task<T> Microsoft.CodeAnalysis.Extensions.IExtensionManagerExtensions.PerformFunctionAsync<T>(IExtensionManager extensionManager, object extension, Func<CancellationToken, Task<T>> function, T defaultValue, CancellationToken cancellationToken) in C:/Roslyn/src/Workspaces/Core/Portable/ExtensionManager/IExtensionManagerExtensions.cs:line 78
   at void System.Runtime.CompilerServices.AsyncTaskMethodBuilder<TResult>.Start<TStateMachine>(ref TStateMachine stateMachine)
   at Task<T> Microsoft.CodeAnalysis.Extensions.IExtensionManagerExtensions.PerformFunctionAsync<T>(IExtensionManager extensionManager, object extension, Func<CancellationToken, Task<T>> function, T defaultValue, CancellationToken cancellationToken)
   at async Task<CodeFixCollection> Microsoft.CodeAnalysis.CodeFixes.CodeFixService.TryGetFixesOrConfigurationsAsync<TCodeFixProvider>(TextDocument textDocument, TextSpan fixesSpan, IEnumerable<DiagnosticData> diagnosticsWithSameSpan, bool fixAllForInSpan, TCodeFixProvider fixer, Func<Diagnostic, bool> hasFix, Func<ImmutableArray<Diagnostic>, Task<ImmutableArray<CodeFix>>> getFixes, CodeActionOptionsProvider fallbackOptions, CancellationToken cancellationToken)
   at void System.Runtime.CompilerServices.AsyncTaskMethodBuilder<TResult>.Start<TStateMachine>(ref TStateMachine stateMachine)
   at Task<CodeFixCollection> Microsoft.CodeAnalysis.CodeFixes.CodeFixService.TryGetFixesOrConfigurationsAsync<TCodeFixProvider>(TextDocument textDocument, TextSpan fixesSpan, IEnumerable<DiagnosticData> diagnosticsWithSameSpan, bool fixAllForInSpan, TCodeFixProvider fixer, Func<Diagnostic, bool> hasFix, Func<ImmutableArray<Diagnostic>, Task<ImmutableArray<CodeFix>>> getFixes, CodeActionOptionsProvider fallbackOptions, CancellationToken cancellationToken)
   at async IAsyncEnumerable<CodeFixCollection> Microsoft.CodeAnalysis.CodeFixes.CodeFixService.StreamFixesAsync(TextDocument document, SortedDictionary<TextSpan, List<DiagnosticData>> spanToDiagnostics, bool fixAllForInSpan, ICodeActionRequestPriorityProvider priorityProvider, CodeActionOptionsProvider fallbackOptions, CancellationToken cancellationToken) in C:/Roslyn/src/LanguageServer/Protocol/Features/CodeFixes/CodeFixService.cs:line 535
   at void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.Start<TStateMachine>(ref TStateMachine stateMachine)
   at async IAsyncEnumerable<CodeFixCollection> Microsoft.CodeAnalysis.CodeFixes.CodeFixService.StreamFixesAsync(TextDocument document, SortedDictionary<TextSpan, List<DiagnosticData>> spanToDiagnostics, bool fixAllForInSpan, ICodeActionRequestPriorityProvider priorityProvider, CodeActionOptionsProvider fallbackOptions, CancellationToken cancellationToken)
   at async Task<CodeFixCollection> Microsoft.CodeAnalysis.CodeFixes.CodeFixService+<>c__DisplayClass13_0.<GetMostSevereFixAsync>g__GetFirstFixAsync|0(?)+GetFirstFixAsync(?) in C:/Roslyn/src/LanguageServer/Protocol/Features/CodeFixes/CodeFixService.cs:line 157
   at void System.Runtime.CompilerServices.AsyncMethodBuilderCore+MoveNextRunner.Run()
   at void System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
   at bool System.Threading.ThreadPoolWorkQueue.Dispatch()
CyrusNajmabadi commented 1 month ago

No clue what could happen here. That entire method is:

    protected override Task<ImmutableArray<CodeAction>> GetCodeActionsAsync(
        Document document, SyntaxNode node, CancellationToken cancellationToken)
    {
        var service = document.GetLanguageService<IGenerateVariableService>();
        return service.GenerateVariableAsync(document, node, cancellationToken);
    }

The only thing that could null ref would be if we got a null document or GetLanguageService failed. The latter can't fail as this is exported for just C# only. The former also can't happen as we would only have a Document to get into the C# case to begin with. This isn't actionable in the current state. We'll need a dump or something.

KirillOsenkov commented 1 month ago

ah OK I have it under debugger.

The call to GetLanguageService() returns null. Here's the callstack:

    Microsoft.CodeAnalysis.Workspaces   LayeredServiceUtilities.PickService Line 37
    Microsoft.CodeAnalysis.Workspaces   MefLanguageServices.<TryGetService>b__12_0 Line 67
    System.Collections.Immutable    ImmutableInterlocked.GetOrAdd
    Microsoft.CodeAnalysis.Workspaces   MefLanguageServices.TryGetService Line 67
    Microsoft.CodeAnalysis.Workspaces   MefLanguageServices.GetService Line 53
    Microsoft.CodeAnalysis.CodeStyle.Fixes  CodeStyleHostLanguageServices.GetService
    Microsoft.CodeAnalysis.CodeStyle.Fixes  ProjectExtensions.GetLanguageService
    Microsoft.CodeAnalysis.CodeStyle.Fixes  DocumentExtensions.GetLanguageService
    Microsoft.CodeAnalysis.CSharp.CodeStyle.Fixes   CSharpGenerateVariableCodeFixProvider.GetCodeActionsAsync

This is the code that fails to find the service: https://github.com/dotnet/roslyn/blob/244b7e544a168ab5c610791336638a8e51abe748/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Workspace/Mef/LayeredServiceUtilities.cs#L37

assemblyQualifiedName is: Microsoft.CodeAnalysis.GenerateMember.GenerateVariable.IGenerateVariableService, Microsoft.CodeAnalysis.CodeStyle.Fixes, Version=4.12.10.42208, Culture=neutral, PublicKeyToken=31bf3856ad364e35 it comes from ...\AnalyzerAssemblyLoader\ae96c9d5722b4b3eaeba100b0377bbde\86391754-9220-4ba8-98ef-406e465fa4f3\Microsoft.CodeAnalysis.CodeStyle.Fixes.dll

whereas the service in the services list has the ServiceType metadata: Microsoft.CodeAnalysis.GenerateMember.GenerateVariable.IGenerateVariableService, Microsoft.CodeAnalysis.Features, Version=4.11.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35

since it's comparing the entire string, the comparison fails, and so the service isn't found.

KirillOsenkov commented 1 month ago

The analyzers involved do come from the SDK: C:\Program Files\dotnet\sdk\9.0.100-rc.1.24452.12\Sdks\Microsoft.NET.Sdk\codestyle\cs\Microsoft.CodeAnalysis.CSharp.CodeStyle.Fixes.dll

KirillOsenkov commented 1 month ago

I can't share the dump here but it's available privately.

KirillOsenkov commented 1 month ago

This one also has the service, but with the wrong full name: https://github.com/dotnet/roslyn/blob/244b7e544a168ab5c610791336638a8e51abe748/src/CodeStyle/Core/CodeFixes/Host/Mef/CodeStyleHostLanguageServices.MefHostExportProvider.cs#L19

The full name here is also: Microsoft.CodeAnalysis.GenerateMember.GenerateVariable.IGenerateVariableService, Microsoft.CodeAnalysis.Features, Version=4.11.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35

KirillOsenkov commented 1 month ago

It's likely because the host IDE uses Roslyn 17.11.2, whereas the analyzers are from 17.12 already, and the code fixes have been moved down three months ago: https://github.com/dotnet/roslyn/pull/74567

So we're in the torn state.

KirillOsenkov commented 1 month ago

Repro:

  1. Use VS 17.11.4
  2. install .NET 9 SDK
  3. dotnet new console
  4. set <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild> in the .csproj
  5. in the .cs file, add a new line inside a method body foo;
  6. Invoke lightbulb to generate variable
KirillOsenkov commented 1 month ago

I'm not sure it's actionable unless there's a bug in the services lookup logic where it uses the wrong identity for a service (the one from the IDE vs. the one from the CodeFixes analyzers)

CyrusNajmabadi commented 1 month ago

@JoeRobich does your work with a torn sdk help here?

JoeRobich commented 1 month ago

Yes, VS and VSCode should no longer load the CodeStyle analyzers or fixes. See https://github.com/dotnet/roslyn/pull/75250

KirillOsenkov commented 1 month ago

So Roslyn hosts are expected to just not load the CodeStyle analyzers from the SDK, right?

JoeRobich commented 1 month ago

So Roslyn hosts are expected to just not load the CodeStyle analyzers from the SDK, right?

Right, the VS hosts carry the Features layer assemblies which contain the same analyzers and fixes. This change will be in 17.12 along with other work to help torn state issues between VS and SDK.

KirillOsenkov commented 1 month ago

Wait, I'm confused. Didn't @CyrusNajmabadi move these down in https://github.com/dotnet/roslyn/pull/74567

The PR description is a bit lacking there so I'm not sure where it moved from and to.