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

Don't provide CodeActionWithDialog if option service is null #67951

Merged
merged 1 commit into from
Apr 26, 2023

Conversation

Cosifne
Copy link
Member

@Cosifne Cosifne commented Apr 25, 2023

Right now GenerateEqualsAndGetHashCodeFromMembersCodeRefactoringProvider.cs) is calling GetRequiredService() for ILegacyGlobalOptionsWorkspaceService
This would fail in the LSP scenario, because ILegacyGlobalOptionsWorkspaceService is not guaranteed to be there.

Updates:
I also modify the IPickMemberService usage here. It is also needed to be non-null.
This turn outs to be a bigger work, need to modify a lot of tests. Will do later

@Cosifne Cosifne requested a review from a team as a code owner April 25, 2023 00:57
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 25, 2023
@@ -113,9 +113,13 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
GetExistingMemberInfo(
containingType, out var hasEquals, out var hasGetHashCode);

var globalOptions = document.Project.Solution.Services.GetService<ILegacyGlobalOptionsWorkspaceService>();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core of the change

@@ -113,9 +113,13 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
GetExistingMemberInfo(
containingType, out var hasEquals, out var hasGetHashCode);

var globalOptions = document.Project.Solution.Services.GetService<ILegacyGlobalOptionsWorkspaceService>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Is it possible to leverage #66798 instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I don't know this service exists

Copy link
Member Author

@Cosifne Cosifne Apr 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see it provides reading these values

private static readonly PerLanguageOption2<bool> s_generateOperators = new(

It seems like it has a fall back of CodeCleanUpOptions, but the values used here are not part of it, like
https://sourceroslyn.io/#Microsoft.CodeAnalysis.Workspaces/Options/ILegacyGlobalOptionsWorkspaceService.cs,ebbb686a0c6361c3,references

@Cosifne Cosifne changed the title Don't provide CodeActionWithDialog if option service is null Don't provide CodeActionWithDialog if option service or pick member service is null Apr 25, 2023
@Cosifne Cosifne force-pushed the dev/shech/checkGlobalOption branch from 4d7535c to 74db2aa Compare April 25, 2023 22:59
@Cosifne Cosifne enabled auto-merge April 25, 2023 23:02
@Cosifne Cosifne changed the title Don't provide CodeActionWithDialog if option service or pick member service is null Don't provide CodeActionWithDialog if option service is null Apr 25, 2023
@Cosifne Cosifne merged commit 97fc8f1 into dotnet:main Apr 26, 2023
@ghost ghost added this to the Next milestone Apr 26, 2023
@Cosifne Cosifne modified the milestones: Next, 17.7 P2 May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants