Continuous improvement with hlint code smells

Lately at Co–Star, our codebase has been getting a little unwieldy. We’ve been growing it for the past two years now and have been accruing technical debt steadily. I recently found out about Danger, and wanted to use this with hlint to help automate some code style guidelines we adopted internally to help us take a more proactive approach to fixing up our codebase.

hlint is an amazing tool for catching all sorts of little Haskell code improvements, but it didn’t catch all of the issues I wanted it to. So I decided to extend hlint with the ability to detect certain code smells. A code smell is any kind of indicator that some code should be refactored.

Based on the guidelines that we adopted internally, I started by adding support to detect four types of smells:

• Long functions
• Functions with many arguments
• Long type level lists
• Lots of imports

Long functions are hard to reason about.

Haskell code can be really dense, information-wise. I find that I spend similar amounts of time doing code review for a few lines of Haskell as I do for a mid-sized class in, say, Swift. This is awesome, it means that as programmers we’re not paying attention to boilerplate + superfluous syntax and instead writing + looking at important details. But it also means that things can pile up and overload really quickly. Even a ten line function in Haskell can be doing so much that it’s hard to keep it all in mind at once.

When functions get lengthy, the easiest thing thing to do is split out subsections into smaller helper functions or where clauses. hlint is now smart enough to check the length of function bodies and individual where clauses independently.

Functions with many arguments

Haskell has a really lightweight syntax for declaring and calling functions with arguments. When extending the functionality of some previously written piece of code, it can be tempting to simply add in a new argument and call it a day. But when we do this over and over, we can pretty quickly end up with blobs of code that are operating on 7+ arguments. It’s an oft quoted statistic that people can only hold 7 things in their head at once. So if we can’t even conceive of more than 7 independent things simultaneously, what could a function possibly be doing with 7 arguments?

The answer in my experience is that functions with many arguments don’t really have too many independent arguments, but rather clumps of associated ones. A strategy to deal with lots of arguments is to turn them into datatypes (and especially records). Organizing function arguments into data types can have two benefits: By grouping function arguments into logical units, we can see the overarching structure of the code at a glance more easily. Instead of 4 Strings, 2 Bools, and 3 Ints, we might have a DatabaseConfig and a Logger, which tells us more immediately that our function is going to set up a database connection which logs results. Similarly, for functions that have a lot of arguments, a record can give names to arguments that would otherwise only be visible at the function definition (in comments and as argument bindings). This is especially helpful in functions that repeat arguments (e.g. 3 Bools). Records also provide flexibility in terms of ordering, not to mention the ability to pass widely used arguments through a codebase.

Long type level lists

At Co–Star, we use the freer-simple package to organize our code and model out effects. This library uses type-level lists to represent computations that require multiple side effects. For example, a computation might have a type Eff '[Reader String, State Bool] Integer which means that it’s going to read from some common String parameter and modify some Bool state.

We’ve defined all sorts of new low-level effects, but I think that we’re just now crossing a threshold where it’s going to be helpful for us to start defining higher-level effects. For example, we have a Sql effect for connecting to our database and an Http effect for making web requests. We have a Planets effect that calculates positions of astronomical bodies. These effects are everywhere in our codebase, since we’ll lazily attempt to set up different aspects of our users’ profiles (and cache them for next time). Instead of thinking about computations that are using all of these different effects, it makes sense to start thinking about computations in terms of a higher-level User effect. This effect could expose an interface that promises a certain part of a user profile and will handle the lazy setup and caching behind the scenes.

The benefit of this is that it’s easier to reason about what functions are doing, and it’s also easier to test. Instead of setting up three different test environments, we can set up a single one.

hlint won’t yell if you have a long type list in a type alias. It’s only a smell if it’s in a type declaration.

Lots of imports

When I’m writing code in Haskell, I lean on the compiler and other tools as much as possible. I love using type holes to isolate different parts of a program that I still need to write. I like to get into a loop of type a little, compile, type a little, compile etc.

One thing that interrupts this cycle is adding missing imports. Especially when writing a new module, it can feel frustrating to be constantly compiling, adding import, compiling, adding import. Often times there are few simple things to try to keep import lists tidy: Re-export modules that you heavily depend on. If you have a Controllers module that is unusable without also importing Models, then Controllers should consider re-exporting this module. Create modules that group together logically related sub-modules. Servant.API is a great example of this Use a custom prelude! There’s no point in repeatedly importing all of those modules that show up everywhere. Use a prebuilt custom prelude or roll your own with things you use all the time.## Putting it all together Now that all of these different smells can be detected automatically, you can hook it up with danger-hlint and get notified by the robots anytime your code starts to get a little smelly. All of these different smells are configurable to be detected independently, so you don’t have to take an all or nothing approach:

 - smell: { type: long functions, limit: 12 }
 - smell: { type: long type lists, limit: 4 }
 - smell: { type: many arg functions, limit: 5 }
 - smell: { type: many imports, limit: 15 }

And as always with hlint, you can exempt certain modules and certain functions using ANN pragmas, so you can let that one behemoth function continue to grow if you really want to.

I’m excited to announce that this is now available in hlint-2.1.16. What’s next for smells with hlint? What other bad code patterns can we detect automatically?

Interested in working at Co-Star? Apply Here