-
Notifications
You must be signed in to change notification settings - Fork 4k
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
NullRef in GenerateVariable.CSharpGenerateVariableCodeFixProvider.GetCodeActionsAsync #75204
Comments
No clue what could happen here. That entire method is:
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. |
ah OK I have it under debugger. The call to GetLanguageService() returns null. Here's the callstack:
This is the code that fails to find the service: Line 37 in 244b7e5
whereas the service in the services list has the ServiceType metadata: since it's comparing the entire string, the comparison fails, and so the service isn't found. |
The analyzers involved do come from the SDK: |
I can't share the dump here but it's available privately. |
This one also has the service, but with the wrong full name: roslyn/src/CodeStyle/Core/CodeFixes/Host/Mef/CodeStyleHostLanguageServices.MefHostExportProvider.cs Line 19 in 244b7e5
The full name here is also: |
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: So we're in the torn state. |
Repro:
|
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) |
@JoeRobich does your work with a torn sdk help here? |
Yes, VS and VSCode should no longer load the CodeStyle analyzers or fixes. See #75250 |
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. |
Wait, I'm confused. Didn't @CyrusNajmabadi move these down in #74567 The PR description is a bit lacking there so I'm not sure where it moved from and to. |
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.
The text was updated successfully, but these errors were encountered: