My motivation to write this article stems from the large amount of materials on static analysis that have increasingly caught my attention. Firstly, it is the , which actively promotes itself on Habr through reviews of errors found by their tool in open-source projects. Recently, PVS-studio has implemented , and of course, the developers of IntelliJ IDEA, whose built-in analyzer is arguably the most advanced for Java today, .
When reading such reviews, one gets the impression that it is like a magical elixir: press a button, and there it is—a list of defects before your eyes. It seems that as analyzers improve, bugs will be detected more and more automatically, and products scanned by these robots will only get better and better, requiring no effort on our part.
But there are no magical elixirs. I would like to talk about what is usually not discussed in posts like 'here's what our robot can find': what analyzers cannot do, what their real role and place is in the software delivery process, and how to implement them correctly.
Ratchet (source: ).
)
What static analyzers will never be able to accomplish
What, from a practical perspective, is source code analysis? We input some source files, and in a short time (much shorter than running tests) we obtain some information about our system. The fundamental and mathematically insurmountable limitation is that we can only gain a rather narrow class of information in this way. halting problem , asserting that for any non-trivial property of computable functions, determining whether an arbitrary program computes a function with that property is an algorithmically undecidable problem. For example, it is impossible to write an analyzer that, given any source code, determines whether the analyzed program is an implementation of an algorithm that computes, say, the square of an integer.
Thus, the functionality of static analyzers has insurmountable limitations. A static analyzer will never be able to determine things like the occurrence of a 'null pointer exception' in languages that allow null values, or to determine in all cases the occurrence of 'attribute not found' in dynamically typed languages. All that the most advanced static analyzer can do is highlight specific instances, which number among the potential issues with your source code is, without exaggeration, a drop in the ocean.
Static analysis is not bug hunting
From the above, it follows that static analysis is not a means to reduce the number of defects in a program. I dare to assert: when first applied to your project, it will find 'interesting' places in the code, but it will most likely not find any defects that affect the quality of your program's operation.
Examples of defects automatically found by analyzers are impressive, but it should not be forgotten that these examples were found by scanning a large set of substantial codebases. In the same way, hackers with the ability to cycle through a few simple passwords on a large number of accounts will eventually find those accounts with simple passwords.
Does this mean that static analysis should not be applied? Of course not! And for the same reason that every new password should be checked against the list of 'simple' passwords.
Static analysis is more than just bug hunting
In fact, the practically solvable problems addressed by analysis are much broader. After all, static analysis is any source code check carried out before its execution. Here are some things that can be done:
- Code style checking in the broadest sense includes both formatting checks and searching for unnecessary brackets, setting thresholds on metrics such as the number of lines or cyclomatic complexity of methods, etc. — anything that can potentially hinder the readability and maintainability of the code. In Java, a tool for this is Checkstyle, while in Python it is flake8. Such programs are generally referred to as 'linters'.
- Not only executable code can be analyzed. Resource files such as JSON, YAML, XML, .properties can (and should!) be automatically validated. After all, it's better to discover that the structure of JSON is broken due to some unmatched quotes at an early stage of automated Pull Request verification, rather than during test execution or at run time. Relevant tools are available, for example, , .
- Compilation (or parsing for dynamic programming languages) is also a form of static analysis. Generally, compilers can issue warnings indicating problems with the quality of the source code, which should not be ignored.
- Sometimes compilation isn't just about compiling executable code. For instance, if you have documentation in the format , at the moment of converting it to HTML/PDF, the AsciiDoctor processor () may issue warnings regarding broken internal links. This is a significant reason to reject a Pull Request with documentation changes.
- Spell checking is also a form of static analysis. The utility is capable of checking spelling not only in documentation but also in source code comments and literals across various programming languages, including C/C++, Java, and Python. A spelling mistake in the user interface or documentation is also a defect!
- Configuration tests (to find out what this is, see and reports), although performed in the runtime environment of unit tests like pytest, also count as a form of static analysis, as they do not execute the source code during their execution.
As we can see, bug finding plays the least important role in this list, while everything else is accessible through free open source tools.
Which of these types of static analysis should you apply in your project? Of course, all of them—the more, the better! The key is to implement this correctly, which will be discussed further.
The delivery pipeline as a multi-tiered filter and static analysis as its first cascade
The classic metaphor for continuous integration is a pipeline through which changes flow—from source code changes to delivery in production. The standard sequence of stages in this pipeline looks like this:
- static analysis
- compilation
- unit tests
- integration tests
- UI tests
- manual review
Changes that are rejected at the N-th stage of the pipeline are not passed on to stage N+1.
Why this way and not otherwise? In the part of the pipeline that deals with testing, testers encounter the widely recognized testing pyramid.
The testing pyramid. Source: Martin Fowler.
At the bottom of this pyramid are tests that are easier to write, execute faster, and have less chance of false positives. Therefore, there should be more of them; they should cover more code and be run first. At the top of the pyramid, the situation is reversed, so the number of integration and UI tests should be minimized to the necessary minimum. The person in this chain is the most expensive, slowest, and least reliable resource, which is why they are at the end and only perform their work if no defects were found in the previous stages. However, the same principles apply to the pipeline in areas not directly related to testing!
I would like to suggest an analogy in the form of a multi-tiered water filtration system. Dirty water (changes with defects) is fed in, and we must obtain clean water, with all undesirable contaminants filtered out.
Multi-tiered filter. Source:
As we know, cleansing filters are designed so that each subsequent cascade can filter out increasingly finer fractions of contaminants. In this analogy, it means that the input quality gates have higher throughput, require less effort to initiate, and are themselves more forgiving in operation — and they are arranged in that order. The role of static analysis, which we now understand can only filter out the coarsest defects, is akin to that of a "debris screen" at the very beginning of the cascade of filters.
Static analysis alone does not improve the quality of the final product, much like a debris screen does not make water drinkable. Nonetheless, when combined with other elements of the pipeline, its importance is clear. Although in a multi-cascade filter, the output cascades are potentially capable of capturing everything the input ones can — it is evident what consequences will arise from trying to rely solely on the fine cleaning cascades, without the input cascades.
The purpose of the debris screen is to relieve subsequent cascades from capturing the most blatant defects. For example, at a minimum, a person performing code reviews should not be distracted by incorrectly formatted code or violations of established coding standards (like unnecessary brackets or overly deep nesting). Bugs such as NPE should be caught by unit tests, but if the analyzer indicates to us before testing that a bug is bound to occur — this will significantly speed up its resolution.
I believe it's now clear why static analysis does not improve product quality if applied sporadically, and why it must be applied continuously to filter out changes with coarse defects. The question of whether using a static analyzer will improve the quality of your product is roughly equivalent to asking, "Will the drinking quality of water taken from a dirty reservoir improve if it is passed through a strainer?"
Implementation in a legacy project
An important practical question: how to integrate static analysis into the continuous integration process as a quality gate? With automated tests, everything is clear: there is a set of tests, and the failure of any one of them is sufficient grounds to consider that the build did not pass the quality gate. Attempting to establish a gate based on the results of static analysis fails: there are too many warnings in legacy code, and while we don't want to ignore them completely, we also cannot halt product delivery just because there are warnings from the analyzer.
When first applied, any analyzer generates a huge number of warnings, the overwhelming majority of which are unrelated to the proper functioning of the product. It is impossible to fix all these remarks immediately, and for many, it's unnecessary. After all, we know that our product functions well overall, and this was true even before the implementation of static analysis!
As a result, many limit themselves to episodic use of static analysis or use it merely in an informative mode, where a report from the analyzer is simply generated during the build. This is equivalent to the absence of any analysis because if we already have numerous warnings, the emergence of another (no matter how serious) during code changes goes unnoticed.
The following methods of introducing quality gates are known:
- Setting a limit on the total number of warnings or the number of warnings divided by the lines of code. This works poorly, as such a gate easily allows changes with new defects as long as their limit is not exceeded.
- Capturing all old warnings in the code as ignored at a certain point and failing the build when new warnings arise. This functionality is provided by PVS-Studio and some online resources, such as Codacy. I haven't worked with PVS-Studio, but regarding my experience with Codacy, their main issue is that determining what constitutes an 'old' versus a 'new' error is quite complex and often doesn't work correctly, especially if files are significantly altered or renamed. As far as I recall, Codacy could overlook new warnings in a pull request while at the same time rejecting a pull request due to warnings unrelated to the changes in that pull request.
- In my opinion, the most effective solution is described in the book the 'ratcheting' method. The main idea is that each release is characterized by the number of static analysis warnings, and only changes that do not increase the total number of warnings are allowed.
Ratcheting
Works as follows:
- At the initial stage, metadata about the release is recorded, indicating the number of warnings in the code found by the analyzers. Thus, during the build of the main branch, your repository manager logs not just 'release 7.0.2,' but 'release 7.0.2, containing 100,500 Checkstyle warnings.' If you use an advanced repository manager (like Artifactory), storing such metadata about your release is easy.
- Now, each pull request during the build compares the number of resulting warnings with the number present in the current release. If the PR leads to an increase in this number, the code fails the quality gate for static analysis. If the number of warnings decreases or remains unchanged, it passes.
- In the next release, the recalculated number of warnings will once again be recorded in the release metadata.
Over time, though steadily (much like the operation of a ratchet), the number of warnings will tend toward zero. Of course, the system can be tricked by adding a new warning while correcting someone else's. This is acceptable, as it produces results in the long run: warnings are usually corrected not one by one, but in groups of a certain type, and all easily resolvable warnings tend to be addressed quickly.
This chart shows the total number of Checkstyle warnings over six months of such a 'ratchet' operating on . The number of warnings decreased significantly, and this happened naturally, alongside product development!
I use a modified version of this method, separately counting warnings broken down by project modules and analysis tools, the resulting YAML file with build metadata looks something like this:
celesta-sql:
checkstyle: 434
spotbugs: 45
celesta-core:
checkstyle: 206
spotbugs: 13
celesta-maven-plugin:
checkstyle: 19
spotbugs: 0
celesta-unit:
checkstyle: 0
spotbugs: 0
In any advanced CI system, a 'ratchet' can be implemented for any static analysis tools, without relying on plugins or third-party tools. Each analyzer produces its report in a simple text or XML format that is easy to analyze. Only the necessary logic needs to be coded in the CI script. You can see how this is implemented in our open-source projects based on Jenkins and Artifactory or . Both examples depend on the library : the method countWarnings() counts XML tags in the files generated by Checkstyle and Spotbugs in the usual way, while compareWarningMaps() implements the ratchet, throwing an error if the number of warnings in any of the categories increases.
An interesting implementation option for a "ratchet" is possible for analyzing the spelling of comments, text literals, and documentation using aspell. As is known, not all words unknown to the standard dictionary are incorrect when spell checking; they can be added to the user dictionary. If you make the user dictionary part of the source code of the project, then a quality gate for spelling can be formulated this way: running aspell with both the standard and user dictionaries. find any spelling errors.
On the importance of version fixation for the analyzer
In conclusion, it is important to note the following: no matter how you implement the analysis into your delivery pipeline, the version of the analyzer must be fixed. If spontaneous updates of the analyzer are allowed, new defects may "surface" when building the next pull request that are not related to code changes but rather because the new analyzer is simply capable of finding more defects – and this will disrupt your pull request acceptance process. Upgrading the analyzer should be a deliberate action. However, strict version fixation for each build component is generally a necessary requirement and a topic for separate discussion.
Conclusions
- Static analysis will not find you bugs and will not improve the quality of your product as a result of a one-time application. The positive effect on quality only comes from its continuous use in the delivery process.
- Finding bugs is not the main task of analysis; the vast majority of useful functions are available in open-source tools.
- Implement quality gates based on the results of static analysis at the very first stage of the delivery pipeline, using the "ratchet" for legacy code.
Links
- a report on different methods of code analysis (not only static!)
Source: habr.com
