Skip to content

Commit

Permalink
Don't use patterns when name is reusable - relates to #1011
Browse files Browse the repository at this point in the history
  • Loading branch information
@GrahamTheCoder
GrahamTheCoder committed Jun 25, 2023
1 parent ebb52bf commit 9dd981d
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 116 deletions.
6 changes: 3 additions & 3 deletions CodeConverter/CSharp/TypeConversionAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ private ExpressionSyntax AddTypeConversion(VBSyntax.ExpressionSyntax vbNode, Exp
switch (conversionKind) {
case TypeConversionKind.FractionalNumberRoundThenCast:
csNode = vbType.IsNullable() && vbConvertedType.IsNullable()
? _vbNullableExpressionsConverter.InvokeConversionWhenNotNull(csNode, GetMathRoundMemberAccess(), GetTypeSyntax(vbConvertedType))
? _vbNullableExpressionsConverter.InvokeConversionWhenNotNull(vbNode, csNode, GetMathRoundMemberAccess(), GetTypeSyntax(vbConvertedType))
: AddRoundInvocation(vbType.IsNullable() ? csNode.NullableGetValueExpression() : csNode);

return AddTypeConversion(vbNode, csNode, TypeConversionKind.NonDestructiveCast, addParenthesisIfNeeded, vbType, vbConvertedType);
case TypeConversionKind.EnumConversionThenCast:
vbConvertedType.IsNullable(out var convertedNullableType);
var underlyingEnumType = ((INamedTypeSymbol)(convertedNullableType ?? vbConvertedType)).EnumUnderlyingType;
csNode = vbType.IsNullable() && convertedNullableType != null
? _vbNullableExpressionsConverter.InvokeConversionWhenNotNull(csNode, GetConversionsMemberAccess(underlyingEnumType), GetTypeSyntax(vbConvertedType))
? _vbNullableExpressionsConverter.InvokeConversionWhenNotNull(vbNode, csNode, GetConversionsMemberAccess(underlyingEnumType), GetTypeSyntax(vbConvertedType))
: AddTypeConversion(vbNode, csNode, TypeConversionKind.Conversion, addParenthesisIfNeeded, vbType, underlyingEnumType);

return AddTypeConversion(vbNode, csNode, TypeConversionKind.NonDestructiveCast, addParenthesisIfNeeded, vbType, vbConvertedType);
Expand Down Expand Up @@ -449,7 +449,7 @@ invoke.Expression is MemberAccessExpressionSyntax expr &&
}

if (nullableCurrentType != null && nullableTargetType != null) {
return _vbNullableExpressionsConverter.InvokeConversionWhenNotNull(csNode, memberAccess, GetTypeSyntax(targetType));
return _vbNullableExpressionsConverter.InvokeConversionWhenNotNull(vbNode, csNode, memberAccess, GetTypeSyntax(targetType));
}

var arguments = SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList(SyntaxFactory.Argument(csNode)));
Expand Down
4 changes: 2 additions & 2 deletions CodeConverter/CSharp/ValidSyntaxFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public static IdentifierNameSyntax NameOf() => SyntaxFactory.IdentifierName(
SyntaxFactory.Identifier(SyntaxTriviaList.Empty, SyntaxKind.NameOfKeyword, "nameof", "nameof", SyntaxTriviaList.Empty)
);

public static ExpressionSyntax NullableHasValueExpression(this ExpressionSyntax node) => SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, node.AddParens(), SyntaxFactory.IdentifierName("HasValue"));
public static ExpressionSyntax NullableGetValueExpression(this ExpressionSyntax node) => SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, node.AddParens(), SyntaxFactory.IdentifierName("Value"));
public static ExpressionSyntax NullableHasValueExpression(this ExpressionSyntax node) => SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, node.AddParens(), SyntaxFactory.IdentifierName(nameof(Nullable<int>.HasValue)));
public static ExpressionSyntax NullableGetValueExpression(this ExpressionSyntax node) => SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, node.AddParens(), SyntaxFactory.IdentifierName(nameof(Nullable<int>.Value)));

}
72 changes: 46 additions & 26 deletions CodeConverter/CSharp/VisualBasicNullableExpressionsConverter.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using System.Xml.Linq;
using ICSharpCode.CodeConverter.Util.FromRoslyn;
using Microsoft.CodeAnalysis;
using ICSharpCode.CodeConverter.Util.FromRoslyn;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

Expand Down Expand Up @@ -35,14 +33,14 @@ internal class VisualBasicNullableExpressionsConverter

public VisualBasicNullableExpressionsConverter(SemanticModel semanticModel) => _semanticModel = semanticModel;

public ExpressionSyntax InvokeConversionWhenNotNull(ExpressionSyntax expression, MemberAccessExpressionSyntax conversionMethod, TypeSyntax castType)
public ExpressionSyntax InvokeConversionWhenNotNull(VBSyntax.ExpressionSyntax vbExpr, ExpressionSyntax csExpr, MemberAccessExpressionSyntax conversionMethod, TypeSyntax castType)
{
var pattern = PatternObject(expression, out var argName);
var arguments = SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList(SyntaxFactory.Argument(argName)));
var hasValueCheck = HasValue(vbExpr, ref csExpr);
var arguments = SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList(SyntaxFactory.Argument(csExpr)));
ExpressionSyntax invocation = SyntaxFactory.InvocationExpression(conversionMethod, arguments);
invocation = ValidSyntaxFactory.CastExpression(castType, invocation);

return pattern.Conditional(invocation, ValidSyntaxFactory.NullExpression).AddParens();
return hasValueCheck.Conditional(invocation, ValidSyntaxFactory.NullExpression).AddParens();
}

public ExpressionSyntax WithBinaryExpressionLogicForNullableTypes(VBSyntax.BinaryExpressionSyntax vbNode, TypeInfo lhsTypeInfo, TypeInfo rhsTypeInfo, BinaryExpressionSyntax csBinExp, ExpressionSyntax lhs, ExpressionSyntax rhs)
Expand Down Expand Up @@ -78,13 +76,23 @@ private ExpressionSyntax WithBinaryExpressionLogicForNullableTypes(Microsoft.Cod
return ForRelationalOperators(vbNode, csBinExp, lhs, rhs, isLhsNullable, isRhsNullable).AddParens();
}

private bool IsNullable(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, TypeInfo lhsTypeInfo)
private bool IsNullable(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, TypeInfo typeInfo)
{
return lhsTypeInfo.Type.IsNullable() && !csNode.AnyInParens(x => x.HasAnnotation(IsNotNullableAnnotation)) && GetNullabilityWithinBooleanExpression(vbNode) != KnownNullability.NotNull;
return typeInfo.Type.IsNullable() && !csNode.AnyInParens(x => x.HasAnnotation(IsNotNullableAnnotation)) && GetNullabilityWithinBooleanExpression(vbNode) != KnownNullability.NotNull;
}

private string GetArgName() => $"arg{Interlocked.Increment(ref _argCounter)}";

private ExpressionSyntax? PatternVar(VBSyntax.ExpressionSyntax vbLeft, ExpressionSyntax csLeft, out ExpressionSyntax lhsName)
{
if (IsSafelyReusable(vbLeft)) {
lhsName = csLeft;
return null;
} else {
return PatternVar(csLeft, out lhsName);
}
}

private ExpressionSyntax PatternVar(ExpressionSyntax expr, out ExpressionSyntax name)
{
var arg = GetArgName();
Expand Down Expand Up @@ -135,8 +143,7 @@ private ExpressionSyntax ForAndAlsoOperator(VBSyntax.BinaryExpressionSyntax vbNo
ExpressionSyntax lhs, ExpressionSyntax rhs,
bool isLhsNullable, bool isRhsNullable)
{
var lhsPattern = PatternVar(lhs, out var lhsName);
var rhsPattern = PatternObject(rhs, out var rhsName);
var lhsPattern = PatternVar(vbNode.Left, lhs, out var lhsName);

if (vbNode.AlwaysHasBooleanTypeInCSharp()) {
if (!isLhsNullable) {
Expand All @@ -145,7 +152,7 @@ private ExpressionSyntax ForAndAlsoOperator(VBSyntax.BinaryExpressionSyntax vbNo
if (!isRhsNullable) {
return lhsPattern.AndHasNoValue(lhsName).OrIsTrue(lhsName).And(rhs).AndHasValue(lhsName);
}
return FullAndExpression().EqualsTrue();
return FullAndExpression(vbNode.Right, rhs).EqualsTrue();
}

if (!isLhsNullable) {
Expand All @@ -154,11 +161,14 @@ private ExpressionSyntax ForAndAlsoOperator(VBSyntax.BinaryExpressionSyntax vbNo
if (!isRhsNullable) {
return lhsPattern.AndHasValue(lhsName).AndIsFalse(lhsName).Conditional(False, rhs.Conditional(lhsName, False));
}
return FullAndExpression();
return FullAndExpression(vbNode.Right, rhs);

ExpressionSyntax FullAndExpression() =>
lhsPattern.AndHasValue(lhsName).AndIsFalse(lhsName)
ExpressionSyntax FullAndExpression(VBSyntax.ExpressionSyntax vbExpr, ExpressionSyntax rhsName)
{
var rhsPattern = HasValue(vbExpr, ref rhsName);
return lhsPattern.AndHasValue(lhsName).AndIsFalse(lhsName)
.Conditional(False, rhsPattern.Negate().Conditional(Null, rhsName.Conditional(lhsName, False)));
}
}

private ExpressionSyntax ForOrElseOperator(VBSyntax.BinaryExpressionSyntax vbNode,
Expand All @@ -177,7 +187,8 @@ private ExpressionSyntax ForOrElseOperator(VBSyntax.BinaryExpressionSyntax vbNod
return lhs.Conditional(True, rhs);
}

var lhsPattern = PatternVar(lhs, out var lhsName);
var lhsPattern = PatternVar(vbNode.Left, lhs, out var lhsName);

var rhsPattern = NegatedPatternObject(rhs, out var rhsName);

var whenFalse = !isRhsNullable ?
Expand All @@ -191,9 +202,6 @@ private ExpressionSyntax ForRelationalOperators(VBSyntax.BinaryExpressionSyntax
BinaryExpressionSyntax csNode, ExpressionSyntax lhs, ExpressionSyntax rhs,
bool isLhsNullable, bool isRhsNullable)
{
ExpressionSyntax GetCondition(ref ExpressionSyntax csName, bool reusable) =>
reusable ? csName.NullableHasValueExpression() : PatternObject(csName, out csName);

ExpressionSyntax? lhsName = lhs, rhsName = rhs;
var lhsReusable = IsSafelyReusable(vbNode.Left);
var rhsReusable = IsSafelyReusable(vbNode.Right);
Expand All @@ -205,11 +213,11 @@ ExpressionSyntax GetCondition(ref ExpressionSyntax csName, bool reusable) =>
}

if (isLhsNullable || !lhsReusable) {
var lhsCondition = GetCondition(ref lhsName, lhsReusable);
var lhsCondition = HasValue(lhsReusable, ref lhsName);
conditions.Add((lhsReusable, lhsCondition));
}
if (isRhsNullable || !rhsReusable) {
var rhsCondition = GetCondition(ref rhsName, rhsReusable);
var rhsCondition = HasValue(rhsReusable, ref rhsName);
conditions.Add((rhsReusable, rhsCondition));
}

Expand All @@ -227,6 +235,18 @@ ExpressionSyntax GetCondition(ref ExpressionSyntax csName, bool reusable) =>
return notNullCondition is null ? bin : notNullCondition.Conditional(bin, Null);
}

private ExpressionSyntax HasValue(VBasic.Syntax.ExpressionSyntax vbNode, ref ExpressionSyntax csName) => HasValue(IsSafelyReusable(vbNode), ref csName);
private ExpressionSyntax HasValue(bool reusable, ref ExpressionSyntax csName)
{
if (reusable) {
var nullableHasValueExpression = csName.NullableHasValueExpression();
csName = csName.NullableGetValueExpression();
return nullableHasValueExpression;
} else {
return PatternObject(csName, out csName);
}
}

private bool IsSafelyReusable(VBasic.Syntax.ExpressionSyntax e)
{
e = e.SkipIntoParens();
Expand All @@ -241,7 +261,7 @@ private bool IsSafelyReusable(VBasic.Syntax.ExpressionSyntax e)
if (original.SkipIntoParens() is not VBSyntax.IdentifierNameSyntax {Identifier.Text: { } nameText}) return null;
for (VBSyntax.ExpressionSyntax? currentNode = original.Parent?.Parent as VBSyntax.ExpressionSyntax, childNode = original.Parent as VBSyntax.ExpressionSyntax;
currentNode is VBSyntax.BinaryExpressionSyntax {Left: var l, OperatorToken: var op, Right: var r};
currentNode = currentNode?.Parent as VBSyntax.ExpressionSyntax) {
currentNode = currentNode.Parent as VBSyntax.ExpressionSyntax) {

if (r == childNode) {
if (op.IsKind(VBasic.SyntaxKind.AndAlsoKeyword)) {
Expand Down Expand Up @@ -310,10 +330,10 @@ internal static class NullableTypesLogicExtensions
public static ExpressionSyntax GetValueOrDefault(this ExpressionSyntax node) => SyntaxFactory.InvocationExpression(SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, node.AddParens(), SyntaxFactory.IdentifierName("GetValueOrDefault")));
public static ExpressionSyntax Negate(this ExpressionSyntax node) => SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, node.AddParens());

public static ExpressionSyntax And(this ExpressionSyntax a, ExpressionSyntax b) => SyntaxFactory.BinaryExpression(SyntaxKind.LogicalAndExpression, a.AddParens(), b.AddParens()).AddParens();
public static ExpressionSyntax AndHasValue(this ExpressionSyntax a, ExpressionSyntax b) => And(a, b.NullableHasValueExpression());
public static ExpressionSyntax AndHasNoValue(this ExpressionSyntax a, ExpressionSyntax b) => And(a, b.NullableHasValueExpression().Negate());
public static ExpressionSyntax AndIsFalse(this ExpressionSyntax a, ExpressionSyntax b) => And(a, b.NullableGetValueExpression().Negate());
public static ExpressionSyntax And(this ExpressionSyntax? a, ExpressionSyntax b) => a is null ? b : SyntaxFactory.BinaryExpression(SyntaxKind.LogicalAndExpression, a.AddParens(), b.AddParens()).AddParens();
public static ExpressionSyntax AndHasValue(this ExpressionSyntax? a, ExpressionSyntax b) => And(a, b.NullableHasValueExpression());
public static ExpressionSyntax AndHasNoValue(this ExpressionSyntax? a, ExpressionSyntax b) => And(a, b.NullableHasValueExpression().Negate());
public static ExpressionSyntax AndIsFalse(this ExpressionSyntax? a, ExpressionSyntax b) => And(a, b.NullableGetValueExpression().Negate());

public static ExpressionSyntax Or(this ExpressionSyntax a, ExpressionSyntax b) => SyntaxFactory.BinaryExpression(SyntaxKind.LogicalOrExpression, a.AddParens(), b.AddParens()).AddParens();
public static ExpressionSyntax OrIsTrue(this ExpressionSyntax a, ExpressionSyntax b) => Or(a, b.NullableGetValueExpression());
Expand Down
Loading

0 comments on commit 9dd981d

Please sign in to comment.