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

Decouple error from fix location #124

Open lue-bird opened 2 years ago

lue-bird commented 2 years ago

The range the error is shown for can already be different from where fixes are being made.

There are also rules where the use of certain functions/imports/declarations/names/... in one module should trigger fixes in another module. Example: using a name from Nat.N<x> will generate a type alias declaration for <x> in Nat. NoMissingRecordFieldLens already does this, but needs to put the error location on the generation module header name, which leaves users wondering why it was created while showing a rather unhelpful error range. A similar example is generating tests from examples which I've always wanted to implement nicely.

I suggest adding

Review.Fix.inModule : Review.Rule.ModuleKey -> Fix -> Fix

used like

[ -- defaults to error location module
  Review.Fix.removeRange importRange
, Review.Fix.inModule originModule.key
    (Review.Fix.removeRange usedDeclarationRange)
]
jfmengels commented 2 years ago

I think this would be nice to have, as that could make some rules way more powerful.

I think we'd need to think of how to report this to the user in a terminal, in an editor (I'm not sure that multi-file fixes would be compatible with LSP), and how to communicate that to any programs that use the JSON output of the CLI (would probably be a breaking change).

The API for Review.Rule.errorForElmJsonWithFix would not be compatible with the idea, but we could add a new function for that. For instance, it could take an additional argument like List ( ModuleKey, List Fix ). But in that case [ ( moduleKey, Review.Fix.inModule otherModuleKey (Review.Fix.removeRange usedDeclarationRange ) ] would be problematic :thinking:

lue-bird commented 6 months ago

Some more examples of where this would be useful: