Merge pull request #29037 from JoeRobich/fix-cast-is-redundant

Remove spurious "cast is redundant" when accessing hidden member

src/EditorFeatures/CSharpTest/Diagnostics/RemoveUnnecessaryCast/RemoveUnnecessaryCastTests.cs

@@ -4506,6 +4506,30 @@ void M()
         object o = null;
         TypedReference r2 = [|(TypedReference)o|];
     }
+}");
+        }
+
+        [WorkItem(28412, "https://github.com/dotnet/roslyn/issues/28412")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryCast)]
+        public async Task DontOfferToRemoveCastWhenAccessingHiddenProperty()
+        {
+            await TestMissingInRegularAndScriptAsync(@"
+using System.Collections.Generic;
+class Fruit
+{
+    public IDictionary<string, object> Properties { get; set; }
+}
+class Apple : Fruit
+{
+    public new IDictionary<string, object> Properties { get; }
+}
+class Tester
+{
+    public void Test()
+    {
+        var a = new Apple();
+        ([|(Fruit)a|]).Properties[""Color""] = ""Red"";
+    }
 }");
         }
     }

src/EditorFeatures/CSharpTest/Semantics/SpeculationAnalyzerTests.cs

@@ -388,6 +388,106 @@ static void Main(string[] arts)
             ", "1", semanticChanges: true);
         }
 
+        [Fact, WorkItem(28412, "https://github.com/dotnet/roslyn/issues/28412")]
+        public void SpeculationAnalyzerIndexerPropertyWithRedundantCast()
+        {
+            Test(code: @"
+class Indexer
+{
+    public int this[int x] { get { return x; } }
+}
+class A
+{
+    public Indexer Foo { get; } = new Indexer();
+}
+class B : A
+{
+}
+class Program
+{
+    static void Main(string[] args)
+    {
+        var b = new B();
+        var y = ([|(A)b|]).Foo[1];
+    }
+}
+", replacementExpression: "b", semanticChanges: false);
+        }
+
+        [Fact, WorkItem(28412, "https://github.com/dotnet/roslyn/issues/28412")]
+        public void SpeculationAnalyzerIndexerPropertyWithRequiredCast()
+        {
+            Test(code: @"
+class Indexer
+{
+    public int this[int x] { get { return x; } }
+}
+class A
+{
+    public Indexer Foo { get; } = new Indexer();
+}
+class B : A
+{
+    public new Indexer Foo { get; } = new Indexer();
+}
+class Program
+{
+    static void Main(string[] args)
+    {
+        var b = new B();
+        var y = ([|(A)b|]).Foo[1];
+    }
+}
+", replacementExpression: "b", semanticChanges: true);
+        }
+
+        [Fact, WorkItem(28412, "https://github.com/dotnet/roslyn/issues/28412")]
+        public void SpeculationAnalyzerDelegatePropertyWithRedundantCast()
+        {
+            Test(code: @"
+public delegate void MyDelegate();
+class A
+{
+    public MyDelegate Foo { get; }
+}
+class B : A
+{
+}
+class Program
+{
+    static void Main(string[] args)
+    {
+        var b = new B();
+        ([|(A)b|]).Foo();
+    }
+}
+", replacementExpression: "b", semanticChanges: false);
+        }
+
+        [Fact, WorkItem(28412, "https://github.com/dotnet/roslyn/issues/28412")]
+        public void SpeculationAnalyzerDelegatePropertyWithRequiredCast()
+        {
+            Test(code: @"
+public delegate void MyDelegate();
+class A
+{
+    public MyDelegate Foo { get; }
+}
+class B : A
+{
+    public new MyDelegate Foo { get; }
+}
+class Program
+{
+    static void Main(string[] args)
+    {
+        var b = new B();
+        ([|(A)b|]).Foo();
+    }
+}
+", replacementExpression: "b", semanticChanges: true);
+        }
+
         protected override SyntaxTree Parse(string text)
         {
             return SyntaxFactory.ParseSyntaxTree(text);

src/EditorFeatures/VisualBasicTest/Diagnostics/RemoveUnnecessaryCast/RemoveUnnecessaryCastTests.vb

@@ -2776,5 +2776,27 @@ End Structure
 </File>
             Await TestMissingAsync(markup)
         End Function
+
+        <WorkItem(11008, "https://github.com/dotnet/roslyn/issues/11008#issuecomment-230786838")>
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryCast)>
+        Public Async Function DontOfferToRemoveCastWhenAccessingHiddenProperty() As Task
+            Await TestMissingInRegularAndScriptAsync(
+<Code>
+Imports System.Collections.Generic
+Class Fruit
+    Public Property Properties As IDictionary(Of String, Object)
+End Class
+Class Apple
+    Inherits Fruit
+    Public Shadows Property Properties As IDictionary(Of String, Object)
+End Class
+Class Tester
+    Public Sub Test()
+        Dim a = New Apple()
+        [|CType(a, Fruit)|].Properties(""Color"") = ""Red""
+    End Sub
+End Class
+</Code>.Value)
+        End Function
     End Class
 End Namespace

src/EditorFeatures/VisualBasicTest/Semantics/SpeculationAnalyzerTests.vb

@@ -26,6 +26,96 @@ End Module
             </Code>.Value, "Vain(5)", False)
         End Sub
 
+        <Fact, WorkItem(28412, "https://github.com/dotnet/roslyn/issues/28412")>
+        Public Sub SpeculationAnalyzerIndexerPropertyWithRedundantCast()
+            Test(<Code>
+Class Indexer
+    Default Public ReadOnly Property Item(ByVal x As Integer) As Integer
+        Get
+            Return x
+        End Get
+    End Property
+End Class
+Class A
+    Public ReadOnly Property Foo As Indexer
+End Class
+Class B
+    Inherits A
+End Class
+Class Program
+    Sub Main()
+        Dim b As B = New B()
+        Dim y As Integer = [|DirectCast(b, A)|].Foo(2)
+    End Sub
+End Class
+            </Code>.Value, "b", False)
+        End Sub
+
+        <Fact, WorkItem(28412, "https://github.com/dotnet/roslyn/issues/28412")>
+        Public Sub SpeculationAnalyzerIndexerPropertyWithRequiredCast()
+            Test(<Code>
+Class Indexer
+    Default Public ReadOnly Property Item(ByVal x As Integer) As Integer
+        Get
+            Return x
+        End Get
+    End Property
+End Class
+Class A
+    Public ReadOnly Property Foo As Indexer
+End Class
+Class B
+    Inherits A
+    Public Shadows ReadOnly Property Foo As Indexer
+End Class
+Class Program
+    Sub Main()
+        Dim b As B = New B()
+        Dim y As Integer = [|DirectCast(b, A)|].Foo(2)
+    End Sub
+End Class
+            </Code>.Value, "b", True)
+        End Sub
+
+        <Fact, WorkItem(28412, "https://github.com/dotnet/roslyn/issues/28412")>
+        Public Sub SpeculationAnalyzerDelegatePropertyWithRedundantCast()
+            Test(<Code>
+Public Delegate Sub MyDelegate()
+Class A
+    Public ReadOnly Property Foo As MyDelegate
+End Class
+Class B
+    Inherits A
+End Class
+Class Program
+    Sub Main()
+        Dim b As B = New B()
+        [|DirectCast(b, A)|].Foo.Invoke()
+    End Sub
+End Class
+            </Code>.Value, "b", False)
+        End Sub
+
+        <Fact, WorkItem(28412, "https://github.com/dotnet/roslyn/issues/28412")>
+        Public Sub SpeculationAnalyzerDelegatePropertyWithRequiredCast()
+            Test(<Code>
+Public Delegate Sub MyDelegate()
+Class A
+    Public ReadOnly Property Foo As MyDelegate
+End Class
+Class B
+    Inherits A
+    Public Shadows ReadOnly Property Foo As MyDelegate
+End Class
+Class Program
+    Sub Main()
+        Dim b As B = New B()
+        [|DirectCast(b, A)|].Foo.Invoke()
+    End Sub
+End Class
+            </Code>.Value, "b", True)
+        End Sub
+
         Protected Overrides Function Parse(text As String) As SyntaxTree
             Return SyntaxFactory.ParseSyntaxTree(text)
         End Function

src/Workspaces/CSharp/Portable/Utilities/SpeculationAnalyzer.cs

@@ -508,24 +508,11 @@ private bool ReplacementBreaksCollectionInitializerAddMethod(ExpressionSyntax or
             return !SymbolsAreCompatible(originalSymbol, newSymbol);
         }
 
-        protected override bool IsInvocableExpression(SyntaxNode node)
+        protected override bool ExpressionMightReferenceMember(SyntaxNode node)
         {
-            if (node.IsKind(SyntaxKind.InvocationExpression) ||
-                node.IsKind(SyntaxKind.ObjectCreationExpression) ||
-                node.IsKind(SyntaxKind.ElementAccessExpression))
-            {
-                return true;
-            }
-
-            if (node.IsKind(SyntaxKind.SimpleMemberAccessExpression) &&
-                !node.IsParentKind(SyntaxKind.InvocationExpression) &&
-                !node.IsParentKind(SyntaxKind.ObjectCreationExpression) &&
-                !node.IsParentKind(SyntaxKind.ElementAccessExpression))
-            {
-                return true;
-            }
-
-            return false;
+            return node.IsKind(SyntaxKind.InvocationExpression) ||
+                node.IsKind(SyntaxKind.ElementAccessExpression) ||
+                node.IsKind(SyntaxKind.SimpleMemberAccessExpression);
         }
 
         protected override ImmutableArray<ArgumentSyntax> GetArguments(ExpressionSyntax expression)
@@ -533,7 +520,7 @@ protected override ImmutableArray<ArgumentSyntax> GetArguments(ExpressionSyntax
             var argumentsList = GetArgumentList(expression);
             return argumentsList != null ?
                 argumentsList.Arguments.AsImmutableOrEmpty() :
-                ImmutableArray.Create<ArgumentSyntax>();
+                default;
         }
 
         private static BaseArgumentListSyntax GetArgumentList(ExpressionSyntax expression)

src/Workspaces/Core/Portable/Utilities/AbstractSpeculationAnalyzer.cs

@@ -471,12 +471,12 @@ private bool ReplacementChangesSemanticsForNode(SyntaxNode currentOriginalNode,
             Debug.Assert(previousOriginalNode == null || previousOriginalNode.Parent == currentOriginalNode);
             Debug.Assert(previousReplacedNode == null || previousReplacedNode.Parent == currentReplacedNode);
 
-            if (IsInvocableExpression(currentOriginalNode))
+            if (ExpressionMightReferenceMember(currentOriginalNode))
             {
                 // If replacing the node will result in a change in overload resolution, we won't remove it.
                 var originalExpression = (TExpressionSyntax)currentOriginalNode;
                 var newExpression = (TExpressionSyntax)currentReplacedNode;
-                if (ReplacementBreaksInvocableExpression(originalExpression, newExpression))
+                if (ReplacementBreaksExpression(originalExpression, newExpression))
                 {
                     return true;
                 }
@@ -643,7 +643,7 @@ private bool ReplacementBreaksTypeResolution(TTypeSyntax type, TTypeSyntax newTy
             return symbol != null && !SymbolsAreCompatible(symbol, newSymbol);
         }
 
-        protected abstract bool IsInvocableExpression(SyntaxNode node);
+        protected abstract bool ExpressionMightReferenceMember(SyntaxNode node);
 
         private static bool IsDelegateInvoke(ISymbol symbol)
         {
@@ -658,7 +658,7 @@ private static bool IsAnonymousDelegateInvoke(ISymbol symbol)
                 symbol.ContainingType.IsAnonymousType();
         }
 
-        private bool ReplacementBreaksInvocableExpression(TExpressionSyntax expression, TExpressionSyntax newExpression)
+        private bool ReplacementBreaksExpression(TExpressionSyntax expression, TExpressionSyntax newExpression)
         {
             var originalSymbolInfo = _semanticModel.GetSymbolInfo(expression);
             if (_failOnOverloadResolutionFailuresInOriginalCode && originalSymbolInfo.CandidateReason == CandidateReason.OverloadResolutionFailure)
@@ -754,8 +754,8 @@ private bool ReplacementBreaksInvocableExpression(TExpressionSyntax expression,
         private bool IsCompatibleInterfaceMemberImplementation(
             ISymbol symbol,
             ISymbol newSymbol,
-            TExpressionSyntax originalInvocationExpression,
-            TExpressionSyntax newInvocationExpression,
+            TExpressionSyntax originalExpression,
+            TExpressionSyntax newExpression,
             SemanticModel speculativeSemanticModel)
         {
             // If this is an interface member, we have to be careful. In general,
@@ -783,7 +783,7 @@ private bool ReplacementBreaksInvocableExpression(TExpressionSyntax expression,
                 return false;
             }
 
-            var newReceiver = GetReceiver(newInvocationExpression);
+            var newReceiver = GetReceiver(newExpression);
             ITypeSymbol newReceiverType = newReceiver != null ?
                 speculativeSemanticModel.GetTypeInfo(newReceiver).ConvertedType :
                 newSymbolContainingType;
@@ -823,7 +823,7 @@ private bool ReplacementBreaksInvocableExpression(TExpressionSyntax expression,
                 return false;
             }
 
-            return SymbolsHaveCompatibleParameterLists(symbol, implementationMember, originalInvocationExpression);
+            return SymbolsHaveCompatibleParameterLists(symbol, implementationMember, originalExpression);
         }
 
         private static bool IsEffectivelySealedClass(ITypeSymbol type)
@@ -901,7 +901,7 @@ private bool SymbolsHaveCompatibleParameterLists(ISymbol originalSymbol, ISymbol
             if (originalSymbol.IsKind(SymbolKind.Method) || originalSymbol.IsIndexer())
             {
                 var specifiedArguments = GetArguments(originalInvocation);
-                if (specifiedArguments != null)
+                if (!specifiedArguments.IsDefault)
                 {
                     var symbolParameters = originalSymbol.GetParameters();
                     var newSymbolParameters = newSymbol.GetParameters();

src/Workspaces/VisualBasic/Portable/Utilities/SpeculationAnalyzer.vb

@@ -431,18 +431,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
             Return forEachControlVariable IsNot Nothing AndAlso forEachControlVariable.IsTypeInferred(Me.OriginalSemanticModel)
         End Function
 
-        Protected Overrides Function IsInvocableExpression(node As SyntaxNode) As Boolean
-            If node.IsKind(SyntaxKind.InvocationExpression) OrElse node.IsKind(SyntaxKind.ObjectCreationExpression) Then
-                Return True
-            End If
-
-            If node.IsKind(SyntaxKind.SimpleMemberAccessExpression) AndAlso
-                Not node.IsParentKind(SyntaxKind.InvocationExpression) AndAlso
-                Not node.IsParentKind(SyntaxKind.ObjectCreationExpression) Then
-                Return True
-            End If
-
-            Return False
+        Protected Overrides Function ExpressionMightReferenceMember(node As SyntaxNode) As Boolean
+            Return node.IsKind(SyntaxKind.InvocationExpression) OrElse
+                node.IsKind(SyntaxKind.SimpleMemberAccessExpression)
         End Function
 
         Protected Overrides Function GetReceiver(expression As ExpressionSyntax) As ExpressionSyntax
@@ -477,7 +468,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
             Dim argumentList = GetArgumentList(expression)
             Return If(argumentList IsNot Nothing,
                       argumentList.Arguments.AsImmutable(),
-                      ImmutableArray.Create(Of ArgumentSyntax)())
+                      Nothing)
         End Function
 
         Private Shared Function GetArgumentList(expression As ExpressionSyntax) As ArgumentListSyntax
@@ -544,7 +535,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
 
             Me.GetConversions(originalExpression, originalTargetType, newExpression, newTargetType, originalConversion, newConversion)
 
-            If originalConversion Is Nothing OrElse newConversion Is Nothing
+            If originalConversion Is Nothing OrElse newConversion Is Nothing Then
                 Return False
             End If