From e654f2611ef927aa6c6c0626abd57d3eed5c7fbc Mon Sep 17 00:00:00 2001 From: gherards99 Date: Fri, 4 Sep 2026 14:47:55 +0200 Subject: [PATCH] Support the tractable C# patterns when converting to VB VisitIsPatternExpression handled only declaration and constant patterns and threw for everything else, so most pattern matching failed to convert. It is now split into a recursive ConvertPattern that also covers: * type patterns, as TypeOf x Is T * negated patterns, flipped in place to TypeOf x IsNot T or TryCast(x, T) Is Nothing rather than wrapped in Not * relational patterns, as plain comparisons A constant pattern whose expression binds to a type is a type test, not a comparison, and was being emitted as Is, which compares references in VB. It now emits TypeOf x Is T. A constant compared by value went the same way and did not compile at all; it now uses = for a value type and Equals for anything else, which is what the pattern means when the operand is an Object. "x is not T v" declares v just as "x is T v" does, so the hoisting in CommonConversions follows the negation to find it. Also here, both smaller and separable: * ImplicitObjectCreationExpression had no visitor, so target-typed new threw. The type the compiler settled on is written out instead. * GetSymbolInfo(node.Left).Symbol was dereferenced without a null check while converting an event subscription, so an unresolved symbol crashed the whole file rather than degrading. * A failed expression conversion comes back as an empty statement carrying its report as trivia. A return statement dropped both, losing the value and the diagnostic together; the report now travels with the statement. Property patterns and ??= are deliberately left out: both need the tested expression evaluated once, which needs a temporary rather than repeating it. Co-Authored-By: Claude Opus 5 --- CodeConverter/VB/CommonConversions.cs | 15 +- .../MethodBodyExecutableStatementVisitor.cs | 22 ++- CodeConverter/VB/NodesVisitor.cs | 111 +++++++++++++- Tests/VB/ExpressionTests.cs | 142 ++++++++++++++++++ 4 files changed, 275 insertions(+), 15 deletions(-) diff --git a/CodeConverter/VB/CommonConversions.cs b/CodeConverter/VB/CommonConversions.cs index a150aea91..d07ac9098 100644 --- a/CodeConverter/VB/CommonConversions.cs +++ b/CodeConverter/VB/CommonConversions.cs @@ -157,7 +157,12 @@ private VariableDeclaratorSyntax ConvertToVariableDeclarator(CSSyntax.Declaratio private VariableDeclaratorSyntax ConvertToVariableDeclaratorOrNull(CSSyntax.IsPatternExpressionSyntax node) { - switch (node.Pattern) { + return ConvertPatternToVariableDeclaratorOrNull(node.Pattern); + } + + private VariableDeclaratorSyntax ConvertPatternToVariableDeclaratorOrNull(CSSyntax.PatternSyntax pattern) + { + switch (pattern) { case CSSyntax.DeclarationPatternSyntax d: { var id = ((IdentifierNameSyntax)d.Designation.Accept(_nodesVisitor)).Identifier; var ids = SyntaxFactory.SingletonSeparatedList(SyntaxFactory.ModifiedIdentifier(id)); @@ -169,10 +174,12 @@ private VariableDeclaratorSyntax ConvertToVariableDeclaratorOrNull(CSSyntax.IsPa SyntaxFactory.Token(SyntaxKind.NothingKeyword))); return SyntaxFactory.VariableDeclarator(ids, simpleAsClauseSyntax, equalsValueSyntax); } - case CSSyntax.ConstantPatternSyntax _: - return null; + case CSSyntax.UnaryPatternSyntax u: + // "x is not T v" declares v just the same, so the negated pattern is hoisted too + return ConvertPatternToVariableDeclaratorOrNull(u.Pattern); default: - throw new ArgumentOutOfRangeException(nameof(node), node.Pattern, null); + // no other pattern introduces a variable, and the caller already filters nulls out + return null; } } diff --git a/CodeConverter/VB/MethodBodyExecutableStatementVisitor.cs b/CodeConverter/VB/MethodBodyExecutableStatementVisitor.cs index 58c9cabd1..8e650a474 100644 --- a/CodeConverter/VB/MethodBodyExecutableStatementVisitor.cs +++ b/CodeConverter/VB/MethodBodyExecutableStatementVisitor.cs @@ -572,11 +572,23 @@ public override SyntaxList VisitReturnStatement(CSSyntax.Return private static StatementSyntax ReturnStatement(VisualBasicSyntaxNode vbExpression) { - return vbExpression == null - ? SyntaxFactory.ReturnStatement() - : vbExpression.IsKind(SyntaxKind.EmptyStatement) - ? SyntaxFactory.ReturnStatement() - : SyntaxFactory.ReturnStatement((ExpressionSyntax)vbExpression); + if (vbExpression == null) { + return SyntaxFactory.ReturnStatement(); + } + + // A failed conversion comes back as an empty statement carrying the report as trivia. The + // expression cannot be returned, but the report has to travel with the statement or the + // return silently loses its value and nothing says so. It goes ahead of the statement + // because a comment runs to the end of its line and would otherwise swallow the Return. + if (vbExpression.IsKind(SyntaxKind.EmptyStatement)) { + var report = vbExpression.GetLeadingTrivia() + .AddRange(vbExpression.GetTrailingTrivia()) + .Add(SyntaxFactory.CarriageReturnLineFeed); + + return SyntaxFactory.ReturnStatement().WithLeadingTrivia(report); + } + + return SyntaxFactory.ReturnStatement((ExpressionSyntax)vbExpression); } public override SyntaxList VisitYieldStatement(CSSyntax.YieldStatementSyntax node) diff --git a/CodeConverter/VB/NodesVisitor.cs b/CodeConverter/VB/NodesVisitor.cs index 979c58d2c..f5464813e 100644 --- a/CodeConverter/VB/NodesVisitor.cs +++ b/CodeConverter/VB/NodesVisitor.cs @@ -11,6 +11,7 @@ using ArrayRankSpecifierSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.ArrayRankSpecifierSyntax; using AttributeListSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.AttributeListSyntax; using AttributeSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.AttributeSyntax; +using BinaryExpressionSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.BinaryExpressionSyntax; using ExpressionSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.ExpressionSyntax; using IdentifierNameSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.IdentifierNameSyntax; using InterpolatedStringContentSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.InterpolatedStringContentSyntax; @@ -30,6 +31,7 @@ using SyntaxKind = Microsoft.CodeAnalysis.VisualBasic.SyntaxKind; using TupleElementSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.TupleElementSyntax; using TypeArgumentListSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.TypeArgumentListSyntax; +using TypeOfExpressionSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.TypeOfExpressionSyntax; using TypeParameterConstraintClauseSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.TypeParameterConstraintClauseSyntax; using TypeParameterListSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.TypeParameterListSyntax; using TypeParameterSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.TypeParameterSyntax; @@ -348,7 +350,17 @@ public override VisualBasicSyntaxNode VisitConstructorDeclaration(CSSyntax.Const public override VisualBasicSyntaxNode VisitIsPatternExpression(CSSyntax.IsPatternExpressionSyntax node) { ExpressionSyntax lhs = (ExpressionSyntax)node.Expression.Accept(TriviaConvertingVisitor); - switch (node.Pattern) { + return ConvertPattern(node, lhs, _semanticModel.GetTypeInfo(node.Expression).Type, node.Pattern); + } + + /// + /// Separate from the visitor so that a negated pattern can convert the pattern it wraps. + /// The type of the value under test travels along because a nested pattern is applied to a + /// member of it, and picking the right equality operator depends on that member's type. + /// + private ExpressionSyntax ConvertPattern(CSSyntax.IsPatternExpressionSyntax node, ExpressionSyntax lhs, ITypeSymbol lhsType, CSSyntax.PatternSyntax pattern) + { + switch (pattern) { case CSSyntax.DeclarationPatternSyntax d: { var left = (ExpressionSyntax)d.Designation.Accept(TriviaConvertingVisitor); ExpressionSyntax right = SyntaxFactory.TryCastExpression( @@ -360,11 +372,77 @@ public override VisualBasicSyntaxNode VisitIsPatternExpression(CSSyntax.IsPatter SyntaxFactory.Token(SyntaxKind.NothingKeyword)); return SyntaxFactory.IsNotExpression(tryCast, nothingExpression); } - case CSSyntax.ConstantPatternSyntax cps: - return SyntaxFactory.IsExpression(lhs, - (ExpressionSyntax)cps.Expression.Accept(TriviaConvertingVisitor)); + case CSSyntax.TypePatternSyntax tp: + return SyntaxFactory.TypeOfIsExpression(lhs, (TypeSyntax)tp.Type.Accept(TriviaConvertingVisitor)); + case CSSyntax.UnaryPatternSyntax u when u.OperatorToken.IsKind(CS.SyntaxKind.NotKeyword): + return NegatePattern(ConvertPattern(node, lhs, lhsType, u.Pattern)); + case CSSyntax.RelationalPatternSyntax rel: + return ConvertRelationalPattern(lhs, rel); + case CSSyntax.ConstantPatternSyntax cps: { + // an identifier bound to a type is a type test, not a comparison against a value + if (_semanticModel.GetSymbolInfo(cps.Expression).ExtractBestMatch() is { } typeSymbol) { + return SyntaxFactory.TypeOfIsExpression(lhs, + (TypeSyntax)_commonConversions.VbSyntaxGenerator.TypeExpression(typeSymbol)); + } + + var constant = (ExpressionSyntax)cps.Expression.Accept(TriviaConvertingVisitor); + + if (cps.Expression.IsKind(CS.SyntaxKind.NullLiteralExpression)) { + return SyntaxFactory.IsExpression(lhs, constant); + } + + // A constant pattern compares by value. VB's = does that on a value type, but on an + // Object it goes through late binding and throws where C# would answer False, so + // anything not known to be a value type is compared through Equals instead. + if (lhsType != null && lhsType.IsValueType) { + return SyntaxFactory.EqualsExpression(lhs, constant); + } + + return SyntaxFactory.InvocationExpression(SyntaxFactory.IdentifierName(nameof(Equals)), + ExpressionSyntaxExtensions.CreateArgList(lhs, constant)); + } + default: + throw new ArgumentOutOfRangeException(nameof(node), pattern, null); + } + } + + /// + /// "x is > 0" and its siblings are plain comparisons once the value under test is known. + /// + private ExpressionSyntax ConvertRelationalPattern(ExpressionSyntax lhs, CSSyntax.RelationalPatternSyntax pattern) + { + var value = (ExpressionSyntax)pattern.Expression.Accept(TriviaConvertingVisitor); + var op = pattern.OperatorToken; + + if (op.IsKind(CS.SyntaxKind.GreaterThanToken)) { + return SyntaxFactory.GreaterThanExpression(lhs, value); + } + if (op.IsKind(CS.SyntaxKind.GreaterThanEqualsToken)) { + return SyntaxFactory.GreaterThanOrEqualExpression(lhs, value); + } + if (op.IsKind(CS.SyntaxKind.LessThanToken)) { + return SyntaxFactory.LessThanExpression(lhs, value); + } + if (op.IsKind(CS.SyntaxKind.LessThanEqualsToken)) { + return SyntaxFactory.LessThanOrEqualExpression(lhs, value); + } + + throw new ArgumentOutOfRangeException(nameof(pattern), op.Text, null); + } + + /// + /// Visual Basic can negate a type test and a Nothing test in place, which reads far better + /// than wrapping the whole condition in Not. + /// + private static ExpressionSyntax NegatePattern(ExpressionSyntax converted) + { + switch (converted) { + case TypeOfExpressionSyntax typeOf when typeOf.IsKind(SyntaxKind.TypeOfIsExpression): + return SyntaxFactory.TypeOfIsNotExpression(typeOf.Expression, typeOf.Type); + case BinaryExpressionSyntax binary when binary.IsKind(SyntaxKind.IsNotExpression): + return SyntaxFactory.IsExpression(binary.Left, binary.Right); default: - throw new ArgumentOutOfRangeException(nameof(node), node.Pattern, null); + return SyntaxFactory.NotExpression(SyntaxFactory.ParenthesizedExpression(converted)); } } @@ -991,7 +1069,7 @@ public override VisualBasicSyntaxNode VisitAssignmentExpression(CSSyntax.Assignm var right = (ExpressionSyntax)node.Right.Accept(TriviaConvertingVisitor); if (IsReturnValueDiscarded(node)) { if (_semanticModel.GetTypeInfo(node.Right).ConvertedType.IsDelegateType()) { - if (_semanticModel.GetSymbolInfo(node.Left).Symbol.Kind != SymbolKind.Event) { + if (_semanticModel.GetSymbolInfo(node.Left).Symbol?.Kind != SymbolKind.Event) { var kind = node.GetAncestor()?.Kind(); if (kind != null && (kind.Value == CS.SyntaxKind.AddAccessorDeclaration || kind.Value == CS.SyntaxKind.RemoveAccessorDeclaration)) { var methodName = kind.Value == CS.SyntaxKind.AddAccessorDeclaration ? "Combine" : "Remove"; @@ -1391,6 +1469,27 @@ public override VisualBasicSyntaxNode VisitObjectCreationExpression(CSSyntax.Obj ); } + /// + /// "new(...)" takes its type from what it is assigned to, which Visual Basic cannot do, so the + /// type the compiler settled on is written out. + /// + public override VisualBasicSyntaxNode VisitImplicitObjectCreationExpression(CSSyntax.ImplicitObjectCreationExpressionSyntax node) + { + var typeInfo = _semanticModel.GetTypeInfo(node); + var typeSymbol = typeInfo.Type ?? typeInfo.ConvertedType; + + if (typeSymbol == null) { + throw new NotSupportedException("The type of an implicit object creation could not be resolved."); + } + + return SyntaxFactory.ObjectCreationExpression( + SyntaxFactory.List(), + (TypeSyntax)_commonConversions.VbSyntaxGenerator.TypeExpression(typeSymbol), + (ArgumentListSyntax)node.ArgumentList?.Accept(TriviaConvertingVisitor), + (ObjectCreationInitializerSyntax)node.Initializer?.Accept(TriviaConvertingVisitor) + ); + } + public override VisualBasicSyntaxNode VisitAnonymousObjectCreationExpression(CSSyntax.AnonymousObjectCreationExpressionSyntax node) { return SyntaxFactory.AnonymousObjectCreationExpression( diff --git a/Tests/VB/ExpressionTests.cs b/Tests/VB/ExpressionTests.cs index fd10e5672..aca892413 100644 --- a/Tests/VB/ExpressionTests.cs +++ b/Tests/VB/ExpressionTests.cs @@ -253,6 +253,148 @@ End Class BC30451: 'CSharpImpl.__Assign' is not declared. It may be inaccessible due to its protection level."); } + [Fact] + public async Task NegatedTypePatternExpressionAsync() + { + await TestConversionCSharpToVisualBasicAsync(@"class TestClass +{ + private static bool IsNotString(object node) + { + return node is not string; + } +}", @"Friend Class TestClass + Private Shared Function IsNotString(node As Object) As Boolean + Return TypeOf node IsNot String + End Function +End Class"); + } + + [Fact] + public async Task NegatedNamedTypePatternExpressionAsync() + { + await TestConversionCSharpToVisualBasicAsync(@"class Picture +{ +} + +class TestClass +{ + private static bool IsNotPicture(object node) + { + return node is not Picture; + } +}", @"Friend Class Picture +End Class + +Friend Class TestClass + Private Shared Function IsNotPicture(node As Object) As Boolean + Return TypeOf node IsNot Picture + End Function +End Class"); + } + + [Fact] + public async Task NegatedDeclarationPatternExpressionAsync() + { + await TestConversionCSharpToVisualBasicAsync(@"class TestClass +{ + private static int GetLength(object node) + { + if (node is not string s) + { + return -1; + } + + return s.Length; + } +}", @"Friend Class TestClass + Private Shared Function GetLength(node As Object) As Integer + Dim s As String = Nothing + + If CSharpImpl.__Assign(s, TryCast(node, String)) Is Nothing Then + Return -1 + End If + + Return s.Length + End Function + + Private Class CSharpImpl + + Shared Function __Assign(Of T)(ByRef target As T, value As T) As T + target = value + Return value + End Function + End Class +End Class + +1 target compilation errors: +BC30451: 'CSharpImpl.__Assign' is not declared. It may be inaccessible due to its protection level."); + } + + [Fact] + public async Task RelationalPatternExpressionAsync() + { + await TestConversionCSharpToVisualBasicAsync(@"class TestClass +{ + private static bool IsPositive(int i) + { + return i is > 0; + } +}", @"Friend Class TestClass + Private Shared Function IsPositive(i As Integer) As Boolean + Return i > 0 + End Function +End Class"); + } + + [Fact] + public async Task ConstantPatternExpressionAsync() + { + await TestConversionCSharpToVisualBasicAsync(@"class TestClass +{ + private static bool ValueTypeConstant(int i) + { + return i is 5; + } + + private static bool ObjectConstant(object o) + { + return o is 5; + } + + private static bool StringConstant(string s) + { + return s is ""abc""; + } +}", @"Friend Class TestClass + Private Shared Function ValueTypeConstant(i As Integer) As Boolean + Return i = 5 + End Function + + Private Shared Function ObjectConstant(o As Object) As Boolean + Return Equals(o, 5) + End Function + + Private Shared Function StringConstant(s As String) As Boolean + Return Equals(s, ""abc"") + End Function +End Class"); + } + + [Fact] + public async Task ImplicitObjectCreationExpressionAsync() + { + await TestConversionCSharpToVisualBasicAsync(@"using System.Text; + +class TestClass +{ + private StringBuilder builder = new(16); +}", @"Imports System.Text + +Friend Class TestClass + Private builder As StringBuilder = New StringBuilder(16) +End Class"); + } + [Fact] public async Task DeclarationExpressionAsync() {