Merge pull request dotnet#18116 from CyrusNajmabadi/patternMatchingCh…

…angedType

Don't offer pattern matching if it would change the type of a variable.

src/EditorFeatures/CSharpTest/UsePatternMatching/CSharpAsAndNullCheckTests.cs

@@ -524,6 +524,39 @@ public static void Main()
                 Console.WriteLine(i);
         }
     }
+}");
+        }
+
+        [WorkItem(18053, "https://github.com/dotnet/roslyn/issues/18053")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineTypeCheck)]
+        public async Task TestMissingWhenTypesDoNotMatch()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class SyntaxNode
+{
+    public SyntaxNode Parent;
+}
+
+class BaseParameterListSyntax : SyntaxNode
+{
+}
+
+class ParameterSyntax : SyntaxNode
+{
+
+}
+
+public static class C
+{
+    static void M(ParameterSyntax parameter)
+    {
+        [|SyntaxNode|] parent = parameter.Parent as BaseParameterListSyntax;
+
+        if (parent != null)
+        {
+            parent = parent.Parent;
+        }
+    }
 }");
         }
     }

src/EditorFeatures/CSharpTest/UsePatternMatching/CSharpIsAndCastCheckTests.cs

@@ -484,6 +484,38 @@ void M()
             var v = 1;
         }
     }
+}");
+        }
+
+        [WorkItem(18053, "https://github.com/dotnet/roslyn/issues/18053")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineTypeCheck)]
+        public async Task TestMissingWhenTypesDoNotMatch()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class SyntaxNode
+{
+    public SyntaxNode Parent;
+}
+
+class BaseParameterListSyntax : SyntaxNode
+{
+}
+
+class ParameterSyntax : SyntaxNode
+{
+
+}
+
+public static class C
+{
+    static void N(ParameterSyntax parameter)
+    {
+        if (parameter.Parent is BaseParameterListSyntax)
+        {
+            [|SyntaxNode|] parent = (BaseParameterListSyntax)parameter.Parent;
+            parent = parent.Parent;
+        }
+    }
 }");
         }
     }

src/Features/CSharp/Portable/UsePatternMatching/CSharpAsAndNullCheckDiagnosticAnalyzer.cs

@@ -133,15 +133,35 @@ private void SyntaxNodeAction(SyntaxNodeAnalysisContext syntaxContext)
                 return;
             }
 
+            var semanticModel = syntaxContext.SemanticModel;
             var asExpression = (BinaryExpressionSyntax)initializerValue;
             var typeNode = (TypeSyntax)asExpression.Right;
-            var type = syntaxContext.SemanticModel.GetTypeInfo(typeNode, cancellationToken).Type;
-            if (type.IsNullable())
+            var asType = semanticModel.GetTypeInfo(typeNode, cancellationToken).Type;
+            if (asType.IsNullable())
             {
                 // not legal to write "if (x is int? y)"
                 return;
             }
 
+            var localSymbol = (ILocalSymbol)semanticModel.GetDeclaredSymbol(variableDeclaration.Variables[0]);
+            if (!localSymbol.Type.Equals(asType))
+            {
+                // we have something like:
+                //
+                //      BaseType b = x as DerivedType;
+                //      if (b != null) { ... }
+                //
+                // It's not necessarily safe to convert this to:
+                //
+                //      if (x is DerivedType b) { ... }
+                //
+                // That's because there may be later code that wants to do something like assign a 
+                // 'BaseType' into 'b'.  As we've now claimed that it must be DerivedType, that 
+                // won't work.  This might also cause unintended changes like changing overload
+                // resolution.  So, we conservatively do not offer the change in a situation like this.
+                return;
+            }
+
             // If we convert this to 'if (o is Type x)' then 'x' will not be definitely assigned 
             // in the Else branch of the IfStatement, or after the IfStatement.  Make sure 
             // that doesn't cause definite assignment issues.

src/Features/CSharp/Portable/UsePatternMatching/CSharpIsAndCastCheckDiagnosticAnalyzer.cs

@@ -141,6 +141,29 @@ private void SyntaxNodeAction(SyntaxNodeAnalysisContext syntaxContext)
                 return;
             }
 
+            var semanticModel = syntaxContext.SemanticModel;
+            var localSymbol = (ILocalSymbol)semanticModel.GetDeclaredSymbol(declarator);
+            var isType = semanticModel.GetTypeInfo(castExpression.Type).Type;
+            if (!localSymbol.Type.Equals(isType))
+            {
+                // we have something like:
+                //
+                //      if (x is DerivedType)
+                //      {
+                //          BaseType b = (DerivedType)x;
+                //      }
+                //
+                // It's not necessarily safe to convert this to:
+                //
+                //      if (x is DerivedType b) { ... }
+                //
+                // That's because there may be later code that wants to do something like assign a 
+                // 'BaseType' into 'b'.  As we've now claimed that it must be DerivedType, that 
+                // won't work.  This might also cause unintended changes like changing overload
+                // resolution.  So, we conservatively do not offer the change in a situation like this.
+                return;
+            }
+
             // Looks good!
             var additionalLocations = ImmutableArray.Create(
                 ifStatement.GetLocation(),