From patchwork Sat Sep 30 12:02:23 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [17, of,
 17] contrib: add a check to check-code to ban superfluous pass
 statements
From: Augie Fackler <raf@durin42.com>
X-Patchwork-Id: 24250
Message-Id: <f4cdd3ad19cf7638f1b8.1506772943@augie-macbookpro2.roam.corp.google.com>
To: mercurial-devel@mercurial-scm.org
Date: Sat, 30 Sep 2017 08:02:23 -0400

# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1506700544 14400
#      Fri Sep 29 11:55:44 2017 -0400
# Node ID f4cdd3ad19cf7638f1b80dc33f292064177efe52
# Parent  8016928028e542909be78f9f0233b5e6cf73cc40
contrib: add a check to check-code to ban superfluous pass statements

These have annoyed me for a long time, and I'm tired of commenting on
them in reviews. I'm sorry for how complicated the regular expression
is, but I was too lazy to go crack open pylint's code and add the
check there.

diff --git a/contrib/check-code.py b/contrib/check-code.py
--- a/contrib/check-code.py
+++ b/contrib/check-code.py
@@ -257,6 +257,16 @@ pypats = [
     (r'(\w|\))[+/*\-<>]\w', "missing whitespace in expression"),
     (r'^\s+(\w|\.)+=\w[^,()\n]*$', "missing whitespace in assignment"),
     (r'\w\s=\s\s+\w', "gratuitous whitespace after ="),
+    ((
+        # a line ending with a colon, potentially with trailing comments
+        r':([ \t]*#[^\n]*)?\n'
+        # one that is not a pass and not only a comment
+        r'(?P<indent>[ \t]+)[^#][^\n]+\n'
+        # more lines at the same indent level
+        r'((?P=indent)[^\n]+\n)*'
+        # a pass at the same indent level, which is bogus
+        r'(?P=indent)pass[ \t\n#]'
+      ), 'omit superfluous pass'),
     (r'.{81}', "line too long"),
     (r'[^\n]\Z', "no trailing newline"),
     (r'(\S[ \t]+|^[ \t]+)\n', "trailing whitespace"),
diff --git a/tests/test-contrib-check-code.t b/tests/test-contrib-check-code.t
--- a/tests/test-contrib-check-code.t
+++ b/tests/test-contrib-check-code.t
@@ -285,11 +285,43 @@ web templates
   >           ''' "%-6d \n 123456 .:*+-= foobar")
   > EOF
 
+superfluous pass
+
+  $ cat > superfluous_pass.py <<EOF
+  > # correct examples
+  > if foo:
+  >     pass
+  > else:
+  >     # comment-only line means still need pass
+  >     pass
+  > def nothing():
+  >     pass
+  > class empty(object):
+  >     pass
+  > if whatever:
+  >     passvalue(value)
+  > # bad examples
+  > if foo:
+  >     "foo"
+  >     pass
+  > else: # trailing comment doesn't fool checker
+  >     wat()
+  >     pass
+  > def nothing():
+  >     "docstring means no pass"
+  >     pass
+  > class empty(object):
+  >     """multiline
+  >     docstring also
+  >     means no pass"""
+  >     pass
+  > EOF
+
 (Checking multiple invalid files at once examines whether caching
 translation table for repquote() works as expected or not. All files
 should break rules depending on result of repquote(), in this case)
 
-  $ "$check_code" stringjoin.py uigettext.py
+  $ "$check_code" stringjoin.py uigettext.py superfluous_pass.py
   stringjoin.py:1:
    > foo = (' foo'
    string join across lines with no space
@@ -317,4 +349,16 @@ should break rules depending on result o
   uigettext.py:1:
    > ui.status("% 10s %05d % -3.2f %*s %%"
    missing _() in ui message (use () to hide false-positives)
+  superfluous_pass.py:14:
+   > if foo:
+   omit superfluous pass
+  superfluous_pass.py:17:
+   > else: # trailing comment doesn't fool checker
+   omit superfluous pass
+  superfluous_pass.py:20:
+   > def nothing():
+   omit superfluous pass
+  superfluous_pass.py:23:
+   > class empty(object):
+   omit superfluous pass
   [1]