search

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

False positive for "shadowed" type #5

Closed Janiczek closed 3 years ago

Janiczek commented 4 years ago

Describe the bug When a module defines a type alias for a custom type (of the same name, from different module), the constructors of the custom type used in the module aren't taken into consideration in the NoUnused.CustomTypeConstructors rule.

SSCCE (Short Self-Contained Correct Example) Steps to reproduce the behavior:

  1. Create these two files: 
    
-- A.elm
module A exposing (x)

import B exposing (Foo(..))

type alias Foo = B.Foo -- this "shadowing" is causing the bug, removing the alias removes the false positive

x = Foo 1 -- usage of the custom type constructor!

```elm
-- B.elm
module B exposing (Foo(..))

type Foo = Foo Int
  1. Make your elm-review rules contain NoUnused.CustomTypeConstructors.rule []
  2. Run elm-review
  3. See false positive

Expected behavior I'd expect not to see warning about the B.Foo constructor being unused, because I'm using it in A.x.

jfmengels commented 4 years ago

This was a confusing one! I tried it out, and you're right that this code works and should not be reported.

I have created failing test cases for this one, should anyone feel like taking this one https://github.com/jfmengels/elm-review-unused/tree/failing-tests-for-issue-5

I noticed that this false positive becomes a true positive when you change A.Foo to type alias Foo = { name : Int } as that declaration adds a function to the scope, which overrides the import. I have added a test case for that one in the branch so that we don't introduce this problem :slightly_smiling_face: