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
40 changes: 39 additions & 1 deletion scripts/commands/impact.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,11 @@
f"Choices: {', '.join(ALL_CHECKS)}. Default: impact.")
parser.add_argument("--name", default=None,
help="impact: symbol name to analyze")
parser.add_argument("--action", choices=["modify", "delete"], default="modify",
parser.add_argument("--action", choices=["modify", "delete", "rename"], default="modify",
help="impact: planned action (default: modify)")
parser.add_argument("--new-name", default=None,
help="impact: new symbol name, required with --action rename "
"(issue #241)")
parser.add_argument("--domain", default="auto",
help="impact: frontend|backend|auto (default: auto)")
parser.add_argument("--depth", type=int, default=None,
Expand Down Expand Up @@ -111,14 +114,49 @@
return parts or ["impact"]


def _run_legacy_impact(args, workspace):

Check failure on line 117 in scripts/commands/impact.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=Wolfvin_CodeLens&issues=AZ9WZ0OPLy-gnkPqZyfm&open=AZ9WZ0OPLy-gnkPqZyfm&pullRequest=247
"""Run the original impact.execute logic (issue #195: absorbed)."""
from impact_engine import analyze_impact
name = getattr(args, "name", None) or ""
action = getattr(args, "action", "modify")
domain = getattr(args, "domain", "auto")
depth = getattr(args, "depth", None) or 5
new_name = getattr(args, "new_name", None)

# Issue #241: rename simulation — every real call site needs updating
# to the new name, unlike modify (same signature, callers unaffected)
# or delete (callers need to stop referencing it entirely).
if action == "rename" and not new_name:
return {
"status": "error",
"error": "--action rename requires --new-name <new symbol name>",
}

result = analyze_impact(name, workspace, action=action, domain=domain, depth=depth)
if action == "rename" and result.get("status") == "ok":
result["new_name"] = new_name
checklist = []
for item in result.get("affected", {}).get("direct", []):
checklist.append({
"file": item.get("file"),
"line": item.get("line"),
"caller": item.get("name"),
})
result["rename_checklist"] = checklist
result["rename_caveat"] = (
f"This lists {len(checklist)} statically-resolved call site(s) that "
f"reference '{name}' and need updating to '{new_name}'. It does NOT "
"catch dynamic/string-based references (e.g. dynamic import(), "
"reflection, string-keyed dispatch tables, or the identifier "
"appearing in comments/docs) — grep for the old name as a final "
"check before considering the rename complete."
)
result.setdefault("recommendations", []).insert(
0,
f"Update {len(checklist)} call site(s) listed in rename_checklist, "
f"then grep for remaining '{name}' references (dynamic/string-based "
"usage is not covered by this analysis).",
)
if result.get("status") == "ok":
engine_risk = result.get("risk", "low")
stats = result.get("stats", {})
Expand Down
100 changes: 100 additions & 0 deletions tests/test_impact_command.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
"""Tests for the impact command's --action rename support (issue #241).

`impact_engine.analyze_impact(action="delete"|"modify")` already computed
most of what a "safe to change" sandbox needs — this extends it to rename,
the most common refactor an AI agent performs, by attaching a concrete
checklist of every statically-resolved call site that needs updating to
the new name, plus an explicit caveat about what's NOT covered (dynamic/
string-based references).
"""

import argparse
import os
import sys
from unittest import mock

import pytest

SCRIPT_DIR = os.path.join(
os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts"
)
if SCRIPT_DIR not in sys.path:
sys.path.insert(0, SCRIPT_DIR)

from commands.impact import _run_legacy_impact # noqa: E402


def _args(**overrides):
ns = argparse.Namespace(
name="oldName", action="modify", domain="auto", depth=None, new_name=None,
)
for k, v in overrides.items():
setattr(ns, k, v)
return ns


def _fake_analyze_impact_result():
return {
"status": "ok",
"symbol": "oldName",
"action": "rename",
"risk": "medium",
"affected": {
"direct": [
{"name": "callerA", "file": "a.ts", "line": 10},
{"name": "callerB", "file": "b.ts", "line": 22},
],
"indirect": [],
"files": ["a.ts", "b.ts"],
"tests": [],
},
"stats": {
"direct_dependents": 2,
"indirect_dependents": 0,
"affected_files": 2,
"test_files_found": 0,
},
}


class TestRenameAction:
def test_rename_without_new_name_errors(self):
result = _run_legacy_impact(_args(action="rename"), ".")
assert result["status"] == "error"
assert "--new-name" in result["error"]

def test_rename_with_new_name_produces_checklist(self):
with mock.patch(
"impact_engine.analyze_impact",
return_value=_fake_analyze_impact_result(),
):
result = _run_legacy_impact(
_args(action="rename", new_name="newName"), "."
)

assert result["status"] == "ok"
assert result["new_name"] == "newName"
assert len(result["rename_checklist"]) == 2
assert result["rename_checklist"][0] == {
"file": "a.ts", "line": 10, "caller": "callerA",
}
assert "dynamic" in result["rename_caveat"]
assert "rename_checklist" in result["recommendations"][0]

def test_modify_action_unaffected_by_rename_logic(self):
"""--action modify (the default) must not get rename_checklist or
the rename-specific error path — regression guard for the new
branching added alongside rename support."""
with mock.patch(
"impact_engine.analyze_impact",
return_value={
"status": "ok", "risk": "low",
"affected": {"direct": [], "indirect": [], "files": [], "tests": []},
"stats": {"direct_dependents": 0, "indirect_dependents": 0,
"affected_files": 0, "test_files_found": 0},
},
):
result = _run_legacy_impact(_args(action="modify"), ".")

assert "rename_checklist" not in result
assert "new_name" not in result
Loading