Skip to content

Commit 195a676

Browse files
GrahamTheCoderGrahamTheCoder
authored
Merge pull request #582 from icsharpcode/option-compare-text
VB -> CS: Select Case improvements
2 parents cddacb8 + 44a449d commit 195a676

15 files changed

Lines changed: 737 additions & 87 deletions

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
99

1010

1111
### VB -> C#
12-
12+
* Handle Option Compare Text case insensitive comparisons in switch statements [#579](https://github.com/icsharpcode/CodeConverter/issues/579)
13+
* Fix compilation error when switching with enum cases [#549](https://github.com/icsharpcode/CodeConverter/issues/549)
1314

1415
### C# -> VB
1516

‎CodeConverter/CSharp/BuiltInVisualBasicOperatorSubsitutions.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ private async Task<ExpressionSyntax> ConvertToObjectComparisonOperatorAsync(VBSy
109109
{
110110
var (lhs, rhs) = await AcceptSidesAsync(node);
111111
member = (member.Import, member.TypeName, "Conditional" + member.MethodName); //The VB compiler would late bind, but this should provide identical results in most cases I think
112-
var compareTextKind = _visualBasicEqualityComparison.OptionCompareTextCaseInsensitive ? SyntaxKind.TrueLiteralExpression : SyntaxKind.FalseLiteralExpression;
113-
return member.Invoke(_visualBasicEqualityComparison.ExtraUsingDirectives, lhs, rhs, SyntaxFactory.LiteralExpression(compareTextKind));
112+
var optionaCompareTextBoolLiteralExpression = _visualBasicEqualityComparison.OptionCompareTextCaseInsensitiveBoolExpression;
113+
return member.Invoke(_visualBasicEqualityComparison.ExtraUsingDirectives, lhs, rhs, optionaCompareTextBoolLiteralExpression);
114114
}
115115

116116
private async Task<ExpressionSyntax> ConvertToConcatenateOperatorAsync(VBSyntax.BinaryExpressionSyntax node)

‎CodeConverter/CSharp/CommonConversions.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ internal class CommonConversions
4141
public Document Document { get; }
4242
private readonly SemanticModel _semanticModel;
4343
public SyntaxGenerator CsSyntaxGenerator { get; }
44+
public VisualBasicEqualityComparison VisualBasicEqualityComparison { get; }
45+
4446
private readonly CSharpCompilation _csCompilation;
4547
private readonly ITypeContext _typeContext;
4648

@@ -49,14 +51,15 @@ internal class CommonConversions
4951

5052
public CommonConversions(Document document, SemanticModel semanticModel,
5153
TypeConversionAnalyzer typeConversionAnalyzer, SyntaxGenerator csSyntaxGenerator,
52-
CSharpCompilation csCompilation, ITypeContext typeContext)
54+
CSharpCompilation csCompilation, ITypeContext typeContext, VisualBasicEqualityComparison visualBasicEqualityComparison)
5355
{
5456
TypeConversionAnalyzer = typeConversionAnalyzer;
5557
Document = document;
5658
_semanticModel = semanticModel;
5759
CsSyntaxGenerator = csSyntaxGenerator;
5860
_csCompilation = csCompilation;
5961
_typeContext = typeContext;
62+
VisualBasicEqualityComparison = visualBasicEqualityComparison;
6063
}
6164

6265
public async Task<(IReadOnlyCollection<(VariableDeclarationSyntax Decl, ITypeSymbol Type)> Variables, IReadOnlyCollection<CSharpSyntaxNode> Methods)> SplitVariableDeclarationsAsync(

‎CodeConverter/CSharp/DeclarationNodeVisitor.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public DeclarationNodeVisitor(Document document, Compilation compilation, Semant
6161
TriviaConvertingDeclarationVisitor = new CommentConvertingVisitorWrapper(this, _semanticModel.SyntaxTree);
6262
var expressionEvaluator = new ExpressionEvaluator(semanticModel, _visualBasicEqualityComparison);
6363
var typeConversionAnalyzer = new TypeConversionAnalyzer(semanticModel, csCompilation, _extraUsingDirectives, _csSyntaxGenerator, expressionEvaluator);
64-
CommonConversions = new CommonConversions(document, semanticModel, typeConversionAnalyzer, csSyntaxGenerator, csCompilation, _typeContext);
64+
CommonConversions = new CommonConversions(document, semanticModel, typeConversionAnalyzer, csSyntaxGenerator, csCompilation, _typeContext, _visualBasicEqualityComparison);
6565
var expressionNodeVisitor = new ExpressionNodeVisitor(semanticModel, _visualBasicEqualityComparison, csCompilation, _typeContext, CommonConversions, _extraUsingDirectives);
6666
_triviaConvertingExpressionVisitor = expressionNodeVisitor.TriviaConvertingExpressionVisitor;
6767
_createMethodBodyVisitorAsync = expressionNodeVisitor.CreateMethodBodyVisitorAsync;
@@ -84,14 +84,14 @@ public override async Task<CSharpSyntaxNode> VisitCompilationUnit(VBSyntax.Compi
8484
var options = (VBasic.VisualBasicCompilationOptions)_semanticModel.Compilation.Options;
8585
var importsClauses = options.GlobalImports.Select(gi => gi.Clause).Concat(node.Imports.SelectMany(imp => imp.ImportsClauses)).ToList();
8686

87-
var optionCompareText = node.Options.Any(x => x.NameKeyword.ValueText.Equals("Compare", StringComparison.OrdinalIgnoreCase) &&
88-
x.ValueKeyword.ValueText.Equals("Text", StringComparison.OrdinalIgnoreCase));
8987
_topAncestorNamespace = node.Members.Any(m => !IsNamespaceDeclaration(m)) ? options.RootNamespace : null;
90-
_visualBasicEqualityComparison.OptionCompareTextCaseInsensitive = optionCompareText;
88+
var fileOptionCompareValue = node.Options.Where(x => x.NameKeyword.IsKind(VBasic.SyntaxKind.CompareKeyword)).LastOrDefault()?.ValueKeyword;
89+
_visualBasicEqualityComparison.OptionCompareTextCaseInsensitive = fileOptionCompareValue?.IsKind(VBasic.SyntaxKind.TextKeyword) ?? options.OptionCompareText;
9190

9291
var attributes = SyntaxFactory.List(await node.Attributes.SelectMany(a => a.AttributeLists).SelectManyAsync(CommonConversions.ConvertAttributeAsync));
92+
9393
var sourceAndConverted = await node.Members
94-
.Where(m => !(m is VBSyntax.OptionStatementSyntax)) //TODO Record values for use in conversion
94+
.Where(m => !(m is VBSyntax.OptionStatementSyntax))
9595
.SelectAsync(async m => (Source: m, Converted: await ConvertMemberAsync(m)));
9696

9797

‎CodeConverter/CSharp/ExpressionNodeVisitor.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ internal class ExpressionNodeVisitor : VBasic.VisualBasicSyntaxVisitor<Task<CSha
3333
private readonly SemanticModel _semanticModel;
3434
private readonly HashSet<string> _extraUsingDirectives;
3535
private readonly IOperatorConverter _operatorConverter;
36-
private readonly bool _optionCompareText = false;
3736
private readonly VisualBasicEqualityComparison _visualBasicEqualityComparison;
3837
private readonly Stack<ExpressionSyntax> _withBlockLhs = new Stack<ExpressionSyntax>();
3938
private readonly ITypeContext _typeContext;
@@ -730,10 +729,11 @@ public override async Task<CSharpSyntaxNode> VisitBinaryExpression(VBasic.Syntax
730729
_visualBasicEqualityComparison.TryConvertToNullOrEmptyCheck(node, lhs, rhs, out CSharpSyntaxNode visitBinaryExpression)) {
731730
return visitBinaryExpression;
732731
}
733-
(lhs, rhs) = _visualBasicEqualityComparison.AdjustForVbStringComparison(node.Left, lhs, lhsTypeInfo, node.Right, rhs, rhsTypeInfo);
732+
(lhs, rhs) = _visualBasicEqualityComparison.AdjustForVbStringComparison(node.Left, lhs, lhsTypeInfo, false, node.Right, rhs, rhsTypeInfo, false);
733+
omitConversion = true; // Already handled within for the appropriate types (rhs can become int in comparison)
734734
break;
735735
case VisualBasicEqualityComparison.RequiredType.Object:
736-
return _visualBasicEqualityComparison.GetFullExpressionForVbObjectComparison(node, lhs, rhs);
736+
return _visualBasicEqualityComparison.GetFullExpressionForVbObjectComparison(lhs, rhs, node.IsKind(VBasic.SyntaxKind.NotEqualsExpression));
737737
}
738738

739739
omitConversion |= lhsTypeInfo.Type != null && rhsTypeInfo.Type != null &&

‎CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs‎

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -641,34 +641,59 @@ public override async Task<SyntaxList<StatementSyntax>> VisitGoToStatement(VBSyn
641641
public override async Task<SyntaxList<StatementSyntax>> VisitSelectBlock(VBSyntax.SelectBlockSyntax node)
642642
{
643643
var vbExpr = node.SelectStatement.Expression;
644-
var (csExpr, stmts, csExprWithSourceMapping) = await GetExpressionWithoutSideEffectsAsync(vbExpr, "switchExpr");
644+
var vbEquality = CommonConversions.VisualBasicEqualityComparison;
645+
646+
var csSwitchExpr = (ExpressionSyntax)await vbExpr.AcceptAsync(_expressionVisitor);
647+
csSwitchExpr = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(vbExpr, csSwitchExpr);
648+
var switchExprTypeInfo = _semanticModel.GetTypeInfo(vbExpr);
649+
var isStringComparison = switchExprTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String || switchExprTypeInfo.ConvertedType?.IsArrayOf(SpecialType.System_Char) == true;
650+
var caseInsensitiveStringComparison = vbEquality.OptionCompareTextCaseInsensitive &&
651+
isStringComparison;
652+
if (isStringComparison) {
653+
csSwitchExpr = vbEquality.VbCoerceToNonNullString(vbExpr, csSwitchExpr, switchExprTypeInfo);
654+
}
655+
645656
var usedConstantValues = new HashSet<object>();
646657
var sections = new List<SwitchSectionSyntax>();
647658
foreach (var block in node.CaseBlocks) {
648659
var labels = new List<SwitchLabelSyntax>();
649660
foreach (var c in block.CaseStatement.Cases) {
650661
if (c is VBSyntax.SimpleCaseClauseSyntax s) {
651662
var originalExpressionSyntax = (ExpressionSyntax)await s.Value.AcceptAsync(_expressionVisitor);
652-
// CSharp requires an explicit cast from the base type (e.g. int) in most cases switching on an enum
663+
var caseTypeInfo = _semanticModel.GetTypeInfo(s.Value);
653664
var typeConversionKind = CommonConversions.TypeConversionAnalyzer.AnalyzeConversion(s.Value);
654665
var correctTypeExpressionSyntax = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(s.Value, originalExpressionSyntax, typeConversionKind, true, true);
655666
var constantValue = _semanticModel.GetConstantValue(s.Value);
656667
var notAlreadyUsed = !constantValue.HasValue || usedConstantValues.Add(constantValue.Value);
657-
var caseSwitchLabelSyntax = correctTypeExpressionSyntax.IsConst && notAlreadyUsed
658-
? (SwitchLabelSyntax)SyntaxFactory.CaseSwitchLabel(correctTypeExpressionSyntax.Expr)
659-
: WrapInCasePatternSwitchLabelSyntax(node, correctTypeExpressionSyntax.Expr);
668+
669+
// Pass both halves in case we can optimize away the check based on the switch expr
670+
var wrapForStringComparison = isStringComparison && (caseInsensitiveStringComparison ||
671+
vbEquality.VbCoerceToNonNullString(vbExpr, csSwitchExpr, switchExprTypeInfo, true, s.Value, originalExpressionSyntax, caseTypeInfo, false).rhs != originalExpressionSyntax);
672+
673+
// CSharp requires an explicit cast from the base type (e.g. int) in most cases switching on an enum
674+
var csExpressionToUse = switchExprTypeInfo.ConvertedType?.IsEnumType() == true ^ caseTypeInfo.Type?.IsEnumType() == true ? correctTypeExpressionSyntax.Expr : originalExpressionSyntax;
675+
676+
var caseSwitchLabelSyntax = !wrapForStringComparison && correctTypeExpressionSyntax.IsConst && notAlreadyUsed
677+
? (SwitchLabelSyntax)SyntaxFactory.CaseSwitchLabel(csExpressionToUse)
678+
: WrapInCasePatternSwitchLabelSyntax(node, s.Value, csExpressionToUse);
660679
labels.Add(caseSwitchLabelSyntax);
661680
} else if (c is VBSyntax.ElseCaseClauseSyntax) {
662681
labels.Add(SyntaxFactory.DefaultSwitchLabel());
663682
} else if (c is VBSyntax.RelationalCaseClauseSyntax relational) {
683+
684+
var varName = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "case"));
685+
ExpressionSyntax csLeft = SyntaxFactory.IdentifierName(varName);
664686
var operatorKind = VBasic.VisualBasicExtensions.Kind(relational);
665-
var binaryExp = SyntaxFactory.BinaryExpression(operatorKind.ConvertToken(TokenContext.Local), csExpr, (ExpressionSyntax)await relational.Value.AcceptAsync(_expressionVisitor));
666-
labels.Add(WrapInCasePatternSwitchLabelSyntax(node, binaryExp, treatAsBoolean: true));
687+
var relationalValue = (ExpressionSyntax)await relational.Value.AcceptAsync(_expressionVisitor);
688+
var binaryExp = SyntaxFactory.BinaryExpression(operatorKind.ConvertToken(TokenContext.Local), csLeft, relationalValue);
689+
labels.Add(VarWhen(varName, binaryExp));
667690
} else if (c is VBSyntax.RangeCaseClauseSyntax range) {
668-
var lowerBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, (ExpressionSyntax)await range.LowerBound.AcceptAsync(_expressionVisitor), csExpr);
669-
var upperBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, csExpr, (ExpressionSyntax)await range.UpperBound.AcceptAsync(_expressionVisitor));
691+
var varName = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "case"));
692+
ExpressionSyntax csLeft = SyntaxFactory.IdentifierName(varName);
693+
var lowerBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, (ExpressionSyntax)await range.LowerBound.AcceptAsync(_expressionVisitor), csLeft);
694+
var upperBoundCheck = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, csLeft, (ExpressionSyntax)await range.UpperBound.AcceptAsync(_expressionVisitor));
670695
var withinBounds = SyntaxFactory.BinaryExpression(SyntaxKind.LogicalAndExpression, lowerBoundCheck, upperBoundCheck);
671-
labels.Add(WrapInCasePatternSwitchLabelSyntax(node, withinBounds, treatAsBoolean: true));
696+
labels.Add(VarWhen(varName, withinBounds));
672697
} else throw new NotSupportedException(c.Kind().ToString());
673698
}
674699

@@ -681,8 +706,15 @@ public override async Task<SyntaxList<StatementSyntax>> VisitSelectBlock(VBSynta
681706
sections.Add(SyntaxFactory.SwitchSection(SyntaxFactory.List(labels), list));
682707
}
683708

684-
var switchStatementSyntax = ValidSyntaxFactory.SwitchStatement(csExprWithSourceMapping, sections);
685-
return stmts.Add(switchStatementSyntax);
709+
var switchStatementSyntax = ValidSyntaxFactory.SwitchStatement(csSwitchExpr, sections);
710+
return SingleStatement(switchStatementSyntax);
711+
}
712+
713+
private static CasePatternSwitchLabelSyntax VarWhen(SyntaxToken varName, ExpressionSyntax binaryExp)
714+
{
715+
var patternMatch = ValidSyntaxFactory.VarPattern(varName);
716+
return SyntaxFactory.CasePatternSwitchLabel(patternMatch,
717+
SyntaxFactory.WhenClause(binaryExp), SyntaxFactory.Token(SyntaxKind.ColonToken));
686718
}
687719

688720
private async Task<(ExpressionSyntax Reusable, SyntaxList<StatementSyntax> Statements, ExpressionSyntax SingleUse)> GetExpressionWithoutSideEffectsAsync(VBSyntax.ExpressionSyntax vbExpr, string variableNameBase, bool forceVariable = false)
@@ -720,7 +752,7 @@ private async Task<bool> IsNeverMutatedAsync(VBSyntax.NameSyntax ns)
720752
return symbol.MatchesKind(SymbolKind.Parameter, SymbolKind.Local) && await CommonConversions.Document.Project.Solution.IsNeverWrittenAsync(symbol, allowedLocation);
721753
}
722754

723-
private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax.SelectBlockSyntax node, ExpressionSyntax expression, bool treatAsBoolean = false)
755+
private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax.SelectBlockSyntax node, VBSyntax.ExpressionSyntax vbCase, ExpressionSyntax expression, bool treatAsBoolean = false)
724756
{
725757
var typeInfo = _semanticModel.GetTypeInfo(node.SelectStatement.Expression);
726758

@@ -731,10 +763,18 @@ private CasePatternSwitchLabelSyntax WrapInCasePatternSwitchLabelSyntax(VBSyntax
731763
SyntaxFactory.DiscardDesignation());
732764
} else {
733765
var varName = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "case"));
734-
//CodeAnalysis upgrade to 3.0.0 needed for VarPattern. Correct text comes out, but tree is invalid so the tests this will generate "CS0825: The contextual keyword 'var' may only appear within a local variable declaration or in script code"
735-
patternMatch = SyntaxFactory.DeclarationPattern(
736-
ValidSyntaxFactory.VarType, SyntaxFactory.SingleVariableDesignation(varName));
737-
expression = SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, SyntaxFactory.IdentifierName(varName), expression);
766+
patternMatch = ValidSyntaxFactory.VarPattern(varName);
767+
ExpressionSyntax csLeft = SyntaxFactory.IdentifierName(varName), csRight = expression;
768+
var caseTypeInfo = _semanticModel.GetTypeInfo(vbCase);
769+
var vbEquality = CommonConversions.VisualBasicEqualityComparison;
770+
if (vbEquality.GetObjectEqualityType(typeInfo, caseTypeInfo) == VisualBasicEqualityComparison.RequiredType.Object) {
771+
expression = vbEquality.GetFullExpressionForVbObjectComparison(csLeft, csRight);
772+
} else {
773+
// We know lhs isn't null, because we always coalesce it in the switch expression
774+
(csLeft, csRight) = vbEquality
775+
.AdjustForVbStringComparison(node.SelectStatement.Expression, csLeft, typeInfo, true, vbCase, csRight, caseTypeInfo, false);
776+
expression = SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, csLeft, csRight);
777+
}
738778
}
739779

740780
var casePatternSwitchLabelSyntax = SyntaxFactory.CasePatternSwitchLabel(patternMatch,

‎CodeConverter/CSharp/TypeConversionAnalyzer.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public ExpressionSyntax AddExplicitConversion(Microsoft.CodeAnalysis.VisualBasic
5959

6060
public (ExpressionSyntax Expr, bool IsConst) AddExplicitConversion(Microsoft.CodeAnalysis.VisualBasic.Syntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, TypeConversionKind conversionKind, bool addParenthesisIfNeeded = false, bool requiresConst = false, ITypeSymbol forceSourceType = null, ITypeSymbol forceTargetType = null)
6161
{
62-
var typeInfo = ModelExtensions.GetTypeInfo(_semanticModel, vbNode);
62+
var typeInfo = _semanticModel.GetTypeInfo(vbNode);
6363
var vbType = forceSourceType ?? typeInfo.Type;
6464
var vbConvertedType = forceTargetType ?? typeInfo.ConvertedType;
6565
bool resultConst = false;

0 commit comments

Comments
 (0)