diff --git a/desloppify/engine/_plan/step_completion.py b/desloppify/engine/_plan/step_completion.py index 10822047a..e5f0f29d7 100644 --- a/desloppify/engine/_plan/step_completion.py +++ b/desloppify/engine/_plan/step_completion.py @@ -4,14 +4,20 @@ def auto_complete_steps(plan: dict) -> list[str]: - """Mark steps done when all their issue_refs are no longer in the queue. + """Mark steps done when all their issue_refs leave the living plan. Returns list of human-readable messages for completed steps. """ messages: list[str] = [] - queue_set = set(plan.get("queue_order", [])) + actionable_ids = set(plan.get("queue_order", [])) + actionable_ids.update((plan.get("skipped") or {}).keys()) + actionable_ids.update(plan.get("promoted_ids") or []) + for cluster in (plan.get("clusters") or {}).values(): + if isinstance(cluster, dict): + actionable_ids.update(cluster.get("issue_ids") or []) for name, cluster in plan.get("clusters", {}).items(): + cluster_issue_ids = set(cluster.get("issue_ids") or []) for i, step in enumerate(cluster.get("action_steps") or []): if not isinstance(step, dict) or step.get("done"): continue @@ -20,9 +26,19 @@ def auto_complete_steps(plan: dict) -> list[str]: continue # Match by suffix: ref "abc123" matches "review::path::abc123" all_gone = all( - not any(qid.endswith(ref) or qid == ref for qid in queue_set) + not any( + issue_id.endswith(ref) or issue_id == ref + for issue_id in actionable_ids + ) for ref in refs ) + # Some triage runners emit summary hashes as step refs while the + # living plan stores canonical issue IDs. If the cluster still + # has members, an unmatched ref is ambiguous rather than proof + # that the step's finding was resolved. Fail closed and leave + # the step open until its cluster membership is drained. + if all_gone and cluster_issue_ids: + continue if all_gone: step["done"] = True messages.append( diff --git a/desloppify/engine/_work_queue/snapshot.py b/desloppify/engine/_work_queue/snapshot.py index 04984cded..9de758948 100644 --- a/desloppify/engine/_work_queue/snapshot.py +++ b/desloppify/engine/_work_queue/snapshot.py @@ -219,6 +219,10 @@ def _subjective_partitions( candidates = build_subjective_items( state, scoped_issues, threshold=threshold, plan=plan ) + skipped_ids = set((plan or {}).get("skipped", {}).keys()) + candidates = [ + item for item in candidates if item.get("id", "") not in skipped_ids + ] initial = [item for item in candidates if item.get("initial_review")] postflight = [item for item in candidates if not item.get("initial_review")] return initial, postflight diff --git a/desloppify/languages/typescript/detectors/smells/detector_core.py b/desloppify/languages/typescript/detectors/smells/detector_core.py index 12e08d88a..11f1d2384 100644 --- a/desloppify/languages/typescript/detectors/smells/detector_core.py +++ b/desloppify/languages/typescript/detectors/smells/detector_core.py @@ -7,7 +7,6 @@ from .helpers import ( _code_text, _strip_ts_comments, - _track_brace_body, ) _MONSTER_FUNCTION_LOC = 150 @@ -19,7 +18,12 @@ _MAX_CATCH_BODY = 1000 _MAX_SWITCH_BODY_SCAN = 5000 -_ERROR_HANDLER_BASENAMES = ("logger", "errorpresentation", "errorhandler", "errorreporting") +_ERROR_HANDLER_BASENAMES = ( + "logger", + "errorpresentation", + "errorhandler", + "errorreporting", +) _PRECEDING_SKIP_PATTERNS = re.compile( r"componentDidCatch|import\.meta\.env\.DEV|process\.env\.NODE_ENV" ) @@ -84,7 +88,9 @@ def _find_function_start(line: str, next_lines: list[str]) -> str | None: if not assignment_match: return None - combined = "\n".join([stripped] + [next_line.strip() for next_line in next_lines[:2]]) + combined = "\n".join( + [stripped] + [next_line.strip() for next_line in next_lines[:2]] + ) eq_pos = combined.find("=", assignment_match.end()) if eq_pos == -1: return None @@ -96,7 +102,9 @@ def _find_function_start(line: str, next_lines: list[str]) -> str | None: return None -def _find_opening_brace_line(lines: list[str], start: int, *, window: int = 5) -> int | None: +def _find_opening_brace_line( + lines: list[str], start: int, *, window: int = 5 +) -> int | None: for idx in range(start, min(start + window, len(lines))): if "{" in lines[idx]: return idx @@ -104,21 +112,68 @@ def _find_opening_brace_line(lines: list[str], start: int, *, window: int = 5) - def _extract_function_body( - lines: list[str], start_line: int, *, max_scan: int = 2000, + lines: list[str], + start_line: int, + *, + max_scan: int = 2000, ) -> str | None: """Extract the inner body text of a function starting at start_line.""" - brace_line = _find_opening_brace_line(lines, start_line, window=5) - if brace_line is None: - return None - end_line = _track_brace_body(lines, brace_line, max_scan=max_scan) - if end_line is None: - return None - body_text = "\n".join(lines[brace_line : end_line + 1]) - first_brace = body_text.find("{") - last_brace = body_text.rfind("}") - if first_brace == -1 or last_brace == -1 or first_brace >= last_brace: + segment = "\n".join(lines[start_line : min(start_line + max_scan, len(lines))]) + code = _code_text(segment) + paren_depth = 0 + bracket_depth = 0 + signature_brace_depth = 0 + parameter_list_started = False + parameters_closed = False + previous_code_character = "" + body_start: int | None = None + + for index, character in enumerate(code): + if character == "(": + parameter_list_started = True + paren_depth += 1 + elif character == ")" and paren_depth > 0: + paren_depth -= 1 + if parameter_list_started and paren_depth == 0: + parameters_closed = True + elif character == "[": + bracket_depth += 1 + elif character == "]" and bracket_depth > 0: + bracket_depth -= 1 + elif character == "{" and paren_depth == 0 and bracket_depth == 0: + if not parameters_closed: + # Generic constraints can contain object types before the parameter list. + # They cannot be executable bodies for the function shapes detected here. + pass + elif signature_brace_depth > 0: + signature_brace_depth += 1 + elif previous_code_character in ":<|&,(=[": + # Object-shaped parameter/return types are part of the signature, not + # the executable function body. Keep scanning after their matching brace. + signature_brace_depth = 1 + else: + body_start = index + break + elif character == "}" and signature_brace_depth > 0: + signature_brace_depth -= 1 + + if not character.isspace(): + previous_code_character = character + + if body_start is None: return None - return body_text[first_brace + 1 : last_brace] + + depth = 0 + for index in range(body_start, len(code)): + character = code[index] + if character == "{": + depth += 1 + elif character == "}": + depth -= 1 + if depth == 0: + return segment[body_start + 1 : index] + + return None def _count_pattern_in_body(body: str, pattern: re.Pattern[str]) -> int: diff --git a/desloppify/languages/typescript/detectors/unused.py b/desloppify/languages/typescript/detectors/unused.py index 16cfca3fb..9226aecf0 100644 --- a/desloppify/languages/typescript/detectors/unused.py +++ b/desloppify/languages/typescript/detectors/unused.py @@ -78,24 +78,44 @@ def _run_tsc_unused_check( ) +def _find_base_tsconfig(path: Path, project_root: Path) -> Path: + """Find the closest TypeScript project config that owns the scan path.""" + scan_path = path.resolve() + root = project_root.resolve() + + for directory in (scan_path, *scan_path.parents): + if directory == root.parent: + break + candidate = directory / "tsconfig.json" + if candidate.is_file(): + return candidate + if directory == root: + break + + app_config = root / "tsconfig.app.json" + return app_config if app_config.is_file() else root / "tsconfig.json" + + def detect_unused(path: Path, category: str = "all") -> tuple[list[dict], int]: ts_files = find_ts_and_tsx_files(path) total_files = len(ts_files) if _should_use_deno_fallback(path, ts_files): return _detect_unused_fallback(path, category) + project_root = get_project_root() + base_tsconfig = _find_base_tsconfig(path, project_root) tmp_tsconfig = { - "extends": "./tsconfig.app.json", + "extends": f"./{base_tsconfig.name}", "compilerOptions": { "noUnusedLocals": True, "noUnusedParameters": True, }, } - tmp_path = get_project_root() / "tsconfig.desloppify.json" + tmp_path = base_tsconfig.parent / "tsconfig.desloppify.json" try: safe_write_text(tmp_path, json.dumps(tmp_tsconfig, indent=2)) try: - result = _run_tsc_unused_check(get_project_root(), tmp_path) + result = _run_tsc_unused_check(project_root, tmp_path) except (_proc_runtime.SubprocessError, OSError) as exc: logger.debug("Falling back to source-based unused detection: %s", exc) return _detect_unused_fallback(path, category) @@ -146,11 +166,19 @@ def detect_unused(path: Path, category: str = "all") -> tuple[list[dict], int]: def _categorize_unused(filepath: str, lineno: int) -> str: try: - p = Path(filepath) if Path(filepath).is_absolute() else get_project_root() / filepath + p = ( + Path(filepath) + if Path(filepath).is_absolute() + else get_project_root() / filepath + ) lines = p.read_text().splitlines() if lineno <= len(lines): src_line = lines[lineno - 1].strip() - if src_line.startswith("import ") or "from '" in src_line or 'from "' in src_line: + if ( + src_line.startswith("import ") + or "from '" in src_line + or 'from "' in src_line + ): return "imports" if src_line.startswith( ( @@ -173,7 +201,9 @@ def _categorize_unused(filepath: str, lineno: int) -> str: if prev.startswith("import "): return "imports" if not prev or ( - not prev.startswith("{") and not prev.startswith(",") and "," not in prev + not prev.startswith("{") + and not prev.startswith(",") + and "," not in prev ): break except (OSError, UnicodeDecodeError) as exc: @@ -193,7 +223,9 @@ def cmd_unused(args: argparse.Namespace) -> None: file=sys.stderr, ) else: - print(colorize("Running tsc... (this may take a moment)", "dim"), file=sys.stderr) + print( + colorize("Running tsc... (this may take a moment)", "dim"), file=sys.stderr + ) entries, _ = detect_unused(path, args.category) if args.json: diff --git a/desloppify/languages/typescript/test_coverage.py b/desloppify/languages/typescript/test_coverage.py index 5e777020e..58c14155b 100644 --- a/desloppify/languages/typescript/test_coverage.py +++ b/desloppify/languages/typescript/test_coverage.py @@ -23,6 +23,7 @@ re.compile(p) for p in [ r"expect\(", + r"\bexpectTypeOf(?:\s*<[^;()]+>)?\s*\(", r"assert\.", r"\bassert(?:[A-Z]\w*)?\(", r"\.should\.", diff --git a/desloppify/languages/typescript/tests/smells/test_ts_smell_helpers.py b/desloppify/languages/typescript/tests/smells/test_ts_smell_helpers.py index 27c140209..957a018ed 100644 --- a/desloppify/languages/typescript/tests/smells/test_ts_smell_helpers.py +++ b/desloppify/languages/typescript/tests/smells/test_ts_smell_helpers.py @@ -1,5 +1,6 @@ """Tests for desloppify.languages.typescript.detectors.smells.helpers.""" +from desloppify.languages.typescript.detectors.smells import TS_SMELL_CHECKS from desloppify.languages.typescript.detectors.smells.detector_core import ( _find_function_start, ) @@ -25,7 +26,6 @@ _track_brace_body, _ts_match_is_in_string, ) -from desloppify.languages.typescript.detectors.smells import TS_SMELL_CHECKS def _ctx(content: str, filepath: str = "test.ts") -> _FileContext: @@ -329,6 +329,66 @@ def test_skips_async_with_await(self): _detect_async_no_await(_ctx(content), counts) assert len(counts["async_no_await"]) == 0 + def test_skips_await_after_multiline_object_parameter_type(self): + content = """async function load( + tx: Transaction, + args: { + readonly accountId: string; + readonly customerId: string; + }, +): Promise { + const result = await tx.execute(args); + return result; +} +""" + counts = _make_counts() + _detect_async_no_await(_ctx(content), counts) + assert len(counts["async_no_await"]) == 0 + + def test_skips_await_after_extract_parameter_type(self): + content = """async function load( + reference: Extract, +): Promise { + return await resolveReference(reference); +} +""" + counts = _make_counts() + _detect_async_no_await(_ctx(content), counts) + assert len(counts["async_no_await"]) == 0 + + def test_skips_await_after_object_return_type(self): + content = """async function load(): Promise<{ + readonly accountId: string; +}> { + return await resolveAccount(); +} +""" + counts = _make_counts() + _detect_async_no_await(_ctx(content), counts) + assert len(counts["async_no_await"]) == 0 + + def test_skips_await_after_generic_object_constraint(self): + content = """async function load(args: { + readonly value: T; +}): Promise { + return await resolveValue(args.value); +} +""" + counts = _make_counts() + _detect_async_no_await(_ctx(content), counts) + assert len(counts["async_no_await"]) == 0 + + def test_flags_multiline_object_parameter_without_await(self): + content = """async function load(args: { + readonly accountId: string; +}): Promise { + return args.accountId; +} +""" + counts = _make_counts() + _detect_async_no_await(_ctx(content), counts) + assert len(counts["async_no_await"]) == 1 + def test_arrow_async_without_await(self): content = "const fn = async () => {\n return 42;\n}\n" counts = _make_counts() @@ -337,7 +397,9 @@ def test_arrow_async_without_await(self): def test_await_in_comment_still_flagged(self): """'await' inside a comment should not count — function is still flagged.""" - content = "async function fetchData() {\n // await fetch('/');\n return 1;\n}\n" + content = ( + "async function fetchData() {\n // await fetch('/');\n return 1;\n}\n" + ) counts = _make_counts() _detect_async_no_await(_ctx(content), counts) assert len(counts["async_no_await"]) == 1 diff --git a/desloppify/languages/typescript/tests/test_ts_unused.py b/desloppify/languages/typescript/tests/test_ts_unused.py index d9ba376df..f1541e0b3 100644 --- a/desloppify/languages/typescript/tests/test_ts_unused.py +++ b/desloppify/languages/typescript/tests/test_ts_unused.py @@ -4,6 +4,7 @@ so we test what is feasible: the helper function _categorize_unused and module imports. """ +import json from pathlib import Path import pytest @@ -161,7 +162,7 @@ def _fake_run(*args, **kwargs): "--project", str(tsconfig), "--noEmit", - ] + ] assert recorded["cwd"] == tmp_path assert recorded["timeout"] == 120 @@ -171,7 +172,9 @@ def test_run_tsc_unused_check_raises_without_npx(self, tmp_path, monkeypatch): with pytest.raises(OSError, match="TypeScript compiler not found"): ts_unused_mod._run_tsc_unused_check(tmp_path, tmp_path / "tsconfig.json") - def test_detect_unused_uses_deno_fallback_for_url_imports(self, tmp_path, monkeypatch): + def test_detect_unused_uses_deno_fallback_for_url_imports( + self, tmp_path, monkeypatch + ): """Deno-style URL imports should bypass tsc and use source-based fallback.""" _write( tmp_path, @@ -227,9 +230,7 @@ def test_detect_unused_non_deno_keeps_tsc_path(self, tmp_path, monkeypatch): _write(tmp_path, "src/app.ts", "const x = 1;\n") class _Result: - stdout = ( - "src/app.ts(1,7): error TS6133: 'x' is declared but its value is never read.\n" - ) + stdout = "src/app.ts(1,7): error TS6133: 'x' is declared but its value is never read.\n" stderr = "" calls = {"count": 0} @@ -249,6 +250,43 @@ def _fake_run(*args, **kwargs): assert total == 1 assert entries and entries[0]["name"] == "x" + def test_detect_unused_uses_nearest_project_tsconfig(self, tmp_path, monkeypatch): + """A monorepo subproject scan should not compile the root project config.""" + api_path = tmp_path / "services" / "api" + _write(tmp_path, "tsconfig.json", '{"files": []}\n') + _write(api_path, "tsconfig.json", '{"include": ["src/**/*.ts"]}\n') + _write(api_path, "src/app.ts", "const x = 1;\n") + recorded: dict[str, object] = {} + + class _Result: + stdout = "" + stderr = "" + + def _fake_run(*args, **kwargs): + tsconfig_path = Path(args[0][3]) + recorded["path"] = tsconfig_path + recorded["config"] = json.loads(tsconfig_path.read_text()) + return _Result() + + monkeypatch.setattr( + ts_unused_mod.shutil, + "which", + lambda name: "/opt/homebrew/bin/npx" if name == "npx" else None, + ) + monkeypatch.setattr(ts_unused_mod._proc_runtime, "run", _fake_run) + + detect_unused(api_path) + + assert recorded["path"] == api_path / "tsconfig.desloppify.json" + assert recorded["config"] == { + "extends": "./tsconfig.json", + "compilerOptions": { + "noUnusedLocals": True, + "noUnusedParameters": True, + }, + } + assert not (api_path / "tsconfig.desloppify.json").exists() + def test_detect_unused_root_deno_lock_does_not_force_fallback( self, tmp_path, monkeypatch ): @@ -257,9 +295,7 @@ def test_detect_unused_root_deno_lock_does_not_force_fallback( _write(tmp_path, "src/app.ts", "const x = 1;\n") class _Result: - stdout = ( - "src/app.ts(1,7): error TS6133: 'x' is declared but its value is never read.\n" - ) + stdout = "src/app.ts(1,7): error TS6133: 'x' is declared but its value is never read.\n" stderr = "" calls = {"count": 0} diff --git a/desloppify/tests/detectors/coverage/test_test_coverage_mapping_import_and_logic.py b/desloppify/tests/detectors/coverage/test_test_coverage_mapping_import_and_logic.py index 4a17e2326..f32e48986 100644 --- a/desloppify/tests/detectors/coverage/test_test_coverage_mapping_import_and_logic.py +++ b/desloppify/tests/detectors/coverage/test_test_coverage_mapping_import_and_logic.py @@ -326,6 +326,35 @@ def test_py_comment_strips_after_code(self): # ── RTL assertion patterns ─────────────────────────────── +class TestTypeLevelAssertions: + def test_vitest_expect_type_of_calls_count_as_assertions(self, tmp_path): + content = ( + "import { expectTypeOf, it } from 'vitest';\n" + "it('checks types', () => {\n" + " expectTypeOf(value).toEqualTypeOf();\n" + " expectTypeOf>>().toMatchTypeOf(value);\n" + "});\n" + ) + tf = _write_file(tmp_path, "types.test.ts", content) + + result = analyze_test_quality({tf}, "typescript") + + assert result[tf]["assertions"] == 2 + assert result[tf]["quality"] == "adequate" + + def test_imported_expect_type_of_without_call_is_not_an_assertion(self, tmp_path): + content = ( + "import { expectTypeOf, it } from 'vitest';\n" + "it('does not check a type', () => {});\n" + ) + tf = _write_file(tmp_path, "types-unused.test.ts", content) + + result = analyze_test_quality({tf}, "typescript") + + assert result[tf]["assertions"] == 0 + assert result[tf]["quality"] == "assertion_free" + + class TestRTLPatterns: def test_getby_counted(self, tmp_path): content = "it(\"renders\", () => {\n screen.getByText('hello');\n});\n" diff --git a/desloppify/tests/engine/test_sync_split_modules_direct.py b/desloppify/tests/engine/test_sync_split_modules_direct.py index a5f678799..0f636e754 100644 --- a/desloppify/tests/engine/test_sync_split_modules_direct.py +++ b/desloppify/tests/engine/test_sync_split_modules_direct.py @@ -832,6 +832,53 @@ def test_queue_snapshot_keeps_executing_real_queue_items_before_postflight_scan( assert [item["id"] for item in snapshot.execution_items] == ["unused::a"] +def test_queue_snapshot_skipped_subjective_item_does_not_mask_execution() -> None: + state = { + "issues": { + "unused::a": { + "id": "unused::a", + "detector": "unused", + "status": "open", + "file": "src/a.py", + "tier": 1, + "confidence": "high", + "summary": "unused import", + "detail": {}, + } + }, + "dimension_scores": { + "Naming quality": { + "score": 70.0, + "strict": 70.0, + "failing": 1, + "detectors": { + "subjective_assessment": {"dimension_key": "naming_quality"} + }, + } + }, + "subjective_assessments": {"naming_quality": {"score": 70.0}}, + } + plan = { + "queue_order": ["unused::a", "subjective::naming_quality"], + "skipped": { + "subjective::naming_quality": { + "issue_id": "subjective::naming_quality", + "kind": "temporary", + } + }, + "plan_start_scores": {"strict": 80.0}, + "refresh_state": {"lifecycle_phase": "plan"}, + } + + snapshot = snapshot_mod.build_queue_snapshot(state, plan=plan) + + assert snapshot.phase == refresh_lifecycle_mod.LIFECYCLE_PHASE_EXECUTE + assert [item["id"] for item in snapshot.execution_items] == ["unused::a"] + assert "subjective::naming_quality" not in { + item["id"] for item in snapshot.backlog_items + } + + def test_queue_snapshot_does_not_execute_autofix_cluster_without_queue_ownership() -> None: state = { "issues": { diff --git a/desloppify/tests/plan/test_step_completion_direct.py b/desloppify/tests/plan/test_step_completion_direct.py index 4ce32f068..56611a3c7 100644 --- a/desloppify/tests/plan/test_step_completion_direct.py +++ b/desloppify/tests/plan/test_step_completion_direct.py @@ -65,3 +65,32 @@ def test_auto_complete_steps_ignores_done_steps_and_invalid_step_shapes() -> Non assert messages == [] assert plan["clusters"]["epic/mixed"]["action_steps"][0]["done"] is True + + +def test_auto_complete_steps_keeps_clustered_issue_outside_execution_queue_open() -> ( + None +): + plan = { + "queue_order": ["review::selected::issue-a"], + "clusters": { + "epic/selected": { + "issue_ids": ["review::selected::issue-a"], + "action_steps": [ + {"title": "Fix selected", "issue_refs": ["issue-a"]}, + ], + }, + "epic/not-selected": { + "issue_ids": ["review::not-selected::issue-b"], + "action_steps": [ + {"title": "Fix later", "issue_refs": ["summary-hash-b"]}, + ], + }, + }, + } + + messages = auto_complete_steps(plan) + + assert messages == [] + assert ( + plan["clusters"]["epic/not-selected"]["action_steps"][0].get("done") is not True + )