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

Disable extract class/interface to a new file when unsupported by the workspace. #76717

Merged
merged 4 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -10,8 +10,6 @@
using System.Threading;
using Microsoft.CodeAnalysis.ExtractInterface;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Notification;

namespace Microsoft.CodeAnalysis.Editor.UnitTests.ExtractInterface;

Expand All @@ -33,14 +31,12 @@ internal sealed class TestExtractInterfaceOptionsService() : IExtractInterfaceOp
public bool SameFile { get; set; }

public ExtractInterfaceOptionsResult GetExtractInterfaceOptions(
ISyntaxFactsService syntaxFactsService,
INotificationService notificationService,
Document document,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The services and language can be pulled from the document.

List<ISymbol> extractableMembers,
string defaultInterfaceName,
List<string> conflictingTypeNames,
string defaultNamespace,
string generatedNameTypeParameterSuffix,
string languageName,
CancellationToken cancellationToken)
{
this.AllExtractableMembers = extractableMembers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,7 @@ internal abstract class AbstractExtractClassRefactoringProvider(IExtractClassOpt

public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context)
{
// For simplicity if we can't add a document the don't supply this refactoring. Not checking this results in known
// cases that won't work because the refactoring may try to add a document. There's non-trivial
// work to support a user interaction that makes sense for those cases.
// See: https://github.com/dotnet/roslyn/issues/50868
var solution = context.Document.Project.Solution;
if (!solution.CanApplyChange(ApplyChangesKind.AddDocument))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will now be handled by disabling the new file option on the extract dialog.

{
return;
}

var optionsService = _optionsService ?? solution.Services.GetService<IExtractClassOptionsService>();
if (optionsService is null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,20 +264,16 @@ internal static ExtractInterfaceOptionsResult GetExtractInterfaceOptions(
var conflictingTypeNames = type.ContainingNamespace.GetAllTypes(cancellationToken).Select(t => t.Name);
var candidateInterfaceName = type.TypeKind == TypeKind.Interface ? type.Name : "I" + type.Name;
var defaultInterfaceName = NameGenerator.GenerateUniqueName(candidateInterfaceName, name => !conflictingTypeNames.Contains(name));
var syntaxFactsService = document.GetRequiredLanguageService<ISyntaxFactsService>();
var notificationService = document.Project.Solution.Services.GetRequiredService<INotificationService>();
var generatedNameTypeParameterSuffix = ExtractTypeHelpers.GetTypeParameterSuffix(document, formattingOptions, type, extractableMembers, cancellationToken);

var service = document.Project.Solution.Services.GetRequiredService<IExtractInterfaceOptionsService>();
return service.GetExtractInterfaceOptions(
syntaxFactsService,
notificationService,
document,
[.. extractableMembers],
defaultInterfaceName,
[.. conflictingTypeNames],
containingNamespace,
generatedNameTypeParameterSuffix,
document.Project.Language,
cancellationToken);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,17 @@
using System.Collections.Generic;
using System.Threading;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Notification;

namespace Microsoft.CodeAnalysis.ExtractInterface;

internal interface IExtractInterfaceOptionsService : IWorkspaceService
{
ExtractInterfaceOptionsResult GetExtractInterfaceOptions(
ISyntaxFactsService syntaxFactsService,
INotificationService notificationService,
Document document,
List<ISymbol> extractableMembers,
string defaultInterfaceName,
List<string> conflictingTypeNames,
string defaultNamespace,
string generatedNameTypeParameterSuffix,
string languageName,
CancellationToken cancellationToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
using Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.ExtractInterface;
using Microsoft.CodeAnalysis.ExtractInterface;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Notification;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Internal.ExtractInterface;

Expand All @@ -24,14 +21,12 @@ internal sealed class OmniSharpExtractInterfaceOptionsService(
private readonly IOmniSharpExtractInterfaceOptionsService _omniSharpExtractInterfaceOptionsService = omniSharpExtractInterfaceOptionsService;

public ExtractInterfaceOptionsResult GetExtractInterfaceOptions(
ISyntaxFactsService syntaxFactsService,
INotificationService notificationService,
Document document,
List<ISymbol> extractableMembers,
string defaultInterfaceName,
List<string> conflictingTypeNames,
string defaultNamespace,
string generatedNameTypeParameterSuffix,
string languageName,
CancellationToken cancellationToken)
{
var result = _omniSharpExtractInterfaceOptionsService.GetExtractInterfaceOptions(extractableMembers, defaultInterfaceName);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
Grid.Column="0"
Content="{Binding ElementName=control, Path=SelectNewFileAsDestination}"
IsChecked="{Binding Destination, Converter={StaticResource enumBoolConverter}, ConverterParameter={x:Static local:NewTypeDestination.NewFile}}"
IsEnabled="{Binding CanAddDocument}"
Padding="{StaticResource ResourceKey=radioButtonPadding}"
VerticalAlignment="Center"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ internal class NewTypeDestinationSelectionViewModel : AbstractNotifyPropertyChan
string.Empty,
string.Empty,
[],
null
null,
false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

named parameters.

JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
);

private readonly string _fileExtension;
Expand All @@ -37,7 +38,8 @@ public NewTypeDestinationSelectionViewModel(
string defaultNamespace,
string generatedNameTypeParameterSuffix,
ImmutableArray<string> conflictingNames,
ISyntaxFactsService? syntaxFactsService)
ISyntaxFactsService? syntaxFactsService,
bool canAddDocument)
{
_defaultName = defaultName;
_fileExtension = languageName == LanguageNames.CSharp ? ".cs" : ".vb";
Expand All @@ -48,8 +50,12 @@ public NewTypeDestinationSelectionViewModel(
_typeName = _defaultName;
_syntaxFactsService = syntaxFactsService;
_fileName = $"{defaultName}{_fileExtension}";
CanAddDocument = canAddDocument;
_destination = canAddDocument ? NewTypeDestination.NewFile : NewTypeDestination.CurrentFile;
}

public bool CanAddDocument { get; }

private string _typeName;
public string TypeName
{
Expand Down Expand Up @@ -87,7 +93,7 @@ public string FileName
set { SetProperty(ref _fileName, value); }
}

private NewTypeDestination _destination = NewTypeDestination.NewFile;
private NewTypeDestination _destination;
public NewTypeDestination Destination
{
get { return _destination; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public ExtractClassViewModel(
string languageName,
string typeParameterSuffix,
ImmutableArray<string> conflictingNames,
ISyntaxFactsService syntaxFactsService)
ISyntaxFactsService syntaxFactsService,
bool canAddDocument)
{
_notificationService = notificationService;
_selectedType = selectedType;
Expand All @@ -46,7 +47,8 @@ public ExtractClassViewModel(
defaultNamespace,
typeParameterSuffix,
conflictingNames,
syntaxFactsService);
syntaxFactsService,
canAddDocument);
}

internal bool TrySubmit()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ internal sealed class VisualStudioExtractClassOptionsService(
CancellationToken cancellationToken)
{
_threadingContext.ThrowIfNotOnUIThread();
var notificationService = document.Project.Solution.Services.GetRequiredService<INotificationService>();

var solution = document.Project.Solution;
var canAddDocument = solution.CanApplyChange(ApplyChangesKind.AddDocument);
var notificationService = solution.Services.GetRequiredService<INotificationService>();

var membersInType = selectedType.GetMembers().
WhereAsArray(MemberAndDestinationValidator.IsMemberValid);
Expand Down Expand Up @@ -85,7 +88,8 @@ internal sealed class VisualStudioExtractClassOptionsService(
document.Project.Language,
generatedNameTypeParameterSuffix,
[.. conflictingTypeNames],
document.GetRequiredLanguageService<ISyntaxFactsService>());
document.GetRequiredLanguageService<ISyntaxFactsService>(),
canAddDocument);

var dialog = new ExtractClassDialog(viewModel);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ internal ExtractInterfaceDialogViewModel(
ImmutableArray<LanguageServices.Utilities.MemberSymbolViewModel> memberViewModels,
string defaultNamespace,
string generatedNameTypeParameterSuffix,
string languageName)
string languageName,
bool canAddDocument)
{
_notificationService = notificationService;

Expand All @@ -49,7 +50,8 @@ internal ExtractInterfaceDialogViewModel(
defaultNamespace,
generatedNameTypeParameterSuffix,
[.. conflictingTypeNames],
syntaxFactsService);
syntaxFactsService,
canAddDocument);
}

internal bool TrySubmit()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,19 @@
// 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.Generic;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeGeneration;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.ExtractInterface;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Notification;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.VisualStudio.Language.Intellisense;
using Microsoft.VisualStudio.LanguageServices.Implementation.CommonControls;
using Microsoft.VisualStudio.LanguageServices.Utilities;
Expand All @@ -36,19 +33,18 @@ internal sealed class VisualStudioExtractInterfaceOptionsService(IGlyphService g
private readonly IUIThreadOperationExecutor _uiThreadOperationExecutor = uiThreadOperationExecutor;

public ExtractInterfaceOptionsResult GetExtractInterfaceOptions(
ISyntaxFactsService syntaxFactsService,
INotificationService notificationService,
Document document,
List<ISymbol> extractableMembers,
string defaultInterfaceName,
List<string> allTypeNames,
string defaultNamespace,
string generatedNameTypeParameterSuffix,
string languageName,
CancellationToken cancellationToken)
{
_threadingContext.ThrowIfNotOnUIThread();

using var cancellationTokenSource = new CancellationTokenSource();
var solution = document.Project.Solution;
var canAddDocument = solution.CanApplyChange(ApplyChangesKind.AddDocument);
var notificationService = solution.Services.GetRequiredService<INotificationService>();

var memberViewModels = extractableMembers
.SelectAsArray(member =>
Expand All @@ -61,15 +57,16 @@ public ExtractInterfaceOptionsResult GetExtractInterfaceOptions(
});

var viewModel = new ExtractInterfaceDialogViewModel(
syntaxFactsService,
document.GetRequiredLanguageService<ISyntaxFactsService>(),
_uiThreadOperationExecutor,
notificationService,
defaultInterfaceName,
allTypeNames,
memberViewModels,
defaultNamespace,
generatedNameTypeParameterSuffix,
languageName);
document.Project.Language,
canAddDocument);

var dialog = new ExtractInterfaceDialog(viewModel);
var result = dialog.ShowModal();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,8 @@ class $$MyClass
languageName:=languageName,
generatedNameTypeParameterSuffix:=generatedNameTypeParameterSuffix,
conflictingNames:=symbol.ContainingNamespace.GetAllTypes(CancellationToken.None).SelectAsArray(Function(t) t.Name),
syntaxFactsService:=workspaceDoc.GetRequiredLanguageService(Of ISyntaxFactsService))
syntaxFactsService:=workspaceDoc.GetRequiredLanguageService(Of ISyntaxFactsService),
canAddDocument:=True)
End Using
End Function
End Class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ public class $$MyClass
memberViewModels:=memberViewModels.ToImmutableArray(),
defaultNamespace:=defaultNamespace,
generatedNameTypeParameterSuffix:=generatedNameTypeParameterSuffix,
languageName:=doc.Project.Language)
languageName:=doc.Project.Language,
canAddDocument:=True)
End Using
End Function
End Class
Expand Down
Loading