diff --git a/scripts/validate_architecture.py b/scripts/validate_architecture.py index 956251be..d85b8945 100755 --- a/scripts/validate_architecture.py +++ b/scripts/validate_architecture.py @@ -12,10 +12,21 @@ This script checks that the codebase follows key architectural decisions: - API endpoint patterns Usage: - python scripts/validate_architecture.py # Check all files - python scripts/validate_architecture.py --fix # Auto-fix where possible - python scripts/validate_architecture.py --verbose # Detailed output - python scripts/validate_architecture.py app/api/ # Check specific directory + python scripts/validate_architecture.py # Check all files in current directory + python scripts/validate_architecture.py -d app/api/ # Check specific directory + python scripts/validate_architecture.py -f app/api/v1/vendors.py # Check single file + python scripts/validate_architecture.py -o company # Check all company-related files + python scripts/validate_architecture.py -o vendor --verbose # Check vendor files with details + python scripts/validate_architecture.py --json # JSON output + +Options: + -f, --file PATH Validate a single file (.py, .js, or .html) + -d, --folder PATH Validate all files in a directory (recursive) + -o, --object NAME Validate all files related to an entity (e.g., company, vendor, order) + -c, --config PATH Path to architecture rules config + -v, --verbose Show detailed output including context + --errors-only Only show errors, suppress warnings + --json Output results as JSON """ import argparse @@ -52,6 +63,35 @@ class Violation: suggestion: str = "" +@dataclass +class FileResult: + """Results for a single file validation""" + + file_path: Path + errors: int = 0 + warnings: int = 0 + + @property + def passed(self) -> bool: + return self.errors == 0 + + @property + def status(self) -> str: + if self.errors > 0: + return "FAILED" + elif self.warnings > 0: + return "PASSED*" + return "PASSED" + + @property + def status_icon(self) -> str: + if self.errors > 0: + return "āŒ" + elif self.warnings > 0: + return "āš ļø" + return "āœ…" + + @dataclass class ValidationResult: """Results of architecture validation""" @@ -59,6 +99,7 @@ class ValidationResult: violations: list[Violation] = field(default_factory=list) files_checked: int = 0 rules_applied: int = 0 + file_results: list[FileResult] = field(default_factory=list) def has_errors(self) -> bool: """Check if there are any error-level violations""" @@ -93,7 +134,7 @@ class ArchitectureValidator: return config def validate_all(self, target_path: Path = None) -> ValidationResult: - """Validate all files or specific path""" + """Validate all files in a directory""" print("\nšŸ” Starting architecture validation...\n") target = target_path or self.project_root @@ -118,6 +159,245 @@ class ArchitectureValidator: return self.result + def validate_file( + self, file_path: Path, quiet: bool = False + ) -> ValidationResult: + """Validate a single file""" + if not file_path.exists(): + if not quiet: + print(f"āŒ File not found: {file_path}") + return self.result + + if not file_path.is_file(): + if not quiet: + print(f"āŒ Not a file: {file_path}") + return self.result + + if not quiet: + print(f"\nšŸ” Validating single file: {file_path}\n") + + # Resolve file path to absolute + file_path = file_path.resolve() + file_path_str = str(file_path) + + if self._should_ignore_file(file_path): + if not quiet: + print("ā­ļø File is in ignore list, skipping") + return self.result + + self.result.files_checked += 1 + + # Track violations before this file + violations_before = len(self.result.violations) + + content = file_path.read_text() + lines = content.split("\n") + + # Determine file type and run appropriate validators + if file_path.suffix == ".py": + self._validate_python_file(file_path, content, lines, file_path_str) + elif file_path.suffix == ".js": + self._validate_js_file(file_path, content, lines) + elif file_path.suffix == ".html": + self._validate_html_file(file_path, content, lines) + else: + if not quiet: + print(f"āš ļø Unsupported file type: {file_path.suffix}") + + # Calculate violations for this file + file_violations = self.result.violations[violations_before:] + errors = sum(1 for v in file_violations if v.severity == Severity.ERROR) + warnings = sum(1 for v in file_violations if v.severity == Severity.WARNING) + + # Track file result + self.result.file_results.append( + FileResult(file_path=file_path, errors=errors, warnings=warnings) + ) + + return self.result + + def validate_object(self, object_name: str) -> ValidationResult: + """Validate all files related to an entity (e.g., company, vendor, order)""" + print(f"\nšŸ” Searching for '{object_name}'-related files...\n") + + # Generate name variants (singular/plural forms) + name = object_name.lower() + variants = {name} + + # Handle common plural patterns + if name.endswith("ies"): + # companies -> company + variants.add(name[:-3] + "y") + elif name.endswith("s"): + # vendors -> vendor + variants.add(name[:-1]) + else: + # company -> companies, vendor -> vendors + if name.endswith("y"): + variants.add(name[:-1] + "ies") + variants.add(name + "s") + + # Search patterns for different file types + patterns = [] + for variant in variants: + patterns.extend([ + f"app/api/**/*{variant}*.py", + f"app/services/*{variant}*.py", + f"app/exceptions/*{variant}*.py", + f"models/database/*{variant}*.py", + f"models/schema/*{variant}*.py", + f"static/admin/js/*{variant}*.js", + f"app/templates/admin/*{variant}*.html", + ]) + + # Find all matching files + found_files: set[Path] = set() + for pattern in patterns: + matches = list(self.project_root.glob(pattern)) + for match in matches: + if match.is_file() and not self._should_ignore_file(match): + found_files.add(match) + + if not found_files: + print(f"āŒ No files found matching '{object_name}'") + return self.result + + # Sort files by type for better readability + sorted_files = sorted(found_files, key=lambda p: (p.suffix, str(p))) + + print(f"šŸ“ Found {len(sorted_files)} files:\n") + for f in sorted_files: + rel_path = f.relative_to(self.project_root) + print(f" • {rel_path}") + + print("\n" + "-" * 60 + "\n") + + # Validate each file + for file_path in sorted_files: + rel_path = file_path.relative_to(self.project_root) + print(f"šŸ“„ {rel_path}") + self.validate_file(file_path, quiet=True) + + return self.result + + def _validate_python_file( + self, file_path: Path, content: str, lines: list[str], file_path_str: str + ): + """Validate a single Python file based on its location""" + # API endpoints + if "/app/api/" in file_path_str or "\\app\\api\\" in file_path_str: + print("šŸ“” Validating as API endpoint...") + self._check_pydantic_usage(file_path, content, lines) + self._check_no_business_logic_in_endpoints(file_path, content, lines) + self._check_endpoint_exception_handling(file_path, content, lines) + self._check_endpoint_authentication(file_path, content, lines) + + # Service layer + elif "/app/services/" in file_path_str or "\\app\\services\\" in file_path_str: + print("šŸ”§ Validating as service layer...") + self._check_no_http_exception_in_services(file_path, content, lines) + self._check_service_exceptions(file_path, content, lines) + self._check_db_session_parameter(file_path, content, lines) + + # Models + elif "/app/models/" in file_path_str or "\\app\\models\\" in file_path_str: + print("šŸ“¦ Validating as model...") + for i, line in enumerate(lines, 1): + if re.search(r"class.*\(Base.*,.*BaseModel.*\)", line): + self._add_violation( + rule_id="MDL-002", + rule_name="Separate SQLAlchemy and Pydantic models", + severity=Severity.ERROR, + file_path=file_path, + line_number=i, + message="Model mixes SQLAlchemy Base and Pydantic BaseModel", + context=line.strip(), + suggestion="Keep SQLAlchemy models and Pydantic models separate", + ) + + # Generic Python file - check exception handling + print("āš ļø Validating exception handling...") + for i, line in enumerate(lines, 1): + if re.match(r"\s*except\s*:", line): + self._add_violation( + rule_id="EXC-002", + rule_name="No bare except clauses", + severity=Severity.ERROR, + file_path=file_path, + line_number=i, + message="Bare except clause catches all exceptions including system exits", + context=line.strip(), + suggestion="Specify exception type: except ValueError: or except Exception:", + ) + + def _validate_js_file(self, file_path: Path, content: str, lines: list[str]): + """Validate a single JavaScript file""" + print("🟨 Validating JavaScript...") + + # JS-001: Check for window.apiClient + for i, line in enumerate(lines, 1): + if "window.apiClient" in line: + before_occurrence = line[: line.find("window.apiClient")] + if "//" not in before_occurrence: + self._add_violation( + rule_id="JS-001", + rule_name="Use apiClient directly", + severity=Severity.WARNING, + file_path=file_path, + line_number=i, + message="Use apiClient directly instead of window.apiClient", + context=line.strip(), + suggestion="Replace window.apiClient with apiClient", + ) + + # JS-002: Check for console usage + for i, line in enumerate(lines, 1): + if re.search(r"console\.(log|warn|error)", line): + if "//" in line or "āœ…" in line or "eslint-disable" in line: + continue + self._add_violation( + rule_id="JS-002", + rule_name="Use centralized logger", + severity=Severity.WARNING, + file_path=file_path, + line_number=i, + message="Use centralized logger instead of console", + context=line.strip()[:80], + suggestion="Use window.LogConfig.createLogger('moduleName')", + ) + + def _validate_html_file(self, file_path: Path, content: str, lines: list[str]): + """Validate a single HTML template file""" + print("šŸ“„ Validating template...") + + # Skip base template and partials + if "base.html" in file_path.name or "partials" in str(file_path): + print("ā­ļø Skipping base/partial template") + return + + # Only check admin templates + file_path_str = str(file_path) + if "/admin/" not in file_path_str and "\\admin\\" not in file_path_str: + print("ā­ļø Not an admin template, skipping extends check") + return + + # TPL-001: Check for extends + has_extends = any( + "{% extends" in line and "admin/base.html" in line for line in lines + ) + + if not has_extends: + self._add_violation( + rule_id="TPL-001", + rule_name="Templates must extend base", + severity=Severity.ERROR, + file_path=file_path, + line_number=1, + message="Admin template does not extend admin/base.html", + context=file_path.name, + suggestion="Add {% extends 'admin/base.html' %} at the top", + ) + def _validate_api_endpoints(self, target_path: Path): """Validate API endpoint rules (API-001, API-002, API-003, API-004)""" print("šŸ“” Validating API endpoints...") @@ -215,53 +495,40 @@ class ArchitectureValidator: def _check_endpoint_exception_handling( self, file_path: Path, content: str, lines: list[str] ): - """API-003: Check proper exception handling in endpoints""" + """API-003: Check that endpoints do NOT raise HTTPException directly. + + The architecture uses a global exception handler that catches domain + exceptions (WizamartException subclasses) and converts them to HTTP + responses. Endpoints should let exceptions bubble up, not catch and + convert them manually. + """ rule = self._get_rule("API-003") if not rule: return - # Parse file to check for try/except in route handlers - try: - tree = ast.parse(content) - except SyntaxError: + # Skip exception handler file - it's allowed to use HTTPException + if "exceptions/handler.py" in str(file_path): return - for node in ast.walk(tree): - if isinstance(node, ast.FunctionDef): - # Check if it's a route handler - has_router_decorator = any( - isinstance(d, ast.Call) - and isinstance(d.func, ast.Attribute) - and getattr(d.func.value, "id", None) == "router" - for d in node.decorator_list + for i, line in enumerate(lines, 1): + # Check for raise HTTPException + if "raise HTTPException" in line: + # Skip if it's a comment + stripped = line.strip() + if stripped.startswith("#"): + continue + + self._add_violation( + rule_id="API-003", + rule_name=rule["name"], + severity=Severity.ERROR, + file_path=file_path, + line_number=i, + message="Endpoint raises HTTPException directly", + context=line.strip()[:80], + suggestion="Use domain exceptions (e.g., VendorNotFoundException) and let global handler convert", ) - if has_router_decorator: - # Check if function body has try/except - has_try_except = any( - isinstance(child, ast.Try) for child in ast.walk(node) - ) - - # Check if function calls service methods - has_service_call = any( - isinstance(child, ast.Call) - and isinstance(child.func, ast.Attribute) - and "service" in getattr(child.func.value, "id", "").lower() - for child in ast.walk(node) - ) - - if has_service_call and not has_try_except: - self._add_violation( - rule_id="API-003", - rule_name=rule["name"], - severity=Severity.WARNING, - file_path=file_path, - line_number=node.lineno, - message=f"Endpoint '{node.name}' calls service but lacks exception handling", - context=f"def {node.name}(...)", - suggestion="Wrap service calls in try/except and convert to HTTPException", - ) - def _check_endpoint_authentication( self, file_path: Path, content: str, lines: list[str] ): @@ -603,6 +870,10 @@ class ArchitectureValidator: print(f"Files checked: {self.result.files_checked}") print(f"Total violations: {len(self.result.violations)}\n") + # Print file summary table if we have file results + if self.result.file_results: + self._print_summary_table() + # Group by severity errors = [v for v in self.result.violations if v.severity == Severity.ERROR] warnings = [v for v in self.result.violations if v.severity == Severity.WARNING] @@ -633,6 +904,58 @@ class ArchitectureValidator: print("=" * 80) return 0 + def _print_summary_table(self): + """Print a summary table of file results""" + print("šŸ“‹ FILE SUMMARY:") + print("-" * 80) + + # Calculate column widths + max_path_len = max( + len( + str( + fr.file_path.relative_to(self.project_root) + if self.project_root in fr.file_path.parents + else fr.file_path + ) + ) + for fr in self.result.file_results + ) + max_path_len = min(max_path_len, 55) # Cap at 55 chars + + # Print header + print(f" {'File':<{max_path_len}} {'Status':<8} {'Errors':<7} {'Warnings':<8}") + print(f" {'-' * max_path_len} {'-' * 8} {'-' * 7} {'-' * 8}") + + # Print each file + for fr in self.result.file_results: + rel_path = ( + fr.file_path.relative_to(self.project_root) + if self.project_root in fr.file_path.parents + else fr.file_path + ) + path_str = str(rel_path) + if len(path_str) > max_path_len: + path_str = "..." + path_str[-(max_path_len - 3) :] + + print( + f" {path_str:<{max_path_len}} " + f"{fr.status_icon} {fr.status:<5} " + f"{fr.errors:<7} " + f"{fr.warnings:<8}" + ) + + print("-" * 80) + + # Print totals + total_errors = sum(fr.errors for fr in self.result.file_results) + total_warnings = sum(fr.warnings for fr in self.result.file_results) + passed = sum(1 for fr in self.result.file_results if fr.passed) + failed = len(self.result.file_results) - passed + + print(f"\n Total: {len(self.result.file_results)} files | " + f"āœ… {passed} passed | āŒ {failed} failed | " + f"{total_errors} errors | {total_warnings} warnings\n") + def print_json(self) -> int: """Print validation results as JSON""" import json @@ -700,12 +1023,31 @@ def main(): epilog=__doc__, ) - parser.add_argument( - "path", - nargs="?", + # Target options (mutually exclusive) + target_group = parser.add_mutually_exclusive_group() + + target_group.add_argument( + "-f", + "--file", type=Path, - default=Path.cwd(), - help="Path to validate (default: current directory)", + metavar="PATH", + help="Validate a single file (.py, .js, or .html)", + ) + + target_group.add_argument( + "-d", + "--folder", + type=Path, + metavar="PATH", + help="Validate all files in a directory (recursive)", + ) + + target_group.add_argument( + "-o", + "--object", + type=str, + metavar="NAME", + help="Validate all files related to an entity (e.g., company, vendor, order)", ) parser.add_argument( @@ -738,8 +1080,22 @@ def main(): # Create validator validator = ArchitectureValidator(args.config, verbose=args.verbose) - # Run validation - result = validator.validate_all(args.path) + # Determine validation mode + if args.file: + # Validate single file + result = validator.validate_file(args.file) + elif args.folder: + # Validate directory + if not args.folder.is_dir(): + print(f"āŒ Not a directory: {args.folder}") + sys.exit(1) + result = validator.validate_all(args.folder) + elif args.object: + # Validate all files related to an entity + result = validator.validate_object(args.object) + else: + # Default: validate current directory + result = validator.validate_all(Path.cwd()) # Output results if args.json: