Static analyzer wishlist – Haskell – Aelve Guide

Writing into 'ix'

2017-08-06T23:41:52Z

Writing into 'ix'

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

Infinite loops in instances

2017-08-06T23:33:36Z

Infinite loops in instances

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

[0..length xs]

2017-08-06T23:28:27Z

[0..length xs]

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

Buggy serialization instances

2017-08-06T23:25:18Z

Buggy serialization instances

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.

'length', 'null', etc applied to tuples

2017-08-06T23:23:43Z

'length', 'null', etc applied to tuples

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.

Warn about unexpectedly slow functions

2017-08-06T23:19:06Z

Warn about unexpectedly slow functions

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.

Potential infinite recursion

2017-08-06T23:15:04Z

Potential infinite recursion

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.

Modifying state in MonadBaseControl

2017-08-05T20:01:39Z

Modifying state in MonadBaseControl

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

Using Data.ByteString.Char8.pack

2017-08-05T19:55:13Z

Using Data.ByteString.Char8.pack

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

Weird boolean computations

2017-08-05T19:41:15Z

Weird boolean computations

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 'pure' or 'return' in the middle of a function

2017-08-05T19:38:01Z

Using 'pure' or 'return' in the middle of a function

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

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

Using () with URLs

2017-08-05T19:35:35Z

Using (</>) with URLs

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

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

Floated-out lists

2017-08-05T19:11:30Z

Floated-out lists

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.

Having outdated computations in comments

2017-08-05T19:07:01Z

Having outdated computations in comments

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

Discarding data and then using it

2017-08-05T18:59:57Z

Discarding data and then using it

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 () -}

Copypaste

2017-08-05T18:42:24Z

Copypaste

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)

printf arguments

2017-08-05T17:10:26Z

printf arguments

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

Weird range comparison

2017-08-05T17:09:52Z

Weird range comparison

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

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

Dead code elimination

2017-07-01T15:27:08Z

Dead code elimination

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.