diff --git a/.gitignore b/.gitignore index d607a89f..30891645 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,7 @@ tests/* !tests/test_module_tree_validation.py !tests/test_ruby_analyzer.py !tests/test_processing_order_update.py +!tests/test_leaf_selection.py # Jupyter *.ipynb diff --git a/codewiki/src/be/dependency_analyzer/leaf_selection.py b/codewiki/src/be/dependency_analyzer/leaf_selection.py index fd37b512..cd972d3e 100644 --- a/codewiki/src/be/dependency_analyzer/leaf_selection.py +++ b/codewiki/src/be/dependency_analyzer/leaf_selection.py @@ -1,8 +1,7 @@ -from typing import Dict, List, Set +import logging from codewiki.src.be.dependency_analyzer.models.core import Node -import logging logger = logging.getLogger(__name__) # Below this many class/interface/struct components, a repo is considered @@ -17,7 +16,7 @@ OOP_TYPES = {"class", "interface", "struct"} -def compute_valid_leaf_types(components: Dict[str, Node]) -> Set[str]: +def compute_valid_leaf_types(components: dict[str, Node]) -> set[str]: """ Determine which component types qualify as leaf nodes. @@ -54,19 +53,23 @@ def compute_valid_leaf_types(components: Dict[str, Node]) -> Set[str]: def filter_leaf_nodes( leaf_nodes, - components: Dict[str, Node], - valid_types: Set[str], -) -> List[str]: - """Keep leaf nodes that are known components of a valid type.""" + components: dict[str, Node], + valid_types: set[str], +) -> list[str]: + """Keep leaf nodes that are known components of a valid type. + + Anything that is not a known component id (None, empty strings, error + messages that occasionally reach leaf-node selection) is dropped by the + membership check alone. Do not add keyword-based filtering on top of it: + it would also reject identifiers such as `handleInvalidInput` or `ErrorLog`. + """ keep_leaf_nodes = [] for leaf_node in leaf_nodes: - # Skip any leaf nodes that are clearly error strings or invalid identifiers - if not isinstance(leaf_node, str) or leaf_node.strip() == "" or any(err_keyword in leaf_node.lower() for err_keyword in ['error', 'exception', 'failed', 'invalid']): - logger.debug(f"Skipping invalid leaf node identifier: '{leaf_node}'") + if not isinstance(leaf_node, str) or leaf_node not in components: + logger.debug(f"Skipping unknown leaf node identifier: '{leaf_node}'") continue - if leaf_node in components: - if components[leaf_node].component_type in valid_types: - keep_leaf_nodes.append(leaf_node) + if components[leaf_node].component_type in valid_types: + keep_leaf_nodes.append(leaf_node) return keep_leaf_nodes diff --git a/tests/test_leaf_selection.py b/tests/test_leaf_selection.py new file mode 100644 index 00000000..31c30eff --- /dev/null +++ b/tests/test_leaf_selection.py @@ -0,0 +1,79 @@ +"""Tests for leaf-node identifier filtering. + +Covers filter_leaf_nodes: identifiers that merely contain a word like +"invalid" must survive, while strings that are not known component ids +(error messages reaching leaf-node selection, malformed entries) are dropped. +""" + +from __future__ import annotations + +from codewiki.src.be.dependency_analyzer.leaf_selection import filter_leaf_nodes +from codewiki.src.be.dependency_analyzer.models.core import Node + + +def _component(component_id: str, component_type: str = "function") -> Node: + file_path = component_id.split("::")[0] + return Node( + id=component_id, + name=component_id.split("::")[-1], + component_type=component_type, + file_path=file_path, + relative_path=file_path, + ) + + +def _components(*component_ids: str) -> dict[str, Node]: + return {cid: _component(cid) for cid in component_ids} + + +def test_identifiers_containing_error_words_are_kept() -> None: + components = _components( + "src/input.cpp::handleInvalidInput", + "src/game.cpp::gameFailedCheck", + "src/log.cpp::ErrorLog", + "src/parser.cpp::parseExceptionTable", + ) + + kept = filter_leaf_nodes(list(components), components, {"function"}) + + assert set(kept) == set(components) + + +def test_error_messages_are_dropped() -> None: + components = _components("src/player.cpp::playerMove") + candidates = [ + "src/player.cpp::playerMove", + "Error: could not parse file", + "invalid syntax at line 3", + "Analysis failed for this component", + ] + + kept = filter_leaf_nodes(candidates, components, {"function"}) + + assert kept == ["src/player.cpp::playerMove"] + + +def test_unknown_and_malformed_entries_are_dropped() -> None: + components = _components("src/player.cpp::playerMove") + candidates = [ + "src/player.cpp::playerMove", + "src/player.cpp::doesNotExist", + "", + " ", + None, + ] + + kept = filter_leaf_nodes(candidates, components, {"function"}) + + assert kept == ["src/player.cpp::playerMove"] + + +def test_components_of_other_types_are_dropped() -> None: + components = { + "src/game.cpp::runGame": _component("src/game.cpp::runGame", "function"), + "src/game.h::GameState": _component("src/game.h::GameState", "struct"), + } + + kept = filter_leaf_nodes(list(components), components, {"function"}) + + assert kept == ["src/game.cpp::runGame"]