Skip to content

Conversation

jrmoreno1
Copy link
Contributor

@jrmoreno1 jrmoreno1 commented Jul 27, 2025

  • Added unit test to AccessExpressionTest.cs
  • CommonConversions.cs
    • use SyntaxFactory.ParenthesizedExpression to Parenthesize the converted expression if it is a ConditionalExpressionSyntax
  • Ran all test, no change in existing test, new test passes after initially failing.

Link to issue(s) this covers

#1170

Problem

C# ternary expression do not inherently have a termination, unlike VB ternary. When converted without parameterization, IncreaseArrayUpperBoundExpressionAsync adds 1 to the false result.

Solution

  • Is mutating convertedExpression acceptable, or should it be it's own return?
  • All existing test continue to pass
  • There is now a test for this scenario.

- Added unit test to AccessExpressionTest.cs
- CommonConversions.cs
  - use SyntaxFactory.ParenthesizedExpression to ParenthesizedExpression the converted expression if it is a ConditionalExpressionSyntax
- Ran all test, no change in existing test, new test passes after initially failing.

icsharpcode#1170
@GrahamTheCoder GrahamTheCoder merged commit add696c into icsharpcode:master Jul 27, 2025
2 checks passed

if (convertedExpression is CSSyntax.ConditionalExpressionSyntax ce) {
convertedExpression = SyntaxFactory.ParenthesizedExpression(convertedExpression);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general the codebase is easier to reason about if you avoid using the shape of the output to influence further output. This is simple enough for now though, so let's go with it!

@GrahamTheCoder
Copy link
Member

Thanks!

@jrmoreno1 jrmoreno1 deleted the Issue1170 branch July 29, 2025 17:06
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