Ensure equivalence key is not null by Youssef1313 · Pull Request #52111 · dotnet/roslyn

Youssef1313 · 2021-03-24T14:23:15Z

Fixes #52074

Youssef1313 · 2021-03-30T16:27:34Z

src/Features/Core/Portable/AddImport/CodeActions/InstallPackageAndAddImportCodeAction.cs

                // Make a SolutionChangeAction.  This way we can let it generate the diff
                // preview appropriately.
-                var solutionChangeAction = new SolutionChangeAction("", c => GetUpdatedSolutionAsync(c));
+                var solutionChangeAction = new SolutionChangeAction("", c => GetUpdatedSolutionAsync(c), "");


Not sure why the title is empty string here. Is this code action (for some reason) non user-facing?

yes. it is only used to get the operations out.

Youssef1313 · 2021-03-30T16:30:52Z

src/Workspaces/Core/Portable/CodeActions/CodeAction.cs

                string title,
                Func<CancellationToken, Task<Solution>> createChangedSolution,
-                string? equivalenceKey = null,
+                string? equivalenceKey,


I didn't change this to non-nullable because it's called by a public API:

roslyn/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs

Line 335 in 1a4922e

public static CodeAction Create(string title, Func<CancellationToken, Task<Solution>> createChangedSolution, string? equivalenceKey = null)

I'll have to either add a suppression or change the public API nullability. I thought it's good enough for this to not be optional.

Youssef1313 · 2021-04-01T11:35:44Z

...alyzers/CSharp/CodeFixes/MisplacedUsingDirectives/MisplacedUsingDirectivesCodeFixProvider.cs

        {
            public MoveMisplacedUsingsCodeAction(Func<CancellationToken, Task<Document>> createChangedDocument)
-                : base(CSharpAnalyzersResources.Move_misplaced_using_directives, createChangedDocument)
+                : base(CSharpAnalyzersResources.Move_misplaced_using_directives, createChangedDocument, nameof(CSharpAnalyzersResources.Move_misplaced_using_directives))


This change causing cleanup tests to fail. I still didn't figure out why, would be helpful if someone knows.

In:

roslyn/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/BatchFixAllProvider.cs

Line 150 in 6da1274

var context = new CodeFixContext(document, diagnostic, GetRegisterCodeFixAction(fixAllContext.CodeActionEquivalenceKey, codeActions), cancellationToken);

fixAllContext.CodeActionEquivalenceKey is null, causing the following condition to be false:

roslyn/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/BatchFixAllProvider.cs

Lines 233 to 238 in 6da1274

if (currentAction is { EquivalenceKey: var equivalenceKey }

&& codeActionEquivalenceKey == equivalenceKey)

{

lock (codeActions)

codeActions.Add(currentAction);

}

Hence, no code actions are executed:

roslyn/src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/BatchFixAllProvider.cs

Line 159 in 6da1274

foreach (var codeAction in codeActions)

I fixed that in e6b3238 and 4e54b71.

I used the first fix as similar thing is done in:

roslyn/src/Features/Core/Portable/CodeFixes/CodeFixService.cs

Lines 245 to 247 in f3462ac

// TODO: Just get the first fix for now until we have a way to config user's preferred fix

// https://github.com/dotnet/roslyn/issues/27066

return result.ToImmutable().FirstOrDefault();

Youssef1313 · 2021-04-08T12:05:54Z

@sharwell @CyrusNajmabadi Can you review please? Thanks.

Youssef1313 · 2021-04-22T08:56:01Z

Also pinging @mavasani specifically for the last two commits.

Youssef1313 · 2021-05-29T12:30:26Z

@sharwell @mavasani @CyrusNajmabadi Can someone review? Thanks!

Youssef1313 · 2021-07-11T07:08:02Z

Ping @sharwell @CyrusNajmabadi @mavasani

CyrusNajmabadi · 2021-07-11T07:13:33Z

...lderAndNamespace/AbstractChangeNamespaceToMatchFolderCodeFixProvider.CustomFixAllProvider.cs

                if (diagnostics.IsDefaultOrEmpty)
                    return null;

+                var title = FixAllContextHelper.GetDefaultFixAllTitle(fixAllContext);


why?

Possibly I was using it as equivalence key so I extracted to a variable, and changed that in a later commit to only use it as title.

CyrusNajmabadi · 2021-07-11T07:16:35Z

Thank you :)

Youssef1313 requested a review from a team as a code owner March 24, 2021 14:23

ghost added the Area-IDE label Mar 24, 2021

Youssef1313 force-pushed the equiv-key branch from 7261581 to e972347 Compare March 24, 2021 14:42

jinujoseph added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Mar 25, 2021

jinujoseph assigned sharwell Mar 25, 2021

Youssef1313 marked this pull request as draft March 30, 2021 14:54

Youssef1313 added 2 commits March 30, 2021 14:59

Update CustomCodeActions to ensure non-null equivalence key

0044de5

Pass non-null equivalence key

0e71601

Youssef1313 force-pushed the equiv-key branch from fea85cb to 0e71601 Compare March 30, 2021 15:29

Youssef1313 marked this pull request as ready for review March 30, 2021 15:29

Youssef1313 added 2 commits March 30, 2021 15:35

Update CodeAction.cs to require equivalence key

d483f32

Pass non-null equivalence key

1a4922e

Youssef1313 commented Mar 30, 2021

View reviewed changes

Fix formatting

835f9d3

Youssef1313 commented Apr 1, 2021

View reviewed changes

Youssef1313 added 2 commits April 1, 2021 13:01

Fix failing code cleanup test

e6b3238

First action instead of last and use indexer

4e54b71

Youssef1313 added 2 commits May 11, 2021 12:42

Merge branch 'main' into equiv-key

108d7c5

Update AbstractIntroduceParameterService.cs

f3cba47

CyrusNajmabadi reviewed Jul 11, 2021

View reviewed changes

CyrusNajmabadi approved these changes Jul 11, 2021

View reviewed changes

CyrusNajmabadi merged commit 7b4b65c into dotnet:main Jul 11, 2021

ghost added this to the Next milestone Jul 11, 2021

Youssef1313 deleted the equiv-key branch July 11, 2021 07:16

allisonchou modified the milestones: Next, 17.0.P3 Jul 27, 2021

	if (currentAction is { EquivalenceKey: var equivalenceKey }
	&& codeActionEquivalenceKey == equivalenceKey)
	{
	lock (codeActions)
	codeActions.Add(currentAction);
	}

	// TODO: Just get the first fix for now until we have a way to config user's preferred fix
	// https://github.com/dotnet/roslyn/issues/27066
	return result.ToImmutable().FirstOrDefault();

Conversation

Youssef1313 commented Mar 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Youssef1313 Mar 30, 2021

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Jul 11, 2021

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Mar 30, 2021

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Apr 1, 2021

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Apr 1, 2021

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Apr 1, 2021

Choose a reason for hiding this comment

Uh oh!

Youssef1313 commented Apr 8, 2021

Uh oh!

Youssef1313 commented Apr 22, 2021

Uh oh!

Youssef1313 commented May 29, 2021

Uh oh!

Youssef1313 commented Jul 11, 2021

Uh oh!

CyrusNajmabadi Jul 11, 2021

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Jul 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi commented Jul 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Youssef1313 commented Mar 24, 2021 •

edited

Loading

Youssef1313 Jul 11, 2021 •

edited

Loading