jfmengels / elm-review-rule-ideas

Rule ideas for elm-review
https://github.com/topics/elm-review
9 stars 0 forks source link

UnreachableCase rule #8

Closed MartinSStewart closed 4 years ago

MartinSStewart commented 4 years ago

What the rule should do: This rule checks if a function called unreachableCase really is unreachable. The function is defined as

unreachableCase : () -> a
unreachableCase () = unreachableCase ()

The use for this is to avoid having to deal with type variants that should never occur. For example

email : Email
email = 
    case Email.parse "blah@test.com" of 
        Just email -> email
        Nothing -> unreachableCase ()

This is essentially a generalization of https://jfmengels.net/safe-unsafe-operations-in-elm/

How I think this rule will work:

  1. Find an unreachableCase function call
  2. Check that it's the return value in a pattern match
  3. Get the expression inside the case...of
  4. Recursively find all the references from that expression to other functions or values and build up a tree of function dependencies
  5. If any of the leaves of the tree end with something that isn't a literal value then the rule fails
  6. Run an interpreter that starts from the leaves to compute the final value inside the case...of*. Here's a talk showing how that can be done https://www.youtube.com/watch?v=afMD-hkWPsQ
  7. If the value computed doesn't lead to unreachableCase then the rule passes, otherwise it fails.

*Since we are running an interpreter, we can count how many steps we've taken and deterministically timeout to prevent getting caught in an endless loop. The deterministic part is important as it prevents this rule from passing on your dev machine and then failing during CI.

Example of things the rule would report:

email : Email
email = 
    case Email.parse "bad email address" of 
        Just email -> email
        Nothing -> unreachableCase ()
value = 
    if 1 == 0 then
        Just ""
    else 
        Nothing

email : Email
email = 
    case value of 
        Just text -> text
        Nothing -> unreachableCase ()
text : String
text = 
    if True then
        ""
    else
        unreachableCase ()

Here the unreachableCase isn't inside a pattern match. While this situation probably could be handled, I don't see any value in supporting this.

result : Int
result = 
    case longRunningComputation of 
          Just value -> value
          Nothing -> unreachableCase ()

Here longRunningComputation will eventually return Just but it takes too many steps so the rule times out and fails.

Example of things the rule would not report:

email : Email
email = 
    case Email.parse "blah@test.com" of 
        Just email -> email
        Nothing -> unreachableCase ()

I am looking for:

MartinSStewart commented 4 years ago

I've started on a proof of concept for this idea using elm-script instead of elm-review for collecting Elm modules. My experience from working on this so far is that I think it is complicated enough that it's better to have as a stand alone tool rather than have it as an elm-review rule.

I think it makes sense then to close this issue.

jfmengels commented 4 years ago

Sure, let us know how that goes. I would love to know what (else) you would need in elm-review to make this possible and easier.