Skip to content

Commit 981c7cd

Browse files
Michael UsachenkoGitLab
authored andcommitted
Merge branch 'ajuckel/java-record-accessors-and-constructors' into 'main'
feat(code-graph): capture compact constructor defs and implicit accessors Closes #846 See merge request gitlab-org/orbit/knowledge-graph!1741
2 parents 6892658 + b862122 commit 981c7cd

3 files changed

Lines changed: 204 additions & 58 deletions

File tree

crates/code-graph/src/v2/langs/generic/java.rs

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,69 @@ fn java_super_types(node: &N<'_>) -> Vec<String> {
5959
result
6060
}
6161

62+
/// Extract implicit accessor definitions for record components, only when
63+
/// the body does not explicitly define a no-arg method that shares a name
64+
/// with that component.
65+
fn java_record_accessors(
66+
node: &N<'_>,
67+
defs: &mut Vec<crate::v2::types::CanonicalDefinition>,
68+
scope_stack: &[std::sync::Arc<str>],
69+
sep: &'static str,
70+
) -> bool {
71+
if node.kind() != "record_declaration" {
72+
return false;
73+
}
74+
let Some(params) = node.field("parameters") else {
75+
return false;
76+
};
77+
let no_arg_body_methods: Vec<String> = node
78+
.field("body")
79+
.map(|body| {
80+
body.children()
81+
.filter(|c| c.kind() == "method_declaration")
82+
.filter(|m| {
83+
m.field("parameters").is_none_or(|p| {
84+
!p.children().any(|c| {
85+
matches!(c.kind().as_ref(), "formal_parameter" | "spread_parameter")
86+
})
87+
})
88+
})
89+
.filter_map(|m| m.field("name").map(|n| n.text().to_string()))
90+
.collect()
91+
})
92+
.unwrap_or_default();
93+
for param in params.children() {
94+
if param.kind() != "formal_parameter" {
95+
continue;
96+
}
97+
let Some(name) = param.field("name").map(|n| n.text().to_string()) else {
98+
continue;
99+
};
100+
if no_arg_body_methods.contains(&name) {
101+
continue;
102+
}
103+
let return_type = field("type")
104+
.inner("type_arguments", "type_identifier")
105+
.apply(&param);
106+
let fqn = crate::v2::types::Fqn::from_scope(scope_stack, &name, sep);
107+
defs.push(crate::v2::types::CanonicalDefinition {
108+
definition_type: "Method",
109+
kind: DefKind::Method,
110+
name,
111+
fqn,
112+
range: crate::v2::dsl::utils::canonical_range(&crate::utils::node_to_range(&param)),
113+
is_top_level: false,
114+
metadata: return_type.map(|rt| {
115+
Box::new(crate::v2::types::DefinitionMetadata {
116+
return_type: Some(rt),
117+
..Default::default()
118+
})
119+
}),
120+
});
121+
}
122+
false
123+
}
124+
62125
impl DslLanguage for JavaDsl {
63126
fn name() -> &'static str {
64127
"java"
@@ -72,6 +135,7 @@ impl DslLanguage for JavaDsl {
72135
LanguageHooks {
73136
return_kinds: &["return_statement"],
74137
adopt_sibling_refs: &["marker_annotation", "annotation"],
138+
on_scope: Some(java_record_accessors),
75139
..LanguageHooks::default()
76140
}
77141
}
@@ -106,6 +170,7 @@ impl DslLanguage for JavaDsl {
106170
.type_annotation(field("type").inner("type_arguments", "type_identifier")),
107171
),
108172
scope("constructor_declaration", "Constructor").def_kind(DefKind::Constructor),
173+
scope("compact_constructor_declaration", "Constructor").def_kind(DefKind::Constructor),
109174
scope("method_declaration", "Method")
110175
.def_kind(DefKind::Method)
111176
.metadata(
@@ -443,6 +508,87 @@ mod tests {
443508
}
444509
}
445510

511+
#[test]
512+
fn record_compact_constructor() {
513+
let result = parse(
514+
"public record Bounds(int lo, int hi) {\n public Bounds {\n if (lo > hi) throw new IllegalArgumentException(\"lo > hi\");\n }\n}\n",
515+
)
516+
.unwrap();
517+
let ctor = result
518+
.definitions
519+
.iter()
520+
.find(|d| d.kind == DefKind::Constructor)
521+
.expect("compact constructor should be extracted");
522+
assert_eq!(ctor.name, "Bounds");
523+
assert_eq!(ctor.fqn.to_string(), "Bounds.Bounds");
524+
}
525+
526+
#[test]
527+
fn record_implicit_accessors() {
528+
let result = parse("public record Point(int x, int y) {}\n").unwrap();
529+
let accessors: Vec<_> = result
530+
.definitions
531+
.iter()
532+
.filter(|d| d.kind == DefKind::Method)
533+
.collect();
534+
assert_eq!(accessors.len(), 2);
535+
assert_eq!(accessors[0].fqn.to_string(), "Point.x");
536+
assert_eq!(accessors[1].fqn.to_string(), "Point.y");
537+
assert_eq!(
538+
accessors[0].metadata.as_ref().unwrap().return_type,
539+
Some("int".to_string())
540+
);
541+
}
542+
543+
#[test]
544+
fn record_parameterized_overload_does_not_suppress_accessor() {
545+
let result = parse(
546+
"public record Circle(int radius) {\n public int radius(int base) {\n return this.radius() + base;\n }\n}\n",
547+
)
548+
.unwrap();
549+
let mut lines: Vec<_> = result
550+
.definitions
551+
.iter()
552+
.filter(|d| d.name == "radius" && d.kind == DefKind::Method)
553+
.map(|d| d.range.start.line)
554+
.collect();
555+
lines.sort_unstable();
556+
// Line 0 is the synthetic accessor anchored to the component; line 1
557+
// is the explicit radius(int) overload, which must not suppress it.
558+
assert_eq!(lines, [0, 1]);
559+
}
560+
561+
#[test]
562+
fn record_zero_arg_override_beside_overload_suppresses_accessor() {
563+
let result = parse(
564+
"public record Circle(int radius) {\n public int radius() {\n return 0;\n }\n public int radius(int base) {\n return this.radius() + base;\n }\n}\n",
565+
)
566+
.unwrap();
567+
let mut lines: Vec<_> = result
568+
.definitions
569+
.iter()
570+
.filter(|d| d.name == "radius" && d.kind == DefKind::Method)
571+
.map(|d| d.range.start.line)
572+
.collect();
573+
lines.sort_unstable();
574+
assert_eq!(lines, [1, 4], "both defs must be the explicit declarations");
575+
}
576+
577+
#[test]
578+
fn record_explicit_accessor_override_not_duplicated() {
579+
let result = parse(
580+
"public record Wrapped(int raw) {\n public int raw() {\n return raw < 0 ? 0 : raw;\n }\n}\n",
581+
)
582+
.unwrap();
583+
let raws: Vec<_> = result
584+
.definitions
585+
.iter()
586+
.filter(|d| d.name == "raw" && d.kind == DefKind::Method)
587+
.collect();
588+
assert_eq!(raws.len(), 1);
589+
assert_ne!(raws[0].range.start.line, 0, "must be the explicit override");
590+
}
591+
446592
#[test]
447593
fn imports_extracted() {
448594
let result =

crates/integration-tests-codegraph/fixtures/java/enums.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ name: "Java enums — constants, constructor, fields, methods, interface impleme
99
#
1010
# Known gap (not asserted here): the compiler-synthesised enum members values(),
1111
# valueOf(), and ordinal() are not present in the source AST, so they are not
12-
# indexed — only members written in source are extracted. This mirrors the
13-
# synthesised-accessor gap documented in fixtures/java/records.yaml (#846).
12+
# indexed — only members written in source are extracted. Record component
13+
# accessors get synthetic definitions from the java.rs on_scope hook; enum
14+
# members have no equivalent yet.
1415

1516
fixtures:
1617
- path: com/example/enums/Describable.java

crates/integration-tests-codegraph/fixtures/java/records.yaml

Lines changed: 55 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
name: "Java 14+ records — definitions, members, call resolution, and pattern matching"
22

3-
# Gaps confirmed by this fixture — tracked in #846, do not expand this issue to fix them:
4-
# - compact_constructor_declaration has no scope rule → compact constructors not indexed.
5-
# Follow-up: add scope("compact_constructor_declaration", "Constructor") to java.rs.
6-
# - Implicit accessor methods generated from record components (e.g. x(), y()) are
7-
# compiler-synthesised and absent from the source AST → not indexed.
8-
# Additional gap tracked in #851:
3+
# Remaining gaps confirmed by this fixture:
4+
# - Implicit equals/hashCode/toString on records are compiler-synthesised and
5+
# not indexed; the java.rs on_scope hook only synthesises component accessors.
6+
# Gap tracked in #851:
97
# - Java 21 record patterns (record_pattern nodes) produce no edges at all:
108
# neither the type reference (instanceof Point(int x, int y)) nor accessor
11-
# calls for destructured components. Accessor-call emission for destructured
12-
# components additionally depends on implicit accessors existing (#846).
9+
# calls for destructured components.
1310

1411
fixtures:
1512
- path: com/example/records/Measurable.java
@@ -276,6 +273,15 @@ tests:
276273
- { row_count: 1 }
277274
- { row: { name: "Range" } }
278275

276+
- name: "Bounds compact constructor is extracted"
277+
query: |
278+
MATCH (cls:Definition)-[:DefinitionToDefinition]->(m:Definition)
279+
WHERE cls.fqn = 'com.example.records.Bounds' AND m.name = 'Bounds'
280+
AND m.definition_type = 'Constructor'
281+
RETURN m.fqn AS fqn
282+
assert:
283+
- { row_count: 1 }
284+
279285
# ── Methods ───────────────────────────────────────────────────────
280286

281287
- name: "Custom method length() in Range is extracted"
@@ -296,6 +302,20 @@ tests:
296302
- { row_count: 1 }
297303
- { row: { type: "Method" } }
298304

305+
- name: "Implicit accessors x() and y() are synthesised for Point components"
306+
query: |
307+
MATCH (cls:Definition)-[:DefinitionToDefinition]->(m:Definition)
308+
WHERE cls.fqn = 'com.example.records.Point'
309+
AND m.definition_type = 'Method'
310+
RETURN m.name AS name, m.start_line AS line
311+
ORDER BY name
312+
assert:
313+
# Line 4 is the record declaration: synthetic accessors anchor to
314+
# their component's range.
315+
- { row_count: 2 }
316+
- { row: { name: "x", line: 4 } }
317+
- { row: { name: "y", line: 4 } }
318+
299319
# ── Containment edges ─────────────────────────────────────────────
300320

301321
- name: "Range record defines its canonical constructor"
@@ -428,6 +448,33 @@ tests:
428448
assert:
429449
- { row_count: 1 }
430450

451+
- name: "PointUser.describe() calls implicit accessors Point.x and Point.y"
452+
query: |
453+
MATCH (caller:Definition)-[e:DefinitionToDefinition]->(callee:Definition)
454+
WHERE caller.fqn = 'com.example.records.PointUser.describe'
455+
AND e.edge_kind = 'Calls'
456+
AND callee.name IN ['x', 'y']
457+
RETURN callee.fqn AS callee
458+
ORDER BY callee
459+
assert:
460+
- { row_count: 2 }
461+
- { row: { callee: "com.example.records.Point.x" } }
462+
- { row: { callee: "com.example.records.Point.y" } }
463+
464+
- name: "ShapeUser.area() implicit accessor calls on instanceof-bound variables"
465+
query: |
466+
MATCH (caller:Definition)-[e:DefinitionToDefinition]->(callee:Definition)
467+
WHERE caller.fqn = 'com.example.records.ShapeUser.area'
468+
AND e.edge_kind = 'Calls'
469+
AND callee.name IN ['radius', 'width', 'height']
470+
RETURN DISTINCT callee.fqn AS callee
471+
ORDER BY callee
472+
assert:
473+
- { row_count: 3 }
474+
- { row: { callee: "com.example.records.Circle.radius" } }
475+
- { row: { callee: "com.example.records.Rectangle.height" } }
476+
- { row: { callee: "com.example.records.Rectangle.width" } }
477+
431478
# ── Explicit overrides of implicit members ────────────────────────
432479

433480
- name: "Explicit accessor override is a single Method definition"
@@ -491,18 +538,6 @@ tests:
491538

492539
# ── Gaps (severity: warning — see header comment) ─────────────────
493540

494-
- name: "GAP: Point accessor methods defined"
495-
severity: warning
496-
query: |
497-
MATCH (cls:Definition)-[:DefinitionToDefinition]->(m:Definition)
498-
WHERE cls.fqn = 'com.example.records.Point'
499-
AND m.definition_type = 'Method'
500-
RETURN m.name AS name
501-
ORDER BY name
502-
assert:
503-
- { row: { name: "x" } }
504-
- { row: { name: "y" } }
505-
506541
- name: "GAP: implicit equals/hashCode/toString defined on Point"
507542
severity: warning
508543
query: |
@@ -516,42 +551,6 @@ tests:
516551
- { row: { name: "hashCode" } }
517552
- { row: { name: "toString" } }
518553

519-
- name: "GAP: PointUser.describe() calls Point.x and Point.y accessors"
520-
severity: warning
521-
query: |
522-
MATCH (caller:Definition)-[e:DefinitionToDefinition]->(callee:Definition)
523-
WHERE caller.fqn = 'com.example.records.PointUser.describe'
524-
AND e.edge_kind = 'Calls'
525-
RETURN callee.name AS callee
526-
ORDER BY callee
527-
assert:
528-
- { row: { callee: "x" } }
529-
- { row: { callee: "y" } }
530-
531-
- name: "GAP: ShapeUser.area() accessor calls on instanceof-bound variables"
532-
severity: warning
533-
query: |
534-
MATCH (caller:Definition)-[e:DefinitionToDefinition]->(callee:Definition)
535-
WHERE caller.fqn = 'com.example.records.ShapeUser.area'
536-
AND e.edge_kind = 'Calls'
537-
AND callee.name IN ['radius', 'width', 'height']
538-
RETURN callee.name AS callee
539-
ORDER BY callee
540-
assert:
541-
- { row: { callee: "height" } }
542-
- { row: { callee: "radius" } }
543-
- { row: { callee: "width" } }
544-
545-
- name: "GAP: Bounds compact constructor is extracted"
546-
severity: warning
547-
query: |
548-
MATCH (cls:Definition)-[:DefinitionToDefinition]->(m:Definition)
549-
WHERE cls.fqn = 'com.example.records.Bounds' AND m.name = 'Bounds'
550-
AND m.definition_type IN ['Constructor', 'Method']
551-
RETURN m.fqn AS fqn
552-
assert:
553-
- { row_count: 1 }
554-
555554
- name: "GAP: instanceof record pattern emits Point type reference"
556555
severity: warning
557556
query: |

0 commit comments

Comments
 (0)