Fix issue 503 - Githubissues

ocaml-ppx / ppxlib

Base library and tools for ppx rewriters

MIT License

246 stars 98 forks source link

Fix issue 503 #504

Closed jchavarri closed 3 months ago

jchavarri commented 3 months ago

Fixes #503.

In the first commit, notice the difference in the ending location of the Pexp_fun with the labelled bar.

In 5.1: https://github.com/ocaml-ppx/ppxlib/blob/4661a93e6029f86bd3e0d3fa6d519fe4c658a6dc/test/501_migrations/one_migration.t#L1726

In 5.2: https://github.com/ocaml-ppx/ppxlib/blob/4661a93e6029f86bd3e0d3fa6d519fe4c658a6dc/test/502_migrations/one_migration.t#L719

The second commit updates Migrate_502_501 so that the ranges of locations of Pexp_fun stay consistent with the behavior in 5.1.

jchavarri commented 3 months ago

Hi @NathanReb thanks for the quick review :)

I applied the following changes:

In https://github.com/ocaml-ppx/ppxlib/pull/504/commits/b8e11706d460c20929286af2f3dea4d217c41ba2, I reverted the fix and added a small exe that checks the locations integrity for the given AST nodes
In https://github.com/ocaml-ppx/ppxlib/pull/504/commits/6cc652ce3c322c6c913f9325fb75e9d02e40c53a, I re-applied the fix and can see how the tests errors are gone

I also removed the previous location tests.

Please lmk if something like this is what you had in mind.