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.CustomTypeConstructors: Follow the trail of phantom types #4

Closed jfmengels closed 3 years ago

jfmengels commented 4 years ago

Context

The NoUnused.CustomTypeConstructors reports unused custom type constructors. But it doesn't report them if the custom type only has a single constructor and it looks like it is used as a phantom type, in the stead of a phantom variable - a type variable never used in the type (I don't know if there is an official name for that).

Problem

At the time of writing, there are cases where phantom types are not well detected.

This rule could do a much better job than it currently does at figuring this out, by following the definitions of custom types and type aliases, until it finds out that the type variable is not used, or that it hits an opaque type. In the meantime, the rule is configurable for the user to specify which type variables for a type are "phantom" ones.

When an opaque type is defined in a dependency, we don't know whether a type variable should be considered as a phantom one, so the need for configuration will always be there, but we can highly reduce the need for this I think.

What I'd like the rule to detect

Case 1: If a dependency defines and exposes:

module Exposed (CustomType(..))

type CustomType a = A | B

we can determine that a is a phantom variable.

Case 2:

module Exposed (CustomType(..), AliasToCustomType)

type CustomType a = A | B
type alias AliasToCustomType a = CustomType a

we can determine that a in AliasToCustomType is a phantom variable.

Case 3:

module Exposed (CustomType(..), AliasToCustomType)

type CustomType a = A | B
type alias AliasToCustomType a b = ( CustomType a, CustomType b )

we can determine that a and b in AliasToCustomType are phantom variables.

Case 4:

module Exposed (CustomType(..), AliasToCustomType)

type CustomType a = A | B
type alias AliasToCustomType a = { thing : CustomType a }

we can determine that a in AliasToCustomType is a phantom variable.

Case 4:

module Exposed (CustomType(..), AliasToCustomType)

type CustomType a = A | B
type alias AliasToCustomType a = { a : a, thing : CustomType a }

we can determine that a in AliasToCustomType is NOT a phantom variable.

We can apply this logic recursively until we find that a very nested type has a type variable, using the help from the user in the configuration.

I think the https://package.elm-lang.org/packages/ianmackenzie/elm-units/ is a very good test case (as a dependency to a project) to work against, due to how nested phantom variables can go.

jfmengels commented 3 years ago

This is likely not worth the effort. Instead, we should push users towards making it clear that their type is a phantom type.