Parens: Fix some parenthesization corner-cases in record expressions

[brianrourkeboll]

Another followup to #16079.

Description

• Keep parens around problematic expressions (sequential, try-with/finally, matches, lambdas, etc.) in record copy expressions.

  { (try { A = 3 } with _ -> reraise ()) with A = 4 }

• Keep parens around dangling problematic expressions in record field assignments when there are subsequent record fields on the same line.

  { A = (fun x -> printfn $"{x}"); B = 3 }

Checklist

• Test cases added.
• Release notes entry updated.

[psfinaki]

Good stuff, Brian, thanks.

A thought on testing (again). Looking at those followups... one way to reorganize the massive amount of testing material would be to have the "basic" test cases that you added at the very beginning and then to separately have all those corner cases we keep encountering. This way, there will be more code duplication - but on the other hand, the PRs would be somewhat clearer and the tests won't be so intimidating.

It would be some resemblant of other test suites we have all around the code base which have some big theories of initial testing and then things like "bug 12345 - corner case X", "regression 23456 - corner case Y" and so on.

Just a thought.

[smoothdeveloper]

for the test, I think when time comes (of regression / issues being found), it can be refactored from literal tuples to a function call that construct a case with a case name, and the two strings

fixParensTestCase "name of the case"
   """ ... """
   """ ... """

I think it is good to keep it as declarative and avoid bloat of many tests in the test explorer, if the test runs fast enough.

It is good if @brianrourkeboll has a name for those cases already and we could make this adjustment.

I also personally prefer the cases to be separate files (including for trying out in a debug version of VS, just by opening the .fsx for the case), but it will be trivial to refactor to support both.

I noticed also, that xunit test runners, when running a single "case" of a "theory" with parameters, actually run all of them, at least that's how it looked like when putting breakpoint in debugger and running a single case from test explorer in both Rider and VS, so we aren't earning efficiency to run a single case.

[brianrourkeboll]

@psfinaki

... one way to reorganize the massive amount of testing material would be to have the "basic" test cases that you added at the very beginning and then to separately have all those corner cases we keep encountering.

Hmm, while I did call the two cases addressed in this PR "corner-cases" in the title, they're arguably just run-of-the-mill (albeit relatively uncommon) expression nestings that I just didn't think of the first time around.

That's partly why I took the more comprehensive approach I did in #16313, and why I suggested that I might eventually want to try something similar for expressions—to generate nestings that I might never think of, since there's no way I will ever think of them all manually.

In my mind, the true "corner-cases" for this analysis are ones where semantically irrelevant differences in the source are represented significantly differently in the AST, like let (… : …) = … versus let … : … = …, fun (()) -> … versus fun () -> …, or #16254 and #16257.

So if we want to try to separate "regular" tests from "corner-case" tests here, I think we'll need to agree on a definition of "corner-case" 🙂:

• Analyzer/code fix behavior that is likely to be surprising to the programmer, e.g., not allowing removal of parentheses when a programmer not steeped in all of the parsing rules and their interactions is likely to think removal should be allowed?
• Source code whose AST representation is surprising to the analyzer author?
• Something else?

Either way, @smoothdeveloper is right that greater comprehensiveness can come at the expense of greater unwieldiness. Whenever I do get around to looking at the expression tests as a whole again, I'll definitely strive for some kind of balance.

[psfinaki]

I think the lines would be blurring anyway here - I mean those between normal and corner cases. Hence it's fine to be somewhat subjective and hence we can pick the easiest approach to the subjectiveness which would be your perspective aka the analyzer author perspective :)