From 8c50fa9f0e07bb1a3c4272209590a9def7b1bfc6 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sat, 23 Aug 2025 18:07:57 +0000 Subject: [PATCH] fix: improve Java parsing to handle complete grammar and fix duplication issues - Created comprehensive Java test fixture with complete grammar coverage - Fixed duplication issues in processCaptures function - Improved deduplication logic with priority-based capture selection - Added tests for all Java constructs including interfaces, enums, records, sealed classes - Enhanced query patterns to avoid overlapping captures --- src/debug-full-java-output.txt | 49 --- src/debug-interface-output.txt | 29 -- src/debug-output.txt | 24 -- .../__tests__/debug-full-java.test.ts | 49 --- .../__tests__/debug-interface.test.ts | 81 ---- .../tree-sitter/__tests__/debug-java.test.ts | 71 ---- .../fixtures/sample-java-comprehensive.ts | 351 ++++++++++++++++++ ...CodeDefinitions.java-comprehensive.spec.ts | 254 +++++++++++++ src/services/tree-sitter/index.ts | 139 ++++--- src/services/tree-sitter/queries/java.ts | 19 +- src/test-output.txt | 51 --- 11 files changed, 708 insertions(+), 409 deletions(-) delete mode 100644 src/debug-full-java-output.txt delete mode 100644 src/debug-interface-output.txt delete mode 100644 src/debug-output.txt delete mode 100644 src/services/tree-sitter/__tests__/debug-full-java.test.ts delete mode 100644 src/services/tree-sitter/__tests__/debug-interface.test.ts delete mode 100644 src/services/tree-sitter/__tests__/debug-java.test.ts create mode 100644 src/services/tree-sitter/__tests__/fixtures/sample-java-comprehensive.ts create mode 100644 src/services/tree-sitter/__tests__/parseSourceCodeDefinitions.java-comprehensive.spec.ts delete mode 100644 src/test-output.txt diff --git a/src/debug-full-java-output.txt b/src/debug-full-java-output.txt deleted file mode 100644 index a3c6c1f776..0000000000 --- a/src/debug-full-java-output.txt +++ /dev/null @@ -1,49 +0,0 @@ -# file.java -2--193 | module test.module.definition { -3--7 | module test.module.definition { -8--8 | package test.package.definition; -10--10 | import java.util.List; -11--11 | import java.util.Map; -12--12 | import java.util.function.Function; -13--13 | import java.time.LocalDateTime; -30--44 | public interface TestInterfaceDefinition> { -30--30 | public interface TestInterfaceDefinition> { -47--63 | public enum TestEnumDefinition { -53--53 | private final int level; -53--53 | private final int level; -54--54 | private final String description; -54--54 | private final String description; -56--62 | TestEnumDefinition( -66--119 | value = "test", -71--71 | public class TestClassDefinition> -75--79 | value = "field", -79--79 | private final String prefix; -80--80 | private static int instanceCount = 0; -80--80 | private static int instanceCount = 0; -83--90 | public TestClassDefinition( -93--99 | public void testInterfaceMethod( -102--108 | public > R testGenericMethodDefinition( -102--102 | public > R testGenericMethodDefinition( -111--118 | private final Function testLambdaDefinition = ( -111--118 | private final Function testLambdaDefinition = ( -111--118 | private final Function testLambdaDefinition = ( -122--143 | public record TestRecordDefinition( -135--142 | public String formatMessage() { -146--160 | public abstract class TestAbstractClassDefinition { -146--146 | public abstract class TestAbstractClassDefinition { -147--147 | protected final T data; -147--147 | protected final T data; -149--153 | protected TestAbstractClassDefinition( -156--159 | public abstract String testAbstractMethod( -163--192 | public class TestOuterClassDefinition { -164--164 | private int value; -164--164 | private int value; -166--180 | public class TestInnerClassDefinition { -167--167 | private String innerField; -167--167 | private String innerField; -169--173 | public TestInnerClassDefinition( -175--179 | public void testInnerMethod() { -183--191 | public static class TestStaticNestedClassDefinition { -184--184 | private final String nestedField; -184--184 | private final String nestedField; -186--190 | public TestStaticNestedClassDefinition( diff --git a/src/debug-interface-output.txt b/src/debug-interface-output.txt deleted file mode 100644 index f5768a1018..0000000000 --- a/src/debug-interface-output.txt +++ /dev/null @@ -1,29 +0,0 @@ - -=== FULL PARSE RESULT === -# TestFile.java -2--6 | interface TestInterface { -9--29 | class TestClass implements TestInterface { -11--14 | public void testMethod() { -16--19 | public String getName() { -21--24 | public int calculate(int a, int b) { -26--28 | private void helperMethod() { - -======================== - -=== INDIVIDUAL LINES === -Line 0: # TestFile.java -Line 1: 2--6 | interface TestInterface { -Line 2: 9--29 | class TestClass implements TestInterface { -Line 3: 11--14 | public void testMethod() { -Line 4: 16--19 | public String getName() { -Line 5: 21--24 | public int calculate(int a, int b) { -Line 6: 26--28 | private void helperMethod() { -======================== - -=== INTERFACE LINES === -2--6 | interface TestInterface { -======================== - -=== testMethod LINES === -11--14 | public void testMethod() { -======================== diff --git a/src/debug-output.txt b/src/debug-output.txt deleted file mode 100644 index 08c12dccab..0000000000 --- a/src/debug-output.txt +++ /dev/null @@ -1,24 +0,0 @@ - -=== FULL PARSE RESULT === -# TestClass.java -1--16 | class TestClass implements TestInterface { -3--6 | public void testMethod() { -8--11 | public String getName() { -13--15 | private void helperMethod() { - -======================== - -=== INDIVIDUAL LINES === -Line 0: # TestClass.java -Line 1: 1--16 | class TestClass implements TestInterface { -Line 2: 3--6 | public void testMethod() { -Line 3: 8--11 | public String getName() { -Line 4: 13--15 | private void helperMethod() { -======================== - -=== testMethod LINES === -3--6 | public void testMethod() { -======================== - -=== @Override ONLY LINES === -============================ diff --git a/src/services/tree-sitter/__tests__/debug-full-java.test.ts b/src/services/tree-sitter/__tests__/debug-full-java.test.ts deleted file mode 100644 index 39bd98a49b..0000000000 --- a/src/services/tree-sitter/__tests__/debug-full-java.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { describe, it, expect } from "vitest" -import { testParseSourceCodeDefinitions } from "./helpers" -import { javaQuery } from "../queries" -import sampleJavaContent from "./fixtures/sample-java" -import * as fs from "fs" - -describe("Debug full Java parsing", () => { - it("should show what's being captured for full sample", async () => { - const testOptions = { - language: "java", - wasmFile: "tree-sitter-java.wasm", - queryString: javaQuery, - extKey: "java", - } - - const parseResult = await testParseSourceCodeDefinitions("/test/file.java", sampleJavaContent, testOptions) - - if (parseResult) { - // Write to file - fs.writeFileSync("debug-full-java-output.txt", parseResult) - console.log("Debug output written to debug-full-java-output.txt") - - // Check for specific patterns - const lines = parseResult.split("\n").filter((line) => line.trim()) - - // Check for class with annotations - const classLines = lines.filter((line) => line.includes("TestClassDefinition")) - console.log("\n=== CLASS LINES ===") - classLines.forEach((line) => console.log(line)) - - // Check for annotation declarations - const annotationLines = lines.filter((line) => line.includes("@Target") || line.includes("@TestAnnotation")) - console.log("\n=== ANNOTATION LINES ===") - annotationLines.forEach((line) => console.log(line)) - - // Check for interface methods - const interfaceMethodLines = lines.filter( - (line) => - line.includes("void testInterfaceMethod") || - line.includes("default String testInterfaceDefaultMethod"), - ) - console.log("\n=== INTERFACE METHOD LINES ===") - interfaceMethodLines.forEach((line) => console.log(line)) - } - - // This test is just for debugging, always pass - expect(true).toBe(true) - }) -}) diff --git a/src/services/tree-sitter/__tests__/debug-interface.test.ts b/src/services/tree-sitter/__tests__/debug-interface.test.ts deleted file mode 100644 index aeb2b3607a..0000000000 --- a/src/services/tree-sitter/__tests__/debug-interface.test.ts +++ /dev/null @@ -1,81 +0,0 @@ -import { describe, it, expect } from "vitest" -import { testParseSourceCodeDefinitions } from "./helpers" -import { javaQuery } from "../queries" -import * as fs from "fs" - -describe("Debug Java interface parsing", () => { - it("should show what's being captured for interface", async () => { - const javaContent = `// Test interface with methods -interface TestInterface { - void testMethod(); - String getName(); - int calculate(int a, int b); -} - -// Test class implementing interface with annotations -class TestClass implements TestInterface { - - @Override - public void testMethod() { - // Implementation goes here - } - - @Override - public String getName() { - return "TestClass"; - } - - @Override - public int calculate(int a, int b) { - return a + b; - } - - private void helperMethod() { - // Helper implementation - } -}` - - const testOptions = { - language: "java", - wasmFile: "tree-sitter-java.wasm", - queryString: javaQuery, - extKey: "java", - } - - const parseResult = await testParseSourceCodeDefinitions("/test/TestFile.java", javaContent, testOptions) - - let debugOutput = "" - - if (parseResult) { - debugOutput += "\n=== FULL PARSE RESULT ===\n" - debugOutput += parseResult + "\n" - debugOutput += "========================\n" - - const lines = parseResult.split("\n").filter((line) => line.trim()) - debugOutput += "\n=== INDIVIDUAL LINES ===\n" - lines.forEach((line, i) => { - debugOutput += `Line ${i}: ${line}\n` - }) - debugOutput += "========================\n" - - // Check for interface - const interfaceLines = lines.filter((line) => line.includes("interface TestInterface")) - debugOutput += "\n=== INTERFACE LINES ===\n" - interfaceLines.forEach((line) => (debugOutput += line + "\n")) - debugOutput += "========================\n" - - // Check for testMethod - const methodLines = lines.filter((line) => line.includes("testMethod")) - debugOutput += "\n=== testMethod LINES ===\n" - methodLines.forEach((line) => (debugOutput += line + "\n")) - debugOutput += "========================\n" - - // Write to file - fs.writeFileSync("debug-interface-output.txt", debugOutput) - console.log("Debug output written to debug-interface-output.txt") - } - - // This test is just for debugging, always pass - expect(true).toBe(true) - }) -}) diff --git a/src/services/tree-sitter/__tests__/debug-java.test.ts b/src/services/tree-sitter/__tests__/debug-java.test.ts deleted file mode 100644 index b0dcb6bbad..0000000000 --- a/src/services/tree-sitter/__tests__/debug-java.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { describe, it, expect } from "vitest" -import { testParseSourceCodeDefinitions } from "./helpers" -import { javaQuery } from "../queries" -import * as fs from "fs" - -describe("Debug Java parsing", () => { - it("should show what's being captured", async () => { - const javaContent = `class TestClass implements TestInterface { - - @Override - public void testMethod() { - // Implementation goes here - } - - @Override - public String getName() { - return "TestClass"; - } - - private void helperMethod() { - // Helper implementation - } -}` - - const testOptions = { - language: "java", - wasmFile: "tree-sitter-java.wasm", - queryString: javaQuery, - extKey: "java", - } - - const parseResult = await testParseSourceCodeDefinitions("/test/TestClass.java", javaContent, testOptions) - - let debugOutput = "" - - if (parseResult) { - debugOutput += "\n=== FULL PARSE RESULT ===\n" - debugOutput += parseResult + "\n" - debugOutput += "========================\n" - - const lines = parseResult.split("\n").filter((line) => line.trim()) - debugOutput += "\n=== INDIVIDUAL LINES ===\n" - lines.forEach((line, i) => { - debugOutput += `Line ${i}: ${line}\n` - }) - debugOutput += "========================\n" - - // Check for duplicates - const methodLines = lines.filter((line) => line.includes("testMethod")) - debugOutput += "\n=== testMethod LINES ===\n" - methodLines.forEach((line) => (debugOutput += line + "\n")) - debugOutput += "========================\n" - - // Check for @Override lines - const overrideLines = lines.filter((line) => { - const content = line.split("|")[1]?.trim() || "" - return content === "@Override" - }) - debugOutput += "\n=== @Override ONLY LINES ===\n" - overrideLines.forEach((line) => (debugOutput += line + "\n")) - debugOutput += "============================\n" - - // Write to file - fs.writeFileSync("debug-output.txt", debugOutput) - console.log("Debug output written to debug-output.txt") - } - - // This test is just for debugging, always pass - expect(true).toBe(true) - }) -}) diff --git a/src/services/tree-sitter/__tests__/fixtures/sample-java-comprehensive.ts b/src/services/tree-sitter/__tests__/fixtures/sample-java-comprehensive.ts new file mode 100644 index 0000000000..34b7c8a926 --- /dev/null +++ b/src/services/tree-sitter/__tests__/fixtures/sample-java-comprehensive.ts @@ -0,0 +1,351 @@ +export default String.raw` +// Package declaration +package com.example.comprehensive; + +// Import statements +import java.util.*; +import java.util.concurrent.CompletableFuture; +import java.util.function.Function; +import java.io.IOException; +import java.lang.annotation.*; +import static java.lang.Math.PI; +import static java.util.Collections.*; + +// Single-line comment +/* Multi-line comment + spanning multiple lines */ + +/** + * JavaDoc comment for annotation + * @since 1.0 + */ +@Target({ElementType.TYPE, ElementType.METHOD, ElementType.FIELD}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Inherited +public @interface CustomAnnotation { + String value() default ""; + int priority() default 0; + Class[] types() default {}; + String[] tags() default {}; +} + +/** + * Interface with various method types + */ +public interface GenericInterface, U> { + // Abstract method + void abstractMethod(T param); + + // Default method with implementation + default U defaultMethod(T input) { + return processInput(input); + } + + // Static method in interface + static V staticInterfaceMethod(V value) { + return value; + } + + // Private method in interface (Java 9+) + private U processInput(T input) { + return null; + } +} + +/** + * Abstract class with various members + */ +public abstract class AbstractBase implements GenericInterface { + // Protected field + protected T data; + + // Static field + private static final String CONSTANT = "CONST_VALUE"; + + // Constructor + protected AbstractBase(T data) { + this.data = data; + } + + // Abstract method + public abstract T process(T input); + + // Concrete method + public final String getName() { + return this.getClass().getSimpleName(); + } +} + +/** + * Enum with constructor and methods + */ +public enum Status { + PENDING(0, "Pending"), + ACTIVE(1, "Active") { + @Override + public String getDescription() { + return "Currently active: " + description; + } + }, + COMPLETED(2, "Completed"), + FAILED(-1, "Failed"); + + private final int code; + protected final String description; + + Status(int code, String description) { + this.code = code; + this.description = description; + } + + public String getDescription() { + return description; + } +} + +/** + * Main class with comprehensive Java features + */ +@CustomAnnotation(value = "MainClass", priority = 1, types = {String.class, Integer.class}) +@SuppressWarnings("unchecked") +public class ComprehensiveExample> + extends AbstractBase + implements Serializable, Cloneable { + + // Serial version UID + private static final long serialVersionUID = 1L; + + // Various field types + private volatile int counter; + private transient String tempData; + public static final double PI_VALUE = 3.14159; + private final List items = new ArrayList<>(); + + // Static initializer block + static { + System.out.println("Static initializer"); + } + + // Instance initializer block + { + counter = 0; + tempData = "temp"; + } + + // Constructor with annotations + @SuppressWarnings("deprecation") + public ComprehensiveExample(@NonNull T initialData) { + super(initialData); + } + + // Overloaded constructor + public ComprehensiveExample(T data, int counter) { + this(data); + this.counter = counter; + } + + // Method with generic return type and throws clause + @Override + public T process(T input) throws IllegalArgumentException { + if (input == null) { + throw new IllegalArgumentException("Input cannot be null"); + } + return input; + } + + // Synchronized method + public synchronized void incrementCounter() { + counter++; + } + + // Method with varargs + public void processMultiple(T... items) { + for (T item : items) { + this.items.add(item); + } + } + + // Generic method with bounds + public > U genericMethod(U value) { + return value; + } + + // Method with array parameter + public static void arrayMethod(String[] args, int[][] matrix) { + System.out.println(Arrays.toString(args)); + } + + // Inner class + public class InnerClass { + private String innerField; + + public InnerClass(String field) { + this.innerField = field; + } + + public void accessOuter() { + System.out.println(ComprehensiveExample.this.counter); + } + } + + // Static nested class + public static class StaticNestedClass { + private static int nestedCounter; + + public StaticNestedClass() { + nestedCounter++; + } + + public static int getCounter() { + return nestedCounter; + } + } + + // Local class inside method + public void methodWithLocalClass() { + class LocalClass { + private String localField; + + public LocalClass(String field) { + this.localField = field; + } + + public void printLocal() { + System.out.println(localField); + } + } + + LocalClass local = new LocalClass("local"); + local.printLocal(); + } + + // Anonymous class + public Runnable createRunnable() { + return new Runnable() { + @Override + public void run() { + System.out.println("Anonymous class"); + } + }; + } + + // Lambda expressions + public void lambdaExamples() { + // Simple lambda + Runnable r1 = () -> System.out.println("Lambda"); + + // Lambda with parameters + Function f1 = s -> s.length(); + + // Lambda with block + Function f2 = (Integer i) -> { + String result = "Number: " + i; + return result; + }; + + // Method reference + Function f3 = String::length; + } + + // Try-with-resources + public void tryWithResources() throws IOException { + try (var resource = new AutoCloseable() { + @Override + public void close() throws Exception { + System.out.println("Closing"); + } + }) { + // Use resource + } catch (Exception e) { + e.printStackTrace(); + } finally { + System.out.println("Finally"); + } + } + + // Switch expression (Java 14+) + public String switchExpression(Status status) { + return switch (status) { + case PENDING -> "Waiting"; + case ACTIVE -> "Running"; + case COMPLETED -> "Done"; + case FAILED -> { + System.out.println("Failed status"); + yield "Error"; + } + }; + } +} + +/** + * Record class (Java 14+) + */ +public record PersonRecord( + String name, + int age, + List hobbies +) { + // Compact constructor + public PersonRecord { + Objects.requireNonNull(name); + if (age < 0) { + throw new IllegalArgumentException("Age cannot be negative"); + } + } + + // Additional method + public String getInfo() { + return String.format("%s (%d years)", name, age); + } + + // Static factory method + public static PersonRecord of(String name, int age) { + return new PersonRecord(name, age, new ArrayList<>()); + } +} + +/** + * Sealed class (Java 17+) + */ +public sealed class Shape + permits Circle, Rectangle, Triangle { + + protected final double area; + + protected Shape(double area) { + this.area = area; + } + + public double getArea() { + return area; + } +} + +// Permitted subclasses +final class Circle extends Shape { + private final double radius; + + public Circle(double radius) { + super(Math.PI * radius * radius); + this.radius = radius; + } +} + +final class Rectangle extends Shape { + private final double width; + private final double height; + + public Rectangle(double width, double height) { + super(width * height); + this.width = width; + this.height = height; + } +} + +non-sealed class Triangle extends Shape { + public Triangle(double base, double height) { + super(0.5 * base * height); + } +} +` diff --git a/src/services/tree-sitter/__tests__/parseSourceCodeDefinitions.java-comprehensive.spec.ts b/src/services/tree-sitter/__tests__/parseSourceCodeDefinitions.java-comprehensive.spec.ts new file mode 100644 index 0000000000..1537b7efce --- /dev/null +++ b/src/services/tree-sitter/__tests__/parseSourceCodeDefinitions.java-comprehensive.spec.ts @@ -0,0 +1,254 @@ +import { describe, it, expect, beforeAll } from "vitest" +import { testParseSourceCodeDefinitions } from "./helpers" +import { javaQuery } from "../queries" +import sampleJavaComprehensiveContent from "./fixtures/sample-java-comprehensive" + +describe("Java parsing - comprehensive grammar test", () => { + let parseResult: string = "" + let lines: string[] = [] + + beforeAll(async () => { + const testOptions = { + language: "java", + wasmFile: "tree-sitter-java.wasm", + queryString: javaQuery, + extKey: "java", + } + + const result = await testParseSourceCodeDefinitions( + "/test/ComprehensiveExample.java", + sampleJavaComprehensiveContent, + testOptions, + ) + if (!result) { + throw new Error("Failed to parse Java source code") + } + parseResult = result + lines = parseResult.split("\n").filter((line) => line.trim()) + + // Debug output + console.log("\n=== COMPREHENSIVE PARSE RESULT ===") + console.log(parseResult) + console.log("==================================\n") + }) + + describe("No duplications", () => { + it("should not have duplicate class declarations", () => { + const classDeclarations = lines.filter( + (line) => + line.includes("class ComprehensiveExample") || + line.includes("class AbstractBase") || + line.includes("class Shape"), + ) + + // Check each class appears only once + const comprehensiveLines = classDeclarations.filter((line) => line.includes("ComprehensiveExample")) + const abstractLines = classDeclarations.filter((line) => line.includes("AbstractBase")) + const shapeLines = classDeclarations.filter((line) => line.includes("Shape")) + + expect(comprehensiveLines.length).toBeLessThanOrEqual(1) + expect(abstractLines.length).toBeLessThanOrEqual(1) + expect(shapeLines.length).toBeLessThanOrEqual(1) + }) + + it("should not have duplicate interface declarations", () => { + const interfaceLines = lines.filter((line) => line.includes("interface GenericInterface")) + expect(interfaceLines.length).toBeLessThanOrEqual(1) + }) + + it("should not have duplicate method declarations", () => { + // Check specific methods don't appear multiple times + const processLines = lines.filter((line) => line.includes("process(")) + const incrementLines = lines.filter((line) => line.includes("incrementCounter")) + const genericMethodLines = lines.filter((line) => line.includes("genericMethod")) + + // Each method should appear at most once + expect(processLines.length).toBeLessThanOrEqual(1) + expect(incrementLines.length).toBeLessThanOrEqual(1) + expect(genericMethodLines.length).toBeLessThanOrEqual(1) + }) + + it("should not show @Override as standalone definition", () => { + const overrideOnlyLines = lines.filter((line) => { + const content = line.split("|")[1]?.trim() || "" + return content === "@Override" + }) + expect(overrideOnlyLines.length).toBe(0) + }) + }) + + describe("Package and imports", () => { + it("should parse package declaration", () => { + const packageLine = lines.find((line) => line.includes("package com.example.comprehensive")) + expect(packageLine).toBeDefined() + }) + }) + + describe("Annotations", () => { + it("should parse annotation declarations", () => { + const annotationLine = lines.find((line) => line.includes("@interface CustomAnnotation")) + expect(annotationLine).toBeDefined() + }) + + it("should show annotated class with class declaration, not annotation", () => { + const classLine = lines.find((line) => line.includes("class ComprehensiveExample")) + expect(classLine).toBeDefined() + expect(classLine).toContain("class ComprehensiveExample") + expect(classLine).not.toContain("@CustomAnnotation") + }) + }) + + describe("Interfaces", () => { + it("should parse interface declaration", () => { + const interfaceLine = lines.find((line) => line.includes("interface GenericInterface")) + expect(interfaceLine).toBeDefined() + }) + + it("should not duplicate interface methods", () => { + // Interface methods should be part of interface declaration, not separate + const abstractMethodLines = lines.filter((line) => line.includes("void abstractMethod")) + const defaultMethodLines = lines.filter((line) => line.includes("defaultMethod")) + + expect(abstractMethodLines.length).toBeLessThanOrEqual(1) + expect(defaultMethodLines.length).toBeLessThanOrEqual(1) + }) + }) + + describe("Classes", () => { + it("should parse abstract class", () => { + const abstractLine = lines.find((line) => line.includes("abstract class AbstractBase")) + expect(abstractLine).toBeDefined() + }) + + it("should parse main class", () => { + const mainClassLine = lines.find((line) => line.includes("class ComprehensiveExample")) + expect(mainClassLine).toBeDefined() + }) + + it("should parse sealed class", () => { + const sealedLine = lines.find((line) => line.includes("sealed class Shape")) + expect(sealedLine).toBeDefined() + }) + + it("should parse final classes", () => { + const circleLine = lines.find((line) => line.includes("class Circle")) + const rectangleLine = lines.find((line) => line.includes("class Rectangle")) + expect(circleLine).toBeDefined() + expect(rectangleLine).toBeDefined() + }) + }) + + describe("Enums", () => { + it("should parse enum declaration", () => { + const enumLine = lines.find((line) => line.includes("enum Status")) + expect(enumLine).toBeDefined() + }) + }) + + describe("Records", () => { + it("should parse record declaration", () => { + const recordLine = lines.find((line) => line.includes("record PersonRecord")) + expect(recordLine).toBeDefined() + }) + }) + + describe("Inner classes", () => { + it("should parse inner class", () => { + const innerLine = lines.find((line) => line.includes("class InnerClass")) + expect(innerLine).toBeDefined() + }) + + it("should parse static nested class", () => { + const nestedLine = lines.find((line) => line.includes("class StaticNestedClass")) + expect(nestedLine).toBeDefined() + }) + }) + + describe("Methods", () => { + it("should parse overridden methods with correct signature", () => { + const processMethod = lines.find((line) => line.includes("process(T input)")) + expect(processMethod).toBeDefined() + if (processMethod) { + expect(processMethod).toContain("process") + expect(processMethod).not.toContain("@Override") + } + }) + + it("should parse synchronized methods", () => { + const syncMethod = lines.find((line) => line.includes("incrementCounter")) + expect(syncMethod).toBeDefined() + }) + + it("should parse generic methods", () => { + const genericMethod = lines.find((line) => line.includes("genericMethod")) + expect(genericMethod).toBeDefined() + }) + + it("should parse varargs methods", () => { + const varargMethod = lines.find((line) => line.includes("processMultiple")) + expect(varargMethod).toBeDefined() + }) + + it("should parse static methods", () => { + const staticMethod = lines.find((line) => line.includes("arrayMethod")) + expect(staticMethod).toBeDefined() + }) + }) + + describe("Constructors", () => { + it("should parse constructors", () => { + const constructorLines = lines.filter( + (line) => + line.includes("ComprehensiveExample(") || + line.includes("AbstractBase(") || + line.includes("Circle(") || + line.includes("Rectangle("), + ) + expect(constructorLines.length).toBeGreaterThan(0) + }) + }) + + describe("Lambda expressions", () => { + it("should parse lambda expressions", () => { + const lambdaLines = lines.filter((line) => line.includes("->")) + // Should find at least some lambda expressions + expect(lambdaLines.length).toBeGreaterThan(0) + }) + }) + + describe("Line ranges", () => { + it("should have correct line ranges for multi-line definitions", () => { + lines.forEach((line) => { + const match = line.match(/(\d+)--(\d+)/) + if (match) { + const startLine = parseInt(match[1]) + const endLine = parseInt(match[2]) + + // Multi-line definitions should have different start and end + if (endLine - startLine >= 3) { + // This is a multi-line definition (4+ lines) + expect(endLine).toBeGreaterThan(startLine) + } + } + }) + }) + }) + + describe("Output format", () => { + it("should format output correctly", () => { + lines.forEach((line) => { + // Each line should have the format: "startLine--endLine | content" + expect(line).toMatch(/^\d+--\d+ \| .+/) + }) + }) + + it("should not include comment-only lines as definitions", () => { + const commentOnlyLines = lines.filter((line) => { + const content = line.split("|")[1]?.trim() || "" + return content.startsWith("//") || content.startsWith("/*") || content.startsWith("*") + }) + // Comments should not be standalone definitions + expect(commentOnlyLines.length).toBe(0) + }) + }) +}) diff --git a/src/services/tree-sitter/index.ts b/src/services/tree-sitter/index.ts index 9aa51ec496..bc13dae84a 100644 --- a/src/services/tree-sitter/index.ts +++ b/src/services/tree-sitter/index.ts @@ -289,7 +289,11 @@ function processCaptures(captures: QueryCapture[], lines: string[], language: st captures.sort((a, b) => a.node.startPosition.row - b.node.startPosition.row) // Track already processed definitions to avoid duplicates - const processedDefinitions = new Map() + // Use a more comprehensive key that includes the actual content to better detect duplicates + const processedDefinitions = new Map< + string, + { startLine: number; endLine: number; displayLine: number; priority: number } + >() // Process captures and group by definition type and location captures.forEach((capture) => { @@ -302,40 +306,72 @@ function processCaptures(captures: QueryCapture[], lines: string[], language: st // For Java, skip certain captures to avoid duplication if (language === "java") { - // Skip class body definitions as they duplicate the class declaration - if (name === "definition.class" && node.parent?.type === "class_body") { - return - } - // Skip standalone annotation type declarations - if (name.includes("definition.annotation")) { - return - } // Skip comment definitions if (name === "definition.comment") { return } + // Skip individual interface method definitions to avoid duplication // The interface declaration already shows the interface with its methods - // Check various parent levels as the structure might vary const parent = node.parent const grandParent = parent?.parent const greatGrandParent = grandParent?.parent - if (name === "definition.method" || name === "name.definition.method") { - // Check if this method is inside an interface + // Check if this is a method inside an interface body + if (name.includes("method")) { if ( parent?.type === "interface_body" || grandParent?.type === "interface_body" || - greatGrandParent?.type === "interface_body" || - (parent?.type === "method_declaration" && grandParent?.type === "interface_body") + greatGrandParent?.type === "interface_body" ) { + // Skip interface methods as they're part of the interface declaration return } } + + // Handle overlapping class captures + // Skip general class definitions if we have more specific inner/nested class captures + if (name === "definition.class" || name === "name.definition.class") { + const nodeStartRow = node.startPosition.row + + // Check if this class is inside another class body (making it an inner/nested class) + let currentParent = parent + while (currentParent) { + if (currentParent.type === "class_body") { + // This is a nested class, check if we have a more specific capture + const hasSpecificCapture = captures.some( + (c) => + (c.name === "definition.inner_class" || + c.name === "name.definition.inner_class" || + c.name === "definition.static_nested_class" || + c.name === "name.definition.static_nested_class") && + Math.abs(c.node.startPosition.row - nodeStartRow) <= 1, + ) + if (hasSpecificCapture) { + return // Skip this general capture in favor of the specific one + } + break + } + currentParent = currentParent.parent + } + } + + // Skip duplicate inner/static nested class captures + // Keep only the most specific one + if (name === "definition.inner_class" || name === "definition.static_nested_class") { + const nodeStartRow = node.startPosition.row + // Check if we already have a class definition at this location + const hasGeneralClass = captures.some( + (c) => + (c.name === "definition.class" || c.name === "name.definition.class") && + Math.abs(c.node.startPosition.row - nodeStartRow) <= 1, + ) + // If we have both, we'll keep this specific one and the general one will be skipped above + } } // Get the parent node that contains the full definition - const definitionNode = name.includes("name") ? node.parent : node + const definitionNode = name.includes("name") && node.parent ? node.parent : node if (!definitionNode) return // Get the start and end lines of the full definition @@ -352,46 +388,59 @@ function processCaptures(captures: QueryCapture[], lines: string[], language: st let displayLine = startLine if (language === "java") { // For methods, classes, interfaces, etc. with annotations, find the actual declaration line - if (name.includes("definition")) { - for (let i = startLine; i <= endLine; i++) { - const line = lines[i].trim() - // Skip empty lines, annotations, and comments - if ( - line && - !line.startsWith("@") && - !line.startsWith("//") && - !line.startsWith("/*") && - !line.startsWith("*") - ) { - displayLine = i - break - } + for (let i = startLine; i <= endLine; i++) { + const line = lines[i]?.trim() || "" + // Skip empty lines, annotations, and comments + if ( + line && + !line.startsWith("@") && + !line.startsWith("//") && + !line.startsWith("/*") && + !line.startsWith("*") + ) { + displayLine = i + break } } } - // Create a unique key for this definition - const defKey = `${definitionNode.type}-${startLine}-${endLine}` - - // Check if we've already processed a definition at this location - const existing = processedDefinitions.get(defKey) - if (existing) { - // For Java, prefer method definitions over other types at the same location - if (language === "java" && name === "definition.method" && !existing.displayLine) { - // Update with the method definition which has the correct display line - processedDefinitions.set(defKey, { startLine, endLine, displayLine }) - } - return - } - // Check if this is a valid component definition (not an HTML element) - const displayLineContent = lines[displayLine].trim() + const displayLineContent = lines[displayLine]?.trim() || "" if (!isNotHtmlElement(displayLineContent)) { return } + // Create a unique key for this definition based on location and content + // This helps prevent duplicates when the same definition is captured multiple times + const defKey = `${startLine}-${endLine}-${displayLineContent.substring(0, 50)}` + + // Assign priority based on capture type (more specific captures have higher priority) + let priority = 0 + if (name.includes("inner_class") || name.includes("static_nested_class")) { + priority = 3 + } else if (name.includes("method") || name.includes("constructor")) { + priority = 2 + } else if ( + name.includes("class") || + name.includes("interface") || + name.includes("enum") || + name.includes("record") + ) { + priority = 1 + } + + // Check if we've already processed a definition at this location + const existing = processedDefinitions.get(defKey) + if (existing) { + // Keep the capture with higher priority (more specific) + if (priority > existing.priority) { + processedDefinitions.set(defKey, { startLine, endLine, displayLine, priority }) + } + return + } + // Store this definition - processedDefinitions.set(defKey, { startLine, endLine, displayLine }) + processedDefinitions.set(defKey, { startLine, endLine, displayLine, priority }) }) // Generate output from processed definitions diff --git a/src/services/tree-sitter/queries/java.ts b/src/services/tree-sitter/queries/java.ts index 01b007dee1..cba88a7eb7 100644 --- a/src/services/tree-sitter/queries/java.ts +++ b/src/services/tree-sitter/queries/java.ts @@ -42,17 +42,16 @@ export default ` (method_declaration name: (identifier) @name.definition.method) @definition.method -; Inner class declarations -(class_declaration - (class_body - (class_declaration - name: (identifier) @name.definition.inner_class))) @definition.inner_class +; Inner class declarations (inside class body) +(class_body + (class_declaration + name: (identifier) @name.definition.inner_class)) @definition.inner_class -; Static nested class declarations -(class_declaration - (class_body - (class_declaration - name: (identifier) @name.definition.static_nested_class))) @definition.static_nested_class +; Static nested class declarations (with static modifier) +(class_body + (class_declaration + (modifiers "static") + name: (identifier) @name.definition.static_nested_class)) @definition.static_nested_class ; Lambda expressions (lambda_expression) @definition.lambda diff --git a/src/test-output.txt b/src/test-output.txt deleted file mode 100644 index eec4cf4e3a..0000000000 --- a/src/test-output.txt +++ /dev/null @@ -1,51 +0,0 @@ - - RUN v3.2.4 /data/repos/Roo-Code/src - -··xx· - -⎯⎯⎯⎯⎯⎯⎯ Failed Tests 2 ⎯⎯⎯⎯⎯⎯⎯ - - FAIL services/tree-sitter/__tests__/parseSourceCodeDefinitions.java-simple.spec.ts > Java parsing - duplication issue > should parse each method without duplication -AssertionError: expected 2 to be 1 // Object.is equality - -- Expected -+ Received - -- 1 -+ 2 - - ❯ services/tree-sitter/__tests__/parseSourceCodeDefinitions.java-simple.spec.ts:63:34 - 61| const testMethodLines = lines.filter(line => line.includes("testMeth… - 62| console.log("testMethod lines found:", testMethodLines) - 63| expect(testMethodLines.length).toBe(1) - | ^ - 64| - 65| // Check getName - -⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/2]⎯ - - FAIL services/tree-sitter/__tests__/parseSourceCodeDefinitions.java-simple.spec.ts > Java parsing - duplication issue > should show method signatures, not annotations -AssertionError: expected 3 to be +0 // Object.is equality - -- Expected -+ Received - -- 0 -+ 3 - - ❯ services/tree-sitter/__tests__/parseSourceCodeDefinitions.java-simple.spec.ts:92:36 - 90| - 91| // Should not have any lines with just @Override - 92| expect(overrideOnlyLines.length).toBe(0) - | ^ - 93| }) - 94| - -⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/2]⎯ - - - Test Files 1 failed (1) - Tests 2 failed | 3 passed (5) - Start at 17:39:51 - Duration 870ms (transform 285ms, setup 98ms, collect 333ms, tests 83ms, environment 0ms, prepare 92ms) -