Change the validation report output to show a breakdown by severity (errors, warnings, info) instead of a confusing "Total violations" count that included info-level items. Before: "Total violations: 4" followed by "VALIDATION PASSED" After: "Findings: 0 errors, 0 warnings, 4 info" with "VALIDATION PASSED" Also improve the failure/warning messages to include counts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2760 lines
116 KiB
Python
Executable File
2760 lines
116 KiB
Python
Executable File
#!/usr/bin/env python3
|
||
"""
|
||
Architecture Validator
|
||
======================
|
||
Validates code against architectural rules defined in .architecture-rules.yaml
|
||
|
||
This script checks that the codebase follows key architectural decisions:
|
||
- Separation of concerns (routes vs services)
|
||
- Proper exception handling (domain exceptions vs HTTPException)
|
||
- Correct use of Pydantic vs SQLAlchemy models
|
||
- Service layer patterns
|
||
- API endpoint patterns
|
||
|
||
Usage:
|
||
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
|
||
import ast
|
||
import re
|
||
import sys
|
||
from dataclasses import dataclass, field
|
||
from enum import Enum
|
||
from pathlib import Path
|
||
from typing import Any
|
||
|
||
import yaml
|
||
|
||
|
||
class Severity(Enum):
|
||
"""Validation severity levels"""
|
||
|
||
ERROR = "error"
|
||
WARNING = "warning"
|
||
INFO = "info"
|
||
|
||
|
||
@dataclass
|
||
class Violation:
|
||
"""Represents an architectural rule violation"""
|
||
|
||
rule_id: str
|
||
rule_name: str
|
||
severity: Severity
|
||
file_path: Path
|
||
line_number: int
|
||
message: str
|
||
context: str = ""
|
||
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"""
|
||
|
||
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"""
|
||
return any(v.severity == Severity.ERROR for v in self.violations)
|
||
|
||
def has_warnings(self) -> bool:
|
||
"""Check if there are any warning-level violations"""
|
||
return any(v.severity == Severity.WARNING for v in self.violations)
|
||
|
||
|
||
class ArchitectureValidator:
|
||
"""Main validator class"""
|
||
|
||
def __init__(self, config_path: Path, verbose: bool = False):
|
||
"""Initialize validator with configuration"""
|
||
self.config_path = config_path
|
||
self.verbose = verbose
|
||
self.config = self._load_config()
|
||
self.result = ValidationResult()
|
||
self.project_root = Path.cwd()
|
||
|
||
def _load_config(self) -> dict[str, Any]:
|
||
"""Load validation rules from YAML config"""
|
||
if not self.config_path.exists():
|
||
print(f"❌ Configuration file not found: {self.config_path}")
|
||
sys.exit(1)
|
||
|
||
with open(self.config_path) as f:
|
||
config = yaml.safe_load(f)
|
||
|
||
print(f"📋 Loaded architecture rules: {config.get('project', 'unknown')}")
|
||
return config
|
||
|
||
def validate_all(self, target_path: Path = None) -> ValidationResult:
|
||
"""Validate all files in a directory"""
|
||
print("\n🔍 Starting architecture validation...\n")
|
||
|
||
target = target_path or self.project_root
|
||
|
||
# Validate API endpoints
|
||
self._validate_api_endpoints(target)
|
||
|
||
# Validate service layer
|
||
self._validate_service_layer(target)
|
||
|
||
# Validate models
|
||
self._validate_models(target)
|
||
|
||
# Validate exception handling
|
||
self._validate_exceptions(target)
|
||
|
||
# Validate naming conventions
|
||
self._validate_naming_conventions(target)
|
||
|
||
# Validate auth patterns
|
||
self._validate_auth_patterns(target)
|
||
|
||
# Validate middleware
|
||
self._validate_middleware(target)
|
||
|
||
# Validate JavaScript
|
||
self._validate_javascript(target)
|
||
|
||
# Validate templates
|
||
self._validate_templates(target)
|
||
|
||
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)
|
||
self._check_no_commit_in_services(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 console usage (must use centralized logger)
|
||
# Skip init-*.js files - they run before logger is available
|
||
if not file_path.name.startswith("init-"):
|
||
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-001",
|
||
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')",
|
||
)
|
||
|
||
# JS-002: Check for window.apiClient (must use lowercase 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-002",
|
||
rule_name="Use lowercase apiClient",
|
||
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-003: Check Alpine components spread ...data() for base layout inheritance
|
||
# Only check files that define Alpine component functions (function adminXxx() { return {...} })
|
||
self._check_alpine_data_spread(file_path, content, lines)
|
||
|
||
# JS-004: Check Alpine components set currentPage
|
||
self._check_alpine_current_page(file_path, content, lines)
|
||
|
||
# JS-005: Check initialization guard
|
||
self._check_init_guard(file_path, content, lines)
|
||
|
||
# JS-006: Check async error handling
|
||
self._check_async_error_handling(file_path, content, lines)
|
||
|
||
# JS-007: Check loading state management
|
||
self._check_loading_state(file_path, content, lines)
|
||
|
||
# JS-008: Check for raw fetch() calls instead of apiClient
|
||
self._check_fetch_vs_api_client(file_path, content, lines)
|
||
|
||
# JS-009: Check for alert() or window.showToast instead of Utils.showToast()
|
||
self._check_toast_usage(file_path, content, lines)
|
||
|
||
def _check_toast_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""JS-009: Check for alert() or window.showToast instead of Utils.showToast()"""
|
||
# Skip utils.js (where showToast is defined)
|
||
if "utils.js" in file_path.name:
|
||
return
|
||
|
||
# Check for file-level noqa comment
|
||
if "noqa: js-009" in content.lower():
|
||
return
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
stripped = line.strip()
|
||
|
||
# Skip comments
|
||
if stripped.startswith("//") or stripped.startswith("/*"):
|
||
continue
|
||
|
||
# Skip lines with inline noqa comment
|
||
if "noqa: js-009" in line.lower():
|
||
continue
|
||
|
||
# Check for alert() usage
|
||
if re.search(r"\balert\s*\(", line):
|
||
self._add_violation(
|
||
rule_id="JS-009",
|
||
rule_name="Use Utils.showToast() for notifications",
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Browser alert() dialog - use Utils.showToast() for consistent UX",
|
||
context=stripped[:80],
|
||
suggestion="Replace alert('message') with Utils.showToast('message', 'success'|'error'|'warning'|'info')",
|
||
)
|
||
|
||
# Check for window.showToast usage
|
||
if "window.showToast" in line:
|
||
self._add_violation(
|
||
rule_id="JS-009",
|
||
rule_name="Use Utils.showToast() for notifications",
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="window.showToast is undefined - use Utils.showToast()",
|
||
context=stripped[:80],
|
||
suggestion="Replace window.showToast('msg', 'type') with Utils.showToast('msg', 'type')",
|
||
)
|
||
|
||
def _check_alpine_data_spread(self, file_path: Path, content: str, lines: list[str]):
|
||
"""JS-003: Check that Alpine components inherit base layout data using ...data()"""
|
||
# Skip utility/init files that aren't page components
|
||
skip_patterns = ["init-", "api-client", "log-config", "utils", "helpers"]
|
||
if any(pattern in file_path.name for pattern in skip_patterns):
|
||
return
|
||
|
||
# Check for noqa comment
|
||
if "noqa: js-003" in content.lower():
|
||
return
|
||
|
||
# Look for Alpine component function pattern: function adminXxx() { return { ... } }
|
||
# These are page-level components that should inherit from data()
|
||
component_pattern = re.compile(
|
||
r"function\s+(admin\w+|vendor\w+|shop\w+)\s*\(\s*\)\s*\{",
|
||
re.IGNORECASE
|
||
)
|
||
|
||
for match in component_pattern.finditer(content):
|
||
func_name = match.group(1)
|
||
func_start = match.start()
|
||
line_num = content[:func_start].count('\n') + 1
|
||
|
||
# Find the return statement with object literal
|
||
# Look for "return {" within reasonable distance
|
||
search_region = content[func_start:func_start + 500]
|
||
|
||
if "return {" in search_region:
|
||
# Check if ...data() is present in the return object
|
||
# Look for pattern like "return {\n ...data()," or similar
|
||
return_match = re.search(r"return\s*\{([^}]{0,200})", search_region, re.DOTALL)
|
||
if return_match:
|
||
return_content = return_match.group(1)
|
||
if "...data()" not in return_content:
|
||
self._add_violation(
|
||
rule_id="JS-003",
|
||
rule_name="Alpine components must spread ...data()",
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=line_num,
|
||
message=f"Alpine component '{func_name}' does not inherit base layout data",
|
||
context=f"function {func_name}() {{ return {{ ... }}",
|
||
suggestion="Add '...data(),' as first property in return object to inherit dark mode, sidebar state, etc.",
|
||
)
|
||
|
||
def _check_fetch_vs_api_client(self, file_path: Path, content: str, lines: list[str]):
|
||
"""JS-008: Check for raw fetch() calls that should use apiClient instead"""
|
||
# Skip init files and api-client itself
|
||
if "init-" in file_path.name or "api-client" in file_path.name:
|
||
return
|
||
|
||
# Check for noqa comment
|
||
if "noqa: js-008" in content.lower():
|
||
return
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
# Look for fetch( calls that hit API endpoints
|
||
if re.search(r"\bfetch\s*\(", line):
|
||
# Skip if it's a comment
|
||
stripped = line.strip()
|
||
if stripped.startswith("//") or stripped.startswith("/*"):
|
||
continue
|
||
|
||
# Check if it's calling an API endpoint (contains /api/)
|
||
if "/api/" in line or "apiClient" in line:
|
||
# If using fetch with /api/, should use apiClient instead
|
||
if "/api/" in line and "apiClient" not in line:
|
||
self._add_violation(
|
||
rule_id="JS-008",
|
||
rule_name="Use apiClient for API calls",
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Raw fetch() call to API endpoint - use apiClient for automatic auth",
|
||
context=stripped[:80],
|
||
suggestion="Replace fetch('/api/...') with apiClient.get('/endpoint') or apiClient.post('/endpoint', data)",
|
||
)
|
||
|
||
def _check_alpine_current_page(self, file_path: Path, content: str, lines: list[str]):
|
||
"""JS-004: Check that Alpine components set currentPage identifier"""
|
||
# Skip utility/init files
|
||
skip_patterns = ["init-", "api-client", "log-config", "utils", "helpers"]
|
||
if any(pattern in file_path.name for pattern in skip_patterns):
|
||
return
|
||
|
||
if "noqa: js-004" in content.lower():
|
||
return
|
||
|
||
# Look for Alpine component function pattern
|
||
component_pattern = re.compile(
|
||
r"function\s+(admin\w+|vendor\w+|shop\w+)\s*\(\s*\)\s*\{",
|
||
re.IGNORECASE
|
||
)
|
||
|
||
for match in component_pattern.finditer(content):
|
||
func_name = match.group(1)
|
||
func_start = match.start()
|
||
line_num = content[:func_start].count('\n') + 1
|
||
|
||
# Check if currentPage is set in the return object
|
||
search_region = content[func_start:func_start + 800]
|
||
if "return {" in search_region:
|
||
return_match = re.search(r"return\s*\{([^}]{0,500})", search_region, re.DOTALL)
|
||
if return_match:
|
||
return_content = return_match.group(1)
|
||
if "currentPage:" not in return_content and "currentPage :" not in return_content:
|
||
self._add_violation(
|
||
rule_id="JS-004",
|
||
rule_name="Alpine components must set currentPage",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=line_num,
|
||
message=f"Alpine component '{func_name}' does not set currentPage identifier",
|
||
context=f"function {func_name}()",
|
||
suggestion="Add 'currentPage: \"page-name\",' in return object for sidebar highlighting",
|
||
)
|
||
|
||
def _check_init_guard(self, file_path: Path, content: str, lines: list[str]):
|
||
"""JS-005: Check that init methods have duplicate initialization guards"""
|
||
# Skip utility/init files
|
||
skip_patterns = ["init-", "api-client", "log-config", "utils", "helpers"]
|
||
if any(pattern in file_path.name for pattern in skip_patterns):
|
||
return
|
||
|
||
if "noqa: js-005" in content.lower():
|
||
return
|
||
|
||
# Look for init() methods in Alpine components
|
||
init_pattern = re.compile(r"async\s+init\s*\(\s*\)\s*\{|init\s*\(\s*\)\s*\{")
|
||
|
||
for match in init_pattern.finditer(content):
|
||
init_start = match.start()
|
||
line_num = content[:init_start].count('\n') + 1
|
||
|
||
# Check next 200 chars for initialization guard pattern
|
||
search_region = content[init_start:init_start + 300]
|
||
guard_patterns = [
|
||
"window._",
|
||
"if (this._initialized)",
|
||
"if (window.",
|
||
"_initialized",
|
||
]
|
||
|
||
has_guard = any(pattern in search_region for pattern in guard_patterns)
|
||
if not has_guard:
|
||
self._add_violation(
|
||
rule_id="JS-005",
|
||
rule_name="Initialization methods must include guard",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=line_num,
|
||
message="init() method lacks duplicate initialization guard",
|
||
context="init() { ... }",
|
||
suggestion="Add guard: if (window._pageInitialized) return; window._pageInitialized = true;",
|
||
)
|
||
return # Only report once per file
|
||
|
||
def _check_async_error_handling(self, file_path: Path, content: str, lines: list[str]):
|
||
"""JS-006: Check that async operations have try/catch error handling"""
|
||
# Skip utility/init files
|
||
skip_patterns = ["init-", "api-client", "log-config"]
|
||
if any(pattern in file_path.name for pattern in skip_patterns):
|
||
return
|
||
|
||
if "noqa: js-006" in content.lower():
|
||
return
|
||
|
||
# Look for async functions/methods
|
||
async_pattern = re.compile(r"async\s+\w+\s*\([^)]*\)\s*\{")
|
||
|
||
for match in async_pattern.finditer(content):
|
||
func_start = match.start()
|
||
line_num = content[:func_start].count('\n') + 1
|
||
|
||
# Find the function body (look for matching braces)
|
||
# Simplified: check next 500 chars for try/catch
|
||
search_region = content[func_start:func_start + 800]
|
||
|
||
# Check if there's await without try/catch
|
||
if "await " in search_region and "try {" not in search_region and "try{" not in search_region:
|
||
# Check if it's a simple one-liner or has error handling elsewhere
|
||
if ".catch(" not in search_region:
|
||
self._add_violation(
|
||
rule_id="JS-006",
|
||
rule_name="Async operations must have error handling",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=line_num,
|
||
message="Async function with await lacks try/catch error handling",
|
||
context=match.group(0)[:50],
|
||
suggestion="Wrap await calls in try/catch with proper error logging",
|
||
)
|
||
return # Only report once per file
|
||
|
||
def _check_loading_state(self, file_path: Path, content: str, lines: list[str]):
|
||
"""JS-007: Check that async operations manage loading state"""
|
||
# Skip utility/init files
|
||
skip_patterns = ["init-", "api-client", "log-config", "utils"]
|
||
if any(pattern in file_path.name for pattern in skip_patterns):
|
||
return
|
||
|
||
if "noqa: js-007" in content.lower():
|
||
return
|
||
|
||
# Look for Alpine component functions that have async methods with API calls
|
||
component_pattern = re.compile(
|
||
r"function\s+(admin\w+|vendor\w+|shop\w+)\s*\(\s*\)\s*\{",
|
||
re.IGNORECASE
|
||
)
|
||
|
||
for match in component_pattern.finditer(content):
|
||
func_start = match.start()
|
||
# Get the component body (rough extraction)
|
||
component_region = content[func_start:func_start + 5000]
|
||
|
||
# Check if component has API calls but no loading state
|
||
has_api_calls = "apiClient." in component_region or "await " in component_region
|
||
has_loading_state = "loading:" in component_region or "loading :" in component_region
|
||
has_loading_assignment = "this.loading = " in component_region or "loading = true" in component_region
|
||
|
||
if has_api_calls and not has_loading_state:
|
||
line_num = content[:func_start].count('\n') + 1
|
||
self._add_violation(
|
||
rule_id="JS-007",
|
||
rule_name="Set loading state for async operations",
|
||
severity=Severity.INFO,
|
||
file_path=file_path,
|
||
line_number=line_num,
|
||
message="Component has API calls but no loading state property",
|
||
context=match.group(1),
|
||
suggestion="Add 'loading: false,' to component state and set it before/after API calls",
|
||
)
|
||
return
|
||
|
||
def _validate_html_file(self, file_path: Path, content: str, lines: list[str]):
|
||
"""Validate a single HTML template file"""
|
||
print("📄 Validating template...")
|
||
|
||
file_path_str = str(file_path)
|
||
|
||
# Skip base template and partials for extends check
|
||
is_base_or_partial = "base.html" in file_path.name or "partials" in file_path_str
|
||
|
||
# Skip macros directory for FE rules
|
||
is_macro = "shared/macros/" in file_path_str or "shared\\macros\\" in file_path_str
|
||
|
||
# Skip components showcase page
|
||
is_components_page = "components.html" in file_path.name
|
||
|
||
# Determine template type
|
||
is_admin = "/admin/" in file_path_str or "\\admin\\" in file_path_str
|
||
is_vendor = "/vendor/" in file_path_str or "\\vendor\\" in file_path_str
|
||
is_shop = "/shop/" in file_path_str or "\\shop\\" in file_path_str
|
||
|
||
if is_base_or_partial:
|
||
print("⏭️ Skipping base/partial template")
|
||
elif is_macro:
|
||
print("⏭️ Skipping macro file")
|
||
else:
|
||
# FE-001: Check for inline pagination (should use macro)
|
||
if not is_components_page:
|
||
self._check_pagination_macro_usage(file_path, content, lines)
|
||
|
||
# FE-002: Check for inline SVGs (should use $icon())
|
||
if not is_components_page and not is_macro:
|
||
self._check_icon_helper_usage(file_path, content, lines)
|
||
|
||
# FE-008: Check for raw number inputs (should use number_stepper)
|
||
if not is_components_page and not is_macro:
|
||
self._check_number_stepper_macro_usage(file_path, content, lines)
|
||
|
||
# TPL-004: Check x-text usage for dynamic content
|
||
self._check_xtext_usage(file_path, content, lines)
|
||
|
||
# TPL-005: Check x-html safety
|
||
self._check_xhtml_safety(file_path, content, lines)
|
||
|
||
# TPL-006: Check loading state implementation
|
||
self._check_template_loading_state(file_path, content, lines)
|
||
|
||
# TPL-007: Check empty state implementation
|
||
self._check_template_empty_state(file_path, content, lines)
|
||
|
||
if is_base_or_partial:
|
||
return
|
||
|
||
# Check for standalone marker in template (first 5 lines)
|
||
first_lines = "\n".join(lines[:5]).lower()
|
||
if "standalone" in first_lines or "noqa: tpl-00" in first_lines:
|
||
print("⏭️ Template marked as standalone, skipping extends check")
|
||
return
|
||
|
||
# TPL-001: Admin templates extends check
|
||
if is_admin:
|
||
self._check_template_extends(
|
||
file_path, lines, "admin/base.html", "TPL-001",
|
||
["login.html", "errors/", "test-"]
|
||
)
|
||
|
||
# TPL-002: Vendor templates extends check
|
||
if is_vendor:
|
||
self._check_template_extends(
|
||
file_path, lines, "vendor/base.html", "TPL-002",
|
||
["login.html", "errors/", "test-"]
|
||
)
|
||
|
||
# TPL-003: Shop templates extends check
|
||
if is_shop:
|
||
self._check_template_extends(
|
||
file_path, lines, "shop/base.html", "TPL-003",
|
||
["errors/", "test-"]
|
||
)
|
||
|
||
def _check_template_extends(
|
||
self, file_path: Path, lines: list[str],
|
||
base_template: str, rule_id: str, exclusions: list[str]
|
||
):
|
||
"""Check that template extends the appropriate base template"""
|
||
file_path_str = str(file_path)
|
||
|
||
# Check exclusion patterns
|
||
for exclusion in exclusions:
|
||
if exclusion in file_path_str:
|
||
print(f"⏭️ Template matches exclusion pattern '{exclusion}', skipping")
|
||
return
|
||
|
||
# Check for extends
|
||
has_extends = any(
|
||
"{% extends" in line and base_template in line for line in lines
|
||
)
|
||
|
||
if not has_extends:
|
||
template_type = base_template.split("/")[0]
|
||
self._add_violation(
|
||
rule_id=rule_id,
|
||
rule_name=f"{template_type.title()} templates must extend base",
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=1,
|
||
message=f"{template_type.title()} template does not extend {base_template}",
|
||
context=file_path.name,
|
||
suggestion=f"Add {{% extends '{base_template}' %}} at the top, or add {{# standalone #}} if intentional",
|
||
)
|
||
|
||
def _check_xtext_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""TPL-004: Check that x-text is used for dynamic text content"""
|
||
if "noqa: tpl-004" in content.lower():
|
||
return
|
||
|
||
# Look for potentially unsafe patterns where user data might be interpolated
|
||
# This is a heuristic check - not perfect
|
||
unsafe_patterns = [
|
||
(r'\{\{\s*\w+\.name\s*\}\}', "User data interpolation without x-text"),
|
||
(r'\{\{\s*\w+\.title\s*\}\}', "User data interpolation without x-text"),
|
||
(r'\{\{\s*\w+\.description\s*\}\}', "User data interpolation without x-text"),
|
||
]
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
# Skip Jinja2 template syntax (server-side safe)
|
||
if "{%" in line:
|
||
continue
|
||
|
||
for pattern, message in unsafe_patterns:
|
||
if re.search(pattern, line):
|
||
# Check if it's in an x-text context already
|
||
if 'x-text="' not in line and "x-text='" not in line:
|
||
self._add_violation(
|
||
rule_id="TPL-004",
|
||
rule_name="Use x-text for dynamic text content",
|
||
severity=Severity.INFO,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message=message,
|
||
context=line.strip()[:60],
|
||
suggestion='Use x-text="item.name" instead of {{ item.name }} for XSS safety',
|
||
)
|
||
return # Only report once per file
|
||
|
||
def _check_xhtml_safety(self, file_path: Path, content: str, lines: list[str]):
|
||
"""TPL-005: Check that x-html is only used for safe content"""
|
||
if "noqa: tpl-005" in content.lower():
|
||
return
|
||
|
||
safe_xhtml_patterns = [
|
||
r'x-html="\$icon\(', # Icon helper is safe
|
||
r"x-html='\$icon\(",
|
||
r'x-html="`\$\{.*icon', # Template literal with icon
|
||
]
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
if "x-html=" in line:
|
||
# Check if it matches safe patterns
|
||
is_safe = any(re.search(pattern, line) for pattern in safe_xhtml_patterns)
|
||
|
||
if not is_safe:
|
||
# Check for potentially unsafe user content
|
||
unsafe_indicators = [
|
||
".description", ".content", ".body", ".text",
|
||
".message", ".comment", ".review", ".html"
|
||
]
|
||
for indicator in unsafe_indicators:
|
||
if indicator in line:
|
||
self._add_violation(
|
||
rule_id="TPL-005",
|
||
rule_name="Use x-html ONLY for safe content",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="x-html used with potentially unsafe user content",
|
||
context=line.strip()[:60],
|
||
suggestion="Sanitize HTML content or use x-text for plain text",
|
||
)
|
||
return
|
||
|
||
def _check_template_loading_state(self, file_path: Path, content: str, lines: list[str]):
|
||
"""TPL-006: Check that templates with data loads show loading state"""
|
||
if "noqa: tpl-006" in content.lower():
|
||
return
|
||
|
||
# Check if template has data loading (Alpine init, fetch, apiClient)
|
||
has_data_loading = any([
|
||
"x-init=" in content,
|
||
"@load=" in content,
|
||
"apiClient" in content,
|
||
"loadData" in content,
|
||
"fetchData" in content,
|
||
])
|
||
|
||
if not has_data_loading:
|
||
return
|
||
|
||
# Check for loading state display
|
||
has_loading_state = any([
|
||
'x-show="loading"' in content,
|
||
"x-show='loading'" in content,
|
||
'x-if="loading"' in content,
|
||
"x-if='loading'" in content,
|
||
'loading_state' in content,
|
||
'Loading...' in content,
|
||
'spinner' in content.lower(),
|
||
])
|
||
|
||
if not has_loading_state:
|
||
self._add_violation(
|
||
rule_id="TPL-006",
|
||
rule_name="Implement loading state for data loads",
|
||
severity=Severity.INFO,
|
||
file_path=file_path,
|
||
line_number=1,
|
||
message="Template loads data but has no visible loading state",
|
||
context=file_path.name,
|
||
suggestion='Add <div x-show="loading">Loading...</div> or use loading_state macro',
|
||
)
|
||
|
||
def _check_template_empty_state(self, file_path: Path, content: str, lines: list[str]):
|
||
"""TPL-007: Check that templates with lists show empty state"""
|
||
if "noqa: tpl-007" in content.lower():
|
||
return
|
||
|
||
# Check if template has list iteration
|
||
has_list = any([
|
||
"x-for=" in content,
|
||
"{% for " in content,
|
||
])
|
||
|
||
if not has_list:
|
||
return
|
||
|
||
# Check for empty state handling
|
||
has_empty_state = any([
|
||
".length === 0" in content,
|
||
".length == 0" in content,
|
||
"items.length" in content,
|
||
"empty_state" in content,
|
||
"No items" in content,
|
||
"No results" in content,
|
||
"No data" in content,
|
||
"table_empty_state" in content,
|
||
])
|
||
|
||
if not has_empty_state:
|
||
self._add_violation(
|
||
rule_id="TPL-007",
|
||
rule_name="Implement empty state when no data",
|
||
severity=Severity.INFO,
|
||
file_path=file_path,
|
||
line_number=1,
|
||
message="Template has list but no empty state handling",
|
||
context=file_path.name,
|
||
suggestion='Add <template x-if="items.length === 0">No items found</template>',
|
||
)
|
||
|
||
def _check_pagination_macro_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""FE-001: Check for inline pagination that should use macro"""
|
||
# Check if already using the pagination macro
|
||
uses_macro = any("from 'shared/macros/pagination.html'" in line for line in lines)
|
||
if uses_macro:
|
||
return
|
||
|
||
# Check for noqa: FE-001 comment
|
||
has_noqa = any("noqa: fe-001" in line.lower() for line in lines)
|
||
if has_noqa:
|
||
return
|
||
|
||
# Look for signs of inline pagination
|
||
pagination_indicators = [
|
||
('aria-label="Table navigation"', "Inline table navigation found"),
|
||
("previousPage()" , "Inline pagination controls found"),
|
||
("nextPage()" , "Inline pagination controls found"),
|
||
("goToPage(" , "Inline pagination controls found"),
|
||
]
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
for pattern, message in pagination_indicators:
|
||
if pattern in line:
|
||
# Skip if it's in a comment
|
||
stripped = line.strip()
|
||
if stripped.startswith("{#") or stripped.startswith("<!--"):
|
||
continue
|
||
|
||
self._add_violation(
|
||
rule_id="FE-001",
|
||
rule_name="Use pagination macro",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message=message + " - use shared macro instead",
|
||
context=stripped[:60],
|
||
suggestion="{% from 'shared/macros/pagination.html' import pagination %}\n{{ pagination() }}",
|
||
)
|
||
return # Only report once per file
|
||
|
||
def _check_icon_helper_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""FE-002: Check for inline SVGs that should use $icon() helper"""
|
||
# Check for noqa: FE-002 comment
|
||
has_noqa = any("noqa: fe-002" in line.lower() for line in lines)
|
||
if has_noqa:
|
||
return
|
||
|
||
# Pattern to find inline SVGs
|
||
svg_pattern = re.compile(r'<svg[^>]*viewBox[^>]*>.*?</svg>', re.DOTALL | re.IGNORECASE)
|
||
|
||
# Find all SVG occurrences
|
||
for match in svg_pattern.finditer(content):
|
||
# Find line number
|
||
line_num = content[:match.start()].count('\n') + 1
|
||
|
||
# Skip if this is likely in a code example (inside <pre> or <code>)
|
||
context_before = content[max(0, match.start()-200):match.start()]
|
||
if '<pre' in context_before or '<code' in context_before:
|
||
continue
|
||
|
||
self._add_violation(
|
||
rule_id="FE-002",
|
||
rule_name="Use $icon() helper",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=line_num,
|
||
message="Inline SVG found - use $icon() helper for consistency",
|
||
context="<svg...>",
|
||
suggestion='<span x-html="$icon(\'icon-name\', \'w-4 h-4\')"></span>',
|
||
)
|
||
|
||
def _check_alerts_macro_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""FE-003: Check for inline loading/error states that should use alerts macro"""
|
||
# Check if already using the alerts macro
|
||
uses_macro = any("from 'shared/macros/alerts.html'" in line for line in lines)
|
||
if uses_macro:
|
||
return
|
||
|
||
# Check for noqa comment
|
||
has_noqa = any("noqa: fe-003" in line.lower() for line in lines)
|
||
if has_noqa:
|
||
return
|
||
|
||
# Look for inline loading states
|
||
for i, line in enumerate(lines, 1):
|
||
# Loading state pattern: text-center py-12 with loading content
|
||
if 'x-show="loading"' in line or "x-show='loading'" in line:
|
||
# Check if next few lines have spinner pattern
|
||
context_lines = "\n".join(lines[i-1:i+3])
|
||
if "text-center" in context_lines and "py-12" in context_lines:
|
||
self._add_violation(
|
||
rule_id="FE-003",
|
||
rule_name="Use alerts macro",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Inline loading state found - use loading_state macro",
|
||
context=line.strip()[:60],
|
||
suggestion="{% from 'shared/macros/alerts.html' import loading_state %}\n{{ loading_state('Loading...') }}",
|
||
)
|
||
return # Only report once per file
|
||
|
||
# Error state pattern: bg-red-100 border-red-400
|
||
if "bg-red-100" in line and "border-red-400" in line:
|
||
self._add_violation(
|
||
rule_id="FE-003",
|
||
rule_name="Use alerts macro",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Inline error state found - use error_state macro",
|
||
context=line.strip()[:60],
|
||
suggestion="{% from 'shared/macros/alerts.html' import error_state %}\n{{ error_state('Error', 'error') }}",
|
||
)
|
||
return
|
||
|
||
def _check_modals_macro_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""FE-004: Check for inline modals that should use modals macro"""
|
||
# Check if already using the modals macro
|
||
uses_macro = any("from 'shared/macros/modals.html'" in line for line in lines)
|
||
if uses_macro:
|
||
return
|
||
|
||
# Check for noqa comment
|
||
has_noqa = any("noqa: fe-004" in line.lower() for line in lines)
|
||
if has_noqa:
|
||
return
|
||
|
||
# Look for modal patterns: fixed inset-0 with role="dialog" or modal backdrop
|
||
for i, line in enumerate(lines, 1):
|
||
if "fixed inset-0" in line and ("z-50" in line or "z-30" in line or "z-40" in line):
|
||
# Check context for modal indicators
|
||
context_lines = "\n".join(lines[max(0, i-1):min(len(lines), i+5)])
|
||
if 'role="dialog"' in context_lines or "bg-opacity-50" in context_lines or "bg-black/50" in context_lines:
|
||
self._add_violation(
|
||
rule_id="FE-004",
|
||
rule_name="Use modals macro",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Inline modal found - use modal macro for consistency",
|
||
context=line.strip()[:60],
|
||
suggestion="{% from 'shared/macros/modals.html' import modal %}\n{% call modal('myModal', 'Title', 'isModalOpen') %}...{% endcall %}",
|
||
)
|
||
return
|
||
|
||
def _check_tables_macro_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""FE-005: Check for inline table wrappers that should use tables macro"""
|
||
# Check if already using the tables macro
|
||
uses_macro = any("from 'shared/macros/tables.html'" in line for line in lines)
|
||
if uses_macro:
|
||
return
|
||
|
||
# Check for noqa comment
|
||
has_noqa = any("noqa: fe-005" in line.lower() for line in lines)
|
||
if has_noqa:
|
||
return
|
||
|
||
# Look for table wrapper pattern: overflow-hidden rounded-lg shadow-xs
|
||
for i, line in enumerate(lines, 1):
|
||
if "overflow-hidden" in line and "rounded-lg" in line and "shadow-xs" in line:
|
||
# Check if there's a table nearby
|
||
context_lines = "\n".join(lines[i-1:min(len(lines), i+10)])
|
||
if "<table" in context_lines:
|
||
self._add_violation(
|
||
rule_id="FE-005",
|
||
rule_name="Use tables macro",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Inline table wrapper found - use table_wrapper macro",
|
||
context=line.strip()[:60],
|
||
suggestion="{% from 'shared/macros/tables.html' import table_wrapper, table_header %}\n{% call table_wrapper() %}...{% endcall %}",
|
||
)
|
||
return
|
||
|
||
def _check_dropdowns_macro_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""FE-006: Check for inline dropdowns that should use dropdowns macro"""
|
||
# Check if already using the dropdowns macro
|
||
uses_macro = any("from 'shared/macros/dropdowns.html'" in line for line in lines)
|
||
if uses_macro:
|
||
return
|
||
|
||
# Check for noqa comment
|
||
has_noqa = any("noqa: fe-006" in line.lower() for line in lines)
|
||
if has_noqa:
|
||
return
|
||
|
||
# Look for dropdown patterns: @click.outside or @click.away with menu positioning
|
||
for i, line in enumerate(lines, 1):
|
||
# Match @click.away="something = false" or @click.outside="..."
|
||
if "@click.away=" in line or "@click.outside=" in line:
|
||
# Skip if this is a modal (modals also use @click.away but have different structure)
|
||
context_lines = "\n".join(lines[max(0, i-5):min(len(lines), i+10)])
|
||
|
||
# Skip if it looks like a modal (fixed inset-0)
|
||
if "fixed inset-0" in context_lines:
|
||
continue
|
||
|
||
# Look for dropdown menu styling: absolute positioning with z-index
|
||
if "absolute" in context_lines and ("z-10" in context_lines or "z-20" in context_lines or "z-50" in context_lines):
|
||
# Additional indicators: shadow, border, bg-white/dark, rounded
|
||
if "shadow" in context_lines or "border" in context_lines:
|
||
self._add_violation(
|
||
rule_id="FE-006",
|
||
rule_name="Use dropdowns macro",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Inline dropdown menu found - use dropdown macro",
|
||
context=line.strip()[:60],
|
||
suggestion="{% from 'shared/macros/dropdowns.html' import dropdown, dropdown_item %}\n{% call dropdown('Label', 'isOpen') %}...{% endcall %}",
|
||
)
|
||
return
|
||
|
||
def _check_headers_macro_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""FE-007: Check for inline page headers that should use headers macro"""
|
||
# Check if already using the headers macro
|
||
uses_macro = any("from 'shared/macros/headers.html'" in line for line in lines)
|
||
if uses_macro:
|
||
return
|
||
|
||
# Check for noqa comment
|
||
has_noqa = any("noqa: fe-007" in line.lower() for line in lines)
|
||
if has_noqa:
|
||
return
|
||
|
||
# Look for page header pattern: flex with h2 text-2xl font-semibold
|
||
for i, line in enumerate(lines, 1):
|
||
if '<h2' in line and 'text-2xl' in line and 'font-semibold' in line:
|
||
# Check context for typical page header pattern
|
||
context_before = "\n".join(lines[max(0, i-3):i])
|
||
if "flex" in context_before and ("justify-between" in context_before or "items-center" in context_before):
|
||
self._add_violation(
|
||
rule_id="FE-007",
|
||
rule_name="Use headers macro",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Inline page header found - use page_header macro",
|
||
context=line.strip()[:60],
|
||
suggestion="{% from 'shared/macros/headers.html' import page_header %}\n{{ page_header('Title', action_label='Create', action_url='/create') }}",
|
||
)
|
||
return
|
||
|
||
def _check_number_stepper_macro_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""FE-008: Check for raw number inputs that should use number_stepper macro
|
||
|
||
Detects <input type="number"> that should use the number_stepper macro for
|
||
consistent styling and dark mode support.
|
||
|
||
Exceptions:
|
||
- ID fields (placeholder contains 'id' or 'ID')
|
||
- Files already importing number_stepper
|
||
- Lines with noqa: FE-008 comment
|
||
"""
|
||
# Check if already using the number_stepper macro
|
||
uses_macro = any("number_stepper" in line for line in lines)
|
||
if uses_macro:
|
||
return
|
||
|
||
# Check for file-level noqa comment
|
||
has_noqa = any("noqa: fe-008" in line.lower() for line in lines)
|
||
if has_noqa:
|
||
return
|
||
|
||
# Look for raw number inputs
|
||
for i, line in enumerate(lines, 1):
|
||
if 'type="number"' in line or "type='number'" in line:
|
||
# Skip if line has noqa comment
|
||
if "noqa" in line.lower():
|
||
continue
|
||
|
||
# Skip if it looks like an ID field (check surrounding lines for context)
|
||
context_lines = "\n".join(lines[max(0, i-3):min(len(lines), i+2)]).lower()
|
||
if "user id" in context_lines or "placeholder" in context_lines and "id" in context_lines:
|
||
continue
|
||
|
||
# Skip if it's in a comment
|
||
stripped = line.strip()
|
||
if stripped.startswith("{#") or stripped.startswith("<!--") or stripped.startswith("//"):
|
||
continue
|
||
|
||
self._add_violation(
|
||
rule_id="FE-008",
|
||
rule_name="Use number_stepper macro",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Raw number input found - use number_stepper macro for consistent styling",
|
||
context=stripped[:70],
|
||
suggestion="{% from 'shared/macros/inputs.html' import number_stepper %}\n{{ number_stepper(model='fieldName', min=1, max=100) }}\nOr add {# noqa: FE-008 #} if this is intentional (e.g., ID field)",
|
||
)
|
||
return # Only report once per file
|
||
|
||
def _validate_api_endpoints(self, target_path: Path):
|
||
"""Validate API endpoint rules (API-001, API-002, API-003, API-004)"""
|
||
print("📡 Validating API endpoints...")
|
||
|
||
api_files = list(target_path.glob("app/api/v1/**/*.py"))
|
||
self.result.files_checked += len(api_files)
|
||
|
||
for file_path in api_files:
|
||
if self._should_ignore_file(file_path):
|
||
continue
|
||
|
||
content = file_path.read_text()
|
||
lines = content.split("\n")
|
||
|
||
# API-001: Check for Pydantic model usage
|
||
self._check_pydantic_usage(file_path, content, lines)
|
||
|
||
# API-002: Check for business logic in endpoints
|
||
self._check_no_business_logic_in_endpoints(file_path, content, lines)
|
||
|
||
# API-003: Check exception handling
|
||
self._check_endpoint_exception_handling(file_path, content, lines)
|
||
|
||
# API-004: Check authentication
|
||
self._check_endpoint_authentication(file_path, content, lines)
|
||
|
||
# API-005: Check vendor_id scoping for vendor/shop endpoints
|
||
self._check_vendor_scoping(file_path, content, lines)
|
||
|
||
def _check_pydantic_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""API-001: Ensure endpoints use Pydantic models"""
|
||
rule = self._get_rule("API-001")
|
||
if not rule:
|
||
return
|
||
|
||
# Check for response_model in route decorators
|
||
route_pattern = r"@router\.(get|post|put|delete|patch)"
|
||
dict_return_pattern = r"return\s+\{.*\}"
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
# Check for dict returns in endpoints
|
||
if re.search(route_pattern, line):
|
||
# Look ahead for function body
|
||
func_start = i
|
||
indent = len(line) - len(line.lstrip())
|
||
|
||
# Find function body
|
||
for j in range(func_start, min(func_start + 20, len(lines))):
|
||
if j >= len(lines):
|
||
break
|
||
|
||
func_line = lines[j]
|
||
if re.search(dict_return_pattern, func_line):
|
||
self._add_violation(
|
||
rule_id="API-001",
|
||
rule_name=rule["name"],
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=j + 1,
|
||
message="Endpoint returns raw dict instead of Pydantic model",
|
||
context=func_line.strip(),
|
||
suggestion="Define a Pydantic response model and use response_model parameter",
|
||
)
|
||
|
||
def _check_no_business_logic_in_endpoints(
|
||
self, file_path: Path, content: str, lines: list[str]
|
||
):
|
||
"""API-002: Ensure no business logic in endpoints"""
|
||
rule = self._get_rule("API-002")
|
||
if not rule:
|
||
return
|
||
|
||
# NOTE: db.commit() is intentionally NOT included here
|
||
# Transaction control (commit) is allowed at endpoint level
|
||
# Only business logic operations are flagged
|
||
anti_patterns = [
|
||
(r"db\.add\(", "Creating entities should be in service layer"),
|
||
(r"db\.delete\(", "Deleting entities should be in service layer"),
|
||
(r"db\.query\(", "Database queries should be in service layer"),
|
||
(r"db\.execute\(", "Database operations should be in service layer"),
|
||
]
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
# Skip service method calls (allowed)
|
||
if "_service." in line or "service." in line:
|
||
continue
|
||
|
||
for pattern, message in anti_patterns:
|
||
if re.search(pattern, line):
|
||
self._add_violation(
|
||
rule_id="API-002",
|
||
rule_name=rule["name"],
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message=message,
|
||
context=line.strip(),
|
||
suggestion="Move database operations to service layer",
|
||
)
|
||
|
||
def _check_endpoint_exception_handling(
|
||
self, file_path: Path, content: str, lines: list[str]
|
||
):
|
||
"""API-003: Check that endpoints do NOT raise exceptions directly.
|
||
|
||
The architecture uses:
|
||
- Dependencies (deps.py) for authentication/authorization validation
|
||
- Services for business logic validation
|
||
- Global exception handler that catches WizamartException subclasses
|
||
|
||
Endpoints should be a thin orchestration layer that trusts dependencies
|
||
and services to handle all validation. They should NOT raise exceptions.
|
||
"""
|
||
rule = self._get_rule("API-003")
|
||
if not rule:
|
||
return
|
||
|
||
# Skip exception handler file and deps.py - they're allowed to raise exceptions
|
||
file_path_str = str(file_path)
|
||
if "exceptions/handler.py" in file_path_str or file_path_str.endswith("deps.py"):
|
||
return
|
||
|
||
# Patterns that indicate endpoints are raising exceptions (BAD)
|
||
exception_patterns = [
|
||
("raise HTTPException", "Endpoint raises HTTPException directly"),
|
||
("raise InvalidTokenException", "Endpoint raises InvalidTokenException - move to dependency"),
|
||
("raise InsufficientPermissionsException", "Endpoint raises permission exception - move to dependency"),
|
||
("raise UnauthorizedVendorAccessException", "Endpoint raises auth exception - move to dependency or service"),
|
||
]
|
||
|
||
# Pattern that indicates redundant validation (BAD)
|
||
redundant_patterns = [
|
||
(r"if not hasattr\(current_user.*token_vendor", "Redundant token_vendor check - get_current_vendor_api guarantees this"),
|
||
(r"if not hasattr\(current_user.*token_vendor_id", "Redundant token_vendor_id check - dependency guarantees this"),
|
||
]
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
# Skip comments
|
||
stripped = line.strip()
|
||
if stripped.startswith("#"):
|
||
continue
|
||
|
||
# Check for direct exception raising
|
||
for pattern, message in exception_patterns:
|
||
if pattern in line:
|
||
self._add_violation(
|
||
rule_id="API-003",
|
||
rule_name=rule["name"],
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message=message,
|
||
context=stripped[:80],
|
||
suggestion="Let dependencies or services handle validation and raise exceptions",
|
||
)
|
||
|
||
# Check for redundant validation patterns
|
||
for pattern, message in redundant_patterns:
|
||
if re.search(pattern, line):
|
||
self._add_violation(
|
||
rule_id="API-003",
|
||
rule_name=rule["name"],
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message=message,
|
||
context=stripped[:80],
|
||
suggestion="Remove redundant check - auth dependency guarantees this attribute is present",
|
||
)
|
||
|
||
def _check_endpoint_authentication(
|
||
self, file_path: Path, content: str, lines: list[str]
|
||
):
|
||
"""API-004: Check authentication on endpoints
|
||
|
||
Automatically skips:
|
||
- Auth endpoint files (*/auth.py) - login/logout are intentionally public
|
||
- Endpoints marked with '# public' comment
|
||
"""
|
||
rule = self._get_rule("API-004")
|
||
if not rule:
|
||
return
|
||
|
||
# Skip auth endpoint files entirely - they are intentionally public
|
||
file_path_str = str(file_path)
|
||
if file_path_str.endswith("/auth.py") or file_path_str.endswith("\\auth.py"):
|
||
return
|
||
|
||
# This is a warning-level check
|
||
# Look for endpoints without proper authentication
|
||
# Valid auth patterns:
|
||
# - Depends(get_current_*) - direct user authentication
|
||
# - Depends(require_vendor_*) - vendor permission dependencies
|
||
# - Depends(require_any_vendor_*) - any permission check
|
||
# - Depends(require_all_vendor*) - all permissions check
|
||
# - Depends(get_user_permissions) - permission fetching
|
||
auth_patterns = [
|
||
"Depends(get_current_",
|
||
"Depends(require_vendor_",
|
||
"Depends(require_any_vendor_",
|
||
"Depends(require_all_vendor",
|
||
"Depends(get_user_permissions",
|
||
]
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
if "@router." in line and (
|
||
"post" in line or "put" in line or "delete" in line
|
||
):
|
||
# Check next 15 lines for auth or public marker
|
||
# (increased from 5 to handle multi-line decorators and long function signatures)
|
||
has_auth = False
|
||
is_public = False
|
||
context_lines = lines[i - 1 : i + 15] # Include line before decorator
|
||
|
||
for ctx_line in context_lines:
|
||
# Check for any valid auth pattern
|
||
if any(pattern in ctx_line for pattern in auth_patterns):
|
||
has_auth = True
|
||
break
|
||
# Check for public endpoint markers
|
||
if "# public" in ctx_line.lower() or "# noqa: api-004" in ctx_line.lower():
|
||
is_public = True
|
||
break
|
||
|
||
if not has_auth and not is_public and "include_in_schema=False" not in " ".join(
|
||
lines[i : i + 15]
|
||
):
|
||
# Determine appropriate suggestion based on file path
|
||
file_path_str = str(file_path)
|
||
if "/vendor/" in file_path_str:
|
||
suggestion = "Add Depends(get_current_vendor_api) or permission dependency, or mark as '# public'"
|
||
elif "/admin/" in file_path_str:
|
||
suggestion = "Add Depends(get_current_admin_api), or mark as '# public'"
|
||
elif "/shop/" in file_path_str:
|
||
suggestion = "Add Depends(get_current_customer_api), or mark as '# public'"
|
||
else:
|
||
suggestion = "Add authentication dependency or mark as '# public' if intentionally unauthenticated"
|
||
|
||
self._add_violation(
|
||
rule_id="API-004",
|
||
rule_name=rule["name"],
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Endpoint may be missing authentication",
|
||
context=line.strip(),
|
||
suggestion=suggestion,
|
||
)
|
||
|
||
def _check_vendor_scoping(self, file_path: Path, content: str, lines: list[str]):
|
||
"""API-005: Check that vendor/shop endpoints scope queries to vendor_id"""
|
||
file_path_str = str(file_path)
|
||
|
||
# Only check vendor and shop API files
|
||
if "/vendor/" not in file_path_str and "/shop/" not in file_path_str:
|
||
return
|
||
|
||
# Skip auth files
|
||
if file_path_str.endswith("auth.py"):
|
||
return
|
||
|
||
# Check for noqa
|
||
if "noqa: api-005" in content.lower():
|
||
return
|
||
|
||
# Look for database queries without vendor_id filtering
|
||
# This is a heuristic check - not perfect
|
||
for i, line in enumerate(lines, 1):
|
||
# Look for .query().all() patterns without vendor filtering
|
||
if ".query(" in line and ".all()" in line:
|
||
# Check if vendor_id filter is nearby
|
||
context_start = max(0, i - 5)
|
||
context_end = min(len(lines), i + 3)
|
||
context_lines = "\n".join(lines[context_start:context_end])
|
||
|
||
if "vendor_id" not in context_lines and "token_vendor_id" not in context_lines:
|
||
self._add_violation(
|
||
rule_id="API-005",
|
||
rule_name="Multi-tenant queries must scope to vendor_id",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Query in vendor/shop endpoint may not be scoped to vendor_id",
|
||
context=line.strip()[:60],
|
||
suggestion="Add .filter(Model.vendor_id == vendor_id) to ensure tenant isolation",
|
||
)
|
||
return # Only report once per file
|
||
|
||
def _validate_service_layer(self, target_path: Path):
|
||
"""Validate service layer rules (SVC-001 to SVC-007)"""
|
||
print("🔧 Validating service layer...")
|
||
|
||
service_files = list(target_path.glob("app/services/**/*.py"))
|
||
self.result.files_checked += len(service_files)
|
||
|
||
for file_path in service_files:
|
||
if self._should_ignore_file(file_path):
|
||
continue
|
||
|
||
content = file_path.read_text()
|
||
lines = content.split("\n")
|
||
|
||
# SVC-001: No HTTPException in services
|
||
self._check_no_http_exception_in_services(file_path, content, lines)
|
||
|
||
# SVC-002: Proper exception handling
|
||
self._check_service_exceptions(file_path, content, lines)
|
||
|
||
# SVC-003: DB session as parameter
|
||
self._check_db_session_parameter(file_path, content, lines)
|
||
|
||
# SVC-005: Vendor scoping in multi-tenant services
|
||
self._check_service_vendor_scoping(file_path, content, lines)
|
||
|
||
# SVC-006: No db.commit() in services
|
||
self._check_no_commit_in_services(file_path, content, lines)
|
||
|
||
def _check_service_vendor_scoping(self, file_path: Path, content: str, lines: list[str]):
|
||
"""SVC-005: Check that service queries are scoped to vendor_id in multi-tenant context"""
|
||
# Skip admin services that may legitimately access all vendors
|
||
file_path_str = str(file_path)
|
||
if "admin" in file_path_str.lower():
|
||
return
|
||
|
||
if "noqa: svc-005" in content.lower():
|
||
return
|
||
|
||
# Look for patterns that suggest unscoped queries
|
||
for i, line in enumerate(lines, 1):
|
||
# Check for .all() queries that might not be scoped
|
||
if ".query(" in line and ".all()" in line:
|
||
# Check context for vendor filtering
|
||
context_start = max(0, i - 5)
|
||
context_end = min(len(lines), i + 3)
|
||
context_lines = "\n".join(lines[context_start:context_end])
|
||
|
||
if "vendor_id" not in context_lines:
|
||
# Check if the method has vendor_id as parameter
|
||
method_start = self._find_method_start(lines, i)
|
||
if method_start:
|
||
method_sig = lines[method_start]
|
||
if "vendor_id" not in method_sig:
|
||
self._add_violation(
|
||
rule_id="SVC-005",
|
||
rule_name="Service must scope queries to vendor_id",
|
||
severity=Severity.INFO,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Query may not be scoped to vendor_id",
|
||
context=line.strip()[:60],
|
||
suggestion="Add vendor_id parameter and filter queries by it",
|
||
)
|
||
return
|
||
|
||
def _find_method_start(self, lines: list[str], current_line: int) -> int | None:
|
||
"""Find the start line of the method containing current_line"""
|
||
for i in range(current_line - 1, max(0, current_line - 30), -1):
|
||
if "def " in lines[i]:
|
||
return i
|
||
return None
|
||
|
||
def _check_no_http_exception_in_services(
|
||
self, file_path: Path, content: str, lines: list[str]
|
||
):
|
||
"""SVC-001: Services must not raise HTTPException"""
|
||
rule = self._get_rule("SVC-001")
|
||
if not rule:
|
||
return
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
if "raise HTTPException" in line:
|
||
self._add_violation(
|
||
rule_id="SVC-001",
|
||
rule_name=rule["name"],
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Service raises HTTPException - use domain exceptions instead",
|
||
context=line.strip(),
|
||
suggestion="Create custom exception class (e.g., VendorNotFoundError) and raise that",
|
||
)
|
||
|
||
if (
|
||
"from fastapi import HTTPException" in line
|
||
or "from fastapi.exceptions import HTTPException" in line
|
||
):
|
||
self._add_violation(
|
||
rule_id="SVC-001",
|
||
rule_name=rule["name"],
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Service imports HTTPException - services should not know about HTTP",
|
||
context=line.strip(),
|
||
suggestion="Remove HTTPException import and use domain exceptions",
|
||
)
|
||
|
||
def _check_service_exceptions(
|
||
self, file_path: Path, content: str, lines: list[str]
|
||
):
|
||
"""SVC-002: Check for proper exception handling"""
|
||
rule = self._get_rule("SVC-002")
|
||
if not rule:
|
||
return
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
# Check for generic Exception raises
|
||
if re.match(r"\s*raise Exception\(", line):
|
||
self._add_violation(
|
||
rule_id="SVC-002",
|
||
rule_name=rule["name"],
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Service raises generic Exception - use specific domain exception",
|
||
context=line.strip(),
|
||
suggestion="Create custom exception class for this error case",
|
||
)
|
||
|
||
def _check_db_session_parameter(
|
||
self, file_path: Path, content: str, lines: list[str]
|
||
):
|
||
"""SVC-003: Service methods should accept db session as parameter"""
|
||
rule = self._get_rule("SVC-003")
|
||
if not rule:
|
||
return
|
||
|
||
# Check for SessionLocal() creation in service files
|
||
for i, line in enumerate(lines, 1):
|
||
if "SessionLocal()" in line and "class" not in line:
|
||
self._add_violation(
|
||
rule_id="SVC-003",
|
||
rule_name=rule["name"],
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Service creates database session internally",
|
||
context=line.strip(),
|
||
suggestion="Accept db: Session as method parameter instead",
|
||
)
|
||
|
||
def _check_no_commit_in_services(
|
||
self, file_path: Path, content: str, lines: list[str]
|
||
):
|
||
"""SVC-006: Services should NOT call db.commit()
|
||
|
||
Transaction control belongs at the API endpoint level.
|
||
Exception: log_service.py may need immediate commits for audit logs.
|
||
"""
|
||
rule = self._get_rule("SVC-006")
|
||
if not rule:
|
||
return
|
||
|
||
# Exception: log_service.py is allowed to commit (audit logs)
|
||
if "log_service.py" in str(file_path):
|
||
return
|
||
|
||
for i, line in enumerate(lines, 1):
|
||
if "db.commit()" in line:
|
||
# Skip if it's a comment
|
||
stripped = line.strip()
|
||
if stripped.startswith("#"):
|
||
continue
|
||
|
||
self._add_violation(
|
||
rule_id="SVC-006",
|
||
rule_name=rule["name"],
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Service calls db.commit() - transaction control should be at endpoint level",
|
||
context=stripped,
|
||
suggestion="Remove db.commit() from service; let endpoint handle transaction",
|
||
)
|
||
|
||
def _validate_models(self, target_path: Path):
|
||
"""Validate model rules (MDL-001 to MDL-004)"""
|
||
print("📦 Validating models...")
|
||
|
||
# Validate database models
|
||
db_model_files = list(target_path.glob("models/database/**/*.py"))
|
||
self.result.files_checked += len(db_model_files)
|
||
|
||
for file_path in db_model_files:
|
||
if self._should_ignore_file(file_path) or file_path.name == "__init__.py":
|
||
continue
|
||
|
||
content = file_path.read_text()
|
||
lines = content.split("\n")
|
||
|
||
# MDL-001: Check for SQLAlchemy Base inheritance
|
||
self._check_sqlalchemy_base_usage(file_path, content, lines)
|
||
|
||
# MDL-004: Check table naming (singular)
|
||
self._check_table_naming(file_path, content)
|
||
|
||
# Validate schema models
|
||
schema_model_files = list(target_path.glob("models/schema/**/*.py"))
|
||
self.result.files_checked += len(schema_model_files)
|
||
|
||
for file_path in schema_model_files:
|
||
if self._should_ignore_file(file_path) or file_path.name == "__init__.py":
|
||
continue
|
||
|
||
content = file_path.read_text()
|
||
lines = content.split("\n")
|
||
|
||
# MDL-002: Check for mixing SQLAlchemy and Pydantic
|
||
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",
|
||
)
|
||
|
||
# MDL-003: Check for from_attributes in Pydantic response models
|
||
self._check_pydantic_from_attributes(file_path, content)
|
||
|
||
def _check_sqlalchemy_base_usage(self, file_path: Path, content: str, lines: list[str]):
|
||
"""MDL-001: Check that database models inherit from SQLAlchemy Base"""
|
||
# Look for class definitions
|
||
class_pattern = re.compile(r"class\s+(\w+)\s*\(([^)]+)\):")
|
||
|
||
for match in class_pattern.finditer(content):
|
||
class_name = match.group(1)
|
||
base_classes = match.group(2)
|
||
line_num = content[:match.start()].count('\n') + 1
|
||
|
||
# Skip non-model classes (mixins, helpers, etc.)
|
||
if class_name.endswith("Mixin") or class_name.startswith("_"):
|
||
continue
|
||
|
||
# Skip Enum classes - they inherit from (str, Enum) or (Enum)
|
||
if "Enum" in base_classes:
|
||
continue
|
||
|
||
# Check if it inherits from Base
|
||
if "Base" not in base_classes and "db.Model" not in base_classes:
|
||
# Check if it might be a model - MUST have __tablename__ in its own class body
|
||
class_body_start = match.end()
|
||
# Find the end of this class body (next class definition at same indentation or EOF)
|
||
next_class_match = class_pattern.search(content, class_body_start)
|
||
if next_class_match:
|
||
class_body = content[class_body_start:next_class_match.start()]
|
||
else:
|
||
class_body = content[class_body_start:]
|
||
|
||
# Only flag if this specific class has __tablename__ (definite model indicator)
|
||
if "__tablename__" in class_body:
|
||
self._add_violation(
|
||
rule_id="MDL-001",
|
||
rule_name="Database models must use SQLAlchemy Base",
|
||
severity=Severity.ERROR,
|
||
file_path=file_path,
|
||
line_number=line_num,
|
||
message=f"Model '{class_name}' appears to be a database model but doesn't inherit from Base",
|
||
context=f"class {class_name}({base_classes})",
|
||
suggestion="Inherit from Base: class MyModel(Base):",
|
||
)
|
||
|
||
def _check_table_naming(self, file_path: Path, content: str):
|
||
"""MDL-004: Check that table names are plural (industry standard)
|
||
|
||
Following Rails, Django, Laravel conventions - tables represent collections
|
||
of entities so should use plural names: users, orders, products.
|
||
"""
|
||
# Look for __tablename__ definitions
|
||
tablename_pattern = re.compile(r'__tablename__\s*=\s*["\'](\w+)["\']')
|
||
|
||
for match in tablename_pattern.finditer(content):
|
||
table_name = match.group(1)
|
||
line_num = content[:match.start()].count('\n') + 1
|
||
|
||
# Common singular names that should be plural
|
||
singular_indicators = {
|
||
"vendor": "vendors",
|
||
"product": "products",
|
||
"order": "orders",
|
||
"user": "users",
|
||
"customer": "customers",
|
||
"category": "categories",
|
||
"payment": "payments",
|
||
"setting": "settings",
|
||
"item": "items",
|
||
"image": "images",
|
||
"role": "roles",
|
||
"company": "companies",
|
||
}
|
||
|
||
if table_name in singular_indicators:
|
||
plural = singular_indicators[table_name]
|
||
self._add_violation(
|
||
rule_id="MDL-004",
|
||
rule_name="Database tables use plural names",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=line_num,
|
||
message=f"Table name '{table_name}' should be plural (industry standard)",
|
||
context=f'__tablename__ = "{table_name}"',
|
||
suggestion=f'Use __tablename__ = "{plural}"',
|
||
)
|
||
|
||
def _check_pydantic_from_attributes(self, file_path: Path, content: str):
|
||
"""MDL-003: Check that Pydantic response models have from_attributes"""
|
||
# Look for response model classes that likely represent ORM entities
|
||
# Skip stats/aggregate models which use plain dicts
|
||
response_pattern = re.compile(
|
||
r"class\s+(\w*Response\w*|\w*Out\w*|\w*Read\w*)\s*\(([^)]+)\):"
|
||
)
|
||
|
||
for match in response_pattern.finditer(content):
|
||
class_name = match.group(1)
|
||
base_classes = match.group(2)
|
||
class_start = match.end()
|
||
line_num = content[:match.start()].count('\n') + 1
|
||
|
||
# Skip stats/aggregate response models - they use plain dicts, not ORM objects
|
||
stats_patterns = ["Stats", "Analytics", "Dashboard", "Summary", "Count", "Total"]
|
||
if any(p in class_name for p in stats_patterns):
|
||
continue
|
||
|
||
# Get class body - find the next class definition to properly scope
|
||
next_class = re.search(r"\nclass\s+\w+", content[class_start:])
|
||
if next_class:
|
||
class_body = content[class_start:class_start + next_class.start()]
|
||
else:
|
||
class_body = content[class_start:class_start + 800]
|
||
|
||
# Check for from_attributes in Config or model_config
|
||
# Supports multiple formats:
|
||
# - model_config = {"from_attributes": True}
|
||
# - model_config = ConfigDict(from_attributes=True)
|
||
# - class Config: from_attributes = True
|
||
# - orm_mode = True (Pydantic v1)
|
||
has_from_attributes = any([
|
||
"from_attributes" in class_body and "True" in class_body,
|
||
"orm_mode" in class_body and "True" in class_body,
|
||
])
|
||
|
||
if not has_from_attributes:
|
||
# Only flag if it looks like an ORM entity response:
|
||
# - Has an 'id: int' field as a direct attribute (typical ORM primary key)
|
||
# - AND has at least one other typical ORM field (created_at, updated_at, etc.)
|
||
# Skip wrapper/list responses that just contain other models
|
||
has_id_field = re.search(r"^\s+id\s*:\s*int", class_body, re.MULTILINE)
|
||
has_timestamp = "created_at:" in class_body or "updated_at:" in class_body
|
||
|
||
# Skip list/wrapper responses (contain 'list[' or just have message/total fields)
|
||
is_wrapper = any([
|
||
": list[" in class_body and "id:" not in class_body.split(": list[")[0],
|
||
re.search(r"^\s+items\s*:", class_body, re.MULTILINE),
|
||
re.search(r"^\s+total\s*:", class_body, re.MULTILINE) and not has_id_field,
|
||
])
|
||
|
||
# Only flag if it has id AND timestamps, or id and multiple other ORM-like fields
|
||
looks_like_orm = has_id_field and (has_timestamp or (
|
||
# Has multiple fields suggesting ORM entity
|
||
sum([
|
||
"vendor_id:" in class_body,
|
||
"user_id:" in class_body,
|
||
"is_active:" in class_body,
|
||
"email:" in class_body,
|
||
]) >= 2
|
||
))
|
||
|
||
if "BaseModel" in base_classes and looks_like_orm and not is_wrapper:
|
||
self._add_violation(
|
||
rule_id="MDL-003",
|
||
rule_name="Pydantic response models must use from_attributes",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=line_num,
|
||
message=f"Response model '{class_name}' may need from_attributes for ORM compatibility",
|
||
context=f"class {class_name}({base_classes})",
|
||
suggestion="Add 'model_config = ConfigDict(from_attributes=True)' or 'class Config: from_attributes = True'",
|
||
)
|
||
|
||
def _validate_exceptions(self, target_path: Path):
|
||
"""Validate exception handling patterns"""
|
||
print("⚠️ Validating exception handling...")
|
||
|
||
py_files = list(target_path.glob("**/*.py"))
|
||
|
||
for file_path in py_files:
|
||
if self._should_ignore_file(file_path):
|
||
continue
|
||
|
||
content = file_path.read_text()
|
||
lines = content.split("\n")
|
||
|
||
# EXC-002: Check for bare except
|
||
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:",
|
||
)
|
||
|
||
# EXC-004: Check exception inheritance in exceptions module
|
||
exception_files = list(target_path.glob("app/exceptions/**/*.py"))
|
||
for file_path in exception_files:
|
||
if "__init__" in file_path.name or "handler" in file_path.name:
|
||
continue
|
||
content = file_path.read_text()
|
||
self._check_exception_inheritance(file_path, content)
|
||
|
||
# EXC-005: Check exception handler registration in main.py
|
||
main_py = target_path / "app" / "main.py"
|
||
if main_py.exists():
|
||
content = main_py.read_text()
|
||
if "setup_exception_handlers" not in content and "exception_handler" not in content:
|
||
self._add_violation(
|
||
rule_id="EXC-005",
|
||
rule_name="Exception handler must be registered",
|
||
severity=Severity.ERROR,
|
||
file_path=main_py,
|
||
line_number=1,
|
||
message="No exception handler registration found in main.py",
|
||
context="app/main.py",
|
||
suggestion="Add setup_exception_handlers(app) or register exception handlers",
|
||
)
|
||
|
||
def _check_exception_inheritance(self, file_path: Path, content: str):
|
||
"""EXC-004: Check that custom exceptions inherit from WizamartException"""
|
||
# Look for class definitions that look like exceptions
|
||
exception_pattern = re.compile(r"class\s+(\w+Exception|\w+Error)\s*\(([^)]+)\)")
|
||
|
||
for match in exception_pattern.finditer(content):
|
||
class_name = match.group(1)
|
||
base_classes = match.group(2)
|
||
line_num = content[:match.start()].count('\n') + 1
|
||
|
||
# Check if it inherits from appropriate base
|
||
valid_bases = [
|
||
"WizamartException", "ResourceNotFoundException",
|
||
"ValidationException", "AuthenticationException",
|
||
"AuthorizationException", "ConflictException",
|
||
"Exception", # Allow base Exception for some cases
|
||
]
|
||
|
||
has_valid_base = any(base in base_classes for base in valid_bases)
|
||
if not has_valid_base:
|
||
self._add_violation(
|
||
rule_id="EXC-004",
|
||
rule_name="Domain exceptions must inherit from WizamartException",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=line_num,
|
||
message=f"Exception {class_name} should inherit from WizamartException hierarchy",
|
||
context=f"class {class_name}({base_classes})",
|
||
suggestion="Inherit from WizamartException or one of its subclasses",
|
||
)
|
||
|
||
def _validate_naming_conventions(self, target_path: Path):
|
||
"""Validate naming convention rules (NAM-001 to NAM-005)"""
|
||
print("📛 Validating naming conventions...")
|
||
|
||
# NAM-001: API files use PLURAL names
|
||
api_files = list(target_path.glob("app/api/v1/**/*.py"))
|
||
for file_path in api_files:
|
||
if file_path.name in ["__init__.py", "auth.py", "health.py"]:
|
||
continue
|
||
self._check_api_file_naming(file_path)
|
||
|
||
# NAM-002: Service files use SINGULAR + 'service' suffix
|
||
service_files = list(target_path.glob("app/services/**/*.py"))
|
||
for file_path in service_files:
|
||
if file_path.name == "__init__.py":
|
||
continue
|
||
self._check_service_file_naming(file_path)
|
||
|
||
# NAM-003: Model files use SINGULAR names
|
||
model_files = list(target_path.glob("models/**/*.py"))
|
||
for file_path in model_files:
|
||
if file_path.name == "__init__.py":
|
||
continue
|
||
self._check_model_file_naming(file_path)
|
||
|
||
# NAM-004 & NAM-005: Check terminology consistency
|
||
py_files = list(target_path.glob("app/**/*.py"))
|
||
for file_path in py_files:
|
||
if self._should_ignore_file(file_path):
|
||
continue
|
||
content = file_path.read_text()
|
||
self._check_terminology_consistency(file_path, content)
|
||
|
||
def _check_api_file_naming(self, file_path: Path):
|
||
"""NAM-001: Check that API files use plural names"""
|
||
name = file_path.stem
|
||
|
||
# Common singular forms that should be plural
|
||
singular_to_plural = {
|
||
"vendor": "vendors",
|
||
"product": "products",
|
||
"order": "orders",
|
||
"user": "users",
|
||
"category": "categories",
|
||
"import": "imports",
|
||
"export": "exports",
|
||
"customer": "customers",
|
||
"cart": "carts",
|
||
"payment": "payments",
|
||
"setting": "settings",
|
||
}
|
||
|
||
if name in singular_to_plural:
|
||
self._add_violation(
|
||
rule_id="NAM-001",
|
||
rule_name="API files use PLURAL names",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=1,
|
||
message=f"API file '{name}.py' should use plural form",
|
||
context=file_path.name,
|
||
suggestion=f"Rename to '{singular_to_plural[name]}.py'",
|
||
)
|
||
|
||
def _check_service_file_naming(self, file_path: Path):
|
||
"""NAM-002: Check that service files use singular + _service suffix"""
|
||
name = file_path.stem
|
||
|
||
# Check for _service suffix
|
||
if not name.endswith("_service"):
|
||
self._add_violation(
|
||
rule_id="NAM-002",
|
||
rule_name="Service files use SINGULAR + 'service' suffix",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=1,
|
||
message=f"Service file '{name}.py' should end with '_service'",
|
||
context=file_path.name,
|
||
suggestion=f"Rename to '{name}_service.py' or ensure it's a service file",
|
||
)
|
||
return
|
||
|
||
# Check if the base name is plural (should be singular)
|
||
base_name = name.replace("_service", "")
|
||
plurals = ["vendors", "products", "orders", "users", "categories", "customers", "payments"]
|
||
if base_name in plurals:
|
||
singular = base_name.rstrip("s")
|
||
if base_name == "categories":
|
||
singular = "category"
|
||
self._add_violation(
|
||
rule_id="NAM-002",
|
||
rule_name="Service files use SINGULAR + 'service' suffix",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=1,
|
||
message=f"Service file '{name}.py' should use singular form",
|
||
context=file_path.name,
|
||
suggestion=f"Rename to '{singular}_service.py'",
|
||
)
|
||
|
||
def _check_model_file_naming(self, file_path: Path):
|
||
"""NAM-003: Check that model files use singular names"""
|
||
name = file_path.stem
|
||
|
||
# Common plural forms that should be singular
|
||
plural_to_singular = {
|
||
"vendors": "vendor",
|
||
"products": "product",
|
||
"orders": "order",
|
||
"users": "user",
|
||
"categories": "category",
|
||
"customers": "customer",
|
||
"payments": "payment",
|
||
"settings": "setting",
|
||
}
|
||
|
||
if name in plural_to_singular:
|
||
self._add_violation(
|
||
rule_id="NAM-003",
|
||
rule_name="Model files use SINGULAR names",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=1,
|
||
message=f"Model file '{name}.py' should use singular form",
|
||
context=file_path.name,
|
||
suggestion=f"Rename to '{plural_to_singular[name]}.py'",
|
||
)
|
||
|
||
def _check_terminology_consistency(self, file_path: Path, content: str):
|
||
"""NAM-004 & NAM-005: Check for inconsistent terminology"""
|
||
lines = content.split("\n")
|
||
|
||
# NAM-004: Check for 'shop_id' (should be vendor_id)
|
||
# Skip shop-specific files where shop_id might be legitimate
|
||
if "/shop/" not in str(file_path):
|
||
for i, line in enumerate(lines, 1):
|
||
if "shop_id" in line and "# noqa" not in line.lower():
|
||
# Avoid false positives for legitimate shop context
|
||
if "shop_service" not in str(file_path):
|
||
self._add_violation(
|
||
rule_id="NAM-004",
|
||
rule_name="Use 'vendor' not 'shop'",
|
||
severity=Severity.INFO,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Consider using 'vendor_id' instead of 'shop_id'",
|
||
context=line.strip()[:60],
|
||
suggestion="Use vendor_id for multi-tenant context",
|
||
)
|
||
return # Only report once per file
|
||
|
||
# NAM-005: Check for 'stock' (should be inventory)
|
||
for i, line in enumerate(lines, 1):
|
||
if "stock_service" in line or "stock_id" in line:
|
||
if "# noqa" not in line.lower():
|
||
self._add_violation(
|
||
rule_id="NAM-005",
|
||
rule_name="Use 'inventory' not 'stock'",
|
||
severity=Severity.INFO,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Consider using 'inventory' instead of 'stock'",
|
||
context=line.strip()[:60],
|
||
suggestion="Use inventory_service or inventory_id for consistency",
|
||
)
|
||
return
|
||
|
||
def _validate_auth_patterns(self, target_path: Path):
|
||
"""Validate authentication patterns (AUTH-001 to AUTH-004)"""
|
||
print("🔐 Validating auth patterns...")
|
||
|
||
# AUTH-003: Check password hashing in auth service
|
||
auth_service = target_path / "app" / "services" / "auth_service.py"
|
||
if auth_service.exists():
|
||
content = auth_service.read_text()
|
||
if "bcrypt" not in content and "hash" not in content.lower():
|
||
self._add_violation(
|
||
rule_id="AUTH-003",
|
||
rule_name="Never store plain passwords",
|
||
severity=Severity.ERROR,
|
||
file_path=auth_service,
|
||
line_number=1,
|
||
message="Auth service may not be hashing passwords",
|
||
context="auth_service.py",
|
||
suggestion="Use bcrypt or similar library to hash passwords before storing",
|
||
)
|
||
|
||
# AUTH-004: Check vendor context patterns
|
||
vendor_api_files = list(target_path.glob("app/api/v1/vendor/**/*.py"))
|
||
for file_path in vendor_api_files:
|
||
if file_path.name == "__init__.py" or file_path.name == "auth.py":
|
||
continue
|
||
content = file_path.read_text()
|
||
self._check_vendor_context_pattern(file_path, content)
|
||
|
||
shop_api_files = list(target_path.glob("app/api/v1/shop/**/*.py"))
|
||
for file_path in shop_api_files:
|
||
if file_path.name == "__init__.py" or file_path.name == "auth.py":
|
||
continue
|
||
content = file_path.read_text()
|
||
self._check_shop_context_pattern(file_path, content)
|
||
|
||
def _check_vendor_context_pattern(self, file_path: Path, content: str):
|
||
"""AUTH-004: Check that vendor API endpoints use token-based vendor context"""
|
||
if "noqa: auth-004" in content.lower():
|
||
return
|
||
|
||
# Vendor APIs should NOT use require_vendor_context() - that's for shop
|
||
if "require_vendor_context()" in content:
|
||
lines = content.split("\n")
|
||
for i, line in enumerate(lines, 1):
|
||
if "require_vendor_context()" in line:
|
||
self._add_violation(
|
||
rule_id="AUTH-004",
|
||
rule_name="Use correct vendor context pattern",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Vendor API should use token_vendor_id, not require_vendor_context()",
|
||
context=line.strip()[:60],
|
||
suggestion="Use current_user.token_vendor_id from JWT token instead",
|
||
)
|
||
return
|
||
|
||
def _check_shop_context_pattern(self, file_path: Path, content: str):
|
||
"""AUTH-004: Check that shop API endpoints use proper vendor context"""
|
||
if "noqa: auth-004" in content.lower():
|
||
return
|
||
|
||
# Shop APIs that need vendor context should use require_vendor_context or # public
|
||
has_vendor_context = "require_vendor_context" in content or "# public" in content
|
||
|
||
# Check for routes that might need vendor context
|
||
if "@router." in content and not has_vendor_context:
|
||
# Only flag if there are non-public endpoints without vendor context
|
||
lines = content.split("\n")
|
||
for i, line in enumerate(lines, 1):
|
||
if "@router." in line:
|
||
# Check next few lines for public marker or vendor context
|
||
context_lines = "\n".join(lines[i-1:i+10])
|
||
if "# public" not in context_lines and "require_vendor_context" not in context_lines:
|
||
self._add_violation(
|
||
rule_id="AUTH-004",
|
||
rule_name="Shop endpoints need vendor context",
|
||
severity=Severity.INFO,
|
||
file_path=file_path,
|
||
line_number=i,
|
||
message="Shop endpoint may need vendor context dependency",
|
||
context=line.strip()[:60],
|
||
suggestion="Add Depends(require_vendor_context()) or mark as '# public'",
|
||
)
|
||
return
|
||
|
||
def _validate_middleware(self, target_path: Path):
|
||
"""Validate middleware patterns (MDW-001, MDW-002)"""
|
||
print("🔌 Validating middleware...")
|
||
|
||
middleware_dir = target_path / "middleware"
|
||
if not middleware_dir.exists():
|
||
return
|
||
|
||
middleware_files = list(middleware_dir.glob("*.py"))
|
||
|
||
for file_path in middleware_files:
|
||
if file_path.name == "__init__.py":
|
||
continue
|
||
|
||
# MDW-001: Check naming convention
|
||
if "_middleware" in file_path.stem:
|
||
self._add_violation(
|
||
rule_id="MDW-001",
|
||
rule_name="Middleware uses simple noun naming",
|
||
severity=Severity.WARNING,
|
||
file_path=file_path,
|
||
line_number=1,
|
||
message="Middleware file should use simple noun naming",
|
||
context=file_path.name,
|
||
suggestion=f"Rename to '{file_path.stem.replace('_middleware', '')}.py'",
|
||
)
|
||
|
||
# MDW-002: Check vendor context middleware exists and sets proper state
|
||
vendor_context = middleware_dir / "vendor_context.py"
|
||
if vendor_context.exists():
|
||
content = vendor_context.read_text()
|
||
# The middleware can set either request.state.vendor (full object)
|
||
# or request.state.vendor_id - vendor object is preferred as it allows
|
||
# accessing vendor_id via request.state.vendor.id
|
||
if "request.state.vendor" not in content:
|
||
self._add_violation(
|
||
rule_id="MDW-002",
|
||
rule_name="Vendor context middleware must set state",
|
||
severity=Severity.ERROR,
|
||
file_path=vendor_context,
|
||
line_number=1,
|
||
message="Vendor context middleware should set request.state.vendor",
|
||
context="vendor_context.py",
|
||
suggestion="Add 'request.state.vendor = vendor' in the middleware",
|
||
)
|
||
|
||
def _validate_javascript(self, target_path: Path):
|
||
"""Validate JavaScript patterns"""
|
||
print("🟨 Validating JavaScript...")
|
||
|
||
js_files = list(target_path.glob("static/admin/js/**/*.js"))
|
||
self.result.files_checked += len(js_files)
|
||
|
||
for file_path in js_files:
|
||
content = file_path.read_text()
|
||
lines = content.split("\n")
|
||
|
||
# JS-001: Check for console usage (must use centralized logger)
|
||
# Skip init-*.js files - they run before logger is available
|
||
if not file_path.name.startswith("init-"):
|
||
for i, line in enumerate(lines, 1):
|
||
if re.search(r"console\.(log|warn|error)", line):
|
||
# Skip if it's a comment or bootstrap message
|
||
if "//" in line or "✅" in line or "eslint-disable" in line:
|
||
continue
|
||
|
||
self._add_violation(
|
||
rule_id="JS-001",
|
||
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')",
|
||
)
|
||
|
||
# JS-002: Check for window.apiClient (must use lowercase apiClient)
|
||
for i, line in enumerate(lines, 1):
|
||
if "window.apiClient" in line:
|
||
# Check if it's not in a comment
|
||
before_occurrence = line[: line.find("window.apiClient")]
|
||
if "//" not in before_occurrence:
|
||
self._add_violation(
|
||
rule_id="JS-002",
|
||
rule_name="Use lowercase apiClient",
|
||
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",
|
||
)
|
||
|
||
def _validate_templates(self, target_path: Path):
|
||
"""Validate template patterns"""
|
||
print("📄 Validating templates...")
|
||
|
||
template_files = list(target_path.glob("app/templates/admin/**/*.html"))
|
||
self.result.files_checked += len(template_files)
|
||
|
||
# TPL-001 exclusion patterns
|
||
tpl_001_exclusions = [
|
||
"login.html", # Standalone login page
|
||
"errors/", # Error pages extend errors/base.html
|
||
"test-", # Test templates
|
||
]
|
||
|
||
for file_path in template_files:
|
||
file_path_str = str(file_path)
|
||
|
||
# Skip base template and partials
|
||
is_base_or_partial = "base.html" in file_path.name or "partials" in file_path_str
|
||
|
||
# Skip macros directory for FE rules
|
||
is_macro = "shared/macros/" in file_path_str or "shared\\macros\\" in file_path_str
|
||
|
||
# Skip components showcase page
|
||
is_components_page = "components.html" in file_path.name
|
||
|
||
content = file_path.read_text()
|
||
lines = content.split("\n")
|
||
|
||
# FE-001: Check for inline pagination (should use macro)
|
||
if not is_base_or_partial and not is_macro and not is_components_page:
|
||
self._check_pagination_macro_usage(file_path, content, lines)
|
||
|
||
# FE-002: Check for inline SVGs (should use $icon())
|
||
if not is_base_or_partial and not is_macro and not is_components_page:
|
||
self._check_icon_helper_usage(file_path, content, lines)
|
||
|
||
# FE-003: Check for inline loading/error states (should use alerts macro)
|
||
if not is_base_or_partial and not is_macro and not is_components_page:
|
||
self._check_alerts_macro_usage(file_path, content, lines)
|
||
|
||
# FE-004: Check for inline modals (should use modals macro)
|
||
if not is_base_or_partial and not is_macro and not is_components_page:
|
||
self._check_modals_macro_usage(file_path, content, lines)
|
||
|
||
# FE-005: Check for inline table wrappers (should use tables macro)
|
||
if not is_base_or_partial and not is_macro and not is_components_page:
|
||
self._check_tables_macro_usage(file_path, content, lines)
|
||
|
||
# FE-006: Check for inline dropdowns (should use dropdowns macro)
|
||
if not is_base_or_partial and not is_macro and not is_components_page:
|
||
self._check_dropdowns_macro_usage(file_path, content, lines)
|
||
|
||
# FE-007: Check for inline page headers (should use headers macro)
|
||
if not is_base_or_partial and not is_macro and not is_components_page:
|
||
self._check_headers_macro_usage(file_path, content, lines)
|
||
|
||
# FE-008: Check for raw number inputs (should use number_stepper)
|
||
if not is_base_or_partial and not is_macro and not is_components_page:
|
||
self._check_number_stepper_macro_usage(file_path, content, lines)
|
||
|
||
# Skip base/partials for TPL-001 check
|
||
if is_base_or_partial:
|
||
continue
|
||
|
||
# Check exclusion patterns for TPL-001
|
||
skip = False
|
||
for exclusion in tpl_001_exclusions:
|
||
if exclusion in file_path_str:
|
||
skip = True
|
||
break
|
||
if skip:
|
||
continue
|
||
|
||
# Check for standalone marker in template (first 5 lines)
|
||
first_lines = "\n".join(lines[:5]).lower()
|
||
if "standalone" in first_lines or "noqa: tpl-001" in first_lines:
|
||
continue
|
||
|
||
# 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, or add {# standalone #} if intentional",
|
||
)
|
||
|
||
def _get_rule(self, rule_id: str) -> dict[str, Any]:
|
||
"""Get rule configuration by ID"""
|
||
# Look in different rule categories
|
||
for category in [
|
||
"api_endpoint_rules",
|
||
"service_layer_rules",
|
||
"model_rules",
|
||
"exception_rules",
|
||
"javascript_rules",
|
||
"template_rules",
|
||
"frontend_component_rules",
|
||
]:
|
||
rules = self.config.get(category, [])
|
||
for rule in rules:
|
||
if rule.get("id") == rule_id:
|
||
return rule
|
||
return None
|
||
|
||
def _should_ignore_file(self, file_path: Path) -> bool:
|
||
"""Check if file should be ignored"""
|
||
ignore_patterns = self.config.get("ignore", {}).get("files", [])
|
||
|
||
# Convert to string for easier matching
|
||
file_path_str = str(file_path)
|
||
|
||
for pattern in ignore_patterns:
|
||
# Check if any part of the path matches the pattern
|
||
if file_path.match(pattern):
|
||
return True
|
||
# Also check if pattern appears in the path (for .venv, venv, etc.)
|
||
if "/.venv/" in file_path_str or file_path_str.startswith(".venv/"):
|
||
return True
|
||
if "/venv/" in file_path_str or file_path_str.startswith("venv/"):
|
||
return True
|
||
|
||
return False
|
||
|
||
def _add_violation(
|
||
self,
|
||
rule_id: str,
|
||
rule_name: str,
|
||
severity: Severity,
|
||
file_path: Path,
|
||
line_number: int,
|
||
message: str,
|
||
context: str = "",
|
||
suggestion: str = "",
|
||
):
|
||
"""Add a violation to results"""
|
||
violation = Violation(
|
||
rule_id=rule_id,
|
||
rule_name=rule_name,
|
||
severity=severity,
|
||
file_path=file_path,
|
||
line_number=line_number,
|
||
message=message,
|
||
context=context,
|
||
suggestion=suggestion,
|
||
)
|
||
self.result.violations.append(violation)
|
||
|
||
def print_report(self):
|
||
"""Print validation report"""
|
||
print("\n" + "=" * 80)
|
||
print("📊 ARCHITECTURE VALIDATION REPORT")
|
||
print("=" * 80 + "\n")
|
||
|
||
# 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]
|
||
infos = [v for v in self.result.violations if v.severity == Severity.INFO]
|
||
|
||
print(f"Files checked: {self.result.files_checked}")
|
||
print(f"Findings: {len(errors)} errors, {len(warnings)} warnings, {len(infos)} info\n")
|
||
|
||
# Print file summary table if we have file results
|
||
if self.result.file_results:
|
||
self._print_summary_table()
|
||
|
||
if errors:
|
||
print(f"\n❌ ERRORS ({len(errors)}):")
|
||
print("-" * 80)
|
||
for violation in errors:
|
||
self._print_violation(violation)
|
||
|
||
if warnings:
|
||
print(f"\n⚠️ WARNINGS ({len(warnings)}):")
|
||
print("-" * 80)
|
||
for violation in warnings:
|
||
self._print_violation(violation)
|
||
|
||
if infos and self.verbose:
|
||
print(f"\nℹ️ INFO ({len(infos)}):")
|
||
print("-" * 80)
|
||
for violation in infos:
|
||
self._print_violation(violation)
|
||
|
||
# Summary
|
||
print("\n" + "=" * 80)
|
||
if self.result.has_errors():
|
||
print(f"❌ VALIDATION FAILED - {len(errors)} error(s) must be fixed before committing")
|
||
print("=" * 80)
|
||
return 1
|
||
if self.result.has_warnings():
|
||
print(f"⚠️ VALIDATION PASSED WITH {len(warnings)} WARNING(S)")
|
||
print("=" * 80)
|
||
return 0
|
||
print("✅ VALIDATION PASSED")
|
||
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
|
||
|
||
violations_json = []
|
||
for v in self.result.violations:
|
||
rel_path = (
|
||
str(v.file_path.relative_to(self.project_root))
|
||
if self.project_root in v.file_path.parents
|
||
else str(v.file_path)
|
||
)
|
||
violations_json.append(
|
||
{
|
||
"rule_id": v.rule_id,
|
||
"rule_name": v.rule_name,
|
||
"severity": v.severity.value,
|
||
"file_path": rel_path,
|
||
"line_number": v.line_number,
|
||
"message": v.message,
|
||
"context": v.context or "",
|
||
"suggestion": v.suggestion or "",
|
||
}
|
||
)
|
||
|
||
output = {
|
||
"files_checked": self.result.files_checked,
|
||
"total_violations": len(self.result.violations),
|
||
"errors": len(
|
||
[v for v in self.result.violations if v.severity == Severity.ERROR]
|
||
),
|
||
"warnings": len(
|
||
[v for v in self.result.violations if v.severity == Severity.WARNING]
|
||
),
|
||
"violations": violations_json,
|
||
}
|
||
|
||
print(json.dumps(output, indent=2))
|
||
|
||
return 1 if self.result.has_errors() else 0
|
||
|
||
def _print_violation(self, v: Violation):
|
||
"""Print a single violation"""
|
||
rel_path = (
|
||
v.file_path.relative_to(self.project_root)
|
||
if self.project_root in v.file_path.parents
|
||
else v.file_path
|
||
)
|
||
|
||
print(f"\n [{v.rule_id}] {v.rule_name}")
|
||
print(f" File: {rel_path}:{v.line_number}")
|
||
print(f" Issue: {v.message}")
|
||
|
||
if v.context and self.verbose:
|
||
print(f" Context: {v.context}")
|
||
|
||
if v.suggestion:
|
||
print(f" 💡 Suggestion: {v.suggestion}")
|
||
|
||
|
||
def main():
|
||
"""Main entry point"""
|
||
parser = argparse.ArgumentParser(
|
||
description="Validate architecture patterns in codebase",
|
||
formatter_class=argparse.RawDescriptionHelpFormatter,
|
||
epilog=__doc__,
|
||
)
|
||
|
||
# Target options (mutually exclusive)
|
||
target_group = parser.add_mutually_exclusive_group()
|
||
|
||
target_group.add_argument(
|
||
"-f",
|
||
"--file",
|
||
type=Path,
|
||
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(
|
||
"-c",
|
||
"--config",
|
||
type=Path,
|
||
default=Path.cwd() / ".architecture-rules.yaml",
|
||
help="Path to architecture rules config (default: .architecture-rules.yaml)",
|
||
)
|
||
|
||
parser.add_argument(
|
||
"-v",
|
||
"--verbose",
|
||
action="store_true",
|
||
help="Show detailed output including context",
|
||
)
|
||
|
||
parser.add_argument(
|
||
"--errors-only", action="store_true", help="Only show errors, suppress warnings"
|
||
)
|
||
|
||
parser.add_argument(
|
||
"--json",
|
||
action="store_true",
|
||
help="Output results as JSON (for programmatic use)",
|
||
)
|
||
|
||
args = parser.parse_args()
|
||
|
||
# Create validator
|
||
validator = ArchitectureValidator(args.config, verbose=args.verbose)
|
||
|
||
# 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:
|
||
exit_code = validator.print_json()
|
||
else:
|
||
exit_code = validator.print_report()
|
||
|
||
sys.exit(exit_code)
|
||
|
||
|
||
if __name__ == "__main__":
|
||
main()
|