Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NoUnused.CustomTypeConstructors: Follow the trail of phantom types #4

Closed
jfmengels opened this issue May 29, 2020 · 1 comment
Closed
Labels
hacktoberfest help wanted Extra attention is needed

Comments

@jfmengels
Copy link
Owner

jfmengels commented May 29, 2020

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 jfmengels transferred this issue from jfmengels/review-unused Aug 14, 2020
@jfmengels jfmengels added hacktoberfest help wanted Extra attention is needed labels Sep 27, 2020
@jfmengels
Copy link
Owner Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant