search

jfmengels / elm-review

Analyzes Elm projects, to help find mistakes before your users find them.
https://package.elm-lang.org/packages/jfmengels/elm-review/latest/
Other
252 stars 13 forks source link

Broken Elm code when the moved code contained \" #127

Closed lucamug closed 2 years ago

lucamug commented 2 years ago

I got several times broken Elm code if the autofix feature move a string containing \". For example, this line of Elm

    , en_us = "An error occurred during authentication. Please try again or register your device from \"Go to password input screen\"."

was transformed to

    , en_us = "An error occurred during authentication. Please try again or register your device from " Go to password input screen "."

also

    , en_us = """You can update your email address by answering to "Secret Question and Answer". **Your email address will only be updated once you have reset your password, not after submitting this form.**"""

was transformed to

    , en_us = "You can update your email address by answering to " Secret Question and Answer ". **Your email address will only be updated once you have reset your password, not after submitting this form.**"

This case in particular happened with the rule NoUnsortedRecordFields but I think it happened with other rules too, if I remember correctly.

jfmengels commented 2 years ago

Hey Luca,

Thank you for the issue. Could you create an SSCCE maybe? That would help knowing whether a solution I could implement solves the issue or not.

I would have to look at whether it's the rule you mentioned is doing something incorrect or weird, or if it's elm-review. Anyway, elm-review should have noticed there was a problem after the fix, so in any case I think this is something that should be improved 👍

jfmengels commented 2 years ago

I just re-read the issue, and the result is valid Elm syntax, though a weird one. I don't think there's much that elm-review can do about this, at least with our current means of checking the result of a fix (checking whether the new code is syntactically valid Elm code). We could do more by running the compiler but that would be very slow, and I'd hope to be able to avoid that as much as possible.

I will close the issue as I don't know of anything else this tool could do. Please open an issue in the project exposing NoUnsortedRecordFields, and if there is something else that you think elm-review could help with, please open a new issue here.