Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow moving static members between different type kinds #76313

Merged
merged 3 commits into from
Dec 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading