A few improvements to validation.
This commit is contained in:
Storm Dragon
@ -125,6 +125,43 @@ else
|
||||
VALIDATION_FAILED=1
|
||||
fi
|
||||
|
||||
# 2a2. PEP8/flake8 Validation (for staged Python files only)
|
||||
echo -e "\n${YELLOW} 2a2. Checking PEP8 compliance...${NC}"
|
||||
if command -v flake8 >/dev/null 2>&1 && [ -n "$STAGED_PYTHON_FILES" ]; then
|
||||
PEP8_ISSUES=0
|
||||
|
||||
# Check staged Python files with flake8
|
||||
# Focus on critical issues, ignore cosmetic ones for pre-commit
|
||||
FLAKE8_SELECT="E9,F63,F7,F82" # Critical syntax/import errors only
|
||||
FLAKE8_IGNORE="E501,W503,E203" # Ignore line length and some formatting
|
||||
|
||||
for file in $STAGED_PYTHON_FILES; do
|
||||
if [ -f "$file" ]; then
|
||||
flake8_output=$(flake8 --select="$FLAKE8_SELECT" --ignore="$FLAKE8_IGNORE" "$file" 2>/dev/null || true)
|
||||
if [ -n "$flake8_output" ]; then
|
||||
if [ $PEP8_ISSUES -eq 0 ]; then
|
||||
echo -e "${RED} ✗ Critical PEP8 issues found:${NC}"
|
||||
fi
|
||||
echo -e "${RED} $file:${NC}"
|
||||
echo "$flake8_output" | sed 's/^/ /'
|
||||
PEP8_ISSUES=1
|
||||
fi
|
||||
fi
|
||||
done
|
||||
|
||||
if [ $PEP8_ISSUES -eq 0 ]; then
|
||||
echo -e "${GREEN} ✓ No critical PEP8 issues in staged files${NC}"
|
||||
else
|
||||
echo -e "${RED} ✗ Critical PEP8 issues found${NC}"
|
||||
echo -e "${YELLOW} Run: flake8 --select=E9,F63,F7,F82 <file> for details${NC}"
|
||||
VALIDATION_FAILED=1
|
||||
fi
|
||||
elif [ -n "$STAGED_PYTHON_FILES" ]; then
|
||||
echo -e "${YELLOW} ⚠ flake8 not available (install with: pip install flake8)${NC}"
|
||||
else
|
||||
echo -e "${GREEN} ✓ No Python files to check${NC}"
|
||||
fi
|
||||
|
||||
# 2b. Check for common issues in modified files
|
||||
echo -e "\n${YELLOW} 2b. Checking modified files for common issues...${NC}"
|
||||
|
||||
|
||||
@ -24,6 +24,7 @@ fi
|
||||
# Make validation scripts executable
|
||||
echo -e "\n${YELLOW}1. Making validation scripts executable...${NC}"
|
||||
chmod +x tools/validate_syntax.py
|
||||
chmod +x tools/validate_pep8.py
|
||||
chmod +x tools/validate_release.py
|
||||
chmod +x tools/cleanup_cache.py
|
||||
chmod +x tools/pre-commit-hook
|
||||
|
||||
365
tools/validate_pep8.py
Executable file
365
tools/validate_pep8.py
Executable file
@ -0,0 +1,365 @@
|
||||
#!/usr/bin/env python3
|
||||
"""
|
||||
Fenrir PEP8 Validation and Auto-Fix Tool
|
||||
|
||||
Validates Python code style using flake8 and applies safe automatic fixes.
|
||||
Designed to work with Fenrir's existing codebase while respecting timing-critical code.
|
||||
|
||||
Usage:
|
||||
python3 tools/validate_pep8.py # Check all Python files
|
||||
python3 tools/validate_pep8.py --fix-safe # Auto-fix safe issues
|
||||
python3 tools/validate_pep8.py --check-only # Exit with error if issues found
|
||||
python3 tools/validate_pep8.py --staged # Check only staged files
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import argparse
|
||||
import subprocess
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
class PEP8Validator:
|
||||
def __init__(self, verbose=True):
|
||||
self.verbose = verbose
|
||||
self.errors = []
|
||||
self.warnings = []
|
||||
self.fixes_applied = []
|
||||
|
||||
def log(self, message, level="INFO"):
|
||||
"""Log a message with appropriate formatting."""
|
||||
if not self.verbose and level == "INFO":
|
||||
return
|
||||
|
||||
colors = {
|
||||
"INFO": "\033[0;36m", # Cyan
|
||||
"SUCCESS": "\033[0;32m", # Green
|
||||
"WARNING": "\033[1;33m", # Yellow
|
||||
"ERROR": "\033[0;31m", # Red
|
||||
"HEADER": "\033[1;34m", # Bold Blue
|
||||
}
|
||||
|
||||
reset = "\033[0m"
|
||||
color = colors.get(level, "")
|
||||
|
||||
if level == "HEADER":
|
||||
print(f"\n{color}{'='*60}")
|
||||
print(f"{message}")
|
||||
print(f"{'='*60}{reset}")
|
||||
else:
|
||||
symbol = {
|
||||
"SUCCESS": "✓",
|
||||
"ERROR": "✗",
|
||||
"WARNING": "⚠",
|
||||
"INFO": "•"
|
||||
}.get(level, "•")
|
||||
|
||||
print(f"{color}{symbol} {message}{reset}")
|
||||
|
||||
def check_flake8_available(self):
|
||||
"""Check if flake8 is available."""
|
||||
try:
|
||||
result = subprocess.run(["flake8", "--version"],
|
||||
capture_output=True, text=True, timeout=5)
|
||||
if result.returncode == 0:
|
||||
version = result.stdout.strip().split('\n')[0]
|
||||
self.log(f"Using flake8: {version}")
|
||||
return True
|
||||
else:
|
||||
return False
|
||||
except (subprocess.TimeoutExpired, FileNotFoundError):
|
||||
return False
|
||||
|
||||
def get_python_files(self, directory=None, staged_only=False):
|
||||
"""Get list of Python files to check."""
|
||||
if staged_only:
|
||||
try:
|
||||
result = subprocess.run([
|
||||
"git", "diff", "--cached", "--name-only", "--diff-filter=ACM"
|
||||
], capture_output=True, text=True, timeout=10)
|
||||
|
||||
if result.returncode == 0:
|
||||
files = [f for f in result.stdout.strip().split('\n')
|
||||
if f.endswith('.py') and Path(f).exists()]
|
||||
return [Path(f) for f in files if f]
|
||||
else:
|
||||
self.warnings.append("Could not get staged files, checking all files")
|
||||
staged_only = False
|
||||
except subprocess.TimeoutExpired:
|
||||
self.warnings.append("Git command timed out, checking all files")
|
||||
staged_only = False
|
||||
|
||||
if not staged_only:
|
||||
directory = Path(directory or "src/fenrirscreenreader")
|
||||
if not directory.exists():
|
||||
self.errors.append(f"Directory {directory} does not exist")
|
||||
return []
|
||||
|
||||
python_files = list(directory.rglob("*.py"))
|
||||
# Filter out cache and build directories
|
||||
python_files = [f for f in python_files if not any(
|
||||
part.startswith(('__pycache__', '.git', 'build', 'dist'))
|
||||
for part in f.parts)]
|
||||
return python_files
|
||||
|
||||
return []
|
||||
|
||||
def run_flake8(self, files, select=None, ignore=None):
|
||||
"""Run flake8 on the given files."""
|
||||
if not files:
|
||||
return True, ""
|
||||
|
||||
cmd = ["flake8"]
|
||||
|
||||
if select:
|
||||
cmd.extend(["--select", select])
|
||||
if ignore:
|
||||
cmd.extend(["--ignore", ignore])
|
||||
|
||||
# Add files
|
||||
cmd.extend([str(f) for f in files])
|
||||
|
||||
try:
|
||||
result = subprocess.run(cmd, capture_output=True, text=True, timeout=30)
|
||||
return result.returncode == 0, result.stdout
|
||||
except subprocess.TimeoutExpired:
|
||||
self.errors.append("flake8 command timed out")
|
||||
return False, ""
|
||||
except Exception as e:
|
||||
self.errors.append(f"Failed to run flake8: {e}")
|
||||
return False, ""
|
||||
|
||||
def categorize_issues(self, flake8_output):
|
||||
"""Categorize flake8 issues by severity and safety for auto-fixing."""
|
||||
lines = flake8_output.strip().split('\n')
|
||||
issues = {'critical': [], 'safe_fixable': [], 'manual': []}
|
||||
|
||||
for line in lines:
|
||||
if not line.strip():
|
||||
continue
|
||||
|
||||
# Parse flake8 output: filename:line:col: code message
|
||||
parts = line.split(':', 3)
|
||||
if len(parts) < 4:
|
||||
continue
|
||||
|
||||
filename = parts[0]
|
||||
line_num = parts[1]
|
||||
col = parts[2]
|
||||
code_msg = parts[3].strip()
|
||||
|
||||
code = code_msg.split()[0] if code_msg else ""
|
||||
|
||||
# Categorize by error code
|
||||
if code.startswith('E9') or code.startswith('F'):
|
||||
# Critical syntax/import errors
|
||||
issues['critical'].append(line)
|
||||
elif code in ['E111', 'E114', 'E117', 'E121', 'E122', 'E123', 'E124',
|
||||
'E125', 'E126', 'E127', 'E128', 'E129', 'E131', 'E133',
|
||||
'W291', 'W292', 'W293']:
|
||||
# Safe indentation and whitespace fixes
|
||||
# But skip timing-critical files
|
||||
if not any(critical in filename.lower() for critical in
|
||||
['evdevdriver', 'vcsadriver', 'screenmanager', 'inputmanager']):
|
||||
issues['safe_fixable'].append(line)
|
||||
else:
|
||||
issues['manual'].append(line)
|
||||
else:
|
||||
# Everything else needs manual review
|
||||
issues['manual'].append(line)
|
||||
|
||||
return issues
|
||||
|
||||
def apply_safe_fixes(self, files):
|
||||
"""Apply safe automatic fixes using autopep8."""
|
||||
try:
|
||||
# Check if autopep8 is available
|
||||
result = subprocess.run(["autopep8", "--version"],
|
||||
capture_output=True, text=True, timeout=5)
|
||||
if result.returncode != 0:
|
||||
self.warnings.append("autopep8 not available for auto-fixing")
|
||||
return False
|
||||
except (subprocess.TimeoutExpired, FileNotFoundError):
|
||||
self.warnings.append("autopep8 not available for auto-fixing")
|
||||
return False
|
||||
|
||||
fixed_count = 0
|
||||
|
||||
for file_path in files:
|
||||
# Skip timing-critical files
|
||||
if any(critical in str(file_path).lower() for critical in
|
||||
['evdevdriver', 'vcsadriver', 'screenmanager', 'inputmanager']):
|
||||
self.log(f"Skipping timing-critical file: {file_path}", "WARNING")
|
||||
continue
|
||||
|
||||
try:
|
||||
# Apply safe fixes only
|
||||
cmd = [
|
||||
"autopep8",
|
||||
"--in-place",
|
||||
"--select", "E111,E114,E117,E121,E122,E123,E124,E125,E126,E127,E128,E129,E131,E133,W291,W292,W293",
|
||||
str(file_path)
|
||||
]
|
||||
|
||||
result = subprocess.run(cmd, capture_output=True, text=True, timeout=10)
|
||||
if result.returncode == 0:
|
||||
self.fixes_applied.append(f"Applied safe PEP8 fixes to {file_path}")
|
||||
fixed_count += 1
|
||||
else:
|
||||
self.warnings.append(f"Could not auto-fix {file_path}: {result.stderr}")
|
||||
|
||||
except subprocess.TimeoutExpired:
|
||||
self.warnings.append(f"Auto-fix timed out for {file_path}")
|
||||
except Exception as e:
|
||||
self.warnings.append(f"Error auto-fixing {file_path}: {e}")
|
||||
|
||||
return fixed_count > 0
|
||||
|
||||
def validate_files(self, files, fix_safe=False):
|
||||
"""Validate Python files for PEP8 compliance."""
|
||||
if not files:
|
||||
self.log("No Python files to validate")
|
||||
return True
|
||||
|
||||
self.log(f"Validating {len(files)} Python files")
|
||||
|
||||
# Run comprehensive flake8 check
|
||||
success, output = self.run_flake8(files)
|
||||
|
||||
if not output.strip():
|
||||
self.log("All files pass PEP8 validation", "SUCCESS")
|
||||
return True
|
||||
|
||||
# Categorize issues
|
||||
issues = self.categorize_issues(output)
|
||||
|
||||
# Report critical issues
|
||||
if issues['critical']:
|
||||
self.log(f"Critical issues found ({len(issues['critical'])}):", "ERROR")
|
||||
for issue in issues['critical'][:10]: # Limit output
|
||||
self.log(f" {issue}", "ERROR")
|
||||
if len(issues['critical']) > 10:
|
||||
self.log(f" ... and {len(issues['critical']) - 10} more", "ERROR")
|
||||
|
||||
# Handle safe fixable issues
|
||||
if issues['safe_fixable']:
|
||||
if fix_safe:
|
||||
self.log(f"Auto-fixing {len(issues['safe_fixable'])} safe issues...", "INFO")
|
||||
# Get unique files from safe_fixable issues
|
||||
fix_files = set()
|
||||
for issue in issues['safe_fixable']:
|
||||
filename = issue.split(':')[0]
|
||||
fix_files.add(Path(filename))
|
||||
|
||||
if self.apply_safe_fixes(fix_files):
|
||||
self.log("Safe auto-fixes applied", "SUCCESS")
|
||||
# Re-run flake8 to see remaining issues
|
||||
success, output = self.run_flake8(files)
|
||||
if output.strip():
|
||||
remaining_issues = self.categorize_issues(output)
|
||||
issues = remaining_issues
|
||||
else:
|
||||
issues = {'critical': [], 'safe_fixable': [], 'manual': []}
|
||||
else:
|
||||
self.log(f"Safe fixable issues found ({len(issues['safe_fixable'])}):", "WARNING")
|
||||
for issue in issues['safe_fixable'][:5]:
|
||||
self.log(f" {issue}", "WARNING")
|
||||
if len(issues['safe_fixable']) > 5:
|
||||
self.log(f" ... and {len(issues['safe_fixable']) - 5} more", "WARNING")
|
||||
self.log("Run with --fix-safe to auto-fix these", "INFO")
|
||||
|
||||
# Report manual issues
|
||||
if issues['manual']:
|
||||
self.log(f"Manual review needed ({len(issues['manual'])}):", "WARNING")
|
||||
for issue in issues['manual'][:5]:
|
||||
self.log(f" {issue}", "WARNING")
|
||||
if len(issues['manual']) > 5:
|
||||
self.log(f" ... and {len(issues['manual']) - 5} more", "WARNING")
|
||||
|
||||
# Return success if only manual issues remain (non-critical)
|
||||
return len(issues['critical']) == 0
|
||||
|
||||
def generate_report(self):
|
||||
"""Generate final validation report."""
|
||||
total_issues = len(self.errors) + len(self.warnings)
|
||||
|
||||
if self.fixes_applied:
|
||||
self.log(f"\nAUTO-FIXES APPLIED ({len(self.fixes_applied)}):", "HEADER")
|
||||
for fix in self.fixes_applied:
|
||||
self.log(fix, "SUCCESS")
|
||||
|
||||
if self.errors:
|
||||
self.log(f"\nERRORS ({len(self.errors)}):", "HEADER")
|
||||
for error in self.errors:
|
||||
self.log(error, "ERROR")
|
||||
|
||||
if self.warnings:
|
||||
self.log(f"\nWARNINGS ({len(self.warnings)}):", "HEADER")
|
||||
for warning in self.warnings:
|
||||
self.log(warning, "WARNING")
|
||||
|
||||
if len(self.errors) == 0:
|
||||
self.log("\n✅ PEP8 VALIDATION PASSED", "SUCCESS")
|
||||
if self.warnings:
|
||||
self.log("Non-critical style issues found - consider manual review", "INFO")
|
||||
return True
|
||||
else:
|
||||
self.log("\n❌ PEP8 VALIDATION FAILED", "ERROR")
|
||||
self.log("Critical issues must be fixed", "ERROR")
|
||||
return False
|
||||
|
||||
|
||||
def main():
|
||||
parser = argparse.ArgumentParser(description='Validate and fix PEP8 compliance in Fenrir')
|
||||
parser.add_argument('--fix-safe', action='store_true',
|
||||
help='Apply safe automatic fixes (avoids timing-critical files)')
|
||||
parser.add_argument('--check-only', action='store_true',
|
||||
help='Exit with non-zero code if issues found')
|
||||
parser.add_argument('--staged', action='store_true',
|
||||
help='Check only staged files')
|
||||
parser.add_argument('--quiet', action='store_true',
|
||||
help='Reduce output verbosity')
|
||||
parser.add_argument('--directory', default='src/fenrirscreenreader',
|
||||
help='Directory to scan (default: src/fenrirscreenreader)')
|
||||
|
||||
args = parser.parse_args()
|
||||
|
||||
validator = PEP8Validator(verbose=not args.quiet)
|
||||
|
||||
validator.log("FENRIR PEP8 VALIDATION", "HEADER")
|
||||
|
||||
# Check if flake8 is available
|
||||
if not validator.check_flake8_available():
|
||||
validator.log("flake8 is required but not available", "ERROR")
|
||||
validator.log("Install with: pip install flake8", "INFO")
|
||||
if args.fix_safe:
|
||||
validator.log("For auto-fixing, also install: pip install autopep8", "INFO")
|
||||
sys.exit(1)
|
||||
|
||||
# Get files to validate
|
||||
files = validator.get_python_files(
|
||||
directory=args.directory if not args.staged else None,
|
||||
staged_only=args.staged
|
||||
)
|
||||
|
||||
if not files:
|
||||
validator.log("No Python files found to validate")
|
||||
sys.exit(0)
|
||||
|
||||
# Validate files
|
||||
success = validator.validate_files(files, fix_safe=args.fix_safe)
|
||||
|
||||
# Generate report
|
||||
validation_passed = validator.generate_report()
|
||||
|
||||
if args.check_only and not validation_passed:
|
||||
sys.exit(1)
|
||||
elif validation_passed:
|
||||
sys.exit(0)
|
||||
else:
|
||||
sys.exit(1)
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
main()
|
||||
Reference in New Issue
Block a user