Clean github checks

This commit is contained in:
Adrien Ufferte
2026-04-17 09:35:06 +02:00
parent 9413c1fd89
commit e169f066ac
2 changed files with 244 additions and 86 deletions
+243 -85
View File
@@ -1,6 +1,7 @@
import os
import re
import sys
from collections import defaultdict, Counter
def split_params(param_string: str):
@@ -46,7 +47,7 @@ def split_params(param_string: str):
current = []
else:
current.append(char)
if stack:
raise ValueError("unbalance in delimiter")
if in_quote:
@@ -56,58 +57,128 @@ def split_params(param_string: str):
params = [p for p in params if p]
return params
EXCLUDED_DIRS = {"addons", ".git", ".github"}
EXCLUDED_FILES = {os.path.normpath("script_templates/Node/default.gd")}
issues = []
issues: list = []
# ─── Messages ────────────────────────────────────────────────────────────────
# Structural issue messages. Use {key} for context-specific values.
# Naming issues are rendered dynamically (see format_message / SUGGESTION_FN).
MESSAGES = {
'class': 'is a class name and should be in PascalCase',
'function': 'is a function name and should be in snake_case',
'variable': 'is a variable name and should be in snake_case',
'constant': 'is a constant name and should be in UPPER_SNAKE_CASE',
'signal': 'is a signal name and should be in snake_case',
'annotation_order': 'annotations must be on the first line and include only \@tool, \@icon or \@static_unload',
'class_position': 'class_name must appear after annotations (if any)',
'extends_position': 'extends must appear after annotation and class_name',
'extends_missing': 'extends is required and must follow annotation and class_name',
'func_blank': 'functions must be preceded by exactly two empty lines (or only one if preceded by a #region comment)',
'signal_position': 'signals must come right after extends',
'signal_format': 'signals must end with ()',
'signal_blank': 'signals must not be separated by empty lines',
'enum_position': 'enums must come after signals and be grouped together',
'enum_format': "enum declaration should be 'enum Name {'",
'enum_member_blank': 'enum members must not be separated by empty lines',
'enum_member_indent': 'enum members must be indented with a single tab',
'enum_no_close': 'enum declaration is missing a closing }',
'enum_blank': 'enums must be preceded by one empty line and must not be separated by empty lines',
'const_position': 'constants must come after enums and be grouped together',
'const_blank': 'constants must be preceded by one empty line and must not be separated by empty lines',
'static_position': 'static variables must come after constants and be grouped together',
'static_blank': 'static variables must be preceded by one empty line and must not be separated by empty lines',
'export_position': 'export variables must come after static variables and be grouped together',
'export_blank': 'export variables must be preceded by one empty line and must not be separated by empty lines',
'var_position': 'variables must come after export variables and be grouped together',
'var_blank': 'variables must be preceded by one empty line and must not be separated by empty lines',
'onready_position': '@onready variables must come after other variables and be grouped together',
'onready_blank': '@onready variables must be preceded by one empty line and must not be separated by empty lines',
# File/header structure
'annotation_order': "annotation not allowed here: '{found}' — only @tool, @icon or @static_unload are allowed at the top of the file",
'class_position': "class_name must appear right after annotations (if any)",
'extends_position': "extends must appear after annotations and class_name",
'extends_missing': "extends is missing — add 'extends BaseClass' (or 'extends RefCounted' for base classes)",
# Function spacing
'func_blank': "expected 2 blank lines before function (found {found})",
# Signals
'signal_position': "signal found after {after} — expected order: signal > enum > const > static var > @export > var > @onready",
'signal_format': "signal must declare parameters with () — found: '{found}'",
'signal_blank_extra': "remove blank line between signal declarations — signals must be grouped together",
# Enums
'enum_position': "enum found after {after} — expected order: signal > enum > const > static var > @export > var > @onready",
'enum_format': "enum declaration should be 'enum Name {{' — found: '{found}'",
'enum_member_blank': "remove blank line between enum members",
'enum_member_indent': "enum member must be indented with a single tab — found: '{found}'",
'enum_no_close': "enum declaration is missing a closing '}'",
'enum_blank_missing': "missing blank line before the enum section",
'enum_blank_extra': "remove blank line between enum declarations — enums must be grouped together",
# Constants
'const_position': "const found after {after} — expected order: signal > enum > const > static var > @export > var > @onready",
'const_blank_missing': "missing blank line before the const section",
'const_blank_extra': "remove blank line between const declarations — constants must be grouped together",
# Static variables
'static_position': "static var found after {after} — expected order: signal > enum > const > static var > @export > var > @onready",
'static_blank_missing': "missing blank line before the static var section",
'static_blank_extra': "remove blank line between static var declarations — static variables must be grouped together",
# Export variables
'export_position': "@export found after {after} — expected order: signal > enum > const > static var > @export > var > @onready",
'export_blank_missing': "missing blank line before the @export section",
'export_blank_extra': "remove blank line between @export declarations — export variables must be grouped together",
# Regular variables
'var_position': "var found after {after} — expected order: signal > enum > const > static var > @export > var > @onready",
'var_blank_missing': "missing blank line before the var section",
'var_blank_extra': "remove blank line between var declarations — variables must be grouped together",
# @onready variables
'onready_position': "@onready var found after {after} — expected order: signal > enum > const > static var > @export > var > @onready",
'onready_blank_missing': "missing blank line before the @onready section",
'onready_blank_extra': "remove blank line between @onready declarations — @onready variables must be grouped together",
}
# Naming conventions
# ─── Naming conventions ───────────────────────────────────────────────────────
PASCAL_CASE = re.compile(r"^[A-Z][A-Za-z0-9]*$")
SNAKE_CASE = re.compile(r"^_?[a-z][a-z0-9_]*$")
UPPER_SNAKE_CASE = re.compile(r"^_?[A-Z][A-Z0-9_]*$")
REGION_RE = re.compile(r"#\s*(region|endregion)\b", re.IGNORECASE)
def _to_snake_case(name: str) -> str:
prefix = '_' if name.startswith('_') else ''
core = name.lstrip('_')
s = re.sub(r'([a-z0-9])([A-Z])', r'\1_\2', core)
s = re.sub(r'([A-Z]+)([A-Z][a-z])', r'\1_\2', s)
return prefix + s.lower()
def _to_pascal_case(name: str) -> str:
prefix = '_' if name.startswith('_') else ''
core = name.lstrip('_')
return prefix + ''.join(w.capitalize() for w in re.split(r'[_\s]+', core) if w)
def _to_upper_snake_case(name: str) -> str:
prefix = '_' if name.startswith('_') else ''
core = name.lstrip('_')
s = re.sub(r'([a-z0-9])([A-Z])', r'\1_\2', core)
s = re.sub(r'([A-Z]+)([A-Z][a-z])', r'\1_\2', s)
return prefix + s.upper()
CONVENTION_NAMES: dict[str, str] = {
'class': 'PascalCase',
'function': 'snake_case',
'variable': 'snake_case',
'constant': 'UPPER_SNAKE_CASE',
'signal': 'snake_case',
}
SUGGESTION_FN: dict = {
'class': _to_pascal_case,
'function': _to_snake_case,
'variable': _to_snake_case,
'constant': _to_upper_snake_case,
'signal': _to_snake_case,
}
NAMING_KINDS = frozenset({'class', 'function', 'variable', 'constant', 'signal'})
# ─── Naming check ─────────────────────────────────────────────────────────────
def check_naming(path: str, lines: list[str]):
for idx, line in enumerate(lines, 1):
stripped = line.strip()
if stripped.startswith('#') or stripped.startswith('@warning_ignore(') or not stripped:
continue
match_class = re.match(r"class_name\s+([A-Za-z0-9_]+)", stripped)
if match_class:
name = match_class.group(1)
if not PASCAL_CASE.match(name):
issues.append((path, idx, 'class', name))
match_func = re.match(r"(?:static\s+)?func\s+([A-Za-z0-9_]+)\s*(\([^)]*\))?", stripped)
if match_func:
name = match_func.group(1)
@@ -120,51 +191,62 @@ def check_naming(path: str, lines: list[str]):
param_name = param.split(':')[0].split('=')[0].strip()
if param_name and not SNAKE_CASE.match(param_name):
issues.append((path, idx, 'variable', param_name))
match_export = re.match(r"(?:@export\s+)?var\s+([A-Za-z0-9_]+)", stripped)
if match_export:
name = match_export.group(1)
# Match variable declarations with any annotation/modifier prefix.
# Handles: var, static var, @export var, @export_range(...) var,
# @export_multiline var, @onready var, etc.
# Does NOT match annotation-only lines like @export_category("Difficulty").
match_var = re.match(r"(?:(?:static|@\w+(?:\([^)]*\))?)\s+)*var\s+([A-Za-z0-9_]+)", stripped)
if match_var:
name = match_var.group(1)
if not SNAKE_CASE.match(name):
issues.append((path, idx, 'variable', name))
match_const = re.match(r"const\s+([A-Za-z0-9_]+)", stripped)
if match_const:
name = match_const.group(1)
if not UPPER_SNAKE_CASE.match(name):
issues.append((path, idx, 'constant', name))
match_signal = re.match(r"signal\s+([A-Za-z0-9_]+)", stripped)
if match_signal:
name = match_signal.group(1)
if not SNAKE_CASE.match(name):
issues.append((path, idx, 'signal', name))
match_for = re.match(r"for\s+([A-Za-z0-9_]+)(?:\s*:\s*[^\s]+)?\s+in\b", stripped)
if match_for:
name = match_for.group(1)
if not SNAKE_CASE.match(name):
issues.append((path, idx, 'variable', name))
# Content order
# ─── Content order check ──────────────────────────────────────────────────────
ANNOTATION_LINE_RE = re.compile(
r"^(?:@(tool|icon|static_unload)(?:\([^\n]*\))?)(?:,\s*@(tool|icon|static_unload)(?:\([^\n]*\))?)*$"
)
ALLOWED_ANNOTATION_RE = re.compile(r"@(tool|icon|static_unload)\b")
def check_content_order(path: str, lines: list[str]):
content = [
(line.rstrip('\n'), idx)
for idx, line in enumerate(lines, 1)
if not line.startswith(' ') # Line is not indented
if not line.startswith(' ') # Line is not indented (tab character)
and not line.lstrip().startswith('#')
and not line.lstrip().startswith('@warning_ignore(')
]
idx = 0
n = len(content)
# 1) annotations
# 1) Annotations (@tool / @icon / @static_unload must be first)
if idx < n and ANNOTATION_LINE_RE.fullmatch(content[idx][0].strip()):
idx += 1
else:
for j in range(idx, n):
if ALLOWED_ANNOTATION_RE.search(content[j][0]):
issues.append((path, content[j][1], 'annotation_order', content[j][0].strip()))
issues.append((path, content[j][1], 'annotation_order', {'found': content[j][0].strip()}))
break
# 2) class_name (optional)
@@ -176,24 +258,27 @@ def check_content_order(path: str, lines: list[str]):
idx += 1
break
# 3) extends (required)
# 3) extends (recommended; missing is flagged but does not block ordering checks)
extends_pos = None
for j in range(idx, n):
if content[j][0].strip().startswith('extends'):
extends_pos = j
break
if extends_pos is None:
issues.append((path, 0, 'extends_missing', 'extends'))
return
if extends_pos != idx:
issues.append((path, content[extends_pos][1], 'extends_position', 'extends'))
idx = extends_pos + 1
# Don't return — continue ordering checks from current position
prev_token: str = content[idx - 1][0].strip() if idx > 0 else ''
else:
if extends_pos != idx:
issues.append((path, content[extends_pos][1], 'extends_position', 'extends'))
idx = extends_pos + 1
prev_token = content[extends_pos][0].strip()
order = ['signal', 'enum', 'const', 'static var', '@export', 'var', '@onready var']
order_index = {name: i for i, name in enumerate(order)}
seen: set[str] = set()
current_order = -1
prev_token: str | None = content[extends_pos][0].strip()
j = idx
while j < n:
line, line_no = content[j]
@@ -208,27 +293,27 @@ def check_content_order(path: str, lines: list[str]):
if stripped.startswith('signal'):
token = 'signal'
if not re.fullmatch(r"signal\s+\w+\([^)]*\)", stripped):
issues.append((path, line_no, 'signal_format', stripped))
issues.append((path, line_no, 'signal_format', {'found': stripped}))
if 'signal' in seen and prev_token == '':
issues.append((path, content[j - 1][1], 'signal_blank', 'signal'))
issues.append((path, content[j - 1][1], 'signal_blank_extra', 'signal'))
seen.add('signal')
elif stripped.startswith('enum'):
token = 'enum'
if not re.fullmatch(r"enum\s+\w+\s*{", stripped):
issues.append((path, line_no, 'enum_format', stripped))
issues.append((path, line_no, 'enum_format', {'found': stripped}))
if 'enum' not in seen:
if prev_token != '':
issues.append((path, line_no, 'enum_blank', 'enum'))
issues.append((path, line_no, 'enum_blank_missing', 'enum'))
else:
if prev_token != '}' and prev_token != ']':
issues.append((path, line_no, 'enum_blank', 'enum'))
issues.append((path, line_no, 'enum_blank_extra', 'enum'))
k = j + 1
while k < n and content[k][0].strip() != '}':
member, m_line_no = content[k]
if member.strip() == '':
issues.append((path, m_line_no, 'enum_member_blank', 'enum'))
if not member.startswith('\t') or member.startswith('\t\t'):
issues.append((path, m_line_no, 'enum_member_indent', member.strip()))
issues.append((path, m_line_no, 'enum_member_indent', {'found': member.strip()}))
k += 1
if k >= n:
issues.append((path, line_no, 'enum_no_close', 'enum'))
@@ -238,7 +323,8 @@ def check_content_order(path: str, lines: list[str]):
seen.add('enum')
curr_order = order_index['enum']
if curr_order < current_order:
issues.append((path, line_no, 'enum_position', 'enum'))
after_token = order[current_order]
issues.append((path, line_no, 'enum_position', {'after': after_token}))
else:
current_order = max(current_order, curr_order)
j += 1
@@ -247,63 +333,65 @@ def check_content_order(path: str, lines: list[str]):
token = 'const'
if 'const' not in seen:
if prev_token != '':
issues.append((path, line_no, 'const_blank', 'const'))
issues.append((path, line_no, 'const_blank_missing', 'const'))
else:
if prev_token != 'const' and prev_token != '}' and prev_token != ']':
issues.append((path, line_no, 'const_blank', 'const'))
issues.append((path, line_no, 'const_blank_extra', 'const'))
seen.add('const')
elif stripped.startswith('static var '):
token = 'static var'
if 'static var' not in seen:
if prev_token != '':
issues.append((path, line_no, 'static_blank', 'static var'))
issues.append((path, line_no, 'static_blank_missing', 'static var'))
else:
if prev_token != 'static var' and prev_token != '}' and prev_token != ']':
issues.append((path, line_no, 'static_blank', 'static var'))
issues.append((path, line_no, 'static_blank_extra', 'static var'))
seen.add('static var')
elif stripped.startswith('@export'):
token = '@export'
if '@export' not in seen:
if prev_token != '':
issues.append((path, line_no, 'export_blank', '@export'))
issues.append((path, line_no, 'export_blank_missing', '@export'))
else:
if prev_token != '@export' and prev_token != '}' and prev_token != ']':
issues.append((path, line_no, 'export_blank', '@export'))
issues.append((path, line_no, 'export_blank_extra', '@export'))
seen.add('@export')
elif stripped.startswith('var '):
token = 'var'
if 'var' not in seen:
if prev_token != '':
issues.append((path, line_no, 'var_blank', 'var'))
issues.append((path, line_no, 'var_blank_missing', 'var'))
else:
if prev_token != 'var' and prev_token != '}' and prev_token != ']':
issues.append((path, line_no, 'var_blank', 'var'))
issues.append((path, line_no, 'var_blank_extra', 'var'))
seen.add('var')
elif stripped.startswith('@onready var '):
token = '@onready var'
if '@onready var' not in seen:
if prev_token != '':
issues.append((path, line_no, 'onready_blank', '@onready var'))
issues.append((path, line_no, 'onready_blank_missing', '@onready var'))
else:
if prev_token != '@onready var' and prev_token != '}' and prev_token != ']':
issues.append((path, line_no, 'onready_blank', '@onready var'))
issues.append((path, line_no, 'onready_blank_extra', '@onready var'))
seen.add('@onready var')
else:
prev_token = stripped
j += 1
continue
curr_order = order_index[token]
key_map = {
'signal': 'signal_position',
'enum': 'enum_position',
'const': 'const_position',
'static var': 'static_position',
'@export': 'export_position',
'var': 'var_position',
'signal': 'signal_position',
'enum': 'enum_position',
'const': 'const_position',
'static var': 'static_position',
'@export': 'export_position',
'var': 'var_position',
'@onready var': 'onready_position',
}
if curr_order < current_order:
issues.append((path, line_no, key_map[token], token))
after_token = order[current_order]
issues.append((path, line_no, key_map[token], {'after': after_token}))
else:
current_order = max(current_order, curr_order)
@@ -311,7 +399,8 @@ def check_content_order(path: str, lines: list[str]):
j += 1
# Spacing for functions
# ─── Function spacing check ───────────────────────────────────────────────────
def check_func_spacing(path: str):
with open(path, 'r', encoding='utf-8') as file:
lines = file.readlines()
@@ -348,14 +437,15 @@ def check_func_spacing(path: str):
region_above = test_index >= 0 and REGION_RE.match(lines[test_index].lstrip())
if region_above:
if blank_count > 1:
issues.append((path, idx + 1, 'func_blank', 'func'))
issues.append((path, idx + 1, 'func_blank', {'found': blank_count}))
continue
elif blank_count != 2:
if not (blank_count == 1 and test_index >= 0 and lines[test_index].lstrip().startswith('#region')):
issues.append((path, idx + 1, 'func_blank', 'func'))
issues.append((path, idx + 1, 'func_blank', {'found': blank_count}))
# Main function
# ─── Main ─────────────────────────────────────────────────────────────────────
for root, dirs, files in os.walk('.', topdown=True):
rel_root = os.path.relpath(root, '.')
if any(rel_root == excluded or rel_root.startswith(f"{excluded}{os.sep}") for excluded in EXCLUDED_DIRS):
@@ -378,15 +468,83 @@ for root, dirs, files in os.walk('.', topdown=True):
check_content_order(path, lines)
check_func_spacing(path)
if issues:
print("### \u274c GDScript Naming Convention Check Failed\n")
print(f"The project must follow Godot GDScript naming conventions.\nTotal issues: {len(issues)}\n")
for path, idx, kind, name in issues:
if kind == 'error':
message = name
else:
message = f"'{name}' {MESSAGES[kind]}"
print(f"- `{path}:{idx}` {message}")
sys.exit(1)
else:
print("\u2705 All GDScript files follow the naming conventions.")
# ─── Output ───────────────────────────────────────────────────────────────────
ORDERING_KINDS = frozenset({
'annotation_order', 'class_position', 'extends_position', 'extends_missing',
'signal_position', 'enum_position', 'const_position', 'static_position',
'export_position', 'var_position', 'onready_position',
})
FORMATTING_KINDS = frozenset({
'func_blank',
'signal_format', 'signal_blank_extra',
'enum_format', 'enum_blank_missing', 'enum_blank_extra',
'enum_member_blank', 'enum_member_indent', 'enum_no_close',
'const_blank_missing', 'const_blank_extra',
'static_blank_missing', 'static_blank_extra',
'export_blank_missing', 'export_blank_extra',
'var_blank_missing', 'var_blank_extra',
'onready_blank_missing', 'onready_blank_extra',
})
def categorize(kind: str) -> str:
if kind in NAMING_KINDS:
return 'naming'
if kind in ORDERING_KINDS:
return 'ordering'
if kind in FORMATTING_KINDS:
return 'formatting'
return 'other'
def format_message(kind: str, data) -> str:
if kind in NAMING_KINDS:
suggestion = SUGGESTION_FN[kind](data)
conv = CONVENTION_NAMES[kind]
return f"'{data}' should be '{suggestion}' ({kind}s must be {conv})"
if kind == 'error':
return str(data)
template = MESSAGES.get(kind, kind)
if isinstance(data, dict):
return template.format(**data)
return template
if not issues:
print("✅ All GDScript files follow the naming conventions.")
sys.exit(0)
# Group issues by file, sort by line number within each file
by_file: dict = defaultdict(list)
counts: Counter = Counter()
for path, line_no, kind, data in issues:
by_file[path].append((line_no, kind, data))
counts[categorize(kind)] += 1
total = len(issues)
file_count = len(by_file)
summary_parts = []
for cat in ('naming', 'ordering', 'formatting', 'other'):
if counts[cat]:
summary_parts.append(f"{counts[cat]} {cat}")
summary = ', '.join(summary_parts)
print(f"### ❌ GDScript Naming Convention Check Failed\n")
print(f"**{total} issue{'s' if total != 1 else ''}** in {file_count} file{'s' if file_count != 1 else ''} ({summary})\n")
for fpath in sorted(by_file):
file_issues = sorted(by_file[fpath], key=lambda x: x[0])
count = len(file_issues)
print("<details>")
print(f"<summary><code>{fpath}</code> — {count} issue{'s' if count != 1 else ''}</summary>\n")
print("| Line | Issue |")
print("|------|-------|")
for line_no, kind, data in file_issues:
msg = format_message(kind, data)
print(f"| {line_no} | {msg} |")
print("\n</details>\n")
sys.exit(1)
+1 -1
View File
@@ -10,7 +10,7 @@ jobs:
permissions:
pull-requests: write
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Run GDScript naming convention check
id: naming
run: |