2017-07-28

Update: The following article recommends using the -Wincomplete-uni-patterns and -Wincomplete-record-updates warning flags in addition to -Wall. However, a recent GHC Proposal was accepted that adds these flags into -Wall! This blog post is credited in the proposal! In future versions of GHC, you won't have to worry about adding -Wincomplete-uni-patterns and -Wincomplete-record-updates as long as you are using -Wall.

GHC has many great warnings. It will tell you when you have missed a case in pattern matching, when you have shadowed a variable, and when you have unneeded imports. All these warnings can be turned on by using the -Wall flag.

However, there are some useful warnings that are not turned on by -Wall. This post will describe which additional warnings you should enable. It will also show examples of bad code that will trigger the warnings. Keep in mind that this bad code will not get caught by -Wall.

In short, you should be using using this set of warnings:

-Wall -Wincomplete-uni-patterns -Wincomplete-record-updates -Wmissing-import-lists

Here's an example of a library section from a .cabal file with the above options specified:

library
  hs-source-dirs:      src
  exposed-modules:     Lib
  build-depends:       base >= 4.7 && < 5
  default-language:    Haskell2010
  ghc-options:         -Wall -Wincomplete-uni-patterns -Wincomplete-record-updates -Wmissing-import-lists

The Warnings

The following three sections give an explanation of the warnings specified above. They are in order of least-controversial to most-controversial.

-Wincomplete-uni-patterns

If you are already using -Wall, then you should definitely also turn on -Wincomplete-uni-patterns.

-Wincomplete-uni-patterns produces a warning when doing an incomplete pattern match using a lambda. For instance, the following code will not produce a warning even with -Wall:

myHead :: [a] -> a
myHead = \(a:as) -> a

This is basically unforgivable. I can't think of a time when I would want this to not produce a warning.

Turning on -Wincomplete-uni-patterns will produce the following error:

example.hs:2:10: warning: [-Wincomplete-uni-patterns]
    Pattern match(es) are non-exhaustive
    In a lambda abstraction: Patterns not matched: []

Even if you did want to write a partial function like myHead, you should explicitly use error so it will be obvious to someone reading your code. It should be written like this:

myHead :: [a] -> a
myHead (a:as) = a
myHead [] = error "myHead: passed an empty list!"

-Wincomplete-record-updates

Mixing records and sum types is somewhat dangerous. Take the following data type:

data Foo
  = Bar { barInt :: Int, barString :: String }
  | Baz

Foo is a sum type, but Bar has records. The record members barInt and barString can be used in two different ways.

The first way is like a getter. Take a look at their types:

barInt :: Foo -> Int
barString :: Foo -> String

If you pass them a Foo, they will return either an Int or String. However, they are both actually partial functions. See what happens in GHCi when we pass barInt a Baz:

> barInt Baz
*** Exception: No match in record selector barInt

It throws an exception!

The second way that barInt and barString can be used is like a setter:

mySetter :: Int -> Foo -> Foo
mySetter int foo = foo { barInt = int }

This takes an Int and Foo and updates the Int in Foo (using barInt). This is where -Wincomplete-record-updates comes in. If you turn on this flag, you get the following error:

example.hs:14:20: warning: [-Wincomplete-record-updates]
    Pattern match(es) are non-exhaustive
    In a record-update construct: Patterns not matched: Baz

This is exactly what we want!

In fact, I would argue that you should never use records inside of sum types. Instead, you should use one of the following three styles below:

1. Don't use records.

   Here's an example of getting similar functionality from above without using records:

   data Foo
     = Bar Int String
     | Baz
   
   getBarInt :: Foo -> Maybe Int
   getBarInt (Bar int _) = Just int
   getBarInt Baz = Nothing
   
   setBarInt :: Int -> Foo -> Foo
   setBarInt int (Bar _ string) = Bar int string
   setBarInt _ Baz = Baz

   The getBarInt and setBarInt functions would be used instead of barInt from above. Notice how they are no longer partial functions.

2. Use a separate data type.

   If you really want to use records, you can split out Bar.

   data Foo
     = Bar RealBar
     | Baz
   
   data RealBar = RealBar
     { barInt :: Int
     , barString :: String
     }
   
   getBarInt :: Foo -> Maybe Int
   getBarInt (Bar realBar) = Just (barInt realBar)
   getBarInt Baz = Nothing
   
   setBarInt :: Int -> Foo -> Foo
   setBarInt int (Bar realBar) = Bar (realBar { barInt = int })
   setBarInt _ Baz = Baz

   Now we can use barInt like above, but it is no longer a partial function.

3. Use lenses.

   We could have a couple functions like the following:

   import Control.Lens (Prism', Traversal')
   
   data Foo
     = Bar Int String
     | Baz
   
   _Bar :: Prism' Foo (Int, String)
   _Baz :: Prism' Foo ()
   
   barIntTraversal :: Traversal' Foo Int
   barStringTraversal :: Traversal' Foo String

   barIntTraversal and barStringTraversal can be used along with the preview and set functions for getting and setting. These will be completely safe.

   Checkout this gist or the lens library for more information.

It would be nice to be able to make GHC give a warning on any sum type that contains records, but I don't think it is currently possible.

-Wmissing-import-lists

Let's talk about the difference between explicit and implicit imports.

explicit imports are when you list all the imports for a given module. In the following file, the two imports (Reader and runReader) from Control.Monad.Trans.Reader have been imported explicitly:

import Control.Monad.Trans.Reader (Reader, runReader)

myFunc :: Reader Int a -> a
myFunc x = runReader x 3

implicit imports are when you do not list any imports for a given module. In the following file, everything from Control.Monad.Trans.Reader has been imported implicitly (including Reader and runReader):

import Control.Monad.Trans.Reader

myFunc :: Reader Int a -> a
myFunc x = runReader x 3

-Wmissing-import-lists will make GHC produce a warning when there are any impliclit imports in a module. For instance, if you compile the code above with -Wmissing-import-lists enabled you'll get the following warning:

example.hs:1:1: warning: [-Wmissing-import-lists]
    The module ‘Control.Monad.Trans.Reader’ does not have an explicit import list

Turning on -Wmissing-import-lists is generally a good idea. Code with explicit imports is generally easier to read. There is no confusion where an identifier or type comes from. It is especially helpful for people new to Haskell development¹.

However, it does take longer to write code when using -Wmissing-import-lists. You have to go back and specify exactly what module each function comes from. This does not take a huge amount of time, but it is still faster not to do it.

As a rule of thumb, any project where you are already using -Wall, you should also turn on -Wmissing-import-lists and make sure all of your imports are explicit. Most things that go into production should be using both -Wall and -Wmissing-import-lists. Prototypes and throw-away code can safely use implicit imports².

This is a contentious topic, and you'll find many different styles of imports in both public and private projects³.

Conclusion

I highly recommend that most people use -Wall, -Wincomplete-uni-patterns, and -Wincomplete-record-updates.

If you are already using -Wall, -Wmissing-import-lists is also a good idea to make your code more readable.

Footnotes

tags: haskell