Skip to content

Commit 4310d6e

Browse files
google-labs-jules[bot]GrahamTheCoder
google-labs-jules[bot]
and
GrahamTheCoder
committed
Fix GetType on open generic types falling back to typeof(object)
VB open generics like `Nullable(Of )` generate `IErrorTypeSymbol` when using Roslyn's semantic model which previously forced `TypeConversionAnalyzer` and equality comparisons to fall back to `object` instead of preserving the syntax tree representing `typeof(X<>)`. This patch correctly yields `OmittedTypeArgumentSyntax` and bypasses the `System.Object` coercion fallback for equality testing. Co-authored-by: GrahamTheCoder <2490482+GrahamTheCoder@users.noreply.github.com>
1 parent fadac1b commit 4310d6e

File tree

7 files changed

+106
-10
lines changed

7 files changed

+106
-10
lines changed

‎CodeConverter/CSharp/BinaryExpressionConverter.cs‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using ICSharpCode.CodeConverter.Util.FromRoslyn;
1+
using ICSharpCode.CodeConverter.Util.FromRoslyn;
22

33
namespace ICSharpCode.CodeConverter.CSharp;
44

@@ -101,6 +101,19 @@ private async Task<CSharpSyntaxNode> ConvertBinaryExpressionAsync(VBasic.Syntax.
101101
lhs = omitConversion ? lhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Left, lhs, forceTargetType: forceLhsTargetType);
102102
rhs = omitConversion || omitRightConversion ? rhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Right, rhs);
103103

104+
if (node.Right is VBSyntax.GetTypeExpressionSyntax getTypeExpr) {
105+
var isUnboundGeneric = getTypeExpr.Type.DescendantNodesAndSelf().OfType<VBSyntax.TypeArgumentListSyntax>().Any(t => t.Arguments.Any(a => a is VBSyntax.IdentifierNameSyntax id && id.Identifier.IsMissing));
106+
if (isUnboundGeneric) {
107+
rhs = await node.Right.AcceptAsync<CSSyntax.ExpressionSyntax>(TriviaConvertingExpressionVisitor);
108+
}
109+
}
110+
if (node.Left is VBSyntax.GetTypeExpressionSyntax getTypeExprLeft) {
111+
var isUnboundGeneric = getTypeExprLeft.Type.DescendantNodesAndSelf().OfType<VBSyntax.TypeArgumentListSyntax>().Any(t => t.Arguments.Any(a => a is VBSyntax.IdentifierNameSyntax id && id.Identifier.IsMissing));
112+
if (isUnboundGeneric) {
113+
lhs = await node.Left.AcceptAsync<CSSyntax.ExpressionSyntax>(TriviaConvertingExpressionVisitor);
114+
}
115+
}
116+
104117
var kind = VBasic.VisualBasicExtensions.Kind(node).ConvertToken();
105118
var op = CS.SyntaxFactory.Token(CSharpUtil.GetExpressionOperatorTokenKind(kind));
106119

‎CodeConverter/CSharp/BuiltInVisualBasicOperatorSubstitutions.cs‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using Microsoft.CodeAnalysis.CSharp.Syntax;
1+
using Microsoft.CodeAnalysis.CSharp.Syntax;
22
using Microsoft.VisualBasic.CompilerServices;
33
using SyntaxFactory = Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
44
using SyntaxKind = Microsoft.CodeAnalysis.CSharp.SyntaxKind;
@@ -58,7 +58,7 @@ public async Task<ExpressionSyntax> ConvertReferenceOrNothingComparisonOrNullAsy
5858

5959
var equalityCheck = new KnownMethod(nameof(System), nameof(Object), nameof(object.ReferenceEquals))
6060
.Invoke(_visualBasicEqualityComparison.ExtraUsingDirectives,
61-
ConvertTo(node.Left, lhs, SpecialType.System_Object), rhs);
61+
ConvertToIfNecessary(node.Left, lhs, SpecialType.System_Object), ConvertToIfNecessary(node.Right, rhs, SpecialType.System_Object));
6262
return notted
6363
? SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, equalityCheck)
6464
: equalityCheck;
@@ -80,9 +80,14 @@ private static VBSyntax.ExpressionSyntax ArgComparedToNull(VBSyntax.BinaryExpres
8080
return null;
8181
}
8282

83-
private async Task<ExpressionSyntax> ConvertIsOrIsNotExpressionArgAsync(VBSyntax.ExpressionSyntax binaryExpressionArg) =>
84-
await ConvertMyGroupCollectionPropertyGetWithUnderlyingFieldAsync(binaryExpressionArg)
85-
?? await binaryExpressionArg.AcceptAsync<ExpressionSyntax>(_triviaConvertingVisitor);
83+
private async Task<ExpressionSyntax> ConvertIsOrIsNotExpressionArgAsync(VBSyntax.ExpressionSyntax binaryExpressionArg)
84+
{
85+
if (binaryExpressionArg is VBSyntax.GetTypeExpressionSyntax getTypeExpr && getTypeExpr.Type.DescendantNodesAndSelf().OfType<VBSyntax.TypeArgumentListSyntax>().Any(t => t.Arguments.Any(a => a is VBSyntax.IdentifierNameSyntax id && id.Identifier.IsMissing))) {
86+
return await getTypeExpr.AcceptAsync<ExpressionSyntax>(_triviaConvertingVisitor);
87+
}
88+
return await ConvertMyGroupCollectionPropertyGetWithUnderlyingFieldAsync(binaryExpressionArg)
89+
?? await binaryExpressionArg.AcceptAsync<ExpressionSyntax>(_triviaConvertingVisitor);
90+
}
8691

8792
private async Task<ExpressionSyntax> ConvertMyGroupCollectionPropertyGetWithUnderlyingFieldAsync(SyntaxNode node)
8893
{
@@ -122,6 +127,17 @@ private ExpressionSyntax ConvertTo(VBSyntax.ExpressionSyntax vbNode, ExpressionS
122127
return _typeConversionAnalyzer.AddExplicitConversion(vbNode, csNode, forceTargetType: _semanticModel.Compilation.GetSpecialType(targetType));
123128
}
124129

130+
private ExpressionSyntax ConvertToIfNecessary(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, SpecialType targetType)
131+
{
132+
if (vbNode is VBSyntax.GetTypeExpressionSyntax getTypeExpr && getTypeExpr.Type.DescendantNodesAndSelf().OfType<VBSyntax.TypeArgumentListSyntax>().Any(t => t.Arguments.Any(a => a is VBSyntax.IdentifierNameSyntax id && id.Identifier.IsMissing))) {
133+
return csNode;
134+
}
135+
if (csNode is TypeOfExpressionSyntax typeOfExpr && typeOfExpr.Type is GenericNameSyntax gen && gen.TypeArgumentList.Arguments.Any(a => a is OmittedTypeArgumentSyntax)) {
136+
return csNode;
137+
}
138+
return ConvertTo(vbNode, csNode, targetType);
139+
}
140+
125141
/// <remarks>No need to implement these since this is only called for things that are already decimal and hence will resolve operator in C#</remarks>
126142
private static async Task<ExpressionSyntax> ConvertToDecimalBinaryOperatorAsync(VBSyntax.BinaryExpressionSyntax node, KnownMethod member) =>
127143
default;

‎CodeConverter/CSharp/ExpressionNodeVisitor.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -578,8 +578,13 @@ public override async Task<CSharpSyntaxNode> VisitArrayRankSpecifier(VBasic.Synt
578578

579579
public override async Task<CSharpSyntaxNode> VisitTypeArgumentList(VBasic.Syntax.TypeArgumentListSyntax node)
580580
{
581-
var args = await node.Arguments.SelectAsync(async a => await a.AcceptAsync<TypeSyntax>(TriviaConvertingExpressionVisitor));
582-
return CS.SyntaxFactory.TypeArgumentList(CS.SyntaxFactory.SeparatedList(args));
581+
var args = await node.Arguments.SelectAsync(async a => {
582+
if (a is VBasic.Syntax.IdentifierNameSyntax id && id.Identifier.IsMissing) {
583+
return CS.SyntaxFactory.OmittedTypeArgument();
584+
}
585+
return await a.AcceptAsync<TypeSyntax>(TriviaConvertingExpressionVisitor);
586+
});
587+
return CS.SyntaxFactory.TypeArgumentList(CS.SyntaxFactory.SeparatedList<TypeSyntax>(args));
583588
}
584589

585590
private async Task<CSharpSyntaxNode> ConvertCastExpressionAsync(VBSyntax.CastExpressionSyntax node,

‎CodeConverter/CSharp/NameExpressionNodeVisitor.cs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,10 @@ public async Task<CSharpSyntaxNode> ConvertQualifiedNameAsync(VBasic.Syntax.Qual
162162
/// <remarks>PERF: This is a hot code path, try to avoid using things like GetOperation except where needed.</remarks>
163163
public async Task<CSharpSyntaxNode> ConvertIdentifierNameAsync(VBasic.Syntax.IdentifierNameSyntax node)
164164
{
165+
if (node.Identifier.IsMissing && node.Parent is VBasic.Syntax.TypeArgumentListSyntax) {
166+
return SyntaxFactory.OmittedTypeArgument();
167+
}
168+
165169
var identifier = SyntaxFactory.IdentifierName(CommonConversions.ConvertIdentifier(node.Identifier, node.GetAncestor<VBasic.Syntax.AttributeSyntax>() != null));
166170

167171
bool requiresQualification = !node.Parent.IsKind(VBasic.SyntaxKind.SimpleMemberAccessExpression, VBasic.SyntaxKind.QualifiedName, VBasic.SyntaxKind.NameColonEquals, VBasic.SyntaxKind.ImportsStatement, VBasic.SyntaxKind.NamespaceStatement, VBasic.SyntaxKind.NamedFieldInitializer) ||
@@ -618,7 +622,13 @@ private ITypeSymbol[] GetOrNullAllTypeArgsIncludingInferred(IMethodSymbol vbMeth
618622

619623
private async Task<TypeArgumentListSyntax> ConvertTypeArgumentListAsync(VBSyntax.GenericNameSyntax node)
620624
{
621-
return await node.TypeArgumentList.AcceptAsync<TypeArgumentListSyntax>(TriviaConvertingExpressionVisitor);
625+
var args = await node.TypeArgumentList.Arguments.SelectAsync(async a => {
626+
if (a is VBasic.Syntax.IdentifierNameSyntax id && id.Identifier.IsMissing) {
627+
return CS.SyntaxFactory.OmittedTypeArgument();
628+
}
629+
return await a.AcceptAsync<TypeSyntax>(TriviaConvertingExpressionVisitor);
630+
});
631+
return CS.SyntaxFactory.TypeArgumentList(CS.SyntaxFactory.SeparatedList<TypeSyntax>(args));
622632
}
623633

624634
private CSharpSyntaxNode AddEmptyArgumentListIfImplicit(SyntaxNode node, ExpressionSyntax id)

‎CodeConverter/CSharp/TypeConversionAnalyzer.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ public TypeConversionAnalyzer(SemanticModel semanticModel, CSharpCompilation csC
4949
public ExpressionSyntax AddExplicitConversion(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, bool addParenthesisIfNeeded = true, bool defaultToCast = false, bool isConst = false, ITypeSymbol forceSourceType = null, ITypeSymbol forceTargetType = null)
5050
{
5151
if (csNode == null) return null;
52+
if (vbNode is VBSyntax.GetTypeExpressionSyntax getTypeExpr && getTypeExpr.Type.DescendantNodesAndSelf().OfType<VBSyntax.TypeArgumentListSyntax>().Any(t => t.Arguments.Any(a => a is VBSyntax.IdentifierNameSyntax id && id.Identifier.IsMissing))) {
53+
return csNode;
54+
}
5255

5356
var conversionKind = AnalyzeConversion(vbNode, defaultToCast, isConst, forceSourceType, forceTargetType);
5457
csNode = addParenthesisIfNeeded && conversionKind is TypeConversionKind.DestructiveCast or TypeConversionKind.NonDestructiveCast
@@ -59,6 +62,9 @@ public ExpressionSyntax AddExplicitConversion(VBSyntax.ExpressionSyntax vbNode,
5962

6063
public (ExpressionSyntax Expr, bool IsConst) AddExplicitConversion(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, TypeConversionKind conversionKind, bool addParenthesisIfNeeded = false, bool requiresConst = false, ITypeSymbol forceSourceType = null, ITypeSymbol forceTargetType = null)
6164
{
65+
if (vbNode is VBSyntax.GetTypeExpressionSyntax getTypeExpr2 && getTypeExpr2.Type.DescendantNodesAndSelf().OfType<VBSyntax.TypeArgumentListSyntax>().Any(t => t.Arguments.Any(a => a is VBSyntax.IdentifierNameSyntax id && id.Identifier.IsMissing))) {
66+
return (csNode, false);
67+
}
6268
var (vbType, vbConvertedType) = GetTypeInfo(vbNode, forceSourceType, forceTargetType);
6369
bool resultConst = false;
6470

@@ -142,6 +148,9 @@ private ExpressionSyntax CreateCast(ExpressionSyntax csNode, ITypeSymbol vbConve
142148

143149
public TypeConversionKind AnalyzeConversion(VBSyntax.ExpressionSyntax vbNode, bool alwaysExplicit = false, bool isConst = false, ITypeSymbol forceSourceType = null, ITypeSymbol forceTargetType = null)
144150
{
151+
if (vbNode is VBSyntax.GetTypeExpressionSyntax getTypeExpr && getTypeExpr.Type.DescendantNodesAndSelf().OfType<VBSyntax.TypeArgumentListSyntax>().Any(t => t.Arguments.Any(a => a is VBSyntax.IdentifierNameSyntax id && id.Identifier.IsMissing))) {
152+
return TypeConversionKind.NonDestructiveCast;
153+
}
145154
var (vbType, vbConvertedType) = GetTypeInfo(vbNode, forceSourceType, forceTargetType);
146155

147156
if (vbConvertedType is null)

‎CodeConverter/CSharp/VisualBasicEqualityComparison.cs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#nullable enable
1+
#nullable enable
22
using System.Globalization;
33
using Microsoft.CodeAnalysis.CSharp.Syntax;
44
using Microsoft.VisualBasic.CompilerServices;
@@ -77,6 +77,22 @@ public RequiredType GetObjectEqualityType(params TypeInfo[] typeInfos)
7777
{
7878
bool requiresVbEqualityCheck = typeInfos.Any(t => t.Type?.SpecialType == SpecialType.System_Object);
7979

80+
if (typeInfos.Any(t => t.Type?.TypeKind == Microsoft.CodeAnalysis.TypeKind.Error && t.ConvertedType?.TypeKind == Microsoft.CodeAnalysis.TypeKind.Error)) {
81+
return RequiredType.None;
82+
}
83+
if (requiresVbEqualityCheck) {
84+
if (typeInfos.Any(t => t.Type?.TypeKind == TypeKind.Error && t.ConvertedType?.SpecialType == SpecialType.None)) {
85+
return RequiredType.None;
86+
}
87+
}
88+
89+
if (requiresVbEqualityCheck) {
90+
if (typeInfos.Any(t => t.Type?.TypeKind == TypeKind.Error && t.Type.Name == "Nullable")) {
91+
// VB considers GetType(Nullable(Of)) as IErrorTypeSymbol and it falls back to object, avoid equality check
92+
return RequiredType.None;
93+
}
94+
}
95+
8096
if (typeInfos.All(
8197
t => t.Type == null || t.Type.SpecialType == SpecialType.System_String ||
8298
t.Type.IsArrayOf(SpecialType.System_Char) ) ) {

‎Tests/CSharp/ExpressionTests/OmittedTypeArgumentTest.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
using System;
2+
using System.Threading.Tasks;
3+
using ICSharpCode.CodeConverter.Tests.TestRunners;
4+
using Xunit;
5+
6+
namespace ICSharpCode.CodeConverter.Tests.CSharp.ExpressionTests;
7+
8+
public class OmittedTypeArgumentTest : ConverterTestBase
9+
{
10+
[Fact]
11+
public async Task TestGetTypeOmittedArgument()
12+
{
13+
await TestConversionVisualBasicToCSharpAsync(@"Public Class Test
14+
Public Function IsNullable(ByVal type As Type) As Boolean
15+
Return type.IsGenericType AndAlso type.GetGenericTypeDefinition() Is GetType(Nullable(Of))
16+
End Function
17+
End Class", @"using System;
18+
19+
public class Test
20+
{
21+
public bool IsNullable(Type type)
22+
{
23+
return type.IsGenericType && ReferenceEquals(type.GetGenericTypeDefinition(), typeof(Nullable<>));
24+
}
25+
}");
26+
}
27+
}

0 commit comments

Comments
 (0)