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

NoUnused.Exports: Tests without a type annotation are reported as unused #93

Open jfmengels opened 2 months ago

jfmengels commented 2 months ago

In this example, someTest would incorrectly be reported as unused, because it doesn't have a type annotation indicating it is of type Test.Test.

-- tests/SomethingTest.elm
module SomethingTest exposing (..., someTest)

import Test

someTest = Test.test ...

We could check that the body has some specific shape (no arguments, and uses Test.test, but then you'd have the following false positive when indirection is created:

module SomethingTest exposing (..., someTest)

import Test

createTest = Test.test
someTest = createTest ...

This is something where type inference could fix the issue

Even with type inference, we could end up with false positives if a type aliases Test.Test:

module SomethingTest exposing (..., someTest)

import Expect exposing (Expectation)
import Test

type alias CustomTest = Test.Test
createTest = String -> (() -> Expectation) -> CustomTest

createTest = Test.test
someTest = createTest ...

We'd therefore need to have both type inference and resolve aliases.


Note: It seems like we're checking for a Test import to mark a module as used:

        , usedModules =
            if Set.member "Test" moduleContext.importedModules || moduleContext.containsMainFunction then

source

A better check, that would not suffer from indirect aliasing, would be to check whether the file exposes any value of type Test.Test.