NoUnused.CustomTypeConstructors: Ignore constructors used in if branches that require themselves
#17 opened on Dec 15, 2020
Repository metrics
- Stars
- (26 stars)
- PR merge metrics
- (No merged PRs in 30d)
Description
We already don't count custom type constructors as used if they appear in case branches that require themselves:
type Value = A | B | C
a =
case value of
A -> A
B -> C
C -> B
b = B
c = C
In the example above, A is never outside of the branch, that we only enter if we already have an A. In other words, if nowhere else we create an A, then we can never create an A here either. Therefore, we won't count this usage of A. If it used anywhere else, then it is used, if it isn't, then we mark the constructor as not used.
(here is another example of this)
We also don't count constructors used in "static" comparisons like a == A, as highlighted here.
The addition I want to suggest is to do the same thing for if branches. The idea was brought originally in https://github.com/jfmengels/elm-review-unused/issues/2#issuecomment-644605706
In the example below, we again need an A to create an A in the then branch. For the same reasons as above, we can not count the usage of A.
type Value = A | B
a =
if a == A then
A
else
B
b = B
We already ignore the construction of A in a == A, so we only need to know that in branch then we can ignore all constructions of A (or in the else branch if it was a /= A or not (a == A)).
I believe that we can reasonably infer most "static" constructions.
I will annotate expressions with ([A], [B]) meaning that constructor A will be ignored in then and B in else.
if a == A then -- ([A],[])
if a /= A then -- ([],[A])
if not (a == A) then -- ([],[A])
Combinations makes things more interesting
&& means we can union the elements in each tuple that represents a condition
if a == A && b == B then -- ([A], []) && ([B], []) --> ([A, B], [])
if a == A && a == B then -- same as above (condition is useless though)
if a == A && b == B && c /= C then -- ([A], []) && ([B], []) && ([], [C]) --> ([A, B], [C])
|| means we need to take the intersection of the elements found in each condition
if a == A || a == B then -- ([A], []) || ([B], []) --> ([], []) --> ignore nothing
if a == A || b == A then -- ([A], []) || ([A], []) --> ([A], [])
If we find A used in a "dynamic" comparison like if a == someFunction A then we don't really care about ignoring that consstructor in either branches, since that usage of A will already mark the constructor as used, and ignored/including branches will not change the result.
I feel like we can come a long way already with the things above. But my highest priority is not having false positives, and I struggle to find how to figure how to handle cases like the ones below, where a constructor appears on both sides of the comparison operator.
if A == A then
if (a, A) == (b, A) then
if (a, A) == (A, b) then
I would love some help figuring out these last cases. If not for this, I think we would already have the feature on master :sweat_smile: If you can think of other cases that we could reasonably handle, or cases that could be a problem, please comment!