diff --git a/src/Analyzers/CSharp/CodeFixes/GenerateParameterizedMember/CSharpGenerateMethodService.cs b/src/Analyzers/CSharp/CodeFixes/GenerateParameterizedMember/CSharpGenerateMethodService.cs index f6986582d93d0..2c92ece627cf0 100644 --- a/src/Analyzers/CSharp/CodeFixes/GenerateParameterizedMember/CSharpGenerateMethodService.cs +++ b/src/Analyzers/CSharp/CodeFixes/GenerateParameterizedMember/CSharpGenerateMethodService.cs @@ -87,15 +87,18 @@ protected override bool TryInitializeSimpleNameState( identifierToken = simpleName.Identifier; var memberAccess = simpleName?.Parent as MemberAccessExpressionSyntax; - var conditionalMemberAccess = simpleName?.Parent?.Parent?.Parent as ConditionalAccessExpressionSyntax; - var inConditionalMemberAccess = conditionalMemberAccess != null; + var (conditionalAccessExpression, invocation) = + simpleName is { Parent: MemberBindingExpressionSyntax { Parent: InvocationExpressionSyntax { Parent: ConditionalAccessExpressionSyntax conditionalAccessExpression1 } invocation1 } memberBinding } && + conditionalAccessExpression1.WhenNotNull == invocation1 && + invocation1.Expression == memberBinding && + memberBinding.Name == simpleName ? (conditionalAccessExpression1, invocation1) : default; if (memberAccess != null) { simpleNameOrMemberAccessExpression = memberAccess; } - else if (inConditionalMemberAccess) + else if (conditionalAccessExpression != null) { - simpleNameOrMemberAccessExpression = conditionalMemberAccess; + simpleNameOrMemberAccessExpression = conditionalAccessExpression; } else { @@ -106,37 +109,29 @@ protected override bool TryInitializeSimpleNameState( { if (simpleNameOrMemberAccessExpression.IsParentKind(SyntaxKind.InvocationExpression, out invocationExpressionOpt)) { - isInConditionalAccessExpression = inConditionalMemberAccess; + // want to look for anything of the form: a?.B() a?.B.C() a?.B.C.D() etc + isInConditionalAccessExpression = invocationExpressionOpt.Parent is ConditionalAccessExpressionSyntax { WhenNotNull: var whenNotNull } && + whenNotNull == invocationExpressionOpt; return !invocationExpressionOpt.ArgumentList.CloseParenToken.IsMissing; } - // We need to check that the tree is structured like so: - // ConditionalAccessExpressionSyntax - // -> InvocationExpressionSyntax - // -> MemberBindingExpressionSyntax - // and that the name at the end of this expression matches the simple name we were given - else if ((((simpleNameOrMemberAccessExpression as ConditionalAccessExpressionSyntax) - ?.WhenNotNull as InvocationExpressionSyntax) - ?.Expression as MemberBindingExpressionSyntax) - ?.Name == simpleName) + + if (conditionalAccessExpression != null) { - invocationExpressionOpt = (InvocationExpressionSyntax)((ConditionalAccessExpressionSyntax)simpleNameOrMemberAccessExpression).WhenNotNull; - isInConditionalAccessExpression = inConditionalMemberAccess; + invocationExpressionOpt = invocation; + isInConditionalAccessExpression = true; return !invocationExpressionOpt.ArgumentList.CloseParenToken.IsMissing; } - else if (simpleName.IsKind(SyntaxKind.IdentifierName)) + + // If we don't have an invocation node, then see if we can infer a delegate in + // this location. Check if this is a place where a delegate can go. Only do this + // for identifier names. for now. It gets really funky if you have to deal with + // a generic name here. + if (simpleName is IdentifierNameSyntax && + !simpleNameOrMemberAccessExpression.IsLeftSideOfAnyAssignExpression()) { - // If we don't have an invocation node, then see if we can infer a delegate in - // this location. Check if this is a place where a delegate can go. Only do this - // for identifier names. for now. It gets really funky if you have to deal with - // a generic name here. - - // Can't assign into a method. - if (!simpleNameOrMemberAccessExpression.IsLeftSideOfAnyAssignExpression()) - { - invocationExpressionOpt = null; - isInConditionalAccessExpression = inConditionalMemberAccess; - return true; - } + invocationExpressionOpt = null; + isInConditionalAccessExpression = conditionalAccessExpression != null; + return true; } } diff --git a/src/Analyzers/CSharp/Tests/GenerateMethod/GenerateMethodTests.cs b/src/Analyzers/CSharp/Tests/GenerateMethod/GenerateMethodTests.cs index 5dc8bbc1fc38c..37b12812aab52 100644 --- a/src/Analyzers/CSharp/Tests/GenerateMethod/GenerateMethodTests.cs +++ b/src/Analyzers/CSharp/Tests/GenerateMethod/GenerateMethodTests.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.CodeFixes.GenerateMethod; using Microsoft.CodeAnalysis.CSharp.CodeStyle; +using Microsoft.CodeAnalysis.CSharp.Shared.Extensions; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Test.Utilities; @@ -7686,6 +7687,50 @@ internal int C() """); } + [Fact, WorkItem("http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1064748")] + public async Task TestGenerateMethodInConditionalAccess7_B() + { + await TestInRegularAndScriptAsync( + """ + class C + { + public C B { get; private set; } + public E D { get; private set; } + + void Main(C a) + { + int? x = a?.B.B.D.[|C|](); + } + + public class E + { + } + } + """, + """ + using System; + + class C + { + public C B { get; private set; } + public E D { get; private set; } + + void Main(C a) + { + int? x = a?.B.B.D.C(); + } + + public class E + { + internal int C() + { + throw new NotImplementedException(); + } + } + } + """); + } + [Fact, WorkItem("http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1064748")] public async Task TestGenerateMethodInConditionalAccess8() { @@ -11145,4 +11190,80 @@ private static void Test() } """, parseOptions: CSharpParseOptions.Default); } + + [Fact] + public async Task TestNullConditionalAssignment1() + { + await TestAsync( + """ + using System; + + internal class Program + { + int x; + + void M(Program p) + { + p?.x = [|Goo|](); + } + } + """, + """ + using System; + + internal class Program + { + int x; + + void M(Program p) + { + p?.x = [|Goo|](); + } + + private int Goo() + { + throw new NotImplementedException(); + } + } + """, parseOptions: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersionExtensions.CSharpNext)); + } + + [Fact] + public async Task TestNullConditionalAssignment2() + { + await TestAsync( + """ + using System; + + internal class Program + { + Program P; + int x; + + void M(Program p) + { + p?.P.x = [|Goo|](); + } + } + """, + """ + using System; + + internal class Program + { + Program P; + int x; + + void M(Program p) + { + p?.P.x = [|Goo|](); + } + + private int Goo() + { + throw new NotImplementedException(); + } + } + """, parseOptions: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersionExtensions.CSharpNext)); + } } diff --git a/src/Analyzers/Core/CodeFixes/GenerateMember/AbstractGenerateMemberService.cs b/src/Analyzers/Core/CodeFixes/GenerateMember/AbstractGenerateMemberService.cs index c804217770776..066b43781528e 100644 --- a/src/Analyzers/Core/CodeFixes/GenerateMember/AbstractGenerateMemberService.cs +++ b/src/Analyzers/Core/CodeFixes/GenerateMember/AbstractGenerateMemberService.cs @@ -64,8 +64,13 @@ protected static bool TryDetermineTypeToGenerateIn( TryDetermineTypeToGenerateInWorker( document, containingType, simpleNameOrMemberAccessExpression, cancellationToken, out typeToGenerateIn, out isStatic, out isColorColorCase); - typeToGenerateIn = typeToGenerateIn?.OriginalDefinition; + if (typeToGenerateIn.IsNullable(out var underlyingType) && + underlyingType is INamedTypeSymbol underlyingNamedType) + { + typeToGenerateIn = underlyingNamedType; + } + typeToGenerateIn = typeToGenerateIn?.OriginalDefinition; return typeToGenerateIn != null; } @@ -96,11 +101,8 @@ private static void TryDetermineTypeToGenerateInWorker( DetermineTypeToGenerateInWorker( semanticModel, beforeDotExpression, out typeToGenerateIn, out isStatic, out isColorColorCase, cancellationToken); } - - return; } - - if (syntaxFacts.IsConditionalAccessExpression(expression)) + else if (syntaxFacts.IsConditionalAccessExpression(expression)) { var beforeDotExpression = syntaxFacts.GetExpressionOfConditionalAccessExpression(expression); @@ -108,17 +110,9 @@ private static void TryDetermineTypeToGenerateInWorker( { DetermineTypeToGenerateInWorker( semanticModel, beforeDotExpression, out typeToGenerateIn, out isStatic, out isColorColorCase, cancellationToken); - if (typeToGenerateIn.IsNullable(out var underlyingType) && - underlyingType is INamedTypeSymbol underlyingNamedType) - { - typeToGenerateIn = underlyingNamedType; - } } - - return; } - - if (syntaxFacts.IsPointerMemberAccessExpression(expression)) + else if (syntaxFacts.IsPointerMemberAccessExpression(expression)) { var beforeArrowExpression = syntaxFacts.GetExpressionOfMemberAccessExpression(expression); if (beforeArrowExpression != null) @@ -128,14 +122,10 @@ private static void TryDetermineTypeToGenerateInWorker( if (typeInfo.Type is IPointerTypeSymbol pointerType) { typeToGenerateIn = pointerType.PointedAtType as INamedTypeSymbol; - isStatic = false; } } - - return; } - - if (syntaxFacts.IsAttributeNamedArgumentIdentifier(expression)) + else if (syntaxFacts.IsAttributeNamedArgumentIdentifier(expression)) { var attributeNode = expression.GetAncestors().FirstOrDefault(syntaxFacts.IsAttribute); Contract.ThrowIfNull(attributeNode); @@ -144,16 +134,11 @@ private static void TryDetermineTypeToGenerateInWorker( var attributeType = semanticModel.GetTypeInfo(attributeName, cancellationToken); typeToGenerateIn = attributeType.Type as INamedTypeSymbol; - isStatic = false; - return; } - - if (syntaxFacts.IsMemberInitializerNamedAssignmentIdentifier( + else if (syntaxFacts.IsMemberInitializerNamedAssignmentIdentifier( expression, out var initializedObject)) { typeToGenerateIn = semanticModel.GetTypeInfo(initializedObject, cancellationToken).Type as INamedTypeSymbol; - isStatic = false; - return; } else if (syntaxFacts.IsNameOfSubpattern(expression)) { @@ -164,15 +149,23 @@ private static void TryDetermineTypeToGenerateInWorker( // something like: { [|X|]: int i } or like: Blah { [|X|]: int i } var inferenceService = semanticDocument.Document.GetRequiredLanguageService(); typeToGenerateIn = inferenceService.InferType(semanticModel, propertyPatternClause, objectAsDefault: true, cancellationToken) as INamedTypeSymbol; - - isStatic = false; - return; } } + else if (syntaxFacts.IsMemberBindingExpression(expression)) + { + var target = syntaxFacts.GetTargetOfMemberBinding(expression); - // Generating into the containing type. - typeToGenerateIn = containingType; - isStatic = syntaxFacts.IsInStaticContext(expression); + if (target != null) + { + typeToGenerateIn = semanticModel.GetTypeInfo(target, cancellationToken).Type as INamedTypeSymbol; + } + } + else + { + // Generating into the containing type. + typeToGenerateIn = containingType; + isStatic = syntaxFacts.IsInStaticContext(expression); + } } private static void DetermineTypeToGenerateInWorker( diff --git a/src/Features/CSharp/Portable/GenerateMember/GenerateVariable/CSharpGenerateVariableService.cs b/src/Features/CSharp/Portable/GenerateMember/GenerateVariable/CSharpGenerateVariableService.cs index f1fc04e07d91b..7a3aaeccf1c89 100644 --- a/src/Features/CSharp/Portable/GenerateMember/GenerateVariable/CSharpGenerateVariableService.cs +++ b/src/Features/CSharp/Portable/GenerateMember/GenerateVariable/CSharpGenerateVariableService.cs @@ -114,6 +114,8 @@ identifierName.Parent.Parent is AssignmentExpressionSyntax assignmentExpression simpleNameOrMemberAccessExpression = identifierName; } + isConditionalAccessExpression = identifierName.Parent.Parent is ConditionalAccessExpressionSyntax; + // If we're being invoked, then don't offer this, offer generate method instead. // Note: we could offer to generate a field with a delegate type. However, that's // very esoteric and probably not what most users want. @@ -126,7 +128,6 @@ identifierName.Parent.Parent is AssignmentExpressionSyntax assignmentExpression var block = identifierName.GetAncestor(); isInExecutableBlock = block != null && !block.OverlapsHiddenPosition(cancellationToken); - isConditionalAccessExpression = identifierName.Parent.Parent is ConditionalAccessExpressionSyntax; return true; } diff --git a/src/Features/CSharpTest/GenerateVariable/GenerateVariableTests.cs b/src/Features/CSharpTest/GenerateVariable/GenerateVariableTests.cs index f71f536e542ac..ae11956e5cffa 100644 --- a/src/Features/CSharpTest/GenerateVariable/GenerateVariableTests.cs +++ b/src/Features/CSharpTest/GenerateVariable/GenerateVariableTests.cs @@ -11202,4 +11202,70 @@ void Method(Class c) } """); } + + [Fact] + public async Task TestNullConditionalAssignment3() + { + await TestInRegularAndScriptAsync( + """ + class Class + { + void Method(D c) + { + c?.[|Goo|] = 1; + } + } + + class D + { + } + """, + """ + class Class + { + void Method(D c) + { + c?.Goo = 1; + } + } + + class D + { + public int Goo { get; internal set; } + } + """); + } + + [Fact] + public async Task TestNullConditionalAssignment4() + { + await TestInRegularAndScriptAsync( + """ + class Class + { + void Method(D? c) + { + c?.[|Goo|] = 1; + } + } + + struct D + { + } + """, + """ + class Class + { + void Method(D? c) + { + c?.Goo = 1; + } + } + + struct D + { + public int Goo { get; internal set; } + } + """); + } }