Aelve Guide | Haskell

A common pattern when developing big Haskell applications is “library + executable” (with the library not being used anywhere other than the executable). Thus, it often happens that functions (and sometimes whole modules) are not used in the final executable, but GHC doesn't report them as unused because they are still exported from the library. It'd be nice to be able to get a list of such functions.

Note: the weeder tool already seems to be doing it.

Instead of the right 3 < a && a < 5:

if a < 3 && a > 5
if a > 5 && a < 3
...

It's often possible to detect in compile-time when printf or format (from text-format) is used with the wrong number of arguments:

printf "%s %s" a b c

• Using the same expression with an operator:

  a == a    a /= a    a + a    a - a    a && a    a || a    ...

• Duplicated expressions in a list:

  [a..a]    [a, a..]

• Duplicated branches in an if:

  if ... then A else A

• Very similar but different bindings:

  -- in one function
  chainLength = blockSecurityParam * (length participants - 1)
  
  -- in another function
  chainLength = blockSecurityParam * (length participants + 1)

The following code usually doesn't make sense because having a single () is rarely useful:

xs <- replicateM_ ...
ys <- forM_ ...

Having a list of ()s isn't useful either:

xs <- replicateM 10 {- something that returns () -}

Often people are afraid that the compiler won't simplify some arithmetic expression, and they calculate it, write the answer, and leave the expression in comments. Then someone updates the comment but forgets to recalculate the expression (or vice versa). Here the correct answer is 191, but the number written is 187:

size :: Int
size = 187       -- 4*(28+16) + 15

GHC will needlessly share [1..10000000] between two computations and cause a memory leak here:

foo = for_ [1..10000000] $ \i ->
  ...

bar = for_ [1..10000000] $ \k ->
  ...

The solution is to use something like loop.

This will fail on Windows because </> will append "\", not "/":

foo = "https://example.com/get" </> show a

An inexperienced developer might write this, thinking that return means “early exit”. It doesn't.

main = do
  ...
  when p $ return ()
  ...

These are often copypaste errors:

a || (a && b)    ==>    a
a || not a       ==>    True
a && (a || b)    ==>    a
a && not a       ==>    False

Even if not, they still could be simplified.

Using BS8.pack is almost always a mistake, because it messes up any Unicode characters. Instead you probably want to encode the string into UTF-8 (with e.g. utf8-string or text).

When you use e.g. bracket from lifted-base, and you do anything with State, Writer, etc in the release action, it will be discarded. Thus, when a release action explicitly modifies state, it's likely a mistake and the developer simply doesn't know that it's not safe to do. An example:

bracket ... (\a -> put (a, 0)) $ do
  ...

Here's a case where an infinite loop could happen because the n < 0 case wasn't handled:

f :: Int -> something
f n = ... f (n-1) ...

In some cases it's going to be a false positive, but it's still better to report such cases.

Data.HashMap.(Strict|Lazy).size and Data.HashSet.size are O(n), which is surprising to many people because corresponding functions for ordinary maps and sets are fast. It would be good to detect when it can likely lead to performance problems and emit a warning.

Data.Text.index and Data.Text.length are in the same category, by the way.

You almost never want to apply length or null (and most other Foldable methods) to tuples because they treat tuples as one-element containers. So, when an application of one of those methods is detected, it's likely a bug.

Binary instances are usually written like this:

data Foo = Foo {this, that, other :: Int}

instance Binary Foo where
  put Foo{..) = put that >> put this >> put other
  get = Foo <$> get <*> get <*> get

Here the order was mixed-up: we serialize that, this, other, but deserialize this, that, other (this and that are swapped).

Another, similar, bug is forgetting to serialize one of the fields altogether.

Sometimes people forget that if they write into ix from lens, the corresponding element does not get created if it didn't exist already. Thus, this (and variants of this) are wrong:

do absent <- isNothing <$> preuse (ix k)
   when absent $ ix k .= blah

People often write [0..length xs] when they intend to write [0..length xs - 1].

This is almost always a mistake (and unfortunately it's more common than it might seem):

instance Foo Int where
  foo = foo

It should also be possible to detect a trickier variant of this mistake. Here f is a function (or chain of functions) that doesn't change the type:

instance Foo Int where
  foo = f . foo

instance Foo Int where
  foo = foo . f