Skip to content

Commit 4ca22e2

Browse files
jmlepranavgaikwad
jmle
and
pranavgaikwad
authored
🐛 Fix annotation matching (#149)
* tmp Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> * Working: Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> * Move in check for annotated classes Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> * tmp Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> * Fix stuff Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> * Final fix Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> * Fix shouldVisit Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> * Fix condition Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> * Undo Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> * make sure that qualification matches considers fqn usage without import and queries like (A|B|C) Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com> * fix a minor bug in processing regex Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com> * Fix regex Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> * Fix potential npe Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> --------- Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com> Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com> Co-authored-by: Pranav Gaikwad <pgaikwad@redhat.com>
1 parent 95176d0 commit 4ca22e2

7 files changed

Lines changed: 163 additions & 23 deletions

File tree

‎java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/SampleDelegateCommandHandler.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ private static SearchPattern mapLocationToSearchPatternLocation(int location, St
162162
*/
163163
private static SearchPattern getPatternSingleQuery(int location, String query) throws Exception {
164164
var pattern = SearchPattern.R_PATTERN_MATCH;
165-
if ((!query.contains("?") || !query.contains("*")) && (location != 11)) {
165+
if ((!query.contains("?") && !query.contains("*")) && (location != 11)) {
166166
logInfo("Using full match");
167167
pattern = SearchPattern.R_EXACT_MATCH | SearchPattern.R_CASE_SENSITIVE;
168168
}
@@ -376,7 +376,8 @@ private static List<SymbolInformation> search(String projectName, ArrayList<Stri
376376

377377
// Now run ImportScanner only on units in scope
378378
ASTParser parser = ASTParser.newParser(AST.getJLSLatest());
379-
Pattern regex = Pattern.compile(query);
379+
// when creating the regex, replace * with .*
380+
Pattern regex = Pattern.compile(query.replaceAll("(?<!\\.)\\*", ".*"));
380381
for (ICompilationUnit unit : units) {
381382
parser.setSource(unit);
382383
CompilationUnit cu = (CompilationUnit) parser.createAST(null);

‎java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/AnnotationSymbolProvider.java‎

Lines changed: 82 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,45 +6,107 @@
66
import java.util.List;
77

88
import io.konveyor.tackle.core.internal.query.AnnotationQuery;
9+
import io.konveyor.tackle.core.internal.symbol.CustomASTVisitor.QueryLocation;
10+
911
import org.eclipse.core.runtime.CoreException;
1012
import org.eclipse.jdt.core.IAnnotatable;
1113
import org.eclipse.jdt.core.IAnnotation;
14+
import org.eclipse.jdt.core.IClassFile;
15+
import org.eclipse.jdt.core.ICompilationUnit;
1216
import org.eclipse.jdt.core.IJavaElement;
17+
import org.eclipse.jdt.core.compiler.IProblem;
18+
import org.eclipse.jdt.core.dom.AST;
19+
import org.eclipse.jdt.core.dom.ASTParser;
20+
import org.eclipse.jdt.core.dom.CompilationUnit;
1321
import org.eclipse.jdt.core.search.SearchMatch;
1422
import org.eclipse.jdt.internal.core.ResolvedSourceField;
1523
import org.eclipse.jdt.internal.core.ResolvedSourceMethod;
1624
import org.eclipse.jdt.internal.core.ResolvedSourceType;
1725
import org.eclipse.jdt.internal.core.SourceRefElement;
26+
import org.eclipse.lsp4j.Location;
1827
import org.eclipse.lsp4j.SymbolInformation;
1928

20-
public class AnnotationSymbolProvider implements SymbolProvider, WithAnnotationQuery {
29+
public class AnnotationSymbolProvider implements SymbolProvider, WithQuery, WithAnnotationQuery {
2130

2231
private AnnotationQuery annotationQuery;
32+
private String query;
2333

2434
@Override
2535
public List<SymbolInformation> get(SearchMatch match) throws CoreException {
2636
List<SymbolInformation> symbols = new ArrayList<>();
2737
try {
28-
IAnnotatable mod = (IAnnotatable) match.getElement();
29-
IJavaElement element = (IJavaElement) match.getElement();
30-
for (IAnnotation annotation : mod.getAnnotations()) {
38+
IAnnotatable annotatable = (IAnnotatable) match.getElement();
39+
for (IAnnotation annotation : annotatable.getAnnotations()) {
40+
IJavaElement annotationElement = annotation.getPrimaryElement();
3141
SymbolInformation symbol = new SymbolInformation();
3242
symbol.setName(annotation.getElementName());
33-
symbol.setKind(convertSymbolKind(element));
43+
symbol.setKind(convertSymbolKind(annotationElement));
3444
symbol.setContainerName(annotation.getParent().getElementName());
35-
symbol.setLocation(getLocation(element, match));
36-
37-
if (annotationQuery != null) {
38-
List<Class<? extends SourceRefElement>> classes = new ArrayList<>();
39-
classes.add(ResolvedSourceMethod.class);
40-
classes.add(ResolvedSourceField.class);
41-
classes.add(ResolvedSourceType.class);
42-
if (matchesAnnotationQuery(match, classes)) {
43-
symbols.add(symbol);
45+
Location location = getLocation(annotationElement, match);
46+
symbol.setLocation(location);
47+
if (this.query.contains(".")) {
48+
// First try to get compilation unit for source files
49+
ICompilationUnit unit = (ICompilationUnit) annotationElement.getAncestor(IJavaElement.COMPILATION_UNIT);
50+
if (unit == null) {
51+
// If not in source, try to get class file for compiled classes
52+
IClassFile cls = (IClassFile) annotationElement.getAncestor(IJavaElement.CLASS_FILE);
53+
if (cls != null) {
54+
unit = cls.getWorkingCopy(new WorkingCopyOwnerImpl(), null);
55+
}
56+
}
57+
if (this.queryQualificationMatches(this.query.replaceAll("\\(([A-Za-z_][A-Za-z0-9_]*(\\|[A-Za-z_][A-Za-z0-9_]*)*)\\)", ".*"), annotationElement, unit, location)) {
58+
ASTParser astParser = ASTParser.newParser(AST.getJLSLatest());
59+
astParser.setSource(unit);
60+
astParser.setResolveBindings(true);
61+
CompilationUnit cu = (CompilationUnit) astParser.createAST(null);
62+
CustomASTVisitor visitor = new CustomASTVisitor(query, match, QueryLocation.ANNOTATION);
63+
// Under tests, resolveConstructorBinding will return null if there are problems
64+
IProblem[] problems = cu.getProblems();
65+
if (problems != null && problems.length > 0) {
66+
logInfo("KONVEYOR_LOG: " + "Found " + problems.length + " problems while compiling");
67+
int count = 0;
68+
for (IProblem problem : problems) {
69+
logInfo("KONVEYOR_LOG: Problem - ID: " + problem.getID() + " Message: " + problem.getMessage());
70+
count++;
71+
if (count >= SymbolProvider.MAX_PROBLEMS_TO_LOG) {
72+
logInfo("KONVEYOR_LOG: Only showing first " + SymbolProvider.MAX_PROBLEMS_TO_LOG + " problems, " + (problems.length - SymbolProvider.MAX_PROBLEMS_TO_LOG) + " more not displayed");
73+
break;
74+
}
75+
}
76+
}
77+
cu.accept(visitor);
78+
if (visitor.symbolMatches()) {
79+
if (annotationQuery != null) {
80+
List<Class<? extends SourceRefElement>> classes = new ArrayList<>();
81+
classes.add(ResolvedSourceMethod.class);
82+
classes.add(ResolvedSourceField.class);
83+
classes.add(ResolvedSourceType.class);
84+
if (matchesAnnotationQuery(match, classes)) {
85+
symbols.add(symbol);
86+
}
87+
} else {
88+
symbols.add(symbol);
89+
}
90+
}
91+
}
92+
if (unit != null && unit.isWorkingCopy()) {
93+
unit.discardWorkingCopy();
94+
unit.close();
4495
}
4596
} else {
46-
symbols.add(symbol);
97+
if (annotationQuery != null) {
98+
List<Class<? extends SourceRefElement>> classes = new ArrayList<>();
99+
classes.add(ResolvedSourceMethod.class);
100+
classes.add(ResolvedSourceField.class);
101+
classes.add(ResolvedSourceType.class);
102+
if (matchesAnnotationQuery(match, classes)) {
103+
symbols.add(symbol);
104+
}
105+
} else {
106+
symbols.add(symbol);
107+
}
47108
}
109+
48110
}
49111
return symbols;
50112
} catch (Exception e) {
@@ -60,4 +122,9 @@ public AnnotationQuery getAnnotationQuery() {
60122
public void setAnnotationQuery(AnnotationQuery annotationQuery) {
61123
this.annotationQuery = annotationQuery;
62124
}
125+
126+
@Override
127+
public void setQuery(String query) {
128+
this.query = query;
129+
}
63130
}

‎java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public List<SymbolInformation> get(SearchMatch match) throws CoreException {
5353
unit = cls.getWorkingCopy(new WorkingCopyOwnerImpl(), null);
5454
}
5555
}
56-
if (this.queryQualificationMatches(this.query, unit, location)) {
56+
if (this.queryQualificationMatches(this.query, mod, unit, location)) {
5757
ASTParser astParser = ASTParser.newParser(AST.getJLSLatest());
5858
astParser.setSource(unit);
5959
astParser.setResolveBindings(true);

‎java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,14 @@
66
import org.eclipse.jdt.core.dom.ASTVisitor;
77
import org.eclipse.jdt.core.dom.ClassInstanceCreation;
88
import org.eclipse.jdt.core.dom.ConstructorInvocation;
9+
import org.eclipse.jdt.core.dom.IAnnotationBinding;
910
import org.eclipse.jdt.core.dom.IMethodBinding;
1011
import org.eclipse.jdt.core.dom.ITypeBinding;
12+
import org.eclipse.jdt.core.dom.Annotation;
13+
import org.eclipse.jdt.core.dom.MarkerAnnotation;
1114
import org.eclipse.jdt.core.dom.MethodInvocation;
15+
import org.eclipse.jdt.core.dom.NormalAnnotation;
16+
import org.eclipse.jdt.core.dom.SingleMemberAnnotation;
1217
import org.eclipse.jdt.core.search.SearchMatch;
1318

1419
/*
@@ -32,6 +37,7 @@ public class CustomASTVisitor extends ASTVisitor {
3237
public enum QueryLocation {
3338
METHOD_CALL,
3439
CONSTRUCTOR_CALL,
40+
ANNOTATION,
3541
}
3642

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

71+
@Override
72+
public boolean visit(MarkerAnnotation node) {
73+
return visit((Annotation) node);
74+
}
75+
76+
@Override
77+
public boolean visit(NormalAnnotation node) {
78+
return visit((Annotation) node);
79+
}
80+
81+
@Override
82+
public boolean visit(SingleMemberAnnotation node) {
83+
return visit((Annotation) node);
84+
}
85+
86+
private boolean visit(Annotation node) {
87+
// There is a problem with trying to run shouldVisit() here because
88+
// matches on annotations aren't directly on the annotation node,
89+
// but on the annotated one (class, method, field, etc). So we can't
90+
// use shouldVisit() to filter out nodes we don't want to visit.
91+
// TODO: think of a better way to handle this
92+
if (this.location != QueryLocation.ANNOTATION) {
93+
return true;
94+
}
95+
try {
96+
IAnnotationBinding binding = node.resolveAnnotationBinding();
97+
if (binding != null) {
98+
// get fqn
99+
ITypeBinding declaringClass = binding.getAnnotationType();
100+
if (declaringClass != null) {
101+
// Handle Erasure results
102+
if (declaringClass.getErasure() != null) {
103+
declaringClass = declaringClass.getErasure();
104+
}
105+
String fullyQualifiedName = declaringClass.getQualifiedName();
106+
// match fqn with query pattern
107+
if (fullyQualifiedName.matches(this.query)) {
108+
this.symbolMatches = true;
109+
return false;
110+
} else {
111+
logInfo("method fqn " + fullyQualifiedName + " did not match with " + query);
112+
return true;
113+
}
114+
}
115+
}
116+
logInfo("failed to get accurate info for MethodInvocation, falling back");
117+
// sometimes binding or declaring class cannot be found, usually due to errors
118+
// in source code. in that case, we will fallback and accept the match
119+
this.symbolMatches = true;
120+
return false;
121+
} catch (Exception e) {
122+
logInfo("KONVEYOR_LOG: error visiting MethodInvocation node: " + e);
123+
// this is so that we fallback and don't lose a match when we fail
124+
this.symbolMatches = true;
125+
return false;
126+
}
127+
}
128+
65129
/*
66130
* This is to get information from a MethodInvocation, used for METHOD_CALL
67131
* we discard a match only when we can tell for sure. otherwise we accept

‎java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public List<SymbolInformation> get(SearchMatch match) {
5353
unit = cls.getWorkingCopy(new WorkingCopyOwnerImpl(), null);
5454
}
5555
}
56-
if (this.queryQualificationMatches(this.query, unit, location)) {
56+
if (this.queryQualificationMatches(this.query, e, unit, location)) {
5757
ASTParser astParser = ASTParser.newParser(AST.getJLSLatest());
5858
astParser.setSource(unit);
5959
astParser.setResolveBindings(true);

‎java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ private static void setPosition(Position position, int[] coords) {
188188
* 3. the compilation unit has a package declaration as `konveyor.io.Util`
189189
* we do this so that we can rule out a lot of matches before going the AST route
190190
*/
191-
default boolean queryQualificationMatches(String query, ICompilationUnit unit, Location location) {
191+
default boolean queryQualificationMatches(String query, IJavaElement matchedElement,ICompilationUnit unit, Location location) {
192192
// Make sure that the ICompilationUnit is conistant
193193
try {
194194
unit.makeConsistent(null);
@@ -206,6 +206,14 @@ default boolean queryQualificationMatches(String query, ICompilationUnit unit, L
206206
// for a query, java.io.paths.File*, queryQualification is java.io.paths
207207
queryQualification = query.substring(0, dotIndex);
208208
}
209+
// an element need not be imported if its referenced by fqn
210+
if (!queryQualification.isEmpty() && (
211+
matchedElement.getElementName().equals(queryQualification)
212+
|| matchedElement.getElementName().startsWith(queryQualification + ".")
213+
|| queryQualification.matches(matchedElement.getElementName())
214+
)) {
215+
return true;
216+
}
209217
String packageQueryQualification = "";
210218
int packageDotIndex = queryQualification.lastIndexOf('.');
211219
if (packageDotIndex > 0) {

‎java-analyzer-bundle.tp/java-analyzer-bundle.target‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
<?pde version="3.8"?>
33
<target name="Java Analyzer Target Platform">
44
<locations>
5-
<!-- 1st: pick up latest JDT.LS bits
5+
<!-- 1st: pick up JDT.LS bits (1.35.0)
66
-->
77
<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner"
88
includeSource="true" type="InstallableUnit">
99
<unit id="org.eclipse.jdt.ls.core" version="0.0.0" />
10-
<repository location="https://download.eclipse.org/jdtls/milestones/1.40.0/repository"/>
10+
<repository location="https://download.eclipse.org/jdtls/milestones/1.35.0/repository"/>
1111
</location>
1212

1313

@@ -64,7 +64,7 @@
6464
<repository location="https://download.eclipse.org/releases/2021-09/202109151000/"/>
6565
</location>
6666
<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
67-
<repository location="https://download.eclipse.org/lsp4j/updates/releases/0.23.1/"/>
67+
<repository location="https://download.eclipse.org/lsp4j/updates/releases/0.22.0/"/>
6868
<unit id="org.eclipse.lsp4j.sdk.feature.group" version="0.0.0"/>
6969
</location>
7070
<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">

0 commit comments

Comments
 (0)