GHC Warnings You Should Use in Addition to -Wall

2017-07-28

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 development1.

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 imports2.

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

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


  1. Or even just coworkers new to the project.

  2. There is one small problem with -Wmissing-import-lists. It does not play nicely with the NoImplicitPrelude extension.

    The NoImplicitPrelude extension allows you to have the Prelude module not be imported automatically. It allows you to use an alternative Prelude like classy-prelude or foundation.

    Let’s take the following file. It uses classy-prelude:

    {-# LANGUAGE NoImplicitPrelude #-}
    
    module Main where
    
    import ClassyPrelude
    
    myLen :: Text -> Int
    myLen text = length text
    
    main :: IO ()
    main = print "hello"

    If you try to compile with with -Wmissing-import-lists, you get the following warning:

    example.hs:5:1: warning: [-Wmissing-import-lists]
        The module ‘ClassyPrelude’ does not have an explicit import list

    One sneaky way around this is to use an alternative base package that does not contain a Prelude module. Then, you can create a local, project-wide Prelude module (possibly based on something like ClassyPrelude). If you do this, you do not even have to use the NoImplicitPrelude pragma.

    A popular alternative base package is base-noprelude. Here is a .cabal library section that uses base-noprelude. It also specifies a local Prelude module.

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

    I submitted a feature request to GHC about NoImplicitPrelude and -Wmissing-import-lists.

  3. One other argument is about IDEs. Some people argue that when coding on a small team, explicit imports are not needed if everyone uses an editor with functionality to look up the origin of a function or type. When reading code, you’ll never be confused about where a function or type is coming from.

    A counter-argument to this is that if you allow IDEs, why can’t you just have an IDE that will auto-import functions and types explicitly? Surely, that would be almost as fast as using implicit imports. And it would make it easier if you have to do code reviews without your IDE (for instance, on Github).

tags: haskell