Python code quality with Ruff, one step at a time - Part 2

Python - Ruff

You can find the first part of this post here: https://cyber.airbus.com/en/newsroom/stories/2025-10-python-code-quality-with-ruff-one-step-at-a-time-part-1 

Removing unnecessary directives noqa

At this point, I am overwhelmed by a terrible doubt: what if there are already multiple # noqa directives scattered around the code? By the way, if you were wondering, noqa probably stems from QA, as in Quality Assurance, negated by prefix no, in the same way as duality in nonduality or good in ungood (ah-ha). 

Hopefully, all checks still pass when the tool is told to ignore all noqa directives:

ruff check --ignore-noqa source

This did not entirely answer our inquiry about the potential presence of the directive in the code. I will check with grep (a search in your preferred IDE would have equally done the trick):

grep -rn noqa source

There is only one in source/app/flask_dropzone/__init__.py, line 17. That’s a relief. However, this piece of code is somewhat strange. The source file starts with an unusual copyright notice:

"""

   flask_dropzone
   ~~~~~~~~~~~~~~
   :author: Grey Li <withlihui@gmail.com>
   :copyright: (c) 2017 by Grey Li.
   :license: MIT, see LICENSE for more details.

"""

This smells like the source code of some external package which was directly committed. After some investigation, it seems to be a duplicate of version 1.5.4 of the flask-dropzone package. The only notable difference with the original code, is the removal of some arguments when instantiating the Flask blueprint of the dropzone. Maybe, the component did not entirely meet the needs of the project and it was decided to include the whole component source code with a small homebrewed enhancement.

This practice is easily understandable. And it may have been a pragmatic solution at the time. It is not uncommon either. However, it should better be avoided, as there are several downsides. The most serious one is that the presence of an external component in the codebase will eventually be forgotten.

Vulnerability patches will not be applied. It will be hard to trace the origin of this code. In some cases, it may create intellectual property issues. Instead of relying on updates provided by the third-party, maintenance costs will have to be paid. Basically, doing this, one takes full ownership of the external component’s code and its consequences.

The correct alternative consists in declaring the dependence in the requirements.txt while looking for a way to compensate the missing features. In the ideal case, the third party will fix the issue once it is reported. It is always possible to contribute and propose a pull-request. As a last and preferrably temporary resort, a patch can be devised and applied during the build phase. In any case, source code produced by a third-party should never be mixed inside your project codebase. 

For the time being, I decide to take a delayed action: I report an issue in the DFIR-IRIS tracker, to be cleaned up later.

Automatic fixes

Reaching for the low hanging fruits, I am going to activate the rules which can be automatically fixed first. The documentation indicates which rules have automatic fixes implemented. Cross-referencing these with the rules I disabled, yields E711, E712, F401, F541 and F841. Let us check with only rule E711 activated:

ruff check --output-format=concise --select E711 ./source

It found 6 errors, all of them available as fixes, but all unsafe. Damn, I forgot about that! If I activate each of the other rules separately in sequence, here is the status I get:

All scenarios seem to occur:

• sometimes only unsafe fixes are available (E711 and E712),
• some occurences do not seem to have any automatic fixes (F401),
• sometimes both safe and unsafe fixes are possible (F841),
• only in the case of F541, Ruff appears to remedy every spotted violation.

As usual, I will go for the easiest first. I reactivate rule F541 in the configuration file. This rule flags f-strings which do not contain any placeholders, such as here. The fix simply replaces f-strings by regular strings:

ruff check --fix ./source
git diff

Seems fine with me. I commit and push (wink ;).

Investigating unused variables (F841)

I will go with F841 next. This rule is about variables which are assigned but unused. First, I apply the automatic fixes.

ruff check --select F841 --fix ./source
git diff

It appears to have removed unused variables when catching exceptions, such as here. Fine with me. I commit and push without reactivating the rule in the configuration, yet. Otherwise, the pipeline would fail.

The 7 remaining F841 violations have unsafe fixes. Let’s proceed file by file and check manually.

Starting with variables has_uca_overwritten and has_uca_deny_all in app.iris_engine.access_control.utils. However, the method does not seem to be called anywhere. I prefer to eliminate all dead code. So I remove this method, commit and push.

Then, in app.iris_engine.reporter.reporter, the violation is reported twice in two methods which look diabolically similar. They have the same name. They are defined in two distinct classes, with one a subclass of the other. This smells like an ancient copy-paste. The code could probably benefit from some DRY diet. Most probably by choosing composition over inheritance and applying the strategy pattern. This is not the task I am focused on right now. 

So, I just take the time to write another issue, tagged with the quality label. The automatic fix proposed by Ruff seems riskless. Time to commit and push. Interestingly, my IDE PyCharm also highlights variable ras as unused. Is their static analysis algorithm more elaborate than Ruff?

The next unused variables are in a performance test which calls function strptime from the datetime module. The automatic fix would only remove the unused variable. But, we know function strptime does not perform any side-effects. So the whole statement seems unnecessary and can go away. Commit and push. It would have been better to run the performance test suite before commiting, to check nothing broke. But I don’t know how to execute them.

Documentation about them is sparse and they do not seem to be run during continuous integration, in contrast with components and end-to-end tests. So I report my difficulties in another issue.

The third and last part of this article will be published on the 30th of October!