How SonarCloud finds bugs in high-quality Python projects

by nicolas harraudeau|

As developers, there always comes a time when we find a bug in production and wonder how it passed all our quality checks. The truth is that we can never be sure our code is bug free. We can only choose the tools and workflows which will find the most bugs without slowing us down too much.

SonarQube, SonarLint and SonarCloud are such tools. We used SonarCloud during our recent bug report campaign, which focused on popular projects such as tensorflow, numpy, salt, sentry and biopython. The campaign result was quite interesting, since it shows the kind of bugs we can find in a Python project even when its development workflow includes every best practice: code reviews, high test coverage, and the use of one or more linters (flake8, pylint, ...).

Let's go over a few Bugs we found with SonarCloud and see why it is able to detect them when popular linters don't .

Reference to an undefined variable

SonarCloud can detect buggy references to undefined variables when the variables are defined in another `if-else` branch. It uses a Control Flow Graph to deduce that the definition of the variable will never occur before the buggy reference.

The `response` variable is used in two branches of an `if-else` block, however SonarCloud detects that it is defined in only one of the branches.

Unreachable code

Detecting dead code is easy when it's just after a `return` or a `raise` statement. It's a little harder when the `return` is conditional. We use a control flow graph to detect cases where multiple branches exit just before reaching a statement.

Every branch of an `if-else` block end with a `return` statement, which means that any code after the `if-else` block is unreachable. SonarCloud detects this and raises an issue.

Wrong fields in formatted strings

It is quite common to reference the wrong field name or index during string formatting. Pylint and Flake8 have rules detecting this problem with string literals, but they miss bugs when the format string is in a variable. 

The `format` method is called on the `msg` variable. This variable was assigned earlier a string containing `{1}` instead of `{0}`, which means that it accesses the second argument of `format`. The `format` method call will fail because it has only one argument.

Type errors

SonarCloud has a type inference engine, which enables it to detect advanced type errors. It uses every bit of information it can find to deduce variable type, including Typeshed stubs, assignments, and your type annotations.. At the same time, it won't complain if you don't use type annotations, and it's designed to avoid False Positives.

In this example, control flow analysis is what allows it to understand that `state_shape` is a tuple because it is assigned `output_shape[1:]` when `output_shape` is a `tuple`. The algorithm is able to ignore the later `list` assignments to `output_shape`.

This code tries to concatenate a `list` and a `tuple`, which will fail and raise a `TypeError` exception. This bug is difficult to see during a review because one variable is assigned multiple times with different types (`tuple` and `list`).

Now let's look at some more specific examples.

Wrong argument type 

SonarCloud uses Typeshed stubs to know the types expected by builtins functions. So here it raises an issue because you get a `TypeError` if you call the `len` builtin on an integer. 

In this code the developer made a typo and called the `len` builtin twice. The result of the first `len` call is given to the second `len` call. This will fail as `len` cannot be called on integers.

Comparisons that don't make sense

SonarCloud has many rules detecting code which doesn't make sense. Comparing incompatible types with `==` will never fail, but it will always return False, or True if you use `!=`. Here we can see an issue because `platform.architecture()` returns a tuple.

In this code we can see the line `if platform.architecture() == '32bit':`. The method `platform.architecture()` returns a `tuple`. Comparing a `tuple` and a string with the operator `==` will always return False.

Return values from functions without side effects should not be ignored

Some function calls have no side effect, i.e. they won't change anything by themselves and their only purpose is to return a value. Thus  there is always a bug when their result is not used. SonarCloud knows an extensive list of such functions. In this example the two strings are not concatenated; the `format` method is called on the second string and the result is discarded, so the value of `warning_msg` is "Make sure that your dataset can generate at least ".

In this code a string is split on two lines and the developer forgot to add a backslash at the end of the first line, thus they won't be concatenated. Popular linters won't detect this problem because the `format` method is called on the second string. SonarCloud detects that the result of `format` is not assigned and raise an issue.

Unraised exceptions

When we review code we usually look at classes, variables and other meaningful symbols and we forget to check little details, such as "is there a raise keyword before my exception". SonarCloud analyzes your whole project to extract type hierarchies. Thus it detects when custom exceptions are discarded, not just the builtin ones.

This code creates a `ValueError` exception but does not `raise` it. This is a pretty common mistake as developers usually don't see the missing `raise` keyword..

Flake8 is great but not enough

"Some of the things [SonarCloud] spots are impressive (probably driven by some introspection and/or type inference), not just the simple pattern matching that I am used to in most of the flake8 ecosystem." - Peter J. A. Cock - maintainer of BioPython (original post here)

This is one of the nice pieces of feedback we received during our bug report campaign. (There's more!).

All the projects we examined use one or more linters, such as Flake8, which is very popular, and is often included in CI workflows. There are very good reasons for Flake8's broad use:

  • it focuses on uncontroversial rules that generate few false positives
  • It checks pep8 style
  • It is fast

SonarLint, SonarCloud and SonarQube have the same philosophy about speed and false positives. All three target developers, which means that we work hard to keep "noise" to a minimum. In addition, SonarCloud and SonarQube can both import Flake8 issues. But most importantly:

  • they detect a broader range of issues. Not just style and pattern matching, but a full range of bugs, code smells and vulnerabilities.
  • they help you focus on achieving high quality in recent changes (i.e. Clean as You Code) rather than distracting you with small flaws in old code
  • they support all the languages in your project. For example if you've got JavaScript or TypeScript alongside your Python, it will be analyzed simultaneously, with no more setup or infrastructure.

You can use SonarCloud for free on any open source project and get started with just a few clicks. SonarQube Community Edition is also free for unlimited on-premises use. Don't hesitate to share your feedback, good or bad, in our community forum. It helps us improve our tools everyday.

Have something to add? Join us in the community!