Report custom types with 1 constructor and without any data #94
Replies: 4 comments
-
I think it should report an error here and explain that if you are using this as a phantom type, change |
Beta Was this translation helpful? Give feedback.
-
Oh that's interesting! That's a good way of marking it as a phantom type for I am now thinking that we should probably also never expose the constructors of a type that contains |
Beta Was this translation helpful? Give feedback.
-
Yeah, making sure the constructor of |
Beta Was this translation helpful? Give feedback.
-
I found a use-case where a data-less constructor is useful. -- Made opaque and created only for admins
type ProofOfAdminPermissions = ProofOfAdminPermissions
type Role
= Admin ProofOfAdminPermissions
| User
deleteDatabase : Proof -> Cmd Msg
deleteDatabase _ = ... Here,
These cases are rare, but I am less convinced that this rule is a good idea in all cases. Of course, it is possible to recommend they be changed to I wonder if there is some other way to detect when types become like |
Beta Was this translation helpful? Give feedback.
-
What the rule should do:
Report when a custom type does not hold any data.
What problems does it solve:
I found code that started like
but got trimmed down because of
elm-review-unused
rules totype A = B
, and that's where these rules stop reporting anything.The model still has a field of type
A
and there is code that uses itBy reporting
A
and telling the user to remove it, we will force them to remove thea
field and to simplify the different branching that happens in the code for no reason.Example of things the rule would report:
Example of things the rule would not report:
When the type is used in the stead of a phantom type variable.
Such custom types are typical use-cases for phantom types.
I am wondering whether we should report these types when they're exposed as part of the module. I would like to think that yes, unless they are used in the stead of a phantom type variable somewhere in this file, but I wonder whether there are edge cases where that would be ok.
When (not) to enable this rule:
I am looking for:
elm-review-unused
, as it's kind of a useless thing. What do you think?Beta Was this translation helpful? Give feedback.
All reactions