feat(seedgo): enhance drone-compliance and add naming-convention plugin (#8)

Implement three priority improvements from seed review: 1. DRONE_MODULE key validation: - Add check for required keys (name, version, description) - Fail with ERROR severity if keys are missing - Extract helper function _get_missing_drone_module_keys() 2. get_introspective() check: - Add WARNING-level check for get_introspective() function - Recommended but not required for drone modules - Follows introspective pattern for module self-documentation 3. New naming-convention plugin: - Enforce PEP 8 naming conventions - Check snake_case for functions, variables, module names - Check PascalCase for classes - Check UPPER_CASE for module-level constants - Provide helpful conversion suggestions in fix hints Test coverage: - Update existing tests to include new checks - Add tests for missing DRONE_MODULE keys (ERROR) - Add tests for missing get_introspective (WARNING) - All 444 tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
2026-03-04 03:13:43 -08:00
parent 09423e8a0a
commit e4622d700e
3 changed files with 392 additions and 2 deletions
@@ -17,6 +17,38 @@ from pathlib import Path

 from seedgo.models import CheckItem, CheckResult, Severity

+
+def _get_missing_drone_module_keys(tree: ast.AST, required_keys: set[str]) -> set[str]:
+    """Extract keys from DRONE_MODULE dict and return missing required keys.
+
+    Args:
+        tree: AST tree to search
+        required_keys: Set of required key names
+
+    Returns:
+        Set of missing key names
+    """
+    for node in ast.walk(tree):
+        if not isinstance(node, ast.Assign):
+            continue
+        if not any(isinstance(t, ast.Name) and t.id == "DRONE_MODULE" for t in node.targets):
+            continue
+
+        # Found DRONE_MODULE assignment, check if value is a dict
+        if not isinstance(node.value, ast.Dict):
+            continue
+
+        # Extract string keys from the dict
+        found_keys = set()
+        for key in node.value.keys:
+            if isinstance(key, ast.Constant) and isinstance(key.value, str):
+                found_keys.add(key.value)
+
+        return required_keys - found_keys
+
+    # DRONE_MODULE not found or not a dict
+    return required_keys
+
 PLUGIN_NAME = "drone-compliance"
 PLUGIN_DESCRIPTION = "Verify modules provide drone adapter interface"
 PLUGIN_VERSION = "1.0.0"
@@ -151,6 +183,29 @@ def check(file_path: str, config: dict | None = None) -> CheckResult:
        )
    )

+    # Check 2b: DRONE_MODULE dict has required keys (name, version, description)
+    if has_meta:
+        required_keys = {"name", "version", "description"}
+        missing_keys = _get_missing_drone_module_keys(tree, required_keys)
+        has_all_keys = len(missing_keys) == 0
+        checks.append(
+            CheckItem(
+                name="drone-module-keys",
+                passed=has_all_keys,
+                message=(
+                    "DRONE_MODULE has all required keys (name, version, description)"
+                    if has_all_keys
+                    else f"DRONE_MODULE missing required keys: {', '.join(sorted(missing_keys))}"
+                ),
+                severity=Severity.ERROR if not has_all_keys else Severity.INFO,
+                fix_hint=(
+                    f"Add missing keys to DRONE_MODULE: {', '.join(sorted(missing_keys))}"
+                    if not has_all_keys
+                    else None
+                ),
+            )
+        )
+
    # Check 3: handle_command() function exists
    functions = [
        node.name for node in ast.walk(tree) if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
@@ -191,6 +246,22 @@ def check(file_path: str, config: dict | None = None) -> CheckResult:
        )
    )

+    # Check 5: get_introspective() function exists (optional but recommended)
+    has_introspective = "get_introspective" in functions
+    checks.append(
+        CheckItem(
+            name="get-introspective",
+            passed=has_introspective,
+            message=(
+                "get_introspective() function found — module supports introspection"
+                if has_introspective
+                else "Missing get_introspective() — introspection pattern recommended for drone modules"
+            ),
+            severity=Severity.WARNING if not has_introspective else Severity.INFO,
+            fix_hint="Add: def get_introspective() -> dict:" if not has_introspective else None,
+        )
+    )
+
    # Score: errors block pass, warnings degrade
    error_checks = [c for c in checks if not c.passed and c.severity == Severity.ERROR]
    all_passed = len(error_checks) == 0
@@ -0,0 +1,267 @@
+"""
+Seed Go Plugin: naming-convention
+
+Enforces Python naming conventions:
+- snake_case for functions, variables, and module names
+- PascalCase for classes
+- UPPER_CASE for module-level constants
+
+This is a fundamental Python style rule (PEP 8) that improves code readability
+and consistency across projects.
+"""
+
+import ast
+import re
+from pathlib import Path
+
+from seedgo.models import CheckItem, CheckResult, Severity
+
+PLUGIN_NAME = "naming-convention"
+PLUGIN_DESCRIPTION = "Enforce Python naming conventions (PEP 8)"
+FILE_TYPES = ["*.py"]
+PLUGIN_VERSION = "1.0.0"
+
+# Naming patterns
+_SNAKE_CASE_RE = re.compile(r"^[a-z_][a-z0-9_]*$")
+_PASCAL_CASE_RE = re.compile(r"^[A-Z][a-zA-Z0-9]*$")
+_UPPER_CASE_RE = re.compile(r"^[A-Z_][A-Z0-9_]*$")
+
+# Common exceptions that are acceptable
+_COMMON_EXCEPTIONS = {
+    # Common single-letter variables
+    "i", "j", "k", "n", "x", "y", "z", "a", "b", "c",
+    # Common acronyms/names that are conventionally capitalized
+    "T", "K", "V",
+    # Special methods and variables
+    "_", "__",
+}
+
+
+def _is_constant(node: ast.Assign | ast.AnnAssign, scope: str) -> bool:
+    """Determine if an assignment is a module-level constant.
+
+    Constants are defined as module-level assignments where:
+    1. The name is in UPPER_CASE
+    2. The assignment is at module level (not in a function/class)
+
+    Args:
+        node: Assignment node
+        scope: "module", "class", or "function"
+
+    Returns:
+        True if this appears to be a constant
+    """
+    if scope != "module":
+        return False
+
+    # Get target names
+    if isinstance(node, ast.Assign):
+        targets = node.targets
+    else:  # AnnAssign
+        targets = [node.target]
+
+    for target in targets:
+        if isinstance(target, ast.Name):
+            # Consider it a constant if it matches UPPER_CASE pattern
+            if _UPPER_CASE_RE.match(target.id):
+                return True
+
+    return False
+
+
+def check(file_path: str, config: dict | None = None) -> CheckResult:
+    """Check a Python file for naming convention violations.
+
+    Args:
+        file_path: Absolute path to the Python file to check.
+        config: Optional plugin config dict (unused by this plugin).
+
+    Returns:
+        CheckResult with one CheckItem per naming violation found.
+    """
+    _ = config  # Part of plugin interface contract
+
+    try:
+        source = Path(file_path).read_text(encoding="utf-8", errors="replace")
+    except OSError:
+        return CheckResult(
+            plugin=PLUGIN_NAME,
+            passed=True,
+            checks=[],
+            file_path=file_path,
+            metadata={"skipped": True, "reason": "file_read_error"},
+        )
+
+    if not source.strip():
+        return CheckResult(
+            plugin=PLUGIN_NAME,
+            passed=True,
+            checks=[
+                CheckItem(
+                    name="naming-convention",
+                    passed=True,
+                    message="Empty file — no names to check.",
+                    severity=Severity.WARNING,
+                )
+            ],
+            file_path=file_path,
+        )
+
+    try:
+        tree = ast.parse(source, filename=file_path)
+    except SyntaxError:
+        return CheckResult(
+            plugin=PLUGIN_NAME,
+            passed=True,
+            checks=[],
+            file_path=file_path,
+            metadata={"skipped": True, "reason": "syntax_error"},
+        )
+
+    violations: list[CheckItem] = []
+
+    # Check module name (file name)
+    module_name = Path(file_path).stem
+    if module_name not in ("__init__", "__main__") and not _SNAKE_CASE_RE.match(module_name):
+        violations.append(
+            CheckItem(
+                name="module-name",
+                passed=False,
+                message=f"Module name '{module_name}' should be snake_case",
+                severity=Severity.WARNING,
+                fix_hint=f"Rename file to use snake_case (e.g., {_to_snake_case(module_name)}.py)",
+            )
+        )
+
+    # Walk the AST and check naming
+    for node in ast.walk(tree):
+        # Check class names (should be PascalCase)
+        if isinstance(node, ast.ClassDef):
+            if not _PASCAL_CASE_RE.match(node.name) and node.name not in _COMMON_EXCEPTIONS:
+                violations.append(
+                    CheckItem(
+                        name="class-name",
+                        passed=False,
+                        message=f"Class '{node.name}' at line {node.lineno} should be PascalCase",
+                        severity=Severity.WARNING,
+                        line=node.lineno,
+                        fix_hint=f"Rename to PascalCase (e.g., {_to_pascal_case(node.name)})",
+                    )
+                )
+
+        # Check function names (should be snake_case)
+        elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
+            # Skip dunder methods
+            if node.name.startswith("__") and node.name.endswith("__"):
+                continue
+
+            if not _SNAKE_CASE_RE.match(node.name) and node.name not in _COMMON_EXCEPTIONS:
+                violations.append(
+                    CheckItem(
+                        name="function-name",
+                        passed=False,
+                        message=f"Function '{node.name}' at line {node.lineno} should be snake_case",
+                        severity=Severity.WARNING,
+                        line=node.lineno,
+                        fix_hint=f"Rename to snake_case (e.g., {_to_snake_case(node.name)})",
+                    )
+                )
+
+    # Check module-level variables and constants
+    for node in tree.body:
+        if isinstance(node, (ast.Assign, ast.AnnAssign)):
+            # Determine if this is a constant or variable
+            is_const = _is_constant(node, "module")
+
+            # Get target names
+            if isinstance(node, ast.Assign):
+                targets = node.targets
+            else:  # AnnAssign
+                targets = [node.target]
+
+            for target in targets:
+                if isinstance(target, ast.Name):
+                    name = target.id
+
+                    # Skip special variables
+                    if name.startswith("__") and name.endswith("__"):
+                        continue
+
+                    if name in _COMMON_EXCEPTIONS:
+                        continue
+
+                    # Check naming based on whether it's a constant
+                    if is_const:
+                        if not _UPPER_CASE_RE.match(name):
+                            violations.append(
+                                CheckItem(
+                                    name="constant-name",
+                                    passed=False,
+                                    message=f"Module-level constant '{name}' at line {node.lineno} should be UPPER_CASE",
+                                    severity=Severity.WARNING,
+                                    line=node.lineno,
+                                    fix_hint=f"Rename to UPPER_CASE (e.g., {name.upper()})",
+                                )
+                            )
+                    else:
+                        if not _SNAKE_CASE_RE.match(name):
+                            violations.append(
+                                CheckItem(
+                                    name="variable-name",
+                                    passed=False,
+                                    message=f"Module-level variable '{name}' at line {node.lineno} should be snake_case",
+                                    severity=Severity.WARNING,
+                                    line=node.lineno,
+                                    fix_hint=f"Rename to snake_case (e.g., {_to_snake_case(name)})",
+                                )
+                            )
+
+    if violations:
+        passed = False
+        checks = violations
+    else:
+        passed = True
+        checks = [
+            CheckItem(
+                name="naming-convention",
+                passed=True,
+                message="All names follow Python naming conventions (PEP 8).",
+                severity=Severity.WARNING,
+            )
+        ]
+
+    return CheckResult(
+        plugin=PLUGIN_NAME,
+        passed=passed,
+        checks=checks,
+        file_path=file_path,
+        metadata={"violations_found": len(violations)},
+    )
+
+
+def _to_snake_case(name: str) -> str:
+    """Convert a name to snake_case suggestion.
+
+    Args:
+        name: Original name
+
+    Returns:
+        Suggested snake_case version
+    """
+    # Simple conversion: insert underscores before uppercase letters
+    result = re.sub(r"(?<!^)(?=[A-Z])", "_", name).lower()
+    return result
+
+
+def _to_pascal_case(name: str) -> str:
+    """Convert a name to PascalCase suggestion.
+
+    Args:
+        name: Original name
+
+    Returns:
+        Suggested PascalCase version
+    """
+    # Split on underscores and capitalize each word
+    words = name.split("_")
+    return "".join(word.capitalize() for word in words if word)
@@ -35,6 +35,9 @@ class TestDroneCompliancePass:

            def get_help(command=None):
                return "help text"
+
+            def get_introspective():
+                return {}
            """)
        )

@@ -44,8 +47,8 @@ class TestDroneCompliancePass:
        assert result.passed is True
        assert result.score == 100

-    def test_all_four_checks_present(self, tmp_path: Path):
-        """All 4 checks pass: adapter exists, DRONE_MODULE, handle_command, get_help."""
+    def test_all_checks_present(self, tmp_path: Path):
+        """All checks pass: adapter exists, DRONE_MODULE with keys, handle_command, get_help, get_introspective."""
        pkg = tmp_path / "mymod"
        pkg.mkdir()
        (pkg / "__init__.py").write_text("")
@@ -58,11 +61,15 @@ class TestDroneCompliancePass:

            def get_help(command=None):
                return ""
+
+            def get_introspective():
+                return {}
            """)
        )

        result = check(str(pkg / "__init__.py"), config={"target_packages": ["mymod"]})
        assert result.passed is True
+        assert result.score == 100


 class TestDroneComplianceFail:
@@ -140,6 +147,51 @@ class TestDroneComplianceFail:
        warnings = [c for c in result.checks if not c.passed]
        assert any(c.name == "get-help" for c in warnings)

+    def test_missing_drone_module_keys(self, tmp_path: Path):
+        """Adapter with DRONE_MODULE but missing required keys fails."""
+        pkg = tmp_path / "mymod"
+        pkg.mkdir()
+        (pkg / "__init__.py").write_text("")
+        (pkg / "drone_adapter.py").write_text(
+            textwrap.dedent("""\
+            DRONE_MODULE = {"name": "mymod"}
+
+            def handle_command(command, args=None):
+                return {}
+
+            def get_help(command=None):
+                return ""
+            """)
+        )
+
+        result = check(str(pkg / "__init__.py"), config={"target_packages": ["mymod"]})
+        assert result.passed is False
+        failed = [c.name for c in result.checks if not c.passed]
+        assert "drone-module-keys" in failed
+
+    def test_missing_get_introspective_is_warning(self, tmp_path: Path):
+        """Adapter without get_introspective is a warning, not an error — still passes."""
+        pkg = tmp_path / "mymod"
+        pkg.mkdir()
+        (pkg / "__init__.py").write_text("")
+        (pkg / "drone_adapter.py").write_text(
+            textwrap.dedent("""\
+            DRONE_MODULE = {"name": "mymod", "version": "1.0.0", "description": "test"}
+
+            def handle_command(command, args=None):
+                return {}
+
+            def get_help(command=None):
+                return ""
+            """)
+        )
+
+        result = check(str(pkg / "__init__.py"), config={"target_packages": ["mymod"]})
+        # Missing get_introspective is WARNING severity, not ERROR — should still pass
+        assert result.passed is True
+        warnings = [c for c in result.checks if not c.passed]
+        assert any(c.name == "get-introspective" for c in warnings)
+

 class TestDroneComplianceSkip:
    """Non-target packages should be skipped."""