Adopt linrtunner as the linting tool - take 2 (#15085)

### Description `lintrunner` is a linter runner successfully used by pytorch, onnx and onnx-script. It provides a uniform experience running linters locally and in CI. It supports all major dev systems: Windows, Linux and MacOs. The checks are enforced by the `Python format` workflow. This PR adopts `lintrunner` to onnxruntime and fixed ~2000 flake8 errors in Python code. `lintrunner` now runs all required python lints including `ruff`(replacing `flake8`), `black` and `isort`. Future lints like `clang-format` can be added. Most errors are auto-fixed by `ruff` and the fixes should be considered robust. Lints that are more complicated to fix are applied `# noqa` for now and should be fixed in follow up PRs. ### Notable changes 1. This PR **removed some suboptimal patterns**: - `not xxx in` -> `xxx not in` membership checks - bare excepts (`except:` -> `except Exception`) - unused imports The follow up PR will remove: - `import *` - mutable values as default in function definitions (`def func(a=[])`) - more unused imports - unused local variables 2. Use `ruff` to replace `flake8`. `ruff` is much (40x) faster than flake8 and is more robust. We are using it successfully in onnx and onnx-script. It also supports auto-fixing many flake8 errors. 3. Removed the legacy flake8 ci flow and updated docs. 4. The added workflow supports SARIF code scanning reports on github, example snapshot: ![image](https://user-images.githubusercontent.com/11205048/212598953-d60ce8a9-f242-4fa8-8674-8696b704604a.png) 5. Removed `onnxruntime-python-checks-ci-pipeline` as redundant ### Motivation and Context  Unified linting experience in CI and local. Replacing #14306 --------- Signed-off-by: Justin Chu <[email protected]>
microsoft · Mar 24, 2023 · d834ec8 · d834ec8
1 parent 2de15c5
commit d834ec8
Show file tree

Hide file tree

Showing 506 changed files with 3,974 additions and 4,099 deletions.
diff --git a/.flake8 b/.flake8
diff --git a/.github/workflows/generate-skip-doc-change.py b/.github/workflows/generate-skip-doc-change.py
@@ -7,7 +7,7 @@
 GITHUB_DIR = Path(__file__).resolve().parent.parent
 
 
-class Skipped_Workflow:
+class Skipped_Workflow:  # noqa: N801
     def __init__(self, workflow_name: str, job_names: list, output_file_name: str):
         self.workflow_name = workflow_name
         self.job_names = job_names

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -8,27 +8,11 @@ on:
   pull_request:
 
 jobs:
-  lint-python:
-    name: Lint Python
+  optional-lint:
+    name: Optional Lint
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - name: flake8
-        uses: reviewdog/action-flake8@v3
-        with:
-          github_token: ${{ secrets.github_token }}
-          # Change reviewdog reporter if you need [github-pr-check, github-check, github-pr-review].
-          reporter: github-pr-check
-          # Change reporter level if you need.
-          # GitHub Status Check won't become failure with a warning.
-          level: error
-          filter_mode: file
-      - name: pyflakes
-        uses: reviewdog/action-pyflakes@v1
-        with:
-          github_token: ${{ secrets.github_token }}
-          reporter: github-pr-check
-          level: warning
+      - uses: actions/checkout@v3
       - name: misspell # Check spellings as well
         uses: reviewdog/action-misspell@v1
         with:
@@ -44,43 +28,44 @@ jobs:
           reporter: github-pr-check
           level: info
           filter_mode: file
-      - name: pyright
-        uses: jordemort/action-pyright@v1
-        with:
-          github_token: ${{ secrets.github_token }}
-          reporter: github-pr-check
-          level: warning
-          filter_mode: added
-          lib: true
-          pyright_version: 1.1.291
-      - name: pylint
-        uses: dciborow/[email protected]
-        with:
-          github_token: ${{ secrets.github_token }}
-          reporter: github-pr-check
-          level: warning
-          filter_mode: diff_context
-          glob_pattern: "**/*.py"
 
   lint-python-format:
-    # Separated black/isort from other Python linters because we want this job to
-    # fail and not affect other linters
-    # According to https://black.readthedocs.io/en/stable/integrations/github_actions.html:
-    #     We recommend the use of the @stable tag, but per version tags also exist if you prefer that.
-    #     Note that the action’s version you select is independent of the version of Black the action will use.
-    #     The version of Black the action will use can be configured via version.
+    # Required workflow
     name: Python format
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-python@v2
+      - uses: actions/checkout@v3
+      - name: Setup Python
+        uses: actions/setup-python@v4
         with:
+          # Version range or exact version of Python to use, using SemVer's version range syntax. Reads from .python-version if unset.
           python-version: "3.10"
-      - uses: psf/black@stable
+      - name: Install dependencies
+        run: |
+          python -m pip install -r requirements-dev.txt
+          python -m pip install lintrunner lintrunner-adapters
+          lintrunner init
+      - name: Run lintrunner on all files
+        run: |
+          set +e
+          if ! lintrunner --force-color --all-files --tee-json=lint.json -v; then
+              echo ""
+              echo -e "\e[1m\e[36mYou can reproduce these results locally by using \`lintrunner -m main\`.\e[0m"
+              exit 1
+          fi
+      - name: Produce SARIF
+        if: always()
+        run: |
+          python -m lintrunner_adapters to-sarif lint.json lintrunner.sarif
+      - name: Upload SARIF file
+        if: always()
+        continue-on-error: true
+        uses: github/codeql-action/upload-sarif@v2
         with:
-          options: "--check --diff --color"
-          version: "22.12.0"
-      - uses: isort/isort-action@master
+          # Path to SARIF file relative to the root of the repository
+          sarif_file: lintrunner.sarif
+          category: lintrunner
+          checkout_path: ${{ github.workspace }}
 
   lint-cpp:
     name: Lint C++
@@ -98,13 +83,6 @@ jobs:
             --cmake_extra_defines CMAKE_EXPORT_COMPILE_COMMANDS=ON
       - name: Generate ONNX protobuf files
         run: cmake --build build/Debug --config Debug --target onnx_proto
-#       - name: Run clang-tidy
-#         uses: ZedThree/clang-tidy-review@526cbfb043719639f1ebdeedae0cc1eacd219d8f
-#         with:
-#           token: ${{ secrets.github_token }}
-#           build_dir: "build/Debug"
-#           config_file: ".clang-tidy"
-#           lgtm_comment_body: ""
       - uses: reviewdog/action-cpplint@master
         with:
           github_token: ${{ secrets.github_token }}
@@ -117,7 +95,7 @@ jobs:
     name: Lint JavaScript
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - uses: reviewdog/action-eslint@v1
         with:
           reporter: github-pr-check

diff --git a/.lintrunner.toml b/.lintrunner.toml
@@ -0,0 +1,154 @@
+# Configuration for lintrunner https://github.com/suo/lintrunner
+# You can install the dependencies and initialize with
+#
+# ```sh
+# pip install lintrunner lintrunner-adapters
+# lintrunner init
+# ```
+#
+# This will install lintrunner on your system and download all the necessary
+# dependencies to run linters locally.
+# If you want to see what lintrunner init will install, run
+# `lintrunner init --dry-run`.
+#
+# To lint local changes:
+#
+# ```bash
+# lintrunner -m main
+# ```
+#
+# To lint all files:
+#
+# ```bash
+# lintrunner --all-files
+# ```
+#
+# To format files:
+#
+# ```bash
+# lintrunner f --all-files
+# ```
+#
+# To read more about lintrunner, see [wiki](https://github.com/pytorch/pytorch/wiki/lintrunner).
+# To update an existing linting rule or create a new one, modify this file or create a
+# new adapter following examples in https://github.com/justinchuby/lintrunner-adapters.
+
+[[linter]]
+code = 'RUFF'
+include_patterns = [
+    '**/*.py',
+    '**/*.pyi',
+]
+exclude_patterns = [
+    'cmake/external/**',
+    # ignore generated flatbuffers code
+    'onnxruntime/core/flatbuffers/ort_flatbuffers_py/**',
+]
+command = [
+    'python',
+    '-m',
+    'lintrunner_adapters',
+    'run',
+    'ruff_linter',
+    '--config=pyproject.toml',
+    '@{{PATHSFILE}}'
+]
+init_command = [
+    'python',
+    '-m',
+    'lintrunner_adapters',
+    'run',
+    'pip_init',
+    '--dry-run={{DRYRUN}}',
+    'ruff==0.0.252',
+]
+
+[[linter]]
+code = 'RUFF-FIX'
+include_patterns = [
+    '**/*.py',
+    '**/*.pyi',
+]
+exclude_patterns = [
+    'cmake/external/**',
+    # ignore generated flatbuffers code
+    'onnxruntime/core/flatbuffers/ort_flatbuffers_py/**',
+]
+command = [
+    'python',
+    '-m',
+    'lintrunner_adapters',
+    'run',
+    'ruff_fix_linter',
+    '--config=pyproject.toml',
+    '@{{PATHSFILE}}'
+]
+init_command = [
+    'python',
+    '-m',
+    'lintrunner_adapters',
+    'run',
+    'pip_init',
+    '--dry-run={{DRYRUN}}',
+    'ruff==0.0.252',
+]
+is_formatter = true
+
+
+[[linter]]
+code = 'BLACK-ISORT'
+include_patterns = [
+    '**/*.py',
+]
+exclude_patterns = [
+    'cmake/**',
+    'orttraining/*',
+    'onnxruntime/core/flatbuffers/**',
+]
+command = [
+    'python3',
+    '-m',
+    'lintrunner_adapters',
+    'run',
+    'black_isort_linter',
+    '--',
+    '@{{PATHSFILE}}'
+]
+init_command = [
+    'python3',
+    '-m',
+    'lintrunner_adapters',
+    'run',
+    'pip_init',
+    '--dry-run={{DRYRUN}}',
+    'black==23.1.0',
+    'isort==5.10.1',
+]
+is_formatter = true
+
+[[linter]]
+code = 'PYLINT'
+include_patterns = [
+    # TODO: Opt in to pylint by adding paths here
+]
+exclude_patterns = [
+]
+command = [
+    'python3',
+    '-m',
+    'lintrunner_adapters',
+    'run',
+    'pylint_linter',
+    '--rcfile=pyproject.toml',
+    '--',
+    '@{{PATHSFILE}}'
+]
+init_command = [
+    'python3',
+    '-m',
+    'lintrunner_adapters',
+    'run',
+    'pip_init',
+    '--dry-run={{DRYRUN}}',
+    'pylint==2.15.5',
+]
diff --git a/cgmanifests/generate_cgmanifest.py b/cgmanifests/generate_cgmanifest.py
@@ -50,7 +50,7 @@ def add_github_dep(name, parsed_url):
     if segments[3] != "archive":
         print("unrecognized github url path:" + parsed_url.path)
         return
-    git_repo_url = "https://github.com/%s/%s.git" % (org_name, repo_name)
+    git_repo_url = f"https://github.com/{org_name}/{repo_name}.git"
     # For example, the path might be like '/myorg/myrepo/archive/5a5f8a5935762397aa68429b5493084ff970f774.zip'
     # The last segment, segments[4], is '5a5f8a5935762397aa68429b5493084ff970f774.zip'
     if len(segments) == 5 and re.match(r"[0-9a-f]{40}", PurePosixPath(segments[4]).stem):
@@ -72,7 +72,7 @@ def add_github_dep(name, parsed_url):
             print("unrecognized github url path:" + parsed_url.path)
             return
         # Make a REST call to convert to tag to a git commit
-        url = "https://api.github.com/repos/%s/%s/git/refs/tags/%s" % (org_name, repo_name, tag)
+        url = f"https://api.github.com/repos/{org_name}/{repo_name}/git/refs/tags/{tag}"
         print("requesting %s ..." % url)
         res = requests.get(url, auth=(args.username, args.token))
         response_json = res.json()
@@ -92,7 +92,6 @@ def add_github_dep(name, parsed_url):
 
 with open(
     os.path.join(REPO_DIR, "tools", "ci_build", "github", "linux", "docker", "Dockerfile.manylinux2014_cuda11"),
-    mode="r",
 ) as f:
     for line in f:
         if not line.strip():
@@ -157,9 +156,8 @@ def normalize_path_separators(path):
     ],
     check=True,
     cwd=REPO_DIR,
-    stdout=subprocess.PIPE,
-    stderr=subprocess.PIPE,
-    universal_newlines=True,
+    capture_output=True,
+    text=True,
 )
 
 

diff --git a/cgmanifests/print_submodule_info.py b/cgmanifests/print_submodule_info.py
@@ -15,15 +15,13 @@
     check=True,
     cwd=path,
     stdout=subprocess.PIPE,
-    universal_newlines=True,
+    text=True,
 )
 
 url = proc.stdout.strip()
 
-proc = subprocess.run(
-    ["git", "rev-parse", "HEAD"], check=True, cwd=path, stdout=subprocess.PIPE, universal_newlines=True
-)
+proc = subprocess.run(["git", "rev-parse", "HEAD"], check=True, cwd=path, stdout=subprocess.PIPE, text=True)
 
 commit = proc.stdout.strip()
 
-print("{} {} {}".format(path, url, commit))
+print(f"{path} {url} {commit}")