Bug 1811850 - [lint] Replace isort linter with ruff, r=taskgraph-reviewers,linter-reviewers,marco,jcristau

This results in some changes from our current `isort` configuration. I'm
unclear if it's because ruff isn't at 100% parity with isort, they choose
different defaults or if I missed some configuration.

Either way, the changes all look reasonable to me (see child commit), so I'm
inclined to just accept the new import format it imposes.

Differential Revision: https://phabricator.services.mozilla.com/D172348
This commit is contained in:
Andrew Halberstadt 2023-03-17 01:53:59 +00:00
parent 8a4d48a70d
commit 8b4f890651
34 changed files with 52 additions and 351 deletions

View file

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

View file

@ -4,6 +4,7 @@ line-length = 99
select = [
"E", "W", # pycodestyle
"F", # pyflakes
"I", # isort
]
ignore = [
# These should be triaged and either fixed or moved to the list below.

View file

@ -1,3 +0,0 @@
[settings]
profile=black
known_first_party=lldbutils

View file

@ -0,0 +1,4 @@
extend = "../../pyproject.toml"
[isort]
known-first-party = ["lldbutils"]

View file

@ -1,3 +0,0 @@
[settings]
profile=black
known_first_party=mach

4
python/mach/.ruff.toml Normal file
View file

@ -0,0 +1,4 @@
extend = "../../pyproject.toml"
[isort]
known-first-party = ["mach"]

View file

@ -7,6 +7,7 @@ from textwrap import TextWrapper
from mach.config import TYPE_CLASSES
from mach.decorators import Command, CommandArgument
# Interact with settings for mach.
# Currently, we only provide functionality to view what settings are

View file

@ -1,3 +0,0 @@
[settings]
profile=black
known_first_party=mozboot

View file

@ -0,0 +1,4 @@
extend = "../../pyproject.toml"
[isort]
known-first-party = ["mozboot"]

View file

@ -1,3 +0,0 @@
[settings]
profile=black
known_first_party=mozbuild

View file

@ -0,0 +1,8 @@
extend = "../../pyproject.toml"
src = [
# Treat direct imports in the test modules as first party.
"mozpack/test",
"mozbuild/test",
[isort]
known-first-party = ["mozbuild"]

View file

@ -1,3 +0,0 @@
[settings]
profile=black
known_first_party=mozlint

View file

@ -0,0 +1,4 @@
extend = "../../pyproject.toml"
[isort]
known-first-party = ["mozlint"]

View file

@ -1,3 +0,0 @@
[settings]
profile=black
known_first_party=mozperftest

View file

@ -0,0 +1,4 @@
extend = "../../pyproject.toml"
[isort]
known-first-party = ["mozperftest"]

View file

@ -1,3 +0,0 @@
[settings]
profile=black
known_first_party=mozrelease

View file

@ -0,0 +1,4 @@
extend = "../../pyproject.toml"
[isort]
known-first-party = ["mozrelease"]

View file

@ -1,3 +0,0 @@
[settings]
profile=black
known_first_party=mozterm

View file

@ -0,0 +1,4 @@
extend = "../../pyproject.toml"
[isort]
known-first-party = ["mozterm"]

View file

@ -1,3 +0,0 @@
[settings]
profile=black
known_first_party=mozversioncontrol

View file

@ -0,0 +1,4 @@
extend = "../../pyproject.toml"
[isort]
known-first-party = ["mozversioncontrol"]

View file

@ -287,21 +287,6 @@ py-pylint:
# moz.configure files are also Python files
# However, pylint has some hard time dealing with it
py-isort:
description: isort run over the gecko codebase
treeherder:
symbol: py(isort)
run:
mach: lint -v -l isort -f treeherder -f json:/builds/worker/mozlint.json *
when:
files-changed:
- '**/*.py'
- '**/.flake8'
- '**/.isort.cfg'
- 'tools/lint/isort.yml'
# moz.configure files are also Python files.
- '**/*.configure'
test-manifest:
description: lint test manifests
treeherder:

View file

@ -1,3 +0,0 @@
[settings]
profile=black
known_first_party=gecko_taskgraph

View file

@ -0,0 +1,4 @@
extend = "../../pyproject.toml"
[isort]
known-first-party = ["gecko_taskgraph"]

View file

@ -1,13 +0,0 @@
---
isort:
description: Sort python imports
# Excludes should be added to topsrcdir/.flake8.
exclude: []
extensions: ['configure', 'py']
support-files:
- '**/.flake8'
- '**/.isort.cfg'
- 'tools/lint/python/isort*'
type: external
payload: python.isort:lint
setup: python.isort:setup

View file

@ -1,138 +0,0 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
import configparser
import os
import platform
import re
import signal
import subprocess
import sys
import mozpack.path as mozpath
from mozlint import result
from mozlint.pathutils import expand_exclusions
from mozprocess import ProcessHandler
here = os.path.abspath(os.path.dirname(__file__))
ISORT_REQUIREMENTS_PATH = os.path.join(here, "isort_requirements.txt")
ISORT_INSTALL_ERROR = """
Unable to install correct version of isort
Try to install it manually with:
$ pip install -U --require-hashes -r {}
""".strip().format(
ISORT_REQUIREMENTS_PATH
)
def default_bindir():
# We use sys.prefix to find executables as that gets modified with
# virtualenv's activate_this.py, whereas sys.executable doesn't.
if platform.system() == "Windows":
return os.path.join(sys.prefix, "Scripts")
return os.path.join(sys.prefix, "bin")
def parse_issues(config, output, *, log):
would_sort = re.compile(
"^ERROR: (.*?) Imports are incorrectly sorted and/or formatted.$", re.I
)
sorted = re.compile("^Fixing (.*)$", re.I)
results = []
for line in output:
line = line.decode("utf-8")
match = would_sort.match(line)
if match:
res = {"path": match.group(1)}
results.append(result.from_config(config, **res))
continue
match = sorted.match(line)
if match:
res = {"path": match.group(1), "message": "sorted"}
results.append(result.from_config(config, **res))
continue
log.debug("Unhandled line", line)
return results
class IsortProcess(ProcessHandler):
def __init__(self, config, *args, **kwargs):
self.config = config
kwargs["stream"] = False
ProcessHandler.__init__(self, *args, **kwargs)
def run(self, *args, **kwargs):
orig = signal.signal(signal.SIGINT, signal.SIG_IGN)
ProcessHandler.run(self, *args, **kwargs)
signal.signal(signal.SIGINT, orig)
def run_process(config, cmd):
proc = IsortProcess(config, cmd)
proc.run()
try:
proc.wait()
except KeyboardInterrupt:
proc.kill()
return proc.output
def setup(root, **lintargs):
virtualenv_manager = lintargs["virtualenv_manager"]
try:
virtualenv_manager.install_pip_requirements(ISORT_REQUIREMENTS_PATH, quiet=True)
except subprocess.CalledProcessError:
print(ISORT_INSTALL_ERROR)
return 1
def lint(paths, config, **lintargs):
from isort import __version__ as isort_version
binary = os.path.join(
lintargs.get("virtualenv_bin_path") or default_bindir(), "isort"
)
log = lintargs["log"]
root = lintargs["root"]
log.debug("isort version {}".format(isort_version))
cmd_args = [
binary,
"--resolve-all-configs",
"--config-root",
root,
]
if not lintargs.get("fix"):
cmd_args.append("--check-only")
# We merge exclusion rules from .flake8 to avoid having to repeat the same exclusions twice.
flake8_config_path = os.path.join(root, ".flake8")
flake8_config = configparser.ConfigParser()
flake8_config.read(flake8_config_path)
config["exclude"].extend(
mozpath.normpath(p.strip())
for p in flake8_config.get("flake8", "exclude").split(",")
)
paths = list(expand_exclusions(paths, config, lintargs["root"]))
if len(paths) == 0:
return {"results": [], "fixed": 0}
base_command = cmd_args + paths
log.debug("Command: {}".format(" ".join(base_command)))
output = run_process(config, base_command)
results = parse_issues(config, output, log=log)
fixed = sum(1 for issue in results if issue.message == "sorted")
return {"results": results, "fixed": fixed}

View file

@ -1 +0,0 @@
isort==5.10.1

View file

@ -1,10 +0,0 @@
#
# This file is autogenerated by pip-compile with python 3.10
# To update, run:
#
# pip-compile --generate-hashes --output-file=tools/lint/python/isort_requirements.txt tools/lint/python/isort_requirements.in
#
isort==5.10.1 \
--hash=sha256:6f62d78e2f89b4500b080fe3a81690850cd254227f27f75c3a0c491a1f351ba7 \
--hash=sha256:e8443a5e7a020e9d7f97f1d7d9cd17c88bcb3bc7e218bf9cf5095fe550be2951
# via -r tools/lint/python/isort_requirements.in

View file

@ -96,7 +96,6 @@ class RuffProcess(ProcessHandler):
self.stderr = []
kwargs["stream"] = False
kwargs["universal_newlines"] = True
kwargs["processStderrLine"] = lambda line: print(line, file=sys.stderr)
ProcessHandler.__init__(self, *args, **kwargs)
def run(self, *args, **kwargs):
@ -105,8 +104,8 @@ class RuffProcess(ProcessHandler):
signal.signal(signal.SIGINT, orig)
def run_process(config, cmd):
proc = RuffProcess(config, cmd)
def run_process(config, cmd, **kwargs):
proc = RuffProcess(config, cmd, **kwargs)
proc.run()
try:
proc.wait()
@ -140,15 +139,17 @@ def lint(paths, config, log, **lintargs):
if config["exclude"]:
args.append(f"--extend-exclude={','.join(config['exclude'])}")
process_kwargs = {"processStderrLine": lambda line: log.debug(line)}
if lintargs.get("fix"):
# Do a first pass with --fix-only as the json format doesn't return the
# number of fixed issues.
output = run_process(config, args + ["--fix-only"])
output = run_process(config, args + ["--fix-only"], **process_kwargs)
matches = re.match(r"Fixed (\d+) errors?.", output)
if matches:
fixed = int(matches[1])
output = run_process(config, args + ["--format=json"])
output = run_process(config, args + ["--format=json"], **process_kwargs)
if not output:
return []

View file

@ -1,4 +0,0 @@
[flake8]
max-line-length = 100
exclude =
subdir/exclude,

View file

@ -1,8 +0,0 @@
import prova
import collections
def foobar():
c = collections.Counter()
prova.ciao(c)

View file

@ -1,9 +0,0 @@
import collections
import prova
def foobar():
c = collections.Counter()
prova.ciao(c)

View file

@ -30,5 +30,3 @@ requirements = tools/lint/python/ruff_requirements.txt
[test_shellcheck.py]
[test_trojan_source.py]
[test_yaml.py]
[test_isort.py]
requirements = tools/lint/python/isort_requirements.txt

View file

@ -1,114 +0,0 @@
# -*- coding: utf-8 -*-
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
import os
import mozunit
LINTER = "isort"
fixed = 0
def test_lint_fix(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) == 1
assert results[0].level == "error"
lint([path], fix=True)
assert fixed == 1
def test_lint_excluded_file(lint, paths, config):
# Second file is excluded from .flake8 config.
files = paths("bad.py", "subdir/exclude/bad.py", "subdir/exclude/exclude_subdir")
results = lint(files, config)
assert len(results) == 1
# First file is globally excluded, second one is from .flake8 config.
files = paths("bad.py", "subdir/exclude/bad.py", "subdir/exclude/exclude_subdir")
config["exclude"] = paths("bad.py")
results = lint(files, config)
assert len(results) == 0
# Make sure excludes also apply when running from a different cwd.
cwd = paths("subdir")[0]
os.chdir(cwd)
results = lint(paths("subdir/exclude"))
assert len(results) == 0
def test_lint_uses_all_configs(lint, paths, tmpdir):
myself = tmpdir.join("myself")
myself.mkdir()
flake8_path = tmpdir.join(".flake8")
flake8_path.write(
"""
[flake8]
exclude =
""".lstrip()
)
py_path = myself.join("good.py")
py_path.write(
"""
import os
from myself import something_else
from third_party import something
def ciao():
pass
""".lstrip()
)
results = lint([py_path.strpath])
assert len(results) == 0
isort_cfg_path = myself.join(".isort.cfg")
isort_cfg_path.write(
"""
[settings]
known_first_party = myself
""".lstrip()
)
results = lint([py_path.strpath], root=tmpdir.strpath)
assert len(results) == 1
py_path.write(
"""
import os
from third_party import something
from myself import something_else
def ciao():
pass
""".lstrip()
)
results = lint([py_path.strpath], root=tmpdir.strpath)
assert len(results) == 0
if __name__ == "__main__":
mozunit.main()