--- /dev/null
+Lessons from Wriing a Pylint Plugin
+###################################
+
+:date: 2017-09-24
+:tags: lessons,python,pylint,peer reviews
+:category: lessons
+:author: Jude N
+
+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:
+
+.. code-block:: python
+
+ "interpolate these: {} {}".format(str(a), str(b))
+
+Need only look like:
+
+.. code-block:: python
+
+ "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.
+
+.. raw:: html
+
+ <script src="https://gist.github.com/jude/aa599e3f9ea43fc0fa2f490dd5845690.js"></script>
+
+
+To come up with this I used an embarassing amount of `exploratory programming`_ to figure out astroid. I wrote an initial
+: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
+:code:`dir(node)` and :code:`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.
+
+
+.. _this Ned Batchelder post: https://nedbatchelder.com/blog/201505/writing_pylint_plugins.html
+.. _here: https://github.com/edx/edx-lint/tree/master/edx_lint/pylint
+.. _pylint's own format checker: https://github.com/PyCQA/pylint/blob/master/pylint/checkers/strings.py#L227
+.. _exploratory programming: http://wiki.c2.com/?ExploratoryProgramming
+.. _bandit: https://wiki.openstack.org/wiki/Security/Projects/Bandit
+
projects / pwan.org.git / blobdiff