From 32d3183d82541eddb9eb83cebeeabf8d9dc03462 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Mon, 21 Nov 2016 01:11:41 -0800 Subject: [PATCH] Do not offer 'inline declaration' if it would break scoping. --- .../CSharpInlineDeclarationTests.cs | 59 +++++++++++++++++++ ...harpInlineDeclarationDiagnosticAnalyzer.cs | 42 ++++++++++++- 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs b/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs index 499c4bdf94c..dada7e67b05 100644 --- a/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs +++ b/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs @@ -922,5 +922,64 @@ void M() } }", compareTokens: false); } + + [WorkItem(15336, "https://github.com/dotnet/roslyn/issues/15336")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineDeclaration)] + public async Task TestNotMissingIfCapturedInLambdaAndNotUsedAfterwards() + { + await TestAsync( +@" +using System; + +class C +{ + void M() + { + string [|s|]; + Bar(() => Baz(out s)); + } + + void Baz(out string s) { } + + void Bar(Action a) { } +}", +@" +using System; + +class C +{ + void M() + { + Bar(() => Baz(out string s)); + } + + void Baz(out string s) { } + + void Bar(Action a) { } +}"); + } + + [WorkItem(15336, "https://github.com/dotnet/roslyn/issues/15336")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineDeclaration)] + public async Task TestMissingIfCapturedInLambdaAndUsedAfterwards() + { + await TestMissingAsync( +@" +using System; + +class C +{ + void M() + { + string [|s|]; + Bar(() => Baz(out s)); + Console.WriteLine(s); + } + + void Baz(out string s) { } + + void Bar(Action a) { } +}"); + } } } \ No newline at end of file diff --git a/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs index a0fde4b7baf..ad4015900f3 100644 --- a/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs @@ -1,9 +1,11 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Immutable; using System.Linq; using System.Threading; using Microsoft.CodeAnalysis.CodeStyle; +using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; @@ -157,9 +159,7 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context) // Find the scope that the out-declaration variable will live in after we // rewrite things. - var outArgumentScope = containingStatement.Parent is BlockSyntax - ? (BlockSyntax)containingStatement.Parent - : containingStatement; + var outArgumentScope = GetOutArgumentScope(argumentExpression); // Make sure that variable is not accessed outside of that scope. var dataFlow = semanticModel.AnalyzeDataFlow(outArgumentScope); @@ -200,6 +200,42 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context) additionalLocations: allLocations)); } + private SyntaxNode GetOutArgumentScope(SyntaxNode argumentExpression) + { + for (var current = argumentExpression; current != null; current = current.Parent) + { + if (current.Parent is LambdaExpressionSyntax lambda && + current == lambda.Body) + { + // We were in a lambda. The lambda body will be the new scope of the + // out var. + return current; + } + + if (current is StatementSyntax) + { + // We hit a statement containing the out-argument. Statements can have one of + // two forms. They're either parented by a block, or by another statement + // (i.e. they're an embedded statement). If we're parented by a block, then + // that block will be the scope of the new out-var. + // + // However, if our containing statement is not parented by a block, then that + // means we have something like: + // + // if (x) + // if (Try(out y)) + // + // In this case, there is a 'virtual' block scope surrounding the embedded 'if' + // statement, and that will be the scope the out-var goes into. + return current.IsParentKind(SyntaxKind.Block) + ? current.Parent + : current; + } + } + + return null; + } + private bool IsAccessed( SemanticModel semanticModel, ISymbol outSymbol, -- GitLab