Merge pull request #11834 from ethereum/improve-pylint-all

Improve `pylint_all.sh`

scripts/pylint_all.py

@@ -1,23 +1,32 @@
 #! /usr/bin/env python3
 
 """
-Performs pylint on all python files in the project repo's {test,script,docs} directory recursively.
-
-This script is meant to be run from the CI but can also be easily in local dev environment,
-where you can optionally pass `-d` as command line argument to let this script abort on first error.
+Runs pylint on all Python files in project directories known to contain Python scripts.
 """
 
-from os import path, walk, system
-from sys import argv, exit as exitwith
+from argparse import ArgumentParser
+from os import path, walk
+from sys import exit
+from textwrap import dedent
+import subprocess
+import sys
 
-PROJECT_ROOT = path.dirname(path.realpath(__file__))
-PYLINT_RCFILE = "{}/pylintrc".format(PROJECT_ROOT)
+PROJECT_ROOT = path.dirname(path.dirname(path.realpath(__file__)))
+PYLINT_RCFILE = f"{PROJECT_ROOT}/scripts/pylintrc"
 
 SGR_INFO = "\033[1;32m"
 SGR_CLEAR = "\033[0m"
 
 def pylint_all_filenames(dev_mode, rootdirs):
     """ Performs pylint on all python files within given root directory (recursively).  """
+
+    BARE_COMMAND = [
+        "pylint",
+        f"--rcfile={PYLINT_RCFILE}",
+        "--output-format=colorized",
+        "--score=n"
+    ]
+
     filenames = []
     for rootdir in rootdirs:
         for rootpath, _, filenames_w in walk(rootdir):
@@ -25,33 +34,69 @@ def pylint_all_filenames(dev_mode, rootdirs):
                 if filename.endswith('.py'):
                     filenames.append(path.join(rootpath, filename))
 
-    checked_count = 0
-    failed = []
-    for filename in filenames:
-        checked_count += 1
-        cmdline = "pylint --rcfile=\"{}\" \"{}\"".format(PYLINT_RCFILE, filename)
-        print("{}[{}/{}] Running pylint on file: {}{}".format(SGR_INFO, checked_count, len(filenames),
-                                                              filename, SGR_CLEAR))
-        exit_code = system(cmdline)
-        if exit_code != 0:
-            if dev_mode:
-                return 1, checked_count
-            failed.append(filename)
+    if not dev_mode:
+        # NOTE: We could just give pylint the directories and it would find the files on its
+        # own but it would treat them as packages, which would result in lots of import errors.
+        command_line = BARE_COMMAND + filenames
+        return subprocess.run(command_line, check=False).returncode == 0
+
+    for i, filename in enumerate(filenames):
+        command_line = BARE_COMMAND + [filename]
+        print(
+            f"{SGR_INFO}"
+            f"[{i + 1}/{len(filenames)}] "
+            f"Running pylint on file: {filename}{SGR_CLEAR}"
+        )
+
+        process = subprocess.run(command_line, check=False)
+
+        if process.returncode != 0:
+            return False
+
+    print()
+    return True
+
+
+def parse_command_line():
+    script_description = dedent("""
+        Runs pylint on all Python files in project directories known to contain Python scripts.
+
+        This script is meant to be run from the CI but can also be easily used in the local dev
+        environment.
+    """)
+
+    parser = ArgumentParser(description=script_description)
+    parser.add_argument(
+        '-d', '--dev-mode',
+        dest='dev_mode',
+        default=False,
+        action='store_true',
+        help=(
+            "Abort on first error. "
+            "In this mode every script is passed to pylint separately. "
+        )
+    )
+    return parser.parse_args()
 
-    return len(failed), len(filenames)
 
 def main():
-    """ Collects all python script root dirs and runs pylint on them. You can optionally
-        pass `-d` as command line argument to let this script abort on first error. """
-    dev_mode = len(argv) == 2 and argv[1] == "-d"
-    failed_count, total_count = pylint_all_filenames(dev_mode, [
-        path.abspath(path.dirname(__file__) + "/../docs"),
-        path.abspath(path.dirname(__file__) + "/../scripts"),
-        path.abspath(path.dirname(__file__) + "/../test")])
-    if failed_count != 0:
-        exitwith("pylint failed on {}/{} files.".format(failed_count, total_count))
+    options = parse_command_line()
+
+    rootdirs = [
+        f"{PROJECT_ROOT}/docs",
+        f"{PROJECT_ROOT}/scripts",
+        f"{PROJECT_ROOT}/test",
+    ]
+    success = pylint_all_filenames(options.dev_mode, rootdirs)
+
+    if not success:
+        exit(1)
     else:
-        print("Successfully tested {} files.".format(total_count))
+        print("No problems found.")
+
 
 if __name__ == "__main__":
-    main()
+    try:
+        main()
+    except KeyboardInterrupt:
+        exit("Interrupted by user. Exiting.")

scripts/pylintrc

@@ -21,6 +21,7 @@ disable=
     bad-indentation,
     bad-whitespace,
     consider-using-sys-exit,
+    duplicate-code,
     invalid-name,
     missing-docstring,
     mixed-indentation,