Language Service prefers original parse results

[psfinaki]

See @dsyme's comment down below.

[psfinaki]

@dotnet/fsharp-team-msft and maybe @dsyme, please see the description and look carefully :) either I have identified some useless code deep down in the repo, or this is actually some corner case that might be useful for some other scenarios.

I mean, either way, assert false is not the best thing to do in code, but we might want to adjust things here instead of just deleting the logic.

[psfinaki]

/run fantomas

[github-actions]

Ran fantomas: https://github.com/dotnet/fsharp/actions/runs/3438227723

[kerams]

The error message would indicate some unexpected weirdness with node ranges. fun ((a, b), c) -> () AST is not what I would have expected for instance.

[dsyme]

Yeah the problem is that fun ((a, b), c) -> ... "pushes" the (a,b) pattern to the right hand side of the fun during parsing, rather than during checking. Giving

fun (_arg1, c) -> match _arg1 with (a, b) -> ...

When this happens the ranges associated with the synthetic _arg1 and/or the match are then both claiming a position.

OK so for SynExpr.Lambda the original parse information is held in parsedData holding the original (args, body) before the above is done. See https://github.com/dotnet/fsharp/blob/main/src/Compiler/SyntaxTree/SyntaxTree.fsi#L646

So @psfinaki I think the solution is to systematically have all the non-typechecking SyntaxTree traversals work over the parsedData rather than the args/body. This should be done throughout the whole compiler (e.g. grep for | SynExpr.Lambda)

That is, we should go through all the SyntaxTree travels walking over SynExpr.Lambda and adjust them to do sub-tree traversals on parsedData rather than the immediately available args and body.

I think this will then make the trees look OK and you should be able to keep the assert I think

Add a comment to the assert point out that you can analyse parse trees using sharplab.io or the Fantomas tools, and that the problem may be to do with some synthetic transfomation happening after parsing.

[kerams]

Out of curiousity, what's the rationale behind such a transformation?

[psfinaki]

/run fantomas

[github-actions]

Ran fantomas: https://github.com/dotnet/fsharp/actions/runs/3470716892

[nojaf]

A bit of a heads up here, the body or parsedData isn't perfect.
Some examples where you still have _arg in Match expressions.

[dsyme]

@nojaf looks like we should address those too, whether as part of this PR or separate

[psfinaki]

This got stale, will close it.