Skip to content

fix: Null coalescing not own its line with complex left-hand side#1770

Merged
belav merged 1 commit intobelav:mainfrom
ogaken-1:null-coalescing-null-propagation-property
Dec 13, 2025
Merged

fix: Null coalescing not own its line with complex left-hand side#1770
belav merged 1 commit intobelav:mainfrom
ogaken-1:null-coalescing-null-propagation-property

Conversation

@ogaken-1
Copy link
Copy Markdown
Contributor

@ogaken-1 ogaken-1 commented Dec 7, 2025

There was an issue where if the left-hand side of ?? was even slightly complex, the right-hand side would become part of the group and could not have its own line.

I reconsidered the conditions for grouping and changed it from judging the complexity of the left-hand side by a threshold to a condition where "the left-hand side is an InvocationExpression and consists solely of MemberAccessExpression that does not include an InvocationExpression".

fix #1769

belav
belav previously approved these changes Dec 8, 2025
Copy link
Copy Markdown
Owner

@belav belav left a comment

Choose a reason for hiding this comment

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

Looks good, I'm just waiting on my script to create a PR on https://github.com/belav/csharpier-repos/pulls to see if there are any edge cases you may have missed.

@ogaken-1
Copy link
Copy Markdown
Contributor Author

ogaken-1 commented Dec 8, 2025

Upon reviewing the PR that was created, I noticed there are some oversights. I'll work on the corrections later.

@ogaken-1 ogaken-1 force-pushed the null-coalescing-null-propagation-property branch from 472f5dc to 80f1178 Compare December 8, 2025 16:49
@ogaken-1
Copy link
Copy Markdown
Contributor Author

ogaken-1 commented Dec 8, 2025

After reviewing the results in the csharpier-repos, I have added support for the following:

  • ParenthesizedExpressionSyntax
  • CastExpressionSyntax { Expression: ParenthesizedExpressionSyntax }
  • ConditionalAccessExpressionSyntax

@ogaken-1 ogaken-1 requested a review from belav December 8, 2025 17:02
@ogaken-1
Copy link
Copy Markdown
Contributor Author

@belav
I made additional changes thinking they would be an improvement, but perhaps they weren't?
I would be grateful for your opinion. Thank you.

@belav
Copy link
Copy Markdown
Owner

belav commented Dec 12, 2025

Taking a look at the changes it made I think everythink looks good except this one - belav/csharpier-repos@e097ff1#diff-bb147afabd02afcf48b4b4c09aad50f6bb117fea0bae21a1b2f1adef34f16beb

Your first pass had ?? false on a new line but the most recent version keeps it on the same line.

I don't see a test case for it which is probably why it ended up being undone.

Was there something specific you didn't think was an improvement? I do like the way it groups it with the ending paren

) ?? result

@belav
Copy link
Copy Markdown
Owner

belav commented Dec 12, 2025

Ideally I'd have an action for doing the formatting on csharpier-repos - I think I attempted it a while back and it wasn't super straightforward.

@ogaken-1 ogaken-1 force-pushed the null-coalescing-null-propagation-property branch from 1fe8361 to 8ec1e94 Compare December 13, 2025 08:39
@ogaken-1
Copy link
Copy Markdown
Contributor Author

Added support for when the leftmost element is a parenthesized expression.

There was an issue where if the left-hand side of `??` was even
slightly complex, the right-hand side would become part of the group
and could not have its own line.

The grouping conditions were reviewed and changed from a threshold-based
determination of left-hand side complexity to condition limited to cases
where:
- "The left-hand side is an InvocationExpression and consists solely of
  MemberAccessExpressions that do not contain InvocationExpressions" OR
- "The left-hand side is a ParenthesizedExpression".
@ogaken-1 ogaken-1 force-pushed the null-coalescing-null-propagation-property branch from 8ec1e94 to abcf5d8 Compare December 13, 2025 12:05
Copy link
Copy Markdown
Owner

@belav belav left a comment

Choose a reason for hiding this comment

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

Looks good!

@belav belav merged commit f104105 into belav:main Dec 13, 2025
7 checks passed
@ogaken-1 ogaken-1 deleted the null-coalescing-null-propagation-property branch December 13, 2025 17:10
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.

The null coalescing operator is grouped in an unexpected place

2 participants