Merge pull request #24619 from JieCarolHu/issue24262

Do not suggest var when it change static type of the variable in foreach statement

src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseImplicitTypeTests.cs

@@ -2207,6 +2207,113 @@ static void Main(string[] args)
     private static GetHandler Handler;
 
     delegate object GetHandler();
+}", new TestParameters(options: ImplicitTypeEverywhere()));
+        }
+
+        [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)]
+        [WorkItem(24262, "https://github.com/dotnet/roslyn/issues/24262")]
+        public async Task DoNotSuggestVarForInterfaceVariableInForeachStatement()
+        {
+            await TestMissingInRegularAndScriptAsync(@"
+public interface ITest
+{
+    string Value { get; }
+}
+public class TestInstance : ITest
+{
+    string ITest.Value => ""Hi"";
+}
+
+public class Test
+{
+    public TestInstance[] Instances { get; }
+
+    public void TestIt()
+    {
+        foreach ([|ITest|] test in Instances)
+        {
+            Console.WriteLine(test.Value);
+        }
+    }
+}", new TestParameters(options: ImplicitTypeEverywhere()));
+        }
+
+        [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)]
+        [WorkItem(24262, "https://github.com/dotnet/roslyn/issues/24262")]
+        public async Task DoNotSuggestVarForInterfaceVariableInDeclarationStatement()
+        {
+    await TestMissingInRegularAndScriptAsync(@"
+public interface ITest
+{
+    string Value { get; }
+}
+public class TestInstance : ITest
+{
+    string ITest.Value => ""Hi"";
+}
+
+public class Test
+{
+    public void TestIt()
+    {
+        [|ITest|] test = new TestInstance();
+        Console.WriteLine(test.Value);
+    }
+}", new TestParameters(options: ImplicitTypeEverywhere()));
+        }
+
+        [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)]
+        [WorkItem(24262, "https://github.com/dotnet/roslyn/issues/24262")]
+        public async Task DoNotSuggestVarForAbstractClassVariableInForeachStatement()
+        {
+            await TestMissingInRegularAndScriptAsync(@"
+public abstract class MyAbClass
+{
+    string Value { get; }
+}
+
+public class TestInstance : MyAbClass
+{
+    public string Value => ""Hi"";
+}
+
+public class Test
+{
+    public TestInstance[] Instances { get; }
+
+    public void TestIt()
+    {
+        foreach ([|MyAbClass|] instance in Instances)
+        {
+            Console.WriteLine(instance);
+        }
+    }
+}", new TestParameters(options: ImplicitTypeEverywhere()));
+        }
+
+        [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)]
+        [WorkItem(24262, "https://github.com/dotnet/roslyn/issues/24262")]
+        public async Task DoNotSuggestVarForAbstractClassVariableInDeclarationStatement()
+        {
+            await TestMissingInRegularAndScriptAsync(@"
+public abstract class MyAbClass
+{
+    string Value { get; }
+}
+
+public class TestInstance : MyAbClass
+{
+    public string Value => ""Hi"";
+}
+
+public class Test
+{
+    public TestInstance[] Instances { get; }
+
+    public void TestIt()
+    {
+        [|MyAbClass|]  test = new TestInstance();
+    }
 }", new TestParameters(options: ImplicitTypeEverywhere()));
         }
     }

src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs

@@ -137,7 +137,7 @@ protected override bool TryAnalyzeVariableDeclaration(TypeSyntax typeName, Seman
             else if (typeName.Parent is ForEachStatementSyntax foreachStatement)
             {
                 var foreachStatementInfo = semanticModel.GetForEachStatementInfo(foreachStatement);
-                if (foreachStatementInfo.ElementConversion.IsIdentityOrImplicitReference())
+                if (foreachStatementInfo.ElementConversion.IsIdentity)
                 {
                     issueSpan = candidateIssueSpan;
                     return true;
@@ -160,9 +160,9 @@ protected override bool TryAnalyzeVariableDeclaration(TypeSyntax typeName, Seman
             OptionSet optionSet,
             CancellationToken cancellationToken)
         {
-            // It's not always safe to convert a decl expression like "Method(out int i)" to 
-            // "Method(out var i)".  Changing to 'var' may cause overload resolution errors.  
-            // Have to see if using 'var' means not resolving to the same type as before.  
+            // It's not always safe to convert a decl expression like "Method(out int i)" to
+            // "Method(out var i)".  Changing to 'var' may cause overload resolution errors.
+            // Have to see if using 'var' means not resolving to the same type as before.
             // Note: this is fairly expensive, so we try to avoid this if we can by seeing if
             // there are multiple candidates with the original call.  If not, then we don't
             // have to do anything.
@@ -171,9 +171,9 @@ protected override bool TryAnalyzeVariableDeclaration(TypeSyntax typeName, Seman
                 argumentList.Parent is InvocationExpressionSyntax invocationExpression)
             {
                 // If there was only one member in the group, and it was non-generic itself,
-                // then this change is safe to make without doing any complex analysis.  
+                // then this change is safe to make without doing any complex analysis.
                 // Multiple methods mean that switching to 'var' might remove information
-                // that affects overload resolution.  And if the method is generic, then 
+                // that affects overload resolution.  And if the method is generic, then
                 // switching to 'var' may mean that inference might not work properly.
                 var memberGroup = semanticModel.GetMemberGroup(invocationExpression.Expression, cancellationToken);
                 if (memberGroup.Length == 1 &&
@@ -183,9 +183,9 @@ protected override bool TryAnalyzeVariableDeclaration(TypeSyntax typeName, Seman
                 }
             }
 
-            // Do the expensive check.  Note: we can't use the SpeculationAnalyzer (or any 
+            // Do the expensive check.  Note: we can't use the SpeculationAnalyzer (or any
             // speculative analyzers) here.  This is due to https://github.com/dotnet/roslyn/issues/20724.
-            // Specifically, all the speculative helpers do not deal with with changes to code that 
+            // Specifically, all the speculative helpers do not deal with with changes to code that
             // introduces a variable (in this case, the declaration expression).  The compiler sees
             // this as an error because there are now two colliding variables, which causes all sorts
             // of errors to be reported.
@@ -252,14 +252,14 @@ protected override bool TryAnalyzeVariableDeclaration(TypeSyntax typeName, Seman
             // and if we're replacing the declaration with 'var' we'd be changing the semantics by inferring type of
             // initializer expression and thereby losing the conversion.
             var conversion = semanticModel.GetConversion(expression, cancellationToken);
-            if (conversion.Exists && conversion.IsImplicit && !conversion.IsIdentity)
+            if (conversion.IsIdentity)
             {
-                return false;
+                // final check to compare type information on both sides of assignment.
+                var initializerType = semanticModel.GetTypeInfo(expression, cancellationToken).Type;
+                return declaredType.Equals(initializerType);
             }
 
-            // final check to compare type information on both sides of assignment.
-            var initializerType = semanticModel.GetTypeInfo(expression, cancellationToken).Type;
-            return declaredType.Equals(initializerType);
+            return false;
         }
 
         protected override bool ShouldAnalyzeDeclarationExpression(DeclarationExpressionSyntax declaration, SemanticModel semanticModel, CancellationToken cancellationToken)