A custom linter for python
A Linter or Lint is a static code analysis tool that helps to find syntactic errors, bugs, and structures to avoid. This document describes the proposal to implement a linter that allows us to configure rules in a simple way, with the aim of improving code quality and the manual code review process in pull requests. This proposal is not intended to replace any of the tools we currently use, that is, this proposal is not intended to replace flake8.
Problem Description
During the pull request phase, a manual code review of the proposed changes is performed. On many occasions the failures detected are repeated many times and on others, either due to lack of knowledge or human error, they are not detected.
Likewise, the style rules established in Ebury for its applications depend on this manual revision to be fulfilled.
Also, the most experienced developers with this type of knowledge are not available to always review all proposals for changes in the code, so this review may not be complete in all cases. This problem is especially necessary when a new programmer joins and has to wait for the manual review to be able to correct these errors.
The consequences of this manual check are delayed delivery times, inconsistent style, dependency on senior developers, running full continuous integration every time a bug is fixed, frustration from developer and reviewers...
Background
The quality gates for static verification of python code that we have in our continuous integration is flake8, which checks compliance with the isort and PEP8 rules.
Flake8 make use of predefined rules and modifying or adding them can be expensive and complicated.
Given that each time the number of teams and developers increases, the applications do not stop growing and knowing the problems described in the corresponding section of this document, we propose the possibility of adding a tool that allows establishing code rules in an agile way.
As an example, some of the possible rules: - Use pytestmark = pytest.mark.integration instead @pytest.mark.integration
-
No use @pytest.mark.priority_* because is deprecated
-
Use assertEqual instead assertEquals
-
Start docstring or translation texts in the first line with no blankspaces
-
Do not inherit from SimpleTestCase in integration tests to avoid persisting in the database and random errors appearing. The example comes from this PR: ata-881
Solution
The easiest option in terms of implementing new verifications seems to be Bellybutton.
xAST is to python code what xPATH is to HTML, so its learning curve is practically non-existent if you are familiar with E2E or smoke tests. The other alternative of implementing the checks using regular expressions is even simpler.
The problem with Bellybutton is that it is in an early stage of development. For this reason it is discarded as an option since we do not want an unstable Quality Gate.
The second option would be pylint, which offers a robust development framework for static checks using ast with the astroit library. In the references of this document you can find an article describing the process of creating a new rule with pylint.
Its implementation will be simple. It will be added as a python package to the development pypi and in each python project where you want to use it, there will be a custom configuration file as well as a folder with the created rules.
After this, it will be incorporated as a make command into the project and can be called both locally and from continuous integration. It is exactly the same behavior as flake8.
In the case of a format rule or one that can be corrected automatically, it will be necessary to add a rule to the yapf configuration file to allow automatic formatting. It will be necessary to pass this validation rule to all the BOS code to avoid noise in the rest of the pull requests whenever its automatic correction using yapf is possible.
Alternatives
Python linters:
| Name | Purpose | License | Documentation | Version | Community |
|---|---|---|---|---|---|
| flake8 | Imports + PEP8 rules | MIT License | Good, no docs for plugins development | 3.8.3 | Big, 123 contributors and 943 stars in github |
| Pylint | Errors, coding standard | GPLv2 | Very good | 2.5.3 | Big, 283 contributors and 3k stars in github |
| Bellybutton | Custom linting rules | MIT License | Regular, good examples | 0.3.0 (alpha) | Small, 3 contributors and 183 stars in github |
| PyFlakes | Analyzes programs and detects various errors | MIT License | Regular, good examples | 2.2.0 | Medium, 3 contributors, 910 stars in github |
| pycodestyle | Style conventions in PEP 8 | MIT License | Regular, basic docs | 2.6.0 | Big, 109 contributors and 4.1k stars in github |
Custom rules implementation:
| Name | Custom rules (type) | Hardness |
|---|---|---|
| flake8 | YES: custom plugins since V.2 | Hard |
| Pylint | YES: token, RAW and AST using astroid | Medium |
| Bellybutton | YES: regex and AST (xAST) using astpath | Easy |
| PyFlakes | YES: regex, RAW and AST using astroid | Medium |
| pycodestyle | YES: token using tokenize | Hard |
Caveats
When an old file is modified, as happens now with flake8, this file must be corrected to comply with the established rules.
To solve this, as we do with flake8, we can use a configuration adapted for yapf, which can auto-correct the code according to the indicated rules.
Operation
This is an internal tool for the development team.
It will run automatically during continuous integration or on-demand in the local environment as a make command.
It will be implemented and maintained by the automation team and any developer will be able to implement new rules or request that they be implemented by the automation team, submitting a task to their backlog.
Security Impact
This proposal has no security implications.
Performance Impact
A static code check should not increase the build time in jenkins by more than a few seconds, depending on the amount of code to check.
Developer Impact
Depending on the rules that are defined, it can cause positives when someone modifies an old file that violates the rules. To comply with the verifications, it will be necessary to correct the broken rules in the modified file.
Since the rules are defined by the development or automation team, this issue must be considered.
Data Consumer Impact
This proposal has no data consumer implications.
Deployment
Deployment can be done without the DevOps team beyond participating in Pull Request reviews and uploading the chosen python package to the development pypi repository. Neither is your participation in maintenance necessary, as it would be the same as the flake8 quality gate.
Dependencies
There is no dependency on other projects.