jfmengels / elm-review-unused

Provides elm-review rules to detect unused elements in your Elm project
https://package.elm-lang.org/packages/jfmengels/elm-review-unused/latest/
BSD 3-Clause "New" or "Revised" License
23 stars 12 forks source link

Don't report custom type constructor args used in comparisons #35

Closed jfmengels closed 3 years ago

jfmengels commented 3 years ago

Fixes #25

When used in comparison, the arguments of a custom type constructor won't be reported.

-- Not reported
value == Foo 1
-- Still reported
value == fn (Foo 1)

@r-k-b This won't handle the case for assoc-list and I don't have a good idea to handle it. I added a warning in the rule's documentation though.

Please try it out:

elm-review --template jfmengels/elm-review-unused/preview#issue-25 --rules NoUnused.CustomTypeConstructorArgs

cc @lydell @jpagex

lydell commented 3 years ago

Cool!

I tried to simplify down my case:

module Simple exposing (update, view)

import Html exposing (Html)

type alias Model =
    { version : Version
    }

type Version
    = LatestVersion String
    | OutOfDateVersion

update : String -> Model -> Model
update justFetchedVersionString model =
    { model
        | version =
            if model.version == LatestVersion justFetchedVersionString then
                model.version

            else
                OutOfDateVersion
    }

view : Model -> Html msg
view model =
    case model.version of
        LatestVersion _ ->
            Html.text ""

        OutOfDateVersion ->
            Html.text "You need to update!"

This is the result:

❯ npx elm-review --template jfmengels/elm-review-unused/preview#issue-25 --rules NoUnused.CustomTypeConstructorArgs
npx: installed 125 in 8.083s
✔ Fetching template information
-- ELM-REVIEW ERROR --------------------------------------- src/Simple.elm:12:21

NoUnused.CustomTypeConstructorArgs: Argument is never extracted and therefore
never used.

11| type Version
12|     = LatestVersion String
                        ^^^^^^
13|     | OutOfDateVersion

This argument is never used. You should either use it somewhere, or remove it at
the location I pointed at.

I found 1 error in 1 file.

This shouldn’t be reported, should it? Or am I missing something 🤔

jfmengels commented 3 years ago

Yeah, it shouldn't. I made some mistakes in my tests and forgot to handle an essential thing. It should be fixed now though!

lydell commented 3 years ago

Thanks! Yes, now it seems to be fixed. Awesome! 🥇

jfmengels commented 3 years ago

Alright, released in 1.1.7 :)