feat(arch): add --file, --folder, --object options to architecture validator
- Add -f/--file option to validate a single file - Add -d/--folder option to validate a directory - Add -o/--object option to validate all files related to an entity (e.g., company, vendor, user) with automatic singular/plural handling - Add summary table showing pass/fail status per file with error/warning counts - Remove deprecated positional path argument Usage examples: python scripts/validate_architecture.py -f app/api/v1/vendors.py python scripts/validate_architecture.py -d app/api/ python scripts/validate_architecture.py -o company 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user