SiriusStarr / elm-review-no-unsorted

elm-review rules to ensure sortable code is sorted in the "proper" order
GNU General Public License v3.0
3 stars 1 forks source link

Fixes suggested by NoUnsortedTopLevelDeclarations create invalid Elm code #3

Closed r-k-b closed 3 years ago

r-k-b commented 3 years ago

Problem

The fixes suggested by NoUnsortedTopLevelDeclarations fail to apply. ``` [rkb@nixos:~/projects/tulars]$ elm-review --fix-all -- ELM-REVIEW ERROR ------------------------------------------- app/Main.elm:1:1 (FIX FAILED) NoUnsortedTopLevelDeclarations: Top-level declarations are not sorted. 1| module Main exposing (closeTabAt, main, pickUpFood) ^^^^^^ Top-level declarations were found out of order. They should be sorted as specified in the rule configuration. I failed to apply the automatic fix because it resulted in invalid Elm code. app/Main.elm ↑ ====o======================================================================o==== ↓ app/View.elm -- ELM-REVIEW ERROR ------------------------------------------- app/View.elm:1:1 (FIX FAILED) NoUnsortedTopLevelDeclarations: Top-level declarations are not sorted. 1| module View exposing (view) ^^^^^^ Top-level declarations were found out of order. They should be sorted as specified in the rule configuration. I failed to apply the automatic fix because it resulted in invalid Elm code. I tried applying some fixes but they failed in ways the author(s) didn't expect. Please let the author(s) of the following rules know: - NoUnsortedTopLevelDeclarations I found 2 errors in 2 files. ```

repro

$ git clone git@github.com:r-k-b/tulars.git

$ cd tulars

$ git checkout 64028a61954c7794b5d8fbbe05add82a8a3f6f61

$ node --version
v14.17.6

$ npm i

$ elm-review --version
2.5.5

$ elm-review --fix-all
SiriusStarr commented 3 years ago

Doing some quick investigation my guess is that this is due to this upstream issue: stil4m/elm-syntax#136

Lambdas without wrapping parentheses have incorrect ranges due to that elm-syntax bug. I can try to write a workaround, but it may be a few days, as I'm quite busy at the moment.

In the meantime you can put parentheses around the lambda in View.append (line 721) and Main.justSomethings (line 1630) and run elm-review without having run elm-format (which will strip them out again) and it should work fine (and once it's sorted, the range issue isn't a problem).

r-k-b commented 3 years ago

All good, there's no rush or obligation. Thanks for taking a look!

SiriusStarr commented 3 years ago

Closed by upstream stil4m/elm-syntax@eb7ba44069b9c9db9f71c39ad2ef1ba9e85ab85e and fixed here in b4103b8a7669db91a06a8af8f34fe5a21f880d90.

Please update to v1.0.4 and all should be good!