Add code action equivalence key validation in test framework

[mavasani]

Fixes #44553

[mavasani]

@CyrusNajmabadi I will create a separate PR to loosen the de-dupe logic added in #41768 for back compat to allow same provider to register different code actions with different title and same equivalence key. The test framework check added in this PR should help catch our fixers/refactorings from doing so.

[sharwell]

@mavasani For this pull request, I am only expecting changes to equivalence keys for code actions that must change or else they will not work. Changes beyond that can produce subtle changes in the observed behavior which will be difficult to understand during the review.

[mavasani]

@sharwell @CyrusNajmabadi - I have rebased the PR to add minimal test framework validation:

1. Only validate equivalence key uniqueness for code actions registered by code fixers that support FixAll - this is the only real use case that gets affected by equivalence keys today (modulo the fix to relax the CodeFixService de-dupe logic, which I will create a separate PR)
2. Revert validation for refactorings to require unique, non-null equivalence keys - this is not required until refactorings start supporting FixAll.
3. For any test failures after my rebase, if any, I will do targeted fixes for code fixers that are affected by this.

I will mark the PR for re-review once it is ready.

[sharwell]

Only validate equivalence key uniqueness for code actions registered by code fixers that support FixAll

It should also only apply to cases that register more than one action.

Also, when more than one action is registered, is it a problem for the code fix driver if exactly one of those is null?

[mavasani]

It should also only apply to cases that register more than one action.

Yes, the current validation only asserts when the fixer has more then one registered code action, such that the code actions use different title, but same equivalence key.

Also, when more than one action is registered, is it a problem for the code fix driver if exactly one of those is null?

No, the code fix service and fix all work fine for such a case. The validation method doesn't assert for this scenario.

[mavasani]

This PR is now ready for review.

@CyrusNajmabadi The only remaining 3 test failures are legitimate failures in Implement interface code fixer, when implementing through a member. I believe the following method needs to be fixed, but I am not sure what is the correct fix. Can you please checkout this PR branch and push a correct fix? Thanks!

roslyn/src/Features/Core/Portable/ImplementInterface/AbstractImplementInterfaceService.CodeAction.cs

Lines 142 to 162 in 03c3475

	private static string GetCodeActionEquivalenceKey(
	string interfaceTypeAssemblyName,
	string interfaceTypeFullyQualifiedName,
	bool explicitly,
	bool abstractly,
	bool onlyRemaining,
	ISymbol throughMember,
	string codeActionTypeName)
	{
	if (throughMember != null)
	{
	return null;
	}
	return explicitly.ToString() + ";" +
	abstractly.ToString() + ";" +
	onlyRemaining.ToString() + ":" +
	interfaceTypeAssemblyName + ";" +
	interfaceTypeFullyQualifiedName + ";" +
	codeActionTypeName;
	}

[sharwell]

I'm much happier with this narrower scoping, particularly in how it clearly reveals the feature(s) that were impacted by this bug.

[CyrusNajmabadi]

@CyrusNajmabadi The only remaining 3 test failures are legitimate failures in Implement interface code fixer, when implementing through a member.

I have no idea why implement interface would need a fix-all :-/

but i'll take a look to see if i can figure out waht's going on here.

[mavasani]

I have no idea why implement interface would need a fix-all :-/

It would then be easy to just ensure we pass in false here and the tests should pass:

roslyn/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/CodeFixes/SyntaxEditorBasedCodeFixProvider.cs

Line 17 in 9625a14

protected SyntaxEditorBasedCodeFixProvider(bool supportsFixAll = true)

[sharwell]

It would then be easy to just ensure we pass in false here and the tests should pass:

👍

[CyrusNajmabadi]

I'm going to bring the implement interface issue to design meeting today.

[mavasani]

@CyrusNajmabadi there still seems to be a failing test in implement interface. Can you please take a look?

[CyrusNajmabadi]

looking!

[mavasani]

Thanks @CyrusNajmabadi!