Fix converted nullability on extract method (#39134)

Fix to use ConvertedNullability. Remove type for object check since it would cause incorrect behavior for casting of object to another type. Make sure to get type info of the cast expression, not the expression type
dotnet · Oct 14, 2019 · ba47b2f · ba47b2f
1 parent f6b62a3
commit ba47b2f
Show file tree

Hide file tree

Showing 3 changed files with 312 additions and 4 deletions.
diff --git a/src/EditorFeatures/CSharpTest/CodeActions/ExtractMethod/ExtractMethodTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/ExtractMethod/ExtractMethodTests.cs
@@ -2815,5 +2815,313 @@ private void NewMethod()
     }
 }");
         }
+
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        public Task TestExtractNullableObjectWithExplicitCast()
+        => TestInRegularAndScriptAsync(
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = null;
+        var s = (string?)[|o|];
+        Console.WriteLine(s);
+    }
+}",
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = null;
+        var s = (string?){|Rename:GetO|}(o);
+        Console.WriteLine(s);
+    }
+
+    private static object? GetO(object? o)
+    {
+        return o;
+    }
+}");
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        public Task TestExtractNotNullableObjectWithExplicitCast()
+        => TestInRegularAndScriptAsync(
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = new object();
+        var s = (string)[|o|];
+        Console.WriteLine(s);
+    }
+}",
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = new object();
+        var s = (string){|Rename:GetO|}(o);
+        Console.WriteLine(s);
+    }
+
+    private static object GetO(object o)
+    {
+        return o;
+    }
+}");
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        public Task TestExtractNotNullableWithExplicitCast()
+        => TestInRegularAndScriptAsync(
+@"#nullable enable
+
+using System;
+
+class A
+{
+}
+
+class B : A 
+{
+}
+
+class C
+{
+    void M()
+    {
+        B? b = new B();
+        var s = (A)[|b|];
+    }
+}",
+@"#nullable enable
+
+using System;
+
+class A
+{
+}
+
+class B : A 
+{
+}
+
+class C
+{
+    void M()
+    {
+        B? b = new B();
+        var s = (A){|Rename:GetB|}(b);
+    }
+
+    private static B GetB(B b)
+    {
+        return b;
+    }
+}");
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        public Task TestExtractNullableWithExplicitCast()
+        => TestInRegularAndScriptAsync(
+@"#nullable enable
+
+using System;
+
+class A
+{
+}
+
+class B : A 
+{
+}
+
+class C
+{
+    void M()
+    {
+        B? b = null;
+        var s = (A)[|b|];
+    }
+}",
+@"#nullable enable
+
+using System;
+
+class A
+{
+}
+
+class B : A 
+{
+}
+
+class C
+{
+    void M()
+    {
+        B? b = null;
+        var s = (A){|Rename:GetB|}(b);
+    }
+
+    private static B? GetB(B? b)
+    {
+        return b;
+    }
+}");
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        public Task TestExtractNotNullableWithExplicitCastSelected()
+        => TestInRegularAndScriptAsync(
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = new object();
+        var s = [|(string)o|];
+        Console.WriteLine(s);
+    }
+}",
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = new object();
+        var s = {|Rename:GetS|}(o);
+        Console.WriteLine(s);
+    }
+
+    private static string GetS(object o)
+    {
+        return (string)o;
+    }
+}");
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        public Task TestExtractNullableWithExplicitCastSelected()
+        => TestInRegularAndScriptAsync(
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = null;
+        var s = [|(string?)o|];
+        Console.WriteLine(s);
+    }
+}",
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = null;
+        var s = {|Rename:GetS|}(o);
+        Console.WriteLine(s);
+    }
+
+    private static string? GetS(object? o)
+    {
+        return (string?)o;
+    }
+}");
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        public Task TestExtractNullableNonNullFlowWithExplicitCastSelected()
+        => TestInRegularAndScriptAsync(
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = new object();
+        var s = [|(string?)o|];
+        Console.WriteLine(s);
+    }
+}",
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = new object();
+        var s = {|Rename:GetS|}(o);
+        Console.WriteLine(s);
+    }
+
+    private static string? GetS(object o)
+    {
+        return (string?)o;
+    }
+}");
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
+        public Task TestExtractNullableToNonNullableWithExplicitCastSelected()
+        => TestInRegularAndScriptAsync(
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = null;
+        var s = [|(string)o|];
+        Console.WriteLine(s);
+    }
+}",
+@"#nullable enable
+
+using System;
+
+class C
+{
+    void M()
+    {
+        object? o = null;
+        var s = {|Rename:GetS|}(o);
+        Console.WriteLine(s);
+    }
+
+    private static string? GetS(object? o)
+    {
+        return (string)o;
+    }
+}");
     }
 }
diff --git a/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.ExpressionResult.cs b/src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.ExpressionResult.cs
@@ -74,14 +74,14 @@ public override ITypeSymbol GetContainingScopeType()
                     // 2. if it doesn't, even if the cast itself wasn't included in the selection, we will treat it 
                     //    as it was in the selection
                     var regularType = GetRegularExpressionType(model, node);
-                    if (regularType != null && !regularType.IsObjectType())
+                    if (regularType != null)
                     {
                         return regularType;
                     }
 
                     if (node.Parent is CastExpressionSyntax castExpression)
                     {
-                        return model.GetTypeInfo(castExpression.Type).GetTypeWithAnnotatedNullability();
+                        return model.GetTypeInfo(castExpression).GetTypeWithFlowNullability();
                     }
                 }
 

diff --git a/src/Workspaces/Core/Portable/Utilities/NullableHelpers/NullableExtensions.cs b/src/Workspaces/Core/Portable/Utilities/NullableHelpers/NullableExtensions.cs
@@ -79,10 +79,10 @@ public static T WithoutNullability<T>(this T typeSymbol) where T : INamespaceOrT
         }
 
         public static ITypeSymbol GetConvertedTypeWithFlowNullability(this TypeInfo typeInfo)
-            => typeInfo.ConvertedType?.WithNullability(typeInfo.Nullability.FlowState);
+            => typeInfo.ConvertedType?.WithNullability(typeInfo.ConvertedNullability.FlowState);
 
         public static ITypeSymbol GetConvertedTypeWithAnnotatedNullability(this TypeInfo typeInfo)
-            => typeInfo.ConvertedType?.WithNullability(typeInfo.Nullability.Annotation);
+            => typeInfo.ConvertedType?.WithNullability(typeInfo.ConvertedNullability.Annotation);
 
         public static ITypeSymbol GetTypeWithFlowNullability(this TypeInfo typeInfo)
             => typeInfo.Type?.WithNullability(typeInfo.Nullability.FlowState);