Lessons from Wriing a Pylint Plugin

Posted on Sun 24 September 2017 in lessons

At work there's a python coding convention that I tend to overlook a lot. So when I post merge requests, there's a pretty good chance someone's going to call me out on this, which leads to a followup commit and another round of peer review. This can lead to an extra delay of a few hours until I notice the comments, switch context back to that merge request, making the changes, update the merge request and wait for another round of reviews. If I could find a way to check my code for this convention before posting the merge requsts, I could get my code merged in a few hours faster....

The Convention

The coding convention I cannot internalize is as follows: In python, the format method for strings will call the __format__ method on its arguments for you, so any code that looks like:

"interpolate these: {} {}".format(str(a), str(b))

Need only look like:

"interpolate me: {} {}".format(a, b)

The Pylint Plugin

So googling around led my to this Ned Batchelder post from a few years back. That post also led to a couple pylint plugins here. Looking at pylint's own format checker reminded me that I should also be handling keyword arguments.

From the post and sample code, it looked like I needed to define a checker class with a visit_callfunc method that would check when the 'format' method was used, and then check all the arguments to the format call and throw an error if any of them where a function call to str().

Here's what I eventually ended up.

To come up with this I used an embarassing amount of exploratory programming to figure out astroid. I wrote an initial visit_callfunc() method based on the sample code that didn't do much more than dump out all the data about the node argument via dir(node) and node.__dict__. Then I would call pylint with the plugin against some sample source with the error I was trying to plugin to report.

I run the plugin against the existing code and found one lingering case where the reviewers had allowed one of my unneccessary str() call into the codebase. It's been removed now.

Lessons Learned

  • pylint plugins are pretty powerful and I wouldn't shy away from writing another one. I'm on the lookout for other excuses to write another one.
  • https://greentreesnakes.readthedocs.io is a useful 'missing manual' for the python AST.
  • format() can take both positional and keyword arguments. My original pass at the plugin only supported positional arguments.
  • The bandit project exists and looks useful. I stumbled acros it while looking for other pylint plugins.

lessons python pylint peer reviews