Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExtractMethodCodeRefactoringProvider crashes on selecting a line that references an argument captured by a local method #18347

Closed
HellBrick opened this issue Mar 31, 2017 · 1 comment
Labels
Area-IDE Bug Resolution-Duplicate The described behavior is tracked in another issue

Comments

@HellBrick
Copy link

HellBrick commented Mar 31, 2017

Version Used:

Microsoft Visual Studio Community 2017
Version 15.0.26228.9

Steps to Reproduce:

namespace ExtractMethodCrashRepro
{
	public static class SomeClass
	{
		private static void Repro( int arg )
		{
			int localValue = arg;

			int LocalCapture() => arg;
		}
	}
}

Select the line int localValue = arg;

Expected Behavior:
A lightbulb with available refactorings should appear.

Actual Behavior:
ExtractMethodCodeRefactoringProvider crashes with the following stack trace:

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)
@Pilchie
Copy link
Member

Pilchie commented Mar 31, 2017

Dupe of #17165? (But with a better repro?)

@Pilchie Pilchie closed this as completed Mar 31, 2017
@Pilchie Pilchie added the Resolution-Duplicate The described behavior is tracked in another issue label Mar 31, 2017
sharwell added a commit to sharwell/roslyn that referenced this issue Apr 4, 2017
agocke added a commit to agocke/roslyn that referenced this issue Jun 4, 2017
By design, when data flow analyzes local functions it does so devoid of
surrounding context and records possibly unassigned variables rather
than reporting a diagnostic. Thus, all captured variables are expected
to be marked unassigned during data flow analysis without reporting a
diagnostic.

However, reporting a diagnostic is not the only side effect of running
data flow analysis. By calling virtual methods data flow analysis
informs derived types of unassigned variables. While the diagnostic is
supressed and recorded, currently the virtual method is still called.
This has negative consequences for region analysis, which considers
these calls as indications of variables flowing out of the given
region, which may not be the case for local functions.

This PR changes data flow analysis to only call the ReportUnassigned
virtual method if not inside a local function, excluding local functions
from all unassignment reporting.

Fixes dotnet#17165, dotnet#18347
agocke added a commit that referenced this issue Jun 15, 2017
By design, when data flow analyzes local functions it does so devoid of
surrounding context and records possibly unassigned variables rather
than reporting a diagnostic. Thus, all captured variables are expected
to be marked unassigned during data flow analysis without reporting a
diagnostic.

However, reporting a diagnostic is not the only side effect of running
data flow analysis. By calling virtual methods data flow analysis
informs derived types of unassigned variables. While the diagnostic is
suppressed and recorded, currently the virtual method is still called.
This has negative consequences for region analysis, which considers
these calls as indications of variables flowing out of the given
region, which may not be the case for local functions.

This PR changes data flow analysis to only call the ReportUnassigned
virtual method if not inside a local function, excluding local functions
from all unassignment reporting.

Fixes #17165, #18347
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug Resolution-Duplicate The described behavior is tracked in another issue
Projects
None yet
Development

No branches or pull requests

2 participants