From 537745212d3a7c92995f716cb76c7aaaa410d671 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 7 Sep 2026 17:01:08 -0400 Subject: [PATCH 1/2] feat(java): the six deferred v1 accessors answer, on both backends get_imports, get_variables, get_class_hierarchy, get_methods_with_annotations, get_call_targets and get_calling_lines raised NotImplementedError through leg 3 while the graph carried everything they needed. Each is now implemented once, on JavaAnalysisBackend, over accessors both backends already answer -- so no new Cypher exists at all: the projection's reconstruction already reads J_IMPORTS, J_DECLARES_VAR, J_EXTENDS/J_IMPLEMENTS, J_ANNOTATED_BY and J_CALLS into the models. Signatures are the published 1.x ones, unmoved. What each one decided, where the docstring left room: - get_imports is the distinct sorted set of import targets. The projection aggregates a module's imports per target onto one J_IMPORTS edge, so file order is not recoverable there and a list that kept it locally would be one the two backends disagree about. - get_variables is locals only, keyed by the J-1 call-graph key and sorted by (line, name): :JLocal carries a line-only span, and 6 of daytrader8's callables declare two variables on one line. Fields and parameters keep their own accessors. An unexpected keyword raises TypeError rather than being ignored -- a dropped filter returns an unfiltered answer that looks filtered. - get_class_hierarchy reads each declaration's own base_types/interfaces, not J_EXTENDS/J_IMPLEMENTS: those join 8 of daytrader8's type pairs where the declarations join 103, because a relationship needs a node at both ends and almost every supertype named is a library type. Edges carry EXTENDS or IMPLEMENTS so the split the graph keeps is not thrown away here. - get_methods_with_annotations follows get_test_methods -- the analyzer's own annotations, so it answers over a projection that carries no module source -- with the J-5 marker rule, keyed by the spelling the caller passed and sorted by (class, signature), since the two backends walk their callables in different orders. Entries carry class and signature as well as 1.x's method_name and body; a simple name is not an address in Java. - get_call_targets has no body parameter on the frozen signature, so its domain is the whole application: the declared names some call site actually writes, by simple name, with no overload resolution. - get_calling_lines reads get_call_graph()'s own calling_lines attribute rather than re-deriving file lines. RAISING shrinks from nine to three: the two get_service_entry_point_* accessors the spec's section 4 proposes deleting, and remove_all_comments. --- CHANGELOG.md | 16 ++ cldk/analysis/java/backend.py | 218 +++++++++++++++++- cldk/analysis/java/java_analysis.py | 97 ++++---- docs/agent-api-reference.md | 29 ++- tests/analysis/java/test_java_analysis.py | 75 +++--- .../analysis/java/test_java_public_surface.py | 25 +- 6 files changed, 346 insertions(+), 114 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87975132..97c68e70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,22 @@ surface** (3b). Design records: `docs/design/specs/2026-09-06-leg-2.5-typescript including on the miss paths. - `cldk.models.java.JCallableOverview` and `JClassOverview`, the projections those bulk accessors return. They carry the addressable `"."` key, never a `can://` id. +- **Six Java accessors that only ever raised `NotImplementedError` now answer** (#366), at the + signature they have always been published with: `get_imports()` (the project's distinct import + targets, sorted — the Neo4j projection aggregates a module's imports per target, so file order is + not recoverable and the set is what both backends can give), `get_variables()` (each callable's + local variables, keyed by the `"."` call-graph key and ordered by + `(line, name)`; fields and parameters keep their own accessors, and an unexpected keyword now + raises `TypeError` instead of being ignored), `get_class_hierarchy()` (a `nx.DiGraph`, subclass → + supertype, each edge carrying `type="EXTENDS"` or `"IMPLEMENTS"` — read off each declaration's own + `base_types`/`interfaces`, which is why out-of-project supertypes are in it), + `get_methods_with_annotations()` (grouped by the spelling the caller passed, matched by the J-5 + marker rule, each entry `{class, signature, method_name, body}`), `get_call_targets()` (the + declared names some call site actually writes — simple-name matching, no overload resolution) and + `get_calling_lines()` (sorted, distinct absolute file lines, off the call graph's own + `calling_lines`). Both backends answer identically; none of the six issues any new Cypher. + `get_service_entry_point_classes`/`get_service_entry_point_methods` and `remove_all_comments` + still raise. - **Java reaches analysis levels 3 and 4** — control flow, control and data dependence, and the interprocedural graph. The level now reaches the analyzer, which it never did before. - **A `java` install extra.** `pip install "cldk[java]"` brings the analyzer and its bundled JVM; diff --git a/cldk/analysis/java/backend.py b/cldk/analysis/java/backend.py index d2cea119..18413fe3 100644 --- a/cldk/analysis/java/backend.py +++ b/cldk/analysis/java/backend.py @@ -50,7 +50,7 @@ import re from abc import abstractmethod from functools import cached_property -from typing import ClassVar, Dict, Iterable, List, Mapping, NamedTuple, Optional, Sequence, Tuple, Union +from typing import ClassVar, Dict, Iterable, List, Mapping, NamedTuple, Optional, Sequence, Set, Tuple, Union import networkx as nx @@ -100,6 +100,7 @@ JEnumConstant, JExternalSymbol, JField, + JLocalVariable, JMethodDetail, JType, ) @@ -1125,6 +1126,221 @@ def get_enum_members(self, qualified_enum_name: str) -> List[JEnumConstant]: def _of_kind(self, kind: str) -> Dict[str, JType]: return {name: t for name, t in self._types.items() if t.kind == kind} + # ===================================================================================== + # The six 1.x accessors leg 3 deferred (#366). Each is implemented **once, here**, over what + # both backends already answer -- the symbol table, the class index, the callable index and + # ``get_call_graph()`` -- for the same reason the call-graph half of the dataflow surface is: + # a per-backend implementation would have nothing to read that this one cannot, and would only + # be a second place for the answer to drift. **No new Cypher is issued for any of the six**; + # every fact they need (``J_IMPORTS``, ``J_DECLARES_VAR``, ``J_EXTENDS``/``J_IMPLEMENTS``, + # ``J_ANNOTATED_BY``, ``J_CALLS``) is already read by the projection's reconstruction into the + # pydantic models, so the graph backend answers all six off the application it already builds. + # + # Their signatures are the 1.x facade's, frozen (the Iron Rule): they are *not* Python's, which + # has five of the six not at all, and not TypeScript's, which spells three of them differently + # on purpose. What each one *returns* is decided here, and the decision is written down. + # ===================================================================================== + def get_imports(self) -> List[str]: + """Every distinct import target of the application, sorted -- a **set**, not a per-file + listing, and not the file's import order. + + That is a fact about the projection, not a simplification: ``--emit neo4j`` aggregates every + import of a module that resolves to the same target onto **one** ``J_IMPORTS`` edge carrying + their dotted ``spellings`` (see :func:`cldk.analysis.java.neo4j.reconstruct.imports`), so + the order the imports appear in the file is not recoverable from the graph, and a list that + preserved it locally would be a list the two backends disagree about. Sorting and + de-duplicating is the answer both can give -- and the 1.x signature is a flat + ``List[str]`` anyway, which never carried the file an import belongs to. Per-file import + declarations, with their spans, are on + :attr:`~cldk.models.java.models.JCompilationUnit.import_declarations`. + + A wildcard import keeps its ``.*`` (the analyzer writes the spelling, ``java.util.*``) and a + static import is not marked as such here; both distinctions live on + :class:`~cldk.models.java.models.JImport`. + """ + return sorted({i.path for unit in self.get_symbol_table().values() for i in unit.import_declarations}) + + def get_variables(self) -> Dict[str, List[JLocalVariable]]: + """The **local variables** each callable declares, keyed by the J-1 call-graph key + ``"."`` -- the ``J_DECLARES_VAR`` layer of the graph. + + The 1.x return type is a bare ``Dict`` and the docstring names "local variables, fields, and + parameters". Those are three different things with three different homes, and this is the + one with no other accessor: fields are :meth:`get_all_fields` (``J_HAS_FIELD``) and + parameters are :meth:`get_method_parameters`, both already on this surface, so folding all + three into one dict would give every caller a bag they have to re-split. Locals are what is + left, and ``J_DECLARES_VAR`` is exactly them. + + Every callable in the addressing domain gets an entry, including the ones that declare + nothing: an absent key would be indistinguishable from a callable that is not in the + application, which is the ambiguous empty (D7). Implicit constructors carry no body and so + carry an empty list. + + **Ordered by (line, name), not by declaration order within a line.** The wire's order is the + source's, but the projection carries a line-only span -- ``:JLocal`` has no column -- so two + variables declared on one line (``String htmlString, arrow;``) come back in an order the + graph does not fix. Measured on the reference graph: 6 of daytrader8's 1,216 callables, 12 + variables, same multiset, different order. Sorting is what the two backends can both do; the + alternative is a list whose order means "source" on one backend and "arbitrary" on the + other. The span itself still differs -- ``start_column``/``end_column`` and the byte offsets + are placeholders over the graph, as they are everywhere else on this surface -- so compare + variables on ``name``/``type``/``start_line``. + """ + return { + f"{klass}.{signature}": sorted(c.local_variables, key=lambda v: (v.start_line, v.name)) + for klass, methods in self.get_all_methods_in_application().items() + for signature, c in methods.items() + } + + def get_class_hierarchy(self) -> nx.DiGraph: + """The inheritance graph: a node per type declared in the application, and an edge + **child → base** for each supertype, carrying ``type="EXTENDS"`` or ``type="IMPLEMENTS"``. + + Modelled on :meth:`cldk.analysis.typescript.backend.TypeScriptAnalysisBackend.get_class_hierarchy`, + which is the only sibling that has one, with the one difference Java forces: TypeScript's + wire has a single ``base_classes`` list, while Java's has ``base_types`` and ``interfaces`` + and the projection keeps them as two relationship types (``J_EXTENDS`` / ``J_IMPLEMENTS``). + Collapsing them would throw away the distinction the graph is at pains to keep, so it is on + the edge. + + Nodes are :meth:`get_all_classes`'s keys -- every declared kind, interfaces, enums, + annotations and records included, since all of them participate. A supertype **outside** the + application (``javax.servlet.http.HttpServlet``) becomes a node too, by being an edge + endpoint: it is named exactly as the analyzer wrote it on the declaration, which is the + source spelling and not necessarily a qualified name. Type arguments are part of that + spelling where the source wrote them. + + **Read off each declaration's own** ``base_types`` / ``interfaces``, **not off the** + ``J_EXTENDS`` / ``J_IMPLEMENTS`` **relationships**, and that is the difference between an + answer and a fifteenth of one: those relationships can only join two types the projection + has *nodes* for, and daytrader8 extends and implements almost nothing it declares itself. + Measured on the reference graph: 8 relationships (1 ``J_EXTENDS``, 7 ``J_IMPLEMENTS``) + against the 103 edges (58 / 45) the declarations carry. The properties are projected onto + every ``:JType``, so both backends read the same list. + + A type that neither extends nor is extended is an isolated node rather than a missing one, + which is why the nodes are added before the edges. + """ + graph = nx.DiGraph() + for name, declared in self.get_all_classes().items(): + graph.add_node(name) + for base in declared.base_types: + graph.add_edge(name, base, type="EXTENDS") + for interface in declared.interfaces: + graph.add_edge(name, interface, type="IMPLEMENTS") + return graph + + def get_methods_with_annotations(self, annotations: List[str]) -> Dict[str, List[Dict]]: + """The callables carrying each requested annotation, grouped by the requested spelling. + + Neither sibling language has this accessor, so it is designed rather than ported, and the + pattern it follows is :meth:`JavaAnalysis.get_test_methods`: read the **analyzer's own** + annotations off the model (``J_ANNOTATED_BY``; 26,162 edges on the reference graph) rather + than re-parsing a source string, so it answers identically on both backends -- a + Neo4j-backed analysis carries no module source at all, and the 1.x tree-sitter version + returned ``{}`` there. + + Args: + annotations: Annotation names, matched by the J-5 marker rule + :meth:`get_decorated_callables` uses: both sides compared on the segment after the + last ``.``, with a leading ``@`` stripped, so ``Test``, ``@Test`` and + ``org.junit.Test`` all match a callable annotated ``@Test``. + + Returns: + A dict keyed by **the string the caller passed**, not by the annotation's spelling in + the source, so ``result[a]`` works for every ``a`` the caller asked about. An annotation + no callable carries is **omitted** (the 1.x shape), so an empty dict means none of the + requested annotations was found. Each value is a list of dicts, in + :meth:`get_all_methods_in_application` order, with four keys: + + * ``class`` -- the declaring type's qualified name; + * ``signature`` -- the callable's signature within that type; + * ``method_name`` -- its simple name (the 1.x key, kept); + * ``body`` -- :attr:`~cldk.models.java.models.JCallable.code` (the 1.x key, kept). + Note this is the body block off ``analysis.json`` and the whole declaration off the + Neo4j projection, exactly as it is for :meth:`JavaAnalysis.get_test_methods`; it is a + documented property of the model, not a divergence introduced here. + + ``class`` and ``signature`` are new against 1.x, which returned the simple name alone. + A simple name is not an address in Java -- 200 of daytrader8's 581 distinct signatures + are declared by more than one type -- so without them a caller cannot find again what + this hands them. + + Each list is sorted by ``(class, signature)``. The obvious alternative, "the order + :meth:`get_all_methods_in_application` walks in", is not one order but two: the local + backend walks the symbol table and the graph backend walks its reconstruction, and the + same 328 callables came back in different orders (measured on the reference graph). + """ + wanted = {a: a.lstrip("@").rpartition(".")[2] for a in annotations} + found: Dict[str, List[Dict]] = {} + for klass, methods in sorted(self.get_all_methods_in_application().items()): + for signature, c in sorted(methods.items()): + carried = {d.name.rpartition(".")[2] for d in c.decorators} + for asked, marker in wanted.items(): + if marker in carried: + found.setdefault(asked, []).append({"class": klass, "signature": signature, "method_name": signature.partition("(")[0], "body": c.code}) + return found + + def get_call_targets(self, declared_methods: dict) -> Set[str]: + """The simple names, out of ``declared_methods``, that are actually invoked somewhere in the + application -- "simple name resolution", as the 1.x docstring calls it. + + The 1.x version took a method *body* as well and answered for that one body; the facade + signature the Iron Rule freezes has no body parameter, so the domain is the whole + application: every call site of every callable, filtered to the names asked about. That is + the only reading of the frozen signature that uses the argument it does take. + + Deliberately **not** the resolved call graph. A call site's ``method_name`` is the name as + written at the call, matched against a set of declared names with no overload resolution, no + receiver typing and no hierarchy walk -- which is what the docstring's "without full + semantic analysis" means, and what makes this different from :meth:`get_call_graph`. Use the + call graph when you want the callable that actually runs. + + Args: + declared_methods: The names to match against, read from its **keys** -- so + :meth:`get_all_methods_in_class`'s result can be passed straight in. A key may be a + signature (``cancelOrder(java.lang.Integer, boolean)``) or a bare name + (``cancelOrder``); the parameter tail is cut at the last ``(``, the cut + :func:`java_callable_names` documents. + + Returns: + The matched simple names -- a subset of the cut keys, never a name the caller did not + ask about. Empty when nothing was asked or nothing was called. + """ + declared = {java_callable_names(key)[-1] for key in declared_methods} + return {site.method_name for methods in self.get_all_methods_in_application().values() for c in methods.values() for site in c.call_sites if site.method_name in declared} + + def get_calling_lines(self, target_method_name: str) -> List[int]: + """The **absolute file lines**, sorted and de-duplicated, of every call to a method of this + name anywhere in the application. + + Read straight off ``get_call_graph()``'s ``calling_lines`` edge attribute, which is + :class:`CallingLines` -- the one place this SDK turns a call site into a file line, and + which already resolved (leg 3a) the 1.x bug where the number was an offset into + :attr:`~cldk.models.java.models.JCallable.code` and so meant two different things on the two + backends. Nothing here re-derives it. + + Args: + target_method_name: A method's simple name (``cancelOrder``). A full signature is + accepted and cut at its first ``(``, as the 1.x accessor did -- and as + :meth:`CallingLines.of` keys its own index, which is why an overload pair cannot be + separated here: the index matches on the name a call site *writes*, and a call site + writes no parameter types. Asking for one overload is asking for the name. + + Returns: + Sorted, distinct file lines, 1-based as the analyzer's spans are. Empty when no call to + that name is on the call graph -- including when the name is not a method of this + application at all: the two are the same fact here, because this asks about call sites + and not about declarations. + """ + name = target_method_name.partition("(")[0] + graph = self.get_call_graph() + lines: Set[int] = set() + for _, dst, data in graph.edges(data=True): + if graph.nodes[dst]["method_detail"].method.signature.partition("(")[0] == name: + lines.update(data["calling_lines"]) + return sorted(lines) + # ===================================================================================== # The dataflow surface (leg 3b, Task 2): per-callable graphs, slices, reachability, paths and # the flow predicates. Every signature below is `cldk/analysis/python/backend.py`'s, diff --git a/cldk/analysis/java/java_analysis.py b/cldk/analysis/java/java_analysis.py index 0fad16a6..f0e1a28a 100644 --- a/cldk/analysis/java/java_analysis.py +++ b/cldk/analysis/java/java_analysis.py @@ -188,44 +188,49 @@ def __init__( ) def get_imports(self) -> List[str]: - """Return all import statements in the source code. + """Return every distinct import target of the project, sorted. - This method is intended to extract all import declarations from the - analyzed Java source code, including both single-type imports and - wildcard imports. + A **set**, not a per-file listing and not the file's import order: the Neo4j projection + aggregates every import of a module that resolves to the same target onto one edge, so the + order within a file is not recoverable there and a list that preserved it locally would be + one the two backends disagree about. The 1.x signature is a flat ``List[str]`` and never + carried the file an import belongs to either. Returns: - A list of import statement strings, each representing a fully - qualified import (e.g., ``"java.util.List"``, ``"java.io.*"``). - - Raises: - NotImplementedError: This functionality is not yet implemented. + Fully qualified import targets (``"java.util.List"``, ``"java.io.*"``), sorted and + distinct. A wildcard keeps its ``.*``; static imports are not marked here. See Also: - :meth:`get_symbol_table`: For accessing compilation units which - contain import information. + :meth:`get_symbol_table`: per-file :class:`~cldk.models.java.models.JImport` records, + with spans and the static/wildcard flags. """ - raise NotImplementedError("Support for this functionality has not been implemented yet.") + return self.backend.get_imports() def get_variables(self, **kwargs) -> Dict: - """Return all variables discovered in the source code. - - This method is intended to extract variable declarations from the - analyzed code, including local variables, fields, and parameters. + """Return the local variables each callable declares. Args: - **kwargs: Implementation-specific filtering options. + **kwargs: The 1.x signature's filtering options, of which there are none. An unexpected + keyword raises :class:`TypeError` naming it, rather than being ignored: silently + dropping a filter returns an unfiltered answer that looks filtered. Returns: - An implementation-defined view of variables discovered in the code. + ``{".": [JLocalVariable, ...]}`` — the J-1 call-graph key of + :meth:`get_call_graph`, and one entry per callable including those declaring nothing. + Fields and parameters are *not* folded in; they have their own accessors. Each list is + ordered by ``(start_line, name)``: the Neo4j projection carries a line-only span, so + two variables declared on one line have no order there to preserve. Raises: - NotImplementedError: This functionality is not yet implemented. + TypeError: An unexpected keyword argument was passed. See Also: - :meth:`get_fields`: For class-level field access (implemented). + :meth:`get_fields`: class-level fields. + :meth:`get_method_parameters`: a callable's parameters. """ - raise NotImplementedError("Support for this functionality has not been implemented yet.") + if kwargs: + raise TypeError(f"get_variables() got an unexpected keyword argument {next(iter(kwargs))!r}; it takes no filtering options") + return self.backend.get_variables() def get_service_entry_point_classes(self, **kwargs) -> Dict[str, JType]: """Return all service entry-point classes. @@ -339,19 +344,18 @@ def get_class_hierarchy(self) -> nx.DiGraph: extends and implements relationships. Returns: - Would return a ``networkx.DiGraph`` with classes as nodes and - edges representing inheritance (subclass -> superclass). - - Raises: - NotImplementedError: This functionality is not yet implemented. + A ``networkx.DiGraph`` with one node per declared type (interfaces, enums, annotations + and records included) and an edge **subclass → supertype** carrying + ``type="EXTENDS"`` or ``type="IMPLEMENTS"`` — Java projects the two as separate + relationship types and this keeps them apart. A supertype outside the project is a node + too, spelled as the declaration wrote it. See Also: :meth:`get_sub_classes`: For finding subclasses of a specific class. :meth:`get_extended_classes`: For finding superclasses. :meth:`get_implemented_interfaces`: For interface implementations. """ - - raise NotImplementedError("Class hierarchy is not implemented yet.") + return self.backend.get_class_hierarchy() def is_parsable(self, source_code: str) -> bool: """Check if the given source code is valid Java syntax. @@ -977,18 +981,21 @@ def get_methods_with_annotations(self, annotations: List[str]) -> Dict[str, List should not be included. Returns: - Would return a dictionary mapping annotation names to lists of - method information dictionaries containing method details and - bodies. + A dictionary keyed by **the strings passed in**, each mapping to a list of + ``{"class", "signature", "method_name", "body"}`` dicts, sorted by + ``(class, signature)``. An annotation no callable carries is omitted. ``body`` is + :attr:`~cldk.models.java.models.JCallable.code`, which is the body block off + ``analysis.json`` and the whole declaration off the Neo4j projection — the same + documented model property :meth:`get_test_methods` hands back. - Raises: - NotImplementedError: This functionality is not yet implemented. + Matching reads the analyzer's own annotations rather than re-parsing source, so it + answers on a Neo4j-backed analysis, which carries no module source at all. See Also: :meth:`get_test_methods`: For finding test methods specifically. + :meth:`get_decorated_callables`: The projected form, whose J-5 marker rule this shares. """ - # TODO: This call is missing some implementation. The logic currently resides in java_sitter but tree_sitter will no longer be option, rather it will be default and common. Need to implement this differently. Somthing like, self.commons.treesitter.get_methods_with_annotations(annotations) - raise NotImplementedError("Support for this functionality has not been implemented yet.") + return self.backend.get_methods_with_annotations(annotations) def get_test_methods(self) -> Dict[str, str]: """Return methods identified as test methods. @@ -1032,15 +1039,15 @@ def get_calling_lines(self, target_method_name: str) -> List[int]: target_method_name: The name of the method to find calls to. Returns: - Would return a list of line numbers (integers) where calls occur. - - Raises: - NotImplementedError: This functionality is not yet implemented. + Sorted, distinct **absolute file lines** of every call to a method of that name anywhere + in the project, read off ``get_call_graph()``'s ``calling_lines`` edge attribute. A full + signature is accepted and cut at its first ``(``; overloads share a name at a call site + and so cannot be separated here. Empty when nothing calls that name. See Also: :meth:`get_callers`: For finding caller methods instead of lines. """ - raise NotImplementedError("Support for this functionality has not been implemented yet.") + return self.backend.get_calling_lines(target_method_name) def get_call_targets(self, declared_methods: dict) -> Set[str]: """Return call targets using simple name resolution. @@ -1054,15 +1061,15 @@ def get_call_targets(self, declared_methods: dict) -> Set[str]: signatures to match against. Returns: - Would return a set of method names that are call targets. - - Raises: - NotImplementedError: This functionality is not yet implemented. + The subset of ``declared_methods``' keys — cut to their simple names at the last + ``(``, so a signature-keyed dict such as :meth:`get_methods_in_class`'s can be passed + straight in — that some call site in the project actually writes. Simple-name matching + only: no overload resolution, no receiver typing, no hierarchy walk. See Also: :meth:`get_call_graph`: For full semantic call resolution. """ - raise NotImplementedError("Support for this functionality has not been implemented yet.") + return self.backend.get_call_targets(declared_methods) def get_all_crud_operations(self) -> List[Dict[str, Union[JType, JCallable, List[JCRUDOperation]]]]: """Return all CRUD (Create, Read, Update, Delete) operations. diff --git a/docs/agent-api-reference.md b/docs/agent-api-reference.md index 6ff2d7af..7b9be452 100644 --- a/docs/agent-api-reference.md +++ b/docs/agent-api-reference.md @@ -288,14 +288,27 @@ What Java answers today is the 1.x accessor surface, on the v2 models: `get_symb `get_entry_point_classes` / `get_entry_point_methods`, `get_test_methods`, the comment and docstring accessors, and — new in 3a, from the generic backend ABC — `get_artifacts` / `get_dependencies` / `get_config_keys` (`get_config_uses` and `get_unresolved_config_reads` are -`[]`: the Java analyzer emits neither). **Nine accessors still raise `NotImplementedError`, and 3b -did not retire them** — `get_imports`, `get_variables`, `get_class_hierarchy`, -`get_methods_with_annotations`, `get_calling_lines`, `get_call_targets`, -`get_service_entry_point_classes` / `get_service_entry_point_methods`, and `remove_all_comments` -(which only ever worked in the removed single-file mode). Retiring them is a separate, deliberate -change, not something to expect from the next release: see the **§4 erratum** in -`docs/design/specs/2026-09-06-leg-3-java.md`, and -`tests/analysis/java/test_java_public_surface.py`'s `RAISING`, which pins all nine. +`[]`: the Java analyzer emits neither). Six more joined them in #366 — `get_imports`, +`get_variables`, `get_class_hierarchy`, `get_methods_with_annotations`, `get_call_targets` and +`get_calling_lines`, all at their published 1.x signatures, answering identically on both backends +and issuing no new Cypher. Note what each one is: `get_imports()` is the project's **distinct +sorted set** of import targets (the projection aggregates a module's imports per target, so file +order is not recoverable); `get_variables()` is **local variables only**, keyed by the +`"."` call-graph key and ordered by `(line, name)` because `:JLocal` carries a +line-only span (fields are `get_fields`, parameters are `get_method_parameters`, and an unexpected +keyword raises `TypeError` rather than being ignored); `get_class_hierarchy()` reads each +declaration's own `base_types`/`interfaces` rather than `J_EXTENDS`/`J_IMPLEMENTS`, which is why +library supertypes are in it — those two relationships join **8** of daytrader8's type pairs where +the declarations join **103**; `get_methods_with_annotations()` keys by the spelling you passed and +its `body` is `JCallable.code`, so it is the body block in-process and the whole declaration over +the graph; `get_call_targets()` is simple-name matching with no overload resolution (use the call +graph for what actually runs); `get_calling_lines()` is absolute **file** lines. +**Three accessors still raise `NotImplementedError`** — +`get_service_entry_point_classes` / `get_service_entry_point_methods`, which the **§4 erratum** in +`docs/design/specs/2026-09-06-leg-3-java.md` proposes deleting rather than implementing (the working +`get_entry_point_*` pair already answers), and `remove_all_comments`, which only ever worked in the +removed single-file mode. `tests/analysis/java/test_java_public_surface.py`'s `RAISING` pins those +three. The differences below will mislead you if you don't know them — each is measured, and each names its upstream issue where there is one: diff --git a/tests/analysis/java/test_java_analysis.py b/tests/analysis/java/test_java_analysis.py index bb758866..60e7a214 100644 --- a/tests/analysis/java/test_java_analysis.py +++ b/tests/analysis/java/test_java_analysis.py @@ -72,7 +72,8 @@ def test_get_symbol_table_is_not_null(test_fixture, analysis_json): assert analysis.get_symbol_table() is not None def test_get_imports(test_fixture, analysis_json): - """Should return NotImplemented for get_imports()""" + """The distinct, sorted import targets of the project (#366); the per-accessor policy and both + backends are in ``test_java_v1_accessors.py``.""" # Patch subprocess so that it does not run codeanalyzer with patch("cldk.analysis.java.codeanalyzer.codeanalyzer.subprocess.run") as run_mock: @@ -85,14 +86,13 @@ def test_get_imports(test_fixture, analysis_json): eager_analysis=False, ) - # When this is implemented please add a real test case - with pytest.raises(NotImplementedError) as except_info: - java_analysis.get_imports() - assert except_info.type == NotImplementedError + imports = java_analysis.get_imports() + assert imports == sorted(set(imports)) and len(imports) == 268 + assert "com.ibm.websphere.samples.daytrader.util.Log" in imports def test_get_variables(test_fixture, analysis_json): - """Should return NotImplemented for get_variables()""" + """The locals each callable declares, keyed by the J-1 call-graph key (#366).""" # Patch subprocess so that it does not run codeanalyzer with patch("cldk.analysis.java.codeanalyzer.codeanalyzer.subprocess.run") as run_mock: @@ -105,10 +105,10 @@ def test_get_variables(test_fixture, analysis_json): eager_analysis=False, ) - # When this is implemented please add a real test case - with pytest.raises(NotImplementedError) as except_info: - java_analysis.get_variables() - assert except_info.type == NotImplementedError + variables = java_analysis.get_variables() + assert len(variables) == 1216 and sum(len(v) for v in variables.values()) == 854 + with pytest.raises(TypeError): + java_analysis.get_variables(qualified_class_name="anything") def test_get_service_entry_point_classes(test_fixture, analysis_json): @@ -227,10 +227,9 @@ def test_get_class_hierarchy(test_fixture, analysis_json): eager_analysis=False, ) - # When this is implemented please add a real test case - with pytest.raises(NotImplementedError) as except_info: - java_analysis.get_class_hierarchy() - assert except_info.type == NotImplementedError + hierarchy = java_analysis.get_class_hierarchy() + assert hierarchy.number_of_nodes() == 170 and hierarchy.number_of_edges() == 103 + assert hierarchy.edges["com.ibm.websphere.samples.daytrader.web.prims.PingServlet", "javax.servlet.http.HttpServlet"]["type"] == "EXTENDS" def test_is_parsable(test_fixture, analysis_json): @@ -839,16 +838,12 @@ def test_get_methods_with_annotations(test_fixture, analysis_json): eager_analysis=False, ) - # TODO: The code is broken. It requires Treesitter but JCodeanalyzer does not! - - annotations = ["WebServlet"] - try: - code_with_annotations = java_analysis.get_methods_with_annotations(annotations) - except NotImplementedError: - assert True - return - - assert False, "Did not raise NotImplementedError" + # ``WebServlet`` annotates 53 **types** in this fixture and no callable, so a + # callable-level filter reports nothing for it -- the same split get_decorated_callables has. + assert java_analysis.get_methods_with_annotations(["WebServlet"]) == {} + overridden = java_analysis.get_methods_with_annotations(["Override"])["Override"] + assert len(overridden) == 328 + assert set(overridden[0]) == {"class", "signature", "method_name", "body"} def test_get_test_methods(test_fixture, analysis_json): @@ -914,18 +909,10 @@ def test_get_calling_lines(test_fixture, analysis_json): eager_analysis=False, ) - # TODO: The code is broken. It requires Treesitter but JCodeanalyzer does not! - - try: - calling_lines = java_analysis.get_calling_lines("trace(String)") - assert calling_lines is not None - assert isinstance(calling_lines, List) - assert len(calling_lines) > 0 - except NotImplementedError: - assert True - return - - assert False, "Did not raise NotImplementedError" + # This fixture is a level-1 payload -- an empty call graph (0 nodes, 0 edges) -- so there + # is no call site to report a line for. The real lines are asserted on the level-4 fixture + # in ``test_java_v1_accessors.py`` and against the live graph. + assert java_analysis.get_calling_lines("trace(String)") == [] def test_get_call_targets(test_fixture, analysis_json): @@ -942,17 +929,11 @@ def test_get_call_targets(test_fixture, analysis_json): eager_analysis=False, ) - # TODO: The code is broken. It requires Treesitter but JCodeanalyzer does not! - try: - call_targets = java_analysis.get_call_targets("trace(String)") - assert call_targets is not None - assert isinstance(call_targets, Set) - assert len(call_targets) > 0 - except NotImplementedError: - assert True - return - - assert False, "Did not raise NotImplementedError" + declared = java_analysis.get_methods_in_class("com.ibm.websphere.samples.daytrader.impl.direct.TradeDirect") + call_targets = java_analysis.get_call_targets(declared) + assert isinstance(call_targets, set) and len(call_targets) == 53 + assert "cancelOrder" in call_targets + assert java_analysis.get_call_targets({}) == set() def test_get_all_comments(test_fixture, analysis_json): diff --git a/tests/analysis/java/test_java_public_surface.py b/tests/analysis/java/test_java_public_surface.py index 1d5fa3aa..e77e5388 100644 --- a/tests/analysis/java/test_java_public_surface.py +++ b/tests/analysis/java/test_java_public_surface.py @@ -29,11 +29,14 @@ entrypoint trio, the four bulk projections, ``get_external_symbols``, the artifact six and the four J-7 leaf accessors. -**Nothing pre-existing moved.** In particular the eight 1.x ``NotImplementedError`` raisers listed -in :data:`RAISING` are all still here and still raising: §4 of the spec proposes retiring them (and -deleting ``get_service_entry_point_*``), but no task of the 3b plan carries that work, and the -plan's own Global Constraints say this list "grows by exactly what it adds and changes no existing -entry". Retiring them is therefore a separate, deliberate change. +**Nothing pre-existing moved**, including the six of the 1.x ``NotImplementedError`` raisers that +#366 implemented: ``get_imports``, ``get_variables``, ``get_class_hierarchy``, +``get_methods_with_annotations``, ``get_call_targets`` and ``get_calling_lines`` answer now, at the +signature they have always been published with (Java's own, which is neither Python's -- five of +the six do not exist there -- nor TypeScript's, which spells three of them differently). What +:data:`RAISING` still pins is the remainder: ``get_service_entry_point_*``, which §4 of the spec +proposes deleting rather than implementing, and ``remove_all_comments``, whose single-file mode was +removed in 2.0. """ import inspect @@ -142,18 +145,14 @@ #: J-10: the 1.x constructor minus ``source_code``; everything else in place. CONSTRUCTOR = "(self, project_dir: 'str | Path | None', analysis_level: 'str', target_files: 'List[str] | None', eager_analysis: 'bool', backend: 'JavaBackend | None' = None) -> 'None'" -#: The accessors that only raise ``NotImplementedError`` in 3a: the eight 1.x placeholders the -#: plan keeps until 3b (#311), plus ``remove_all_comments``, which only ever worked in the removed +#: What is left of the 1.x placeholders after #366 implemented six of the eight: the two +#: ``get_service_entry_point_*`` accessors, which §4 of the spec proposes **deleting** rather than +#: implementing (``get_entry_point_classes`` / ``get_entry_point_methods`` already answer, off the +#: analyzer's own marks), plus ``remove_all_comments``, which only ever worked in the removed #: single-file mode and now says so instead of silently changing. RAISING = [ - "get_call_targets", - "get_calling_lines", - "get_class_hierarchy", - "get_imports", - "get_methods_with_annotations", "get_service_entry_point_classes", "get_service_entry_point_methods", - "get_variables", "remove_all_comments", ] From cdac4d0a9c9ec3258c31d20302902f95106cc556 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 7 Sep 2026 17:01:08 -0400 Subject: [PATCH 2/2] test(java): the six v1 accessors, offline and against the reference graph Offline runs the shipped code through both backends over the a1 and a4 fixtures, and compares the two directly rather than only through constants. Live runs daytrader8 in the database that also holds ThingsBoard, against the level-4 reference cache, and pins the three places where the projection differs rather than hiding them: get_variables agrees on name/type/line and not on the span's columns (a line-only span is why the lists are sorted); a body is JCallable.code, the body block in-process and the whole declaration over the graph; and J_EXTENDS/J_IMPLEMENTS join 8 type pairs where the declarations join 103, which is why the hierarchy is not built from them. --- tests/analysis/java/test_java_v1_accessors.py | 270 ++++++++++++++++ .../java/test_java_v1_accessors_live.py | 294 ++++++++++++++++++ 2 files changed, 564 insertions(+) create mode 100644 tests/analysis/java/test_java_v1_accessors.py create mode 100644 tests/analysis/java/test_java_v1_accessors_live.py diff --git a/tests/analysis/java/test_java_v1_accessors.py b/tests/analysis/java/test_java_v1_accessors.py new file mode 100644 index 00000000..8661b3af --- /dev/null +++ b/tests/analysis/java/test_java_v1_accessors.py @@ -0,0 +1,270 @@ +################################################################################ +# Copyright IBM Corporation 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +################################################################################ + +"""The six 1.x accessors #366 implemented, on **both** backends, offline. + +``get_imports``, ``get_variables``, ``get_class_hierarchy``, ``get_methods_with_annotations``, +``get_call_targets`` and ``get_calling_lines`` raised ``NotImplementedError`` through leg 3; the +data they need was on the graph the whole time. Each is implemented **once**, on +:class:`~cldk.analysis.java.backend.JavaAnalysisBackend`, over accessors both backends already +answer — so this suite runs the shipped code twice, through the ``test_java_addressing.py`` +harness, and **no new Cypher exists to audit**: the projection's own reconstruction already reads +``J_IMPORTS``, ``J_DECLARES_VAR``, ``J_EXTENDS``/``J_IMPLEMENTS``, ``J_ANNOTATED_BY`` and +``J_CALLS`` into the pydantic models. + +Every expected number is measured off the fixtures, never off the implementation: + +* **a1** (138 units, level 1): 268 distinct import targets, 1,216 callables of which 235 declare a + local (854 locals in all), 170 types in the hierarchy joined by 103 edges (58 ``EXTENDS``, 45 + ``IMPLEMENTS``) of which 21 endpoints are outside the project and 43 types are isolated, 328 + callables annotated ``@Override``, and 53 of ``TradeDirect``'s declared names actually called. + Level 1 carries **no call graph**, which is why ``get_calling_lines`` is exercised on a4. +* **a4** (4 units, level 4): a 100-node / 247-edge call graph, 36 call lines to ``getStatement`` + and 4 to ``cancelOrder``. + +The last test compares the two backends field for field rather than relying on the parametrisation +to compare them through constants. +""" + +import gzip +import json +from pathlib import Path +from unittest.mock import MagicMock, patch + +import networkx as nx +import pytest + +from cldk.analysis import AnalysisLevel +from cldk.analysis.commons.backend_config import CodeAnalyzerConfig +from cldk.analysis.java.java_analysis import JavaAnalysis + +from .test_java_addressing import _graph, _local + +DIRECT_PKG = "com.ibm.websphere.samples.daytrader.impl.direct" +BEANS_PKG = "com.ibm.websphere.samples.daytrader.beans" +PRIMS_PKG = "com.ibm.websphere.samples.daytrader.web.prims" + +TRADE_DIRECT = f"{DIRECT_PKG}.TradeDirect" +TRADE_SERVICES = "com.ibm.websphere.samples.daytrader.interfaces.TradeServices" +PING_SERVLET = f"{PRIMS_PKG}.PingServlet" +GET_STATEMENT = "getStatement" + + +@pytest.fixture(scope="module", params=["local", "graph"]) +def both(request, analysis_json): + """Both backends over **a1** — the whole application, at level 1.""" + return (_local if request.param == "local" else _graph)(analysis_json) + + +@pytest.fixture(scope="module", params=["local", "graph"]) +def both_l4(request, analysis_json_a4): + """Both backends over **a4** — four units at level 4, the fixture with a call graph.""" + return (_local if request.param == "local" else _graph)(analysis_json_a4) + + +# ---- get_imports: a set, and why ----------------------------------------------------------- +def test_get_imports_is_the_distinct_sorted_set_of_import_targets(both): + imports = both.get_imports() + assert len(imports) == 268, "a1's distinct import targets" + assert imports == sorted(set(imports)), "sorted and distinct" + assert all(isinstance(i, str) for i in imports) + + +def test_get_imports_is_the_union_of_every_units_own_imports(both): + """The set is not narrower than the per-file records: **file order is what is dropped**, not + imports. The projection aggregates a module's imports of one target onto a single ``J_IMPORTS`` + edge, so order within a file is unrecoverable there and a list preserving it locally would be + one the two backends disagree about.""" + per_file = {i.path for unit in both.get_symbol_table().values() for i in unit.import_declarations} + assert set(both.get_imports()) == per_file + assert sum(len(unit.import_declarations) for unit in both.get_symbol_table().values()) > len(per_file), "the same target is imported by several files" + + +# ---- get_variables: locals, keyed as the call graph keys ------------------------------------ +def test_get_variables_keys_every_callable_by_its_j1_key(both): + variables = both.get_variables() + keys = {f"{klass}.{sig}" for klass, methods in both.get_all_methods_in_application().items() for sig in methods} + assert set(variables) == keys, "one entry per callable, so an absent key is not an empty answer (D7)" + assert len(variables) == 1216 + assert all("can://" not in key for key in variables) + + +def test_get_variables_reports_the_declares_var_layer(both): + variables = both.get_variables() + declaring = {key: v for key, v in variables.items() if v} + assert len(declaring) == 235, "a1's callables that declare a local" + assert sum(len(v) for v in variables.values()) == 854, "a1's local variable declarations" + sample = variables[f"{BEANS_PKG}.MarketSummaryDataBean.toString()"] + assert [v.name for v in sample] == ["ret", "it", "quoteData", "quoteData"] + assert all(v.type for v in sample) + + +def test_get_variables_is_not_fields_and_not_parameters(both): + """Three different things with three different homes; this one is ``J_DECLARES_VAR`` only.""" + variables = both.get_variables() + fields = {f.name for f in both.get_all_fields(f"{BEANS_PKG}.MarketSummaryDataBean")} + locals_ = {v.name for entry in variables.values() for v in entry} + assert "summaryDate" in fields and "summaryDate" not in locals_ + + +# ---- get_class_hierarchy --------------------------------------------------------------------- +def test_get_class_hierarchy_is_a_digraph_of_declared_types(both): + graph = both.get_class_hierarchy() + assert isinstance(graph, nx.DiGraph) + assert set(both.get_all_classes()) <= set(graph.nodes), "every declared type is a node, isolated ones included" + assert graph.number_of_nodes() == 170 and graph.number_of_edges() == 103 + assert sum(1 for n in graph.nodes if graph.degree(n) == 0) == 43 + + +def test_get_class_hierarchy_keeps_extends_and_implements_apart(both): + """Java projects the two as separate relationship types (``J_EXTENDS`` 1,197 / ``J_IMPLEMENTS`` + 959 on the reference graph); collapsing them here would throw that away.""" + graph = both.get_class_hierarchy() + kinds = [d["type"] for _, _, d in graph.edges(data=True)] + assert kinds.count("EXTENDS") == 58 and kinds.count("IMPLEMENTS") == 45 + assert graph.edges[TRADE_DIRECT, TRADE_SERVICES]["type"] == "IMPLEMENTS" + assert graph.edges[PING_SERVLET, "javax.servlet.http.HttpServlet"]["type"] == "EXTENDS" + + +def test_get_class_hierarchy_agrees_with_the_per_class_accessors(both): + for name in (TRADE_DIRECT, PING_SERVLET, f"{BEANS_PKG}.MarketSummaryDataBean"): + graph = both.get_class_hierarchy() + out = {v: d["type"] for _, v, d in graph.out_edges(name, data=True)} + assert {v for v, t in out.items() if t == "EXTENDS"} == set(both.get_extended_classes(name)) + assert {v for v, t in out.items() if t == "IMPLEMENTS"} == set(both.get_implemented_interfaces(name)) + + +def test_get_class_hierarchy_names_out_of_project_supertypes_as_declared(both): + """A library base becomes a node by being an edge endpoint, spelled exactly as the declaration + wrote it — **type arguments included**, because that is what the analyzer carries.""" + graph = both.get_class_hierarchy() + external = set(graph.nodes) - set(both.get_all_classes()) + assert len(external) == 21 + assert "javax.servlet.http.HttpServlet" in external + assert "java.util.Comparator" in external + + +# ---- get_methods_with_annotations ------------------------------------------------------------- +def test_get_methods_with_annotations_reads_the_analyzers_own_annotations(both): + found = both.get_methods_with_annotations(["Override"]) + assert len(found["Override"]) == 328, "a1's @Override-annotated callables" + methods = both.get_all_methods_in_application() + for entry in found["Override"]: + assert set(entry) == {"class", "signature", "method_name", "body"} + callable_ = methods[entry["class"]][entry["signature"]] + assert entry["method_name"] == entry["signature"].partition("(")[0] + assert entry["body"] == callable_.code + assert "Override" in {d.name.rpartition(".")[2] for d in callable_.decorators} + + +def test_get_methods_with_annotations_keys_by_the_spelling_the_caller_passed(both): + """The J-5 marker rule (``@`` stripped, compared after the last ``.``) matches; the *key* is + the caller's own string, so ``result[a]`` works for every ``a`` that was asked about.""" + found = both.get_methods_with_annotations(["@Override", "org.junit.Override", "Inject"]) + assert set(found) == {"@Override", "org.junit.Override", "Inject"} + assert len(found["@Override"]) == len(found["org.junit.Override"]) == 328 + assert len(found["Inject"]) == 12 + + +def test_get_methods_with_annotations_omits_what_nothing_carries(both): + """The 1.x shape. ``WebServlet`` is on 53 **types** in a1 and on no callable, so a + callable-level filter must not report it — the same split + :meth:`get_decorated_callables` has.""" + assert both.get_methods_with_annotations(["WebServlet", "Test"]) == {} + assert both.get_methods_with_annotations([]) == {} + + +def test_get_methods_with_annotations_agrees_with_get_decorated_callables(both): + """Two spellings of one filter, so they must not come to disagree about who carries a marker.""" + found = both.get_methods_with_annotations(["Override"])["Override"] + assert {f"{e['class']}.{e['signature']}" for e in found} == {o.key for o in both.get_decorated_callables(["Override"])} + + +# ---- get_call_targets ------------------------------------------------------------------------- +def test_get_call_targets_matches_declared_names_against_every_call_site(both): + declared = both.get_all_methods_in_class(TRADE_DIRECT) + targets = both.get_call_targets(declared) + assert len(targets) == 53 + assert targets <= {sig.rpartition("(")[0] or sig for sig in declared}, "never a name the caller did not ask about" + assert "cancelOrder" in targets + + +def test_get_call_targets_accepts_bare_names_as_well_as_signatures(both): + declared = both.get_all_methods_in_class(TRADE_DIRECT) + bare = {sig.rpartition("(")[0]: c for sig, c in declared.items()} + assert both.get_call_targets(bare) == both.get_call_targets(declared) + + +def test_get_call_targets_of_nothing_is_nothing(both): + assert both.get_call_targets({}) == set() + assert both.get_call_targets({"noSuchMethodAnywhere()": None}) == set() + + +# ---- get_calling_lines (needs a call graph, so a4) -------------------------------------------- +def test_get_calling_lines_reads_the_call_graphs_own_absolute_lines(both_l4): + lines = both_l4.get_calling_lines(GET_STATEMENT) + assert lines == sorted(set(lines)) and len(lines) == 36 + assert lines[0] == 205 + graph = both_l4.get_call_graph() + on_edges = {ln for _, dst, d in graph.edges(data=True) for ln in d["calling_lines"] if graph.nodes[dst]["method_detail"].method.signature.partition("(")[0] == GET_STATEMENT} + assert set(lines) == on_edges, "nothing is re-derived; CallingLines is the one place a call site becomes a file line" + + +def test_get_calling_lines_accepts_a_signature_and_cuts_it(both_l4): + assert both_l4.get_calling_lines("getStatement(java.sql.Connection, java.lang.String)") == both_l4.get_calling_lines(GET_STATEMENT) + assert len(both_l4.get_calling_lines("cancelOrder")) == 4 + + +def test_get_calling_lines_of_an_uncalled_name_is_empty(both_l4): + assert both_l4.get_calling_lines("noSuchMethodAnywhere") == [] + assert both_l4.get_calling_lines("") == [] + + +# ---- the two backends, compared directly ------------------------------------------------------ +def test_the_two_backends_agree_on_all_six(analysis_json, analysis_json_a4): + for payload in (analysis_json, analysis_json_a4): + local, graph = _local(payload), _graph(payload) + assert local.get_imports() == graph.get_imports() + assert local.get_variables() == graph.get_variables() + assert nx.utils.graphs_equal(local.get_class_hierarchy(), graph.get_class_hierarchy()) + assert local.get_methods_with_annotations(["Override", "Inject"]) == graph.get_methods_with_annotations(["Override", "Inject"]) + declared = local.get_all_methods_in_class(TRADE_DIRECT) + assert local.get_call_targets(declared) == graph.get_call_targets(declared) + assert local.get_calling_lines(GET_STATEMENT) == graph.get_calling_lines(GET_STATEMENT) + + +# ---- the facade's own contribution: ``get_variables(**kwargs)`` -------------------------------- +def _facade(test_fixture, analysis_json, tmp_path) -> JavaAnalysis: + with patch("cldk.analysis.java.codeanalyzer.codeanalyzer.subprocess.run") as run_mock: + run_mock.return_value = MagicMock(stdout=analysis_json, returncode=0) + (tmp_path / "java").mkdir() + (tmp_path / "java" / "analysis.json").write_text(analysis_json, encoding="utf-8") + return JavaAnalysis( + project_dir=test_fixture, + analysis_level=AnalysisLevel.symbol_table, + target_files=None, + eager_analysis=False, + backend=CodeAnalyzerConfig(cache_dir=str(tmp_path)), + ) + + +def test_get_variables_refuses_a_filter_it_does_not_have(test_fixture, analysis_json, tmp_path): + """The frozen signature keeps ``**kwargs``; ignoring one would return an unfiltered answer that + reads as a filtered one.""" + analysis = _facade(test_fixture, analysis_json, tmp_path) + assert len(analysis.get_variables()) == 1216 + with pytest.raises(TypeError, match="unexpected keyword argument 'qualified_class_name'"): + analysis.get_variables(qualified_class_name=TRADE_DIRECT) diff --git a/tests/analysis/java/test_java_v1_accessors_live.py b/tests/analysis/java/test_java_v1_accessors_live.py new file mode 100644 index 00000000..581c70f4 --- /dev/null +++ b/tests/analysis/java/test_java_v1_accessors_live.py @@ -0,0 +1,294 @@ +################################################################################ +# Copyright IBM Corporation 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +################################################################################ + +r"""The six 1.x accessors (#366) against a real graph, on both backends. + +The offline suite proves the policy over a fixture both backends are seeded from, which is exactly +what a fixture *cannot* prove: that the projection carries the same facts as ``analysis.json``. +This runs the real graph -- daytrader8 in a database that also holds ThingsBoard, so a scope leak +shows up as a larger answer -- against the level-4 reference cache, and compares the two answers. + +**Three divergences are pinned here rather than hidden**, because each is a property of the +projection and not of these accessors: + +* ``get_variables`` agrees on ``name``/``type``/``start_line`` and *not* on the span's columns or + byte offsets: ``:JLocal`` carries a line-only span, as every other node on this surface does. + That is also why the lists are sorted by ``(line, name)`` -- 6 of daytrader8's callables declare + two variables on one line, and the graph fixes no order between them. +* ``get_methods_with_annotations`` agrees on ``class``/``signature``/``method_name`` and not on + ``body``, which is :attr:`~cldk.models.java.models.JCallable.code` -- the body block off + ``analysis.json`` and the whole declaration off the projection, the documented model property + :meth:`JavaAnalysis.get_test_methods` also hands back. +* ``get_class_hierarchy`` is built from each declaration's own ``base_types``/``interfaces`` rather + than from ``J_EXTENDS``/``J_IMPLEMENTS``, and the test below says what that is worth: the + relationships join **8** of daytrader8's type pairs, the declarations **103**. + +Same environment as ``test_java_addressing_live.py``:: + + CLDK_TEST_NEO4J_URI=bolt://localhost:7691 \ + CLDK_TEST_NEO4J_USER=neo4j \ + CLDK_TEST_NEO4J_PASSWORD=... \ + CLDK_TEST_NEO4J_APP=daytrader8 \ + CLDK_TEST_JAVA_PROJECT=/path/to/project \ + CLDK_TEST_JAVA_CACHE=/path/to/dir \ # a level-4 reference analysis.json + uv run pytest tests/analysis/java/test_java_v1_accessors_live.py + +Read-only, like every other Neo4j suite here. +""" + +import json +import logging +import os +from pathlib import Path + +import networkx as nx +import pytest + +logging.getLogger("neo4j").setLevel(logging.ERROR) + +NEO4J_URI = os.environ.get("CLDK_TEST_NEO4J_URI", "bolt://localhost:7687") +NEO4J_USER = os.environ.get("CLDK_TEST_NEO4J_USER", "neo4j") +NEO4J_PASSWORD = os.environ.get("CLDK_TEST_NEO4J_PASSWORD", "neo4j") +JAVA_APP = os.environ.get("CLDK_TEST_NEO4J_APP") +JAVA_PROJECT = os.environ.get("CLDK_TEST_JAVA_PROJECT") +JAVA_CACHE = os.environ.get("CLDK_TEST_JAVA_CACHE") +SCALE_APP = os.environ.get("CLDK_TEST_NEO4J_SCALE_APP", "thingsboard") + +REFERENCE_LEVEL = "system_dependency_graph" + +DIRECT = "com.ibm.websphere.samples.daytrader.impl.direct" +TRADE_DIRECT = f"{DIRECT}.TradeDirect" +PING_SERVLET = "com.ibm.websphere.samples.daytrader.web.prims.PingServlet" + +#: Measured on the reference graph (daytrader8, codeanalyzer-java 3.0.3), 2026-09-07. +DT_IMPORTS = 268 +DT_CALLABLES = 1216 +DT_DECLARING_LOCALS, DT_LOCALS = 235, 854 +DT_HIERARCHY_NODES, DT_EXTENDS, DT_IMPLEMENTS = 170, 58, 45 +DT_EXTERNAL_SUPERTYPES = 21 +DT_OVERRIDE, DT_INJECT = 328, 12 +DT_TRADE_DIRECT_TARGETS = 53 +DT_GET_STATEMENT_LINES, DT_CANCEL_ORDER_LINES = 56, 5 + +#: The same six on ThingsBoard, which is the corpus with the kinds daytrader8 has none of. +TB_IMPORTS = 5607 +TB_CALLABLES, TB_LOCALS = 28763, 29813 +TB_HIERARCHY_NODES, TB_EXTENDS, TB_IMPLEMENTS = 6279, 2579, 1508 +TB_OVERRIDE, TB_TEST = 6396, 3176 + + +def _reference_cache_is_level_4() -> bool: + if not JAVA_CACHE: + return False + try: + return int(json.loads((Path(JAVA_CACHE) / "analysis.json").read_text(encoding="utf-8")).get("max_level", 0)) >= 4 + except (OSError, ValueError, AttributeError): + return False + + +def _reachable() -> bool: + if not (JAVA_APP and JAVA_PROJECT and _reference_cache_is_level_4()): + return False + try: + from neo4j import GraphDatabase + except ModuleNotFoundError: + return False + try: + driver = GraphDatabase.driver(NEO4J_URI, auth=(NEO4J_USER, NEO4J_PASSWORD)) + driver.verify_connectivity() + driver.close() + return True + except Exception: # noqa: BLE001 + return False + + +pytestmark = pytest.mark.skipif(not _reachable(), reason="needs a pre-populated Neo4j Java graph + a level-4 reference cache (set CLDK_TEST_NEO4J_* / CLDK_TEST_JAVA_*)") + + +@pytest.fixture(scope="module") +def backends(): + from cldk.analysis.java.codeanalyzer.codeanalyzer import JCodeanalyzer + from cldk.analysis.java.neo4j import JNeo4jBackend + + ref = JCodeanalyzer(project_dir=JAVA_PROJECT, analysis_json_path=JAVA_CACHE, analysis_level=REFERENCE_LEVEL, eager_analysis=False, target_files=None) + neo = JNeo4jBackend(neo4j_uri=NEO4J_URI, neo4j_username=NEO4J_USER, neo4j_password=NEO4J_PASSWORD, application_name=JAVA_APP) + yield ref, neo + neo.close() + + +@pytest.fixture(scope="module") +def scale(): + from cldk.analysis.java.neo4j import JNeo4jBackend + + neo = JNeo4jBackend(neo4j_uri=NEO4J_URI, neo4j_username=NEO4J_USER, neo4j_password=NEO4J_PASSWORD, application_name=SCALE_APP) + if not neo._run("MATCH (a:JApplication {name: $app}) RETURN a LIMIT 1", app=SCALE_APP): + neo.close() + pytest.skip(f"the graph does not hold application {SCALE_APP!r}") + yield neo + neo.close() + + +# ---- get_imports ------------------------------------------------------------------------------ +def test_the_import_targets_agree_exactly(backends): + ref, neo = backends + assert ref.get_imports() == neo.get_imports(), "the projection's aggregated J_IMPORTS edges carry the same targets" + assert len(ref.get_imports()) == DT_IMPORTS + + +def test_the_import_set_is_smaller_than_the_per_file_declarations(backends): + """What the set costs: the same target imported by several files collapses, and file order is + gone with it. What it buys: an answer the two backends can both give.""" + ref, _ = backends + per_file = sum(len(unit.import_declarations) for unit in ref.get_symbol_table().values()) + assert per_file > DT_IMPORTS + assert set(ref.get_imports()) == {i.path for unit in ref.get_symbol_table().values() for i in unit.import_declarations} + + +# ---- get_variables ---------------------------------------------------------------------------- +def test_the_local_variables_agree_name_type_and_line(backends): + """``J_DECLARES_VAR`` carries a **line-only** span, so the columns and byte offsets are + placeholders over the graph exactly as they are everywhere else on this surface; everything + that is a fact about the variable agrees.""" + ref, neo = backends + local, graph = ref.get_variables(), neo.get_variables() + assert set(local) == set(graph) and len(local) == DT_CALLABLES + for key, declared in local.items(): + assert [(v.name, v.type, v.start_line, v.initializer) for v in declared] == [(v.name, v.type, v.start_line, v.initializer) for v in graph[key]], key + assert sum(1 for v in local.values() if v) == DT_DECLARING_LOCALS + assert sum(len(v) for v in local.values()) == DT_LOCALS + + +def test_the_ordering_is_fixed_because_the_graph_does_not_fix_it(backends): + """6 of daytrader8's callables declare two variables on one line (``String htmlString, + arrow;``). The projection has no column to order them by, so the accessor sorts and both + backends agree; without the sort the multiset matched and the list did not.""" + ref, neo = backends + local, graph = ref.get_variables(), neo.get_variables() + same_line = [key for key, declared in local.items() if len({v.start_line for v in declared}) < len(declared)] + assert len(same_line) == 6 + for key in same_line: + assert [v.name for v in local[key]] == [v.name for v in graph[key]] + assert [(v.start_line, v.name) for v in local[key]] == sorted((v.start_line, v.name) for v in local[key]), "(line, name), so a shared line has one order" + + +# ---- get_class_hierarchy ---------------------------------------------------------------------- +def test_the_class_hierarchies_are_the_same_graph(backends): + ref, neo = backends + local, graph = ref.get_class_hierarchy(), neo.get_class_hierarchy() + assert nx.utils.graphs_equal(local, graph) + assert local.number_of_nodes() == DT_HIERARCHY_NODES + kinds = [d["type"] for _, _, d in local.edges(data=True)] + assert kinds.count("EXTENDS") == DT_EXTENDS and kinds.count("IMPLEMENTS") == DT_IMPLEMENTS + assert graph.edges[PING_SERVLET, "javax.servlet.http.HttpServlet"]["type"] == "EXTENDS" + + +def test_reading_the_relationships_instead_would_lose_most_of_the_hierarchy(backends): + """The reason the declaration's own ``base_types``/``interfaces`` are read and + ``J_EXTENDS``/``J_IMPLEMENTS`` are not: a relationship needs a node at both ends, and almost + every supertype daytrader8 names is a library type the projection has no node for.""" + _, neo = backends + counted = { + rel: neo._run(f"MATCH (:JApplication {{name: $app}})-[:J_HAS_MODULE]->(:JModule)-[:J_DECLARES*1..4]->(t:JType)-[e:{rel}]->() RETURN count(e) AS n", app=JAVA_APP)[0]["n"] + for rel in ("J_EXTENDS", "J_IMPLEMENTS") + } + assert counted == {"J_EXTENDS": 1, "J_IMPLEMENTS": 7} + assert sum(counted.values()) < neo.get_class_hierarchy().number_of_edges() // 10 + + +def test_out_of_project_supertypes_are_nodes_spelled_as_declared(backends): + ref, neo = backends + external = set(neo.get_class_hierarchy().nodes) - set(neo.get_all_classes()) + assert external == set(ref.get_class_hierarchy().nodes) - set(ref.get_all_classes()) + assert len(external) == DT_EXTERNAL_SUPERTYPES + assert "java.util.Comparator" in external + + +# ---- get_methods_with_annotations -------------------------------------------------------------- +def _without_bodies(found): + return {marker: [{k: v for k, v in entry.items() if k != "body"} for entry in entries] for marker, entries in found.items()} + + +def test_the_annotated_callables_agree_entry_for_entry(backends): + ref, neo = backends + asked = ["Override", "@Inject", "WebServlet"] + local, graph = ref.get_methods_with_annotations(asked), neo.get_methods_with_annotations(asked) + assert _without_bodies(local) == _without_bodies(graph) + assert set(local) == {"Override", "@Inject"}, "WebServlet is a type annotation here, so a callable filter omits it" + assert len(local["Override"]) == DT_OVERRIDE and len(local["@Inject"]) == DT_INJECT + + +def test_the_body_is_the_documented_per_backend_code(backends): + """Not a divergence this accessor introduces: ``body`` is + :attr:`~cldk.models.java.models.JCallable.code`, which the model documents as the body block + off ``analysis.json`` and the whole declaration off the projection. Pinned so that it is + visible rather than surprising.""" + ref, neo = backends + local = ref.get_methods_with_annotations(["Override"])["Override"] + graph = neo.get_methods_with_annotations(["Override"])["Override"] + assert all(entry["body"] for entry in local) and all(entry["body"] for entry in graph) + assert all(entry["body"].lstrip().startswith("{") for entry in local), "the body block" + assert sum(1 for entry in graph if not entry["body"].lstrip().startswith("{")) > DT_OVERRIDE // 2, "the declaration" + + +# ---- get_call_targets --------------------------------------------------------------------------- +def test_the_call_targets_agree(backends): + ref, neo = backends + declared = ref.get_all_methods_in_class(TRADE_DIRECT) + assert ref.get_call_targets(declared) == neo.get_call_targets(neo.get_all_methods_in_class(TRADE_DIRECT)) + assert len(ref.get_call_targets(declared)) == DT_TRADE_DIRECT_TARGETS + assert ref.get_call_targets({}) == neo.get_call_targets({}) == set() + + +# ---- get_calling_lines -------------------------------------------------------------------------- +def test_the_calling_lines_agree_and_are_absolute_file_lines(backends): + """Leg 3a made these agree rather than shift by a declaration prefix; this is the assertion + that keeps them agreeing over a real projection, where ``code`` and ``code_start_line`` really + do differ between the two backends.""" + ref, neo = backends + lines = ref.get_calling_lines("getStatement") + assert lines == neo.get_calling_lines("getStatement") + assert lines == sorted(set(lines)) and len(lines) == DT_GET_STATEMENT_LINES + assert ref.get_calling_lines("cancelOrder") == neo.get_calling_lines("cancelOrder") + assert len(ref.get_calling_lines("cancelOrder")) == DT_CANCEL_ORDER_LINES + assert ref.get_calling_lines("noSuchMethodAnywhere") == neo.get_calling_lines("noSuchMethodAnywhere") == [] + + +def test_a_calling_line_points_at_the_call_in_the_file(backends): + """The number is an index into the file, not into ``JCallable.code`` -- so it can be checked + against the source on disk.""" + ref, _ = backends + path = Path(JAVA_PROJECT) / ref.get_java_file(TRADE_DIRECT) + source = path.read_text(encoding="utf-8").splitlines() + for line in ref.get_calling_lines("getStatement"): + if line <= len(source) and "getStatement" in source[line - 1]: + break + else: # pragma: no cover - a failure path + pytest.fail("no reported line of TradeDirect.java holds a call to getStatement") + + +# ---- the scale corpus ---------------------------------------------------------------------------- +def test_the_six_answer_on_the_scale_corpus(scale): + """ThingsBoard, in the same database, so a scope leak reads as a larger answer. The corpus that + actually has ``@Test`` methods -- daytrader8 has none.""" + assert len(scale.get_imports()) == TB_IMPORTS + variables = scale.get_variables() + assert len(variables) == TB_CALLABLES and sum(len(v) for v in variables.values()) == TB_LOCALS + hierarchy = scale.get_class_hierarchy() + kinds = [d["type"] for _, _, d in hierarchy.edges(data=True)] + assert hierarchy.number_of_nodes() == TB_HIERARCHY_NODES + assert kinds.count("EXTENDS") == TB_EXTENDS and kinds.count("IMPLEMENTS") == TB_IMPLEMENTS + annotated = scale.get_methods_with_annotations(["Override", "Test"]) + assert len(annotated["Override"]) == TB_OVERRIDE and len(annotated["Test"]) == TB_TEST