-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: correctly handle Java methods with @Override annotations in listCodeDefinitionNames #7331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
7f0f2c8
0b1b51c
24ab961
8c50fa9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import { describe, it, expect } from "vitest" | ||
| import { testParseSourceCodeDefinitions } from "./helpers" | ||
| import { javaQuery } from "../queries" | ||
|
|
||
| describe("Simple Java @Override test", () => { | ||
| it("should show what gets captured for @Override methods", async () => { | ||
| const overrideTestContent = `class TestClass { | ||
| @Override | ||
| public void testMethod() { | ||
| // Implementation goes here | ||
| } | ||
| }` | ||
|
|
||
| const testOptions = { | ||
| language: "java", | ||
| wasmFile: "tree-sitter-java.wasm", | ||
| queryString: javaQuery, | ||
| extKey: "java", | ||
| } | ||
|
|
||
| const parseResult = await testParseSourceCodeDefinitions("/test/file.java", overrideTestContent, testOptions) | ||
|
|
||
| console.log("\n=== PARSE RESULT ===") | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should these console.log statements be removed or converted to debug-level logging? They're helpful for debugging but might clutter test output in CI/CD pipelines. |
||
| console.log(parseResult) | ||
| console.log("====================\n") | ||
|
|
||
| if (parseResult) { | ||
| const lines = parseResult.split("\n").filter((line) => line.trim()) | ||
| console.log("\n=== INDIVIDUAL LINES ===") | ||
| lines.forEach((line, i) => { | ||
| console.log(`Line ${i}: ${line}`) | ||
| }) | ||
| console.log("========================\n") | ||
|
|
||
| // Check for the issue | ||
| const hasOverrideLine = lines.some((line) => line.includes("@Override") && !line.includes("testMethod")) | ||
|
|
||
| if (hasOverrideLine) { | ||
| console.log("❌ BUG CONFIRMED: @Override is shown without the method name") | ||
| const problematicLines = lines.filter( | ||
| (line) => line.includes("@Override") && !line.includes("testMethod"), | ||
| ) | ||
| console.log("Problematic lines:", problematicLines) | ||
| } else { | ||
| console.log("✅ No issue found - @Override appears with method name") | ||
| } | ||
|
|
||
| // This test will fail if the bug exists | ||
| expect(hasOverrideLine).toBe(false) | ||
| } | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -339,8 +339,42 @@ function processCaptures(captures: QueryCapture[], lines: string[], language: st | |
| } | ||
| // For other component definitions | ||
| else if (isNotHtmlElement(startLineContent)) { | ||
| formattedOutput += `${startLine + 1}--${endLine + 1} | ${lines[startLine]}\n` | ||
| processedLines.add(lineKey) | ||
| // For Java, special handling to avoid showing @Override as a separate line | ||
| // when it's part of a method declaration | ||
| if (language === "java" && name === "definition.method") { | ||
| // Check if the method has an annotation like @Override | ||
| const methodText = definitionNode.text | ||
| if (methodText.includes("@Override")) { | ||
| // Find the actual method declaration line (not the annotation line) | ||
| let methodDeclarationLine = startLine | ||
| for (let i = startLine; i <= endLine; i++) { | ||
| if ( | ||
|
||
| lines[i].includes("public") || | ||
|
||
| lines[i].includes("private") || | ||
| lines[i].includes("protected") || | ||
| lines[i].includes("void") || | ||
| lines[i].includes("static") | ||
| ) { | ||
| methodDeclarationLine = i | ||
| break | ||
| } | ||
| } | ||
| // Output the method with its proper line range, but show the method declaration line | ||
| formattedOutput += `${startLine + 1}--${endLine + 1} | ${lines[methodDeclarationLine]}\n` | ||
|
||
| processedLines.add(lineKey) | ||
| } else { | ||
| // Normal method without annotations | ||
| formattedOutput += `${startLine + 1}--${endLine + 1} | ${lines[startLine]}\n` | ||
| processedLines.add(lineKey) | ||
| } | ||
| } else if (language === "java" && name === "definition.class") { | ||
| // For Java classes, skip the entire class definition to avoid duplication | ||
| // The class name will be handled by name.definition.class | ||
| return | ||
| } else { | ||
| formattedOutput += `${startLine + 1}--${endLine + 1} | ${lines[startLine]}\n` | ||
| processedLines.add(lineKey) | ||
| } | ||
|
|
||
| // If this is part of a larger definition, include its non-HTML context | ||
| if (node.parent && node.parent.lastChild) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test description says 'should show what gets captured for @OverRide methods' but it's actually testing that @OverRide is NOT shown without the method name. Would it be clearer to rename this to something like 'should correctly display method names with @OverRide annotations'?