Skip to content

Commit

Permalink
CI: Overhaul static checks to use pre-commit
Browse files Browse the repository at this point in the history
  • Loading branch information
Repiteo committed May 8, 2024
1 parent 55b8724 commit df969ff
Show file tree
Hide file tree
Showing 10 changed files with 355 additions and 273 deletions.
64 changes: 5 additions & 59 deletions .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand All @@ -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 }}
152 changes: 139 additions & 13 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
8 changes: 0 additions & 8 deletions misc/scripts/codespell.sh

This file was deleted.

16 changes: 10 additions & 6 deletions misc/scripts/dotnet_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 />")
Expand All @@ -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}")
51 changes: 51 additions & 0 deletions misc/scripts/file_format.py
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit df969ff

Please sign in to comment.