Skip to content

Commit 46fd2b0

Browse files
fix(detection): address CTO code review — 5 issues from PR #20
1. ConfigDefDetector: move seenKeys to file scope to prevent duplicate config nodes when multiple classes in the same file reference the same @value or ConfigDef key. 2. ReactComponentDetector: tighten RENDERS filter from allDetected to self-only guard (!tag.equals(comp.name())). Sibling components in the same file now correctly create RENDERS edges (Header → Footer was previously silently dropped). 3. KafkaDetector: extend CLASS_RE to handle Kotlin modifiers (data class, internal class, abstract class, open class, sealed class, object). Previously only matched plain Java-style `class Foo`. 4. ConfigDefDetector: add receiver discriminator to .define() call matching. Only flags calls where the scope references "ConfigDef" or "CONFIG" — prevents false positives from unrelated .define() calls in files that happen to import ConfigDef. 5. TopicLinker: add determinism test covering multi-sender/multi-receiver SENDS_TO/RECEIVES_FROM scenarios (sorted iteration requirement). All 108 targeted Java tests pass (0 failures). Co-Authored-By: Paperclip <noreply@paperclip.ing>
1 parent 8c021b1 commit 46fd2b0

6 files changed

Lines changed: 136 additions & 6 deletions

File tree

src/main/java/io/github/randomcodespace/iq/detector/frontend/ReactComponentDetector.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,8 @@ record ComponentEntry(String name, String sourceId, int matchStart) {}
130130

131131
// RENDERS edges: scope JSX tag search to each component's body section.
132132
// A component's body is from its match position to the next component's position.
133-
Set<String> allDetected = new HashSet<>(componentNames);
134-
allDetected.addAll(hookNames);
135-
133+
// Filter: only exclude self-renders. Sibling components defined in the same file
134+
// are allowed — they represent real inter-component RENDERS relationships.
136135
componentEntries.sort(Comparator.comparingInt(ComponentEntry::matchStart));
137136

138137
for (int i = 0; i < componentEntries.size(); i++) {
@@ -147,7 +146,7 @@ record ComponentEntry(String name, String sourceId, int matchStart) {}
147146
Matcher jsxM = JSX_TAG.matcher(bodyText);
148147
while (jsxM.find()) {
149148
String tag = jsxM.group(1);
150-
if (!allDetected.contains(tag)) {
149+
if (!tag.equals(comp.name())) {
151150
childNames.add(tag);
152151
}
153152
}

src/main/java/io/github/randomcodespace/iq/detector/java/ConfigDefDetector.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,28 @@ public DetectorResult detect(DetectorContext ctx) {
8080
private DetectorResult detectWithAst(CompilationUnit cu, DetectorContext ctx) {
8181
List<CodeNode> nodes = new ArrayList<>();
8282
List<CodeEdge> edges = new ArrayList<>();
83+
// File-scoped seenKeys prevents duplicate config nodes when multiple classes
84+
// in the same file reference the same key (e.g., @Value("${server.port}")).
85+
Set<String> seenKeys = new LinkedHashSet<>();
8386

8487
cu.findAll(ClassOrInterfaceDeclaration.class).forEach(classDecl -> {
8588
String className = classDecl.getNameAsString();
8689
String classNodeId = ctx.filePath() + ":" + className;
87-
Set<String> seenKeys = new LinkedHashSet<>();
8890

8991
// 1. Kafka ConfigDef.define() calls
92+
// Discriminator: receiver must mention "ConfigDef" or "CONFIG" to avoid matching
93+
// unrelated .define() calls (per CLAUDE.md: framework detectors must have guards).
9094
classDecl.findAll(MethodCallExpr.class).forEach(call -> {
9195
if (!"define".equals(call.getNameAsString())) return;
9296
if (call.getArguments().isEmpty()) return;
97+
// Receiver check: scope must reference a ConfigDef instance or chain
98+
boolean receiverIsConfigDef = call.getScope()
99+
.map(scope -> {
100+
String s = scope.toString();
101+
return s.contains("ConfigDef") || s.toUpperCase().contains("CONFIG");
102+
})
103+
.orElse(false);
104+
if (!receiverIsConfigDef) return;
93105
var firstArg = call.getArguments().get(0);
94106
if (!firstArg.isStringLiteralExpr()) return;
95107
String configKey = firstArg.asStringLiteralExpr().getValue();

src/main/java/io/github/randomcodespace/iq/detector/java/KafkaDetector.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@
2929
@Component
3030
public class KafkaDetector extends AbstractRegexDetector {
3131

32-
private static final Pattern CLASS_RE = Pattern.compile("(?:public\\s+)?class\\s+(\\w+)");
32+
// Matches Java: `public class Foo`, `class Foo`
33+
// Matches Kotlin: `class Foo`, `data class Foo`, `abstract class Foo`, `internal class Foo`,
34+
// `open class Foo`, `sealed class Foo`, `object Foo` (Kotlin singleton)
35+
private static final Pattern CLASS_RE = Pattern.compile(
36+
"(?:(?:public|internal|private|protected|data|abstract|open|sealed|enum|inline|value)\\s+)*" +
37+
"(?:class|object)\\s+(\\w+)");
3338
private static final Pattern KAFKA_LISTENER_RE = Pattern.compile(
3439
"@KafkaListener\\s*\\(\\s*(?:.*?topics?\\s*=\\s*)?[\\{\"]?\\s*\"([^\"]+)\"");
3540
private static final Pattern KAFKA_SEND_RE = Pattern.compile(

src/test/java/io/github/randomcodespace/iq/analyzer/linker/TopicLinkerTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,4 +161,38 @@ void handlesQueueNodes() {
161161

162162
assertEquals(1, result.edges().size());
163163
}
164+
165+
@Test
166+
void isDeterministicWithSendToReceivesFrom() {
167+
var topic = new CodeNode("topic:invoices", NodeKind.TOPIC, "invoices");
168+
var sender1 = new CodeNode("svc:InvoiceService", NodeKind.CLASS, "InvoiceService");
169+
var sender2 = new CodeNode("svc:BillingService", NodeKind.CLASS, "BillingService");
170+
var receiver1 = new CodeNode("svc:AuditService", NodeKind.CLASS, "AuditService");
171+
var receiver2 = new CodeNode("svc:ArchiveService", NodeKind.CLASS, "ArchiveService");
172+
173+
var e1 = new CodeEdge(); e1.setId("e1"); e1.setKind(EdgeKind.SENDS_TO);
174+
e1.setSourceId("svc:InvoiceService"); e1.setTarget(topic);
175+
176+
var e2 = new CodeEdge(); e2.setId("e2"); e2.setKind(EdgeKind.SENDS_TO);
177+
e2.setSourceId("svc:BillingService"); e2.setTarget(topic);
178+
179+
var e3 = new CodeEdge(); e3.setId("e3"); e3.setKind(EdgeKind.RECEIVES_FROM);
180+
e3.setSourceId("svc:AuditService"); e3.setTarget(topic);
181+
182+
var e4 = new CodeEdge(); e4.setId("e4"); e4.setKind(EdgeKind.RECEIVES_FROM);
183+
e4.setSourceId("svc:ArchiveService"); e4.setTarget(topic);
184+
185+
List<CodeNode> nodes = List.of(topic, sender1, sender2, receiver1, receiver2);
186+
List<CodeEdge> edges = List.of(e1, e2, e3, e4);
187+
188+
LinkResult r1 = linker.link(nodes, edges);
189+
LinkResult r2 = linker.link(nodes, edges);
190+
191+
assertEquals(r1.edges().size(), r2.edges().size(), "Edge count must be deterministic");
192+
List<String> ids1 = r1.edges().stream().map(CodeEdge::getId).sorted().toList();
193+
List<String> ids2 = r2.edges().stream().map(CodeEdge::getId).sorted().toList();
194+
assertEquals(ids1, ids2, "Edge IDs must be deterministic");
195+
// 2 senders × 2 receivers = 4 CALLS edges
196+
assertEquals(4, r1.edges().size());
197+
}
164198
}

src/test/java/io/github/randomcodespace/iq/detector/frontend/ReactComponentDetectorTest.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,27 @@ void rendersEdgesScopedToCorrectComponent() {
6666
assertFalse(footerRenders.contains("Button"), "Footer should NOT render Button");
6767
}
6868

69+
@Test
70+
void siblingComponentRendersEdgeIsNotDropped() {
71+
// Header renders Footer (both in same file). Previously dropped by allDetected filter.
72+
String source = """
73+
export const Header = () => {
74+
return <div><Footer /></div>;
75+
};
76+
77+
export const Footer = () => {
78+
return <div>footer</div>;
79+
};
80+
""";
81+
DetectorResult r = d.detect(DetectorTestUtils.contextFor("typescript", source));
82+
assertEquals(2, r.nodes().size());
83+
List<String> headerRenders = r.edges().stream()
84+
.filter(e -> e.getKind() == EdgeKind.RENDERS && e.getSourceId().contains("Header"))
85+
.map(e -> e.getTarget().getId())
86+
.toList();
87+
assertTrue(headerRenders.contains("Footer"), "Header should RENDERS Footer (sibling component)");
88+
}
89+
6990
@Test
7091
void singleComponentGetsAllJsxTags() {
7192
String source = """

src/test/java/io/github/randomcodespace/iq/detector/java/JavaDetectorsTest.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,34 @@ fun consume(msg: String) {}
395395
assertTrue(r.edges().stream().anyMatch(e -> e.getKind().getValue().equals("consumes")));
396396
}
397397

398+
@Test
399+
void detectsKotlinDataClassKafkaListener() {
400+
// data class and internal class modifiers must be handled by CLASS_RE
401+
String kotlinSample = """
402+
internal data class OrderConsumer(val kafkaTemplate: Any) {
403+
@KafkaListener(topics = "payments")
404+
fun onPayment(msg: String) {}
405+
}
406+
""";
407+
var r = new KafkaDetector().detect(ctx("kotlin", kotlinSample));
408+
assertFalse(r.nodes().isEmpty());
409+
assertTrue(r.edges().stream().anyMatch(e -> e.getKind().getValue().equals("consumes")));
410+
}
411+
412+
@Test
413+
void detectsKotlinObjectKafkaTemplate() {
414+
// Kotlin object (singleton) is a common pattern for producer utilities
415+
String kotlinSample = """
416+
object EventPublisher {
417+
lateinit var kafkaTemplate: KafkaTemplate<String, String>
418+
fun publish() { kafkaTemplate.send("events", "data") }
419+
}
420+
""";
421+
var r = new KafkaDetector().detect(ctx("kotlin", kotlinSample));
422+
assertFalse(r.nodes().isEmpty());
423+
assertTrue(r.edges().stream().anyMatch(e -> e.getKind().getValue().equals("produces")));
424+
}
425+
398426
@Test
399427
void detectsKotlinKafkaTemplate() {
400428
String kotlinSample = """
@@ -742,6 +770,37 @@ public class Cfg {
742770
""";
743771
DetectorTestUtils.assertDeterministic(new ConfigDefDetector(), ctx("java", sample));
744772
}
773+
774+
@Test
775+
void genericDefineCallDoesNotFalsePositive() {
776+
// A file containing "ConfigDef" text but calling .define() on an unrelated receiver
777+
// must NOT produce config nodes (discriminator guard).
778+
String sample = """
779+
import org.apache.kafka.common.config.ConfigDef;
780+
public class Schema {
781+
// this define() is on a Schema builder, not ConfigDef
782+
void setup() { schema.define("field", Type.STRING); }
783+
}
784+
""";
785+
var r = new ConfigDefDetector().detect(ctx("java", sample));
786+
assertTrue(r.nodes().isEmpty(), "schema.define() should not match — receiver is not ConfigDef");
787+
}
788+
789+
@Test
790+
void kafkaConfigDefWithReceiverMatches() {
791+
String sample = """
792+
import org.apache.kafka.common.config.ConfigDef;
793+
public class MyConfig {
794+
static final ConfigDef CONFIG = new ConfigDef()
795+
.define("batch.size", ConfigDef.Type.INT, 1000)
796+
.define("timeout.ms", ConfigDef.Type.LONG, 5000L);
797+
}
798+
""";
799+
var r = new ConfigDefDetector().detect(ctx("java", sample));
800+
assertEquals(2, r.nodes().size());
801+
assertTrue(r.nodes().stream().anyMatch(n -> n.getLabel().equals("batch.size")));
802+
assertTrue(r.nodes().stream().anyMatch(n -> n.getLabel().equals("timeout.ms")));
803+
}
745804
}
746805

747806
// ==================== ModuleDepsDetector ====================

0 commit comments

Comments
 (0)