From 94a1acbacaed63a980f163ab8c0992f28badaf30 Mon Sep 17 00:00:00 2001 From: Storm Dragon Date: Thu, 24 Jul 2025 14:05:51 -0400 Subject: [PATCH] A few improvements to validation. --- RELEASE_CHECKLIST.md | 1 + tools/pre-commit-composite | 37 ++++ tools/setup_validation.sh | 1 + tools/validate_pep8.py | 365 +++++++++++++++++++++++++++++++++++++ 4 files changed, 404 insertions(+) create mode 100755 tools/validate_pep8.py diff --git a/RELEASE_CHECKLIST.md b/RELEASE_CHECKLIST.md index f6a64179..8980faa4 100644 --- a/RELEASE_CHECKLIST.md +++ b/RELEASE_CHECKLIST.md @@ -15,6 +15,7 @@ This checklist ensures thorough validation before releasing Fenrir packages. ### Validation Scripts - `tools/validate_syntax.py` - Python syntax validation +- `tools/validate_pep8.py` - PEP8 compliance checking with safe auto-fix - `tools/validate_release.py` - Comprehensive release validation - `tools/cleanup_cache.py` - Remove Python cache files and directories - `tools/pre-commit-hook` - Git pre-commit validation diff --git a/tools/pre-commit-composite b/tools/pre-commit-composite index 132a1444..33057a85 100755 --- a/tools/pre-commit-composite +++ b/tools/pre-commit-composite @@ -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 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}" diff --git a/tools/setup_validation.sh b/tools/setup_validation.sh index f5aa9c7c..663209cf 100755 --- a/tools/setup_validation.sh +++ b/tools/setup_validation.sh @@ -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 diff --git a/tools/validate_pep8.py b/tools/validate_pep8.py new file mode 100755 index 00000000..f2dbbd7b --- /dev/null +++ b/tools/validate_pep8.py @@ -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() \ No newline at end of file