提交 473f2a42 编写于 作者: A Allison Chou

Addressed some feedback but still need to address some comments

上级 b43b35fe
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeRefactorings.ExtractMethod namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeRefactorings.ExtractMethod
{ {
public class ExtractLocalMethodTests : AbstractCSharpCodeActionTest public class ExtractLocalFunctionTests : AbstractCSharpCodeActionTest
{ {
protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspace workspace, TestParameters parameters) protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspace workspace, TestParameters parameters)
=> new CSharpExtractLocalMethodCodeRefactoringProvider(); => new CSharpExtractLocalMethodCodeRefactoringProvider();
...@@ -70,6 +70,33 @@ bool NewMethod(bool b) ...@@ -70,6 +70,33 @@ bool NewMethod(bool b)
}", options: Option(CSharpCodeStyleOptions.PreferStaticLocalFunction, CodeStyleOptions.FalseWithSilentEnforcement)); }", options: Option(CSharpCodeStyleOptions.PreferStaticLocalFunction, CodeStyleOptions.FalseWithSilentEnforcement));
} }
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractLocalMethod)]
public async Task TestPartialSelection_StaticOptionDefault()
{
await TestInRegularAndScriptAsync(
@"class Program
{
static void Main(string[] args)
{
bool b = true;
System.Console.WriteLine([|b != true|] ? b = true : b = false);
}
}",
@"class Program
{
static void Main(string[] args)
{
bool b = true;
System.Console.WriteLine({|Rename:NewMethod|}(b) ? b = true : b = false);
static bool NewMethod(bool b)
{
return b != true;
}
}
}", options: Option(CSharpCodeStyleOptions.PreferStaticLocalFunction, CSharpCodeStyleOptions.PreferStaticLocalFunction.DefaultValue));
}
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractLocalMethod)] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractLocalMethod)]
public async Task TestUseExpressionBodyWhenPossible() public async Task TestUseExpressionBodyWhenPossible()
{ {
...@@ -341,7 +368,7 @@ static int NewMethod(int x) ...@@ -341,7 +368,7 @@ static int NewMethod(int x)
} }
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractLocalMethod)] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractLocalMethod)]
public async Task TestMissingOnNamespace() public async Task TestOnNamespace()
{ {
await TestInRegularAndScriptAsync( await TestInRegularAndScriptAsync(
@"class Program @"class Program
...@@ -366,7 +393,7 @@ static void NewMethod() ...@@ -366,7 +393,7 @@ static void NewMethod()
} }
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractLocalMethod)] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractLocalMethod)]
public async Task TestMissingOnType() public async Task TestOnType()
{ {
await TestInRegularAndScriptAsync( await TestInRegularAndScriptAsync(
@"class Program @"class Program
...@@ -391,7 +418,7 @@ static void NewMethod() ...@@ -391,7 +418,7 @@ static void NewMethod()
} }
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractLocalMethod)] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractLocalMethod)]
public async Task TestMissingOnBase() public async Task TestOnBase()
{ {
await TestInRegularAndScriptAsync( await TestInRegularAndScriptAsync(
@"class Program @"class Program
......
...@@ -137,7 +137,7 @@ protected async Task NotSupported_ExtractMethodAsync(string codeWithMarker) ...@@ -137,7 +137,7 @@ protected async Task NotSupported_ExtractMethodAsync(string codeWithMarker)
Assert.True(selectedCode.ContainsValidContext); Assert.True(selectedCode.ContainsValidContext);
// extract method // extract method
var extractor = new CSharpMethodExtractor((CSharpSelectionResult)selectedCode); var extractor = new CSharpMethodExtractor(result: (CSharpSelectionResult)selectedCode, extractLocalFunction: false);
var result = await extractor.ExtractMethodAsync(CancellationToken.None); var result = await extractor.ExtractMethodAsync(CancellationToken.None);
Assert.NotNull(result); Assert.NotNull(result);
Assert.Equal(succeed, Assert.Equal(succeed,
......
...@@ -10329,7 +10329,7 @@ public async Task ExtractMethod_Argument1() ...@@ -10329,7 +10329,7 @@ public async Task ExtractMethod_Argument1()
var service = new CSharpExtractMethodService(); var service = new CSharpExtractMethodService();
Assert.NotNull(await Record.ExceptionAsync(async () => Assert.NotNull(await Record.ExceptionAsync(async () =>
{ {
var tree = await service.ExtractMethodAsync(null, default, false, null, CancellationToken.None); var tree = await service.ExtractMethodAsync(document: null, textSpan: default, extractLocalFunction: false, options: null, CancellationToken.None);
})); }));
} }
......
...@@ -220,7 +220,7 @@ private bool TryNotifyFailureToUser(Document document, ExtractMethodResult resul ...@@ -220,7 +220,7 @@ private bool TryNotifyFailureToUser(Document document, ExtractMethodResult resul
{ {
options = options.WithChangedOption(ExtractMethodOptions.DontPutOutOrRefOnStruct, document.Project.Language, true); options = options.WithChangedOption(ExtractMethodOptions.DontPutOutOrRefOnStruct, document.Project.Language, true);
var newResult = ExtractMethodService.ExtractMethodAsync( var newResult = ExtractMethodService.ExtractMethodAsync(
document: document, textSpan: spans.Single().Span.ToTextSpan(), options: options, cancellationToken: cancellationToken).WaitAndGetResult(cancellationToken); document: document, textSpan: spans.Single().Span.ToTextSpan(), extractLocalMethod: false, options: options, cancellationToken).WaitAndGetResult(cancellationToken);
// retry succeeded, return new result // retry succeeded, return new result
if (newResult.Succeeded || newResult.SucceededWithSuggestion) if (newResult.Succeeded || newResult.SucceededWithSuggestion)
......
...@@ -30,7 +30,6 @@ public CSharpExtractLocalMethodCodeRefactoringProvider() ...@@ -30,7 +30,6 @@ public CSharpExtractLocalMethodCodeRefactoringProvider()
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
var documentOptions = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false); var documentOptions = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
var preferStatic = documentOptions.GetOption(CSharpCodeStyleOptions.PreferStaticLocalFunction).Value;
var result = await ExtractMethodService.ExtractMethodAsync( var result = await ExtractMethodService.ExtractMethodAsync(
document, document,
......
...@@ -702,7 +702,8 @@ protected StatementSyntax GetStatementContainingInvocationToExtractedMethodWorke ...@@ -702,7 +702,8 @@ protected StatementSyntax GetStatementContainingInvocationToExtractedMethodWorke
} }
var syntaxNode = originalDocument.Root.GetAnnotatedNodesAndTokens(MethodDefinitionAnnotation).FirstOrDefault().AsNode(); var syntaxNode = originalDocument.Root.GetAnnotatedNodesAndTokens(MethodDefinitionAnnotation).FirstOrDefault().AsNode();
if (syntaxNode == null || !(syntaxNode is MethodDeclarationSyntax || syntaxNode is LocalFunctionStatementSyntax)) var nodeIsMethodOrLocalFunction = syntaxNode is MethodDeclarationSyntax || syntaxNode is LocalFunctionStatementSyntax;
if (syntaxNode == null || !nodeIsMethodOrLocalFunction)
{ {
return await base.UpdateMethodAfterGenerationAsync(originalDocument, methodSymbolResult, cancellationToken).ConfigureAwait(false); return await base.UpdateMethodAfterGenerationAsync(originalDocument, methodSymbolResult, cancellationToken).ConfigureAwait(false);
} }
...@@ -712,7 +713,7 @@ protected StatementSyntax GetStatementContainingInvocationToExtractedMethodWorke ...@@ -712,7 +713,7 @@ protected StatementSyntax GetStatementContainingInvocationToExtractedMethodWorke
if (syntaxNode is MethodDeclarationSyntax methodDeclaration) if (syntaxNode is MethodDeclarationSyntax methodDeclaration)
{ {
var nullableReturnOperations = await CheckReturnOperations(methodDeclaration).ConfigureAwait(false); var nullableReturnOperations = await CheckReturnOperations(methodDeclaration).ConfigureAwait(false);
if (nullableReturnOperations != null) if (nullableReturnOperations is object)
{ {
return nullableReturnOperations; return nullableReturnOperations;
} }
......
...@@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.CSharp.ExtractMethod ...@@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.CSharp.ExtractMethod
{ {
internal partial class CSharpMethodExtractor : MethodExtractor internal partial class CSharpMethodExtractor : MethodExtractor
{ {
public CSharpMethodExtractor(CSharpSelectionResult result, bool extractLocalFunction = false) public CSharpMethodExtractor(CSharpSelectionResult result, bool extractLocalFunction)
: base(result, extractLocalFunction) : base(result, extractLocalFunction)
{ {
} }
...@@ -33,10 +33,13 @@ protected override async Task<InsertionPoint> GetInsertionPointAsync(SemanticDoc ...@@ -33,10 +33,13 @@ protected override async Task<InsertionPoint> GetInsertionPointAsync(SemanticDoc
var basePosition = root.FindToken(position); var basePosition = root.FindToken(position);
// Check if we are extracting a local function and are within a local function // Check if we are extracting a local function and are within a local function
var localMethodNode = basePosition.GetAncestor<LocalFunctionStatementSyntax>(); if (ExtractLocalFunction)
if (ExtractLocalFunction && localMethodNode != null)
{ {
return await InsertionPoint.CreateAsync(document, localMethodNode, cancellationToken).ConfigureAwait(false); var localMethodNode = basePosition.GetAncestor<LocalFunctionStatementSyntax>();
if (localMethodNode is object)
{
return await InsertionPoint.CreateAsync(document, localMethodNode, cancellationToken).ConfigureAwait(false);
}
} }
var memberNode = basePosition.GetAncestor<MemberDeclarationSyntax>(); var memberNode = basePosition.GetAncestor<MemberDeclarationSyntax>();
......
...@@ -84,24 +84,13 @@ public async Task<GeneratedCode> GenerateAsync(CancellationToken cancellationTok ...@@ -84,24 +84,13 @@ public async Task<GeneratedCode> GenerateAsync(CancellationToken cancellationTok
var newCallSiteRoot = callSiteDocument.Root; var newCallSiteRoot = callSiteDocument.Root;
var previousMemberNode = GetPreviousMember(callSiteDocument); var previousMemberNode = GetPreviousMember(callSiteDocument);
SyntaxNode destination;
if (ExtractLocalFunction)
{
destination = InsertionPoint.With(callSiteDocument).GetContext();
}
else
{
// it is possible in a script file case where there is no previous member. in that case, insert new text into top level script
destination = previousMemberNode.Parent ?? previousMemberNode;
}
var codeGenerationService = SemanticDocument.Document.GetLanguageService<ICodeGenerationService>(); var codeGenerationService = SemanticDocument.Document.GetLanguageService<ICodeGenerationService>();
var result = GenerateMethodDefinition(cancellationToken); var result = GenerateMethodDefinition(cancellationToken);
SyntaxNode newContainer; SyntaxNode destination, newContainer;
if (ExtractLocalFunction) if (ExtractLocalFunction)
{ {
destination = InsertionPoint.With(callSiteDocument).GetContext();
newContainer = codeGenerationService.AddMethod( newContainer = codeGenerationService.AddMethod(
destination, result.Data, destination, result.Data,
new CodeGenerationOptions(generateDefaultAccessibility: false, generateMethodBodies: true), new CodeGenerationOptions(generateDefaultAccessibility: false, generateMethodBodies: true),
...@@ -109,6 +98,8 @@ public async Task<GeneratedCode> GenerateAsync(CancellationToken cancellationTok ...@@ -109,6 +98,8 @@ public async Task<GeneratedCode> GenerateAsync(CancellationToken cancellationTok
} }
else else
{ {
// it is possible in a script file case where there is no previous member. in that case, insert new text into top level script
destination = previousMemberNode.Parent ?? previousMemberNode;
newContainer = codeGenerationService.AddMethod( newContainer = codeGenerationService.AddMethod(
destination, result.Data, destination, result.Data,
new CodeGenerationOptions(afterThisLocation: previousMemberNode.GetLocation(), generateDefaultAccessibility: true, generateMethodBodies: true), new CodeGenerationOptions(afterThisLocation: previousMemberNode.GetLocation(), generateDefaultAccessibility: true, generateMethodBodies: true),
......
...@@ -16,7 +16,7 @@ internal abstract partial class MethodExtractor ...@@ -16,7 +16,7 @@ internal abstract partial class MethodExtractor
protected readonly SelectionResult OriginalSelectionResult; protected readonly SelectionResult OriginalSelectionResult;
protected readonly bool ExtractLocalFunction; protected readonly bool ExtractLocalFunction;
public MethodExtractor(SelectionResult selectionResult, bool extractLocalFunction = false) public MethodExtractor(SelectionResult selectionResult, bool extractLocalFunction)
{ {
Contract.ThrowIfNull(selectionResult); Contract.ThrowIfNull(selectionResult);
OriginalSelectionResult = selectionResult; OriginalSelectionResult = selectionResult;
......
...@@ -15,7 +15,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExtractMethod ...@@ -15,7 +15,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExtractMethod
Inherits MethodExtractor Inherits MethodExtractor
Public Sub New(result As VisualBasicSelectionResult) Public Sub New(result As VisualBasicSelectionResult)
MyBase.New(result) MyBase.New(result, extractLocalFunction:=False)
End Sub End Sub
Protected Overrides Function AnalyzeAsync(selectionResult As SelectionResult, extractLocalFunction As Boolean, cancellationToken As CancellationToken) As Task(Of AnalyzerResult) Protected Overrides Function AnalyzeAsync(selectionResult As SelectionResult, extractLocalFunction As Boolean, cancellationToken As CancellationToken) As Task(Of AnalyzerResult)
......
...@@ -165,14 +165,12 @@ internal static class MethodGenerator ...@@ -165,14 +165,12 @@ internal static class MethodGenerator
// `method` abstract. This would provide more flexibility. // `method` abstract. This would provide more flexibility.
var hasNoBody = !options.GenerateMethodBodies || method.IsAbstract; var hasNoBody = !options.GenerateMethodBodies || method.IsAbstract;
var explicitInterfaceSpecifier = GenerateExplicitInterfaceSpecifier(method.ExplicitInterfaceImplementations);
var localMethodDeclaration = SyntaxFactory.LocalFunctionStatement( var localMethodDeclaration = SyntaxFactory.LocalFunctionStatement(
modifiers: GenerateModifiers(method, destination, workspace, options), modifiers: GenerateModifiers(method, destination, workspace, options),
returnType: method.GenerateReturnTypeSyntax(), returnType: method.GenerateReturnTypeSyntax(),
identifier: method.Name.ToIdentifierToken(), identifier: method.Name.ToIdentifierToken(),
typeParameterList: GenerateTypeParameterList(method, options), typeParameterList: GenerateTypeParameterList(method, options),
parameterList: ParameterGenerator.GenerateParameterList(method.Parameters, explicitInterfaceSpecifier != null, options), parameterList: ParameterGenerator.GenerateParameterList(method.Parameters, isExplicit: false, options),
constraintClauses: GenerateConstraintClauses(method), constraintClauses: GenerateConstraintClauses(method),
body: hasNoBody ? null : StatementGenerator.GenerateBlock(method), body: hasNoBody ? null : StatementGenerator.GenerateBlock(method),
expressionBody: default, expressionBody: default,
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册