How SonarCloud finds bugs in high-quality Python projects
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.
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.
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.
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`.
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.
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.
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 ".
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.
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
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!