Add FixAll support for code refactorings

[mavasani]

Closes #32461

No public APIs are being added in this PR. #60703 tracks designing and exposing public APIs for this feature.

[mavasani]

This file has all existing code just moved to a new CodeFixes-specific nested static type to avoid name clashes between types named similarly for fix all for code fixes and refactorings.
Existing code:

roslyn/src/Features/LanguageServer/Protocol/Features/UnifiedSuggestions/UnifiedSuggestedActionsSource.cs

Lines 30 to 403 in ae66fbc

	/// <summary>
	/// Gets, filters, and orders code fixes.
	/// </summary>
	public static async ValueTask<ImmutableArray<UnifiedSuggestedActionSet>> GetFilterAndOrderCodeFixesAsync(
	Workspace workspace,
	ICodeFixService codeFixService,
	Document document,
	TextSpan selection,
	CodeActionRequestPriority priority,
	CodeActionOptions options,
	Func<string, IDisposable?> addOperationScope,
	CancellationToken cancellationToken)
	{
	// Intentionally switch to a threadpool thread to compute fixes. We do not want to accidentally
	// run any of this on the UI thread and potentially allow any code to take a dependency on that.
	var fixes = await Task.Run(() => codeFixService.GetFixesAsync(
	document,
	selection,
	priority,
	options,
	addOperationScope,
	cancellationToken), cancellationToken).ConfigureAwait(false);
	var filteredFixes = fixes.WhereAsArray(c => c.Fixes.Length > 0);
	var organizedFixes = await OrganizeFixesAsync(workspace, filteredFixes, cancellationToken).ConfigureAwait(false);
	return organizedFixes;
	}
	/// <summary>
	/// Arrange fixes into groups based on the issue (diagnostic being fixed) and prioritize these groups.
	/// </summary>
	private static async Task<ImmutableArray<UnifiedSuggestedActionSet>> OrganizeFixesAsync(
	Workspace workspace,
	ImmutableArray<CodeFixCollection> fixCollections,
	CancellationToken cancellationToken)
	{
	var map = ImmutableDictionary.CreateBuilder<CodeFixGroupKey, IList<IUnifiedSuggestedAction>>();
	using var _ = ArrayBuilder<CodeFixGroupKey>.GetInstance(out var order);
	// First group fixes by diagnostic and priority.
	await GroupFixesAsync(workspace, fixCollections, map, order, cancellationToken).ConfigureAwait(false);
	// Then prioritize between the groups.
	var prioritizedFixes = PrioritizeFixGroups(map.ToImmutable(), order.ToImmutable(), workspace);
	return prioritizedFixes;
	}
	/// <summary>
	/// Groups fixes by the diagnostic being addressed by each fix.
	/// </summary>
	private static async Task GroupFixesAsync(
	Workspace workspace,
	ImmutableArray<CodeFixCollection> fixCollections,
	IDictionary<CodeFixGroupKey, IList<IUnifiedSuggestedAction>> map,
	ArrayBuilder<CodeFixGroupKey> order,
	CancellationToken cancellationToken)
	{
	foreach (var fixCollection in fixCollections)
	await ProcessFixCollectionAsync(workspace, map, order, fixCollection, cancellationToken).ConfigureAwait(false);
	}
	private static async Task ProcessFixCollectionAsync(
	Workspace workspace,
	IDictionary<CodeFixGroupKey, IList<IUnifiedSuggestedAction>> map,
	ArrayBuilder<CodeFixGroupKey> order,
	CodeFixCollection fixCollection,
	CancellationToken cancellationToken)
	{
	var fixes = fixCollection.Fixes;
	var fixCount = fixes.Length;
	var nonSupressionCodeFixes = fixes.WhereAsArray(f => !IsTopLevelSuppressionAction(f.Action));
	var supressionCodeFixes = fixes.WhereAsArray(f => IsTopLevelSuppressionAction(f.Action));
	await AddCodeActionsAsync(workspace, map, order, fixCollection, GetFixAllSuggestedActionSetAsync, nonSupressionCodeFixes).ConfigureAwait(false);
	// Add suppression fixes to the end of a given SuggestedActionSet so that they
	// always show up last in a group.
	await AddCodeActionsAsync(workspace, map, order, fixCollection, GetFixAllSuggestedActionSetAsync, supressionCodeFixes).ConfigureAwait(false);
	return;
	// Local functions
	Task<UnifiedSuggestedActionSet?> GetFixAllSuggestedActionSetAsync(CodeAction codeAction)
	=> GetUnifiedFixAllSuggestedActionSetAsync(
	codeAction, fixCount, fixCollection.FixAllState,
	fixCollection.SupportedScopes, fixCollection.FirstDiagnostic,
	workspace, cancellationToken);
	}
	private static async Task AddCodeActionsAsync(
	Workspace workspace, IDictionary<CodeFixGroupKey, IList<IUnifiedSuggestedAction>> map,
	ArrayBuilder<CodeFixGroupKey> order, CodeFixCollection fixCollection,
	Func<CodeAction, Task<UnifiedSuggestedActionSet?>> getFixAllSuggestedActionSetAsync,
	ImmutableArray<CodeFix> codeFixes)
	{
	foreach (var fix in codeFixes)
	{
	var unifiedSuggestedAction = await GetUnifiedSuggestedActionAsync(fix.Action, fix).ConfigureAwait(false);
	AddFix(fix, unifiedSuggestedAction, map, order);
	}
	return;
	// Local functions
	async Task<IUnifiedSuggestedAction> GetUnifiedSuggestedActionAsync(CodeAction action, CodeFix fix)
	{
	if (action.NestedCodeActions.Length > 0)
	{
	_ = ArrayBuilder<IUnifiedSuggestedAction>.GetInstance(action.NestedCodeActions.Length, out var unifiedNestedActions);
	foreach (var nestedAction in action.NestedCodeActions)
	{
	var unifiedNestedAction = await GetUnifiedSuggestedActionAsync(nestedAction, fix).ConfigureAwait(false);
	unifiedNestedActions.Add(unifiedNestedAction);
	}
	var set = new UnifiedSuggestedActionSet(
	categoryName: null,
	actions: unifiedNestedActions.ToImmutable(),
	title: null,
	priority: GetUnifiedSuggestedActionSetPriority(action.Priority),
	applicableToSpan: fix.PrimaryDiagnostic.Location.SourceSpan);
	return new UnifiedSuggestedActionWithNestedActions(
	workspace, action, action.Priority, fixCollection.Provider, ImmutableArray.Create(set));
	}
	else
	{
	return new UnifiedCodeFixSuggestedAction(
	workspace, action, action.Priority, fix, fixCollection.Provider,
	await getFixAllSuggestedActionSetAsync(action).ConfigureAwait(false));
	}
	}
	}
	private static void AddFix(
	CodeFix fix, IUnifiedSuggestedAction suggestedAction,
	IDictionary<CodeFixGroupKey, IList<IUnifiedSuggestedAction>> map,
	ArrayBuilder<CodeFixGroupKey> order)
	{
	var groupKey = GetGroupKey(fix);
	if (!map.ContainsKey(groupKey))
	{
	order.Add(groupKey);
	map[groupKey] = ImmutableArray.CreateBuilder<IUnifiedSuggestedAction>();
	}
	map[groupKey].Add(suggestedAction);
	return;
	static CodeFixGroupKey GetGroupKey(CodeFix fix)
	{
	var diag = fix.GetPrimaryDiagnosticData();
	if (fix.Action is AbstractConfigurationActionWithNestedActions configurationAction)
	{
	return new CodeFixGroupKey(
	diag, configurationAction.Priority, configurationAction.AdditionalPriority);
	}
	return new CodeFixGroupKey(diag, fix.Action.Priority, null);
	}
	}
	// If the provided fix all context is non-null and the context's code action Id matches
	// the given code action's Id, returns the set of fix all occurrences actions associated
	// with the code action.
	private static async Task<UnifiedSuggestedActionSet?> GetUnifiedFixAllSuggestedActionSetAsync(
	CodeAction action,
	int actionCount,
	FixAllState fixAllState,
	ImmutableArray<FixAllScope> supportedScopes,
	Diagnostic firstDiagnostic,
	Workspace workspace,
	CancellationToken cancellationToken)
	{
	if (fixAllState == null)
	{
	return null;
	}
	if (actionCount > 1 && action.EquivalenceKey == null)
	{
	return null;
	}
	var document = fixAllState.Document!;
	using var fixAllSuggestedActionsDisposer = ArrayBuilder<IUnifiedSuggestedAction>.GetInstance(out var fixAllSuggestedActions);
	foreach (var scope in supportedScopes)
	{
	if (scope is FixAllScope.ContainingMember or FixAllScope.ContainingType)
	{
	// Skip showing ContainingMember and ContainingType FixAll scopes if the language
	// does not implement 'IFixAllSpanMappingService' langauge service or
	// we have no mapped FixAll spans to fix.
	var spanMappingService = document.GetLanguageService<IFixAllSpanMappingService>();
	if (spanMappingService is null)
	continue;
	var documentsAndSpans = await spanMappingService.GetFixAllSpansAsync(
	document, firstDiagnostic.Location.SourceSpan, scope, cancellationToken).ConfigureAwait(false);
	if (documentsAndSpans.IsEmpty)
	continue;
	}
	var fixAllStateForScope = fixAllState.With(scope: scope, codeActionEquivalenceKey: action.EquivalenceKey);
	var fixAllSuggestedAction = new UnifiedFixAllSuggestedAction(
	workspace, action, action.Priority, fixAllStateForScope, firstDiagnostic);
	fixAllSuggestedActions.Add(fixAllSuggestedAction);
	}
	return new UnifiedSuggestedActionSet(
	categoryName: null,
	actions: fixAllSuggestedActions.ToImmutable(),
	title: CodeFixesResources.Fix_all_occurrences_in,
	priority: UnifiedSuggestedActionSetPriority.Lowest,
	applicableToSpan: null);
	}
	/// <summary>
	/// Return prioritized set of fix groups such that fix group for suppression always show up at the bottom of the list.
	/// </summary>
	/// <remarks>
	/// Fix groups are returned in priority order determined based on <see cref="ExtensionOrderAttribute"/>.
	/// Priority for all <see cref="UnifiedSuggestedActionSet"/>s containing fixes is set to
	/// <see cref="UnifiedSuggestedActionSetPriority.Medium"/> by default.
	/// The only exception is the case where a <see cref="UnifiedSuggestedActionSet"/> only contains suppression fixes -
	/// the priority of such <see cref="UnifiedSuggestedActionSet"/>s is set to
	/// <see cref="UnifiedSuggestedActionSetPriority.Lowest"/> so that suppression fixes
	/// always show up last after all other fixes (and refactorings) for the selected line of code.
	/// </remarks>
	private static ImmutableArray<UnifiedSuggestedActionSet> PrioritizeFixGroups(
	ImmutableDictionary<CodeFixGroupKey, IList<IUnifiedSuggestedAction>> map,
	ImmutableArray<CodeFixGroupKey> order,
	Workspace workspace)
	{
	using var _1 = ArrayBuilder<UnifiedSuggestedActionSet>.GetInstance(out var nonSuppressionSets);
	using var _2 = ArrayBuilder<UnifiedSuggestedActionSet>.GetInstance(out var suppressionSets);
	using var _3 = ArrayBuilder<IUnifiedSuggestedAction>.GetInstance(out var bulkConfigurationActions);
	foreach (var groupKey in order)
	{
	var actions = map[groupKey];
	var nonSuppressionActions = actions.Where(a => !IsTopLevelSuppressionAction(a.OriginalCodeAction)).ToImmutableArray();
	AddUnifiedSuggestedActionsSet(nonSuppressionActions, groupKey, nonSuppressionSets);
	var suppressionActions = actions.Where(a => IsTopLevelSuppressionAction(a.OriginalCodeAction) &&
	!IsBulkConfigurationAction(a.OriginalCodeAction)).ToImmutableArray();
	AddUnifiedSuggestedActionsSet(suppressionActions, groupKey, suppressionSets);
	bulkConfigurationActions.AddRange(actions.Where(a => IsBulkConfigurationAction(a.OriginalCodeAction)));
	}
	var sets = nonSuppressionSets.ToImmutable();
	// Append bulk configuration fixes at the end of suppression/configuration fixes.
	if (bulkConfigurationActions.Count > 0)
	{
	var bulkConfigurationSet = new UnifiedSuggestedActionSet(
	UnifiedPredefinedSuggestedActionCategoryNames.CodeFix,
	bulkConfigurationActions.ToImmutable(),
	title: null,
	priority: UnifiedSuggestedActionSetPriority.Lowest,
	applicableToSpan: null);
	suppressionSets.Add(bulkConfigurationSet);
	}
	if (suppressionSets.Count > 0)
	{
	// Wrap the suppression/configuration actions within another top level suggested action
	// to avoid clutter in the light bulb menu.
	var suppressOrConfigureCodeAction = NoChangeAction.Create(CodeFixesResources.Suppress_or_Configure_issues, nameof(CodeFixesResources.Suppress_or_Configure_issues));
	var wrappingSuggestedAction = new UnifiedSuggestedActionWithNestedActions(
	workspace, codeAction: suppressOrConfigureCodeAction,
	codeActionPriority: suppressOrConfigureCodeAction.Priority, provider: null,
	nestedActionSets: suppressionSets.ToImmutable());
	// Combine the spans and the category of each of the nested suggested actions
	// to get the span and category for the new top level suggested action.
	var (span, category) = CombineSpansAndCategory(suppressionSets);
	var wrappingSet = new UnifiedSuggestedActionSet(
	category,
	actions: ImmutableArray.Create<IUnifiedSuggestedAction>(wrappingSuggestedAction),
	title: CodeFixesResources.Suppress_or_Configure_issues,
	priority: UnifiedSuggestedActionSetPriority.Lowest,
	applicableToSpan: span);
	sets = sets.Add(wrappingSet);
	}
	return sets;
	// Local functions
	static (TextSpan? span, string category) CombineSpansAndCategory(ArrayBuilder<UnifiedSuggestedActionSet> sets)
	{
	// We are combining the spans and categories of the given set of suggested action sets
	// to generate a result span containing the spans of individual suggested action sets and
	// a result category which is the maximum severity category amongst the set
	var minStart = -1;
	var maxEnd = -1;
	var category = UnifiedPredefinedSuggestedActionCategoryNames.CodeFix;
	foreach (var set in sets)
	{
	if (set.ApplicableToSpan.HasValue)
	{
	var currentStart = set.ApplicableToSpan.Value.Start;
	var currentEnd = set.ApplicableToSpan.Value.End;
	if (minStart == -1 || currentStart < minStart)
	{
	minStart = currentStart;
	}
	if (maxEnd == -1 || currentEnd > maxEnd)
	{
	maxEnd = currentEnd;
	}
	}
	Debug.Assert(set.CategoryName is UnifiedPredefinedSuggestedActionCategoryNames.CodeFix or
	UnifiedPredefinedSuggestedActionCategoryNames.ErrorFix);
	// If this set contains an error fix, then change the result category to ErrorFix
	if (set.CategoryName == UnifiedPredefinedSuggestedActionCategoryNames.ErrorFix)
	{
	category = UnifiedPredefinedSuggestedActionCategoryNames.ErrorFix;
	}
	}
	var combinedSpan = minStart >= 0 ? TextSpan.FromBounds(minStart, maxEnd) : (TextSpan?)null;
	return (combinedSpan, category);
	}
	}
	private static void AddUnifiedSuggestedActionsSet(
	ImmutableArray<IUnifiedSuggestedAction> actions,
	CodeFixGroupKey groupKey,
	ArrayBuilder<UnifiedSuggestedActionSet> sets)
	{
	foreach (var group in actions.GroupBy(a => a.CodeActionPriority))
	{
	var priority = GetUnifiedSuggestedActionSetPriority(group.Key);
	// diagnostic from things like build shouldn't reach here since we don't support LB for those diagnostics
	Debug.Assert(groupKey.Item1.HasTextSpan);
	var category = GetFixCategory(groupKey.Item1.Severity);
	sets.Add(new UnifiedSuggestedActionSet(
	category, group.ToImmutableArray(), title: null, priority, applicableToSpan: groupKey.Item1.GetTextSpan()));
	}
	}
	private static string GetFixCategory(DiagnosticSeverity severity)
	{
	switch (severity)
	{
	case DiagnosticSeverity.Hidden:
	case DiagnosticSeverity.Info:
	case DiagnosticSeverity.Warning:
	return UnifiedPredefinedSuggestedActionCategoryNames.CodeFix;
	case DiagnosticSeverity.Error:
	return UnifiedPredefinedSuggestedActionCategoryNames.ErrorFix;
	default:
	throw ExceptionUtilities.Unreachable;
	}
	}
	private static bool IsTopLevelSuppressionAction(CodeAction action)
	=> action is AbstractConfigurationActionWithNestedActions;
	private static bool IsBulkConfigurationAction(CodeAction action)
	=> (action as AbstractConfigurationActionWithNestedActions)?.IsBulkConfigurationAction == true;

[mavasani]

This is an existing type that was renamed from FixAllGetFixesService to FixAllCodeFixGetFixesService and some code split into a helper type FixAllGetFixesServiceHelper which is shared between FixAllCodeFixGetFixesService and FixAllCodeRefactoringGetFixesService.

[CyrusNajmabadi]

why is it a workspaceservicefactory, instead of just an IWorkspaceService?

[mavasani]

This is existing code, and I have mimic'ed the same for FixAllCodeRefactoringGetFixesService. I can clean this up.

[mavasani]

Existing code just moved to a shared helper type

[mavasani]

Most of the code in this file is existing code split out from UnifiedSuggestedActionsSource.cs. The only new code is GetUnifiedFixAllSuggestedActionSetAsync towards the end of this file.

[CyrusNajmabadi]

ok. i'm trusting you on this :)

[mavasani]

Code moved to a shared helper type.

[mavasani]

Existing code moved to a shared helper type.

[mavasani]

Moved from shared layer to Workspaces layer as (1) this service is not needed in CodeStyle layer and (2) CodeRefactorings.FixAllScope is not accessible in CodeStyle layer

[mavasani]

Existing code moved up the hierarchy from code fix specific test helper AbstractUserDiagnosticTest class to common test helper AbstractCodeActionOrUserDiagnosticTest

[mavasani]

Code moved up the hierarchy

[mavasani]

This is the core change that would be needed in a CodeRefactoringProvider type to support FixAll. It needs to sub-type SyntaxEditorBasedCodeRefactoringProvider and implemented the following two abstract members:

1. ImmutableArray<FixAllScope> SupportedFixAllScopes { get; }
2. Task FixAllAsync(Document document, ImmutableArray<TextSpan> fixAllSpans, SyntaxEditor editor, CancellationToken cancellationToken);

[CyrusNajmabadi]

so is this change jsut for example pirposes? if so, can we remove from this PR?

[CyrusNajmabadi]

(or did we have a strong belief that 'invert if' should be fix-allable?)

[mavasani]

This was one of the easier refactorings to modify to add fix all support. One of the reasons for adding FixAll support to one of the refactorings in this PR is the ability to add both unit tests and integration tests in this PR itself. I agree this is not the most useful refactoring in terms of FixAll, but I don't see any harm in having it.

[CyrusNajmabadi]

why is this returning .Project.Solution. Code in prior class returned just .Solution.

[mavasani]

I thought I removed Solution property from this context because it was just returning Project.Solution. Let me clean this up using the Generics/Type hierarchy suggestion you provided and make it consistent.

[CyrusNajmabadi]

i hope all the fixAllContext.CancellationTokens are being used properly :)

[CyrusNajmabadi]

1. these types need some sort of logic saying that they shoudl be shared with the other.
2. i feel like a common subclass with generics can help here.
3. why is this in the wpf layer?

[mavasani]

i feel like a common subclass with generics can help here.

So the core issue was that similar types like FixAllContext, FixAllProvider, DocumentBasedFixAllProvider are all public types. Adding a common abstract type is not possible, unless we make it public, which would seem odd. One thing we can do is possibly move all the code to shared internal types like CommonFixAllContext, CommonFixAllProvider, etc. and then have conversions between them OR use composition - I'll see which works out better. I think it might be better to just create a separate PR for that approach

[CyrusNajmabadi]

i'm confused by this comment :) You refer to two actions, but then say that both are the actual action. Not sure what that means.

[CyrusNajmabadi]

confused by this. how does thsi help at all. we switched to the UI thread, just to log something, then called base.InnerInvokeAsync?

[mavasani]

Just a clone of

roslyn/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActions/FixAllSuggestedAction.cs

Lines 68 to 77 in e162905

	protected override async Task InnerInvokeAsync(
	IProgressTracker progressTracker, CancellationToken cancellationToken)
	{
	await this.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
	using (Logger.LogBlock(FunctionId.CodeFixes_FixAllOccurrencesSession, FixAllLogger.CreateCorrelationLogMessage(FixAllState.CorrelationId), cancellationToken))
	{
	await base.InnerInvokeAsync(progressTracker, cancellationToken).ConfigureAwait(false);
	}
	}

Will see if the code can be shared.

[CyrusNajmabadi]

Suggested change

	document.GetLanguageService<FixAll.IFixAllSpanMappingService>() is FixAll.IFixAllSpanMappingService spanMappingService)
	document.GetLanguageService<FixAll.IFixAllSpanMappingService>() is {} spanMappingService)

[CyrusNajmabadi]

do we know we always have a progress tracker? is the method annotated properly? are all callers NRT safe?

[mavasani]

Yes, I explicitly checked all the callers. ProgressTracker passed along in the FixAll system is non-null in all the code paths.

[CyrusNajmabadi]

nit: invert.

[CyrusNajmabadi]

mutable? seems... suspect.

[mavasani]

Matched the existing implementation:

roslyn/src/Features/Core/Portable/CodeFixes/FixAllOccurrences/FixSomeCodeAction.cs

Line 23 in e162905

private bool _showPreviewChangesDialog;

This is mutable primarily for test path:

roslyn/src/Features/Core/Portable/CodeFixes/FixAllOccurrences/FixSomeCodeAction.cs

Lines 96 to 108 in e162905

	internal readonly struct TestAccessor
	{
	private readonly FixSomeCodeAction _fixSomeCodeAction;
	internal TestAccessor(FixSomeCodeAction fixSomeCodeAction)
	=> _fixSomeCodeAction = fixSomeCodeAction;
	/// <summary>
	/// Gets a reference to <see cref="_showPreviewChangesDialog"/>, which can be read or written by test code.
	/// </summary>
	public ref bool ShowPreviewChangesDialog
	=> ref _fixSomeCodeAction._showPreviewChangesDialog;
	}

[CyrusNajmabadi]

make public? it's already in something internal.

[CyrusNajmabadi]

is this repeated elsewhere? consider helper?

[CyrusNajmabadi]

what does returning null mean here?

[mavasani]

Null means no changes. This is based on the already annotated method:

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

Line 189 in 315c2e1

internal async Task<Solution?> GetChangedSolutionInternalAsync(bool postProcessChanges = true, CancellationToken cancellationToken = default)

[CyrusNajmabadi]

curius why this needs to be exposed through the interface.

[mavasani]

Matching this:

roslyn/src/Features/LanguageServer/Protocol/Features/UnifiedSuggestions/UnifiedSuggestedActions/IFixAllSuggestedAction.cs

Line 18 in e162905

CodeAction OriginalCodeAction { get; }

It seems to be passed around elsewhere.

[mavasani]

Seems to be eventually used here for telemetry:

roslyn/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActions/FixAllSuggestedAction.cs

Line 61 in e162905

telemetryId = OriginalCodeAction.GetTelemetryId(FixAllState.Scope);

[CyrusNajmabadi]

are ther emultiple impls of this? consider just having a concrete type instead.

[mavasani]

Yes, similar to https://sourceroslyn.io/#Microsoft.CodeAnalysis.LanguageServer.Protocol/Features/UnifiedSuggestions/UnifiedSuggestedActions/IFixAllSuggestedAction.cs,bb68954e1407fac4,references

[CyrusNajmabadi]

doc?

[CyrusNajmabadi]

undo?

[CyrusNajmabadi]

i'm very skeptical we need separate types for these. i am 100% ok with the CodeFixes type, and having CodeRefactorings just reference that. it's not unclean or unclear to me.

[CyrusNajmabadi]

undo?

[CyrusNajmabadi]

so, there's a pattern we can apply to prevent the massive amount of duplication happening in this PR. I get taht you couldn't share code using inheritance because of hte need to subclass a different FixAllProvider in each namespace. But here's a pattern that can work:

namespace CodeFixes
{
    class FixAllProvider
    {
    }
  
    class DocumentBasedFixAllProvider : FixAllProvider, ICommonFixAllProvider
    {
        X Something(Y arg1, Z arg2)
            => CommonFixAllProviderHelper.Something(this, arg1, arg2);
    }
}
namespace CodeRefactorings
{
    class FixAllProvider
    {
    }
  
    class DocumentBasedFixAllProvider : FixAllProvider, ICommonFixAllProvider
    {
    }
}

namespaces CodeFixesAndRefactorings
{
    interface ICommonFixAllProvider
    {
    }

    static class CommonFixAllProviderHelper
    {
        static X Something(ICommonFixAllProvider, Y arg1, Z arg2)
        {
            // do common logic here.
        }
    }
}

You can also use generics here on CommonFixAllProviderHelper (and CommonFixAllProviderHelper.Something) with constraints to get strong type safety if you need it (for example, for things like the different FixAllContexts and whatnot).

The critical thing here is that CommonFixAllProviderHelper is where all the code exists (and can be updated just once), and the two DocumentBasedFixAllProvider just defer to it. The implement ICommonFixAllProvider so that the helper type can do whatever it needs with teh original type without caring which subclass it is.

[mavasani]

Going to give it a try to share more code here. The tricky part was that existing FixAll related types are public and cannot have an internal base type, but probably just using common interfaces and shared static helper class for common code would work.

[CyrusNajmabadi]

the duplicatio nhere is currently a deal breaker for me. THere are things we can do to unify this though, even without suclassing for codesharing.

[CyrusNajmabadi]

is this repeated with some other code somewhere?

[CyrusNajmabadi]

if we share FixAllScope, and we just pass in the title, then this code can be completely shared i think.

[CyrusNajmabadi]

this class doesn't seem to have much value. do we really need it? whoever holds onto it could just hold onto these two fields?

also, consider a struct.

[mavasani]

This is held onto a map by provider in the CodeFixService/CodeRefactoringService:

roslyn/src/Features/LanguageServer/Protocol/Features/CodeFixes/CodeFixService.cs

Line 55 in e162905

private ImmutableDictionary<object, FixAllProviderInfo?> _fixAllProviderMap = ImmutableDictionary<object, FixAllProviderInfo?>.Empty;

[CyrusNajmabadi]

as above, i genuinely think we shoudl not dupe this.

[CyrusNajmabadi]

it is possible for FixAllState to subclass a CommonFixAllState for both fixes and refactorings?

[CyrusNajmabadi]

is this logic duplicated? if we have a base class, can we share?

[CyrusNajmabadi]

This is a really good start. Primary feedback is that the major places of code-duplication need a viable strategy for code sharing.

[mavasani]

Closing in favor of #60906