Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/cli-output.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@ verdicts, records the affected dead-code candidates as abstentions, emits
`analysis_summary.grep_verify.status` and `incomplete_reason`; increase the
budget and rerun before treating the dead-code result as complete.

Circular dependencies (`SKY-CIRC`) are shown in rich, pretty, and concise
output, and remain available in JSON under `circular_dependencies`. When
source evidence is available, the finding points to an actual import in the
cycle. Ordinary package re-exports do not by themselves form a cycle.

`--strict` counts circular dependencies and exits with status `1` when they
remain in the selected report. Without an explicit `--gate`, concise output
also exits `1` for cycles, as it does for other findings. Ordinary non-strict
gates keep their existing thresholds: cycles do not contribute to
`max_quality` or grades. The existing `--force` override can bypass finding
failures, but incomplete analysis still exits `2`.

The legacy flags still work:

```bash
Expand Down
24 changes: 23 additions & 1 deletion skylos/analysis/circular_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,29 @@ def analyze(self) -> List[CircularDependency]:
return findings

def get_findings(self) -> List[Dict[str, Any]]:
return [cd.to_dict() for cd in self.analyze()]
# Use recorded import edges, not filenames guessed from module names.
# A graph assembled without source evidence should remain locationless.
edge_locations = {}
for dep in self.all_deps:
source = self.modules.get(dep.from_module)
if source and dep.import_line > 0:
edge = (dep.from_module, dep.to_module)
location = (str(source), dep.import_line)
edge_locations[edge] = min(edge_locations.get(edge, location), location)

findings = []
for cd in self.analyze():
finding = cd.to_dict()
edges = zip(cd.cycle, cd.cycle[1:] + cd.cycle[:1])
locations = [
edge_locations[edge] for edge in edges if edge in edge_locations
]
if locations:
# Stable across file discovery order and repeated imports. Only
# edges in this cycle qualify, not chords forming another cycle.
finding["file"], finding["line"] = min(locations)
findings.append(finding)
return findings

def get_core_infrastructure(self) -> Set[str]:
cycles = self.find_simple_cycles()
Expand Down
1 change: 1 addition & 0 deletions skylos/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -2291,6 +2291,7 @@ def _concise_scan_exit_code(
("reliability", "reliability issue"),
("ai_defects", "AI defect"),
("quality", "quality issue"),
("circular_dependencies", "circular dependency"),
("secrets", "secret"),
("custom_rules", "custom rule"),
("dependency_vulnerabilities", "dependency vulnerability"),
Expand Down
3 changes: 3 additions & 0 deletions skylos/core/gatekeeper.py
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ def _check_strict_gate(
reliability,
ai_defects,
quality,
circular_dependencies,
secrets,
dependencies,
):
Expand All @@ -434,6 +435,7 @@ def _check_strict_gate(
+ len(reliability)
+ len(ai_defects)
+ len(gate_quality)
+ len(circular_dependencies)
+ len(secrets)
+ len(dependencies)
)
Expand Down Expand Up @@ -629,6 +631,7 @@ def check_gate(results, config, strict=False, provenance=None):
reliability=reliability,
ai_defects=ai_defects,
quality=gate_quality,
circular_dependencies=results.get("circular_dependencies", []) or [],
secrets=secrets,
dependencies=dependencies,
)
Expand Down
2 changes: 2 additions & 0 deletions skylos/ui/terminal_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
("reliability", "Reliability", "reliability issue"),
("secrets", "Secret", "secret detected"),
("quality", "Quality", "quality issue"),
("circular_dependencies", "Architecture", "circular dependency"),
("custom_rules", "Custom", "custom rule"),
("dependency_vulnerabilities", "Dependency", "dependency vulnerability"),
("unused_functions", "Dead Code", "unused function"),
Expand Down Expand Up @@ -67,6 +68,7 @@
"unused_classes",
"ai_defects",
"quality",
"circular_dependencies",
"custom_rules",
"danger",
"reliability",
Expand Down
82 changes: 82 additions & 0 deletions test/test_circular_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,88 @@ def test_direct_module_self_import_is_not_suppressed(mode):
assert findings[0]["cycle"] == ["module"]


@pytest.mark.parametrize("mode", ["ast", "raw"])
def test_absolute_and_relative_package_reexports_match_reported_example(mode):
rule = _rule_for_sources(
{
"demo_pkg": (
"/project/demo_pkg/__init__.py",
"from demo_pkg.core import value\n__all__ = ['value']\n",
),
"demo_pkg.core": ("/project/demo_pkg/core.py", "value = 1\n"),
"relative_pkg": (
"/project/relative_pkg/__init__.py",
"from .core import value\n__all__ = ['value']\n",
),
"relative_pkg.core": ("/project/relative_pkg/core.py", "value = 2\n"),
"consumer": (
"/project/consumer.py",
"import demo_pkg\nimport relative_pkg\n",
),
},
mode,
)

assert rule.analyze() == []
assert dict(rule._analyzer.dependencies) == {
"demo_pkg": {"demo_pkg.core"},
"relative_pkg": {"relative_pkg.core"},
"consumer": {"demo_pkg", "relative_pkg"},
}


@pytest.mark.parametrize("mode", ["ast", "raw"])
@pytest.mark.parametrize("reverse_files", [False, True])
def test_circular_finding_has_stable_location_on_a_real_cycle_edge(mode, reverse_files):
sources = {
"alpha": (
"/project/alpha.py",
"import helper\n\nfrom beta import value\nfrom beta import other\n",
),
"beta": ("/project/beta.py", "from alpha import value\n"),
"helper": ("/project/helper.py", ""),
}
if reverse_files:
sources = dict(reversed(list(sources.items())))
rule = _rule_for_sources(sources, mode)

findings = rule.analyze()

assert len(findings) == 1
assert set(findings[0]["cycle"]) == {"alpha", "beta"}
assert findings[0]["file"] == "/project/alpha.py"
assert findings[0]["line"] == 3


@pytest.mark.parametrize("mode", ["ast", "raw"])
def test_cycle_location_does_not_use_an_edge_from_another_cycle(mode):
rule = _rule_for_sources(
{
"alpha": ("/project/alpha.py", "import gamma\n\nimport beta\n"),
"beta": ("/project/beta.py", "import gamma\n"),
"gamma": ("/project/gamma.py", "import alpha\n"),
},
mode,
)

findings = rule.analyze()
long_cycle = next(finding for finding in findings if finding["cycle_length"] == 3)

assert long_cycle["file"] == "/project/alpha.py"
assert long_cycle["line"] == 3


def test_manual_cycle_graph_does_not_invent_an_import_location():
analyzer = CircularDependencyAnalyzer()
analyzer.modules = {"a": "a.py", "b": "b.py"}
analyzer.dependencies = {"a": {"b"}, "b": {"a"}}

finding = analyzer.get_findings()[0]

assert "file" not in finding
assert "line" not in finding


@pytest.mark.parametrize("child_source", ["value = 'label'", "import package"])
def test_same_package_graph_has_python_native_cycle_parity(child_source):
if circular_deps._fast_find_cycles is None:
Expand Down
145 changes: 145 additions & 0 deletions test/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -2518,6 +2518,151 @@
mock_print.assert_called_once_with(json.dumps(result))


@pytest.mark.parametrize(
("output_format", "extra_args", "incomplete", "expected_exit", "show_cycle"),
[
pytest.param("concise", [], False, 1, True, id="concise"),
pytest.param("concise", ["--limit", "0"], False, 1, False, id="concise-limit"),
pytest.param("json", ["--strict"], False, 1, True, id="json-strict"),
pytest.param(
"json", ["--strict", "--gate"], False, 1, True, id="json-strict-gate"
),
pytest.param("pretty", ["--strict"], False, 1, True, id="pretty-strict"),
pytest.param("rich", ["--strict"], False, 1, True, id="rich-strict"),
pytest.param(
"concise",
["--strict", "--select=SKY-L012"],
False,
0,
False,
id="concise-unselected",
),
pytest.param(
"json",
["--strict", "--gate", "--select=SKY-L012"],
False,
0,
False,
id="json-strict-gate-unselected",
),
pytest.param("json", ["--gate"], False, 0, True, id="json-ordinary-gate"),
pytest.param("concise", ["--gate"], False, 0, True, id="concise-ordinary-gate"),
pytest.param("json", ["--strict", "--force"], False, 0, True, id="json-forced"),
pytest.param(
"json", ["--strict", "--gate"], True, 2, True, id="json-incomplete"
),
pytest.param(
"concise", ["--force"], True, 2, True, id="concise-incomplete-forced"
),
],
)
def test_main_circular_dependency_reporting_and_exit_codes(

Check warning on line 2559 in test/test_cli.py

View workflow job for this annotation

GitHub Actions / scan

Skylos SKY-C304

Function is 90 lines long (limit: 50).

Check warning on line 2559 in test/test_cli.py

View workflow job for this annotation

GitHub Actions / scan

Skylos SKY-C303

Function has 7 required arguments (limit: 5). Consider using a config object or keyword arguments with defaults.
monkeypatch,
capsys,
output_format,
extra_args,
incomplete,
expected_exit,
show_cycle,
):
cycle = {
"rule_id": "SKY-CIRC",
"kind": "circular_dependency",
"category": "ARCHITECTURE",
"severity": "MEDIUM",
"file": "pkg/left.py",
"line": 4,
"message": "Circular dependency: pkg.left → pkg.right → pkg.left",
"cycle": ["pkg.left", "pkg.right"],
"cycle_length": 2,
"suggested_break": "pkg.left → pkg.right",
}
result = {
"analysis_summary": {"total_files": 2},
"circular_dependencies": [cycle],
}
if incomplete:
result["analysis_errors"] = [
{
"rule_id": "SKY-ANALYSIS-INCOMPLETE",
"kind": "syntax_error",
"severity": "HIGH",
"file": "broken.py",
"line": 1,
"message": "invalid syntax",
}
]
monkeypatch.setattr(
cli.sys,
"argv",
[
"skylos",
".",
"--format",
output_format,
"--no-provenance",
"--no-upload",
*extra_args,
],
)
terminal_output = StringIO()
fake_logger = Mock()
fake_logger.console = Console(
file=terminal_output,
width=160,
force_terminal=False,
theme=cli._skylos_console_theme(),
)
exit_code = 0
with (
patch("skylos.cli.setup_logger", return_value=fake_logger),
patch("skylos.cli.Progress", return_value=_progress_ctx()),
patch("skylos.cli.run_analyze", return_value=json.dumps(result)),
patch("skylos.cli.load_config", return_value={"gate": {"max_quality": 0}}),
patch("skylos.cli.print_badge"),
):
try:
cli.main()
except SystemExit as exc:
exit_code = exc.code

output = capsys.readouterr().out + terminal_output.getvalue()
assert exit_code == expected_exit
if output_format == "json":
rendered = json.loads(output)
assert rendered.get("circular_dependencies", []) == (
[cycle] if show_cycle else []
)
elif show_cycle:
if output_format == "rich":
assert "Circular Dependencies" in output
assert "pkg.left → pkg.right → pkg.left" in output
else:
assert output.count("SKY-CIRC") == 1
assert "pkg/left.py:4" in output
assert cycle["message"] in output
else:
assert "SKY-CIRC" not in output
assert cycle["message"] not in output
if incomplete:
assert "SKY-ANALYSIS-INCOMPLETE" in output


def test_concise_circular_dependency_without_location_remains_visible():
result = {
"circular_dependencies": [
{
"rule_id": "SKY-CIRC",
"message": "Circular dependency: left → right → left",
}
]
}

assert cli._format_concise_results(result) == (
"?:1 SKY-CIRC Circular dependency: left → right → left\n"
)


def test_main_json_incomplete_analysis_exits_two_after_output(monkeypatch):
result = {
"analysis_summary": {
Expand Down
38 changes: 38 additions & 0 deletions test/test_gatekeeper.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,44 @@ def test_check_gate_strict_counts_reliability_findings():
assert reasons == ["Strict mode: 1 issue(s) found"]


def test_check_gate_strict_counts_circular_dependencies_but_not_advisory_quality():
results = {
"circular_dependencies": [
{"rule_id": "SKY-CIRC", "cycle": ["pkg.left", "pkg.right"]},
{"rule_id": "SKY-CIRC", "cycle": ["pkg.other", "pkg.last"]},
],
"quality": [{"rule_id": "SKY-Q802", "advisory": True}],
}

passed, reasons = gk.check_gate(results, {}, strict=True)

assert passed is False
assert reasons == ["Strict mode: 2 issue(s) found"]


def test_circular_dependencies_do_not_change_ordinary_gate_thresholds():
results = {
"circular_dependencies": [
{"rule_id": "SKY-CIRC", "severity": "HIGH", "cycle": ["left", "right"]}
],
}

passed, reasons = gk.check_gate(
results,
{
"gate": {
"max_quality": 0,
"max_high": 0,
"max_security": 0,
"max_dead_code": 0,
}
},
)

assert passed is True
assert reasons == []


def test_reliability_does_not_affect_security_thresholds():
results = {
"danger": [],
Expand Down
Loading
Loading