You don't have enough static analysis

Introduction

Pretty much every programming language out there has tools that statically analyze your source code and detect different problems. These problems can range from simple things like ensuring that you have consistent casing for variable names in Java to ruthlessly enforcing method limits in Ruby. If you’ve ever used one of these tools, they may seem overbearing and not worth the hassle, but they will soon prove their value once your application becomes larger, has multiple developers, or is business critical and can’t afford outages caused by trivial mistakes. Static analysis tools are a super-low cost solution for improving the quality of a code-base.

The Testing Pyramid

A pyramid with lowest cost/complexity on the bottom and high complexity on the top. From bottom to top: static analysis, unit tests, integration tests, and UI tests.

How does static analysis fit into the testing pyramid? As you go up the pyramid, the complexity of your tests rise and along with that the cost of implementing them goes up. At the top, you have UI functional tests that execute an entire user story, such as loading a page, logging in etc. These will uncover complex regressions caused by the integration of different components. Since they cover so much code, they are often expensive to implement in developer time and can be brittle, but they’re important to uncover certain user issues. At the bottom, you have unit tests that focus on testing a single method or other unit of work. Since they cover only a small piece of code, they can be very fast to run and are less prone accidental breakage. They’re usually much cheaper to implement than some of the higher order test cases.

Static analysis is even cheaper to implement than unit tests, since you can opt-in an entire code base all at once and if you’re using an advanced enough IDE, you see the results in real-time. This makes them fit in well at the bottom of the pyramid, below even unit tests.

Moving failures earlier

Static analysis enables you to move failures that used to occur at run-time or at code review time, all the way to the build-time or even coding time. This reduces the feedback loop time for developers to realize issues

Types of Checks

Static analysis comes in all shapes and sizes. Depending on what programming language you choose, you might be able to have more detailed type checking (with statically typed languages, like Java.) Below are some examples of different checks that you could enable and enforce:

  • Style guides - If you’re a developer at an enterprise, you might have a defined style guide that ensures that developers always use tabs/spaces, place braces in the right location, and consistent naming. These can be strictly enforced at build time to ensure that all code is consistent.
  • Code complexity - Methods and files that become too long or have too much complex conditional logic will quickly become difficult to understand and manage. Method length and conditional logic can be monitored and minimized using analysis tools with ABC scoring or cyclomatic complexity.
  • Unused/redundant logic - Code that’s no-longer used, redundant, or is unreachable will make code more confusing and hard to read. Removing this code (which can always be recovered with source control) helps keeps your code cleaner.
  • Code coupling - Coupling refers to how much one bit of code depends on another bit of code. Code that’s highly coupled can be difficult to refactor or test. Many tools can detect when one class is overly dependent on another and can alert on it.
  • Undefined behavior - Some languages have undefined behavior, meaning that the language spec does not define exactly how to compile a block of code. Depending on undefined behavior can be unsafe as different compilers could treat it differently, making your code non-portable.
  • Performance - It’s quite easy to write code that’s non-performant in a critical path. Some of these issues, like inefficient string concatenation in Java, can be easily detected and alerted. This wouldn’t be considered a case of premature optimization since generally the fix is trivial and can be done right as the developer is coding.
  • Security - Good security practices come at every point of the development process with security driven development. Many security exploits, like buffer overflows, etc., can be detected by a developer using the wrong string method in C++ and can be alerted at development time as opposed to security review time.
  • Other semantic issues - Often times code will be syntactically correct and is legal code, but can be confusing to developers.

Case Studies

Here’s a few examples of interesting issues that could have been discovered with static analysis.

Bad string concatenation causes poor performance

This is an example of an issue I experienced at my day job. In high-throughput data processing pipelines, the critical path is executed millions of times. This makes it important to watch for mistakes. Take a look at the below block of code and see if you see the mistake:

1
2
3
4
5
import org.slf4j.Logger;
[...]
public void performLogic(SomeDomainModel input) {
   LOGGER.debug("Performing logic for " + input.type);
}

In this case, the application executed the string concatenation regardless of whether or not logging was enabled. This can subtly create a performance bottleneck, especially if the toString is expensive. In many cases, debug logging is disabled in production, causing this string concatenation to be executed and the result be ignored. There’s a couple ways that I’ve discovered that can detect this particular issue: IntelliJ has an inspection that can be enabled (Internationalization issues) that flags all string concatenation, but it does result in false positives as sometimes string concatenation is valuable. Another tool that directly targets this issue is findbugs-slf4j, which will auto flag non static log messages like this one, enabling the logging framework to automatically perform string joining if required.

CVE-2014-1266 - Apple SSL Goto Fail

This security vulnerability in 2014 caused certain Apple clients to improperly validate TLS certificates. It was caused by a trivial coding mistake and one that could have been caught by many different linting tools.

1
2
3
4
5
6
7
if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
    goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
    goto fail;  /\* The second bug \*/
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
    goto fail;

Note the extra goto that was indented and was easy to miss. This bug could have been caught with the Clang/GCC compiler flag -Wunreachable-code and GCC includes the -Wmisleading-indentation which also would have flagged this issue.

Example Tools

Java

As Java is a type-safe language, analysis tools can be a lot stricter about enforcing standards around types (more-so than the compiler already does.) There’s a few tools that I like to use for all of my applications:

  • IntelliJ - My IDE of choice. It comes with a huge library of analyzers, called inspections, that can detect a variety of issues. They highlight directly in the code and some of them can be automatically fixed. I’ve enabled a custom set of inspections that are more strict than the default.
  • Checkstyle
  • FindBugs

Other tools include:

Ruby/Rails

For Ruby developers, a popular static analysis tool is Rubocop. If you thought you wrote clean, idiomatic Ruby code take a look at the Ruby style guide that it enforces and see how many rules you may violate.

Useful tools:

JavaScript

JS has a huge number of options for static analysis. One of the best ones that I’ve found is Eslint. It comes with a number of different linters, but most are disabled by default and you have to explicitly enable them as you want. There’s a few pre-defined rule sets available by companies like Google or Airbnb. Eslint also supports plugins including ones that lint for React.

CloudFormation

Related Article: Structured and auditable changes to infrastructure

Another advantage of modelling your infrastructure as code is that it can also be analyzed to detect possible bugs or other problems. As CloudFormation is written in JSON or YAML, you can benefit from first ensuring that the code is syntactically correct, then use a tool such as cf_nag to detect issues like overly permissive S3 bucket permissions or missing security groups.

Legacy codebases

For applications that don’t already have any analysis running at build-time, it can be difficult to just flip the switch and start enforcing coding standards. Often times the entire application would fail and you would have to spend days fixing everything, adding change risk. My approach has been to enable it at build-time, but then mark most checks as warnings so that developers see issues, and can slowly work to fix the code as they refactor existing code or add new features. Developers working on the code-base can be incentivized to fix a small amount at a time or enforce a single rule at a time, then fix those issues. As long as you continuously work to fix existing issues and increase enforcement, then this approach will slowly increase your code-base quality until you’re eventually enforcing all rules.

Summary

The above is just a small sample of the different issues that static analysis can identify and improve and only a small sample of different tools that perform static analysis. It should function as a jumping off point so that you can start thinking about where static analysis makes sense and how you should start using more. Anytime you discover and fix a bug, you should think whether it should be covered with a unit test, integration test, or another static analysis rule.

Copyright - All Rights Reserved

Comments

Comments are currently unavailable while I move to this new blog platform. To give feedback, send an email to adam [at] this website url.