ExtractMethodMatrix.GetVariableStyle Throws exception on code with errors

[jmarolf]

Version Used:

Steps to Reproduce:

1. Use Alt+Arrow to move members into lambda that is an argument into a method that is missing semicolon

Expected Behavior:
No Crash

Actual Behavior:

System.InvalidOperationException : Unexpected false
   at Roslyn.Utilities.Contract.ThrowIfFalse(Boolean condition,String message)
   at Microsoft.CodeAnalysis.ExtractMethod.ExtractMethodMatrix.GetVariableStyle(Boolean captured,Boolean dataFlowIn,Boolean dataFlowOut,Boolean alwaysAssigned,Boolean variableDeclared,Boolean readInside,Boolean writtenInside,Boolean readOutside,Boolean writtenOutside,Boolean unsafeAddressTaken)
   at Microsoft.CodeAnalysis.ExtractMethod.MethodExtractor.Analyzer.GetVariableStyle(Dictionary`2 symbolMap,ISymbol symbol,SemanticModel model,ITypeSymbol type,Boolean captured,Boolean dataFlowIn,Boolean dataFlowOut,Boolean alwaysAssigned,Boolean variableDeclared,Boolean readInside,Boolean writtenInside,Boolean readOutside,Boolean writtenOutside,Boolean unsafeAddressTaken)
   at Microsoft.CodeAnalysis.ExtractMethod.MethodExtractor.Analyzer.GenerateVariableInfoMap(SemanticModel model,DataFlowAnalysis dataFlowAnalysisData,Dictionary`2 symbolMap)
   at async Microsoft.CodeAnalysis.ExtractMethod.MethodExtractor.Analyzer.AnalyzeAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.ExtractMethod.MethodExtractor.ExtractMethodAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.ExtractMethod.AbstractExtractMethodService`3.ExtractMethodAsync[TValidator,TExtractor,TResult](<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeRefactorings.ExtractMethod.ExtractMethodCodeRefactoringProvider.GetCodeActionAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeRefactorings.ExtractMethod.ExtractMethodCodeRefactoringProvider.ComputeRefactoringsAsync(<Unknown Parameters>)
   at async Microsoft.CodeAnalysis.CodeRefactorings.CodeRefactoringService.GetRefactoringFromProviderAsync(<Unknown Parameters>)
   at Microsoft.VisualStudio.Telemetry.WindowsErrorReporting.WatsonReport.GetClrWatsonExceptionInfo(Exception exceptionObject)

Also reported at https://developercommunity.visualstudio.com/content/problem/14869/extractmethodcoderefactoringprovider-encountered-a.html

[Pilchie]

Do you have a repro or dump?

[tmat]

I've got a dump and a repro solution: %internal_share%\public\tomat\Bugs\17165

[tmat]

Repro: sync to branch https://github.com/tmat/symreader-converter/tree/ExtractMethodCrashRepro
Run restore.cmd
Open src\Microsoft.DiaSymReader.Converter\PdbConverterPortableToWindows.cs
Select code

                        foreach (var localConstantHandle in currentLocalScope.GetLocalConstants())
                        {
                            var constant = pdbReader.GetLocalConstant(localConstantHandle);
                            string name = pdbReader.GetString(constant.Name);

                            if (name.Length > MaxEntityNameLength)
                            {
                                // TODO: report warning
                                continue;
                            }

                            var (value, signature) = PortableConstantSignature.GetConstantValueAndSignature(pdbReader, localConstantHandle, pdbWriter.MetadataImport);
                            if (!metadataModel.TryGetStandaloneSignatureHandle(signature, out var constantSignatureHandle))
                            {
                                // TODO: report warning

                                // TODO: 
                                // Currently the EEs require signature to match exactly the type of the value. 
                                // We could relax that and use the type of the value regardless of the signature for primitive types.
                                // Then we could use any signature here.
                                continue;
                            }

                            pdbWriter.DefineLocalConstant(name, value, MetadataTokens.GetToken(constantSignatureHandle));
                        }

                        foreach (var localVariableHandle in currentLocalScope.GetLocalVariables())
                        {
                            var variable = pdbReader.GetLocalVariable(localVariableHandle);
                            string name = pdbReader.GetString(variable.Name);

                            if (name.Length > MaxEntityNameLength)
                            {
                                // TODO: report warning
                                continue;
                            }

                            int localSignatureToken = methodBody.LocalSignature.IsNil ? 0 : MetadataTokens.GetToken(methodBody.LocalSignature);
                            pdbWriter.DefineLocalVariable(variable.Index, name, variable.Attributes, localSignatureToken);
                        }
}

[Pilchie]

Note the repro in #18347.

[Pilchie]

And reported again at https://developercommunity.visualstudio.com/content/problem/42806/code-refactorying-crashed-while-editing-code.html

[davkean]

@sharwell @agocke I ran into this exact stack with the following (valid) code:

    public override IDependency BeforeAdd(
            string projectPath,
            ITargetFramework targetFramework,
            IDependency dependency, 
            ImmutableDictionary<string, IDependency>.Builder worldBuilder,
            ImmutableHashSet<IDependency>.Builder topLevelBuilder,
            Dictionary<string, IProjectDependenciesSubTreeProvider> subTreeProviders,
            HashSet<string> projectItemSpecs,
            out bool filterAnyChanges)
        {
            filterAnyChanges = false;
            IDependency resultDependency = dependency;

            if (!resultDependency.TopLevel
                || resultDependency.Implicit                
                || !resultDependency.Resolved
                || !resultDependency.Flags.Contains(DependencyTreeFlags.GenericDependencyFlags))
            {
                return resultDependency;
            }

$          if (projectItemSpecs == null)   // No data
                return resultDependency;

            if$ (!projectItemSpecs.Contains(resultDependency.OriginalItemSpec))
            {
                // it is an implicit dependency
                if (!subTreeProviders.TryGetValue(resultDependency.ProviderType, out IProjectDependenciesSubTreeProvider provider))
                {
                    return resultDependency;
                }

                var internalProvider = provider as IProjectDependenciesSubTreeProviderInternal;
                if (internalProvider == null)
                {
                    return resultDependency;
                }

                resultDependency = resultDependency.SetProperties(
                    icon: internalProvider.GetImplicitIcon(),
                    isImplicit: true);
                filterAnyChanges = true;
            }

            return resultDependency;
        }

Changing the selection to the area indicated by the $, causes the crash. Will this be resolved by the changes around this?

[lukaseckert]

I also experience this issue with valid code. As soon as I have local functions within my outer method, the ExtractMethodCodeRefactoringProvider crashes and VS2017 shows the yellow notification bar that asks me if I want to enable it again.
Consider that piece of code:

 private ObjectChart RenderTreeMapChart(ChartProjectLayout layout, TreeMapChartDefinition chartDef, CollectionObjectBases objectList)
        {
            //// Formula to display caption and numeric value of a node in the treemap 
            //const string labelContent_NodeNameAndNodeValue = "LL.ChartObject.ParentNodeText0 + chr$(13) + \" (\" + CStr$(LL.ChartObject.NodeSum,\"%g\") + \")\"";  // show data row title + node value in the square

            //// Local function to set maximum node count for an axis
            //void ApplyMaxEntryCount(PropertyMaxEntryCount prop)
            //{
            //    if (chartDef.TopNLimit > 0)
            //    {
            //        prop.MaxEntries = chartDef.TopNLimit.ToString();
            //        prop.WithOthers = "True";
            //        prop.Label = _utils.TextToFormula(Resources.General_OtherValues);
            //    }
            //}

            //// Local function to disable the regular axis label (drawn over top of a node) and set a label in the center of each node that contains the caption+node value, like "Germany (12)"
            //void SetLabelForNodes(CoordinateLabelPlacement axisLabel, PropertyChartLabelCoordinateOnObject nodeLabel)
            //{
            //    axisLabel.Placement = "0";  // no label on axis
            //    nodeLabel.Contents = labelContent_NodeNameAndNodeValue;
            //    nodeLabel.Placement = "3"; // center
            //    nodeLabel.Font.Size = "36";
            //}

            ...

When I uncomment those local functions at the begin of the outer method, selecting any piece of text in the rest of the method will lead to the crash.