Skip to content

🐛 Fix annotation matching #149

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.

Sign up for GitHub

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

Merged
jmle merged 13 commits into from
Oct 21, 2025
Merged

🐛 Fix annotation matching #149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6102a1e
tmp
jmle Oct 1, 2025
6b645cf
Working:
jmle Oct 1, 2025
d0da87c
Move in check for annotated classes
jmle Oct 1, 2025
cafa848
tmp
jmle Oct 2, 2025
1ab427a
Fix stuff
jmle Oct 2, 2025
a05bf5f
Final fix
jmle Oct 3, 2025
4e1f545
Fix shouldVisit
jmle Oct 3, 2025
d15171d
Fix condition
jmle Oct 3, 2025
9fca62b
Undo
jmle Oct 6, 2025
4ec47b6
make sure that qualification matches considers fqn usage without impo…
pranavgaikwad Oct 14, 2025
3c2c2d1
fix a minor bug in processing regex
pranavgaikwad Oct 14, 2025
d1b35c4
Fix regex
jmle Oct 20, 2025
daf8d09
Fix potential npe
jmle Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ...dle.core/src/main/java/io/konveyor/tackle/core/internal/SampleDelegateCommandHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ private static SearchPattern mapLocationToSearchPatternLocation(int location, St
*/
private static SearchPattern getPatternSingleQuery(int location, String query) throws Exception {
var pattern = SearchPattern.R_PATTERN_MATCH;
if ((!query.contains("?") || !query.contains("*")) && (location != 11)) {
if ((!query.contains("?") && !query.contains("*")) && (location != 11)) {
logInfo("Using full match");
pattern = SearchPattern.R_EXACT_MATCH | SearchPattern.R_CASE_SENSITIVE;
}
Expand Down Expand Up @@ -376,7 +376,8 @@ private static List<SymbolInformation> search(String projectName, ArrayList<Stri

// Now run ImportScanner only on units in scope
ASTParser parser = ASTParser.newParser(AST.getJLSLatest());
Pattern regex = Pattern.compile(query);
// when creating the regex, replace * with .*
Pattern regex = Pattern.compile(query.replaceAll("(?<!\\.)\\*", ".*"));
for (ICompilationUnit unit : units) {
parser.setSource(unit);
CompilationUnit cu = (CompilationUnit) parser.createAST(null);
Expand Down
97 changes: 82 additions & 15 deletions ....core/src/main/java/io/konveyor/tackle/core/internal/symbol/AnnotationSymbolProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,45 +6,107 @@
import java.util.List;

import io.konveyor.tackle.core.internal.query.AnnotationQuery;
import io.konveyor.tackle.core.internal.symbol.CustomASTVisitor.QueryLocation;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.jdt.core.IAnnotatable;
import org.eclipse.jdt.core.IAnnotation;
import org.eclipse.jdt.core.IClassFile;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.core.dom.AST;
import org.eclipse.jdt.core.dom.ASTParser;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.search.SearchMatch;
import org.eclipse.jdt.internal.core.ResolvedSourceField;
import org.eclipse.jdt.internal.core.ResolvedSourceMethod;
import org.eclipse.jdt.internal.core.ResolvedSourceType;
import org.eclipse.jdt.internal.core.SourceRefElement;
import org.eclipse.lsp4j.Location;
import org.eclipse.lsp4j.SymbolInformation;

public class AnnotationSymbolProvider implements SymbolProvider, WithAnnotationQuery {
public class AnnotationSymbolProvider implements SymbolProvider, WithQuery, WithAnnotationQuery {

private AnnotationQuery annotationQuery;
private String query;

@Override
public List<SymbolInformation> get(SearchMatch match) throws CoreException {
List<SymbolInformation> symbols = new ArrayList<>();
try {
IAnnotatable mod = (IAnnotatable) match.getElement();
IJavaElement element = (IJavaElement) match.getElement();
for (IAnnotation annotation : mod.getAnnotations()) {
IAnnotatable annotatable = (IAnnotatable) match.getElement();
for (IAnnotation annotation : annotatable.getAnnotations()) {
IJavaElement annotationElement = annotation.getPrimaryElement();
SymbolInformation symbol = new SymbolInformation();
symbol.setName(annotation.getElementName());
symbol.setKind(convertSymbolKind(element));
symbol.setKind(convertSymbolKind(annotationElement));
symbol.setContainerName(annotation.getParent().getElementName());
symbol.setLocation(getLocation(element, match));

if (annotationQuery != null) {
List<Class<? extends SourceRefElement>> classes = new ArrayList<>();
classes.add(ResolvedSourceMethod.class);
classes.add(ResolvedSourceField.class);
classes.add(ResolvedSourceType.class);
if (matchesAnnotationQuery(match, classes)) {
symbols.add(symbol);
Location location = getLocation(annotationElement, match);
symbol.setLocation(location);
if (this.query.contains(".")) {
// First try to get compilation unit for source files
ICompilationUnit unit = (ICompilationUnit) annotationElement.getAncestor(IJavaElement.COMPILATION_UNIT);
if (unit == null) {
// If not in source, try to get class file for compiled classes
IClassFile cls = (IClassFile) annotationElement.getAncestor(IJavaElement.CLASS_FILE);
if (cls != null) {
unit = cls.getWorkingCopy(new WorkingCopyOwnerImpl(), null);
}
}
if (this.queryQualificationMatches(this.query.replaceAll("\\(([A-Za-z_][A-Za-z0-9_]*(\\|[A-Za-z_][A-Za-z0-9_]*)*)\\)", ".*"), annotationElement, unit, location)) {
ASTParser astParser = ASTParser.newParser(AST.getJLSLatest());
astParser.setSource(unit);
astParser.setResolveBindings(true);
CompilationUnit cu = (CompilationUnit) astParser.createAST(null);
CustomASTVisitor visitor = new CustomASTVisitor(query, match, QueryLocation.ANNOTATION);
// Under tests, resolveConstructorBinding will return null if there are problems
IProblem[] problems = cu.getProblems();
if (problems != null && problems.length > 0) {
logInfo("KONVEYOR_LOG: " + "Found " + problems.length + " problems while compiling");
int count = 0;
for (IProblem problem : problems) {
logInfo("KONVEYOR_LOG: Problem - ID: " + problem.getID() + " Message: " + problem.getMessage());
count++;
if (count >= SymbolProvider.MAX_PROBLEMS_TO_LOG) {
logInfo("KONVEYOR_LOG: Only showing first " + SymbolProvider.MAX_PROBLEMS_TO_LOG + " problems, " + (problems.length - SymbolProvider.MAX_PROBLEMS_TO_LOG) + " more not displayed");
break;
}
}
}
cu.accept(visitor);
if (visitor.symbolMatches()) {
if (annotationQuery != null) {
List<Class<? extends SourceRefElement>> classes = new ArrayList<>();
classes.add(ResolvedSourceMethod.class);
classes.add(ResolvedSourceField.class);
classes.add(ResolvedSourceType.class);
if (matchesAnnotationQuery(match, classes)) {
Copy link
Contributor

@shawn-hurley shawn-hurley Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a way to make the matchesAnnotationQuery be like a Go vardic function?

symbols.add(symbol);
}
} else {
symbols.add(symbol);
}
}
}
if (unit != null && unit.isWorkingCopy()) {
unit.discardWorkingCopy();
unit.close();
}
} else {
symbols.add(symbol);
if (annotationQuery != null) {
List<Class<? extends SourceRefElement>> classes = new ArrayList<>();
classes.add(ResolvedSourceMethod.class);
classes.add(ResolvedSourceField.class);
classes.add(ResolvedSourceType.class);
if (matchesAnnotationQuery(match, classes)) {
symbols.add(symbol);
}
} else {
symbols.add(symbol);
}
}

}
return symbols;
} catch (Exception e) {
Expand All @@ -60,4 +122,9 @@ public AnnotationQuery getAnnotationQuery() {
public void setAnnotationQuery(AnnotationQuery annotationQuery) {
this.annotationQuery = annotationQuery;
}

@Override
public void setQuery(String query) {
this.query = query;
}
}
2 changes: 1 addition & 1 deletion .../src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public List<SymbolInformation> get(SearchMatch match) throws CoreException {
unit = cls.getWorkingCopy(new WorkingCopyOwnerImpl(), null);
}
}
if (this.queryQualificationMatches(this.query, unit, location)) {
if (this.queryQualificationMatches(this.query, mod, unit, location)) {
ASTParser astParser = ASTParser.newParser(AST.getJLSLatest());
astParser.setSource(unit);
astParser.setResolveBindings(true);
Expand Down
64 changes: 64 additions & 0 deletions ...r-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@
import org.eclipse.jdt.core.dom.ASTVisitor;
import org.eclipse.jdt.core.dom.ClassInstanceCreation;
import org.eclipse.jdt.core.dom.ConstructorInvocation;
import org.eclipse.jdt.core.dom.IAnnotationBinding;
import org.eclipse.jdt.core.dom.IMethodBinding;
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.Annotation;
import org.eclipse.jdt.core.dom.MarkerAnnotation;
import org.eclipse.jdt.core.dom.MethodInvocation;
import org.eclipse.jdt.core.dom.NormalAnnotation;
import org.eclipse.jdt.core.dom.SingleMemberAnnotation;
import org.eclipse.jdt.core.search.SearchMatch;

/*
Expand All @@ -32,6 +37,7 @@ public class CustomASTVisitor extends ASTVisitor {
public enum QueryLocation {
METHOD_CALL,
CONSTRUCTOR_CALL,
ANNOTATION,
}

public CustomASTVisitor(String query, SearchMatch match, QueryLocation location) {
Expand Down Expand Up @@ -62,6 +68,64 @@ private boolean shouldVisit(ASTNode node) {
(node.getStartPosition() + node.getLength());
}

@Override
public boolean visit(MarkerAnnotation node) {
return visit((Annotation) node);
}

@Override
public boolean visit(NormalAnnotation node) {
return visit((Annotation) node);
}

@Override
public boolean visit(SingleMemberAnnotation node) {
return visit((Annotation) node);
}

private boolean visit(Annotation node) {
// There is a problem with trying to run shouldVisit() here because
// matches on annotations aren't directly on the annotation node,
// but on the annotated one (class, method, field, etc). So we can't
// use shouldVisit() to filter out nodes we don't want to visit.
// TODO: think of a better way to handle this
if (this.location != QueryLocation.ANNOTATION) {
return true;
}
try {
IAnnotationBinding binding = node.resolveAnnotationBinding();
if (binding != null) {
// get fqn
ITypeBinding declaringClass = binding.getAnnotationType();
if (declaringClass != null) {
Copy link
Contributor

@shawn-hurley shawn-hurley Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we invert and return early so that we don't have nested depth of logic?

// Handle Erasure results
if (declaringClass.getErasure() != null) {
declaringClass = declaringClass.getErasure();
}
String fullyQualifiedName = declaringClass.getQualifiedName();
// match fqn with query pattern
if (fullyQualifiedName.matches(this.query)) {
this.symbolMatches = true;
return false;
} else {
logInfo("method fqn " + fullyQualifiedName + " did not match with " + query);
Copy link

@coderabbitai coderabbitai bot Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix copy-paste errors in log messages.

The log messages reference "method" and "MethodInvocation" but should reference "annotation" since this is the annotation visitor:

  • Line 100: "method fqn" → "annotation fqn"
  • Line 105: "MethodInvocation" → "Annotation"
  • Line 111: "MethodInvocation" → "Annotation"

Apply this diff:

-                    logInfo("method fqn " + fullyQualifiedName + " did not match with " + query);
+                    logInfo("annotation fqn " + fullyQualifiedName + " did not match with " + query);
-            logInfo("failed to get accurate info for MethodInvocation, falling back");
+            logInfo("failed to get accurate info for Annotation, falling back");
-            logInfo("KONVEYOR_LOG: error visiting MethodInvocation node: " + e);
+            logInfo("KONVEYOR_LOG: error visiting Annotation node: " + e);

Also applies to: 105-105, 111-111

🤖 Prompt for AI Agents 
In
java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java
around lines 100, 105 and 111, fix copy-paste errors in the log messages that
incorrectly reference "method" and "MethodInvocation": change the message at
line 100 from "method fqn" to "annotation fqn", and replace occurrences of
"MethodInvocation" at lines 105 and 111 with "Annotation" so the logs correctly
refer to annotations in this visitor.

return true;
}
}
}
logInfo("failed to get accurate info for MethodInvocation, falling back");
// sometimes binding or declaring class cannot be found, usually due to errors
// in source code. in that case, we will fallback and accept the match
this.symbolMatches = true;
return false;
} catch (Exception e) {
logInfo("KONVEYOR_LOG: error visiting MethodInvocation node: " + e);
// this is so that we fallback and don't lose a match when we fail
this.symbolMatches = true;
return false;
}
}

/*
* This is to get information from a MethodInvocation, used for METHOD_CALL
* we discard a match only when we can tell for sure. otherwise we accept
Expand Down
2 changes: 1 addition & 1 deletion ....core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public List<SymbolInformation> get(SearchMatch match) {
unit = cls.getWorkingCopy(new WorkingCopyOwnerImpl(), null);
}
}
if (this.queryQualificationMatches(this.query, unit, location)) {
if (this.queryQualificationMatches(this.query, e, unit, location)) {
ASTParser astParser = ASTParser.newParser(AST.getJLSLatest());
astParser.setSource(unit);
astParser.setResolveBindings(true);
Expand Down
10 changes: 9 additions & 1 deletion ...zer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ private static void setPosition(Position position, int[] coords) {
* 3. the compilation unit has a package declaration as `konveyor.io.Util`
* we do this so that we can rule out a lot of matches before going the AST route
*/
default boolean queryQualificationMatches(String query, ICompilationUnit unit, Location location) {
default boolean queryQualificationMatches(String query, IJavaElement matchedElement,ICompilationUnit unit, Location location) {
// Make sure that the ICompilationUnit is conistant
try {
unit.makeConsistent(null);
Expand All @@ -206,6 +206,14 @@ default boolean queryQualificationMatches(String query, ICompilationUnit unit, L
// for a query, java.io.paths.File*, queryQualification is java.io.paths
queryQualification = query.substring(0, dotIndex);
}
// an element need not be imported if its referenced by fqn
if (!queryQualification.isEmpty() && (
matchedElement.getElementName().equals(queryQualification)
|| matchedElement.getElementName().startsWith(queryQualification + ".")
|| queryQualification.matches(matchedElement.getElementName())
)) {
return true;
}
Comment on lines +209 to +216
Copy link

@coderabbitai coderabbitai bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Guard against null matchedElement and fix backwards regex match.

Two issues:

  1. matchedElement could be null (no prior null check), causing NPEs at lines 211-213.
  2. Line 213 has the match backwards: queryQualification.matches(matchedElement.getElementName()) treats the query as a string and the element name as a regex pattern, which is the opposite of the intended logic.

Apply this diff:

     // an element need not be imported if its referenced by fqn
-    if (!queryQualification.isEmpty() && (
-            matchedElement.getElementName().equals(queryQualification)
-         || matchedElement.getElementName().startsWith(queryQualification + ".")
-         || queryQualification.matches(matchedElement.getElementName())
-    )) {
+    if (matchedElement != null && !queryQualification.isEmpty() && (
+            matchedElement.getElementName().equals(queryQualification)
+         || matchedElement.getElementName().startsWith(queryQualification + ".")
+         || matchedElement.getElementName().matches(queryQualification)
+    )) {
         return true;
     }
🤖 Prompt for AI Agents 
In
java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java
around lines 209 to 216, guard against a null matchedElement and correct the
regex match direction: first check matchedElement != null before accessing
getElementName(), and replace
queryQualification.matches(matchedElement.getElementName()) with
matchedElement.getElementName().matches(queryQualification) (or otherwise
null-safe call) so the element name is matched against the query pattern; keep
the existing equality/startsWith checks but ensure all getElementName()
invocations occur only after the null check.

String packageQueryQualification = "";
int packageDotIndex = queryQualification.lastIndexOf('.');
if (packageDotIndex > 0) {
Expand Down
6 changes: 3 additions & 3 deletions java-analyzer-bundle.tp/java-analyzer-bundle.target
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
<?pde version="3.8"?>
<target name="Java Analyzer Target Platform">
<locations>
<!-- 1st: pick up latest JDT.LS bits
<!-- 1st: pick up JDT.LS bits (1.35.0)
-->
<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner"
includeSource="true" type="InstallableUnit">
<unit id="org.eclipse.jdt.ls.core" version="0.0.0" />
<repository location="https://download.eclipse.org/jdtls/milestones/1.40.0/repository"/>
<repository location="https://download.eclipse.org/jdtls/milestones/1.35.0/repository"/>
</location>


Expand Down Expand Up @@ -64,7 +64,7 @@
<repository location="https://download.eclipse.org/releases/2021-09/202109151000/"/>
</location>
<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
<repository location="https://download.eclipse.org/lsp4j/updates/releases/0.23.1/"/>
<repository location="https://download.eclipse.org/lsp4j/updates/releases/0.22.0/"/>
<unit id="org.eclipse.lsp4j.sdk.feature.group" version="0.0.0"/>
</location>
<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
Expand Down
Loading