Skip to content

Parens: Keep parens for non-identical infix operator pairs with same precedence #16372

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

Merged
merged 11 commits into from
Dec 15, 2023

Conversation

brianrourkeboll
Copy link
Contributor

@brianrourkeboll brianrourkeboll commented Dec 3, 2023

Another followup to #16079.

Description

Keep parentheses for non-identical infix operator pairs that have the same precedence, e.g.:

f <| (g << h)
x + (y ++ z)

etc.

We still offer to remove them when both operators are the same, e.g.:

x + (y + z) // → x + y + z

except for the left-associative "relational" operators (=…, |…, &…, $…, >…, <…, !=…, including |>, <|, >>, <<, etc.), where the order of operations is often arguably non-obvious:

f <| (g <| x)
x <> (y <> z)
x > (y < z)

Checklist

  • Test cases added.
  • Release notes entry updated.

* Keep parens in cases like `f <| (g << h)`, `x + (y ++ z)`, etc.
* The order of operations in `x > (y < z)` or `x <> (y <> z)` is not
  necessarily immediately obvious, and there are built-in operators in
  this class like `<|` for which the associativity of the underlying
  operation depends on the types of the operands.
@brianrourkeboll brianrourkeboll marked this pull request as ready for review December 3, 2023 21:36
@brianrourkeboll brianrourkeboll requested a review from a team as a code owner December 3, 2023 21:36
@psfinaki
Copy link
Member

@brianrourkeboll this can also go once the release notes conflict is resolved :)

@dotnet/fsharp-team-msft please take a look

@psfinaki psfinaki enabled auto-merge (squash) December 12, 2023 15:50
auto-merge was automatically disabled December 13, 2023 20:00

Head branch was pushed to by a user without write access

@brianrourkeboll
Copy link
Contributor Author

All right @psfinaki, I think this should finally be good now that the release notes PR has been merged in.

@psfinaki
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@psfinaki psfinaki enabled auto-merge (squash) December 15, 2023 13:09
@psfinaki psfinaki merged commit fd642a9 into dotnet:main Dec 15, 2023
@brianrourkeboll brianrourkeboll deleted the parens-infix-fix branch December 15, 2023 17:40
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.

3 participants