diff --git a/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs b/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs index 87a4a424c5270..09c8aeb24302d 100644 --- a/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs +++ b/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs @@ -176,18 +176,16 @@ public bool TryGetTelemetryId(out Guid telemetryId) var fixes = GetCodeFixes(supportsFeatureService, requestedActionCategories, workspace, document, range, cancellationToken); var refactorings = GetRefactorings(supportsFeatureService, requestedActionCategories, workspace, document, selectionOpt, cancellationToken); - // If there's a selection, it's likely the user is trying to perform some operation - // directly on that operation (like 'extract method'). Prioritize refactorings over - // fixes in that case. Otherwise, it's likely that the user is just on some error - // and wants to fix it (in which case, prioritize fixes). - var result = selectionOpt?.Length > 0 - ? refactorings.Concat(fixes) - : fixes.Concat(refactorings); + // Get the initial set of action sets, with refactorings and fixes appropriately + // ordered against each other. + var result = GetInitiallyOrderedActionSets(selectionOpt, fixes, refactorings); if (result.IsEmpty) { return null; } + // Now that we have the entire set of action sets, inline, sort and filter + // them appropriately against each other. var allActionSets = InlineActionSetsIfDesirable(result); var orderedActionSets = OrderActionSets(allActionSets); var filteredSets = FilterActionSetsByTitle(orderedActionSets); @@ -196,6 +194,37 @@ public bool TryGetTelemetryId(out Guid telemetryId) } } + private ImmutableArray GetInitiallyOrderedActionSets( + TextSpan? selectionOpt, ImmutableArray fixes, ImmutableArray refactorings) + { + // First, order refactorings based on the order the providers actually gave for their actions. + // This way, a low pri refactoring always shows after a medium pri refactoring, no matter what + // we do below. + refactorings = OrderActionSets(refactorings); + + // If there's a selection, it's likely the user is trying to perform some operation + // directly on that operation (like 'extract method'). Prioritize refactorings over + // fixes in that case. Otherwise, it's likely that the user is just on some error + // and wants to fix it (in which case, prioritize fixes). + + if (selectionOpt?.Length > 0) + { + // There was a selection. Treat refactorings as more important than + // fixes. Note: we still will sort after this. So any high pri fixes + // will come to the front. Any low-pri refactorings will go to the end. + return refactorings.Concat(fixes); + } + else + { + // No selection. Treat all refactorings as low priority, and place + // after fixes. Even a low pri fixes will be above what was *originally* + // a medium pri refactoring. + refactorings = refactorings.SelectAsArray(r => new SuggestedActionSet( + r.CategoryName, r.Actions, r.Title, SuggestedActionSetPriority.Low, r.ApplicableToSpan)); + return fixes.Concat(refactorings); + } + } + private ImmutableArray OrderActionSets( ImmutableArray actionSets) { @@ -452,7 +481,7 @@ private CodeRefactoring FilterOnUIThread(CodeRefactoring refactoring, Workspace nestedAction, getFixAllSuggestedActionSet(nestedAction))); var set = new SuggestedActionSet(categoryName: null, - actions: nestedActions, priority: SuggestedActionSetPriority.Medium, + actions: nestedActions, priority: GetSuggestedActionSetPriority(fix.Action.Priority), applicableToSpan: fix.PrimaryDiagnostic.Location.SourceSpan.ToSpan()); suggestedAction = new SuggestedActionWithNestedActions( @@ -626,21 +655,8 @@ private static SuggestedActionSetPriority GetSuggestedActionSetPriority(CodeActi var filteredRefactorings = FilterOnUIThread(refactorings, workspace); - // Refactorings are given the span the user currently has selected. That - // way they can be accurately sorted against other refactorings/fixes that - // are of the same priority. i.e. refactorings are LowPriority by default. - // But we still want them to come first over a low-pri code fix that is - // further away. A good example of this is "Add null parameter check" which - // should be higher in the list when the caret is on a parameter, vs the - // code-fix for "use expression body" which is given the entire span of a - // method. - - var priority = selection.Length > 0 - ? SuggestedActionSetPriority.Medium - : SuggestedActionSetPriority.Low; - return filteredRefactorings.SelectAsArray( - r => OrganizeRefactorings(workspace, r, priority, selection.ToSpan())); + r => OrganizeRefactorings(workspace, r, selection.ToSpan())); } return ImmutableArray.Empty; @@ -655,8 +671,7 @@ private static SuggestedActionSetPriority GetSuggestedActionSetPriority(CodeActi /// and should show up after fixes but before suppression fixes in the light bulb menu. /// private SuggestedActionSet OrganizeRefactorings( - Workspace workspace, CodeRefactoring refactoring, - SuggestedActionSetPriority priority, Span applicableSpan) + Workspace workspace, CodeRefactoring refactoring, Span applicableSpan) { var refactoringSuggestedActions = ArrayBuilder.GetInstance(); @@ -670,7 +685,7 @@ private static SuggestedActionSetPriority GetSuggestedActionSetPriority(CodeActi _owner, workspace, _subjectBuffer, refactoring.Provider, na)); var set = new SuggestedActionSet(categoryName: null, - actions: nestedActions, priority: SuggestedActionSetPriority.Medium, applicableToSpan: applicableSpan); + actions: nestedActions, priority: GetSuggestedActionSetPriority(action.Priority), applicableToSpan: applicableSpan); refactoringSuggestedActions.Add(new SuggestedActionWithNestedActions( ThreadingContext, @@ -685,10 +700,14 @@ private static SuggestedActionSetPriority GetSuggestedActionSetPriority(CodeActi } } + var actions = refactoringSuggestedActions.ToImmutableAndFree(); + + // An action set gets the the same priority as the highest priority + // action within in. return new SuggestedActionSet( PredefinedSuggestedActionCategoryNames.Refactoring, - refactoringSuggestedActions.ToImmutableAndFree(), - priority: priority, + actions: actions, + priority: GetSuggestedActionSetPriority(actions.Max(a => a.Priority)), applicableToSpan: applicableSpan); } diff --git a/src/EditorFeatures/Core/Wrapping/AbstractCodeActionComputer.cs b/src/EditorFeatures/Core/Wrapping/AbstractCodeActionComputer.cs index 6092e6a690124..7e76d3935a9fd 100644 --- a/src/EditorFeatures/Core/Wrapping/AbstractCodeActionComputer.cs +++ b/src/EditorFeatures/Core/Wrapping/AbstractCodeActionComputer.cs @@ -273,8 +273,13 @@ public async Task> GetTopLevelCodeActionsAsync() // Otherwise, sort items and add to the resultant list var sorted = WrapItemsAction.SortActionsByMostRecentlyUsed(ImmutableArray.CastUp(wrappingActions)); + + // Make our code action low priority. This option will be offered *a lot*, and + // much of the time will not be something the user particularly wants to do. + // It should be offered after all other normal refactorings. result.Add(new CodeActionWithNestedActions( - wrappingActions[0].ParentTitle, sorted, group.IsInlinable)); + wrappingActions[0].ParentTitle, sorted, + group.IsInlinable, CodeActionPriority.Low)); } // Finally, sort the topmost list we're building and return that. This ensures that diff --git a/src/EditorFeatures/Core/Wrapping/WrapItemsAction.cs b/src/EditorFeatures/Core/Wrapping/WrapItemsAction.cs index 0d7ccbb5220fb..d9a58ac13e11c 100644 --- a/src/EditorFeatures/Core/Wrapping/WrapItemsAction.cs +++ b/src/EditorFeatures/Core/Wrapping/WrapItemsAction.cs @@ -29,6 +29,14 @@ internal class WrapItemsAction : DocumentChangeAction public string SortTitle { get; } + // Make our code action low priority. This option will be offered *a lot*, and + // much of the time will not be something the user particularly wants to do. + // It should be offered after all other normal refactorings. + // + // This value is only relevant if this code action is the only one in its group, + // and it ends up getting inlined as a top-level-action that is offered. + internal override CodeActionPriority Priority => CodeActionPriority.Low; + public WrapItemsAction(string title, string parentTitle, Func> createChangedDocument) : base(title, createChangedDocument) { diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 5294517d44ea3..3200ab4066cca 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -359,14 +359,18 @@ protected override Task GetChangedDocumentAsync(CancellationToken canc internal class CodeActionWithNestedActions : SimpleCodeAction { public CodeActionWithNestedActions( - string title, ImmutableArray nestedActions, bool isInlinable) + string title, ImmutableArray nestedActions, + bool isInlinable, CodeActionPriority priority = CodeActionPriority.Medium) : base(title, ComputeEquivalenceKey(nestedActions)) { Debug.Assert(nestedActions.Length > 0); NestedCodeActions = nestedActions; IsInlinable = isInlinable; + Priority = priority; } + internal override CodeActionPriority Priority { get; } + internal sealed override bool IsInlinable { get; } internal sealed override ImmutableArray NestedCodeActions { get; }