Fix uncurry attribute on function application

[anmonteiro]

fixes #2565

cc @bobzhang

[bobzhang]

A bit confused why this is not resolving in the parsing stage instead of post-processing

[anmonteiro]

@bobzhang could you clarify your question? The change I made is in the parser

[bobzhang]

My question is that when parser see f(. a,b), it parse it as uncurried function, why a post-processing is involved here?

[anmonteiro]

I'm not sure, looks like legacy code that's moving attributes from the expression to the structure item.

[bobzhang]

For this particular patch, you can use List.exists

[anmonteiro]

I can certainly switch to that, though I wanted to make the diff as easy to review as possible.

The bug here is that we were checking the entire expression's attributes (could be a let .. in) instead of the application expression's attributes for an uncurry attribute.

[bobzhang]

do you have an example of what mkstrexp is trying to achieve? I am a bit lost here
Maybe you can add a test case

[bobzhang]

ping?

[anmonteiro]

happy to merge if you're happy with it. I don't have bandwidth to get rid of mkstrexp, or dig through history to find why it was added in the first place.

Otherwise I'm also happy to close this PR and let you fix the issue.

[bobzhang]

it seems to be introduced by this commit when introducing uncurried support: dabe031
@IwanKaramazow would you have a look why we need change mkstrexp here? thanks

[IwanKaramazow]

Some context: [@attr] expr is Pstr_eval (expr, @attr). The purpose of mkstrexp is building a Pstr_eval that contains the attributes, not the expression inside the Pstr_eval.
I misunderstood this back in the days when implementing the original uncurried support.
[@bs] should not be lifted to the Pstr_eval level. I'm wondering if this branch https://github.com/facebook/reason/blob/535eb95cacef59eb6dc1ca8c34d053d16ba089df/src/reason-parser/reason_parser.mly#L432-L449 can be removed. Printer will need to be fixed.