Add pylint action to make and include it in precommit. Clean up pylint warnings.

.pylintrc

+[BASIC]
+argument-rgx=^[a-z][a-z0-9_]*$
+attr-rgx=^_{0,2}[a-z][a-z0-9_]*$
+const-rgx=^(_?[A-Z][A-Z0-9_]*|__[a-z0-9_]+__|_?[a-z][a-z0-9_]*)$
+docstring-min-length=10
+function-rgx=^(?:(?P<camel_case>_?[A-Z][a-zA-Z0-9]*)|(?P<snake_case>_?[a-z][a-z0-9_]*))$
+good-names=main,_
+method-rgx=^(?:(?P<exempt>__[a-z0-9_]+__|next|test[A-Z_][A-Za-z0-9_]*)|(?P<camel_case>_{0,2}[A-Z][a-zA-Z0-9]*)|(?P<snake_case>_{0,2}[a-z][a-z0-9_]*))$
+no-docstring-rgx=(__.*__|main|test[A-Z_][A-Za-z0-9_]*|[A-Z][A-Za-z0-9]*Test)
+variable-rgx=^[a-z][a-z0-9_]*$
+
+[FORMAT]
+indent-string='  '
+max-line-length=80
+
+[MESSAGES CONTROL]
+# TODO: Remove cyclic-import once expr_visitor.py -> stmt.py is resolved.
+disable=design,similarities,no-self-use,attribute-defined-outside-init,locally-disabled,star-args,pointless-except,bad-option-value,global-statement,fixme,suppressed-message,useless-suppression,locally-enabled,file-ignored,cyclic-import
+
+[REPORTS]
+msg-template={path}:{line}: {msg} ({symbol})
+reports=no

Makefile

@@ -21,7 +21,7 @@ GOOS ?= $(word 1,$(GO_ENV))
 GOARCH ?= $(word 2,$(GO_ENV))
 ROOT_DIR := $(realpath .)
 PKG_DIR := build/pkg/$(GOOS)_$(GOARCH)
-PY_DIR := build/site-packages
+PY_DIR := build/lib/python2.7/site-packages
 
 export GOPATH := $(ROOT_DIR)/build
 export PYTHONPATH := $(ROOT_DIR)/$(PY_DIR)
@@ -31,10 +31,10 @@ COMPILER_BIN := build/bin/grumpc
 COMPILER_SRCS := $(addprefix $(PY_DIR)/grumpy/compiler/,$(notdir $(shell find compiler -name '*.py' -not -name '*_test.py'))) $(PY_DIR)/grumpy/__init__.py
 COMPILER_TESTS := $(patsubst %.py,grumpy/%,$(filter-out compiler/expr_visitor_test.py compiler/stmt_test.py,$(wildcard compiler/*_test.py)))
 COMPILER_TEST_SRCS := $(patsubst %,$(PY_DIR)/%.py,$(COMPILER_TESTS))
-COMPILER_SHARDED_TEST_SRCS := $(patsubst %,build/site-packages/grumpy/compiler/%,expr_visitor_test.py stmt_test.py)
+COMPILER_SHARDED_TEST_SRCS := $(patsubst %,$(PY_DIR)/grumpy/compiler/%,expr_visitor_test.py stmt_test.py)
 COMPILER_PASS_FILES := $(patsubst %,$(PY_DIR)/%.pass,$(COMPILER_TESTS))
-COMPILER_EXPR_VISITOR_PASS_FILES := $(patsubst %,build/site-packages/grumpy/compiler/expr_visitor_test.%of32.pass,$(shell seq 32))
-COMPILER_STMT_PASS_FILES := $(patsubst %,build/site-packages/grumpy/compiler/stmt_test.%of16.pass,$(shell seq 16))
+COMPILER_EXPR_VISITOR_PASS_FILES := $(patsubst %,$(PY_DIR)/grumpy/compiler/expr_visitor_test.%of32.pass,$(shell seq 32))
+COMPILER_STMT_PASS_FILES := $(patsubst %,$(PY_DIR)/grumpy/compiler/stmt_test.%of16.pass,$(shell seq 16))
 COMPILER_D_FILES := $(patsubst %,$(PY_DIR)/%.d,$(COMPILER_TESTS))
 COMPILER := $(COMPILER_BIN) $(COMPILER_SRCS)
 
@@ -64,6 +64,7 @@ BENCHMARK_BINS := $(patsubst %,build/%_benchmark,$(BENCHMARKS))
 TOOL_BINS = $(patsubst %,build/bin/%,benchcmp coverparse diffrange)
 
 GOLINT_BIN = build/bin/golint
+PYLINT_BIN = build/bin/pylint
 
 all: $(COMPILER) $(RUNTIME) $(STDLIB) $(TOOL_BINS)
 
@@ -80,7 +81,7 @@ test: $(ACCEPT_PASS_FILES) $(COMPILER_PASS_FILES) $(COMPILER_EXPR_VISITOR_PASS_F
 
 precommit: cover lint test
 
-.PHONY: all benchmarks clean cover lint precommit run test
+.PHONY: all benchmarks clean cover golint lint precommit pylint run test
 
 # ------------------------------------------------------------------------------
 # grumpc compiler
@@ -105,13 +106,13 @@ $(COMPILER_D_FILES): $(PY_DIR)/%.d: $(PY_DIR)/%.py $(COMPILER_SRCS)
 -include $(COMPILER_D_FILES)
 
 # Does not depend on stdlibs since it makes minimal use of them.
-$(COMPILER_EXPR_VISITOR_PASS_FILES): build/site-packages/grumpy/compiler/expr_visitor_test.%.pass: build/site-packages/grumpy/compiler/expr_visitor_test.py $(RUNNER_BIN) $(COMPILER) $(RUNTIME)
+$(COMPILER_EXPR_VISITOR_PASS_FILES): $(PY_DIR)/grumpy/compiler/expr_visitor_test.%.pass: $(PY_DIR)/grumpy/compiler/expr_visitor_test.py $(RUNNER_BIN) $(COMPILER) $(RUNTIME)
 	@python $< --shard=$*
 	@touch $@
 	@echo 'compiler/expr_visitor_test $* PASS'
 
 # Does not depend on stdlibs since it makes minimal use of them.
-$(COMPILER_STMT_PASS_FILES): build/site-packages/grumpy/compiler/stmt_test.%.pass: build/site-packages/grumpy/compiler/stmt_test.py $(RUNNER_BIN) $(COMPILER) $(RUNTIME)
+$(COMPILER_STMT_PASS_FILES): $(PY_DIR)/grumpy/compiler/stmt_test.%.pass: $(PY_DIR)/grumpy/compiler/stmt_test.py $(RUNNER_BIN) $(COMPILER) $(RUNTIME)
 	@python $< --shard=$*
 	@touch $@
 	@echo 'compiler/stmt_test $* PASS'
@@ -143,9 +144,17 @@ cover: $(RUNTIME_COVER_FILE) $(TOOL_BINS)
 $(GOLINT_BIN):
 	@go get -u github.com/golang/lint/golint
 
-lint: $(GOLINT_BIN)
+golint: $(GOLINT_BIN) $(PYLINT_BIN)
 	@$(GOLINT_BIN) -set_exit_status runtime
 
+$(PYLINT_BIN):
+	@pip install --prefix=$(ROOT_DIR)/build pylint
+
+pylint: $(PYLINT_BIN)
+	@$(PYLINT_BIN) compiler/*.py tools/{benchcmp,coverparse,diffrange,grumpc,grumprun}
+
+lint: golint pylint
+
 # ------------------------------------------------------------------------------
 # Standard library
 # ------------------------------------------------------------------------------

compiler/block.py

@@ -53,6 +53,16 @@ class Block(object):
 
   __metaclass__ = abc.ABCMeta
 
+  # These are ModuleBlock attributes. Avoid pylint errors for accessing them on
+  # Block objects by defining them here.
+  _filename = None
+  _full_package_name = None
+  _libroot = None
+  _lines = None
+  _runtime = None
+  _strings = None
+  imports = None
+
   def __init__(self, parent_block, name):
     self.parent_block = parent_block
     self.name = name

compiler/expr_visitor.py

@@ -202,7 +202,7 @@ class ExprVisitor(ast.NodeVisitor):
     body = ast.Expr(value=ast.Yield(node.elt), lineno=None)
     for comp_node in reversed(node.generators):
       for if_node in reversed(comp_node.ifs):
-        body = ast.If(test=if_node, body=[body], orelse=[], lineno=None)
+        body = ast.If(test=if_node, body=[body], orelse=[], lineno=None)  # pylint: disable=redefined-variable-type
       body = ast.For(target=comp_node.target, iter=comp_node.iter,
                      body=[body], orelse=[], lineno=None)
 

compiler/expr_visitor_test.py

@@ -43,6 +43,7 @@ def _MakeLiteralTest(lit):
 
 
 def _MakeSliceTest(subscript, want):
+  """Define a test function that evaluates a slice expression."""
   def Test(self):
     code = textwrap.dedent("""\
         class Slicer(object):

compiler/stmt_test.py

@@ -423,7 +423,7 @@ class StatementVisitorTest(unittest.TestCase):
     visitor = stmt.StatementVisitor(_MakeModuleBlock())
     handlers = [ast.ExceptHandler(type=ast.Name(id='foo')),
                 ast.ExceptHandler(type=None)]
-    self.assertEqual(visitor._write_except_dispatcher(
+    self.assertEqual(visitor._write_except_dispatcher(  # pylint: disable=protected-access
         'exc', 'tb', handlers), [1, 2])
     expected = re.compile(r'ResolveGlobal\(.*foo.*\bIsInstance\(.*'
                           r'goto Label1.*goto Label2', re.DOTALL)
@@ -434,14 +434,14 @@ class StatementVisitorTest(unittest.TestCase):
     handlers = [ast.ExceptHandler(type=None),
                 ast.ExceptHandler(type=ast.Name(id='foo'))]
     self.assertRaisesRegexp(util.ParseError, r"default 'except:' must be last",
-                            visitor._write_except_dispatcher,
+                            visitor._write_except_dispatcher,  # pylint: disable=protected-access
                             'exc', 'tb', handlers)
 
   def testWriteExceptDispatcherMultipleExcept(self):
     visitor = stmt.StatementVisitor(_MakeModuleBlock())
     handlers = [ast.ExceptHandler(type=ast.Name(id='foo')),
                 ast.ExceptHandler(type=ast.Name(id='bar'))]
-    self.assertEqual(visitor._write_except_dispatcher(
+    self.assertEqual(visitor._write_except_dispatcher(  # pylint: disable=protected-access
         'exc', 'tb', handlers), [1, 2])
     expected = re.compile(
         r'ResolveGlobal\(.*foo.*\bif .*\bIsInstance\(.*\{.*goto Label1.*'

tools/diffrange

@@ -23,28 +23,28 @@ class _LineBuffer(object):
   """Iterator over lines in a file supporting one-step rewind."""
 
   def __init__(self, f):
-    self.f = f
-    self.prev = None
-    self.next = None
+    self._f = f
+    self._prev = None
+    self._next = None
 
   def __iter__(self):
     return self
 
   def next(self):
-    if self.next is not None:
-      cur = self.next
+    if self._next is not None:
+      cur = self._next
     else:
-      cur = self.f.readline()
+      cur = self._f.readline()
     if not cur:
       raise StopIteration
-    self.next = None
-    self.prev = cur
+    self._next = None
+    self._prev = cur
     return cur
 
   def Rewind(self):
-    assert self.prev is not None
-    self.next = self.prev
-    self.prev = None
+    assert self._prev is not None
+    self._next = self._prev
+    self._prev = None
 
 
 def _ReadHunks(buf):