From c55b693551a6546a5a1fb5c20523af5fa66635f0 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 21 Nov 2023 17:35:58 -0800 Subject: [PATCH 1/4] Pass along ref information --- ...harpMethodExtractor.CSharpCodeGenerator.cs | 2 +- .../CSharpSelectionResult.ExpressionResult.cs | 80 ++++++++++--------- .../CSharpSelectionResult.StatementResult.cs | 36 ++++----- .../ExtractMethod/MethodExtractor.Analyzer.cs | 12 +-- .../MethodExtractor.AnalyzerResult.cs | 5 +- .../Portable/ExtractMethod/SelectionResult.cs | 9 ++- .../VisualBasicSelectionResult.vb | 8 +- 7 files changed, 86 insertions(+), 66 deletions(-) diff --git a/src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.cs b/src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.cs index ed5bad9574428..7b40998e7bfd5 100644 --- a/src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.cs +++ b/src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.cs @@ -99,7 +99,7 @@ protected override IMethodSymbol GenerateMethodDefinition( accessibility: Accessibility.Private, modifiers: CreateMethodModifiers(), returnType: AnalyzerResult.ReturnType, - refKind: RefKind.None, + refKind: AnalyzerResult.ReturnsByRef ? RefKind.Ref : RefKind.None, explicitInterfaceImplementations: default, name: _methodName.ToString(), typeParameters: CreateMethodTypeParameters(), diff --git a/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.ExpressionResult.cs b/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.ExpressionResult.cs index f76bb681b51ad..4d9805db95920 100644 --- a/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.ExpressionResult.cs +++ b/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.ExpressionResult.cs @@ -46,7 +46,7 @@ public override bool ContainingScopeHasAsyncKeyword() return CSharpSyntaxFacts.Instance.GetRootStandaloneExpression(scope); } - public override ITypeSymbol? GetContainingScopeType() + public override (ITypeSymbol? returnType, bool returnsByRef) GetReturnType() { if (GetContainingScope() is not ExpressionSyntax node) { @@ -60,9 +60,7 @@ public override bool ContainingScopeHasAsyncKeyword() { var variableDeclExpression = node.GetAncestorOrThis(); if (variableDeclExpression != null) - { - return model.GetTypeInfo(variableDeclExpression.Type).Type; - } + return (model.GetTypeInfo(variableDeclExpression.Type).Type, returnsByRef: false); } if (node.IsExpressionInCast()) @@ -72,57 +70,65 @@ public override bool ContainingScopeHasAsyncKeyword() // 1. if regular binding returns a meaningful type, we use it as it is // 2. if it doesn't, even if the cast itself wasn't included in the selection, we will treat it // as it was in the selection - var regularType = GetRegularExpressionType(model, node); + var (regularType, returnsByRef) = GetRegularExpressionType(model, node); if (regularType != null) - { - return regularType; - } + return (regularType, returnsByRef); if (node.Parent is CastExpressionSyntax castExpression) - { - return model.GetTypeInfo(castExpression).Type; - } + return (model.GetTypeInfo(castExpression).Type, returnsByRef: false); } return GetRegularExpressionType(model, node); } - private static ITypeSymbol? GetRegularExpressionType(SemanticModel semanticModel, ExpressionSyntax node) + private static (ITypeSymbol? typeSymbol, bool returnsByRef) GetRegularExpressionType(SemanticModel semanticModel, ExpressionSyntax node) { // regular case. always use ConvertedType to get implicit conversion right. var expression = node.GetUnparenthesizedExpression(); - - var info = semanticModel.GetTypeInfo(expression); - var conv = semanticModel.GetConversion(expression); - - if (info.ConvertedType == null || info.ConvertedType.IsErrorType()) + var returnsByRef = false; + if (expression is RefExpressionSyntax refExpression) { - // there is no implicit conversion involved. no need to go further - return info.GetTypeWithAnnotatedNullability(); + expression = refExpression.Expression; + returnsByRef = true; } - // always use converted type if method group - if ((!node.IsKind(SyntaxKind.ObjectCreationExpression) && semanticModel.GetMemberGroup(expression).Length > 0) || - IsCoClassImplicitConversion(info, conv, semanticModel.Compilation.CoClassType())) - { - return info.GetConvertedTypeWithAnnotatedNullability(); - } + var typeSymbol = GetRegularExpressionTypeWorker(); + return (typeSymbol, returnsByRef); - // check implicit conversion - if (conv.IsImplicit && (conv.IsConstantExpression || conv.IsEnumeration)) + ITypeSymbol? GetRegularExpressionTypeWorker() { - return info.GetConvertedTypeWithAnnotatedNullability(); - } + var info = semanticModel.GetTypeInfo(expression); + var conv = semanticModel.GetConversion(expression); - // use FormattableString if conversion between String and FormattableString - if (info.Type?.SpecialType == SpecialType.System_String && - info.ConvertedType?.IsFormattableStringOrIFormattable() == true) - { - return info.GetConvertedTypeWithAnnotatedNullability(); - } + if (info.ConvertedType == null || info.ConvertedType.IsErrorType()) + { + // there is no implicit conversion involved. no need to go further + return info.GetTypeWithAnnotatedNullability(); + } + + // always use converted type if method group + if ((!node.IsKind(SyntaxKind.ObjectCreationExpression) && semanticModel.GetMemberGroup(expression).Length > 0) || + IsCoClassImplicitConversion(info, conv, semanticModel.Compilation.CoClassType())) + { + return info.GetConvertedTypeWithAnnotatedNullability(); + } + + // check implicit conversion + if (conv.IsImplicit && (conv.IsConstantExpression || conv.IsEnumeration)) + { + return info.GetConvertedTypeWithAnnotatedNullability(); + } + + // use FormattableString if conversion between String and FormattableString + if (info.Type?.SpecialType == SpecialType.System_String && + info.ConvertedType?.IsFormattableStringOrIFormattable() == true) + { + return info.GetConvertedTypeWithAnnotatedNullability(); + } - // always try to use type that is more specific than object type if possible. - return !info.Type.IsObjectType() ? info.GetTypeWithAnnotatedNullability() : info.GetConvertedTypeWithAnnotatedNullability(); + // always try to use type that is more specific than object type if possible. + return !info.Type.IsObjectType() ? info.GetTypeWithAnnotatedNullability() : info.GetConvertedTypeWithAnnotatedNullability(); + } } } diff --git a/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.StatementResult.cs b/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.StatementResult.cs index c31a9ee93477c..37655df1f8c16 100644 --- a/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.StatementResult.cs +++ b/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.StatementResult.cs @@ -61,7 +61,7 @@ AnonymousMethodExpressionSyntax or }); } - public override ITypeSymbol GetContainingScopeType() + public override (ITypeSymbol returnType, bool returnsByRef) GetReturnType() { Contract.ThrowIfTrue(SelectionInExpression); @@ -73,31 +73,31 @@ public override ITypeSymbol GetContainingScopeType() case AccessorDeclarationSyntax access: // property or event case if (access.Parent == null || access.Parent.Parent == null) - { - return null; - } + return default; return semanticModel.GetDeclaredSymbol(access.Parent.Parent) switch { - IPropertySymbol propertySymbol => propertySymbol.Type, - IEventSymbol eventSymbol => eventSymbol.Type, - _ => null, + IPropertySymbol propertySymbol => (propertySymbol.Type, propertySymbol.ReturnsByRef), + IEventSymbol eventSymbol => (eventSymbol.Type, false), + _ => default, }; - case MethodDeclarationSyntax method: - return semanticModel.GetDeclaredSymbol(method).ReturnType; - - case ParenthesizedLambdaExpressionSyntax lambda: - return semanticModel.GetLambdaOrAnonymousMethodReturnType(lambda); - - case SimpleLambdaExpressionSyntax lambda: - return semanticModel.GetLambdaOrAnonymousMethodReturnType(lambda); + case MethodDeclarationSyntax methodDeclaration: + { + return semanticModel.GetDeclaredSymbol(methodDeclaration) is not IMethodSymbol method + ? default + : (method.ReturnType, method.ReturnsByRef); + } - case AnonymousMethodExpressionSyntax anonymous: - return semanticModel.GetLambdaOrAnonymousMethodReturnType(anonymous); + case AnonymousFunctionExpressionSyntax function: + { + return semanticModel.GetSymbolInfo(function).Symbol is not IMethodSymbol method + ? default + : (method.ReturnType, method.ReturnsByRef); + } default: - return null; + return default; } } } diff --git a/src/Features/Core/Portable/ExtractMethod/MethodExtractor.Analyzer.cs b/src/Features/Core/Portable/ExtractMethod/MethodExtractor.Analyzer.cs index 6e8843184052f..f5fbae7b80d95 100644 --- a/src/Features/Core/Portable/ExtractMethod/MethodExtractor.Analyzer.cs +++ b/src/Features/Core/Portable/ExtractMethod/MethodExtractor.Analyzer.cs @@ -177,7 +177,7 @@ public AnalyzerResult Analyze() // collects various variable informations // extracted code contains return value var isInExpressionOrHasReturnStatement = IsInExpressionOrHasReturnStatement(model); - var (parameters, returnType, variableToUseAsReturnValue, unsafeAddressTakenUsed) = + var (parameters, returnType, returnsByRef, variableToUseAsReturnValue, unsafeAddressTakenUsed) = GetSignatureInformation(dataFlowAnalysisData, variableInfoMap, isInExpressionOrHasReturnStatement); var returnTypeTuple = AdjustReturnType(model, returnType); @@ -201,6 +201,7 @@ public AnalyzerResult Analyze() parameters, variableToUseAsReturnValue, returnType, + returnsByRef, awaitTaskReturn, instanceMemberIsUsed, shouldBeReadOnly, @@ -289,7 +290,7 @@ private void WrapReturnTypeInTask(SemanticModel model, ref ITypeSymbol returnTyp } } - private (ImmutableArray parameters, ITypeSymbol returnType, VariableInfo? variableToUseAsReturnValue, bool unsafeAddressTakenUsed) + private (ImmutableArray parameters, ITypeSymbol returnType, bool returnsByRef, VariableInfo? variableToUseAsReturnValue, bool unsafeAddressTakenUsed) GetSignatureInformation( DataFlowAnalysis dataFlowAnalysisData, Dictionary variableInfoMap, @@ -301,10 +302,11 @@ private void WrapReturnTypeInTask(SemanticModel model, ref ITypeSymbol returnTyp { // check whether current selection contains return statement var parameters = GetMethodParameters(variableInfoMap); - var returnType = SelectionResult.GetContainingScopeType() ?? compilation.GetSpecialType(SpecialType.System_Object); + var (returnType, returnsByRef) = SelectionResult.GetReturnType(); + returnType ??= compilation.GetSpecialType(SpecialType.System_Object); var unsafeAddressTakenUsed = ContainsVariableUnsafeAddressTaken(dataFlowAnalysisData, variableInfoMap.Keys); - return (parameters, returnType, null, unsafeAddressTakenUsed); + return (parameters, returnType, returnsByRef, null, unsafeAddressTakenUsed); } else { @@ -316,7 +318,7 @@ private void WrapReturnTypeInTask(SemanticModel model, ref ITypeSymbol returnTyp : compilation.GetSpecialType(SpecialType.System_Void); var unsafeAddressTakenUsed = ContainsVariableUnsafeAddressTaken(dataFlowAnalysisData, variableInfoMap.Keys); - return (parameters, returnType, variableToUseAsReturnValue, unsafeAddressTakenUsed); + return (parameters, returnType, returnsByRef: false, variableToUseAsReturnValue, unsafeAddressTakenUsed); } } diff --git a/src/Features/Core/Portable/ExtractMethod/MethodExtractor.AnalyzerResult.cs b/src/Features/Core/Portable/ExtractMethod/MethodExtractor.AnalyzerResult.cs index 61731539c93f7..052a464ed954d 100644 --- a/src/Features/Core/Portable/ExtractMethod/MethodExtractor.AnalyzerResult.cs +++ b/src/Features/Core/Portable/ExtractMethod/MethodExtractor.AnalyzerResult.cs @@ -22,6 +22,7 @@ protected sealed class AnalyzerResult( ImmutableArray variables, VariableInfo variableToUseAsReturnValue, ITypeSymbol returnType, + bool returnsByRef, bool awaitTaskReturn, bool instanceMemberIsUsed, bool shouldBeReadOnly, @@ -53,10 +54,8 @@ protected sealed class AnalyzerResult( /// public bool AwaitTaskReturn { get; } = awaitTaskReturn; - /// - /// return type - /// public ITypeSymbol ReturnType { get; } = returnType; + public bool ReturnsByRef { get; } = returnsByRef; /// /// analyzer result operation status diff --git a/src/Features/Core/Portable/ExtractMethod/SelectionResult.cs b/src/Features/Core/Portable/ExtractMethod/SelectionResult.cs index 5034ae28b96a8..deaf51d619548 100644 --- a/src/Features/Core/Portable/ExtractMethod/SelectionResult.cs +++ b/src/Features/Core/Portable/ExtractMethod/SelectionResult.cs @@ -53,9 +53,16 @@ protected SelectionResult( public abstract bool ContainingScopeHasAsyncKeyword(); public abstract SyntaxNode GetContainingScope(); - public abstract ITypeSymbol GetContainingScopeType(); public abstract SyntaxNode GetOutermostCallSiteContainerToProcess(CancellationToken cancellationToken); + public abstract (ITypeSymbol returnType, bool returnsByRef) GetReturnType(); + + public ITypeSymbol GetContainingScopeType() + { + var (typeSymbol, _) = GetReturnType(); + return typeSymbol; + } + public TextSpan OriginalSpan { get; } public TextSpan FinalSpan { get; } public ExtractMethodOptions Options { get; } diff --git a/src/Features/VisualBasic/Portable/ExtractMethod/VisualBasicSelectionResult.vb b/src/Features/VisualBasic/Portable/ExtractMethod/VisualBasicSelectionResult.vb index b636580e656c2..8324b7c4b49b4 100644 --- a/src/Features/VisualBasic/Portable/ExtractMethod/VisualBasicSelectionResult.vb +++ b/src/Features/VisualBasic/Portable/ExtractMethod/VisualBasicSelectionResult.vb @@ -153,7 +153,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExtractMethod End If End Function - Public Overrides Function GetContainingScopeType() As ITypeSymbol + Public Overrides Function GetReturnType() As (returnType As ITypeSymbol, returnsByRef As Boolean) + ' Todo: consider supporting byref return types in VB + Dim returnType = GetReturnTypeWorker() + Return (returnType, returnsByRef:=False) + End Function + + Private Function GetReturnTypeWorker() As ITypeSymbol Dim node = Me.GetContainingScope() Dim semanticModel = Me.SemanticDocument.SemanticModel From 46af1db08ba28f07430666fbcfea76e25d70f9fd Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 21 Nov 2023 17:44:45 -0800 Subject: [PATCH 2/4] By ref --- ...harpMethodExtractor.CSharpCodeGenerator.cs | 55 ++++++++++--------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.cs b/src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.cs index 7b40998e7bfd5..2473198066568 100644 --- a/src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.cs +++ b/src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.cs @@ -29,6 +29,8 @@ namespace Microsoft.CodeAnalysis.CSharp.ExtractMethod { + using static SyntaxFactory; + internal partial class CSharpMethodExtractor { private abstract partial class CSharpCodeGenerator : CodeGenerator @@ -571,47 +573,48 @@ protected override StatementSyntax CreateReturnStatement(string identifierName = protected override ExpressionSyntax CreateCallSignature() { var methodName = CreateMethodNameForInvocation().WithAdditionalAnnotations(Simplifier.Annotation); - var arguments = new List(); var isLocalFunction = LocalFunction && ShouldLocalFunctionCaptureParameter(SemanticDocument.Root); + using var _ = ArrayBuilder.GetInstance(out var arguments); + foreach (var argument in AnalyzerResult.MethodParameters) { if (!isLocalFunction || !argument.CanBeCapturedByLocalFunction) { var modifier = GetParameterRefSyntaxKind(argument.ParameterModifier); - var refOrOut = modifier == SyntaxKind.None ? default : SyntaxFactory.Token(modifier); - arguments.Add(SyntaxFactory.Argument(SyntaxFactory.IdentifierName(argument.Name)).WithRefOrOutKeyword(refOrOut)); + var refOrOut = modifier == SyntaxKind.None ? default : Token(modifier); + arguments.Add(Argument(IdentifierName(argument.Name)).WithRefOrOutKeyword(refOrOut)); } } - var invocation = SyntaxFactory.InvocationExpression(methodName, - SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(arguments))); - - var shouldPutAsyncModifier = this.SelectionResult.ShouldPutAsyncModifier(); - if (!shouldPutAsyncModifier) - { - return invocation; - } - - if (this.SelectionResult.ShouldCallConfigureAwaitFalse()) + var invocation = (ExpressionSyntax)InvocationExpression(methodName, ArgumentList(SeparatedList(arguments))); + if (this.SelectionResult.ShouldPutAsyncModifier()) { - if (AnalyzerResult.ReturnType.GetMembers().Any(static x => x is IMethodSymbol - { - Name: nameof(Task.ConfigureAwait), - Parameters: { Length: 1 } parameters - } && parameters[0].Type.SpecialType == SpecialType.System_Boolean)) + if (this.SelectionResult.ShouldCallConfigureAwaitFalse()) { - invocation = SyntaxFactory.InvocationExpression( - SyntaxFactory.MemberAccessExpression( - SyntaxKind.SimpleMemberAccessExpression, - invocation, - SyntaxFactory.IdentifierName(nameof(Task.ConfigureAwait))), - SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList( - SyntaxFactory.Argument(SyntaxFactory.LiteralExpression(SyntaxKind.FalseLiteralExpression))))); + if (AnalyzerResult.ReturnType.GetMembers().Any(static x => x is IMethodSymbol + { + Name: nameof(Task.ConfigureAwait), + Parameters: [{ Type.SpecialType: SpecialType.System_Boolean }], + })) + { + invocation = InvocationExpression( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + invocation, + IdentifierName(nameof(Task.ConfigureAwait))), + ArgumentList(SingletonSeparatedList( + Argument(LiteralExpression(SyntaxKind.FalseLiteralExpression))))); + } } + + invocation = AwaitExpression(invocation); } - return SyntaxFactory.AwaitExpression(invocation); + if (AnalyzerResult.ReturnsByRef) + invocation = RefExpression(invocation); + + return invocation; } protected override StatementSyntax CreateAssignmentExpressionStatement(SyntaxToken identifier, ExpressionSyntax rvalue) From fc1753cf7f53021e0f7e3aeafc70e34239fac402 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 21 Nov 2023 17:53:20 -0800 Subject: [PATCH 3/4] in progress --- .../Portable/ExtractMethod/CSharpSelectionResult.cs | 13 ++++++++++++- .../ExtractMethod/MethodExtractor.Analyzer.cs | 4 +--- .../Core/Portable/ExtractMethod/SelectionResult.cs | 2 ++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.cs b/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.cs index 52ce2b1f6e3c2..cdc5754b65e56 100644 --- a/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.cs +++ b/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.cs @@ -75,7 +75,18 @@ protected CSharpSelectionResult( { } - protected override ISyntaxFacts SyntaxFacts => CSharpSyntaxFacts.Instance; + protected override ISyntaxFacts SyntaxFacts + => CSharpSyntaxFacts.Instance; + + public override SyntaxNode GetNodeForDataFlowAnalysis() + { + var node = base.GetNodeForDataFlowAnalysis(); + + // If we're returning a value by ref we actually want to do the analysis on the underlying expression. + return node is RefExpressionSyntax refExpression + ? refExpression.Expression + : node; + } protected override bool UnderAnonymousOrLocalMethod(SyntaxToken token, SyntaxToken firstToken, SyntaxToken lastToken) { diff --git a/src/Features/Core/Portable/ExtractMethod/MethodExtractor.Analyzer.cs b/src/Features/Core/Portable/ExtractMethod/MethodExtractor.Analyzer.cs index f5fbae7b80d95..00a517cf978a8 100644 --- a/src/Features/Core/Portable/ExtractMethod/MethodExtractor.Analyzer.cs +++ b/src/Features/Core/Portable/ExtractMethod/MethodExtractor.Analyzer.cs @@ -411,9 +411,7 @@ private static bool ContainsVariableUnsafeAddressTaken(DataFlowAnalysis dataFlow private DataFlowAnalysis GetDataFlowAnalysisData(SemanticModel model) { if (SelectionResult.SelectionInExpression) - { - return model.AnalyzeDataFlow(SelectionResult.GetContainingScope()); - } + return model.AnalyzeDataFlow(SelectionResult.GetNodeForDataFlowAnalysis()); var pair = GetFlowAnalysisNodeRange(); return model.AnalyzeDataFlow(pair.Item1, pair.Item2); diff --git a/src/Features/Core/Portable/ExtractMethod/SelectionResult.cs b/src/Features/Core/Portable/ExtractMethod/SelectionResult.cs index deaf51d619548..3e74f9d9db88b 100644 --- a/src/Features/Core/Portable/ExtractMethod/SelectionResult.cs +++ b/src/Features/Core/Portable/ExtractMethod/SelectionResult.cs @@ -63,6 +63,8 @@ public ITypeSymbol GetContainingScopeType() return typeSymbol; } + public virtual SyntaxNode GetNodeForDataFlowAnalysis() => GetContainingScope(); + public TextSpan OriginalSpan { get; } public TextSpan FinalSpan { get; } public ExtractMethodOptions Options { get; } From 4cbb9da854761fc2df1a5f15f9f999e18e1a33a1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 21 Nov 2023 17:59:31 -0800 Subject: [PATCH 4/4] Add test --- .../ExtractMethod/CSharpSelectionResult.cs | 29 ++-- .../ExtractMethod/ExtractMethodTests.cs | 133 +++++++++++------- 2 files changed, 93 insertions(+), 69 deletions(-) diff --git a/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.cs b/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.cs index cdc5754b65e56..acd3b853d7c25 100644 --- a/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.cs +++ b/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.cs @@ -90,27 +90,24 @@ public override SyntaxNode GetNodeForDataFlowAnalysis() protected override bool UnderAnonymousOrLocalMethod(SyntaxToken token, SyntaxToken firstToken, SyntaxToken lastToken) { - var current = token.Parent; - for (; current != null; current = current.Parent) + for (var current = token.Parent; current != null; current = current.Parent) { - if (current is MemberDeclarationSyntax or - SimpleLambdaExpressionSyntax or - ParenthesizedLambdaExpressionSyntax or - AnonymousMethodExpressionSyntax or - LocalFunctionStatementSyntax) + if (current is MemberDeclarationSyntax) + return false; + + if (current is + SimpleLambdaExpressionSyntax or + ParenthesizedLambdaExpressionSyntax or + AnonymousMethodExpressionSyntax or + LocalFunctionStatementSyntax) { - break; + // make sure the selection contains the lambda + return firstToken.SpanStart <= current.GetFirstToken().SpanStart && + current.GetLastToken().Span.End <= lastToken.Span.End; } } - if (current is null or MemberDeclarationSyntax) - { - return false; - } - - // make sure the selection contains the lambda - return firstToken.SpanStart <= current.GetFirstToken().SpanStart && - current.GetLastToken().Span.End <= lastToken.Span.End; + return false; } public override SyntaxNode GetOutermostCallSiteContainerToProcess(CancellationToken cancellationToken) diff --git a/src/Features/CSharpTest/ExtractMethod/ExtractMethodTests.cs b/src/Features/CSharpTest/ExtractMethod/ExtractMethodTests.cs index d2958f2e4e05d..7a6440367118b 100644 --- a/src/Features/CSharpTest/ExtractMethod/ExtractMethodTests.cs +++ b/src/Features/CSharpTest/ExtractMethod/ExtractMethodTests.cs @@ -5180,82 +5180,109 @@ private static object NewMethod() [Fact] public Task ExtractMethod_InsideBaseInitializer() - => TestInRegularAndScript1Async( - """ - class Base - { - private readonly int _x; - public Base(int x) + => TestInRegularAndScript1Async( + """ + class Base { - _x = x; + private readonly int _x; + public Base(int x) + { + _x = x; + } } - } - class C : Base - { - public C(int y) - : base([|y + 1|]) + class C : Base { + public C(int y) + : base([|y + 1|]) + { + } } - } - """, - """ - class Base - { - private readonly int _x; - public Base(int x) + """, + """ + class Base { - _x = x; + private readonly int _x; + public Base(int x) + { + _x = x; + } } - } - class C : Base - { - public C(int y) - : base({|Rename:NewMethod|}(y)) + class C : Base { - } + public C(int y) + : base({|Rename:NewMethod|}(y)) + { + } - private static int NewMethod(int y) - { - return y + 1; + private static int NewMethod(int y) + { + return y + 1; + } } - } - """); + """); [Fact] public Task ExtractMethod_InsideThisInitializer() - => TestInRegularAndScript1Async( - """ - class C - { - public C(int y) - : this(y, [|y + 1|]) + => TestInRegularAndScript1Async( + """ + class C { - } + public C(int y) + : this(y, [|y + 1|]) + { + } - public C(int x, int y) - { + public C(int x, int y) + { + } } - } - """, - """ - class C - { - public C(int y) - : this(y, {|Rename:NewMethod|}(y)) + """, + """ + class C { + public C(int y) + : this(y, {|Rename:NewMethod|}(y)) + { + } + + private static int NewMethod(int y) + { + return y + 1; + } + + public C(int x, int y) + { + } } + """); - private static int NewMethod(int y) + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/8439")] + public Task TestRefReturn1() + => TestInRegularAndScript1Async( + """ + class Program { - return y + 1; + public ref int M() + { + return [|ref M()|]; + } } - - public C(int x, int y) + """, + """ + class Program { + public ref int M() + { + return ref {|Rename:NewMethod|}(); + } + + private ref int NewMethod() + { + return ref M(); + } } - } - """); + """); } }