diff --git a/desloppify/app/commands/helpers/lang.py b/desloppify/app/commands/helpers/lang.py index e1e932161..704261e23 100644 --- a/desloppify/app/commands/helpers/lang.py +++ b/desloppify/app/commands/helpers/lang.py @@ -94,7 +94,7 @@ def resolve_detection_root( markers = marker_provider() project_root_path = ( project_root if project_root is not None else get_project_root() - ) + ).resolve() raw_path = getattr(args, "path", None) if not raw_path: @@ -106,7 +106,12 @@ def resolve_detection_root( candidate = candidate.resolve() candidate_root = candidate if candidate.is_dir() else candidate.parent - for probe_root in (candidate_root, *candidate_root.parents): + probe_roots = (candidate_root, *candidate_root.parents) + if candidate_root.is_relative_to(project_root_path): + project_root_index = probe_roots.index(project_root_path) + probe_roots = probe_roots[: project_root_index + 1] + + for probe_root in probe_roots: if any((probe_root / marker).exists() for marker in markers): return probe_root return candidate_root diff --git a/desloppify/languages/_framework/treesitter/imports/graph.py b/desloppify/languages/_framework/treesitter/imports/graph.py index 361586356..728208716 100644 --- a/desloppify/languages/_framework/treesitter/imports/graph.py +++ b/desloppify/languages/_framework/treesitter/imports/graph.py @@ -7,13 +7,33 @@ from pathlib import Path from typing import TYPE_CHECKING, Any -from .cache import get_or_parse_tree +from desloppify.base.discovery.file_paths import resolve_scan_file + from ..analysis.extractors import _get_parser, _make_query, _run_query, _unwrap_node +from .cache import get_or_parse_tree if TYPE_CHECKING: from desloppify.languages._framework.treesitter import TreeSitterLangSpec +def _source_path(filepath: str) -> Path: + """Resolve discovery keys against the project root without changing case.""" + return resolve_scan_file(filepath) + + +def _import_path(filepath: str, scan_path: Path) -> Path: + """Resolve a resolver result using its scan-root-relative contract.""" + path = Path(filepath) + if path.is_absolute(): + return path.resolve() + return (scan_path / path).resolve() + + +def _path_identity(path: Path) -> str: + """Return a comparison identity without changing the path used for I/O.""" + return os.path.normcase(str(path)) + + def ts_build_dep_graph( path: Path, spec: TreeSitterLangSpec, @@ -30,8 +50,18 @@ def ts_build_dep_graph( parser, language = _get_parser(spec.grammar) query = _make_query(language, spec.import_query) - scan_path = str(path.resolve()) - file_set = set(file_list) + scan_path = path.resolve() + file_paths_by_key = {filepath: _source_path(filepath) for filepath in file_list} + file_keys_by_path: dict[str, str] = {} + for filepath, resolved_path in file_paths_by_key.items(): + identity = _path_identity(resolved_path) + previous_key = file_keys_by_path.get(identity) + if previous_key is not None and previous_key != filepath: + raise ValueError( + "Tree-sitter dependency graph received duplicate paths " + f"{previous_key!r} and {filepath!r} for {resolved_path}" + ) + file_keys_by_path[identity] = filepath graph: dict[str, dict[str, Any]] = {} # Initialize all files in the graph. @@ -39,7 +69,8 @@ def ts_build_dep_graph( graph[f] = {"imports": set(), "importers": set()} for filepath in file_list: - cached = get_or_parse_tree(filepath, parser, spec.grammar) + source_path = file_paths_by_key[filepath] + cached = get_or_parse_tree(str(source_path), parser, spec.grammar) if cached is None: continue _source, tree = cached @@ -71,21 +102,21 @@ def ts_build_dep_graph( ).strip("\"'`") import_text = f"{prefix_text}\\{import_text}" - resolved = spec.resolve_import(import_text, filepath, scan_path) + resolved = spec.resolve_import( + import_text, str(source_path), str(scan_path) + ) if resolved is None: continue - # Normalize to absolute path. - if not os.path.isabs(resolved): - resolved = os.path.normpath(os.path.join(scan_path, resolved)) - - # Only track edges within the scanned file set. - if resolved not in file_set: + # Match by filesystem identity, then store the caller's original key. + resolved_key = file_keys_by_path.get( + _path_identity(_import_path(resolved, scan_path)) + ) + if resolved_key is None: continue - graph[filepath]["imports"].add(resolved) - if resolved in graph: - graph[resolved]["importers"].add(filepath) + graph[filepath]["imports"].add(resolved_key) + graph[resolved_key]["importers"].add(filepath) # Finalize: add counts. for data in graph.values(): diff --git a/desloppify/tests/commands/test_cli.py b/desloppify/tests/commands/test_cli.py index 2cb4b4f23..bb0afa2c6 100644 --- a/desloppify/tests/commands/test_cli.py +++ b/desloppify/tests/commands/test_cli.py @@ -823,6 +823,39 @@ def test_auto_detect_falls_back_to_project_root_for_subdir_path( assert lang is not None assert lang.name == "python" + def test_auto_detect_ignores_markers_above_active_project_root( + self, tmp_path, monkeypatch + ): + (tmp_path / "package.json").write_text('{"name":"outside"}\n') + + project_root = tmp_path / "project" + project_root.mkdir() + scripts = project_root / "scripts" + scripts.mkdir() + (scripts / "job.py").write_text("print('x')\n") + + monkeypatch.setattr(lang_helpers_mod, "get_project_root", lambda: project_root) + args = SimpleNamespace(lang=None, path=str(scripts)) + + assert lang_helpers_mod.resolve_detection_root(args) == scripts + lang = resolve_lang(args) + assert lang is not None + assert lang.name == "python" + + def test_detection_root_includes_active_project_root_boundary( + self, tmp_path, monkeypatch + ): + project_root = tmp_path / "project" + project_root.mkdir() + (project_root / "package.json").write_text('{"name":"project"}\n') + source = project_root / "src" + source.mkdir() + + monkeypatch.setattr(lang_helpers_mod, "get_project_root", lambda: project_root) + args = SimpleNamespace(path=str(source)) + + assert lang_helpers_mod.resolve_detection_root(args) == project_root + def test_auto_detect_walks_up_from_external_subdir_path( self, tmp_path, monkeypatch ): @@ -846,6 +879,23 @@ def test_auto_detect_walks_up_from_external_subdir_path( assert lang is not None assert lang.name == "typescript" + def test_detection_root_still_walks_to_external_target_marker( + self, tmp_path, monkeypatch + ): + project_root = tmp_path / "active_project" + project_root.mkdir() + + external_root = tmp_path / "external_project" + external_root.mkdir() + (external_root / "package.json").write_text('{"name":"target"}\n') + external_src = external_root / "src" + external_src.mkdir() + + monkeypatch.setattr(lang_helpers_mod, "get_project_root", lambda: project_root) + args = SimpleNamespace(path=str(external_src)) + + assert lang_helpers_mod.resolve_detection_root(args) == external_root + def test_auto_detect_prefers_path_subtree_when_no_markers( self, tmp_path, monkeypatch ): diff --git a/desloppify/tests/lang/common/test_treesitter_imports_direct.py b/desloppify/tests/lang/common/test_treesitter_imports_direct.py index 2a75eb119..63a1b7a59 100644 --- a/desloppify/tests/lang/common/test_treesitter_imports_direct.py +++ b/desloppify/tests/lang/common/test_treesitter_imports_direct.py @@ -2,11 +2,13 @@ from __future__ import annotations -import json import builtins +import json from pathlib import Path from types import SimpleNamespace +import pytest + import desloppify.languages._framework.treesitter.imports.graph as graph_mod import desloppify.languages._framework.treesitter.imports.normalize as normalize_mod import desloppify.languages._framework.treesitter.imports.resolver_cache as resolver_cache_mod @@ -33,6 +35,29 @@ def __init__( self.end_byte = end_byte +def stub_graph_parser(monkeypatch, importers: set[str]) -> None: + monkeypatch.setattr( + graph_mod, "_get_parser", lambda _grammar: ("parser", "language") + ) + monkeypatch.setattr(graph_mod, "_make_query", lambda _language, source: source) + monkeypatch.setattr( + graph_mod, + "get_or_parse_tree", + lambda filepath, *_a, **_k: (b"", SimpleNamespace(root_node=filepath)), + ) + monkeypatch.setattr( + graph_mod, + "_run_query", + lambda _query, filepath: ( + [(0, {"path": FakeNode("string", text="'./support.js'")})] + if filepath in importers + else [] + ), + ) + monkeypatch.setattr(graph_mod, "_unwrap_node", lambda node: node) + + + def test_graph_helpers_build_internal_edges_and_builder(monkeypatch, tmp_path: Path) -> None: source_file = tmp_path / "src" / "main.php" dep_file = tmp_path / "src" / "support.php" @@ -82,6 +107,170 @@ def test_graph_helpers_build_internal_edges_and_builder(monkeypatch, tmp_path: P ) == {} +@pytest.mark.parametrize( + "absolute_file_list", [False, True], ids=["relative", "absolute"] +) +def test_graph_matches_resolved_paths_without_changing_file_keys( + monkeypatch, + tmp_path: Path, + absolute_file_list: bool, +) -> None: + project_root = tmp_path + scan_path = project_root / "packages" / "app" + source_file = scan_path / "src" / "main.js" + dep_file = scan_path / "src" / "support.js" + source_file.parent.mkdir(parents=True) + source_file.write_text("import './support.js';\n", encoding="utf-8") + dep_file.write_text("export const support = true;\n", encoding="utf-8") + monkeypatch.chdir(project_root) + + if absolute_file_list: + file_list = [str(source_file), str(dep_file)] + else: + file_list = [ + source_file.relative_to(project_root).as_posix(), + dep_file.relative_to(project_root).as_posix(), + ] + + monkeypatch.setattr( + graph_mod, "_get_parser", lambda _grammar: ("parser", "language") + ) + monkeypatch.setattr(graph_mod, "_make_query", lambda _language, source: source) + monkeypatch.setattr( + graph_mod, + "get_or_parse_tree", + lambda filepath, *_a, **_k: ( + b"", + SimpleNamespace(root_node=Path(filepath).name), + ), + ) + monkeypatch.setattr( + graph_mod, + "_run_query", + lambda _query, filename: ( + [(0, {"path": FakeNode("string", text="'./support.js'")})] + if filename == source_file.name + else [] + ), + ) + monkeypatch.setattr(graph_mod, "_unwrap_node", lambda node: node) + + resolver_calls: list[tuple[str, str, str]] = [] + + def resolve_import(import_text: str, source_path: str, root_path: str) -> str: + resolver_calls.append((import_text, source_path, root_path)) + return str(dep_file) + + spec = SimpleNamespace( + grammar="javascript", + import_query="imports", + resolve_import=resolve_import, + ) + + graph = graph_mod.ts_build_dep_graph(scan_path, spec, file_list) + + assert list(graph) == file_list + assert resolver_calls == [("./support.js", str(source_file), str(scan_path))] + assert graph[file_list[0]]["imports"] == {file_list[1]} + assert graph[file_list[1]]["importers"] == {file_list[0]} + + +def test_graph_prefers_project_relative_discovery_keys_over_scan_root( + monkeypatch, + tmp_path: Path, +) -> None: + scan_path = tmp_path / "src" + source_file = scan_path / "main.js" + dep_file = scan_path / "support.js" + nested_decoy = scan_path / "src" / "main.js" + nested_decoy.parent.mkdir(parents=True) + source_file.write_text("import './support.js';\n", encoding="utf-8") + dep_file.write_text("export const support = true;\n", encoding="utf-8") + nested_decoy.write_text("export const decoy = true;\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + + file_list = [ + source_file.relative_to(tmp_path).as_posix(), + dep_file.relative_to(tmp_path).as_posix(), + nested_decoy.relative_to(tmp_path).as_posix(), + ] + stub_graph_parser(monkeypatch, {str(source_file)}) + resolver_sources: list[str] = [] + + def resolve_import(_text: str, source_path: str, _root_path: str) -> str: + resolver_sources.append(source_path) + return str(dep_file) + + spec = SimpleNamespace( + grammar="javascript", + import_query="imports", + resolve_import=resolve_import, + ) + + graph = graph_mod.ts_build_dep_graph(scan_path, spec, file_list) + + assert resolver_sources == [str(source_file)] + assert graph[file_list[0]]["imports"] == {file_list[1]} + assert graph[file_list[2]]["imports"] == set() + + +def test_graph_uses_identity_normalization_only_for_comparison( + monkeypatch, + tmp_path: Path, +) -> None: + source_file = tmp_path / "MixedCase" / "Main.js" + dep_file = source_file.with_name("Support.js") + source_file.parent.mkdir() + source_file.write_text("import './Support.js';\n", encoding="utf-8") + dep_file.write_text("export const support = true;\n", encoding="utf-8") + file_list = [str(source_file), str(dep_file)] + stub_graph_parser(monkeypatch, {str(source_file)}) + monkeypatch.setattr(graph_mod.os.path, "normcase", lambda path: path.lower()) + resolver_sources: list[str] = [] + + def resolve_import(_text: str, source_path: str, _root_path: str) -> str: + resolver_sources.append(source_path) + return str(dep_file) + + spec = SimpleNamespace( + grammar="javascript", + import_query="imports", + resolve_import=resolve_import, + ) + + graph = graph_mod.ts_build_dep_graph(tmp_path, spec, file_list) + + assert resolver_sources == [str(source_file)] + assert graph[file_list[0]]["imports"] == {file_list[1]} + + +@pytest.mark.parametrize( + "reverse", [False, True], ids=["relative-first", "absolute-first"] +) +def test_graph_rejects_duplicate_filesystem_identities( + monkeypatch, + tmp_path: Path, + reverse: bool, +) -> None: + source_file = tmp_path / "src" / "main.js" + source_file.parent.mkdir() + source_file.write_text("export const value = true;\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + aliases = [source_file.relative_to(tmp_path).as_posix(), str(source_file)] + if reverse: + aliases.reverse() + stub_graph_parser(monkeypatch, set()) + spec = SimpleNamespace( + grammar="javascript", + import_query="imports", + resolve_import=lambda *_args: None, + ) + + with pytest.raises(ValueError, match="duplicate paths"): + graph_mod.ts_build_dep_graph(tmp_path, spec, aliases) + + + def test_import_normalize_helpers_strip_comments_and_log_lines() -> None: cached = normalize_mod._get_log_patterns((r"logger\.",)) assert cached is normalize_mod._get_log_patterns((r"logger\.",))