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: Ignore constructors used in if branches that require themselves #17

Open
jfmengels opened this issue Dec 15, 2020 · 0 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@jfmengels
Copy link
Owner

jfmengels commented Dec 15, 2020

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 #2 (comment)

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 😅
If you can think of other cases that we could reasonably handle, or cases that could be a problem, please comment!

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

No branches or pull requests

1 participant