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

A simple API to add imports. #170

Open miniBill opened 6 months ago

miniBill commented 6 months ago

Probably Fix.withAddImport : ModuleName -> Maybe String -> List Exposing -> Fix -> Fix?

dillonkearns commented 6 months ago

I've encountered the need for adding imports as well. From my experience, the API you're proposing wouldn't address my needs. I think it's important to allow for adding a missing import, or if there is an existing import already, getting context for how to refer to that item. This is needed in the case where the target code uses import aliases. For example:

import Html.Attributes as Attr exposing (style)

If you added Fix.withAddImport [ "Html", "Attributes" ] ..., you would have no way to refer to the import alias, Attr, so you wouldn't be able to correctly reference that module.

Here is an elm-review rule where I add an import if needed, or if it already exists I put info about the existing import into the Context so I can add to it.

https://github.com/dillonkearns/elm-pages/blob/a2356d3b8b50fde9cdfb6635c56255f96c784b86/generator/dead-code-review/src/Pages/Review/DeadCodeEliminateData.elm#L278-L291

I gather the import context like this:

https://github.com/dillonkearns/elm-pages/blob/a2356d3b8b50fde9cdfb6635c56255f96c784b86/generator/dead-code-review/src/Pages/Review/DeadCodeEliminateData.elm#L23-L61

So maybe it could be something similar to Review.Rule.withModuleNameLookupTable, where it lets you put something into the Context that you can use to refer to an import? I'm not sure what the cleanest way to have both a Fix and Context would be off the top of my head... one possibility would be having the Fix in the Context, but maybe there's a cleaner way?

dillonkearns commented 6 months ago

Building something with withImportVisitor:

withImportVisitor :
    ( Node Import
      -> moduleContext
      -> ( List (Error {}), moduleContext )
    )
    -> ModuleRuleSchema schemaState moduleContext
    -> ModuleRuleSchema
           { schemaState | hasAtLeastOneVisitor : () }
           moduleContext

It should be possible to have an API like this that gives both a fix and updates Context I would think:

withImport : (ImportReference -> context1 -> context2) -> ModuleName -> ModuleRuleSchema schemaState context1 -> ModuleRuleSchema
           { schemaState | hasAtLeastOneVisitor : () }
           context2

reference : ImportReference -> String -> String
-- example, `reference context.htmlAttr "class"` -> "Attr.class"