haskell / error-messages

73 stars 18 forks source link

Non-exhaustive pattern match warning with guards #20

Open noughtmare opened 2 years ago

noughtmare commented 2 years ago

In this stackoverflow question the asker presents this code:

data Tree = Leaf | Node Int Tree Tree
  deriving (Eq, Show, Read, Ord)

insert :: Int -> Tree -> Tree
insert n Leaf = Node n Leaf Leaf
insert n tree@(Node num lt rt)
                    | n < num  = Node num (insert n lt) rt
                    | n > num  = Node num lt (insert n rt)
                    | n == num = tree

Which produces the following warning (with -Wall):

Test.hs:5:1: warning: [-Wincomplete-patterns]
    Pattern match(es) are non-exhaustive
    In an equation for ‘insert’:
        Patterns not matched:
            _ (Node _ Leaf Leaf)
            _ (Node _ Leaf (Node _ _ _))
            _ (Node _ (Node _ _ _) Leaf)
            _ (Node _ (Node _ _ _) (Node _ _ _))
  |
5 | insert n Leaf = Node n Leaf Leaf
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...

The list of "patterns not matched" here is strange, it seems to me that the pattern not matched is just _ (Node _ _ _) and I think it should also mention something about the guards.

MorrowM commented 2 years ago

Every pattern is matched except for the guard. If you replace the last guard with an otherwise there are no warnings. Which I suppose is even worse. It's warning about the pattern rather than the guard because the guard is actually part of the pattern. But that's very non-obvious from reading the message.

tomjaguarpaw commented 2 years ago

The "correct" way to write this (i.e. the way that preserves intent and satisfies the pattern match checker) is

insert :: Int -> Tree -> Tree
insert n Leaf = Node n Leaf Leaf
insert n tree@(Node num lt rt) =
    case compare n num of
        LT ->  Node num (insert n lt) rt
        GT -> Node num lt (insert n rt)
        EQ -> n == num = tree
noughtmare commented 2 years ago

It seems that the excessive "Patterns not matched" list is a regression, this older stackoverflow question shows a warning with only one missing pattern as I would expect:

test.hs|6| 1:
||     Warning: Pattern match(es) are non-exhaustive
||              In an equation for `insert': Patterns not matched: _ (Node _ _ _)

@tomjaguarpaw I'm aware, this issue is only about the warning. Maybe there could be a specific suggestion to use compare in the specific cases where guards are used with the >, <, == operators, but I don't think that is the most important change I would like to see.

ketzacoatl commented 2 years ago

@noughtmare great wiki write up on this, that's a stellar example and base to build off of!

noughtmare commented 2 years ago

Thanks. For the record, I made a sample error message catalog page which includes this example here: https://github.com/noughtmare/error-messages/wiki/Pattern-Match(es)-Are-Non-Exhaustive. See also: https://github.com/haskell/error-messages/issues/10#issuecomment-921166440.

goldfirere commented 2 years ago

That wiki page is indeed excellent. I'd personally be quite happy for that to be the on the wiki associated with this main repo instead of the fork. Thanks, @noughtmare!

Ericson2314 commented 2 years ago

If HLint could direct one to the the compare solution, and HLS could somehow know to try the linter to get more suggestions to fix warnings, I think that would be great division of labor.

Maybe we need some sort of "lint plugin" higher order function thing so GHC calls the linter just on the snippets for which is needs more suggestions of alternatives?

I would be a bit suspicious of baking in more wired-in stuff to solve this in GHC proper, but I don't really mind doing that sort of thing in downstream projects.