Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decouple error from fix location #124

Open
lue-bird opened this issue May 16, 2022 · 2 comments
Open

Decouple error from fix location #124

lue-bird opened this issue May 16, 2022 · 2 comments

Comments

@lue-bird
Copy link
Contributor

lue-bird commented May 16, 2022

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
Copy link
Owner

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 🤔

@lue-bird
Copy link
Contributor Author

lue-bird commented Feb 22, 2024

Some more examples of where this would be useful:

  • a rule that reports function declarations where arguments are ignored. The fix – removing that argument from all usages and the declaration – is only possible in one go, with fixes in multiple modules
  • the same rule for unused arguments in type declarations (aka for the forbid phantom types rule)
  • renaming a declared thing in all usages and the declaration, e.g. converting it to camel case or removing unnecessary _-suffixes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants