Extract Method on part of an expression generates invalid code

[thetuvix]

Version Used:
Visual Studio 2019 version 16.2.2

Steps to Reproduce:

1. Add the following code to a class:

private void Repro()
{
    int i = 1, j = 2;
    int k = i + j + 1;
}

2. Select i + j and perform an Extract Method refactoring.

Expected Behavior:
Extract Method generates NewMethod(int i, int j):

private void Repro()
{
    int i = 1, j = 2;
    int k = NewMethod(i, j) + 1;
}

private static int NewMethod(int i, int j)
{
    return i + j;
}

Actual Behavior:
Extract Method generates NewMethod() without parameters to pass in i and j:

private void Repro()
{
    int i = 1, j = 2;
    int k = NewMethod() + 1;
}

private static int NewMethod()
{
    return i + j;
}

[RikkiGibson]

This could be a problem in IDE code or in dataflow analysis. We'll see once we take a closer look at the bug.

[CyrusNajmabadi]

This could be a problem in IDE code or in dataflow analysis. We'll see once we take a closer look at the bug.

@RikkiGibson this appears to be an issue with dataflow analysis. We ask for dataflow on the i + j expression and we get back nothing. It's an entirely empty object.

I debugger to this point:

                    _succeeded = !_context.Failed;
                    var result = _context.Failed ? ImmutableArray<ISymbol>.Empty :
                        Normalize(DataFlowsInWalker.Analyze(_context.Compilation, _context.Member, _context.BoundNode, _context.FirstInRegion, _context.LastInRegion, UnassignedVariables, UnassignedVariableAddressOfSyntaxes, out _succeeded));
                    ImmutableInterlocked.InterlockedInitialize(ref _dataFlowsIn, result);

At which point _succeeded is set to false in the 'out' param of DataFlowsInWalker.Analyze.

[CyrusNajmabadi]

This is where we set that it's bad:

In this callstack:

>	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.AbstractFlowPass<Microsoft.CodeAnalysis.CSharp.DefiniteAssignmentPass.LocalState, Microsoft.CodeAnalysis.CSharp.DefiniteAssignmentPass.LocalFunctionState>.Scan(ref bool badRegion) Line 428	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.DefiniteAssignmentPass.Scan(ref bool badRegion) Line 371	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.AbstractRegionDataFlowPass.Scan(ref bool badRegion) Line 36	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.AbstractFlowPass<Microsoft.CodeAnalysis.CSharp.DefiniteAssignmentPass.LocalState, Microsoft.CodeAnalysis.CSharp.DefiniteAssignmentPass.LocalFunctionState>.Analyze(ref bool badRegion, Microsoft.CodeAnalysis.Optional<Microsoft.CodeAnalysis.CSharp.DefiniteAssignmentPass.LocalState> initialState) Line 447	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.DefiniteAssignmentPass.Analyze(ref bool badRegion, Microsoft.CodeAnalysis.DiagnosticBag diagnostics) Line 682	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.DataFlowsInWalker.Analyze(ref bool badRegion) Line 52	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.DataFlowsInWalker.Analyze(Microsoft.CodeAnalysis.CSharp.CSharpCompilation compilation, Microsoft.CodeAnalysis.CSharp.Symbol member, Microsoft.CodeAnalysis.CSharp.BoundNode node, Microsoft.CodeAnalysis.CSharp.BoundNode firstInRegion, Microsoft.CodeAnalysis.CSharp.BoundNode lastInRegion, System.Collections.Generic.HashSet<Microsoft.CodeAnalysis.CSharp.Symbol> unassignedVariables, System.Collections.Generic.HashSet<Microsoft.CodeAnalysis.CSharp.Syntax.PrefixUnaryExpressionSyntax> unassignedVariableAddressOfSyntaxes, out bool? succeeded) Line 40	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpDataFlowAnalysis.DataFlowsIn.get() Line 102	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpDataFlowAnalysis.Succeeded.get() Line 383	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpDataFlowAnalysis.AnalyzeReadWrite() Line 256	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpDataFlowAnalysis.Captured.get() Line 291	C#
 	Microsoft.CodeAnalysis.Features.dll!Microsoft.CodeAnalysis.ExtractMethod.MethodExtractor<Microsoft.CodeAnalysis.CSharp.ExtractMethod.CSharpSelectionResult, Microsoft.CodeAnalysis.CSharp.Syntax.StatementSyntax, Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax>.Analyzer.GenerateVariableInfoMap(bool bestEffort, Microsoft.CodeAnalysis.SemanticModel model, Microsoft.CodeAnalysis.DataFlowAnalysis dataFlowAnalysisData, System.Collections.Generic.Dictionary<Microsoft.CodeAnalysis.ISymbol, System.Collections.Generic.List<Microsoft.CodeAnalysis.SyntaxToken>> symbolMap, out System.Collections.Generic.Dictionary<Microsoft.CodeAnalysis.ISymbol, Microsoft.CodeAnalysis.ExtractMethod.MethodExtractor<Microsoft.CodeAnalysis.CSharp.ExtractMethod.CSharpSelectionResult, Microsoft.CodeAnalysis.CSharp.Syntax.StatementSyntax, Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax>.VariableInfo> variableInfoMap, out System.Collections.Generic.List<Microsoft.CodeAnalysis.ISymbol> failedVariables) Line 478	C#

[CyrusNajmabadi]

Passing to compiler to take a look.