Bug 1492495 - Add flake8-isort plugin to sort Python includes, with support for autofixing through isort. r=linter-reviewers,ahal

Differential Revision: https://phabricator.services.mozilla.com/D157320

.isort.cfg

@@ -0,0 +1,2 @@
+[settings]
+profile=black

tools/lint/flake8.yml

@@ -8,6 +8,8 @@ flake8:
     support-files:
         - '**/.flake8'
         - 'tools/lint/python/flake8*'
+    # Rules that should result in warnings rather than errors.
+    warning-rules: ['I001', 'I003', 'I004', 'I005']
     type: external
     payload: python.flake8:lint
     setup: python.flake8:setup

tools/lint/python/flake8.py

@@ -105,10 +105,20 @@ def lint(paths, config, **lintargs):
             "--recursive",
         ]
 
+        isort_cmd = [
+            os.path.join(virtualenv_bin_path or default_bindir(), "isort"),
+        ]
+
         if config.get("exclude"):
             fix_cmd.extend(["--exclude", ",".join(config["exclude"])])
+            isort_cmd.append("--filter-files")
+            for glob in config.get("exclude"):
+                isort_cmd.extend(["--skip", glob])
 
         subprocess.call(fix_cmd + paths)
+
+        subprocess.call(isort_cmd + paths)
+
         results = run(paths, config, **lintargs)
 
         fixed = fixed - len(results)
@@ -192,6 +202,8 @@ def run(paths, config, **lintargs):
 
     results = []
 
+    WARNING_RULES = set(config.get("warning-rules", []))
+
     def process_line(line):
         # Escape slashes otherwise JSON conversion will not work
         line = line.replace("\\", "\\\\")
@@ -204,6 +216,9 @@ def run(paths, config, **lintargs):
         if res.get("code") in LINE_OFFSETS:
             res["lineoffset"] = LINE_OFFSETS[res["code"]]
 
+        if res["rule"] in WARNING_RULES:
+            res["level"] = "warning"
+
         results.append(result.from_config(config, **res))
 
     list(map(process_line, output_file.readlines()))

tools/lint/python/flake8_requirements.in

@@ -1,4 +1,6 @@
 flake8==5.0.4
+flake8-isort==4.2.0
+isort==5.10.1
 zipp==0.5
 autopep8==1.7.0
 typing-extensions==3.10.0.2

tools/lint/python/flake8_requirements.txt

@@ -11,7 +11,19 @@ autopep8==1.7.0 \
 flake8==5.0.4 \
     --hash=sha256:6fbe320aad8d6b95cec8b8e47bc933004678dc63095be98528b7bdd2a9f510db \
     --hash=sha256:7a1cf6b73744f5806ab95e526f6f0d8c01c66d7bbe349562d22dfca20610b248
+    # via
+    #   -r tools/lint/python/flake8_requirements.in
+    #   flake8-isort
+flake8-isort==4.2.0 \
+    --hash=sha256:26571500cd54976bbc0cf1006ffbcd1a68dd102f816b7a1051b219616ba9fee0 \
+    --hash=sha256:5b87630fb3719bf4c1833fd11e0d9534f43efdeba524863e15d8f14a7ef6adbf
     # via -r tools/lint/python/flake8_requirements.in
+isort==5.10.1 \
+    --hash=sha256:6f62d78e2f89b4500b080fe3a81690850cd254227f27f75c3a0c491a1f351ba7 \
+    --hash=sha256:e8443a5e7a020e9d7f97f1d7d9cd17c88bcb3bc7e218bf9cf5095fe550be2951
+    # via
+    #   -r tools/lint/python/flake8_requirements.in
+    #   flake8-isort
 mccabe==0.7.0 \
     --hash=sha256:348e0240c33b60bbdf4e523192ef919f28cb2c3d7d5c7794f74009290f236325 \
     --hash=sha256:6c2d30ab6be0e4a46919781807b4f0d834ebdd6c6e3dca0bda5a15f863427b6e

tools/lint/test/test_flake8.py

@@ -10,7 +10,9 @@ def test_lint_single_file(lint, paths):
     results = lint(paths("bad.py"))
     assert len(results) == 2
     assert results[0].rule == "F401"
+    assert results[0].level == "error"
     assert results[1].rule == "E501"
+    assert results[1].level == "error"
     assert results[1].lineno == 5
 
     # run lint again to make sure the previous results aren't counted twice
@@ -37,12 +39,12 @@ def foobar():
 
     path = create_temp_file(contents, name="bad.py")
     results = lint([path])
-    assert len(results) == 2
+    assert len(results) == 3
 
     # Make sure the missing blank line is fixed, but the unused import isn't.
     results = lint([path], fix=True)
     assert len(results) == 1
-    assert fixed == 1
+    assert fixed == 2
 
     fixed = 0
 
@@ -51,7 +53,7 @@ def foobar():
     results = lint([path], fix=True)
     # There should now be two files with 2 combined errors
     assert len(results) == 2
-    assert fixed == 1
+    assert fixed == 2
     assert all(r.rule != "E501" for r in results)
 
 
@@ -111,5 +113,25 @@ def test_lint_uses_custom_extensions(lint, paths):
     assert len(lint(paths("ext/bad.configure"))) == 1
 
 
+def test_lint_isort(lint, create_temp_file):
+    contents = """
+import prova
+import collections
+
+
+def foobar():
+    c = collections.Counter()
+    prova.ciao(c)
+""".lstrip()
+
+    path = create_temp_file(contents, name="bad.py")
+    results = lint([path])
+    assert len(results) == 2
+    assert results[0].rule == "I003"
+    assert results[0].level == "warning"
+    assert results[1].rule == "I001"
+    assert results[1].level == "warning"
+
+
 if __name__ == "__main__":
     mozunit.main()