Merge pull request #91597 from Repiteo/ci/pre-commit-handle-everything

CI: Overhaul static checks to use `pre-commit`

.github/workflows/static_checks.yml

@@ -19,28 +19,13 @@ jobs:
       - name: Install APT dependencies
         uses: awalsh128/cache-apt-pkgs-action@latest
         with:
-          packages: dos2unix libxml2-utils moreutils
+          packages: libxml2-utils
 
       - name: Install Python dependencies and general setup
         run: |
-          pip3 install pytest==7.1.2 mypy==0.971
+          pip3 install pytest==7.1.2
           git config diff.wsErrorHighlight all
 
-      - name: Get changed files
-        id: changed-files
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-        run: |
-          if [ "${{ github.event_name }}" == "pull_request" ]; then
-            files=$(git diff-tree --no-commit-id --name-only -r HEAD^1..HEAD 2> /dev/null || true)
-          elif [ "${{ github.event_name }}" == "push" -a "${{ github.event.forced }}" == "false" -a "${{ github.event.created }}" == "false" ]; then
-            files=$(git diff-tree --no-commit-id --name-only -r ${{ github.event.before }}..${{ github.event.after }} 2> /dev/null || true)
-          fi
-          echo "$files" >> changed.txt
-          cat changed.txt
-          files=$(echo "$files" | grep -v 'thirdparty' | xargs -I {} sh -c 'echo "./{}"' | tr '\n' ' ')
-          echo "CHANGED_FILES=$files" >> $GITHUB_ENV
-
       # This needs to happen before Python and npm execution; it must happen before any extra files are written.
       - name: .gitignore checks (gitignore_check.sh)
         run: |
@@ -49,51 +34,12 @@ jobs:
       - name: Style checks via pre-commit
         uses: pre-commit/action@v3.0.1
         with:
-          extra_args: --verbose --files ${{ env.CHANGED_FILES }}
-
-      - name: File formatting checks (file_format.sh)
-        run: |
-          bash ./misc/scripts/file_format.sh changed.txt
-
-      - name: Header guards formatting checks (header_guards.sh)
-        run: |
-          bash ./misc/scripts/header_guards.sh changed.txt
-
-      - name: Python scripts static analysis (mypy_check.sh)
-        run: |
-          if grep -qE '\.py$|SConstruct|SCsub' changed.txt || [ -z "$(cat changed.txt)" ]; then
-            bash ./misc/scripts/mypy_check.sh
-          else
-            echo "Skipping Python static analysis as no Python files were changed."
-          fi
-
-      - name: Python builders checks via pytest (pytest_builders.sh)
-        run: |
-          bash ./misc/scripts/pytest_builders.sh
+          extra_args: --verbose
 
-      - name: JavaScript style and documentation checks via ESLint and JSDoc
+      - name: Python builders checks via pytest
         run: |
-          if grep -q "\.js" changed.txt || [ -z "$(cat changed.txt)" ]; then
-            cd platform/web
-            npm ci
-            npm run lint
-            npm run docs -- -d dry-run
-          else
-            echo "Skipping JavaScript formatting as no Web/JS files were changed."
-          fi
+          pytest ./tests/python_build
 
       - name: Class reference schema checks
         run: |
           xmllint --noout --schema doc/class.xsd doc/classes/*.xml modules/*/doc_classes/*.xml platform/*/doc_classes/*.xml
-
-      - name: Documentation checks
-        run: |
-          doc/tools/doc_status.py doc/classes modules/*/doc_classes platform/*/doc_classes
-
-      - name: Spell checks via codespell
-        if: github.event_name == 'pull_request' && env.CHANGED_FILES != ''
-        uses: codespell-project/actions-codespell@v2
-        with:
-          skip: "./bin,./thirdparty,*.desktop,*.gen.*,*.po,*.pot,*.rc,./AUTHORS.md,./COPYRIGHT.txt,./DONORS.md,./core/input/gamecontrollerdb.txt,./core/string/locales.h,./editor/project_converter_3_to_4.cpp,./misc/scripts/codespell.sh,./platform/android/java/lib/src/com,./platform/web/node_modules,./platform/web/package-lock.json"
-          ignore_words_list: "breaked,colour,curvelinear,doubleclick,expct,findn,gird,hel,inout,lod,mis,nd,numer,ot,requestor,te,thirdparty,vai"
-          path: ${{ env.CHANGED_FILES }}

.pre-commit-config.yaml

@@ -1,3 +1,9 @@
+exclude: |
+  (?x)^(
+    .*thirdparty/.*|
+    .*-so_wrap\.(h|c)$
+  )
+
 repos:
   - repo: https://github.com/pre-commit/mirrors-clang-format
     rev: v17.0.6
@@ -7,10 +13,8 @@ repos:
         types_or: [text]
         exclude: |
           (?x)^(
-            tests/python_build.*|
-            .*thirdparty.*|
-            .*platform/android/java/lib/src/com.*|
-            .*-so_wrap.*
+            tests/python_build/.*|
+            platform/android/java/lib/src/com/.*
           )
 
   - repo: https://github.com/psf/black-pre-commit-mirror
@@ -19,33 +23,155 @@ repos:
       - id: black
         files: (\.py$|SConstruct|SCsub)
         types_or: [text]
-        exclude: .*thirdparty.*
         args:
           - --line-length=120
 
+  - repo: https://github.com/pre-commit/mirrors-mypy
+    rev: v0.971
+    hooks:
+      - id: mypy
+        files: \.py$
+        types_or: [text]
+        args: [--config-file=./misc/scripts/mypy.ini]
+
+  - repo: https://github.com/codespell-project/codespell
+    rev: v2.2.6
+    hooks:
+      - id: codespell
+        types_or: [text]
+        exclude: |
+          (?x)^(
+            .*\.desktop$|
+            .*\.gitignore$|
+            .*\.po$|
+            .*\.pot$|
+            .*\.rc$|
+            \.mailmap$|
+            AUTHORS.md$|
+            COPYRIGHT.txt$|
+            DONORS.md$|
+            core/input/gamecontrollerdb.txt$|
+            core/string/locales.h$|
+            editor/project_converter_3_to_4.cpp$|
+            platform/android/java/lib/src/com/.*|
+            platform/web/package-lock.json$
+          )
+        args:
+          - --enable-colors
+          - --write-changes
+          - --check-hidden
+          - --quiet-level
+          - '3'
+          - --ignore-words-list
+          - aesthetic,aesthetics,breaked,cancelled,colour,curvelinear,doubleclick,expct,findn,gird,hel,inout,lod,mis,nd,numer,ot,requestor,te,thirdparty,vai
+          - --builtin
+          - clear,rare,en-GB_to_en-US
+
+  ### Requires Docker; look into alternative implementation.
+  # - repo: https://github.com/comkieffer/pre-commit-xmllint.git
+  #   rev: 1.0.0
+  #   hooks:
+  #     - id: xmllint
+  #       language: docker
+  #       types_or: [text]
+  #       files: ^(doc/classes|.*/doc_classes)/.*\.xml$
+  #       args: [--schema, doc/class.xsd]
+
+  - repo: https://github.com/pre-commit/mirrors-eslint
+    rev: v8.46.0
+    hooks:
+      - id: eslint
+        name: eslint-engine
+        files: ^(platform/web/js/engine/|js/jsdoc2rst/).*\.js$
+        args: [--fix, --no-eslintrc, --config, platform/web/.eslintrc.engine.js]
+        additional_dependencies: &eslint-deps
+          - eslint@8.46.0
+          - eslint-config-airbnb-base@15.0.0
+          - eslint-plugin-import@2.28.0
+          - eslint-plugin-html@7.1.0
+          - '@html-eslint/eslint-plugin@0.19.1'
+          - '@html-eslint/parser@0.19.1'
+      - id: eslint
+        name: eslint-libs
+        files: ^(platform/web/js/libs/|modules/).*\.js$
+        args: [--fix, --no-eslintrc, --config, platform/web/.eslintrc.libs.js]
+        additional_dependencies: *eslint-deps
+      - id: eslint
+        name: eslint-sw
+        files: ^misc/dist/html/service-worker\.js$
+        args: [--fix, --no-eslintrc, --config, platform/web/.eslintrc.sw.js]
+        additional_dependencies: *eslint-deps
+      - id: eslint
+        name: eslint-html
+        files: ^misc/dist/html/.*\.html$
+        types: [html]
+        args: [--fix, --no-eslintrc, --config, platform/web/.eslintrc.html.js]
+        additional_dependencies: *eslint-deps
+
   - repo: local
     hooks:
       - id: make-rst
         name: make-rst
+        language: python
         entry: python3 doc/tools/make_rst.py doc/classes modules platform --dry-run --color
         pass_filenames: false
+        files: ^(doc/classes|.*/doc_classes)/.*\.xml$
+
+      - id: doc-status
+        name: doc-status
         language: python
-        files: ^(doc|modules|platform).*xml$
+        entry: python3 doc/tools/doc_status.py
+        files: ^(doc/classes|.*/doc_classes)/.*\.xml$
+
+      - id: jsdoc
+        name: jsdoc
+        language: node
+        entry: jsdoc
+        files: ^platform/web/js/engine/(engine|config|features)\.js$
+        args: [--template, platform/web/js/jsdoc2rst/, --destination, '', -d, dry-run]
+        additional_dependencies: ["jsdoc@4.0.2"]
 
       - id: copyright-headers
         name: copyright-headers
         language: python
-        files: \.(c|h|cpp|hpp|cc|cxx|m|mm|inc|java)$
         entry: python3 misc/scripts/copyright_headers.py
+        files: \.(c|h|cpp|hpp|cc|cxx|m|mm|inc|java)$
         exclude: |
           (?x)^(
-            .*thirdparty.*|
-            .*-so_wrap.*|
             core/math/bvh_.*\.inc$|
-            platform/android/java/lib/src/com.*|
-            platform/android/java/lib/src/org/godotengine/godot/gl/GLSurfaceView.*|
-            platform/android/java/lib/src/org/godotengine/godot/gl/EGLLogWrapper.*|
-            platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.*
+            platform/android/java/lib/src/com/.*|
+            platform/android/java/lib/src/org/godotengine/godot/gl/GLSurfaceView\.java$|
+            platform/android/java/lib/src/org/godotengine/godot/gl/EGLLogWrapper\.java$|
+            platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix\.java$
+          )
+
+      - id: header-guards
+        name: header-guards
+        language: python
+        entry: python3 misc/scripts/header_guards.py
+        files: \.(h|hpp)$
+        exclude: |
+          (?x)^(
+            .*/thread\.h$|
+            .*/platform_config\.h$|
+            .*/platform_gl\.$h
+          )
+
+      - id: file-format
+        name: file-format
+        language: python
+        entry: python3 misc/scripts/file_format.py
+        types_or: [text]
+        exclude: |
+          (?x)^(
+            .*\.test\.txt$|
+            .*\.svg$|
+            .*\.patch$|
+            .*\.out$|
+            modules/gdscript/tests/scripts/parser/features/mixed_indentation_on_blank_lines\.gd$|
+            modules/gdscript/tests/scripts/parser/warnings/empty_file_newline_comment\.notest\.gd$|
+            modules/gdscript/tests/scripts/parser/warnings/empty_file_newline\.notest\.gd$|
+            platform/android/java/lib/src/com/google/.*
           )
 
       - id: dotnet-format

misc/scripts/dotnet_format.py

@@ -5,10 +5,16 @@
 import os
 import sys
 
-# Create dummy generated files.
+if len(sys.argv) < 2:
+    print("Invalid usage of dotnet_format.py, it should be called with a path to one or multiple files.")
+    sys.exit(1)
+
+# Create dummy generated files, if needed.
 for path in [
     "modules/mono/SdkPackageVersions.props",
 ]:
+    if os.path.exists(path):
+        continue
     os.makedirs(os.path.dirname(path), exist_ok=True)
     with open(path, "w", encoding="utf-8", newline="\n") as f:
         f.write("<Project />")
@@ -17,14 +23,12 @@
 os.environ["GodotSkipGenerated"] = "true"
 
 # Match all the input files to their respective C# project.
-input_files = [os.path.normpath(x) for x in sys.argv]
 projects = {
-    path: [f for f in sys.argv if os.path.commonpath([f, path]) == path]
+    path: " ".join([f for f in sys.argv[1:] if os.path.commonpath([f, path]) == path])
     for path in [os.path.dirname(f) for f in glob.glob("**/*.csproj", recursive=True)]
 }
 
 # Run dotnet format on all projects with more than 0 modified files.
 for path, files in projects.items():
-    if len(files) > 0:
-        command = f"dotnet format {path} --include {' '.join(files)}"
-        os.system(command)
+    if files:
+        os.system(f"dotnet format {path} --include {files}")

misc/scripts/file_format.py

@@ -0,0 +1,51 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+import sys
+
+if len(sys.argv) < 2:
+    print("Invalid usage of file_format.py, it should be called with a path to one or multiple files.")
+    sys.exit(1)
+
+BOM = b"\xef\xbb\xbf"
+
+changed = []
+invalid = []
+
+for file in sys.argv[1:]:
+    try:
+        with open(file, "rt", encoding="utf-8") as f:
+            original = f.read()
+    except UnicodeDecodeError:
+        invalid.append(file)
+        continue
+
+    if original == "":
+        continue
+
+    EOL = "\r\n" if file.endswith((".csproj", ".sln", ".bat")) or file.startswith("misc/msvs") else "\n"
+    WANTS_BOM = file.endswith((".csproj", ".sln"))
+
+    revamp = EOL.join([line.rstrip("\n\r\t ") for line in original.splitlines(True)]).rstrip(EOL) + EOL
+
+    new_raw = revamp.encode(encoding="utf-8")
+    if not WANTS_BOM and new_raw.startswith(BOM):
+        new_raw = new_raw[len(BOM) :]
+    elif WANTS_BOM and not new_raw.startswith(BOM):
+        new_raw = BOM + new_raw
+
+    with open(file, "rb") as f:
+        old_raw = f.read()
+
+    if old_raw != new_raw:
+        changed.append(file)
+        with open(file, "wb") as f:
+            f.write(new_raw)
+
+if changed:
+    for file in changed:
+        print(f"FIXED: {file}")
+if invalid:
+    for file in invalid:
+        print(f"REQUIRES MANUAL CHANGES: {file}")
+    sys.exit(1)