Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

convert Select Case [object] using Operators.ConditionalCompareObject... #1128

Merged

Conversation

TymurGubayev
Copy link
Contributor

Problem

#1100: Select Case [object] can have incorrect converted code

Solution

  • Any comments on the approach taken, its consistency with surrounding code, etc.
    GetFullExpressionForVbObjectComparison now has a ComparisonKind parameter (new enum).
    VisitSelectBlock now checks if expression type is object first, in which case different code is generated.
  • Which part of this PR is most in need of attention/improvement?
    • ComparisonKind has Unknown value, which might not be necessary - I added it in case something goes terribly wrong.
    • ComparisonAdjustedForStringComparison() (renamed from EqualsAdjustedForStringComparison()) has return SyntaxFactory.BinaryExpression(GetSyntaxKind(comparisonKind), csLeft, csRight); for the non-Object case. With how the code currently looks like, the comparisonKind will always be Equals here (i.e. the call to GetSyntaxKind(comparisonKind) is unnecessary). I've added it for completeness sake, because otherwise the method could return unexpected ExpressionSyntax should it ever be used somewhere else.
  • At least one test covering the code changed

@TymurGubayev TymurGubayev marked this pull request as draft August 5, 2024 10:56
@TymurGubayev TymurGubayev marked this pull request as ready for review August 5, 2024 10:58
@GrahamTheCoder GrahamTheCoder merged commit 625c207 into icsharpcode:master Aug 13, 2024
2 checks passed
@TymurGubayev TymurGubayev deleted the fix/SelectCaseObject/1 branch August 14, 2024 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants