From b3ad1758b3636dcaccf89ed7c8340f0268e87e94 Mon Sep 17 00:00:00 2001 From: John Moreno Date: Thu, 17 Jul 2025 23:29:53 -0400 Subject: [PATCH 1/6] VB -> C#: private partial methods do not compile - Added unit test to SpecialConversionTests (not MethodStatementTests as there is no statements) -ISymbolExtensions.cs - CanHaveMethodBody & IsPartialMethodDefinition - Added check for IsPartialDefinition #1097 --- CodeConverter/Util/ISymbolExtensions.cs | 4 ++-- Tests/CSharp/SpecialConversionTests.cs | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CodeConverter/Util/ISymbolExtensions.cs b/CodeConverter/Util/ISymbolExtensions.cs index ad910b0c6..22b759945 100644 --- a/CodeConverter/Util/ISymbolExtensions.cs +++ b/CodeConverter/Util/ISymbolExtensions.cs @@ -75,12 +75,12 @@ public static bool IsPartialMethodImplementation(this ISymbol declaredSymbol) public static bool CanHaveMethodBody(this ISymbol declaredSymbol) { - return !(declaredSymbol is IMethodSymbol ms) || ms.PartialImplementationPart == null && !ms.IsExtern; + return !(declaredSymbol is IMethodSymbol ms) || (!ms.IsPartialDefinition && (ms.PartialImplementationPart == null && !ms.IsExtern)); } public static bool IsPartialMethodDefinition(this ISymbol declaredSymbol) { - return declaredSymbol is IMethodSymbol ms && ms.PartialImplementationPart != null; + return declaredSymbol is IMethodSymbol ms && (ms.PartialImplementationPart != null || ms.IsPartialDefinition); } public static bool IsPartialClassDefinition(this ISymbol declaredSymbol) diff --git a/Tests/CSharp/SpecialConversionTests.cs b/Tests/CSharp/SpecialConversionTests.cs index 9b0713ca1..9e2543079 100644 --- a/Tests/CSharp/SpecialConversionTests.cs +++ b/Tests/CSharp/SpecialConversionTests.cs @@ -402,4 +402,11 @@ End If } }"); } + + [Fact] + public async Task Issue1097_PartialMethodAsync() + { + await TestConversionVisualBasicToCSharpAsync(@"Partial Private Sub DummyMethod() + End Sub", @"partial void DummyMethod();"); + } } \ No newline at end of file From d384eaa595dd63aac922e6329183dcf974a671f5 Mon Sep 17 00:00:00 2001 From: Graham Date: Sun, 3 Aug 2025 17:55:50 +0100 Subject: [PATCH 2/6] The method shouldn't say non-methods/nulls can have method bodies, knowing what to do in the edge case is the caller's responsibility --- CodeConverter/CSharp/DeclarationNodeVisitor.cs | 5 +++-- CodeConverter/Util/ISymbolExtensions.cs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CodeConverter/CSharp/DeclarationNodeVisitor.cs b/CodeConverter/CSharp/DeclarationNodeVisitor.cs index 1863a4e0c..6e8554710 100644 --- a/CodeConverter/CSharp/DeclarationNodeVisitor.cs +++ b/CodeConverter/CSharp/DeclarationNodeVisitor.cs @@ -655,7 +655,7 @@ public override async Task VisitMethodBlock(VBSyntax.MethodBlo var methodBlock = await node.SubOrFunctionStatement.AcceptAsync(TriviaConvertingDeclarationVisitor, SourceTriviaMapKind.SubNodesOnly); var declaredSymbol = ModelExtensions.GetDeclaredSymbol(_semanticModel, node); - if (!declaredSymbol.CanHaveMethodBody()) { + if (declaredSymbol?.CanHaveMethodBody() == false) { return methodBlock; } var csReturnVariableOrNull = CommonConversions.GetRetVariableNameOrNull(node); @@ -786,7 +786,8 @@ public override async Task VisitMethodStatement(VBSyntax.Metho null ); - return hasBody && declaredSymbol.CanHaveMethodBody() ? decl : decl.WithSemicolonToken(SemicolonToken); + bool canHaveMethodBody = declaredSymbol?.CanHaveMethodBody() != false; + return hasBody && canHaveMethodBody ? decl : decl.WithSemicolonToken(SemicolonToken); } public override async Task VisitEventBlock(VBSyntax.EventBlockSyntax node) diff --git a/CodeConverter/Util/ISymbolExtensions.cs b/CodeConverter/Util/ISymbolExtensions.cs index 22b759945..0aa68c120 100644 --- a/CodeConverter/Util/ISymbolExtensions.cs +++ b/CodeConverter/Util/ISymbolExtensions.cs @@ -75,7 +75,7 @@ public static bool IsPartialMethodImplementation(this ISymbol declaredSymbol) public static bool CanHaveMethodBody(this ISymbol declaredSymbol) { - return !(declaredSymbol is IMethodSymbol ms) || (!ms.IsPartialDefinition && (ms.PartialImplementationPart == null && !ms.IsExtern)); + return declaredSymbol is IMethodSymbol ms && (!ms.IsPartialDefinition && (ms.PartialImplementationPart == null && !ms.IsExtern)); } public static bool IsPartialMethodDefinition(this ISymbol declaredSymbol) From 34aa0bd23555592be957a54d5c64aac3d3c9a0d6 Mon Sep 17 00:00:00 2001 From: Graham Date: Sun, 3 Aug 2025 18:06:42 +0100 Subject: [PATCH 3/6] Enable nullable for file --- CodeConverter/Util/ISymbolExtensions.cs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/CodeConverter/Util/ISymbolExtensions.cs b/CodeConverter/Util/ISymbolExtensions.cs index 0aa68c120..a2edf666d 100644 --- a/CodeConverter/Util/ISymbolExtensions.cs +++ b/CodeConverter/Util/ISymbolExtensions.cs @@ -1,3 +1,4 @@ +#nullable enable namespace ICSharpCode.CodeConverter.Util; internal static class ISymbolExtensions @@ -23,7 +24,7 @@ public static bool IsDefinedInSource(this ISymbol symbol) return symbol.Locations.Any(loc => loc.IsInSource); } - public static TSymbol ExtractBestMatch(this SymbolInfo info, Func isMatch = null) where TSymbol : class, ISymbol + public static TSymbol? ExtractBestMatch(this SymbolInfo info, Func? isMatch = null) where TSymbol : class, ISymbol { isMatch ??= (_ => true); if (info.Symbol == null && info.CandidateSymbols.Length == 0) @@ -38,16 +39,16 @@ public static TSymbol ExtractBestMatch(this SymbolInfo info, Func 1 || ts.ContainingAssembly.Name == ForcePartialTypesAssemblyName); } - public static bool IsReducedTypeParameterMethod(this ISymbol symbol) + public static bool IsReducedTypeParameterMethod(this ISymbol? symbol) { return symbol is IMethodSymbol ms && ms.ReducedFrom?.TypeParameters.Length > ms.TypeParameters.Length; } @@ -98,5 +99,5 @@ public static bool IsReducedTypeParameterMethod(this ISymbol symbol) /// Since non value types can't be ref types for extension methods in C#, convert to a static invocation /// https://github.com/icsharpcode/CodeConverter/issues/785 /// - public static bool ValidCSharpExtensionMethodParameter(this IParameterSymbol vbSymbol) => vbSymbol != null && (vbSymbol.RefKind != RefKind.Ref || vbSymbol.Type.IsValueType); + public static bool ValidCSharpExtensionMethodParameter(this IParameterSymbol? vbSymbol) => vbSymbol != null && (vbSymbol.RefKind != RefKind.Ref || vbSymbol.Type.IsValueType); } \ No newline at end of file From 4d19fd8f9fffda3aad4623dc19304facad4d0793 Mon Sep 17 00:00:00 2001 From: Graham Date: Sun, 3 Aug 2025 18:08:23 +0100 Subject: [PATCH 4/6] Dedupe and remove redundant parens --- CodeConverter/Util/ISymbolExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CodeConverter/Util/ISymbolExtensions.cs b/CodeConverter/Util/ISymbolExtensions.cs index a2edf666d..16f32ea5f 100644 --- a/CodeConverter/Util/ISymbolExtensions.cs +++ b/CodeConverter/Util/ISymbolExtensions.cs @@ -76,7 +76,7 @@ public static bool IsPartialMethodImplementation(this ISymbol? declaredSymbol) public static bool CanHaveMethodBody(this ISymbol? declaredSymbol) { - return declaredSymbol is IMethodSymbol ms && (!ms.IsPartialDefinition && (ms.PartialImplementationPart == null && !ms.IsExtern)); + return declaredSymbol is IMethodSymbol ms && !ms.IsExtern && !IsPartialMethodDefinition(declaredSymbol); } public static bool IsPartialMethodDefinition(this ISymbol? declaredSymbol) From 85f058984e73675e6e1dac2d8497a10ae88b3da3 Mon Sep 17 00:00:00 2001 From: Graham Date: Sun, 3 Aug 2025 18:17:27 +0100 Subject: [PATCH 5/6] Modernise style for file --- CodeConverter/Util/ISymbolExtensions.cs | 34 +++++++++---------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/CodeConverter/Util/ISymbolExtensions.cs b/CodeConverter/Util/ISymbolExtensions.cs index 16f32ea5f..c5fe84a13 100644 --- a/CodeConverter/Util/ISymbolExtensions.cs +++ b/CodeConverter/Util/ISymbolExtensions.cs @@ -19,10 +19,7 @@ public static ISymbol GetBaseSymbol(this ISymbol symbol, Func sel : symbol; } - public static bool IsDefinedInSource(this ISymbol symbol) - { - return symbol.Locations.Any(loc => loc.IsInSource); - } + public static bool IsDefinedInSource(this ISymbol symbol) => symbol.Locations.Any(loc => loc.IsInSource); public static TSymbol? ExtractBestMatch(this SymbolInfo info, Func? isMatch = null) where TSymbol : class, ISymbol { @@ -69,31 +66,24 @@ private static bool TryGetSpecialVBTypeConversion(ISymbol symbol, out string? cS return false; } - public static bool IsPartialMethodImplementation(this ISymbol? declaredSymbol) - { - return declaredSymbol is IMethodSymbol {PartialDefinitionPart: not null}; - } + public static bool IsPartialMethodImplementation(this ISymbol? declaredSymbol) => + declaredSymbol is IMethodSymbol {PartialDefinitionPart: not null}; - public static bool CanHaveMethodBody(this ISymbol? declaredSymbol) - { - return declaredSymbol is IMethodSymbol ms && !ms.IsExtern && !IsPartialMethodDefinition(declaredSymbol); - } + public static bool CanHaveMethodBody(this ISymbol? declaredSymbol) => + declaredSymbol is IMethodSymbol {IsExtern: false} && !IsPartialMethodDefinition(declaredSymbol); - public static bool IsPartialMethodDefinition(this ISymbol? declaredSymbol) - { - return declaredSymbol is IMethodSymbol ms && (ms.PartialImplementationPart != null || ms.IsPartialDefinition); - } + public static bool IsPartialMethodDefinition(this ISymbol? declaredSymbol) => + declaredSymbol is IMethodSymbol {PartialImplementationPart: not null} + or IMethodSymbol {IsPartialDefinition: true}; public static bool IsPartialClassDefinition(this ISymbol? declaredSymbol) { - return declaredSymbol is ITypeSymbol ts && (ts.DeclaringSyntaxReferences.Length > 1 - || ts.ContainingAssembly.Name == ForcePartialTypesAssemblyName); + return declaredSymbol is ITypeSymbol {DeclaringSyntaxReferences.Length: > 1} + or ITypeSymbol {ContainingAssembly.Name: ForcePartialTypesAssemblyName}; } - public static bool IsReducedTypeParameterMethod(this ISymbol? symbol) - { - return symbol is IMethodSymbol ms && ms.ReducedFrom?.TypeParameters.Length > ms.TypeParameters.Length; - } + public static bool IsReducedTypeParameterMethod(this ISymbol? symbol) => + symbol is IMethodSymbol ms && ms.ReducedFrom?.TypeParameters.Length > ms.TypeParameters.Length; /// /// Since non value types can't be ref types for extension methods in C#, convert to a static invocation From 81ff6353dafe74efbf02d252319817109f423339 Mon Sep 17 00:00:00 2001 From: Graham Date: Sun, 3 Aug 2025 18:23:33 +0100 Subject: [PATCH 6/6] Move the test to similar test --- Tests/CSharp/MemberTests/MemberTests.cs | 7 +++++++ Tests/CSharp/SpecialConversionTests.cs | 7 ------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Tests/CSharp/MemberTests/MemberTests.cs b/Tests/CSharp/MemberTests/MemberTests.cs index 8b7c94ce2..c43138a57 100644 --- a/Tests/CSharp/MemberTests/MemberTests.cs +++ b/Tests/CSharp/MemberTests/MemberTests.cs @@ -921,6 +921,13 @@ public partial class TestClass // VB doesn't require partial here (when just a s }"); } + [Fact] + public async Task Issue1097_PartialMethodAsync() + { + await TestConversionVisualBasicToCSharpAsync(@"Partial Private Sub DummyMethod() + End Sub", @"partial void DummyMethod();"); + } + [Fact] public async Task NestedClassAsync() { diff --git a/Tests/CSharp/SpecialConversionTests.cs b/Tests/CSharp/SpecialConversionTests.cs index 9e2543079..9b0713ca1 100644 --- a/Tests/CSharp/SpecialConversionTests.cs +++ b/Tests/CSharp/SpecialConversionTests.cs @@ -402,11 +402,4 @@ End If } }"); } - - [Fact] - public async Task Issue1097_PartialMethodAsync() - { - await TestConversionVisualBasicToCSharpAsync(@"Partial Private Sub DummyMethod() - End Sub", @"partial void DummyMethod();"); - } } \ No newline at end of file