Merge pull request #75643 from CyrusNajmabadi/shortNames

Shorten names shown in 'pull member up'
dotnet · Oct 28, 2024 · 2a27803 · 2a27803
2 parents dae9321 + f184408
commit 2a27803
Show file tree

Hide file tree

Showing 18 changed files with 74 additions and 177 deletions.
diff --git a/src/EditorFeatures/CSharpTest/CodeActions/PullMemberUp/CSharpPullMemberUpTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/PullMemberUp/CSharpPullMemberUpTests.cs
@@ -4644,7 +4644,7 @@ public class TestClass : Base
         }
     }
 }";
-        await TestWithPullMemberDialogAsync(testText, expected, new (string, bool)[] { ("TestMethod", true) }, index: 1);
+        await TestWithPullMemberDialogAsync(testText, expected, [("TestMethod", true)], index: 1);
     }
 
     [Fact]
@@ -4675,7 +4675,7 @@ public abstract class TestClass : Base
     {
     }
 }";
-        await TestWithPullMemberDialogAsync(testText, expected, new (string, bool)[] { ("TestMethod", true) }, index: 0);
+        await TestWithPullMemberDialogAsync(testText, expected, [("TestMethod", true)], index: 0);
     }
 
     [Fact]
@@ -4780,7 +4780,7 @@ public abstract class Testclass2 : Base2
     }
 }";
 
-        await TestWithPullMemberDialogAsync(testText, expected, new (string, bool)[] { ("Event3", false) });
+        await TestWithPullMemberDialogAsync(testText, expected, [("Event3", false)]);
     }
 
     [Fact]
@@ -4814,7 +4814,7 @@ public class Testclass2 : ITest
     }
 }";
 
-        await TestWithPullMemberDialogAsync(testText, expected, new (string, bool)[] { ("Event3", false) });
+        await TestWithPullMemberDialogAsync(testText, expected, [("Event3", false)]);
     }
 
     [Fact]
@@ -4848,7 +4848,7 @@ public abstract class TestClass2 : ITest
     }
 }";
 
-        await TestWithPullMemberDialogAsync(testText, expected, new (string, bool)[] { ("Event3", false) });
+        await TestWithPullMemberDialogAsync(testText, expected, [("Event3", false)]);
     }
 
     [Fact]
@@ -4903,7 +4903,7 @@ public event EventHandler Event1
     }
 }";
 
-        await TestWithPullMemberDialogAsync(testText, expected, new (string, bool)[] { ("Event1", false) });
+        await TestWithPullMemberDialogAsync(testText, expected, [("Event1", false)]);
     }
 
     [Fact]
@@ -5079,7 +5079,7 @@ class D : B
 {
     override int X => 7;
 }";
-        await TestWithPullMemberDialogAsync(testText, expected, selection: new[] { ("X", true) }, index: 1);
+        await TestWithPullMemberDialogAsync(testText, expected, selection: [("X", true)], index: 1);
     }
 
     [Fact]
@@ -5114,7 +5114,7 @@ public class Testclass2 : Base2
         private event EventHandler Event4;
     }
 }";
-        await TestWithPullMemberDialogAsync(testText, expected, selection: new[] { ("Event3", true) }, index: 1);
+        await TestWithPullMemberDialogAsync(testText, expected, selection: [("Event3", true)], index: 1);
     }
 
     [Fact]
@@ -5169,7 +5169,7 @@ public override event EventHandler Event1
     }
 }";
 
-        await TestWithPullMemberDialogAsync(testText, expected, new (string, bool)[] { ("Event1", true) }, index: 1);
+        await TestWithPullMemberDialogAsync(testText, expected, [("Event1", true)], index: 1);
     }
 
     #endregion Dialog

diff --git a/...Sharp/Portable/CodeRefactorings/PullMemberUp/CSharpPullMemberUpCodeRefactoringProvider.cs b/...Sharp/Portable/CodeRefactorings/PullMemberUp/CSharpPullMemberUpCodeRefactoringProvider.cs
@@ -2,8 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 using System;
 using System.Collections.Immutable;
 using System.Composition;
@@ -18,7 +16,8 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeRefactorings.PullMemberUp;
 
 [ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = PredefinedCodeRefactoringProviderNames.PullMemberUp), Shared]
 [method: SuppressMessage("RoslynDiagnosticsReliability", "RS0034:Exported parts should have [ImportingConstructor]", Justification = "Used incorrectly by tests")]
-internal class CSharpPullMemberUpCodeRefactoringProvider(IPullMemberUpOptionsService service) : AbstractPullMemberUpRefactoringProvider(service)
+internal sealed class CSharpPullMemberUpCodeRefactoringProvider(IPullMemberUpOptionsService? service)
+    : AbstractPullMemberUpRefactoringProvider(service)
 {
     [ImportingConstructor]
     [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]

diff --git a/src/Features/Core/Portable/FeaturesResources.resx b/src/Features/Core/Portable/FeaturesResources.resx
@@ -1156,8 +1156,8 @@ This version used in: {2}</value>
   <data name="Merge_with_next_0_statement" xml:space="preserve">
     <value>Merge with next '{0}' statement</value>
   </data>
-  <data name="Pull_0_up" xml:space="preserve">
-    <value>Pull '{0}' up</value>
+  <data name="Pull_0_up_to" xml:space="preserve">
+    <value>Pull '{0}' up to ...</value>
   </data>
   <data name="Pull_members_up_to_base_type" xml:space="preserve">
     <value>Pull members up to base type...</value>
@@ -1171,9 +1171,6 @@ This version used in: {2}</value>
   <data name="Wrap_long_call_chain" xml:space="preserve">
     <value>Wrap long call chain</value>
   </data>
-  <data name="Pull_0_up_to_1" xml:space="preserve">
-    <value>Pull '{0}' up to '{1}'</value>
-  </data>
   <data name="Wrap_and_align_expression" xml:space="preserve">
     <value>Wrap and align expression</value>
   </data>

diff --git a/src/Features/Core/Portable/PullMemberUp/AbstractPullMemberUpRefactoringProvider.cs b/src/Features/Core/Portable/PullMemberUp/AbstractPullMemberUpRefactoringProvider.cs
@@ -15,18 +15,12 @@
 
 namespace Microsoft.CodeAnalysis.CodeRefactorings.PullMemberUp;
 
-internal abstract partial class AbstractPullMemberUpRefactoringProvider : CodeRefactoringProvider
+internal abstract partial class AbstractPullMemberUpRefactoringProvider(IPullMemberUpOptionsService? service) : CodeRefactoringProvider
 {
-    private IPullMemberUpOptionsService? _service;
+    private IPullMemberUpOptionsService? _service = service;
 
     protected abstract Task<ImmutableArray<SyntaxNode>> GetSelectedNodesAsync(CodeRefactoringContext context);
 
-    /// <summary>
-    /// Test purpose only
-    /// </summary>
-    protected AbstractPullMemberUpRefactoringProvider(IPullMemberUpOptionsService? service)
-        => _service = service;
-
     public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context)
     {
         // Currently support to pull field, method, event, property and indexer up,
@@ -35,64 +29,48 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
 
         _service ??= document.Project.Solution.Services.GetService<IPullMemberUpOptionsService>();
         if (_service == null)
-        {
             return;
-        }
 
         var selectedMemberNodes = await GetSelectedNodesAsync(context).ConfigureAwait(false);
         if (selectedMemberNodes.IsEmpty)
-        {
             return;
-        }
 
         var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
         var memberNodeSymbolPairs = selectedMemberNodes
             .SelectAsArray(m => (node: m, symbol: semanticModel.GetRequiredDeclaredSymbol(m, cancellationToken)))
             .WhereAsArray(pair => MemberAndDestinationValidator.IsMemberValid(pair.symbol));
 
         if (memberNodeSymbolPairs.IsEmpty)
-        {
             return;
-        }
 
         var selectedMembers = memberNodeSymbolPairs.SelectAsArray(pair => pair.symbol);
 
         var containingType = selectedMembers.First().ContainingType;
         Contract.ThrowIfNull(containingType);
         if (selectedMembers.Any(m => !m.ContainingType.Equals(containingType)))
-        {
             return;
-        }
-
-        // we want to use a span which covers all the selected viable member nodes, so that more specific nodes have priority
-        var memberSpan = TextSpan.FromBounds(
-            memberNodeSymbolPairs.First().node.FullSpan.Start,
-            memberNodeSymbolPairs.Last().node.FullSpan.End);
 
         var allDestinations = FindAllValidDestinations(
             selectedMembers,
             containingType,
             document.Project.Solution,
             cancellationToken);
         if (allDestinations.Length == 0)
-        {
             return;
-        }
-
-        var allActions = allDestinations.Select(destination => MembersPuller.TryComputeCodeAction(document, selectedMembers, destination))
-            .WhereNotNull()
-            .Concat(new PullMemberUpWithDialogCodeAction(document, selectedMembers, _service))
-            .ToImmutableArray();
-
-        var title = selectedMembers.IsSingle()
-            ? string.Format(FeaturesResources.Pull_0_up, selectedMembers.Single().ToNameDisplayString())
-            : FeaturesResources.Pull_selected_members_up;
-
-        var nestedCodeAction = CodeAction.Create(
-            title,
-            allActions, isInlinable: true);
 
-        context.RegisterRefactoring(nestedCodeAction, memberSpan);
+        context.RegisterRefactoring(CodeAction.Create(
+                selectedMembers.IsSingle()
+                    ? string.Format(FeaturesResources.Pull_0_up_to, selectedMembers.Single().ToNameDisplayString())
+                    : FeaturesResources.Pull_selected_members_up,
+                allDestinations.Select(destination => MembersPuller.TryComputeCodeAction(document, selectedMembers, destination))
+                    .WhereNotNull()
+                    .Concat(new PullMemberUpWithDialogCodeAction(document, selectedMembers, _service))
+                    .ToImmutableArray(),
+                isInlinable: false),
+            // we want to use a span which covers all the selected viable member nodes, so that more specific nodes have priority
+            TextSpan.FromBounds(
+                memberNodeSymbolPairs.First().node.FullSpan.Start,
+                memberNodeSymbolPairs.Last().node.FullSpan.End));
     }
 
     private static ImmutableArray<INamedTypeSymbol> FindAllValidDestinations(

diff --git a/src/Features/Core/Portable/PullMemberUp/MembersPuller.cs b/src/Features/Core/Portable/PullMemberUp/MembersPuller.cs
@@ -48,10 +48,7 @@ public static CodeAction TryComputeCodeAction(
             return null;
         }
 
-        var title = selectedMembers.IsSingle()
-            ? string.Format(FeaturesResources.Pull_0_up_to_1, selectedMembers.Single().Name, result.Destination.Name)
-            : string.Format(FeaturesResources.Pull_selected_members_up_to_0, result.Destination.Name);
-
+        var title = result.Destination.Name;
         return SolutionChangeAction.Create(
             title,
             cancellationToken => PullMembersUpAsync(document, result, cancellationToken),
@@ -250,10 +247,7 @@ private static void ChangeEventToPublicAndNonStatic(
                 accessibility: Accessibility.Public,
                 modifiers: modifiers);
 
-            var eventGenerationInfo = info.WithContext(new CodeGenerationContext(
-                null,
-                null,
-                generateMethodBodies: false));
+            var eventGenerationInfo = info.WithContext(new CodeGenerationContext(generateMethodBodies: false));
 
             var publicAndNonStaticSyntax = codeGenerationService.CreateEventDeclaration(publicAndNonStaticSymbol, CodeGenerationDestination.ClassType, eventGenerationInfo, cancellationToken);
             // Insert a new declaration and remove the original declaration
@@ -439,10 +433,9 @@ private static ImmutableArray<SyntaxNode> GetImports(SyntaxNode start, ISyntaxFa
     {
         return start.AncestorsAndSelf()
             .Where(node => node is ICompilationUnitSyntax || syntaxFacts.IsBaseNamespaceDeclaration(node))
-            .SelectMany(node => node is ICompilationUnitSyntax
+            .SelectManyAsArray(node => node is ICompilationUnitSyntax
                 ? syntaxFacts.GetImportsOfCompilationUnit(node)
-                : syntaxFacts.GetImportsOfBaseNamespaceDeclaration(node))
-            .ToImmutableArray();
+                : syntaxFacts.GetImportsOfBaseNamespaceDeclaration(node));
     }
 
     private static ISymbol MakeAbstractVersion(ISymbol member)
@@ -495,14 +488,9 @@ private static async Task<ImmutableDictionary<ISymbol, ImmutableArray<SyntaxNode
     /// </summary>
     private static bool IsSelectedMemberDeclarationAlreadyInDestination(ISymbol selectedMember, INamedTypeSymbol destination)
     {
-        if (destination.TypeKind == TypeKind.Interface)
-        {
-            return IsSelectedMemberDeclarationAlreadyInDestinationInterface(selectedMember, destination);
-        }
-        else
-        {
-            return IsSelectedMemberDeclarationAlreadyInDestinationClass(selectedMember, destination);
-        }
+        return destination.TypeKind == TypeKind.Interface
+            ? IsSelectedMemberDeclarationAlreadyInDestinationInterface(selectedMember, destination)
+            : IsSelectedMemberDeclarationAlreadyInDestinationClass(selectedMember, destination);
     }
 
     private static bool IsSelectedMemberDeclarationAlreadyInDestinationClass(ISymbol selectedMember, INamedTypeSymbol destination)

diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf
diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf
diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf
diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf
diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf
diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf