Supersede: Support the tractable C# patterns when converting to VB

CodeConverter/Common/LineTriviaMapper.cs

@@ -194,7 +194,10 @@ private TextLine GetTargetLine(int sourceLineIndex, bool isLeading)
             if (errorTrivia.Any()) {
                 targetTrivia.Trailing.Add(errorTrivia);
             }
-            errorTrivia = toReplace.LeadingTrivia.Where(t => t.HasAnnotations(AnnotationConstants.ConversionErrorAnnotationKind)).ToArray();
+            // A leading error comment keeps the line break after it, or it would comment out the token it precedes
+            var leading = toReplace.LeadingTrivia;
+            errorTrivia = leading.Where((t, i) => t.HasAnnotations(AnnotationConstants.ConversionErrorAnnotationKind) ||
+                                                  t.IsEndOfLine() && i > 0 && leading[i - 1].HasAnnotations(AnnotationConstants.ConversionErrorAnnotationKind)).ToArray();
             if (errorTrivia.Any()) {
                 targetTrivia.Leading.Add(errorTrivia);
             }

CodeConverter/Util/SyntaxNodeExtensions.cs

@@ -540,7 +540,9 @@ public static T WithVbTrailingErrorComment<T>(
         this T dummyDestNode, CSharpSyntaxNode problematicSourceNode, Exception exception) where T : VisualBasicSyntaxNode
     {
         var errorDescription = problematicSourceNode.DescribeConversionError(exception);
-        var commentedText = "''' " + errorDescription.Replace("\r\n", "\r\n''' ");
+        // Split on either line ending, since the description mixes Environment.NewLine with source text and stack traces
+        var lines = errorDescription.TrimEnd('\r', '\n').Split(new[] {"\r\n", "\n"}, StringSplitOptions.None);
+        var commentedText = string.Join(Environment.NewLine, lines.Select(line => "''' " + line));
         return dummyDestNode
             .WithTrailingTrivia(VBSyntaxFactory.CommentTrivia(commentedText).WithConversionErrorAnnotation())
             .WithAdditionalAnnotations(new SyntaxAnnotation(AnnotationConstants.ConversionErrorAnnotationKind, exception.ToString()));

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;
         }
     }
 

CodeConverter/VB/MethodBodyExecutableStatementVisitor.cs

@@ -572,11 +572,23 @@ public override SyntaxList<StatementSyntax> 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.EndOfLine(Environment.NewLine));
+
+            return SyntaxFactory.ReturnStatement().WithLeadingTrivia(report);
+        }
+
+        return SyntaxFactory.ReturnStatement((ExpressionSyntax)vbExpression);
     }
 
     public override SyntaxList<StatementSyntax> VisitYieldStatement(CSSyntax.YieldStatementSyntax node)

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,18 @@ public override VisualBasicSyntaxNode VisitConstructorDeclaration(CSSyntax.Const
     public override VisualBasicSyntaxNode VisitIsPatternExpression(CSSyntax.IsPatternExpressionSyntax node)
     {
         ExpressionSyntax lhs = (ExpressionSyntax)node.Expression.Accept(TriviaConvertingVisitor);
-        switch (node.Pattern) {
+        var converted = ConvertPattern(node, lhs, _semanticModel.GetTypeInfo(node.Expression).Type, node.Pattern);
+        // In C# "is" binds tighter than ==, but VB's comparison operators (including Is) share one precedence level
+        return node.Parent is CSSyntax.BinaryExpressionSyntax ? SyntaxFactory.ParenthesizedExpression(converted) : converted;
+    }
+
+    /// <remarks>
+    /// 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 picking the right comparison depends on it.
+    /// </remarks>
+    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 +373,99 @@ 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, lhsType, 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<ITypeSymbol>() 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);
+                }
+
+                // On a nullable value type, = lifts to Boolean? (which throws when narrowed from Nothing), and
+                // Equals compares the boxed types, so a Long? holding 5 is never Equals to the Integer 5
+                if (lhsType.IsNullable()) {
+                    throw new NotSupportedException("A constant pattern on a nullable value type is not converted.");
+                }
+
+                // NaN matches NaN in a pattern, but = never does
+                if (lhsType != null && lhsType.IsValueType && _semanticModel.GetConstantValue(cps.Expression).Value is double.NaN or float.NaN) {
+                    var isNaN = SyntaxFactory.SimpleMemberAccessExpression(
+                        (ExpressionSyntax)_commonConversions.VbSyntaxGenerator.TypeExpression(lhsType),
+                        SyntaxFactory.Token(SyntaxKind.DotToken), SyntaxFactory.IdentifierName(nameof(double.IsNaN)));
+                    return SyntaxFactory.InvocationExpression(isNaN, ExpressionSyntaxExtensions.CreateArgList(lhs));
+                }
+
+                // 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), node.Pattern, null);
+                throw new ArgumentOutOfRangeException(nameof(node), pattern, null);
+        }
+    }
+
+    /// <remarks>
+    /// "x is &gt; 0" and its siblings are plain comparisons once the value under test is known.
+    /// On anything but a non-nullable value type, the pattern also tests the type or that a value is
+    /// present, which a plain comparison would silently drop (late binding, or a lifted Boolean?).
+    /// </remarks>
+    private ExpressionSyntax ConvertRelationalPattern(ExpressionSyntax lhs, ITypeSymbol lhsType, CSSyntax.RelationalPatternSyntax pattern)
+    {
+        if (lhsType == null || !lhsType.IsValueType || lhsType.IsNullable()) {
+            throw new NotSupportedException("A relational pattern is only converted when the value under test is a non-nullable value type.");
+        }
+
+        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);
+    }
+
+    /// <remarks>
+    /// Visual Basic can negate a type test and a Nothing test in place, which reads far better
+    /// than wrapping the whole condition in Not.
+    /// </remarks>
+    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);
+            case BinaryExpressionSyntax binary when binary.IsKind(SyntaxKind.IsExpression):
+                return SyntaxFactory.IsNotExpression(binary.Left, binary.Right);
+            default:
+                return SyntaxFactory.NotExpression(SyntaxFactory.ParenthesizedExpression(converted));
         }
     }
 
@@ -991,7 +1092,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<CSSyntax.AccessorDeclarationSyntax>()?.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 +1492,27 @@ public override VisualBasicSyntaxNode VisitObjectCreationExpression(CSSyntax.Obj
         );
     }
 
+    /// <remarks>
+    /// "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.
+    /// </remarks>
+    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<AttributeListSyntax>(),
+            (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(