X-Git-Url: https://pwan.org/git/?p=pwan.org.git;a=blobdiff_plain;f=content%2Flessons%2FPylintPlugin.rst;fp=content%2Flessons%2FPylintPlugin.rst;h=5681fe11e69c9e1567d34699ddafe2151c62f34f;hp=0000000000000000000000000000000000000000;hb=6b4cb8e61e634b989a887c60fdd26d7140a8224f;hpb=78cc7b9eb414b40bc21c491f6809929573fffc7b

diff --git a/content/lessons/PylintPlugin.rst b/content/lessons/PylintPlugin.rst
new file mode 100755
index 0000000..5681fe1
--- /dev/null
+++ b/content/lessons/PylintPlugin.rst
@@ -0,0 +1,75 @@
+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
+