diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 000000000000..df8e58f085ba --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,54 @@ +name: Lint + +on: [pull_request] + +jobs: + lintrunner: + runs-on: ubuntu-latest + steps: + - name: Pull DGL + uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - name: Checkout master and HEAD + run: | + git checkout -t origin/master + git checkout ${{ github.event.pull_request.head.sha }} + + - name: Setup Python + uses: actions/setup-python@v4 + with: + python-version: '3.8' + + - name: Install requirements + run: | + python -m pip install --upgrade pip + pip install lintrunner --user + + - name: Initialize lint dependencies + run: lintrunner init + + - name: Run lintrunner on all changed files + run: | + set +e + if ! lintrunner --force-color -m master --tee-json=lint.json; then + echo "" + echo -e "\e[1m\e[36mYou can reproduce these results locally by using \`lintrunner\`.\e[0m" + echo -e "\e[1m\e[36mSee https://github.com/pytorch/pytorch/wiki/lintrunner for setup instructions.\e[0m" + exit 1 + fi + + - name: Store annotations + if: always() && github.event_name == 'pull_request' + # Don't show this as an error; the above step will have already failed. + continue-on-error: true + run: | + # Use jq to massage the JSON lint output into GitHub Actions workflow commands. + jq --raw-output \ + '"::\(if .severity == "advice" or .severity == "disabled" then "warning" else .severity end) file=\(.path),line=\(.line),col=\(.char),title=\(.code) \(.name)::" + (.description | gsub("\\n"; "%0A"))' \ + lint.json + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}-${{ github.event_name == 'workflow_dispatch' }} + cancel-in-progress: true diff --git a/.lintrunner.toml b/.lintrunner.toml new file mode 100644 index 000000000000..8a1a2930761d --- /dev/null +++ b/.lintrunner.toml @@ -0,0 +1,56 @@ +# Black + usort +[[linter]] +code = 'UFMT' +include_patterns = [ + '**/*.py', +] +command = [ + 'python3', + 'tests/lint/ufmt_linter.py', + '--', + '@{{PATHSFILE}}' +] +exclude_patterns = [ + '.github/*', + 'build/*', + 'cmake/*', + 'conda/*', + 'docker/*', + 'third_party/*', +] +init_command = [ + 'python3', + 'tests/lint/pip_init.py', + '--dry-run={{DRYRUN}}', + 'black==22.10.0', + 'ufmt==2.0.1', + 'usort==1.0.5', +] +is_formatter = true + +[[linter]] +code = 'CLANGFORMAT' +include_patterns = [ + '**/*.h', + '**/*.c', + '**/*.cc', + '**/*.cpp', + '**/*.cuh', + '**/*.cu', +] +exclude_patterns = [ +] +init_command = [ + 'python3', + 'tests/lint/pip_init.py', + '--dry-run={{DRYRUN}}', + 'clang-format==15.0.4', +] +command = [ + 'python3', + 'tests/lint/clangformat_linter.py', + '--binary=clang-format', + '--', + '@{{PATHSFILE}}' +] +is_formatter = true diff --git a/pyproject.toml b/pyproject.toml index b98f647f27f6..2cba48472eb9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,22 +1,3 @@ [tool.black] line-length = 80 - - -[tool.autopep8] - -max_line_length = 80 -in-place = true -aggressive = 3 -# Add the path to here if you want to exclude them from autopep8 auto reformat. -# When a directory or multiple files are passed to autopep8, it will ignore the -# following directory and files. It is not recommended to pass a directory to -# autopep8. -exclude = ''' - .github/*, - build/*, - cmake/*, - conda/*, - docker/*, - third_party/*, -''' diff --git a/tests/lint/clangformat_linter.py b/tests/lint/clangformat_linter.py new file mode 100644 index 000000000000..eee19e37f8da --- /dev/null +++ b/tests/lint/clangformat_linter.py @@ -0,0 +1,242 @@ +"""Borrowed from github.com/pytorch/pytorch/tools/linter/adapters/clangformat_linter.py""" +import argparse +import concurrent.futures +import json +import logging +import os +import subprocess +import sys +import time +from enum import Enum +from pathlib import Path +from typing import Any, List, NamedTuple, Optional + + +IS_WINDOWS: bool = os.name == "nt" + + +def eprint(*args: Any, **kwargs: Any) -> None: + print(*args, file=sys.stderr, flush=True, **kwargs) + + +class LintSeverity(str, Enum): + ERROR = "error" + WARNING = "warning" + ADVICE = "advice" + DISABLED = "disabled" + + +class LintMessage(NamedTuple): + path: Optional[str] + line: Optional[int] + char: Optional[int] + code: str + severity: LintSeverity + name: str + original: Optional[str] + replacement: Optional[str] + description: Optional[str] + + +def as_posix(name: str) -> str: + return name.replace("\\", "/") if IS_WINDOWS else name + + +def _run_command( + args: List[str], + *, + timeout: int, +) -> "subprocess.CompletedProcess[bytes]": + logging.debug("$ %s", " ".join(args)) + start_time = time.monotonic() + try: + return subprocess.run( + args, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + shell=IS_WINDOWS, # So batch scripts are found. + timeout=timeout, + check=True, + ) + finally: + end_time = time.monotonic() + logging.debug("took %dms", (end_time - start_time) * 1000) + + +def run_command( + args: List[str], + *, + retries: int, + timeout: int, +) -> "subprocess.CompletedProcess[bytes]": + remaining_retries = retries + while True: + try: + return _run_command(args, timeout=timeout) + except subprocess.TimeoutExpired as err: + if remaining_retries == 0: + raise err + remaining_retries -= 1 + logging.warning( + "(%s/%s) Retrying because command failed with: %r", + retries - remaining_retries, + retries, + err, + ) + time.sleep(1) + + +def check_file( + filename: str, + binary: str, + retries: int, + timeout: int, +) -> List[LintMessage]: + try: + with open(filename, "rb") as f: + original = f.read() + proc = run_command( + [binary, filename], + retries=retries, + timeout=timeout, + ) + except subprocess.TimeoutExpired: + return [ + LintMessage( + path=filename, + line=None, + char=None, + code="CLANGFORMAT", + severity=LintSeverity.ERROR, + name="timeout", + original=None, + replacement=None, + description=( + "clang-format timed out while trying to process a file. " + "Please report an issue in pytorch/pytorch with the " + "label 'module: lint'" + ), + ) + ] + except (OSError, subprocess.CalledProcessError) as err: + return [ + LintMessage( + path=filename, + line=None, + char=None, + code="CLANGFORMAT", + severity=LintSeverity.ADVICE, + name="command-failed", + original=None, + replacement=None, + description=( + f"Failed due to {err.__class__.__name__}:\n{err}" + if not isinstance(err, subprocess.CalledProcessError) + else ( + "COMMAND (exit code {returncode})\n" + "{command}\n\n" + "STDERR\n{stderr}\n\n" + "STDOUT\n{stdout}" + ).format( + returncode=err.returncode, + command=" ".join(as_posix(x) for x in err.cmd), + stderr=err.stderr.decode("utf-8").strip() or "(empty)", + stdout=err.stdout.decode("utf-8").strip() or "(empty)", + ) + ), + ) + ] + + replacement = proc.stdout + if original == replacement: + return [] + + line = 0 + original = original.decode("utf-8") + replacement = replacement.decode("utf-8") + for line, (i, j) in enumerate( + zip(original.split("\n"), replacement.split("\n")) + ): + if i != j: + break + + return [ + LintMessage( + path=filename, + line=line, + char=None, + code="CLANGFORMAT", + severity=LintSeverity.WARNING, + name="format", + original=original, + replacement=replacement, + description="See https://clang.llvm.org/docs/ClangFormat.html.\nRun `lintrunner -a` to apply this patch.", + ) + ] + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Format files with clang-format.", + fromfile_prefix_chars="@", + ) + parser.add_argument( + "--binary", + required=True, + help="clang-format binary path", + ) + parser.add_argument( + "--retries", + default=3, + type=int, + help="times to retry timed out clang-format", + ) + parser.add_argument( + "--timeout", + default=90, + type=int, + help="seconds to wait for clang-format", + ) + parser.add_argument( + "--verbose", + action="store_true", + help="verbose logging", + ) + parser.add_argument( + "filenames", + nargs="+", + help="paths to lint", + ) + args = parser.parse_args() + + logging.basicConfig( + format="<%(threadName)s:%(levelname)s> %(message)s", + level=logging.NOTSET + if args.verbose + else logging.DEBUG + if len(args.filenames) < 1000 + else logging.INFO, + stream=sys.stderr, + ) + + with concurrent.futures.ThreadPoolExecutor( + max_workers=os.cpu_count(), + thread_name_prefix="Thread", + ) as executor: + futures = { + executor.submit( + check_file, x, args.binary, args.retries, args.timeout + ): x + for x in args.filenames + } + for future in concurrent.futures.as_completed(futures): + try: + for lint_message in future.result(): + print(json.dumps(lint_message._asdict()), flush=True) + except Exception: + logging.critical('Failed at "%s".', futures[future]) + raise + + +if __name__ == "__main__": + main() diff --git a/tests/lint/pip_init.py b/tests/lint/pip_init.py new file mode 100644 index 000000000000..600976c7add6 --- /dev/null +++ b/tests/lint/pip_init.py @@ -0,0 +1,86 @@ +""" +Initializer script that installs stuff to pip. + +Borrowed from github.com/pytorch/pytorch/tools/linter/adapters/pip_init.py +""" +import argparse +import logging +import os +import subprocess +import sys +import time + +from typing import List + + +def run_command(args: List[str]) -> "subprocess.CompletedProcess[bytes]": + logging.debug("$ %s", " ".join(args)) + start_time = time.monotonic() + try: + return subprocess.run(args, check=True) + finally: + end_time = time.monotonic() + logging.debug("took %dms", (end_time - start_time) * 1000) + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description="pip initializer") + parser.add_argument( + "packages", + nargs="+", + help="pip packages to install", + ) + parser.add_argument( + "--verbose", + action="store_true", + help="verbose logging", + ) + parser.add_argument( + "--dry-run", + help="do not install anything, just print what would be done.", + ) + parser.add_argument( + "--no-black-binary", + help="do not use pre-compiled binaries from pip for black.", + action="store_true", + ) + + args = parser.parse_args() + + logging.basicConfig( + format="<%(threadName)s:%(levelname)s> %(message)s", + level=logging.NOTSET if args.verbose else logging.DEBUG, + stream=sys.stderr, + ) + + pip_args = ["pip3", "install"] + + # If we are in a global install, use `--user` to install so that you do not + # need root access in order to initialize linters. + # + # However, `pip install --user` interacts poorly with virtualenvs (see: + # https://bit.ly/3vD4kvl) and conda (see: https://bit.ly/3KG7ZfU). So in + # these cases perform a regular installation. + in_conda = os.environ.get("CONDA_PREFIX") is not None + in_virtualenv = os.environ.get("VIRTUAL_ENV") is not None + if not in_conda and not in_virtualenv: + pip_args.append("--user") + + pip_args.extend(args.packages) + + for package in args.packages: + package_name, _, version = package.partition("=") + if version == "": + raise RuntimeError( + "Package {package_name} did not have a version specified. " + "Please specify a version to produce a consistent linting experience." + ) + if args.no_black_binary and "black" in package_name: + pip_args.append(f"--no-binary={package_name}") + + dry_run = args.dry_run == "1" + if dry_run: + print(f"Would have run: {pip_args}") + sys.exit(0) + + run_command(pip_args) diff --git a/tests/lint/ufmt_linter.py b/tests/lint/ufmt_linter.py new file mode 100644 index 000000000000..8326083cc271 --- /dev/null +++ b/tests/lint/ufmt_linter.py @@ -0,0 +1,149 @@ +"""Borrowed from github.com/pytorch/pytorch/tools/linter/adapters/ufmt_linter.py""" +import argparse +import concurrent.futures +import json +import logging +import os +import sys +from enum import Enum +from pathlib import Path +from typing import Any, List, NamedTuple, Optional + +from ufmt.core import make_black_config, ufmt_string +from usort import Config as UsortConfig + + +IS_WINDOWS: bool = os.name == "nt" + + +def eprint(*args: Any, **kwargs: Any) -> None: + print(*args, file=sys.stderr, flush=True, **kwargs) + + +class LintSeverity(str, Enum): + ERROR = "error" + WARNING = "warning" + ADVICE = "advice" + DISABLED = "disabled" + + +class LintMessage(NamedTuple): + path: Optional[str] + line: Optional[int] + char: Optional[int] + code: str + severity: LintSeverity + name: str + original: Optional[str] + replacement: Optional[str] + description: Optional[str] + + +def as_posix(name: str) -> str: + return name.replace("\\", "/") if IS_WINDOWS else name + + +def format_error_message(filename: str, err: Exception) -> LintMessage: + return LintMessage( + path=filename, + line=None, + char=None, + code="UFMT", + severity=LintSeverity.ADVICE, + name="command-failed", + original=None, + replacement=None, + description=(f"Failed due to {err.__class__.__name__}:\n{err}"), + ) + + +def check_file( + filename: str, +) -> List[LintMessage]: + with open(filename, "rb") as f: + original = f.read().decode("utf-8") + + try: + path = Path(filename) + + usort_config = UsortConfig.find(path) + black_config = make_black_config(path) + + # Use UFMT API to call both usort and black + replacement = ufmt_string( + path=path, + content=original, + usort_config=usort_config, + black_config=black_config, + ) + + if original == replacement: + return [] + + line = 0 + for line, (i, j) in enumerate( + zip(original.split("\n"), replacement.split("\n")) + ): + if i != j: + break + + return [ + LintMessage( + path=filename, + line=line, + char=None, + code="UFMT", + severity=LintSeverity.WARNING, + name="format", + original=original, + replacement=replacement, + description="Run `lintrunner -a` to apply this patch.", + ) + ] + except Exception as err: + return [format_error_message(filename, err)] + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Format files with ufmt (black + usort).", + fromfile_prefix_chars="@", + ) + parser.add_argument( + "--verbose", + action="store_true", + help="verbose logging", + ) + parser.add_argument( + "filenames", + nargs="+", + help="paths to lint", + ) + args = parser.parse_args() + + logging.basicConfig( + format="<%(threadName)s:%(levelname)s> %(message)s", + level=logging.NOTSET + if args.verbose + else logging.DEBUG + if len(args.filenames) < 1000 + else logging.INFO, + stream=sys.stderr, + ) + + with concurrent.futures.ThreadPoolExecutor( + max_workers=os.cpu_count(), + thread_name_prefix="Thread", + ) as executor: + futures = {executor.submit(check_file, x): x for x in args.filenames} + for future in concurrent.futures.as_completed(futures): + try: + for lint_message in future.result(): + print(json.dumps(lint_message._asdict()), flush=True) + except Exception: + logging.critical('Failed at "%s".', futures[future]) + raise + + +if __name__ == "__main__": + main()