diff --git a/graphify/extractors/apex.py b/graphify/extractors/apex.py index 928923a64..abbdc4f0a 100644 --- a/graphify/extractors/apex.py +++ b/graphify/extractors/apex.py @@ -54,15 +54,57 @@ def add_edge(src: str, tgt: str, relation: str, line: int, _SHARING = r"(?:\s+(?:with|without|inherited)\s+sharing)?" _MOD = r"(?:\s+(?:abstract|virtual|override|static|final|transient|testMethod))?" _ANNOTATION = r"(?:\s*@\w+(?:\s*\([^)]*\))?\s*)*" + # An Apex type expression is not one bare word: it can be namespace-qualified + # (`Database.QueryLocator`) and can carry generic arguments holding commas + # and spaces (`Map`, `List>`). Apex also + # permits whitespace around the angle brackets themselves (`Map `, + # `List< Account >`), so the type is read as segments joined by the type + # punctuators `<`, `>` and `,`, with whitespace allowed only ADJACENT to one + # of them - never between two bare words. That is what keeps a statement such + # as `insert new Account(...)` from being read as a declaration (#3217). + # The same shape covers a heritage clause, which is a comma-separated list of + # such types (#3277). + _TYPE = r"[\w.\[\]]+(?:\s*[<>,]\s*[\w.\[\]]*)*" + + def heritage_names(clause: str) -> list[str]: + """Split an Apex heritage clause into the type names it actually names. + + `Database.Batchable, Schedulable` -> `["Batchable", "Schedulable"]`. + Commas are split at top level only, so a generic argument list + (`Map`) is not shredded into fragments, and each entry + drops its generic arguments and resolves to its tail segment: a qualified + base names a type *in* a namespace, not the namespace itself. This is the + same tail-name treatment Kotlin (#1793) and Scala (#1794) already use. + """ + parts: list[str] = [] + current: list[str] = [] + depth = 0 + for ch in clause: + if ch == "<": + depth += 1 + elif ch == ">": + depth = max(0, depth - 1) + elif ch == "," and depth == 0: + parts.append("".join(current)) + current = [] + continue + current.append(ch) + parts.append("".join(current)) + names: list[str] = [] + for raw in parts: + name = raw.split("<", 1)[0].strip().rsplit(".", 1)[-1].strip() + if name: + names.append(name) + return names cls_re = _re.compile( rf"^{_ANNOTATION}\s*{_ACCESS}{_SHARING}{_MOD}\s*class\s+(\w+)" - rf"(?:\s+extends\s+(\w+))?(?:\s+implements\s+([\w,\s]+))?\s*\{{?", + rf"(?:\s+extends\s+({_TYPE}))?(?:\s+implements\s+({_TYPE}))?\s*\{{?", _re.IGNORECASE, ) iface_re = _re.compile( rf"^{_ANNOTATION}\s*{_ACCESS}{_SHARING}{_MOD}\s*interface\s+(\w+)" - rf"(?:\s+extends\s+([\w,\s]+))?\s*\{{?", + rf"(?:\s+extends\s+({_TYPE}))?\s*\{{?", _re.IGNORECASE, ) enum_re = _re.compile( @@ -74,7 +116,7 @@ def add_edge(src: str, tgt: str, relation: str, line: int, _re.IGNORECASE, ) method_re = _re.compile( - rf"^{_ANNOTATION}\s*{_ACCESS}{_MOD}\s*(?:static\s+)?[\w<>\[\]]+\s+(\w+)\s*\([^)]*\)\s*(?:throws\s+\w+\s*)?\{{?", + rf"^{_ANNOTATION}\s*{_ACCESS}{_MOD}\s*(?:static\s+)?{_TYPE}\s+(\w+)\s*\([^)]*\)\s*(?:throws\s+\w+\s*)?\{{?", _re.IGNORECASE, ) annotation_re = _re.compile(r"@(\w+)", _re.IGNORECASE) @@ -123,23 +165,21 @@ def add_edge(src: str, tgt: str, relation: str, line: int, add_node(class_nid, class_name, lineno) add_edge(file_nid, class_nid, "contains", lineno) if cm.group(2): - base = cm.group(2).strip() - base_nid = _make_id(stem, base) - if base_nid not in seen_ids: - base_nid = _make_id(base) - if base_nid not in seen_ids: - add_node(base_nid, base, lineno) - add_edge(class_nid, base_nid, "extends", lineno, confidence="INFERRED") + for base in heritage_names(cm.group(2)): + base_nid = _make_id(stem, base) + if base_nid not in seen_ids: + base_nid = _make_id(base) + if base_nid not in seen_ids: + add_node(base_nid, base, lineno) + add_edge(class_nid, base_nid, "extends", lineno, confidence="INFERRED") if cm.group(3): - for iface in cm.group(3).split(","): - iface = iface.strip() - if iface: - iface_nid = _make_id(stem, iface) - if iface_nid not in seen_ids: - iface_nid = _make_id(iface) - if iface_nid not in seen_ids: - add_node(iface_nid, iface, lineno) - add_edge(class_nid, iface_nid, "implements", lineno, confidence="INFERRED") + for iface in heritage_names(cm.group(3)): + iface_nid = _make_id(stem, iface) + if iface_nid not in seen_ids: + iface_nid = _make_id(iface) + if iface_nid not in seen_ids: + add_node(iface_nid, iface, lineno) + add_edge(class_nid, iface_nid, "implements", lineno, confidence="INFERRED") current_class_nid = class_nid pending_annotations = [] continue @@ -155,15 +195,13 @@ def add_edge(src: str, tgt: str, relation: str, line: int, add_edge(file_nid if current_class_nid is None else current_class_nid, iface_nid, "contains", lineno) if im.group(2): - for parent in im.group(2).split(","): - parent = parent.strip() - if parent: - parent_nid = _make_id(stem, parent) - if parent_nid not in seen_ids: - parent_nid = _make_id(parent) - if parent_nid not in seen_ids: - add_node(parent_nid, parent, lineno) - add_edge(iface_nid, parent_nid, "extends", lineno, confidence="INFERRED") + for parent in heritage_names(im.group(2)): + parent_nid = _make_id(stem, parent) + if parent_nid not in seen_ids: + parent_nid = _make_id(parent) + if parent_nid not in seen_ids: + add_node(parent_nid, parent, lineno) + add_edge(iface_nid, parent_nid, "extends", lineno, confidence="INFERRED") pending_annotations = [] continue diff --git a/tests/test_languages.py b/tests/test_languages.py index 46dae524c..d3cb1da24 100644 --- a/tests/test_languages.py +++ b/tests/test_languages.py @@ -3391,6 +3391,43 @@ def test_apex_interface_extends(tmp_path): assert ("PaymentProcessor", "Processor") in inheritance assert ("PaymentProcessor", "Auditable") in inheritance +def test_apex_qualified_heritage_uses_tail_type(tmp_path): + """A namespace-qualified base names a type *in* a namespace, not the + namespace itself. `implements Database.Batchable, Schedulable` used + to stop at the dot, fabricating a `Database` node and losing `Schedulable` + with it. Resolve to the tail type name, as Kotlin (#1793) and Scala (#1794) + already do, and split the list on top-level commas only so a generic + argument (`Map`) is not shredded (#3277). + """ + source = tmp_path / "Batch.cls" + source.write_text( + "public with sharing class Batch extends Outer.BaseThing " + "implements Database.Batchable, Map, Schedulable {\n" + " public void execute(Database.BatchableContext bc) { }\n" + "}\n" + ) + result = extract_apex(source) + labels = _labels(result) + assert ("Batch", "BaseThing") in _edge_labels(result, "extends") + implements = _edge_labels(result, "implements") + assert ("Batch", "Batchable") in implements + assert ("Batch", "Map") in implements + assert ("Batch", "Schedulable") in implements + # the namespace is not a type, and a generic argument is not a base + assert "Database" not in labels + assert "Outer" not in labels + assert "sObject" not in labels + assert "Object" not in labels + +def test_apex_interface_qualified_extends_uses_tail_type(tmp_path): + source = tmp_path / "Combo.cls" + source.write_text("public interface Combo extends Pkg.Base, Auditable { }\n") + result = extract_apex(source) + extends = _edge_labels(result, "extends") + assert ("Combo", "Base") in extends + assert ("Combo", "Auditable") in extends + assert "Pkg" not in _labels(result) + def test_apex_method_extraction(): r = extract_apex(FIXTURES / "sample.cls") labels = _labels(r) @@ -3399,6 +3436,56 @@ def test_apex_method_extraction(): assert any("createAccounts" in l for l in labels) assert any("deleteOldAccounts" in l for l in labels) +def test_apex_method_qualified_and_generic_return_types(tmp_path): + source = tmp_path / "Repro.cls" + source.write_text( + "public with sharing class Repro {\n" + " public static String simpleReturn() { return ''; }\n" + " public static Map commaGeneric() { return null; }\n" + " public Database.QueryLocator dottedReturn(Database.BatchableContext bc) { return null; }\n" + " private static List> nestedGeneric() { return null; }\n" + " global Set setReturn() { return null; }\n" + " public String[] arrayReturn() { return null; }\n" + # Apex permits whitespace around the angle brackets themselves + " public Map spaceBeforeAngle() { return null; }\n" + " public List< Account > spacesInside() { return null; }\n" + " public List < Map< String, Id > > roomy() { return null; }\n" + "}\n" + ) + result = extract_apex(source) + labels = _labels(result) + assert ".simpleReturn()" in labels + assert ".commaGeneric()" in labels + assert ".dottedReturn()" in labels + assert ".nestedGeneric()" in labels + assert ".setReturn()" in labels + assert ".arrayReturn()" in labels + assert ".spaceBeforeAngle()" in labels + assert ".spacesInside()" in labels + assert ".roomy()" in labels + +def test_apex_statements_are_not_read_as_methods(tmp_path): + source = tmp_path / "Neg.cls" + source.write_text( + "public class Neg {\n" + " public void real() {\n" + " insert new Account(Name = 'x');\n" + " System.assertEquals(1, ids.size());\n" + " Map m = new Map();\n" + " results.put('a', compute(x));\n" + " this.helper(1, 2);\n" + " Integer a = 1, b = compute();\n" + # a bare `<` / `>` is a comparison, not a generic argument list + " if (a > b) { doIt(x); }\n" + " while (i < list.size()) { next(); }\n" + " String s = (Map) JSON.deserializeUntyped(raw);\n" + " }\n" + "}\n" + ) + result = extract_apex(source) + methods = {l for l in _labels(result) if l.startswith(".")} + assert methods == {".real()"} + def test_apex_contains_and_method_relations(): r = extract_apex(FIXTURES / "sample.cls") relations = _relations(r)