diff --git a/benchmarks/semantic_validation_suite_bench.py b/benchmarks/semantic_validation_suite_bench.py new file mode 100644 index 000000000..4320c5f7b --- /dev/null +++ b/benchmarks/semantic_validation_suite_bench.py @@ -0,0 +1,164 @@ +"""ASV benchmarks for semantic validation suite runtime. + +Measures the performance of running entire fixture suites through the +semantic validation rules, covering: +- Language-porting mismatch fixtures +- Dependency graph violation fixtures +- API surface change fixtures +- Cross-file symbol resolution fixtures +- Duplicate relative import detection fixtures + +Uses :mod:`features.fixtures.validation.suite_helpers` for shared +loading, lookup, and rule-map construction. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from typing import Any + +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +_FIXTURES_DIR = str( + Path(__file__).resolve().parents[1] / "features" / "fixtures" / "validation" +) +if _FIXTURES_DIR not in sys.path: + sys.path.insert(0, _FIXTURES_DIR) + +from suite_helpers import ( # noqa: E402, I001 + ALL_FIXTURE_FILENAMES, + API_FILENAME, + CIRCULAR_FILENAME, + CROSS_FILE_FILENAME, + DEPENDENCY_FILENAME, + PORTING_FILENAME, + RULE_MAP, + load_fixtures, +) + +from cleveragents.application.services.semantic_validation_rules import ( # noqa: E402 + SemanticCheckResult, +) +from cleveragents.application.services.semantic_validation_service import ( # noqa: E402 + SemanticValidationService, +) + + +# --------------------------------------------------------------------------- +# Fixture runner (uses shared RULE_MAP) +# --------------------------------------------------------------------------- + + +def _run_suite(fixtures: list[dict[str, Any]]) -> list[SemanticCheckResult]: + """Run all fixtures through their expected rules.""" + results: list[SemanticCheckResult] = [] + for fixture in fixtures: + rule_name: str = fixture["expected_rule"] + rule = RULE_MAP[rule_name] + result = rule.check(fixture["source"], fixture["filename"]) + results.append(result) + return results + + +# --------------------------------------------------------------------------- +# Benchmark suites +# --------------------------------------------------------------------------- + + +class PortingFixtureSuite: + """Benchmarks for language-porting mismatch fixture suite runtime.""" + + timeout = 60 + + def setup(self) -> None: + self.fixtures = load_fixtures(PORTING_FILENAME) + + def time_run_all_porting_fixtures(self) -> None: + _run_suite(self.fixtures) + + def time_load_porting_fixtures(self) -> None: + load_fixtures(PORTING_FILENAME) + + +class DependencyFixtureSuite: + """Benchmarks for dependency graph violation fixture suite runtime.""" + + timeout = 60 + + def setup(self) -> None: + self.fixtures = load_fixtures(DEPENDENCY_FILENAME) + + def time_run_all_dependency_fixtures(self) -> None: + _run_suite(self.fixtures) + + def time_load_dependency_fixtures(self) -> None: + load_fixtures(DEPENDENCY_FILENAME) + + +class APISurfaceFixtureSuite: + """Benchmarks for API surface change fixture suite runtime.""" + + timeout = 60 + + def setup(self) -> None: + self.fixtures = load_fixtures(API_FILENAME) + + def time_run_all_api_fixtures(self) -> None: + _run_suite(self.fixtures) + + def time_load_api_fixtures(self) -> None: + load_fixtures(API_FILENAME) + + +class CrossFileFixtureSuite: + """Benchmarks for cross-file symbol resolution fixture suite runtime.""" + + timeout = 60 + + def setup(self) -> None: + self.fixtures = load_fixtures(CROSS_FILE_FILENAME) + + def time_run_all_cross_file_fixtures(self) -> None: + _run_suite(self.fixtures) + + def time_load_cross_file_fixtures(self) -> None: + load_fixtures(CROSS_FILE_FILENAME) + + +class CircularImportFixtureSuite: + """Benchmarks for duplicate relative import fixture suite runtime.""" + + timeout = 60 + + def setup(self) -> None: + self.fixtures = load_fixtures(CIRCULAR_FILENAME) + + def time_run_all_circular_fixtures(self) -> None: + _run_suite(self.fixtures) + + def time_load_circular_fixtures(self) -> None: + load_fixtures(CIRCULAR_FILENAME) + + +class FullSuiteRuntime: + """Benchmarks for running all fixture suites together.""" + + timeout = 120 + + def setup(self) -> None: + self.all_fixtures: list[dict[str, Any]] = [] + for filename in ALL_FIXTURE_FILENAMES: + self.all_fixtures.extend(load_fixtures(filename)) + # Pre-create service in setup so timing only measures validation + self.service = SemanticValidationService() + + def time_run_all_suites(self) -> None: + _run_suite(self.all_fixtures) + + def time_service_check_all_sources(self) -> None: + self.service.cache.clear() + for fixture in self.all_fixtures: + self.service.check_file(fixture["source"], fixture["filename"]) diff --git a/docs/reference/semantic_validation_coverage.md b/docs/reference/semantic_validation_coverage.md new file mode 100644 index 000000000..a0842c1cc --- /dev/null +++ b/docs/reference/semantic_validation_coverage.md @@ -0,0 +1,117 @@ +# Semantic Validation Coverage Expectations + +## Overview + +The semantic validation subsystem provides static analysis of Python source code +through a pluggable rule-based architecture. This document describes the test +coverage expectations for the validation fixture suites. + +## Rule Coverage Matrix + +Each built-in rule is exercised by dedicated fixture suites that test both +passing (no violation detected) and failing (violation correctly flagged) +scenarios. + +| Rule | Fixture Suite | Pass Scenarios | Fail Scenarios | +| :--- | :------------ | :------------- | :------------- | +| `SyntaxCheckRule` | Language-porting mismatches | Valid Python patterns (diamond inheritance, metaclasses, walrus operator, star unpacking) | — | +| `MissingImportRule` | Dependency graph violations | Valid stdlib private imports (`_thread`), mixed valid imports | Private module imports, from-private imports | +| `BrokenReferenceRule` | API surface changes, Cross-file symbols | Valid API usage, properly imported cross-module symbols | Renamed function references, incompatible type usage, undefined cross-module references, wildcard imports (known limitation) | +| `DuplicateImportRule` | Dependency graph violations, Duplicate relative imports | Mixed valid imports, clean relative imports, absolute imports (not flagged), syntax errors (skipped) | Duplicate relative imports, triple duplicates, direct duplicates, multiple self-references | +| `APIMisuseRule` | Language-porting mismatches, API surface changes | List comprehension side effects (not flagged), dynamic attribute access, aliased imports (known limitation) | eval-based dynamic calls, eval in porting context, exec for code generation | +| `MissingSymbolRule` | API surface changes, Cross-file symbols | Closure variables (enclosing scope tracking), deeply nested closures | Missing symbols in functions, class method missing symbols, undefined in class methods | + +## Fixture Categories + +### 1. Language-Porting Mismatches + +Fixtures in `features/fixtures/validation/language_porting_mismatches.json` +cover Python-specific patterns that are difficult or impossible to port to +other languages: list comprehension side effects, dynamic attribute access, +diamond inheritance, metaclasses, star unpacking, and walrus operators. + +**Coverage expectation:** Pass fixtures verify that valid Python porting +patterns are not falsely flagged. Fail fixtures (`eval_in_porting_context`, +`exec_for_code_generation`) verify that genuinely dangerous API usage is +detected regardless of the porting context. + +### 2. Dependency Graph Violations + +Fixtures in `features/fixtures/validation/dependency_graph_violations.json` +cover import-related violations: duplicate relative imports (cycle indicators), +private module imports not in stdlib, and valid import patterns as negative +controls. + +**Coverage expectation:** Failure fixtures trigger `duplicate_import` or +`missing_import` rules. Pass fixtures confirm no false positives on valid +patterns. + +### 3. API Surface Changes + +Fixtures in `features/fixtures/validation/api_surface_changes.json` cover +renamed functions, missing symbols, incompatible types, dynamic API calls +via `eval`, and known limitation fixtures for import aliasing bypass. + +**Coverage expectation:** Failure fixtures trigger `broken_reference`, +`missing_symbol`, or `api_misuse` rules. Pass fixtures confirm valid API +usage is not flagged. Aliasing limitation fixtures +(`aliased_pickle_not_caught`, `from_import_alias_not_caught`) document that +`APIMisuseRule` does not resolve import aliases. + +### 4. Cross-File Symbol Resolution + +Fixtures in `features/fixtures/validation/cross_file_symbols.json` cover +cross-module references, properly imported symbols, closure scoping (including +deeply nested closures), class method undefined references, and wildcard +imports. + +**Coverage expectation:** Single-file rules cannot detect true cross-file +resolution failures when the symbol appears imported. Closure fixtures +verify that `MissingSymbolRule` correctly tracks enclosing function scopes. +Fixtures test the boundaries of what single-file analysis can detect. + +### 5. Duplicate Relative Import Detection + +Fixtures in `features/fixtures/validation/circular_import_detection.json` +cover duplicate relative imports within a single file, multiple self- +references, clean imports, and edge cases (absolute imports, syntax errors). + +> **Note:** `DuplicateImportRule` detects duplicate `from . import X` +> statements **within a single file** as a heuristic for structural problems. +> It does **not** perform cross-file circular dependency detection. The +> fixture category name reflects duplicate relative import detection, not +> true circular import analysis. + +**Coverage expectation:** `DuplicateImportRule` flags repeated relative +`from . import X` patterns as duplicate indicators. Absolute imports are +not flagged. Syntax errors cause the rule to skip gracefully. + +## Known Limitations + +The following known limitations are documented via fixture tests: + +| Limitation | Rule | Fixture | Description | +| :--------- | :--- | :------ | :---------- | +| Import aliasing bypass | `APIMisuseRule` | `aliased_pickle_not_caught`, `from_import_alias_not_caught` | Aliased imports (`import X as Y`, `from X import func as alias`) bypass detection because the rule matches module/function names literally | +| Wildcard import resolution | `BrokenReferenceRule` | `wildcard_import_symbol` | Names imported via `from X import *` are not tracked, causing false positives on valid wildcard-imported names | +| No cross-file cycle detection | `DuplicateImportRule` | — | Only detects duplicates within a single file; cannot trace import cycles across modules | + +## Test Infrastructure + +- **Shared utilities:** `features/fixtures/validation/suite_helpers.py` — common + fixture loading, lookup, rule-map, and verification logic used by all three + test frameworks. +- **BDD (Behave):** `features/semantic_validation_suite.feature` — 38 scenarios + covering all five fixture categories with individual fixture validation and + schema-validated loading tests. +- **Integration (Robot):** `robot/semantic_validation_suite.robot` — 11 test + cases that load fixtures and run them through rules via a helper script. +- **Benchmarks (ASV):** `benchmarks/semantic_validation_suite_bench.py` — 6 + benchmark suites measuring fixture load time, individual suite runtime, and + combined full-suite runtime. + +## Coverage Target + +All semantic validation code must maintain **>=97%** line coverage as measured +by `nox -s coverage_report`. The fixture suites contribute to this target by +exercising rule logic across diverse code patterns and edge cases. diff --git a/features/fixtures/validation/api_surface_changes.json b/features/fixtures/validation/api_surface_changes.json new file mode 100644 index 000000000..8c4e03e49 --- /dev/null +++ b/features/fixtures/validation/api_surface_changes.json @@ -0,0 +1,81 @@ +{ + "fixtures": [ + { + "name": "renamed_function_reference", + "description": "Function uses a name that was renamed — broken reference detected", + "source": "def process():\n return old_handler_xyz()\n", + "filename": "refactored.py", + "expected_rule": "broken_reference", + "expected_passed": false, + "expected_message_contains": "broken reference", + "expected_data_contains": {"filename": "refactored.py"} + }, + { + "name": "missing_symbol_in_function", + "description": "Function references a symbol that does not exist in any scope", + "source": "def compute(x):\n return x + missing_constant_xyz\n", + "filename": "missing_api.py", + "expected_rule": "missing_symbol", + "expected_passed": false, + "expected_message_contains": "missing symbol", + "expected_data_contains": {"filename": "missing_api.py"} + }, + { + "name": "incompatible_type_usage", + "description": "Calling an attribute on a string literal that does not exist — detectable as broken reference", + "source": "def transform():\n return nonexistent_transformer_xyz.convert()\n", + "filename": "type_mismatch.py", + "expected_rule": "broken_reference", + "expected_passed": false, + "expected_message_contains": "broken reference", + "expected_data_contains": {"filename": "type_mismatch.py"} + }, + { + "name": "valid_api_usage", + "description": "All function calls use defined names — passes all rules", + "source": "import json\n\ndef parse(data):\n return json.loads(data)\n", + "filename": "valid_api.py", + "expected_rule": "broken_reference", + "expected_passed": true, + "note": "json.loads is a valid API call" + }, + { + "name": "class_method_missing_symbol", + "description": "Class method uses an undefined symbol from a removed API", + "source": "class Processor:\n def run(self):\n return removed_api_function_xyz()\n", + "filename": "class_api.py", + "expected_rule": "missing_symbol", + "expected_passed": false, + "expected_message_contains": "missing symbol", + "expected_data_contains": {"filename": "class_api.py"} + }, + { + "name": "eval_based_dynamic_call", + "description": "Using eval to call functions dynamically — API misuse pattern", + "source": "def dispatch(name, arg):\n return eval(f'{name}({arg})')\n", + "filename": "dynamic_api.py", + "expected_rule": "api_misuse", + "expected_passed": false, + "expected_message_contains": "API misuse", + "expected_data_contains": {"filename": "dynamic_api.py"} + }, + { + "name": "aliased_pickle_not_caught", + "description": "Aliased import of pickle bypasses APIMisuseRule — known limitation", + "source": "import pickle as pk\n\ndef load_data(path):\n with open(path, 'rb') as f:\n return pk.load(f)\n", + "filename": "aliased_pickle.py", + "expected_rule": "api_misuse", + "expected_passed": true, + "note": "APIMisuseRule checks module names literally; aliased imports (import X as Y) are not tracked" + }, + { + "name": "from_import_alias_not_caught", + "description": "From-import alias of dangerous function bypasses APIMisuseRule — known limitation", + "source": "from os import system as run_cmd\n\ndef execute(cmd):\n return run_cmd(cmd)\n", + "filename": "aliased_system.py", + "expected_rule": "api_misuse", + "expected_passed": true, + "note": "APIMisuseRule does not resolve from-import aliases; 'run_cmd' is not in the dangerous names set" + } + ] +} diff --git a/features/fixtures/validation/circular_import_detection.json b/features/fixtures/validation/circular_import_detection.json new file mode 100644 index 000000000..23d600262 --- /dev/null +++ b/features/fixtures/validation/circular_import_detection.json @@ -0,0 +1,51 @@ +{ + "fixtures": [ + { + "name": "direct_circular_import", + "description": "Same relative import appears twice — duplicate detected", + "source": "from . import sibling\nfrom . import sibling\n", + "filename": "circular_a.py", + "expected_rule": "duplicate_import", + "expected_passed": false, + "expected_message_contains": "Duplicate relative imports", + "expected_data_contains": {"filename": "circular_a.py"} + }, + { + "name": "multiple_self_references", + "description": "Multiple duplicate relative imports — multiple duplicates detected", + "source": "from . import alpha\nfrom . import beta\nfrom . import alpha\nfrom . import beta\n", + "filename": "multi_cycle.py", + "expected_rule": "duplicate_import", + "expected_passed": false, + "expected_message_contains": "Duplicate relative imports", + "expected_data_contains": {"filename": "multi_cycle.py"} + }, + { + "name": "clean_relative_imports", + "description": "Unique relative imports only — no duplicates", + "source": "from . import models\nfrom . import views\nfrom . import controllers\n", + "filename": "clean_package.py", + "expected_rule": "duplicate_import", + "expected_passed": true, + "note": "All relative imports are unique" + }, + { + "name": "absolute_imports_not_flagged", + "description": "Absolute imports are never flagged even if duplicated", + "source": "import os\nimport os\nimport sys\n", + "filename": "absolute.py", + "expected_rule": "duplicate_import", + "expected_passed": true, + "note": "DuplicateImportRule only checks relative from-imports" + }, + { + "name": "syntax_error_skips_cycle_check", + "description": "File with syntax error — duplicate check is skipped", + "source": "from . import module\ndef broken(\n return 42\n", + "filename": "broken_cycle.py", + "expected_rule": "duplicate_import", + "expected_passed": true, + "note": "DuplicateImportRule returns pass on unparseable code" + } + ] +} diff --git a/features/fixtures/validation/cross_file_symbols.json b/features/fixtures/validation/cross_file_symbols.json new file mode 100644 index 000000000..0c4854d02 --- /dev/null +++ b/features/fixtures/validation/cross_file_symbols.json @@ -0,0 +1,62 @@ +{ + "fixtures": [ + { + "name": "undefined_cross_module_reference", + "description": "References a name that would come from another module but is not imported", + "source": "def handler():\n return external_service_xyz.process()\n", + "filename": "handler.py", + "expected_rule": "broken_reference", + "expected_passed": false, + "expected_message_contains": "broken reference", + "expected_data_contains": {"filename": "handler.py"} + }, + { + "name": "properly_imported_cross_module", + "description": "Cross-module symbol is properly imported — passes validation", + "source": "from os.path import join\n\ndef build_path():\n return join('a', 'b', 'c')\n", + "filename": "paths.py", + "expected_rule": "broken_reference", + "expected_passed": true, + "note": "join is properly imported from os.path" + }, + { + "name": "nested_function_scoped_symbol", + "description": "Inner function uses a closure variable from enclosing scope — correctly recognised as defined", + "source": "def outer():\n prefix = 'hello'\n def inner(name):\n return f'{prefix} {name}'\n return inner\n", + "filename": "closure.py", + "expected_rule": "missing_symbol", + "expected_passed": true, + "note": "MissingSymbolRule tracks enclosing function scopes so closure variables are recognised" + }, + { + "name": "undefined_in_class_method", + "description": "Class method references a name not defined anywhere in the file", + "source": "class Service:\n def execute(self):\n return undefined_dependency_xyz\n", + "filename": "service.py", + "expected_rule": "missing_symbol", + "expected_passed": false, + "expected_message_contains": "missing symbol", + "expected_data_contains": {"filename": "service.py"} + }, + { + "name": "wildcard_import_symbol", + "description": "Symbol may come from wildcard import — BrokenReferenceRule flags it because it cannot resolve wildcard imports", + "source": "from os.path import *\n\ndef get_dir():\n return dirname('/tmp/test')\n", + "filename": "wildcard.py", + "expected_rule": "broken_reference", + "expected_passed": false, + "expected_message_contains": "broken reference", + "expected_data_contains": {"filename": "wildcard.py"}, + "note": "BrokenReferenceRule does not resolve wildcard imports; names from star imports are not tracked" + }, + { + "name": "deeply_nested_closure", + "description": "Triple-nested closure referencing variables from all enclosing scopes", + "source": "def level1():\n a = 1\n def level2():\n b = 2\n def level3():\n return a + b\n return level3\n return level2\n", + "filename": "deep_closure.py", + "expected_rule": "missing_symbol", + "expected_passed": true, + "note": "Enclosing scope tracking handles multiple nesting levels" + } + ] +} diff --git a/features/fixtures/validation/dependency_graph_violations.json b/features/fixtures/validation/dependency_graph_violations.json new file mode 100644 index 000000000..6e2ee1977 --- /dev/null +++ b/features/fixtures/validation/dependency_graph_violations.json @@ -0,0 +1,62 @@ +{ + "fixtures": [ + { + "name": "duplicate_relative_imports", + "description": "Same relative import repeated — hints at circular dependency", + "source": "from . import utils\nfrom . import utils\n", + "filename": "module_a.py", + "expected_rule": "duplicate_import", + "expected_passed": false, + "expected_message_contains": "Duplicate relative imports", + "expected_data_contains": {"filename": "module_a.py"} + }, + { + "name": "triple_relative_import_cycle", + "description": "Three identical relative imports — strong cycle indicator", + "source": "from . import core\nfrom . import core\nfrom . import core\n", + "filename": "module_b.py", + "expected_rule": "duplicate_import", + "expected_passed": false, + "expected_message_contains": "Duplicate relative imports", + "expected_data_contains": {"filename": "module_b.py"} + }, + { + "name": "mixed_valid_imports", + "description": "Mix of absolute and unique relative imports — no violations", + "source": "import os\nimport sys\nfrom . import alpha\nfrom . import beta\n", + "filename": "clean_module.py", + "expected_rule": "duplicate_import", + "expected_passed": true, + "note": "All imports are unique" + }, + { + "name": "private_module_import", + "description": "Import from a private underscore module not in stdlib", + "source": "import _nonexistent_internal_module\n", + "filename": "risky_import.py", + "expected_rule": "missing_import", + "expected_passed": false, + "expected_message_contains": "suspicious import", + "expected_data_contains": {"filename": "risky_import.py"} + }, + { + "name": "from_private_module_import", + "description": "From-import from a private underscore module not in stdlib", + "source": "from _nonexistent_private_lib import helper\n", + "filename": "risky_from.py", + "expected_rule": "missing_import", + "expected_passed": false, + "expected_message_contains": "suspicious import", + "expected_data_contains": {"filename": "risky_from.py"} + }, + { + "name": "valid_stdlib_private_import", + "description": "Private module that IS in stdlib — should pass", + "source": "import _thread\n", + "filename": "stdlib_private.py", + "expected_rule": "missing_import", + "expected_passed": true, + "note": "_thread is a valid stdlib module" + } + ] +} diff --git a/features/fixtures/validation/language_porting_mismatches.json b/features/fixtures/validation/language_porting_mismatches.json new file mode 100644 index 000000000..244670d6a --- /dev/null +++ b/features/fixtures/validation/language_porting_mismatches.json @@ -0,0 +1,78 @@ +{ + "fixtures": [ + { + "name": "list_comprehension_side_effect", + "description": "List comprehension used purely for side effects — not portable to languages without expression-level iteration", + "source": "import os\n\n[os.remove(f) for f in ['a.tmp', 'b.tmp']]\n", + "filename": "cleanup.py", + "expected_rule": "api_misuse", + "expected_passed": true, + "note": "api_misuse only flags dangerous APIs; this is a porting smell but not an API misuse" + }, + { + "name": "dynamic_attribute_access", + "description": "Getattr with string-based attribute names — no static equivalent in most typed languages", + "source": "class Config:\n debug = True\n verbose = False\n\ndef get_flag(name):\n return getattr(Config, name)\n", + "filename": "config.py", + "expected_rule": "broken_reference", + "expected_passed": true, + "note": "getattr is valid Python; broken_reference cannot detect dynamic access issues" + }, + { + "name": "multiple_inheritance_diamond", + "description": "Diamond inheritance with MRO — difficult to port to single-inheritance languages", + "source": "class Base:\n def method(self):\n return 'base'\n\nclass Left(Base):\n def method(self):\n return 'left'\n\nclass Right(Base):\n def method(self):\n return 'right'\n\nclass Diamond(Left, Right):\n pass\n\nresult = Diamond().method()\n", + "filename": "diamond.py", + "expected_rule": "syntax_error", + "expected_passed": true, + "note": "Syntactically valid Python; porting concern is semantic not syntactic" + }, + { + "name": "metaclass_usage", + "description": "Metaclass pattern — no direct equivalent in most target languages", + "source": "class Meta(type):\n def __new__(mcs, name, bases, namespace):\n cls = super().__new__(mcs, name, bases, namespace)\n return cls\n\nclass MyClass(metaclass=Meta):\n pass\n", + "filename": "meta.py", + "expected_rule": "syntax_error", + "expected_passed": true, + "note": "Valid Python; metaclasses are a porting concern" + }, + { + "name": "unpacking_star_expression", + "description": "Star unpacking not available in many target languages", + "source": "first, *rest = [1, 2, 3, 4, 5]\nresult = first + sum(rest)\n", + "filename": "unpack.py", + "expected_rule": "broken_reference", + "expected_passed": true, + "note": "Valid Python; star unpacking is a porting concern" + }, + { + "name": "walrus_operator", + "description": "Assignment expression (:=) not available in most languages", + "source": "data = [1, 2, 3, 4, 5, 6]\nresult = [y for x in data if (y := x * 2) > 5]\n", + "filename": "walrus.py", + "expected_rule": "syntax_error", + "expected_passed": true, + "note": "Valid Python 3.8+; walrus operator is a porting concern" + }, + { + "name": "eval_in_porting_context", + "description": "eval() used for dynamic dispatch — API misuse flagged regardless of porting intent", + "source": "def dynamic_call(fn_name, arg):\n return eval(fn_name + '(' + repr(arg) + ')')\n", + "filename": "dynamic_dispatch.py", + "expected_rule": "api_misuse", + "expected_passed": false, + "expected_message_contains": "API misuse", + "expected_data_contains": {"filename": "dynamic_dispatch.py"} + }, + { + "name": "exec_for_code_generation", + "description": "exec() used for code generation — common in porting but always flagged as API misuse", + "source": "def make_class(name):\n exec(f'class {name}: pass')\n", + "filename": "codegen.py", + "expected_rule": "api_misuse", + "expected_passed": false, + "expected_message_contains": "API misuse", + "expected_data_contains": {"filename": "codegen.py"} + } + ] +} diff --git a/features/fixtures/validation/suite_helpers.py b/features/fixtures/validation/suite_helpers.py new file mode 100644 index 000000000..78878fe78 --- /dev/null +++ b/features/fixtures/validation/suite_helpers.py @@ -0,0 +1,173 @@ +"""Shared helpers for semantic validation fixture suites. + +Provides fixture loading, lookup, and rule-map construction used by +Behave steps, Robot Framework helpers, and ASV benchmarks. Centralises +the boilerplate so that adding a new rule or fixture file requires a +single update instead of three. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +from cleveragents.application.services.semantic_validation_rules import ( + APIMisuseRule, + BrokenReferenceRule, + DuplicateImportRule, + MissingImportRule, + MissingSymbolRule, + SemanticCheckResult, + SyntaxCheckRule, +) +from cleveragents.application.services.semantic_validation_service import ( + SemanticValidationRule, +) + +# --------------------------------------------------------------------------- +# Fixture directory +# --------------------------------------------------------------------------- + +FIXTURE_DIR: Path = Path(__file__).resolve().parent + +# --------------------------------------------------------------------------- +# Canonical fixture filenames +# --------------------------------------------------------------------------- + +PORTING_FILENAME = "language_porting_mismatches.json" +DEPENDENCY_FILENAME = "dependency_graph_violations.json" +API_FILENAME = "api_surface_changes.json" +CROSS_FILE_FILENAME = "cross_file_symbols.json" +CIRCULAR_FILENAME = "circular_import_detection.json" + +ALL_FIXTURE_FILENAMES: list[str] = [ + PORTING_FILENAME, + DEPENDENCY_FILENAME, + API_FILENAME, + CROSS_FILE_FILENAME, + CIRCULAR_FILENAME, +] + +# --------------------------------------------------------------------------- +# Rule map (Protocol-typed) +# --------------------------------------------------------------------------- + +RULE_MAP: dict[str, SemanticValidationRule] = { + "syntax_error": SyntaxCheckRule(), + "missing_import": MissingImportRule(), + "broken_reference": BrokenReferenceRule(), + "duplicate_import": DuplicateImportRule(), + "api_misuse": APIMisuseRule(), + "missing_symbol": MissingSymbolRule(), +} + +# --------------------------------------------------------------------------- +# Fixture loading & lookup +# --------------------------------------------------------------------------- + + +def load_fixtures(filename: str) -> list[dict[str, Any]]: + """Load fixtures from a JSON file in the validation fixtures directory.""" + path = FIXTURE_DIR / filename + with open(path, encoding="utf-8") as fh: + data: dict[str, Any] = json.load(fh) + result: list[dict[str, Any]] = data["fixtures"] + return result + + +def find_fixture(fixtures: list[dict[str, Any]], name: str) -> dict[str, Any]: + """Find a fixture by name in a fixture list. + + Raises ``ValueError`` if not found. + """ + for f in fixtures: + if f["name"] == name: + return f + msg = f"Fixture '{name}' not found" + raise ValueError(msg) + + +# --------------------------------------------------------------------------- +# Fixture runner +# --------------------------------------------------------------------------- + + +def run_fixture(fixture: dict[str, Any]) -> SemanticCheckResult: + """Run a single fixture through its expected rule and return the result.""" + rule_name: str = fixture["expected_rule"] + rule = RULE_MAP[rule_name] + source: str = fixture["source"] + filename: str = fixture["filename"] + return rule.check(source, filename) + + +def run_all_fixtures(fixtures: list[dict[str, Any]]) -> list[SemanticCheckResult]: + """Run all fixtures through their expected rules.""" + return [run_fixture(f) for f in fixtures] + + +def verify_fixture_result( + fixture: dict[str, Any], + result: SemanticCheckResult, +) -> None: + """Verify a fixture result against expectations. + + Checks ``passed``, optional ``expected_message_contains``, and + optional ``expected_data_contains``. Raises ``AssertionError`` + on mismatch. + """ + expected_passed: bool = fixture["expected_passed"] + assert result.passed is expected_passed, ( + f"Fixture '{fixture['name']}': expected passed={expected_passed}, " + f"got passed={result.passed}, message='{result.message}'" + ) + + expected_msg: str | None = fixture.get("expected_message_contains") + if expected_msg is not None: + assert expected_msg in result.message, ( + f"Fixture '{fixture['name']}': expected message containing " + f"'{expected_msg}', got '{result.message}'" + ) + + expected_data: dict[str, object] | None = fixture.get("expected_data_contains") + if expected_data is not None: + assert result.data is not None, ( + f"Fixture '{fixture['name']}': expected data containing " + f"{expected_data}, but result.data is None" + ) + for key, value in expected_data.items(): + assert key in result.data, ( + f"Fixture '{fixture['name']}': expected key '{key}' in " + f"result.data, got keys {list(result.data.keys())}" + ) + if isinstance(value, str): + assert value in str(result.data[key]), ( + f"Fixture '{fixture['name']}': expected data['{key}'] " + f"to contain '{value}', got '{result.data[key]}'" + ) + + +def validate_fixture_schema(fixture: dict[str, Any]) -> None: + """Validate that a fixture dict has the required keys. + + Raises ``AssertionError`` if any required key is missing. + """ + required_keys = { + "name", + "description", + "source", + "filename", + "expected_rule", + "expected_passed", + } + actual_keys = set(fixture.keys()) + missing = required_keys - actual_keys + assert not missing, ( + f"Fixture '{fixture.get('name', '')}' is missing " + f"required keys: {missing}" + ) + assert fixture["expected_rule"] in RULE_MAP, ( + f"Fixture '{fixture['name']}' references unknown rule " + f"'{fixture['expected_rule']}'" + ) diff --git a/features/semantic_validation_suite.feature b/features/semantic_validation_suite.feature new file mode 100644 index 000000000..9911a9ff1 --- /dev/null +++ b/features/semantic_validation_suite.feature @@ -0,0 +1,200 @@ +Feature: Semantic validation suite fixtures + As a plan executor + I want semantic validation suites covering porting mismatches, dependency violations, + API surface changes, cross-file symbols, and duplicate relative import detection + So that error patterns are caught and categorised before code reaches production + + Background: + Given a semantic validation suite test environment + + # ── Language-porting mismatches ───────────────────────────────── + + Scenario: Porting fixture list_comprehension_side_effect passes api_misuse rule + Given the porting fixture "list_comprehension_side_effect" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Porting fixture dynamic_attribute_access passes broken_reference rule + Given the porting fixture "dynamic_attribute_access" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Porting fixture multiple_inheritance_diamond passes syntax_error rule + Given the porting fixture "multiple_inheritance_diamond" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Porting fixture metaclass_usage passes syntax_error rule + Given the porting fixture "metaclass_usage" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Porting fixture unpacking_star_expression passes broken_reference rule + Given the porting fixture "unpacking_star_expression" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Porting fixture walrus_operator passes syntax_error rule + Given the porting fixture "walrus_operator" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Porting fixture eval_in_porting_context fails api_misuse rule + Given the porting fixture "eval_in_porting_context" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Porting fixture exec_for_code_generation fails api_misuse rule + Given the porting fixture "exec_for_code_generation" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + # ── Dependency graph violations ───────────────────────────────── + + Scenario: Dependency fixture duplicate_relative_imports fails duplicate_import rule + Given the dependency fixture "duplicate_relative_imports" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Dependency fixture triple_relative_import_cycle fails duplicate_import rule + Given the dependency fixture "triple_relative_import_cycle" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Dependency fixture mixed_valid_imports passes duplicate_import rule + Given the dependency fixture "mixed_valid_imports" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Dependency fixture private_module_import fails missing_import rule + Given the dependency fixture "private_module_import" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Dependency fixture from_private_module_import fails missing_import rule + Given the dependency fixture "from_private_module_import" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Dependency fixture valid_stdlib_private_import passes missing_import rule + Given the dependency fixture "valid_stdlib_private_import" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + # ── API surface changes ───────────────────────────────────────── + + Scenario: API fixture renamed_function_reference fails broken_reference rule + Given the API surface fixture "renamed_function_reference" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: API fixture missing_symbol_in_function fails missing_symbol rule + Given the API surface fixture "missing_symbol_in_function" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: API fixture incompatible_type_usage fails broken_reference rule + Given the API surface fixture "incompatible_type_usage" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: API fixture valid_api_usage passes broken_reference rule + Given the API surface fixture "valid_api_usage" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: API fixture class_method_missing_symbol fails missing_symbol rule + Given the API surface fixture "class_method_missing_symbol" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: API fixture eval_based_dynamic_call fails api_misuse rule + Given the API surface fixture "eval_based_dynamic_call" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: API fixture aliased_pickle_not_caught documents known limitation + Given the API surface fixture "aliased_pickle_not_caught" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: API fixture from_import_alias_not_caught documents known limitation + Given the API surface fixture "from_import_alias_not_caught" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + # ── Cross-file symbol resolution ──────────────────────────────── + + Scenario: Cross-file fixture undefined_cross_module_reference fails broken_reference rule + Given the cross-file fixture "undefined_cross_module_reference" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Cross-file fixture properly_imported_cross_module passes broken_reference rule + Given the cross-file fixture "properly_imported_cross_module" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Cross-file fixture nested_function_scoped_symbol passes missing_symbol rule + Given the cross-file fixture "nested_function_scoped_symbol" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Cross-file fixture undefined_in_class_method fails missing_symbol rule + Given the cross-file fixture "undefined_in_class_method" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Cross-file fixture wildcard_import_symbol fails broken_reference rule + Given the cross-file fixture "wildcard_import_symbol" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Cross-file fixture deeply_nested_closure passes missing_symbol rule + Given the cross-file fixture "deeply_nested_closure" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + # ── Duplicate relative import detection ───────────────────────── + + Scenario: Duplicate import fixture direct_circular_import fails duplicate_import rule + Given the circular import fixture "direct_circular_import" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Duplicate import fixture multiple_self_references fails duplicate_import rule + Given the circular import fixture "multiple_self_references" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Duplicate import fixture clean_relative_imports passes duplicate_import rule + Given the circular import fixture "clean_relative_imports" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Duplicate import fixture absolute_imports_not_flagged passes duplicate_import rule + Given the circular import fixture "absolute_imports_not_flagged" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + Scenario: Duplicate import fixture syntax_error_skips_check passes duplicate_import rule + Given the circular import fixture "syntax_error_skips_cycle_check" + When the suite runs the expected rule + Then the suite result matches the fixture expectation + + # ── Full suite validation ─────────────────────────────────────── + + Scenario: All porting fixtures load and validate schema + Then all porting fixtures load without error + + Scenario: All dependency fixtures load and validate schema + Then all dependency fixtures load without error + + Scenario: All API surface fixtures load and validate schema + Then all API surface fixtures load without error + + Scenario: All cross-file fixtures load and validate schema + Then all cross-file fixtures load without error + + Scenario: All circular import fixtures load and validate schema + Then all circular import fixtures load without error diff --git a/features/steps/semantic_validation_suite_steps.py b/features/steps/semantic_validation_suite_steps.py new file mode 100644 index 000000000..d5183c71c --- /dev/null +++ b/features/steps/semantic_validation_suite_steps.py @@ -0,0 +1,173 @@ +"""Step definitions for semantic validation suite scenarios. + +Loads JSON fixture files covering language-porting mismatches, dependency +graph violations, API surface changes, cross-file symbol resolution, +and circular import detection. Runs the appropriate semantic validation +rule against each fixture and asserts the expected outcome. + +Uses :mod:`features.fixtures.validation.suite_helpers` for shared +loading, lookup, and verification logic. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from typing import TYPE_CHECKING, Any + +from behave import given, then, when + +if TYPE_CHECKING: + from behave.runner import Context + +# Ensure the fixture helpers package is importable +_FIXTURES_PARENT = str(Path(__file__).resolve().parents[1] / "fixtures" / "validation") +if _FIXTURES_PARENT not in sys.path: + sys.path.insert(0, _FIXTURES_PARENT) + +from suite_helpers import ( # noqa: E402 + API_FILENAME, + CIRCULAR_FILENAME, + CROSS_FILE_FILENAME, + DEPENDENCY_FILENAME, + PORTING_FILENAME, + find_fixture, + load_fixtures, + validate_fixture_schema, + verify_fixture_result, +) + +# --------------------------------------------------------------------------- +# Lazy fixture cache (FLAW-3 fix: avoid module-level loading) +# --------------------------------------------------------------------------- + +_fixture_cache: dict[str, list[dict[str, Any]]] = {} + + +def _get_fixtures(filename: str) -> list[dict[str, Any]]: + """Lazily load and cache fixtures by filename.""" + if filename not in _fixture_cache: + _fixture_cache[filename] = load_fixtures(filename) + return _fixture_cache[filename] + + +# --------------------------------------------------------------------------- +# Background +# --------------------------------------------------------------------------- + + +@given("a semantic validation suite test environment") +def step_suite_env(context: Context) -> None: + context.suite_fixture = None + context.suite_result = None + + +# --------------------------------------------------------------------------- +# Given steps - load individual fixtures +# --------------------------------------------------------------------------- + + +@given('the porting fixture "{name}"') +def step_porting_fixture(context: Context, name: str) -> None: + context.suite_fixture = find_fixture(_get_fixtures(PORTING_FILENAME), name) + + +@given('the dependency fixture "{name}"') +def step_dependency_fixture(context: Context, name: str) -> None: + context.suite_fixture = find_fixture(_get_fixtures(DEPENDENCY_FILENAME), name) + + +@given('the API surface fixture "{name}"') +def step_api_fixture(context: Context, name: str) -> None: + context.suite_fixture = find_fixture(_get_fixtures(API_FILENAME), name) + + +@given('the cross-file fixture "{name}"') +def step_cross_file_fixture(context: Context, name: str) -> None: + context.suite_fixture = find_fixture(_get_fixtures(CROSS_FILE_FILENAME), name) + + +@given('the circular import fixture "{name}"') +def step_circular_fixture(context: Context, name: str) -> None: + context.suite_fixture = find_fixture(_get_fixtures(CIRCULAR_FILENAME), name) + + +# --------------------------------------------------------------------------- +# When step - run the expected rule +# --------------------------------------------------------------------------- + + +@when("the suite runs the expected rule") +def step_run_expected_rule(context: Context) -> None: + from suite_helpers import RULE_MAP + + fixture = context.suite_fixture + if fixture is None: + msg = "No fixture loaded" + raise RuntimeError(msg) + rule_name: str = fixture["expected_rule"] + rule = RULE_MAP[rule_name] + source: str = fixture["source"] + filename: str = fixture["filename"] + context.suite_result = rule.check(source, filename) + + +# --------------------------------------------------------------------------- +# Then steps - verify outcomes (with data field verification: FLAW-2 fix) +# --------------------------------------------------------------------------- + + +@then("the suite result matches the fixture expectation") +def step_verify_result(context: Context) -> None: + fixture = context.suite_fixture + result = context.suite_result + if fixture is None or result is None: + msg = "Fixture or result not set" + raise RuntimeError(msg) + + verify_fixture_result(fixture, result) + + +# --------------------------------------------------------------------------- +# Fixture-loading checks (FLAW-4 fix: validate schema, not just len > 0) +# --------------------------------------------------------------------------- + + +@then("all porting fixtures load without error") +def step_all_porting(context: Context) -> None: + fixtures = _get_fixtures(PORTING_FILENAME) + assert len(fixtures) > 0 + for f in fixtures: + validate_fixture_schema(f) + + +@then("all dependency fixtures load without error") +def step_all_dependency(context: Context) -> None: + fixtures = _get_fixtures(DEPENDENCY_FILENAME) + assert len(fixtures) > 0 + for f in fixtures: + validate_fixture_schema(f) + + +@then("all API surface fixtures load without error") +def step_all_api(context: Context) -> None: + fixtures = _get_fixtures(API_FILENAME) + assert len(fixtures) > 0 + for f in fixtures: + validate_fixture_schema(f) + + +@then("all cross-file fixtures load without error") +def step_all_cross_file(context: Context) -> None: + fixtures = _get_fixtures(CROSS_FILE_FILENAME) + assert len(fixtures) > 0 + for f in fixtures: + validate_fixture_schema(f) + + +@then("all circular import fixtures load without error") +def step_all_circular(context: Context) -> None: + fixtures = _get_fixtures(CIRCULAR_FILENAME) + assert len(fixtures) > 0 + for f in fixtures: + validate_fixture_schema(f) diff --git a/robot/helper_semantic_validation_suite.py b/robot/helper_semantic_validation_suite.py new file mode 100644 index 000000000..45d9229bd --- /dev/null +++ b/robot/helper_semantic_validation_suite.py @@ -0,0 +1,168 @@ +"""Helper script for Robot Framework semantic validation suite tests. + +Self-contained Python helper that loads and validates semantic validation +fixture suites (porting mismatches, dependency violations, API surface +changes, cross-file symbols, duplicate relative imports). Prints +sentinel strings on success and exits with code 1 on failure. + +Uses :mod:`features.fixtures.validation.suite_helpers` for shared +loading, lookup, and verification logic. +""" + +from __future__ import annotations + +import sys +from collections.abc import Callable +from pathlib import Path + +# Ensure src and fixture helpers are importable when run from workspace root +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src")) +_FIXTURES_DIR = str( + Path(__file__).resolve().parents[1] / "features" / "fixtures" / "validation" +) +if _FIXTURES_DIR not in sys.path: + sys.path.insert(0, _FIXTURES_DIR) + +from suite_helpers import ( # noqa: E402, I001 + API_FILENAME, + CIRCULAR_FILENAME, + CROSS_FILE_FILENAME, + DEPENDENCY_FILENAME, + PORTING_FILENAME, + load_fixtures, + run_fixture, + validate_fixture_schema, + verify_fixture_result, +) + + +# --------------------------------------------------------------------------- +# Sub-commands +# --------------------------------------------------------------------------- + + +def test_porting_fixtures() -> None: + """Load and verify all porting fixtures.""" + fixtures = load_fixtures(PORTING_FILENAME) + assert len(fixtures) >= 6 + for f in fixtures: + validate_fixture_schema(f) + print("porting-fixtures-ok") + + +def test_dependency_fixtures() -> None: + """Load and verify all dependency fixtures.""" + fixtures = load_fixtures(DEPENDENCY_FILENAME) + assert len(fixtures) >= 6 + for f in fixtures: + validate_fixture_schema(f) + print("dependency-fixtures-ok") + + +def test_api_fixtures() -> None: + """Load and verify all API surface fixtures.""" + fixtures = load_fixtures(API_FILENAME) + assert len(fixtures) >= 6 + for f in fixtures: + validate_fixture_schema(f) + print("api-fixtures-ok") + + +def test_cross_file_fixtures() -> None: + """Load and verify all cross-file fixtures.""" + fixtures = load_fixtures(CROSS_FILE_FILENAME) + assert len(fixtures) >= 5 + for f in fixtures: + validate_fixture_schema(f) + print("cross-file-fixtures-ok") + + +def test_circular_fixtures() -> None: + """Load and verify all duplicate import fixtures.""" + fixtures = load_fixtures(CIRCULAR_FILENAME) + assert len(fixtures) >= 5 + for f in fixtures: + validate_fixture_schema(f) + print("circular-fixtures-ok") + + +def test_run_porting() -> None: + """Run all porting fixtures through their expected rules.""" + fixtures = load_fixtures(PORTING_FILENAME) + for fixture in fixtures: + result = run_fixture(fixture) + verify_fixture_result(fixture, result) + print("run-porting-ok") + + +def test_run_dependency() -> None: + """Run all dependency fixtures through their expected rules.""" + fixtures = load_fixtures(DEPENDENCY_FILENAME) + for fixture in fixtures: + result = run_fixture(fixture) + verify_fixture_result(fixture, result) + print("run-dependency-ok") + + +def test_run_api() -> None: + """Run all API surface fixtures through their expected rules.""" + fixtures = load_fixtures(API_FILENAME) + for fixture in fixtures: + result = run_fixture(fixture) + verify_fixture_result(fixture, result) + print("run-api-ok") + + +def test_run_cross_file() -> None: + """Run all cross-file fixtures through their expected rules.""" + fixtures = load_fixtures(CROSS_FILE_FILENAME) + for fixture in fixtures: + result = run_fixture(fixture) + verify_fixture_result(fixture, result) + print("run-cross-file-ok") + + +def test_run_circular() -> None: + """Run all duplicate import fixtures through their expected rules.""" + fixtures = load_fixtures(CIRCULAR_FILENAME) + for fixture in fixtures: + result = run_fixture(fixture) + verify_fixture_result(fixture, result) + print("run-circular-ok") + + +# --------------------------------------------------------------------------- +# Main +# --------------------------------------------------------------------------- + +_COMMANDS: dict[str, Callable[[], None]] = { + "porting_fixtures": test_porting_fixtures, + "dependency_fixtures": test_dependency_fixtures, + "api_fixtures": test_api_fixtures, + "cross_file_fixtures": test_cross_file_fixtures, + "circular_fixtures": test_circular_fixtures, + "run_porting": test_run_porting, + "run_dependency": test_run_dependency, + "run_api": test_run_api, + "run_cross_file": test_run_cross_file, + "run_circular": test_run_circular, +} + +if __name__ == "__main__": + if len(sys.argv) < 2: + print( + "Usage: helper_semantic_validation_suite.py ", + file=sys.stderr, + ) + sys.exit(1) + + cmd = sys.argv[1] + if cmd not in _COMMANDS: + print(f"Unknown command: {cmd}", file=sys.stderr) + sys.exit(1) + + try: + _COMMANDS[cmd]() + except Exception as exc: + print(f"FAILED: {exc}", file=sys.stderr) + sys.exit(1) diff --git a/robot/semantic_validation_suite.robot b/robot/semantic_validation_suite.robot new file mode 100644 index 000000000..8b916b0f4 --- /dev/null +++ b/robot/semantic_validation_suite.robot @@ -0,0 +1,76 @@ +*** Settings *** +Documentation Integration tests for semantic validation fixture suites +Library Process +Library OperatingSystem +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER_SCRIPT} robot/helper_semantic_validation_suite.py + +*** Test Cases *** +Suite Loads All Porting Fixtures + [Documentation] Verify all language-porting mismatch fixtures load and validate + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} porting_fixtures cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} porting-fixtures-ok + +Suite Loads All Dependency Fixtures + [Documentation] Verify all dependency graph violation fixtures load and validate + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} dependency_fixtures cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} dependency-fixtures-ok + +Suite Loads All API Surface Fixtures + [Documentation] Verify all API surface change fixtures load and validate + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} api_fixtures cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} api-fixtures-ok + +Suite Loads All Cross-File Fixtures + [Documentation] Verify all cross-file symbol resolution fixtures load and validate + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} cross_file_fixtures cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} cross-file-fixtures-ok + +Suite Loads All Circular Import Fixtures + [Documentation] Verify all circular import detection fixtures load and validate + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} circular_fixtures cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} circular-fixtures-ok + +Suite Runs Porting Fixture Through Rules + [Documentation] Run a porting fixture through its expected rule and verify result + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} run_porting cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} run-porting-ok + +Suite Runs Dependency Fixture Through Rules + [Documentation] Run a dependency fixture through its expected rule and verify result + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} run_dependency cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} run-dependency-ok + +Suite Runs API Surface Fixture Through Rules + [Documentation] Run an API surface fixture through its expected rule and verify result + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} run_api cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} run-api-ok + +Suite Runs Cross-File Fixture Through Rules + [Documentation] Run a cross-file fixture through its expected rule and verify result + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} run_cross_file cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} run-cross-file-ok + +Suite Runs Circular Import Fixture Through Rules + [Documentation] Run a circular import fixture through its expected rule and verify result + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} run_circular cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} run-circular-ok + +Suite Unknown Command Fails Gracefully + [Documentation] Unknown command returns error exit code + ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} nonexistent_command cwd=${WORKSPACE} + Should Be Equal As Integers ${result.rc} 1 diff --git a/src/cleveragents/application/services/semantic_validation_rules.py b/src/cleveragents/application/services/semantic_validation_rules.py index 19524468b..7e380849a 100644 --- a/src/cleveragents/application/services/semantic_validation_rules.py +++ b/src/cleveragents/application/services/semantic_validation_rules.py @@ -16,6 +16,7 @@ from __future__ import annotations import ast import builtins as _builtins_mod import sys +from collections import Counter from enum import StrEnum from typing import ClassVar @@ -45,7 +46,7 @@ class SemanticCheckResult(BaseModel): """ passed: bool = Field(..., description="Whether the check passed") - message: str = Field(..., description="Human-readable result description") + message: str = Field(default="", description="Human-readable result description") data: dict[str, object] | None = Field( default=None, description="Optional structured finding data" ) @@ -254,12 +255,32 @@ def _collect_all_scope_names(tree: ast.Module) -> set[str]: return _walk_and_collect(tree, deep=True) -class BrokenReferenceRule: - """Detect references to names not defined anywhere in the module. +_SPECIAL_DUNDER_NAMES: frozenset[str] = frozenset( + { + "__name__", + "__file__", + "__doc__", + "__all__", + "__annotations__", + "__spec__", + "__loader__", + "__package__", + "__builtins__", + "__cached__", + "__path__", + } +) - Uses scope-aware collection that walks the entire AST so that - names defined inside functions, classes, loops, comprehensions, - and exception handlers are properly recognised. + +class BrokenReferenceRule: + """Detect references to names not defined in a reachable scope. + + Uses scope-aware checking: module-level references are validated + against module-level definitions only, while references inside + functions and classes are validated against all definitions + (since they can access enclosing scopes). This avoids false + negatives where a name defined only inside a function is treated + as visible at module scope. """ @property @@ -276,28 +297,48 @@ class BrokenReferenceRule: severity=SemanticValidationSeverity.INFO, ) - defined = _collect_all_scope_names(tree) - defined.update(_BUILTINS_SET) + # Module-level definitions: walk each top-level statement deeply + # so that names bound inside compound statements (try/except, + # if/else, for, while, with) at module scope are captured. + # For FunctionDef/ClassDef at module level we only record the + # name itself — internal bindings are scope-private. + module_defined: set[str] = set() + for stmt in tree.body: + if isinstance(stmt, ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef): + module_defined.add(stmt.name) + else: + for node in ast.walk(stmt): + _collect_binding_names(node, module_defined) + if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Store): + module_defined.add(node.id) + module_defined.update(_BUILTINS_SET) - used: set[str] = set() - for node in ast.walk(tree): - if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load): - used.add(node.id) + # All definitions across every scope (for nested code) + all_defined = _collect_all_scope_names(tree) + all_defined.update(_BUILTINS_SET) - broken = used - defined - broken -= { - "__name__", - "__file__", - "__doc__", - "__all__", - "__annotations__", - "__spec__", - "__loader__", - "__package__", - "__builtins__", - "__cached__", - "__path__", - } + # Partition Name(Load) nodes into module-level vs nested + module_level_refs: set[str] = set() + nested_refs: set[str] = set() + + for stmt in tree.body: + if isinstance(stmt, ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef): + # References inside function/class bodies are nested + for node in ast.walk(stmt): + if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load): + nested_refs.add(node.id) + else: + # Everything else is module-level code + for node in ast.walk(stmt): + if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load): + module_level_refs.add(node.id) + + # Module-level refs: check against module-scope definitions only + broken_module = module_level_refs - module_defined + # Nested refs: check against all definitions (more permissive) + broken_nested = nested_refs - all_defined + + broken = (broken_module | broken_nested) - _SPECIAL_DUNDER_NAMES if broken: limited = sorted(broken)[:10] @@ -352,10 +393,9 @@ class DuplicateImportRule: names = ",".join(a.name for a in (node.names or [])) relative_imports.append(f"{base}:{names}") - if len(relative_imports) != len(set(relative_imports)): - duplicates = [ - m for m in set(relative_imports) if relative_imports.count(m) > 1 - ] + counts = Counter(relative_imports) + duplicates = [m for m, c in counts.items() if c > 1] + if duplicates: return SemanticCheckResult( passed=False, message=( @@ -549,13 +589,79 @@ def _iter_functions( return results +def _walk_scope(node: ast.AST) -> list[ast.AST]: + """Walk *node* without descending into nested function/class bodies. + + Returns all AST nodes belonging to the current scope only. + Nested ``FunctionDef`` / ``AsyncFunctionDef`` / ``ClassDef`` + nodes are yielded (so their *name* is visible) but their + child nodes are **not** traversed. This prevents a parent + function's Load-reference check from seeing names that only + exist inside an inner function's scope. + """ + result: list[ast.AST] = [] + stack = [node] + while stack: + current = stack.pop() + result.append(current) + for child in ast.iter_child_nodes(current): + if child is not node and isinstance( + child, + ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef, + ): + # Yield the node itself (its name is visible) but + # do NOT descend into its body. + result.append(child) + else: + stack.append(child) + return result + + +def _build_enclosing_scope_names( + tree: ast.Module, +) -> dict[int, set[str]]: + """Map each function's ``id()`` to the union of all enclosing scope names. + + Walks the AST recursively, maintaining a stack of name-sets from + enclosing functions. For each ``FunctionDef`` / + ``AsyncFunctionDef`` encountered, the merged enclosing names are + recorded so that :class:`MissingSymbolRule` can recognise closure + variables from outer scopes. + """ + result: dict[int, set[str]] = {} + + def _visit( + node: ast.AST, + scope_stack: list[set[str]], + ) -> None: + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef): + # Record enclosing scope names for this function + enclosing = set[str]() + for scope in scope_stack: + enclosing.update(scope) + result[id(node)] = enclosing + + # Collect this function's local names for its children + local = _collect_function_local_names(node) + child_stack = [*scope_stack, local] + for child in ast.iter_child_nodes(node): + _visit(child, child_stack) + else: + for child in ast.iter_child_nodes(node): + _visit(child, scope_stack) + + _visit(tree, []) + return result + + class MissingSymbolRule: """Detect undefined name usage in function and method bodies. Inspects all functions and methods at any nesting depth (including class methods and nested functions) for names that - are used but not defined locally, at module scope, or in - builtins. + are used but not defined locally, in enclosing function scopes, + at module scope, or in builtins. Closure variables from + enclosing functions are correctly recognised. """ @property @@ -574,16 +680,21 @@ class MissingSymbolRule: module_names = _collect_defined_names(tree) functions = _iter_functions(tree) + enclosing_map = _build_enclosing_scope_names(tree) missing: list[dict[str, object]] = [] for func_node in functions: local_names = _collect_function_local_names(func_node) + enclosing_names = enclosing_map.get(id(func_node), set()) - for child in ast.walk(func_node): + # Use _walk_scope to avoid descending into nested + # function/class bodies — each is checked independently. + for child in _walk_scope(func_node): if isinstance(child, ast.Name) and isinstance(child.ctx, ast.Load): nm = child.id if ( nm not in local_names + and nm not in enclosing_names and nm not in module_names and nm not in _BUILTINS_SET ): @@ -601,7 +712,7 @@ class MissingSymbolRule: passed=False, message=f"Found {len(missing)} missing symbol(s) in {filename}", data={"filename": filename, "missing": limited}, - severity=SemanticValidationSeverity.INFO, + severity=SemanticValidationSeverity.WARN, ) return SemanticCheckResult( passed=True,