feat(java): the six deferred v1 accessors answer, on both backends - #367
Merged
Merged
Conversation
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.
…raph 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.
…-java-v1-accessors
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #366.
Six Java accessors that raised
NotImplementedErrornow answer, on both backends.RAISINGshrinks from nine to three.No new Cypher. All six are implemented once on
JavaAnalysisBackendover accessors both backends already answer — the Neo4j reconstruction already readsJ_IMPORTS,J_DECLARES_VAR,J_EXTENDS/J_IMPLEMENTS,J_ANNOTATED_BYandJ_CALLSinto the models. Every frozen v1 signature is unmoved.Decisions the docstrings left open
get_imports— distinct sorted set. Import order is not recoverable from the projection (imports aggregate per target onto one edge), so an ordered list would be one the two backends disagree about. Asserted as a set, and against the union of per-fileimport_declarations.get_variables— locals only, keyed by the J-1 address, sorted by(line, name). Unsorted, the backends returned the same multiset in different orders for 6 daytrader8 callables::JLocalcarries a line-only span, soString htmlString, arrow;has no order to preserve. Unexpected**kwargsraises rather than silently returning an unfiltered answer that looks filtered.get_class_hierarchy— built from each declaration'sbase_types/interfaces, not fromJ_EXTENDS/J_IMPLEMENTS. Measured: those relationships join 8 daytrader8 type pairs where the declarations join 103, because a relationship needs a node at both ends and nearly every supertype is a library type. A test counts both so the choice cannot silently regress.get_methods_with_annotations— no sibling implementation in any language, so modelled on Java's ownget_test_methods. Keyed by the spelling the caller passed; sorted, because the backends walk callables in different orders.bodyisJCallable.code, which is the body block in process and the whole declaration over the graph — the same documented lossinessget_test_methodsalready has, pinned explicitly rather than hidden.get_call_targets— the frozen signature has no body parameter, so the domain is the whole application. Simple-name matching only, as the docstring specifies; no overload resolution.get_calling_lines— readsget_call_graph()'s owncalling_linesrather than re-deriving them, so there is one spelling of "absolute file line", not two.Verification
Full gate 1571 passed, 367 skipped, 85.22% (from 1537 / 352 / 85.11%). Java live against the two-application 3.0.3 graph 686 passed, 4 skipped (from 641 / 2). Counts reconcile: +53 new tests − 6 removed
RAISINGparameters = +47.New figures pinned live — daytrader8: 268 imports, 235 callables declaring 854 locals, 170 hierarchy nodes, 328
@Override, 53TradeDirectcall targets. ThingsBoard: 5,607 imports, 29,813 locals, 6,279 hierarchy nodes, 3,176@Test.Still raising
get_service_entry_point_classes,get_service_entry_point_methods,remove_all_comments— the last needs comment nodes in the projection (codeanalyzer-java#231).