Merge pull request #35916 from CyrusNajmabadi/usingCollision

Do not suggest converting from using-statement to using-local form if it would cause a variable collision.

src/EditorFeatures/CSharpTest/UseSimpleUsingStatement/UseSimpleUsingStatementTests.cs

@@ -9,6 +9,7 @@
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Diagnostics;
 using Microsoft.CodeAnalysis.Test.Utilities;
+using Roslyn.Test.Utilities;
 using Xunit;
 
 namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.UseSimpleUsingStatement
@@ -646,5 +647,293 @@ void M()
     }
 }", parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
         }
+
+        [WorkItem(35879, "https://github.com/dotnet/roslyn/issues/35879")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSimpleUsingStatement)]
+        public async Task TestCollision1()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        [||]using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+    }
+}",
+parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
+        }
+
+        [WorkItem(35879, "https://github.com/dotnet/roslyn/issues/35879")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSimpleUsingStatement)]
+        public async Task TestNoCollision1()
+        {
+            await TestInRegularAndScript1Async(
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        [||]using (Stream stream1 = File.OpenRead(""test""))
+        {
+        }
+    }
+}",
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        using Stream stream1 = File.OpenRead(""test"");
+    }
+}",
+parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
+        }
+
+        [WorkItem(35879, "https://github.com/dotnet/roslyn/issues/35879")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSimpleUsingStatement)]
+        public async Task TestCollision2()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        [||]using (Stream stream1 = File.OpenRead(""test""))
+        {
+            Stream stream;
+        }
+    }
+}",
+parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
+        }
+
+        [WorkItem(35879, "https://github.com/dotnet/roslyn/issues/35879")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSimpleUsingStatement)]
+        public async Task TestNoCollision2()
+        {
+            await TestInRegularAndScript1Async(
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        [||]using (Stream stream1 = File.OpenRead(""test""))
+        {
+            Stream stream2;
+        }
+    }
+}",
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        using Stream stream1 = File.OpenRead(""test"");
+        Stream stream2;
+    }
+}",
+parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
+        }
+
+        [WorkItem(35879, "https://github.com/dotnet/roslyn/issues/35879")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSimpleUsingStatement)]
+        public async Task TestCollision3()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        [||]using (Stream stream1 = File.OpenRead(""test""))
+        {
+            Goo(out var stream);
+        }
+    }
+}",
+parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
+        }
+
+        [WorkItem(35879, "https://github.com/dotnet/roslyn/issues/35879")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSimpleUsingStatement)]
+        public async Task TestNoCollision3()
+        {
+            await TestInRegularAndScript1Async(
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        [||]using (Stream stream1 = File.OpenRead(""test""))
+        {
+            Goo(out var stream2);
+        }
+    }
+}",
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        using Stream stream1 = File.OpenRead(""test"");
+        Goo(out var stream2);
+    }
+}",
+parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
+        }
+
+        [WorkItem(35879, "https://github.com/dotnet/roslyn/issues/35879")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSimpleUsingStatement)]
+        public async Task TestCollision4()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        [||]using (Stream stream1 = File.OpenRead(""test""))
+            Goo(out var stream);
+    }
+}",
+parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
+        }
+
+        [WorkItem(35879, "https://github.com/dotnet/roslyn/issues/35879")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSimpleUsingStatement)]
+        public async Task TestNoCollision4()
+        {
+            await TestInRegularAndScript1Async(
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        [||]using (Stream stream1 = File.OpenRead(""test""))
+            Goo(out var stream2);
+    }
+}",
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+        }
+        using Stream stream1 = File.OpenRead(""test"");
+        Goo(out var stream2);
+    }
+}",
+parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
+        }
+
+        [WorkItem(35879, "https://github.com/dotnet/roslyn/issues/35879")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSimpleUsingStatement)]
+        public async Task TestCollision5()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+            Stream stream1;
+        }
+        [||]using (Stream stream1 = File.OpenRead(""test""))
+        {
+        }
+    }
+}",
+parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
+        }
+
+        [WorkItem(35879, "https://github.com/dotnet/roslyn/issues/35879")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSimpleUsingStatement)]
+        public async Task TestNoCollision5()
+        {
+            await TestInRegularAndScript1Async(
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+            Stream stream1;
+        }
+        [||]using (Stream stream2 = File.OpenRead(""test""))
+        {
+        }
+    }
+}",
+@"using System.IO;
+
+class Program
+{
+    static void Main()
+    {
+        using (Stream stream = File.OpenRead(""test""))
+        {
+            Stream stream1;
+        }
+        using Stream stream2 = File.OpenRead(""test"");
+    }
+}",
+parameters: new TestParameters(parseOptions: CSharp8ParseOptions));
+        }
     }
 }

src/Features/CSharp/Portable/UseSimpleUsingStatement/UseSimpleUsingStatementDiagnosticAnalyzer.cs

 // 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.CodeStyle;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.CSharp.UseSimpleUsingStatement
 {
@@ -96,6 +99,18 @@ private void AnalyzeSyntax(SyntaxNodeAnalysisContext context)
             }
 
             var cancellationToken = context.CancellationToken;
+
+            // Converting a using-statement to a using-variable-declaration will cause the using's
+            // variables to now be pushed up to the parent block's scope. This is also true for any
+            // local variables in the innermost using's block. These may then collide with other
+            // variables in the block, causing an error.  Check for that and bail if this happens.
+            if (CausesVariableCollision(
+                    context.SemanticModel, parentBlock,
+                    outermostUsing, innermostUsing, cancellationToken))
+            {
+                return;
+            }
+
             var optionSet = context.Options.GetDocumentOptionSetAsync(syntaxTree, cancellationToken).GetAwaiter().GetResult();
             if (optionSet == null)
             {
@@ -117,6 +132,36 @@ private void AnalyzeSyntax(SyntaxNodeAnalysisContext context)
                 properties: null));
         }
 
+        private bool CausesVariableCollision(
+            SemanticModel semanticModel, BlockSyntax parentBlock,
+            UsingStatementSyntax outermostUsing, UsingStatementSyntax innermostUsing,
+            CancellationToken cancellationToken)
+        {
+            var symbolNameToExistingSymbol = semanticModel.GetExistingSymbols(parentBlock, cancellationToken).ToLookup(s => s.Name);
+
+            for (var current = outermostUsing; current != null; current = current.Statement as UsingStatementSyntax)
+            {
+                // Check if the using statement itself contains variables that will collide
+                // with other variables in the block.
+                var usingOperation = (IUsingOperation)semanticModel.GetOperation(current, cancellationToken);
+                if (DeclaredLocalCausesCollision(symbolNameToExistingSymbol, usingOperation.Locals))
+                {
+                    return true;
+                }
+            }
+
+            var innerUsingOperation = (IUsingOperation)semanticModel.GetOperation(innermostUsing, cancellationToken);
+            if (innerUsingOperation.Body is IBlockOperation innerUsingBlock)
+            {
+                return DeclaredLocalCausesCollision(symbolNameToExistingSymbol, innerUsingBlock.Locals);
+            }
+
+            return false;
+        }
+
+        private static bool DeclaredLocalCausesCollision(ILookup<string, ISymbol> symbolNameToExistingSymbol, ImmutableArray<ILocalSymbol> locals)
+            => locals.Any(local => symbolNameToExistingSymbol[local.Name].Any(otherLocal => !local.Equals(otherLocal)));
+
         private static bool PreservesSemantics(
             BlockSyntax parentBlock,
             UsingStatementSyntax outermostUsing,
@@ -132,7 +177,6 @@ private void AnalyzeSyntax(SyntaxNodeAnalysisContext context)
         private static bool UsingStatementDoesNotInvolveJumps(
             SyntaxList<StatementSyntax> parentStatements, int index, UsingStatementSyntax innermostUsing)
         {
-
             // Jumps are not allowed to cross a using declaration in the forward direction, 
             // and can't go back unless there is a curly brace between the using and the label.
             //