Skip to content

Commit

Permalink
Fix duplicated namespace in the 'move static member' dialog (#76314)
Browse files Browse the repository at this point in the history
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi authored Dec 7, 2024
2 parents 5e99add + a8cd05d commit d1ff6ad
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 52 deletions.
4 changes: 2 additions & 2 deletions src/VisualStudio/Core/Def/MoveStaticMembers/MoveStaticMembersDialog.xaml
View file
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
49 changes: 33 additions & 16 deletions src/VisualStudio/Core/Def/MoveStaticMembers/MoveStaticMembersDialogViewModel.cs
View file
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
View file
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
4 changes: 2 additions & 2 deletions src/VisualStudio/Core/Def/MoveStaticMembers/VisualStudioMoveStaticMembersOptionsService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,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
48 changes: 24 additions & 24 deletions src/VisualStudio/Core/Test/MoveStaticMembers/MoveStaticMembersViewModelTest.vb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers
' We can call the method, but we need the document still to test submission
Dim viewModel = Await GetViewModelAsync(markUp).ConfigureAwait(False)

Assert.Equal("TestClassHelpers", viewModel.DestinationName.TypeName)
Assert.Equal("TestNs.TestClassHelpers", viewModel.DestinationName.FullyQualifiedTypeName)
SetSearchText(viewModel, "ExtraNs.TestClassHelpers")
Assert.Equal("TestNs.ExtraNs.TestClassHelpers", viewModel.DestinationName.TypeName)
Assert.Equal("TestNs.", viewModel.PrependedNamespace)
Assert.Equal("TestNs.ExtraNs.TestClassHelpers", viewModel.DestinationName.FullyQualifiedTypeName)
Assert.Equal("TestNs.ExtraNs.", viewModel.TypeName_NamespaceOnly)
Assert.True(viewModel.CanSubmit)

Dim cancelledOptions = VisualStudioMoveStaticMembersOptionsService.GenerateOptions(LanguageNames.CSharp, viewModel, False)
Expand Down Expand Up @@ -179,8 +179,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers
</Workspace>]]></Text>
Dim viewModel = Await GetViewModelAsync(markUp)

Assert.Equal(viewModel.DestinationName.TypeName, "TestClassHelpers")
Assert.Equal("TestNs.", viewModel.PrependedNamespace)
Assert.Equal(viewModel.DestinationName.FullyQualifiedTypeName, "TestNs.TestClassHelpers")
Assert.Equal("TestNs.", viewModel.TypeName_NamespaceOnly)

Assert.True(viewModel.ShowMessage)
Assert.Equal(ServicesVSResources.New_Type_Name_colon, viewModel.Message)
Expand Down Expand Up @@ -406,15 +406,15 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers
Assert.Equal(2, viewModel.AvailableTypes.Length)
Assert.Equal(1, viewModel.MemberSelectionViewModel.CheckedMembers.Length)

viewModel.SearchText = viewModel.AvailableTypes.ElementAt(1).TypeName
Assert.Equal("TestNs.ExtraNs.ConflictingNsClassName", viewModel.DestinationName.TypeName)
viewModel.SearchText = viewModel.AvailableTypes.ElementAt(1).FullyQualifiedTypeName
Assert.Equal("TestNs.ExtraNs.ConflictingNsClassName", viewModel.DestinationName.FullyQualifiedTypeName)
Assert.NotNull(viewModel.DestinationName.NamedType)
Assert.False(viewModel.DestinationName.IsNew)
Assert.False(viewModel.ShowMessage)
Assert.True(viewModel.CanSubmit)

viewModel.SearchText = viewModel.AvailableTypes.ElementAt(0).TypeName
Assert.Equal("TestNs.ConflictingClassName", viewModel.DestinationName.TypeName)
viewModel.SearchText = viewModel.AvailableTypes.ElementAt(0).FullyQualifiedTypeName
Assert.Equal("TestNs.ConflictingClassName", viewModel.DestinationName.FullyQualifiedTypeName)
Assert.NotNull(viewModel.DestinationName.NamedType)
Assert.False(viewModel.DestinationName.IsNew)
Assert.False(viewModel.ShowMessage)
Expand Down Expand Up @@ -461,8 +461,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers
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)
viewModel.SearchText = viewModel.AvailableTypes.ElementAt(0).FullyQualifiedTypeName
Assert.Equal("TestNs.TestStruct", viewModel.DestinationName.FullyQualifiedTypeName)
Assert.NotNull(viewModel.DestinationName.NamedType)
Assert.False(viewModel.DestinationName.IsNew)
Assert.False(viewModel.ShowMessage)
Expand Down Expand Up @@ -503,14 +503,14 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers

' 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.Equal("TestNs.TestStruct", viewModel.DestinationName.FullyQualifiedTypeName)
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.Equal("TestNs.ITestInterface", viewModel.DestinationName.FullyQualifiedTypeName)
Assert.NotNull(viewModel.DestinationName.NamedType)
Assert.False(viewModel.DestinationName.IsNew)
Assert.False(viewModel.ShowMessage)
Expand Down Expand Up @@ -539,10 +539,10 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers
' We can call the method, but we need the document still to test submission
Dim viewModel = Await GetViewModelAsync(markUp).ConfigureAwait(False)

Assert.Equal("TestClassHelpers", viewModel.DestinationName.TypeName)
Assert.Equal("TestNs.TestClassHelpers", viewModel.DestinationName.FullyQualifiedTypeName)
SetSearchText(viewModel, "ExtraNs.TestClassHelpers")
Assert.Equal("TestNs.ExtraNs.TestClassHelpers", viewModel.DestinationName.TypeName)
Assert.Equal("TestNs.", viewModel.PrependedNamespace)
Assert.Equal("TestNs.ExtraNs.TestClassHelpers", viewModel.DestinationName.FullyQualifiedTypeName)
Assert.Equal("TestNs.ExtraNs.", viewModel.TypeName_NamespaceOnly)
Assert.True(viewModel.CanSubmit)

Dim cancelledOptions = VisualStudioMoveStaticMembersOptionsService.GenerateOptions(LanguageNames.VisualBasic, viewModel, False)
Expand Down Expand Up @@ -609,8 +609,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers
</Workspace>]]></Text>
Dim viewModel = Await GetViewModelAsync(markUp)

Assert.Equal(viewModel.DestinationName.TypeName, "TestClassHelpers")
Assert.Equal("TestNs.", viewModel.PrependedNamespace)
Assert.Equal(viewModel.DestinationName.FullyQualifiedTypeName, "TestNs.TestClassHelpers")
Assert.Equal("TestNs.", viewModel.TypeName_NamespaceOnly)

Assert.True(viewModel.ShowMessage)
Assert.Equal(ServicesVSResources.New_Type_Name_colon, viewModel.Message)
Expand Down Expand Up @@ -681,8 +681,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers

Dim viewModel = Await GetViewModelAsync(markUp)

Assert.Equal(viewModel.DestinationName.TypeName, "TestClassHelpers")
Assert.Equal("RootNs.TestNs.", viewModel.PrependedNamespace)
Assert.Equal(viewModel.DestinationName.FullyQualifiedTypeName, "RootNs.TestNs.TestClassHelpers")
Assert.Equal("RootNs.TestNs.", viewModel.TypeName_NamespaceOnly)

Assert.True(viewModel.ShowMessage)
Assert.Equal(ServicesVSResources.New_Type_Name_colon, viewModel.Message)
Expand Down Expand Up @@ -846,15 +846,15 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.MoveStaticMembers
Assert.Equal(2, viewModel.AvailableTypes.Length)
Assert.Equal(1, viewModel.MemberSelectionViewModel.CheckedMembers.Length)

viewModel.SearchText = viewModel.AvailableTypes.ElementAt(1).TypeName
Assert.Equal("TestNs.ExtraNs.ConflictingNsClassName", viewModel.DestinationName.TypeName)
viewModel.SearchText = viewModel.AvailableTypes.ElementAt(1).FullyQualifiedTypeName
Assert.Equal("TestNs.ExtraNs.ConflictingNsClassName", viewModel.DestinationName.FullyQualifiedTypeName)
Assert.NotNull(viewModel.DestinationName.NamedType)
Assert.False(viewModel.DestinationName.IsNew)
Assert.False(viewModel.ShowMessage)
Assert.True(viewModel.CanSubmit)

viewModel.SearchText = viewModel.AvailableTypes.ElementAt(0).TypeName
Assert.Equal("TestNs.ConflictingClassName", viewModel.DestinationName.TypeName)
viewModel.SearchText = viewModel.AvailableTypes.ElementAt(0).FullyQualifiedTypeName
Assert.Equal("TestNs.ConflictingClassName", viewModel.DestinationName.FullyQualifiedTypeName)
Assert.NotNull(viewModel.DestinationName.NamedType)
Assert.False(viewModel.DestinationName.IsNew)
Assert.False(viewModel.ShowMessage)
Expand Down

0 comments on commit d1ff6ad

Please sign in to comment.