Prevent overrides and overloads appearing on the same property - fixes …

…#681
icsharpcode · May 25, 2021 · acaadcb · acaadcb
1 parent 1817d3a
commit acaadcb
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,17 +10,20 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ### VB -> C#
 
+* Prevent overrides and overloads appearing on the same property (https://github.com/icsharpcode/CodeConverter/issues/681)
 
 ### C# -> VB
 
 
 ## [8.2.4] - 2021-05-04
 
 ### Command line
+
 * The --Core-Only flag no longer requires a value [#704](https://github.com/icsharpcode/CodeConverter/issues/704)
 
 
 ### VB -> C#
+
 * Deal with NullReferenceException caused by Nothing literal [#707](https://github.com/icsharpcode/CodeConverter/issues/707)
 * Include text of region which can't be converted
 * Convert region names

diff --git a/CodeConverter/CSharp/CommonConversions.cs b/CodeConverter/CSharp/CommonConversions.cs
@@ -349,11 +349,13 @@ public SyntaxTokenList ConvertModifiers(SyntaxNode node, IReadOnlyCollection<Syn
 
         private static bool? RequiresNewKeyword(ISymbol declaredSymbol)
         {
-            if (!(declaredSymbol is IMethodSymbol methodSymbol)) return null;
-            if (declaredSymbol.IsOverride ) return false;
-            var methodSignature = methodSymbol.GetUnqualifiedMethodSignature(true);
-            return declaredSymbol.ContainingType.FollowProperty(s => s.BaseType).Skip(1).Any(t => t.GetMembers()
-                .Any(s => s.Name == declaredSymbol.Name && s is IMethodSymbol m && m.GetUnqualifiedMethodSignature(true) == methodSignature));
+            if (declaredSymbol.IsOverride) return false;
+            if (declaredSymbol is IPropertySymbol propertySymbol || declaredSymbol is IMethodSymbol methodSymbol) {
+                var methodSignature = declaredSymbol.GetUnqualifiedMethodOrPropertySignature(true);
+                return declaredSymbol.ContainingType.FollowProperty(s => s.BaseType).Skip(1).Any(t => t.GetMembers()
+                    .Any(s => s.Name == declaredSymbol.Name && (s is IPropertySymbol || s is IMethodSymbol) && s.GetUnqualifiedMethodOrPropertySignature(true) == methodSignature));
+            }
+            return null;
         }
 
         private static bool ContextHasIdenticalDefaults(TokenContext context, TokenContext[] contextsWithIdenticalDefaults, ISymbol declaredSymbol)

diff --git a/CodeConverter/Util/IMethodSymbolExtensions.cs b/CodeConverter/Util/IMethodSymbolExtensions.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Linq;
 using ICSharpCode.CodeConverter.Util.FromRoslyn;
 using Microsoft.CodeAnalysis;
@@ -6,19 +7,22 @@ namespace ICSharpCode.CodeConverter.Util
 {
     internal static class IMethodSymbolExtensions
     {
-        public static string GetParameterSignature(this IMethodSymbol methodSymbol)
-        {
-            return string.Join(" ", methodSymbol.Parameters.Select(p => p.Type));
-        }
+        public static string GetParameterSignature(this IMethodSymbol methodSymbol) => string.Join(" ", methodSymbol.Parameters.Select(p => p.Type));
+        public static string GetParameterSignature(this IPropertySymbol propertySymbol) => string.Join(" ", propertySymbol.Parameters.Select(p => p.Type));
 
-        public static (string Name, int TypeParameterCount, string ParameterTypes) GetUnqualifiedMethodSignature(this IMethodSymbol methodSymbol, bool caseSensitiveName)
-        {
-            return (caseSensitiveName ? methodSymbol.Name : methodSymbol.Name.ToLowerInvariant() , methodSymbol.TypeParameters.Length, GetParameterSignature(methodSymbol));
-        }
+        public static (string Name, int TypeParameterCount, string ParameterTypes) GetUnqualifiedMethodOrPropertySignature(this ISymbol s, bool caseSensitiveName) => s switch {
+            IMethodSymbol m => m.GetUnqualifiedMethodSignature(caseSensitiveName),
+            IPropertySymbol p => p.GetUnqualifiedPropertySignature(caseSensitiveName),
+            _ => throw new ArgumentOutOfRangeException(nameof(s), s, $"Symbol must be property or method, but was {s.Kind}")
+        };
 
-        public static bool ReturnsVoidOrAsyncTask(this IMethodSymbol enclosingMethodInfo)
-        {
-            return enclosingMethodInfo.ReturnsVoid || enclosingMethodInfo.IsAsync && enclosingMethodInfo.ReturnType.GetArity() == 0;
-        }
+        public static (string Name, int TypeParameterCount, string ParameterTypes) GetUnqualifiedMethodSignature(this IMethodSymbol methodSymbol, bool caseSensitiveName) =>
+            (caseSensitiveName ? methodSymbol.Name : methodSymbol.Name.ToLowerInvariant(), methodSymbol.TypeParameters.Length, GetParameterSignature(methodSymbol));
+
+        public static (string Name, int TypeParameterCount, string ParameterTypes) GetUnqualifiedPropertySignature(this IPropertySymbol propertySymbol, bool caseSensitiveName) =>
+            (caseSensitiveName ? propertySymbol.Name : propertySymbol.Name.ToLowerInvariant(), 0, GetParameterSignature(propertySymbol));
+
+        public static bool ReturnsVoidOrAsyncTask(this IMethodSymbol enclosingMethodInfo) =>
+            enclosingMethodInfo.ReturnsVoid || enclosingMethodInfo.IsAsync && enclosingMethodInfo.ReturnType.GetArity() == 0;
     }
 }
diff --git a/Tests/CSharp/MemberTests/MemberTests.cs b/Tests/CSharp/MemberTests/MemberTests.cs
@@ -630,6 +630,50 @@ internal partial class TestClass
 }");
         }
 
+        [Fact]
+        public async Task Issue681_OverloadsOverridesPropertyAsync()
+        {
+            await TestConversionVisualBasicToCSharpAsync(
+@"Public Class C 
+    Inherits B
+
+    Public ReadOnly Overloads Overrides Property X()
+        Get
+            Return Nothing
+        End Get
+    End Property
+End Class
+
+Public Class B
+    Public ReadOnly Overridable Property X()
+        Get
+            Return Nothing
+        End Get
+    End Property
+End Class", @"
+public partial class C : B
+{
+    public override object X
+    {
+        get
+        {
+            return null;
+        }
+    }
+}
+
+public partial class B
+{
+    public virtual object X
+    {
+        get
+        {
+            return null;
+        }
+    }
+}");
+        }
+
         [Fact]
         public async Task PartialFriendClassWithOverloadsAsync()
         {