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.

Sign up for GitHub

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

Jump to bottom

Remove fallbacks (2) #74319

Merged
merged 3 commits into from
Jul 10, 2024
Merged

Remove fallbacks (2) #74319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
18b6fe0
Remove fallback options 2
tmat Jul 9, 2024
ffd81eb
Remove DoNotPutOutOrRefOnStructOn extract method option
tmat Jul 9, 2024
21fd113
Fix
tmat Jul 9, 2024
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
3 changes: 1 addition & 2 deletions src/EditorFeatures/CSharp/ExtractInterface/ExtractInterfaceCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.ExtractInterface;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Options;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Utilities;

Expand All @@ -19,6 +18,6 @@ namespace Microsoft.CodeAnalysis.CSharp.ExtractInterface;
[Name(PredefinedCommandHandlerNames.ExtractInterface)]
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed class ExtractInterfaceCommandHandler(IThreadingContext threadingContext, IGlobalOptionService globalOptions) : AbstractExtractInterfaceCommandHandler(threadingContext, globalOptions)
internal sealed class ExtractInterfaceCommandHandler(IThreadingContext threadingContext) : AbstractExtractInterfaceCommandHandler(threadingContext)
{
}
2 changes: 1 addition & 1 deletion src/EditorFeatures/CSharpTest/CodeActions/MoveType/MoveTypeTests.MoveScope.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -921,7 +921,7 @@ private async Task TestNamespaceMove(string originalCode, string expectedCode, b
var moveTypeService = documentToModify.GetLanguageService<IMoveTypeService>();
Assert.NotNull(moveTypeService);

var modifiedSolution = await moveTypeService.GetModifiedSolutionAsync(documentToModify, textSpan, MoveTypeOperationKind.MoveTypeNamespaceScope, CodeActionOptions.DefaultProvider, CancellationToken.None).ConfigureAwait(false);
var modifiedSolution = await moveTypeService.GetModifiedSolutionAsync(documentToModify, textSpan, MoveTypeOperationKind.MoveTypeNamespaceScope, CancellationToken.None).ConfigureAwait(false);

if (expectOperation)
{
Expand Down
21 changes: 8 additions & 13 deletions src/EditorFeatures/CSharpTest/ExtractMethod/ExtractMethodBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.ExtractMethod;
[UseExportProvider]
public class ExtractMethodBase
{
protected static async Task ExpectExtractMethodToFailAsync(string codeWithMarker, bool dontPutOutOrRefOnStruct = true, string[] features = null)
protected static async Task ExpectExtractMethodToFailAsync(string codeWithMarker, string[] features = null)
{
ParseOptions parseOptions = null;
if (features != null)
Expand All @@ -36,20 +36,19 @@ protected static async Task ExpectExtractMethodToFailAsync(string codeWithMarker
using var workspace = EditorTestWorkspace.CreateCSharp(codeWithMarker, parseOptions: parseOptions);
var testDocument = workspace.Documents.First();
var textSpan = testDocument.SelectedSpans.Single();
var treeAfterExtractMethod = await ExtractMethodAsync(workspace, testDocument, succeed: false, dontPutOutOrRefOnStruct: dontPutOutOrRefOnStruct);
var treeAfterExtractMethod = await ExtractMethodAsync(workspace, testDocument, succeed: false);
}

protected static async Task ExpectExtractMethodToFailAsync(
string codeWithMarker,
string expected,
bool dontPutOutOrRefOnStruct = true,
CSharpParseOptions parseOptions = null)
{
using var workspace = EditorTestWorkspace.CreateCSharp(codeWithMarker, parseOptions: parseOptions);
var testDocument = workspace.Documents.Single();
var subjectBuffer = testDocument.GetTextBuffer();

var tree = await ExtractMethodAsync(workspace, testDocument, succeed: false, dontPutOutOrRefOnStruct: dontPutOutOrRefOnStruct);
var tree = await ExtractMethodAsync(workspace, testDocument, succeed: false);

using (var edit = subjectBuffer.CreateEdit())
{
Expand Down Expand Up @@ -77,16 +76,14 @@ protected static async Task TestExtractMethodAsync(
string codeWithMarker,
string expected,
bool temporaryFailing = false,
bool dontPutOutOrRefOnStruct = true,
CSharpParseOptions parseOptions = null)
{
using var workspace = EditorTestWorkspace.CreateCSharp(codeWithMarker, parseOptions: parseOptions);
var testDocument = workspace.Documents.Single();
var subjectBuffer = testDocument.GetTextBuffer();

var tree = await ExtractMethodAsync(
workspace, testDocument,
dontPutOutOrRefOnStruct: dontPutOutOrRefOnStruct);
workspace, testDocument);

using (var edit = subjectBuffer.CreateEdit())
{
Expand Down Expand Up @@ -116,8 +113,7 @@ protected static async Task TestExtractMethodAsync(
protected static async Task<SyntaxNode> ExtractMethodAsync(
EditorTestWorkspace workspace,
EditorTestHostDocument testDocument,
bool succeed = true,
bool dontPutOutOrRefOnStruct = true)
bool succeed = true)
{
var document = workspace.CurrentSolution.GetDocument(testDocument.Id);
Assert.NotNull(document);
Expand All @@ -126,11 +122,10 @@ protected static async Task<SyntaxNode> ExtractMethodAsync(
{
CodeGenerationOptions = CodeGenerationOptions.GetDefault(document.Project.Services),
CodeCleanupOptions = CodeCleanupOptions.GetDefault(document.Project.Services),
ExtractOptions = new() { DoNotPutOutOrRefOnStruct = dontPutOutOrRefOnStruct }
};

var semanticDocument = await SemanticDocument.CreateAsync(document, CancellationToken.None);
var validator = new CSharpSelectionValidator(semanticDocument, testDocument.SelectedSpans.Single(), options.ExtractOptions, localFunction: false);
var validator = new CSharpSelectionValidator(semanticDocument, testDocument.SelectedSpans.Single(), localFunction: false);

var (selectedCode, status) = await validator.GetValidSelectionAsync(CancellationToken.None);
if (!succeed && status.Failed)
Expand Down Expand Up @@ -173,7 +168,7 @@ protected static async Task TestSelectionAsync(string codeWithMarker, bool expec
Assert.NotNull(document);

var semanticDocument = await SemanticDocument.CreateAsync(document, CancellationToken.None);
var validator = new CSharpSelectionValidator(semanticDocument, textSpanOverride ?? namedSpans["b"].Single(), ExtractMethodOptions.Default, localFunction: false);
var validator = new CSharpSelectionValidator(semanticDocument, textSpanOverride ?? namedSpans["b"].Single(), localFunction: false);
var (result, status) = await validator.GetValidSelectionAsync(CancellationToken.None);

if (expectedFail)
Expand Down Expand Up @@ -201,7 +196,7 @@ protected static async Task IterateAllAsync(string code)

foreach (var node in iterator)
{
var validator = new CSharpSelectionValidator(semanticDocument, node.Span, ExtractMethodOptions.Default, localFunction: false);
var validator = new CSharpSelectionValidator(semanticDocument, node.Span, localFunction: false);
var (_, status) = await validator.GetValidSelectionAsync(CancellationToken.None);

// check the obvious case
Expand Down
37 changes: 1 addition & 36 deletions src/EditorFeatures/CSharpTest/ExtractMethod/ExtractMethodTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10973,41 +10973,6 @@ public async Task Hello()
await ExpectExtractMethodToFailAsync(code);
}

[Fact]
public async Task TestDoNotPutOutOrRefForStructOff()
{
var code =
"""
using System.Threading.Tasks;

namespace ClassLibrary9
{
public struct S
{
public int I;
}

public class Class1
{
private async Task<int> Test()
{
S s = new S();
s.I = 10;

[|int i = await Task.Run(() =>
{
var i2 = s.I;
return Test();
});|]

return i;
}
}
}
""";
await ExpectExtractMethodToFailAsync(code, dontPutOutOrRefOnStruct: false);
}

[Fact]
public async Task TestDoNotPutOutOrRefForStructOn()
{
Expand Down Expand Up @@ -11075,7 +11040,7 @@ private async Task<int> NewMethod(S s)
}
""";

await TestExtractMethodAsync(code, expected, dontPutOutOrRefOnStruct: true);
await TestExtractMethodAsync(code, expected);
}

[Theory]
Expand Down
1 change: 0 additions & 1 deletion src/EditorFeatures/CSharpTest/MoveToNamespace/MoveToNamespaceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,6 @@ void Method() { }
var actions = await testState.MoveToNamespaceService.GetCodeActionsAsync(
testState.InvocationDocument,
testState.TestInvocationDocument.SelectedSpans.Single(),
CodeActionOptions.DefaultProvider,
CancellationToken.None);

Assert.Empty(actions);
Expand Down
8 changes: 2 additions & 6 deletions src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@ namespace Microsoft.CodeAnalysis.ExternalAccess.IntelliCode;
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal class IntentSourceProvider(
[ImportMany] IEnumerable<Lazy<IIntentProvider, IIntentProviderMetadata>> lazyIntentProviders,
IGlobalOptionService globalOptions) : IIntentSourceProvider
[ImportMany] IEnumerable<Lazy<IIntentProvider, IIntentProviderMetadata>> lazyIntentProviders) : IIntentSourceProvider
{
private readonly ImmutableDictionary<(string LanguageName, string IntentName), Lazy<IIntentProvider, IIntentProviderMetadata>> _lazyIntentProviders = CreateProviderMap(lazyIntentProviders);
private readonly IGlobalOptionService _globalOptions = globalOptions;

private static ImmutableDictionary<(string LanguageName, string IntentName), Lazy<IIntentProvider, IIntentProviderMetadata>> CreateProviderMap(
IEnumerable<Lazy<IIntentProvider, IIntentProviderMetadata>> lazyIntentProviders)
Expand Down Expand Up @@ -66,9 +64,7 @@ public async Task<ImmutableArray<IntentSource>> ComputeIntentsAsync(IntentReques
originalDocument,
selectionTextSpan,
currentDocument,
new IntentDataProvider(
intentRequestContext.IntentData,
_globalOptions.CreateProvider()),
new IntentDataProvider(intentRequestContext.IntentData),
cancellationToken).ConfigureAwait(false);

if (results.IsDefaultOrEmpty)
Expand Down
6 changes: 1 addition & 5 deletions src/EditorFeatures/Core/ExtractInterface/AbstractExtractInterfaceCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Navigation;
using Microsoft.CodeAnalysis.Notification;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Text;
Expand All @@ -20,12 +19,10 @@ namespace Microsoft.CodeAnalysis.ExtractInterface;
internal abstract class AbstractExtractInterfaceCommandHandler : ICommandHandler<ExtractInterfaceCommandArgs>
{
private readonly IThreadingContext _threadingContext;
private readonly IGlobalOptionService _globalOptions;

protected AbstractExtractInterfaceCommandHandler(IThreadingContext threadingContext, IGlobalOptionService globalOptions)
protected AbstractExtractInterfaceCommandHandler(IThreadingContext threadingContext)
{
_threadingContext = threadingContext;
_globalOptions = globalOptions;
}

public string DisplayName => EditorFeaturesResources.Extract_Interface;
Expand Down Expand Up @@ -71,7 +68,6 @@ public bool ExecuteCommand(ExtractInterfaceCommandArgs args, CommandExecutionCon
var result = await extractInterfaceService.ExtractInterfaceAsync(
document,
caretPoint.Value.Position,
_globalOptions.CreateProvider(),
(errorMessage, severity) => workspace.Services.GetService<INotificationService>().SendNotification(errorMessage, severity: severity),
CancellationToken.None).ConfigureAwait(true);

Expand Down
53 changes: 3 additions & 50 deletions src/EditorFeatures/Core/ExtractMethod/ExtractMethodCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,13 @@ private async Task ExecuteWorkerAsync(
if (document is null)
return;

var options = await document.GetExtractMethodGenerationOptionsAsync(_globalOptions, cancellationToken).ConfigureAwait(false);
var options = await document.GetExtractMethodGenerationOptionsAsync(cancellationToken).ConfigureAwait(false);
var result = await ExtractMethodService.ExtractMethodAsync(
document, span, localFunction: false, options, cancellationToken).ConfigureAwait(false);
Contract.ThrowIfNull(result);

result = await NotifyUserIfNecessaryAsync(
document, span, options, result, cancellationToken).ConfigureAwait(false);
document, result, cancellationToken).ConfigureAwait(false);
if (result is null)
return;

Expand Down Expand Up @@ -186,7 +186,7 @@ private void ApplyChange_OnUIThread(
}

private async Task<ExtractMethodResult?> NotifyUserIfNecessaryAsync(
Document document, TextSpan span, ExtractMethodGenerationOptions options, ExtractMethodResult result, CancellationToken cancellationToken)
Document document, ExtractMethodResult result, CancellationToken cancellationToken)
{
// If we succeeded without any problems, just proceed without notifying the user.
if (result is { Succeeded: true, Reasons.Length: 0 })
Expand All @@ -198,31 +198,9 @@ private void ApplyChange_OnUIThread(
if (notificationService is null)
return null;

// The initial extract method failed, or it succeeded with warning reasons. Attempt again with
// different options to see if we get better results.
var alternativeResult = await TryWithoutMakingValueTypesRefAsync(
document, span, result, options, cancellationToken).ConfigureAwait(false);

// We're about to show an notification to the user. Switch to the ui thread to do so.
await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);

if (alternativeResult is { Succeeded: true, Reasons.Length: 0 })
{
if (!notificationService.ConfirmMessageBox(
EditorFeaturesResources.Extract_method_encountered_the_following_issues + Environment.NewLine + Environment.NewLine +
string.Join(Environment.NewLine, result.Reasons) + Environment.NewLine + Environment.NewLine +
EditorFeaturesResources.We_can_fix_the_error_by_not_making_struct_out_ref_parameter_s_Do_you_want_to_proceed,
title: EditorFeaturesResources.Extract_Method,
severity: NotificationSeverity.Error))
{
// We handled the command, displayed a notification and did not produce code.
return null;
}

// Otherwise, prefer the new approach that has less problems.
return alternativeResult;
}

// The alternative approach wasn't better. If we failed, just let the user know and bail out. Otherwise,
// if we succeeded with messages, tell the user and let them decide if they want to proceed or not.
if (!result.Succeeded)
Expand Down Expand Up @@ -253,29 +231,4 @@ private void ApplyChange_OnUIThread(

return result;
}

private static async Task<ExtractMethodResult?> TryWithoutMakingValueTypesRefAsync(
Document document, TextSpan span, ExtractMethodResult result, ExtractMethodGenerationOptions options, CancellationToken cancellationToken)
{
if (options.ExtractOptions.DoNotPutOutOrRefOnStruct || !result.Reasons.IsSingle())
return null;

var reason = result.Reasons.FirstOrDefault();
var length = FeaturesResources.Asynchronous_method_cannot_have_ref_out_parameters_colon_bracket_0_bracket.IndexOf(':');
if (reason != null && length > 0 && reason.IndexOf(FeaturesResources.Asynchronous_method_cannot_have_ref_out_parameters_colon_bracket_0_bracket[..length], 0, length, StringComparison.Ordinal) >= 0)
{
var newResult = await ExtractMethodService.ExtractMethodAsync(
document,
span,
localFunction: false,
options with { ExtractOptions = options.ExtractOptions with { DoNotPutOutOrRefOnStruct = true } },
cancellationToken).ConfigureAwait(false);

// retry succeeded, return new result
if (newResult.Succeeded)
return newResult;
}

return null;
}
}
1 change: 0 additions & 1 deletion src/EditorFeatures/DiagnosticsTestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ public async Task TestMoveToNamespaceAsync(
var actions = await testState.MoveToNamespaceService.GetCodeActionsAsync(
testState.InvocationDocument,
testState.TestInvocationDocument.SelectedSpans.Single(),
CodeActionOptions.DefaultProvider,
CancellationToken.None);

var operationTasks = actions
Expand Down
1 change: 0 additions & 1 deletion src/EditorFeatures/Test/Options/GlobalOptionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ public void ReadingOptionsFromGlobalOptions(string language)
VerifyDataMembersHaveNonDefaultValues(globalOptions.GetAutoFormattingOptions(language), AutoFormattingOptions.Default, language);
VerifyDataMembersHaveNonDefaultValues(globalOptions.GetBlockStructureOptions(language, isMetadataAsSource: false), BlockStructureOptions.Default, language);
VerifyDataMembersHaveNonDefaultValues(globalOptions.GetDocumentationCommentOptions(globalOptions.GetLineFormattingOptions(language), language), DocumentationCommentOptions.Default, language);
VerifyDataMembersHaveNonDefaultValues(globalOptions.GetExtractMethodOptions(language), ExtractMethodOptions.Default, language);
VerifyDataMembersHaveNonDefaultValues(globalOptions.GetImplementTypeOptions(language), ImplementTypeOptions.Default, language);
VerifyDataMembersHaveNonDefaultValues(globalOptions.GetMetadataAsSourceOptions(), MetadataAsSourceOptions.Default, language);
VerifyDataMembersHaveNonDefaultValues(globalOptions.GetSignatureHelpOptions(language), SignatureHelpOptions.Default, language);
Expand Down
6 changes: 1 addition & 5 deletions src/EditorFeatures/TestUtilities/ExtractInterface/ExtractInterfaceTestState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ public Task<ExtractInterfaceTypeAnalysisResult> GetTypeAnalysisResultAsync(TypeD
ExtractFromDocument,
_testDocument.CursorPosition.Value,
typeDiscoveryRule,
Workspace.GlobalOptions.CreateProvider(),
CancellationToken.None);
}

Expand All @@ -86,7 +85,6 @@ public Task<ExtractInterfaceResult> ExtractViaCommandAsync()
return ExtractInterfaceService.ExtractInterfaceAsync(
ExtractFromDocument,
_testDocument.CursorPosition.Value,
Workspace.GlobalOptions.CreateProvider(),
(errorMessage, severity) =>
{
this.ErrorMessage = errorMessage;
Expand All @@ -100,7 +98,6 @@ public async Task<Solution> ExtractViaCodeAction()
var actions = await ExtractInterfaceService.GetExtractInterfaceCodeActionAsync(
ExtractFromDocument,
new TextSpan(_testDocument.CursorPosition.Value, 1),
Workspace.GlobalOptions.CreateProvider(),
CancellationToken.None);

var action = actions.Single();
Expand All @@ -111,8 +108,7 @@ public async Task<Solution> ExtractViaCodeAction()
options.IncludedMembers,
options.InterfaceName,
options.FileName,
ExtractInterfaceOptionsResult.ExtractLocation.SameFile,
options.FallbackOptions);
ExtractInterfaceOptionsResult.ExtractLocation.SameFile);

var operations = await action.GetOperationsAsync(
this.OriginalSolution, changedOptions, CodeAnalysisProgress.None, CancellationToken.None);
Expand Down
Loading
Loading