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

Fix duplicated namespace in the 'move static member' dialog #76314

Merged
merged 4 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 @@ -124,13 +124,13 @@
Grid.Column="0"
HorizontalAlignment="Right"
x:Name="Namespace"
Text="{Binding PrependedNamespace}"
Text="{Binding TypeName_NamespaceOnly}"
TextTrimming="CharacterEllipsis"/>

<vs:LiveTextBlock
Grid.Column="1"
x:Name="TypeName"
Text="{Binding DestinationName.TypeName}"/>
Text="{Binding TypeName_NameOnly}"/>
</Grid>
</GroupBox>
</StackPanel>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ internal class MoveStaticMembersDialogViewModel : AbstractNotifyPropertyChanged
public StaticMemberSelectionViewModel MemberSelectionViewModel { get; }

private readonly ISyntaxFacts _syntaxFacts;
private readonly string _prependedNamespace;

public MoveStaticMembersDialogViewModel(
StaticMemberSelectionViewModel memberSelectionViewModel,
Expand All @@ -30,14 +31,33 @@ public MoveStaticMembersDialogViewModel(
MemberSelectionViewModel = memberSelectionViewModel;
_syntaxFacts = syntaxFacts ?? throw new ArgumentNullException(nameof(syntaxFacts));
_searchText = defaultType;
_destinationName = new TypeNameItem(defaultType);
_prependedNamespace = string.IsNullOrEmpty(prependedNamespace) ? prependedNamespace : prependedNamespace + ".";

_destinationName = new TypeNameItem(_prependedNamespace + defaultType);
AvailableTypes = availableTypes;
PrependedNamespace = string.IsNullOrEmpty(prependedNamespace) ? prependedNamespace : prependedNamespace + ".";

PropertyChanged += MoveMembersToTypeDialogViewModel_PropertyChanged;
OnDestinationUpdated();
}

public string TypeName_NamespaceOnly
{
get
{
var lastDot = _destinationName.FullyQualifiedTypeName.LastIndexOf('.');
return lastDot >= 0 ? _destinationName.FullyQualifiedTypeName[0..(lastDot + 1)] : "";
}
}

public string TypeName_NameOnly
{
get
{
var lastDot = _destinationName.FullyQualifiedTypeName.LastIndexOf('.');
return lastDot >= 0 ? _destinationName.FullyQualifiedTypeName[(lastDot + 1)..] : _destinationName.FullyQualifiedTypeName;
}
}

private void MoveMembersToTypeDialogViewModel_PropertyChanged(object sender, PropertyChangedEventArgs e)
{
switch (e.PropertyName)
Expand All @@ -54,14 +74,18 @@ private void MoveMembersToTypeDialogViewModel_PropertyChanged(object sender, Pro

private void OnSearchTextUpdated()
{
var foundItem = AvailableTypes.FirstOrDefault(t => t.TypeName == SearchText);
var foundItem = AvailableTypes.FirstOrDefault(t => t.FullyQualifiedTypeName == SearchText);
if (foundItem is null)
{
DestinationName = new(PrependedNamespace + SearchText);
return;
DestinationName = new(_prependedNamespace + SearchText);
}
else
{
DestinationName = foundItem;
}

DestinationName = foundItem;
NotifyPropertyChanged(nameof(TypeName_NameOnly));
NotifyPropertyChanged(nameof(TypeName_NamespaceOnly));
}

public void OnDestinationUpdated()
Expand All @@ -73,7 +97,7 @@ public void OnDestinationUpdated()
return;
}

CanSubmit = IsValidType(_destinationName.TypeName);
CanSubmit = IsValidType(_destinationName.FullyQualifiedTypeName);

if (CanSubmit)
{
Expand All @@ -92,24 +116,17 @@ public void OnDestinationUpdated()
private bool IsValidType(string typeName)
{
if (string.IsNullOrEmpty(typeName))
{
return false;
}

foreach (var identifier in typeName.Split('.'))
{
if (_syntaxFacts.IsValidIdentifier(identifier))
{
continue;
}

return false;
if (!_syntaxFacts.IsValidIdentifier(identifier))
return false;
}

return true;
}

public string PrependedNamespace { get; }
public ImmutableArray<TypeNameItem> AvailableTypes { get; }

private TypeNameItem _destinationName;
Expand Down
16 changes: 8 additions & 8 deletions src/VisualStudio/Core/Def/MoveStaticMembers/TypeNameItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

namespace Microsoft.VisualStudio.LanguageServices.Implementation.MoveStaticMembers;

internal class TypeNameItem
internal sealed class TypeNameItem
{
public string TypeName { get; }
public string FullyQualifiedTypeName { get; }
public INamedTypeSymbol? NamedType { get; }
public string DeclarationFilePath { get; }
public string DeclarationFileName { get; }
Expand All @@ -22,22 +22,22 @@ public TypeNameItem(bool isFromHistory, string declarationFile, INamedTypeSymbol
IsFromHistory = isFromHistory;
IsNew = false;
NamedType = type;
TypeName = type.ToDisplayString();
FullyQualifiedTypeName = type.ToDisplayString();
DeclarationFileName = PathUtilities.GetFileName(declarationFile);
DeclarationFilePath = declarationFile;
}

public TypeNameItem(string @typeName)
public TypeNameItem(string fullyQualifiedTypeName)
{
IsFromHistory = false;
IsNew = true;
TypeName = @typeName;
FullyQualifiedTypeName = fullyQualifiedTypeName;
NamedType = null;
DeclarationFileName = string.Empty;
DeclarationFilePath = string.Empty;
}

public override string ToString() => TypeName;
public override string ToString() => FullyQualifiedTypeName;

public static int CompareTo(TypeNameItem x, TypeNameItem y)
{
Expand All @@ -48,8 +48,8 @@ public static int CompareTo(TypeNameItem x, TypeNameItem y)
return x.IsFromHistory ? -1 : 1;
}
// compare by each namespace/finally type
var xnames = x.TypeName.Split('.');
var ynames = y.TypeName.Split('.');
var xnames = x.FullyQualifiedTypeName.Split('.');
var ynames = y.FullyQualifiedTypeName.Split('.');

for (var i = 0; i < Math.Min(xnames.Length, ynames.Length); i++)
{
Expand Down
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 All @@ -67,15 +61,15 @@ internal static MoveStaticMembersOptions GenerateOptions(string language, MoveSt
if (dialogResult)
{
// if the destination name contains extra namespaces, we want the last one as that is the real type name
var typeName = viewModel.DestinationName.TypeName.Split('.').Last();
var typeName = viewModel.DestinationName.FullyQualifiedTypeName.Split('.').Last();
var newFileName = Path.ChangeExtension(typeName, language == LanguageNames.CSharp ? ".cs" : ".vb");
var selectedMembers = viewModel.MemberSelectionViewModel.CheckedMembers.SelectAsArray(vm => vm.Symbol);

if (viewModel.DestinationName.IsNew)
{
return new MoveStaticMembersOptions(
newFileName,
viewModel.DestinationName.TypeName,
viewModel.DestinationName.FullyQualifiedTypeName,
selectedMembers);
}

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;
}
}
Loading
Loading