From 28dc5966b04f9278fc25036d7f03a4183d8cd8cd Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Wed, 19 Apr 2017 12:49:40 -0700 Subject: [PATCH] Fix issue with definite-assignment checking in 'inline-decl' with 'var-typed' locals. --- .../CSharpInlineDeclarationTests.cs | 38 ++++++++++++++++ ...harpInlineDeclarationDiagnosticAnalyzer.cs | 45 +++++++++++++------ 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs b/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs index a479e16e889..8c5600b1eb5 100644 --- a/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs +++ b/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs @@ -1967,6 +1967,44 @@ void M() Console.WriteLine(_); } } +}"); + } + + [WorkItem(18668, "https://github.com/dotnet/roslyn/issues/18668")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineDeclaration)] + public async Task TestDefiniteAssignmentIssueWithVar() + { + await TestMissingInRegularAndScriptAsync( +@" +using System; + +class C +{ + static void M(bool condition) + { + [|var|] x = 1; + var result = condition && int.TryParse(""2"", out x); + Console.WriteLine(x); + } +}"); + } + + [WorkItem(18668, "https://github.com/dotnet/roslyn/issues/18668")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineDeclaration)] + public async Task TestDefiniteAssignmentIssueWithNonVar() + { + await TestMissingInRegularAndScriptAsync( +@" +using System; + +class C +{ + static void M(bool condition) + { + [|int|] x = 1; + var result = condition && int.TryParse(""2"", out x); + Console.WriteLine(x); + } }"); } } diff --git a/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs index 2986aaa3547..4c5faf99c4f 100644 --- a/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs @@ -127,8 +127,8 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context, INamedTypeSymb } var semanticModel = context.SemanticModel; - var outSymbol = semanticModel.GetSymbolInfo(argumentExpression, cancellationToken).Symbol; - if (outSymbol?.Kind != SymbolKind.Local) + var outLocalSymbol = semanticModel.GetSymbolInfo(argumentExpression, cancellationToken).Symbol as ILocalSymbol; + if (outLocalSymbol == null) { // The out-argument wasn't referencing a local. So we don't have an local // declaration that we can attempt to inline here. @@ -139,7 +139,7 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context, INamedTypeSymb // Trying to do things like inline a var-decl in a for-statement is just too // esoteric and would make us have to write a lot more complex code to support // that scenario. - var localReference = outSymbol.DeclaringSyntaxReferences.FirstOrDefault(); + var localReference = outLocalSymbol.DeclaringSyntaxReferences.FirstOrDefault(); var localDeclarator = localReference?.GetSyntax(cancellationToken) as VariableDeclaratorSyntax; if (localDeclarator == null) { @@ -196,7 +196,7 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context, INamedTypeSymb // Make sure that variable is not accessed outside of that scope. var dataFlow = semanticModel.AnalyzeDataFlow(outArgumentScope); - if (dataFlow.ReadOutside.Contains(outSymbol) || dataFlow.WrittenOutside.Contains(outSymbol)) + if (dataFlow.ReadOutside.Contains(outLocalSymbol) || dataFlow.WrittenOutside.Contains(outLocalSymbol)) { // The variable is read or written from outside the block that the new variable // would be scoped in. This would cause a break. @@ -207,7 +207,7 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context, INamedTypeSymb } // Make sure the variable isn't ever accessed before the usage in this out-var. - if (IsAccessed(semanticModel, outSymbol, enclosingBlockOfLocalStatement, + if (IsAccessed(semanticModel, outLocalSymbol, enclosingBlockOfLocalStatement, localStatement, argumentNode, cancellationToken)) { return; @@ -216,8 +216,8 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context, INamedTypeSymb // See if inlining this variable would make it so that some variables were no // longer definitely assigned. if (WouldCauseDefiniteAssignmentErrors( - semanticModel, localDeclarator, enclosingBlockOfLocalStatement, - outSymbol, cancellationToken)) + semanticModel, localDeclaration, localDeclarator, + enclosingBlockOfLocalStatement, outLocalSymbol, cancellationToken)) { return; } @@ -243,8 +243,12 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context, INamedTypeSymb } private bool WouldCauseDefiniteAssignmentErrors( - SemanticModel semanticModel, VariableDeclaratorSyntax localDeclarator, - BlockSyntax enclosingBlock, ISymbol outSymbol, CancellationToken cancellationToken) + SemanticModel semanticModel, + VariableDeclarationSyntax localDeclaration, + VariableDeclaratorSyntax localDeclarator, + BlockSyntax enclosingBlock, + ILocalSymbol outLocalSymbol, + CancellationToken cancellationToken) { // See if we have something like: // @@ -260,12 +264,12 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context, INamedTypeSymb // Find all the current read-references to the local. var query = from t in enclosingBlock.DescendantTokens() where t.Kind() == SyntaxKind.IdentifierToken - where t.ValueText == outSymbol.Name + where t.ValueText == outLocalSymbol.Name let id = t.Parent as IdentifierNameSyntax where id != null where !id.IsOnlyWrittenTo() let symbol = semanticModel.GetSymbolInfo(id).GetAnySymbol() - where outSymbol.Equals(symbol) + where outLocalSymbol.Equals(symbol) select id; var references = query.ToImmutableArray(); @@ -274,16 +278,29 @@ where outSymbol.Equals(symbol) // Ensure we can track the references and the local variable as we make edits // to the tree. - var rootWithTrackedNodes = root.TrackNodes(references.Concat(localDeclarator).Concat(enclosingBlock)); + var rootWithTrackedNodes = root.TrackNodes( + references.Concat(ImmutableArray.Create(localDeclarator, localDeclaration, enclosingBlock))); // Now, take the local variable and remove it's initializer. Then go to all // the locations where we read from it. If they're definitely assigned, then // that means the out-var did it's work and assigned the variable across all // paths. If it's not definitely assigned, then we can't inline this variable. var currentLocalDeclarator = rootWithTrackedNodes.GetCurrentNode(localDeclarator); + var currentLocalDeclaration = rootWithTrackedNodes.GetCurrentNode(localDeclaration); + var updatedDeclaration = currentLocalDeclaration + .ReplaceNode(currentLocalDeclarator, currentLocalDeclarator.WithInitializer(null)); + + // If the declaration was a "var" declaration, then replace "var" with the actual + // type of the local. This way we don't get a "'var v' requires an initializer" which + // will suppress the message about definite assignment later. + if (updatedDeclaration.Type.IsVar) + { + updatedDeclaration = updatedDeclaration.WithType( + outLocalSymbol.Type.GenerateTypeSyntax()); + } + var rootWithoutInitializer = rootWithTrackedNodes.ReplaceNode( - currentLocalDeclarator, - currentLocalDeclarator.WithInitializer(null)); + currentLocalDeclaration, updatedDeclaration); var rootWithoutInitializerTree = root.SyntaxTree.WithRootAndOptions( rootWithoutInitializer, root.SyntaxTree.Options); -- GitLab