content/lessons/PylintPlugin.rst

   1 Lessons from Wriing a Pylint Plugin
   2 ###################################
   3
   4 :date: 2017-09-24
   5 :tags: lessons,python,pylint,peer reviews
   6 :category: lessons
   7 :author: Jude N
   8
   9 At work there's a python coding convention that I tend to overlook a lot.
  10 So when I post merge requests, there's a pretty good chance someone's going to
  11 call me out on this, which leads to a followup commit and another round of
  12 peer review.  This can lead to an extra delay of a few hours until I notice
  13 the comments, switch context back to that merge request, making the changes,
  14 update the merge request and wait for another round of reviews.  If I could
  15 find a way to check my code for this convention before posting the merge
  16 requsts, I could get my code merged in a few hours faster....
  17
  18 The Convention
  19 --------------
  20
  21 The coding convention I cannot internalize is as follows:  In python,
  22 the format method for strings will call the __format__ method on its arguments
  23 for you, so any code that looks like:
  24
  25 .. code-block:: python
  26
  27     "interpolate these: {} {}".format(str(a), str(b))
  28
  29 Need only look like:
  30
  31 .. code-block:: python
  32
  33     "interpolate me: {} {}".format(a, b)
  34
  35 The Pylint Plugin
  36 -----------------
  37 So googling around led my to `this Ned Batchelder post`_ from a few years back.
  38 That post also led to a couple pylint plugins `here`_.    Looking at `pylint's own format checker`_
  39 reminded me that I should also be handling keyword arguments.
  40
  41 From the post and sample code, it looked like I needed to define a checker class with a visit_callfunc method
  42 that would check when the 'format' method was used, and then check all the arguments to the format call and
  43 throw an error if any of them where a function call to str().
  44
  45 Here's what I eventually ended up.
  46
  47 .. raw:: html
  48
  49     <script src="https://gist.github.com/jude/aa599e3f9ea43fc0fa2f490dd5845690.js"></script>
  50
  51
  52 To come up with this I used an embarassing amount of `exploratory programming`_ to figure out astroid.  I wrote an initial
  53 :code:`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
  54 :code:`dir(node)` and :code:`node.__dict__`.
  55 Then I would call pylint with the plugin against some sample source with the error I was trying to plugin to report.
  56
  57 I run the plugin against the existing code and found one lingering case where the reviewers had allowed one of my unneccessary str()
  58 call into the codebase.  It's been removed now.
  59
  60
  61 Lessons Learned
  62 ===============
  63
  64 - 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.
  65 - https://greentreesnakes.readthedocs.io is a useful 'missing manual' for the python AST.
  66 - format() can take both positional and keyword arguments.  My original pass at the plugin only supported positional arguments.
  67 - The `bandit`_ project exists and looks useful.  I stumbled acros it while looking for other pylint plugins.
  68
  69
  70 .. _this Ned Batchelder post: https://nedbatchelder.com/blog/201505/writing_pylint_plugins.html
  71 .. _here: https://github.com/edx/edx-lint/tree/master/edx_lint/pylint
  72 .. _pylint's own format checker: https://github.com/PyCQA/pylint/blob/master/pylint/checkers/strings.py#L227
  73 .. _exploratory programming: http://wiki.c2.com/?ExploratoryProgramming
  74 .. _bandit: https://wiki.openstack.org/wiki/Security/Projects/Bandit
  75