diff --git a/codegraph/analysis/_common.py b/codegraph/analysis/_common.py index 5660d2d..a145ac1 100644 --- a/codegraph/analysis/_common.py +++ b/codegraph/analysis/_common.py @@ -23,6 +23,64 @@ def _kind_str(value: object) -> str: ) +EXCLUDED_PATH_FRAGMENTS: tuple[str, ...] = ( + "tests/fixtures/", + "tests\\fixtures\\", + "/static/", + "\\static\\", +) + + +def is_excluded_path(file_path: str) -> bool: + """True iff the file path is under a directory excluded from analysis. + + Test fixtures and static frontend assets don't have traceable call graphs + and should not be analysed for dead-code or untested-symbol detection. + """ + if not file_path: + return False + return any(fragment in file_path for fragment in EXCLUDED_PATH_FRAGMENTS) + + +def is_protocol_class(graph: nx.MultiDiGraph, class_id: str) -> bool: + """True iff the class inherits from ``typing.Protocol``. + + Walks INHERITS out-edges and matches any parent whose target name ends in + ``Protocol``. This covers ``Protocol``, ``typing.Protocol``, and the + parser's ``unresolved::Protocol`` / ``unresolved::typing.Protocol`` forms. + """ + for _src, dst, key, data in graph.out_edges(class_id, keys=True, data=True): + if key != EdgeKind.INHERITS.value: + continue + target_name = "" + meta = data.get("metadata") or {} + if isinstance(meta, dict): + target_name = str(meta.get("target_name") or "") + if not target_name: + attrs = graph.nodes.get(dst) or {} + target_name = str(attrs.get("name") or attrs.get("qualname") or dst) + # Strip an unresolved:: prefix if the dst ID was used as fallback. + if target_name.startswith("unresolved::"): + target_name = target_name.split("::", 1)[1] + # Match bare "Protocol" or any dotted form ending with ".Protocol". + if target_name == "Protocol" or target_name.endswith(".Protocol"): + return True + return False + + +def in_protocol_class(graph: nx.MultiDiGraph, method_id: str) -> bool: + """True iff this method's owning class is a typing.Protocol.""" + for _src, dst, key in graph.out_edges(method_id, keys=True): + if key != EdgeKind.DEFINED_IN.value: + continue + attrs = graph.nodes.get(dst) or {} + if _kind_str(attrs.get("kind")) != "CLASS": + continue + if is_protocol_class(graph, dst): + return True + return False + + def in_test_module(graph: nx.MultiDiGraph, node_id: str) -> bool: """True iff the node is in a file whose MODULE node is marked is_test.""" attrs = graph.nodes.get(node_id) or {} diff --git a/codegraph/analysis/dead_code.py b/codegraph/analysis/dead_code.py index 4b6175a..02c2b0c 100644 --- a/codegraph/analysis/dead_code.py +++ b/codegraph/analysis/dead_code.py @@ -8,7 +8,10 @@ from codegraph.analysis._common import ( REFERENCE_EDGE_KINDS, _kind_str, + in_protocol_class, in_test_module, + is_excluded_path, + is_protocol_class, ) from codegraph.graph.schema import EdgeKind, NodeKind @@ -21,13 +24,6 @@ "@property", "@cached_property", "functools.cached_property", ) -_EXCLUDED_PATH_FRAGMENTS: tuple[str, ...] = ( - "tests/fixtures/", - "tests\\fixtures\\", - "/static/", - "\\static\\", -) - def _has_property_decorator(metadata: dict[str, object]) -> bool: decorators = metadata.get("decorators") or [] @@ -41,12 +37,6 @@ def _has_property_decorator(metadata: dict[str, object]) -> bool: return False -def _is_excluded_path(file_path: str) -> bool: - if not file_path: - return False - return any(fragment in file_path for fragment in _EXCLUDED_PATH_FRAGMENTS) - - def _class_has_inherits(graph: nx.MultiDiGraph, class_id: str) -> bool: return any( key == EdgeKind.INHERITS.value @@ -133,7 +123,14 @@ def find_dead_code( continue # Generated/static frontend assets and test fixtures don't have # traceable call graphs — exclude them from dead-code detection. - if _is_excluded_path(str(attrs.get("file") or "")): + if is_excluded_path(str(attrs.get("file") or "")): + continue + # Skip ``typing.Protocol`` classes and their methods. Protocols define + # structural types for static type checking; they have no runtime + # call-graph incoming edges by design. + if kind == NodeKind.CLASS.value and is_protocol_class(graph, nid): + continue + if kind == NodeKind.METHOD.value and in_protocol_class(graph, nid): continue # Polymorphic overrides on classes that inherit have no static # incoming CALL edge (dispatch is via the base class). diff --git a/codegraph/analysis/untested.py b/codegraph/analysis/untested.py index 404a292..f74e04c 100644 --- a/codegraph/analysis/untested.py +++ b/codegraph/analysis/untested.py @@ -5,7 +5,12 @@ import networkx as nx -from codegraph.analysis._common import _kind_str, in_test_module +from codegraph.analysis._common import ( + _kind_str, + in_protocol_class, + in_test_module, + is_excluded_path, +) from codegraph.graph.schema import EdgeKind, NodeKind _CANDIDATE_KINDS: frozenset[str] = frozenset( @@ -40,6 +45,15 @@ def find_untested(graph: nx.MultiDiGraph) -> list[UntestedNode]: continue if in_test_module(graph, nid): continue + # Skip test fixtures and static frontend assets — same exclusion as + # the dead-code analyzer. + if is_excluded_path(str(attrs.get("file") or "")): + continue + # Skip methods defined inside a ``typing.Protocol`` class: Protocol + # methods are structural type definitions, not runtime code, so + # "untested" is meaningless for them. + if kind == NodeKind.METHOD.value and in_protocol_class(graph, nid): + continue incoming = 0 from_test = 0 for src, _dst, key in graph.in_edges(nid, keys=True): diff --git a/tests/test_analyzer_cleanup.py b/tests/test_analyzer_cleanup.py new file mode 100644 index 0000000..f2b92f7 --- /dev/null +++ b/tests/test_analyzer_cleanup.py @@ -0,0 +1,306 @@ +"""Tests for analyzer noise reduction (C2).""" +from __future__ import annotations + +import networkx as nx + +from codegraph.analysis import find_dead_code, find_untested +from codegraph.graph.schema import EdgeKind, NodeKind + + +def _add_module(g: nx.MultiDiGraph, mod_id: str, file_path: str) -> None: + g.add_node( + mod_id, + kind=NodeKind.MODULE.value, + name=file_path.rsplit("/", 1)[-1], + qualname=file_path, + file=file_path, + line_start=0, + language="python", + metadata={}, + ) + + +def _add_function( + g: nx.MultiDiGraph, + fn_id: str, + name: str, + qualname: str, + file_path: str, + *, + kind: str = NodeKind.FUNCTION.value, + metadata: dict[str, object] | None = None, +) -> None: + g.add_node( + fn_id, + kind=kind, + name=name, + qualname=qualname, + file=file_path, + line_start=1, + language="python", + metadata=metadata or {}, + ) + + +def _add_class( + g: nx.MultiDiGraph, + cls_id: str, + name: str, + qualname: str, + file_path: str, + *, + inherits: list[str] | None = None, +) -> None: + g.add_node( + cls_id, + kind=NodeKind.CLASS.value, + name=name, + qualname=qualname, + file=file_path, + line_start=1, + language="python", + metadata={}, + ) + for parent_name in inherits or []: + unresolved_id = f"unresolved::{parent_name}" + if unresolved_id not in g: + g.add_node( + unresolved_id, + kind="UNRESOLVED", + name=parent_name, + qualname=parent_name, + file="", + line_start=0, + language="python", + metadata={}, + ) + g.add_edge( + cls_id, + unresolved_id, + key=EdgeKind.INHERITS.value, + kind=EdgeKind.INHERITS.value, + metadata={"target_name": parent_name}, + ) + + +def _add_defined_in(g: nx.MultiDiGraph, child_id: str, parent_id: str) -> None: + g.add_edge( + child_id, + parent_id, + key=EdgeKind.DEFINED_IN.value, + kind=EdgeKind.DEFINED_IN.value, + metadata={}, + ) + + +# ----- Fixture-path exclusion -------------------------------------------- + + +def test_untested_skips_test_fixture_files() -> None: + g: nx.MultiDiGraph = nx.MultiDiGraph() + _add_module(g, "mod::fix", "tests/fixtures/sample/sample.py") + _add_function( + g, + "fn::fixture_helper", + "fixture_helper", + "tests.fixtures.sample.sample.fixture_helper", + "tests/fixtures/sample/sample.py", + ) + _add_defined_in(g, "fn::fixture_helper", "mod::fix") + + untested_qualnames = {u.qualname for u in find_untested(g)} + assert not any( + "fixture_helper" in q for q in untested_qualnames + ), f"Fixture function should be skipped: {untested_qualnames}" + + +def test_untested_still_flags_real_source_files() -> None: + g: nx.MultiDiGraph = nx.MultiDiGraph() + _add_module(g, "mod::src", "src/pkg/svc.py") + _add_function( + g, + "fn::compute", + "compute", + "pkg.svc.compute", + "src/pkg/svc.py", + ) + _add_defined_in(g, "fn::compute", "mod::src") + + untested_qualnames = {u.qualname for u in find_untested(g)} + assert "pkg.svc.compute" in untested_qualnames + + +def test_dead_code_still_uses_shared_path_exclusion() -> None: + """Dead-code analyzer continues to skip fixture-path nodes after the move.""" + g: nx.MultiDiGraph = nx.MultiDiGraph() + _add_module(g, "mod::fix", "tests/fixtures/sample/sample.py") + _add_function( + g, + "fn::fixture_helper", + "fixture_helper", + "tests.fixtures.sample.sample.fixture_helper", + "tests/fixtures/sample/sample.py", + ) + _add_defined_in(g, "fn::fixture_helper", "mod::fix") + + dead_qualnames = {d.qualname for d in find_dead_code(g)} + assert not any("fixture_helper" in q for q in dead_qualnames) + + +# ----- Protocol skip ---------------------------------------------------- + + +def test_dead_code_skips_protocol_class() -> None: + g: nx.MultiDiGraph = nx.MultiDiGraph() + _add_module(g, "mod::p", "src/pkg/proto.py") + _add_class( + g, + "cls::Encoder", + "Encoder", + "pkg.proto.Encoder", + "src/pkg/proto.py", + inherits=["Protocol"], + ) + _add_defined_in(g, "cls::Encoder", "mod::p") + + dead_qualnames = {d.qualname for d in find_dead_code(g)} + assert "pkg.proto.Encoder" not in dead_qualnames + + +def test_dead_code_skips_methods_inside_protocol_class() -> None: + g: nx.MultiDiGraph = nx.MultiDiGraph() + _add_module(g, "mod::p", "src/pkg/proto.py") + _add_class( + g, + "cls::Encoder", + "Encoder", + "pkg.proto.Encoder", + "src/pkg/proto.py", + inherits=["typing.Protocol"], + ) + _add_defined_in(g, "cls::Encoder", "mod::p") + _add_function( + g, + "m::encode", + "encode", + "pkg.proto.Encoder.encode", + "src/pkg/proto.py", + kind=NodeKind.METHOD.value, + ) + _add_defined_in(g, "m::encode", "cls::Encoder") + + dead_qualnames = {d.qualname for d in find_dead_code(g)} + assert "pkg.proto.Encoder.encode" not in dead_qualnames + + +def test_untested_skips_methods_inside_protocol_class() -> None: + g: nx.MultiDiGraph = nx.MultiDiGraph() + _add_module(g, "mod::p", "src/pkg/proto.py") + _add_class( + g, + "cls::Encoder", + "Encoder", + "pkg.proto.Encoder", + "src/pkg/proto.py", + inherits=["Protocol"], + ) + _add_defined_in(g, "cls::Encoder", "mod::p") + _add_function( + g, + "m::encode", + "encode", + "pkg.proto.Encoder.encode", + "src/pkg/proto.py", + kind=NodeKind.METHOD.value, + ) + _add_defined_in(g, "m::encode", "cls::Encoder") + + untested_qualnames = {u.qualname for u in find_untested(g)} + assert "pkg.proto.Encoder.encode" not in untested_qualnames + + +def test_dead_code_still_flags_non_protocol_class() -> None: + g: nx.MultiDiGraph = nx.MultiDiGraph() + _add_module(g, "mod::s", "src/pkg/svc.py") + _add_class( + g, + "cls::Orphan", + "Orphan", + "pkg.svc.Orphan", + "src/pkg/svc.py", + ) + _add_defined_in(g, "cls::Orphan", "mod::s") + + dead_qualnames = {d.qualname for d in find_dead_code(g)} + assert "pkg.svc.Orphan" in dead_qualnames + + +def test_dead_code_still_flags_non_protocol_class_method() -> None: + g: nx.MultiDiGraph = nx.MultiDiGraph() + _add_module(g, "mod::s", "src/pkg/svc.py") + _add_class( + g, + "cls::Plain", + "Plain", + "pkg.svc.Plain", + "src/pkg/svc.py", + ) + _add_defined_in(g, "cls::Plain", "mod::s") + # Give the class an incoming CALL edge to keep it alive (so we are + # specifically asserting the method gets flagged). + _add_function( + g, + "fn::caller", + "caller", + "pkg.svc.caller", + "src/pkg/svc.py", + ) + _add_defined_in(g, "fn::caller", "mod::s") + g.add_edge( + "fn::caller", + "cls::Plain", + key=EdgeKind.CALLS.value, + kind=EdgeKind.CALLS.value, + metadata={}, + ) + _add_function( + g, + "m::do_work", + "do_work", + "pkg.svc.Plain.do_work", + "src/pkg/svc.py", + kind=NodeKind.METHOD.value, + ) + _add_defined_in(g, "m::do_work", "cls::Plain") + + dead_qualnames = {d.qualname for d in find_dead_code(g)} + assert "pkg.svc.Plain.do_work" in dead_qualnames + + +def test_analyzers_handle_missing_inherits_edges() -> None: + """A class with no INHERITS out-edges should not blow up either analyzer.""" + g: nx.MultiDiGraph = nx.MultiDiGraph() + _add_module(g, "mod::s", "src/pkg/svc.py") + _add_class( + g, + "cls::Bare", + "Bare", + "pkg.svc.Bare", + "src/pkg/svc.py", + ) + _add_defined_in(g, "cls::Bare", "mod::s") + _add_function( + g, + "m::tick", + "tick", + "pkg.svc.Bare.tick", + "src/pkg/svc.py", + kind=NodeKind.METHOD.value, + ) + _add_defined_in(g, "m::tick", "cls::Bare") + + # Should not raise. + dead = find_dead_code(g) + untested = find_untested(g) + assert any(d.qualname == "pkg.svc.Bare" for d in dead) + assert any(u.qualname == "pkg.svc.Bare.tick" for u in untested)