Skip to content

Commit

Permalink
Allow moving static members between different type kinds (#76313)
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi authored Dec 7, 2024
2 parents 753b5a7 + e497d62 commit 5e99add
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,10 @@
namespace Microsoft.CodeAnalysis.CSharp.CodeRefactorings.MoveStaticMembers;

[ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = PredefinedCodeRefactoringProviderNames.MoveStaticMembers), Shared]
internal class CSharpMoveStaticMembersRefactoringProvider : AbstractMoveStaticMembersRefactoringProvider
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed class CSharpMoveStaticMembersRefactoringProvider() : AbstractMoveStaticMembersRefactoringProvider()
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public CSharpMoveStaticMembersRefactoringProvider() : base()
{
}

protected override Task<ImmutableArray<SyntaxNode>> GetSelectedNodesAsync(CodeRefactoringContext context)
=> NodeSelectionHelpers.GetSelectedDeclarationsOrVariablesAsync(context);
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,19 @@
namespace Microsoft.VisualStudio.LanguageServices.Implementation.MoveStaticMembers;

[ExportWorkspaceService(typeof(IMoveStaticMembersOptionsService), ServiceLayer.Host), Shared]
internal class VisualStudioMoveStaticMembersOptionsService : IMoveStaticMembersOptionsService
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed class VisualStudioMoveStaticMembersOptionsService(
IGlyphService glyphService,
IUIThreadOperationExecutor uiThreadOperationExecutor) : IMoveStaticMembersOptionsService
{
private readonly IGlyphService _glyphService;
private readonly IUIThreadOperationExecutor _uiThreadOperationExecutor;
private readonly IGlyphService _glyphService = glyphService;
private readonly IUIThreadOperationExecutor _uiThreadOperationExecutor = uiThreadOperationExecutor;

private const int HistorySize = 3;

public readonly LinkedList<INamedTypeSymbol> History = new();

[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public VisualStudioMoveStaticMembersOptionsService(
IGlyphService glyphService,
IUIThreadOperationExecutor uiThreadOperationExecutor)
{
_glyphService = glyphService;
_uiThreadOperationExecutor = uiThreadOperationExecutor;
}

public MoveStaticMembersOptions GetMoveMembersToTypeOptions(Document document, INamedTypeSymbol selectedType, ImmutableArray<ISymbol> selectedNodeSymbols)
{
var viewModel = GetViewModel(document, selectedType, selectedNodeSymbols, History, _glyphService, _uiThreadOperationExecutor);
Expand Down Expand Up @@ -170,22 +164,33 @@ private static ImmutableArray<TypeNameItem> MakeTypeNameItems(
CancellationToken cancellationToken)
{
return currentNamespace.GetAllTypes(cancellationToken)
// only take symbols that are the same kind of type (class, module)
// and remove non-static types only when the current type is static
.Where(t => t.TypeKind == currentType.TypeKind && (t.IsStaticType() || !currentType.IsStaticType()))
// Remove non-static types only when the current type is static
.Where(destinationType => IsValidTypeToMoveBetween(destinationType, currentType) && (destinationType.IsStaticType() || !currentType.IsStaticType()))
.SelectMany(t =>
{
// for partially declared classes, we may want multiple entries for a single type.
// filter to those actually in a real file, and that is not our current location.
return t.Locations
.Where(l => l.IsInSource &&
(currentType.Name != t.Name || GetFile(l) != currentDocument.FilePath))
.Where(l => l.IsInSource && (currentType.Name != t.Name || GetFile(l) != currentDocument.FilePath))
.Select(l => new TypeNameItem(
history.Contains(t),
GetFile(l),
t));
})
.ToImmutableArrayOrEmpty()
.Sort(comparison: TypeNameItem.CompareTo);
.ToImmutableArrayOrEmpty()
.Sort(comparison: TypeNameItem.CompareTo);
}

private static bool IsValidTypeToMoveBetween(INamedTypeSymbol destinationType, INamedTypeSymbol sourceType)
{
// Can only moved to named typed that can actually contain members.
if (destinationType.TypeKind is not (TypeKind.Class or TypeKind.Interface or TypeKind.Module or TypeKind.Struct))
return false;

// Very unlikely to be moving from a non-interface to an interface. Filter out for now.
if (sourceType.TypeKind != TypeKind.Interface && destinationType.TypeKind == TypeKind.Interface)
return false;

return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Imports Microsoft.VisualStudio.LanguageServices.Implementation.MoveStaticMembers
Imports Microsoft.VisualStudio.LanguageServices.Implementation.PullMemberUp
Imports Microsoft.VisualStudio.LanguageServices.Implementation.Utilities
Imports Microsoft.VisualStudio.Utilities
Imports Roslyn.Test.Utilities

Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers
<UseExportProvider>
Expand Down Expand Up @@ -426,6 +427,95 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers
Assert.Equal("TestNs.ConflictingClassName", options.Destination.ToDisplayString())
Assert.Equal("TestFile.cs", options.FileName)
End Function

<Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70896")>
Public Async Function CSTestTypeSelection2() As Task
Dim markUp = <Text><![CDATA[
<Workspace>
<Project Language="C#" AssemblyName="CSAssembly1" CommonReferences="true">
<Document>
namespace TestNs
{
public class FromClass
{
public static int Bar$$bar()
{
return 12345;
}
}

public struct TestStruct
{
}

public interface ITestInterface
{
}
}
</Document>
</Project>
</Workspace>]]></Text>
Dim viewModel = Await GetViewModelAsync(markUp)

' Should Not have the interface in the list as we started from a class. Should still have the struct through.
Assert.Equal(1, viewModel.AvailableTypes.Length)
Assert.Equal(1, viewModel.MemberSelectionViewModel.CheckedMembers.Length)

viewModel.SearchText = viewModel.AvailableTypes.ElementAt(0).TypeName
Assert.Equal("TestNs.TestStruct", viewModel.DestinationName.TypeName)
Assert.NotNull(viewModel.DestinationName.NamedType)
Assert.False(viewModel.DestinationName.IsNew)
Assert.False(viewModel.ShowMessage)
Assert.True(viewModel.CanSubmit)
End Function

<Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70896")>
Public Async Function CSTestTypeSelection3() As Task
Dim markUp = <Text><![CDATA[
<Workspace>
<Project Language="C#" AssemblyName="CSAssembly1" CommonReferences="true">
<Document>
namespace TestNs
{
public interface FromInterface
{
public static int Bar$$bar()
{
return 12345;
}
}

public struct TestStruct
{
}

public interface ITestInterface
{
}
}
</Document>
</Project>
</Workspace>]]></Text>
Dim viewModel = Await GetViewModelAsync(markUp)

Assert.Equal(2, viewModel.AvailableTypes.Length)
Assert.Equal(1, viewModel.MemberSelectionViewModel.CheckedMembers.Length)

' Should have the interface and the struct in the list as we started from an interface.
viewModel.SearchText = "TestNs.TestStruct"
Assert.Equal("TestNs.TestStruct", viewModel.DestinationName.TypeName)
Assert.NotNull(viewModel.DestinationName.NamedType)
Assert.False(viewModel.DestinationName.IsNew)
Assert.False(viewModel.ShowMessage)
Assert.True(viewModel.CanSubmit)

viewModel.SearchText = "TestNs.ITestInterface"
Assert.Equal("TestNs.ITestInterface", viewModel.DestinationName.TypeName)
Assert.NotNull(viewModel.DestinationName.NamedType)
Assert.False(viewModel.DestinationName.IsNew)
Assert.False(viewModel.ShowMessage)
Assert.True(viewModel.CanSubmit)
End Function
#End Region

#Region "VB"
Expand Down

0 comments on commit 5e99add

Please sign in to comment.