Skip to content

Commit fe53c7f

Browse files
konveyor-ci-bot[bot]jmlepranavgaikwadshawn-hurley
authored
🐛 Fix annotation matching (#149) (#161)
* 🐛 Fix annotation matching (#149) * tmp Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * Working: Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * Move in check for annotated classes Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * tmp Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * Fix stuff Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * Final fix Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * Fix shouldVisit Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * Fix condition Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * Undo Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * make sure that qualification matches considers fqn usage without import and queries like (A|B|C) Signed-off-by: Pranav Gaikwad <[email protected]> * fix a minor bug in processing regex Signed-off-by: Pranav Gaikwad <[email protected]> * Fix regex Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * Fix potential npe Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> --------- Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> Signed-off-by: Pranav Gaikwad <[email protected]> Co-authored-by: Pranav Gaikwad <[email protected]> Signed-off-by: Cherry Picker <[email protected]> * 🐛 Fixing annotation searching by getting the FQDN from the compliation. (#163) * Fixing annotation searching by getting the FQDN from the compliation. * Falls back to trying to use the code actions to get the FQDN Signed-off-by: Shawn Hurley <[email protected]> * Couple of fixes Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> * Respect annotated feature Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> --------- Signed-off-by: Shawn Hurley <[email protected]> Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> Co-authored-by: Juan Manuel Leflet Estrada <[email protected]> --------- Signed-off-by: Juan Manuel Leflet Estrada <[email protected]> Signed-off-by: Pranav Gaikwad <[email protected]> Signed-off-by: Cherry Picker <[email protected]> Signed-off-by: Shawn Hurley <[email protected]> Co-authored-by: Juan Manuel Leflet Estrada <[email protected]> Co-authored-by: Pranav Gaikwad <[email protected]> Co-authored-by: Shawn Hurley <[email protected]>
1 parent 2c8515c commit fe53c7f

File tree

7 files changed

+197
-24
lines changed

7 files changed

+197
-24
lines changed

‎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: 116 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,47 +3,142 @@
33
import static org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin.logInfo;
44

55
import java.util.ArrayList;
6+
import java.util.Arrays;
67
import java.util.List;
78

8-
import io.konveyor.tackle.core.internal.query.AnnotationQuery;
99
import org.eclipse.core.runtime.CoreException;
1010
import org.eclipse.jdt.core.IAnnotatable;
1111
import org.eclipse.jdt.core.IAnnotation;
12+
import org.eclipse.jdt.core.IClassFile;
13+
import org.eclipse.jdt.core.ICompilationUnit;
1214
import org.eclipse.jdt.core.IJavaElement;
15+
import org.eclipse.jdt.core.IType;
16+
import org.eclipse.jdt.core.compiler.IProblem;
17+
import org.eclipse.jdt.core.dom.AST;
18+
import org.eclipse.jdt.core.dom.ASTParser;
19+
import org.eclipse.jdt.core.dom.CompilationUnit;
1320
import org.eclipse.jdt.core.search.SearchMatch;
1421
import org.eclipse.jdt.internal.core.ResolvedSourceField;
1522
import org.eclipse.jdt.internal.core.ResolvedSourceMethod;
1623
import org.eclipse.jdt.internal.core.ResolvedSourceType;
1724
import org.eclipse.jdt.internal.core.SourceRefElement;
25+
import org.eclipse.lsp4j.Location;
1826
import org.eclipse.lsp4j.SymbolInformation;
1927

20-
public class AnnotationSymbolProvider implements SymbolProvider, WithAnnotationQuery {
28+
import io.konveyor.tackle.core.internal.query.AnnotationQuery;
29+
import io.konveyor.tackle.core.internal.symbol.CustomASTVisitor.QueryLocation;
30+
31+
public class AnnotationSymbolProvider implements SymbolProvider, WithQuery, WithAnnotationQuery {
2132

2233
private AnnotationQuery annotationQuery;
34+
private String query;
35+
36+
private static final List<Class<? extends SourceRefElement>> ACCEPTED_CLASSES = new ArrayList<>();
37+
static {
38+
ACCEPTED_CLASSES.add(ResolvedSourceMethod.class);
39+
ACCEPTED_CLASSES.add(ResolvedSourceField.class);
40+
ACCEPTED_CLASSES.add(ResolvedSourceType.class);
41+
}
2342

2443
@Override
2544
public List<SymbolInformation> get(SearchMatch match) throws CoreException {
2645
List<SymbolInformation> symbols = new ArrayList<>();
2746
try {
28-
IAnnotatable mod = (IAnnotatable) match.getElement();
29-
IJavaElement element = (IJavaElement) match.getElement();
30-
for (IAnnotation annotation : mod.getAnnotations()) {
47+
IAnnotatable annotatable = (IAnnotatable) match.getElement();
48+
for (IAnnotation annotation : annotatable.getAnnotations()) {
49+
IJavaElement annotationElement = annotation.getPrimaryElement();
3150
SymbolInformation symbol = new SymbolInformation();
3251
symbol.setName(annotation.getElementName());
33-
symbol.setKind(convertSymbolKind(element));
52+
symbol.setKind(convertSymbolKind(annotationElement));
3453
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);
54+
Location location = getLocation(annotationElement, match);
55+
symbol.setLocation(location);
56+
if (this.query.contains(".")) {
57+
// First try to get compilation unit for source files
58+
ICompilationUnit unit = (ICompilationUnit) annotationElement.getAncestor(IJavaElement.COMPILATION_UNIT);
59+
if (unit == null) {
60+
// If not in source, try to get class file for compiled classes
61+
IClassFile cls = (IClassFile) annotationElement.getAncestor(IJavaElement.CLASS_FILE);
62+
if (cls != null) {
63+
unit = cls.getWorkingCopy(new WorkingCopyOwnerImpl(), null);
64+
}
65+
}
66+
if (unit != null) {
67+
IType t = unit.getType(annotationElement.getElementName());
68+
String fqdn = "";
69+
if (!t.isResolved()) {
70+
var elements = unit.codeSelect(match.getOffset(), match.getLength());
71+
for (IJavaElement e: Arrays.asList(elements)) {
72+
if (e instanceof IType) {
73+
var newT = (IType) e;
74+
if (newT.isResolved()) {
75+
fqdn = newT.getFullyQualifiedName('.');
76+
logInfo("FQDN from code select: " + fqdn);
77+
}
78+
}
79+
}
80+
} else {
81+
fqdn = t.getFullyQualifiedName('.');
82+
logInfo("resolved type: " + fqdn);
83+
}
84+
if (query.matches(fqdn) || fqdn.matches(query)) {
85+
if (unit.isWorkingCopy()) {
86+
unit.discardWorkingCopy();
87+
unit.close();
88+
}
89+
90+
if (matchesAnnotationQuery(match, ACCEPTED_CLASSES)) {
91+
symbols.add(symbol);
92+
}
93+
return symbols;
94+
}
95+
}
96+
97+
logInfo("falling back to resolving via AST");
98+
99+
if (this.queryQualificationMatches(this.query.replaceAll("\\(([A-Za-z_][A-Za-z0-9_]*(\\|[A-Za-z_][A-Za-z0-9_]*)*)\\)", ".*"), annotationElement, unit, location)) {
100+
ASTParser astParser = ASTParser.newParser(AST.getJLSLatest());
101+
astParser.setSource(unit);
102+
astParser.setResolveBindings(true);
103+
CompilationUnit cu = (CompilationUnit) astParser.createAST(null);
104+
CustomASTVisitor visitor = new CustomASTVisitor(query, match, QueryLocation.ANNOTATION);
105+
// Under tests, resolveConstructorBinding will return null if there are problems
106+
IProblem[] problems = cu.getProblems();
107+
if (problems != null && problems.length > 0) {
108+
logInfo("KONVEYOR_LOG: " + "Found " + problems.length + " problems while compiling");
109+
int count = 0;
110+
for (IProblem problem : problems) {
111+
logInfo("KONVEYOR_LOG: Problem - ID: " + problem.getID() + " Message: " + problem.getMessage());
112+
count++;
113+
if (count >= SymbolProvider.MAX_PROBLEMS_TO_LOG) {
114+
logInfo("KONVEYOR_LOG: Only showing first " + SymbolProvider.MAX_PROBLEMS_TO_LOG + " problems, " + (problems.length - SymbolProvider.MAX_PROBLEMS_TO_LOG) + " more not displayed");
115+
break;
116+
}
117+
}
118+
}
119+
cu.accept(visitor);
120+
if (visitor.symbolMatches()) {
121+
if (annotationQuery != null) {
122+
if (matchesAnnotationQuery(match, ACCEPTED_CLASSES)) {
123+
symbols.add(symbol);
124+
}
125+
} else {
126+
symbols.add(symbol);
127+
}
128+
}
129+
}
130+
if (unit != null && unit.isWorkingCopy()) {
131+
unit.discardWorkingCopy();
132+
unit.close();
44133
}
45134
} else {
46-
symbols.add(symbol);
135+
if (annotationQuery != null) {
136+
if (matchesAnnotationQuery(match, ACCEPTED_CLASSES)) {
137+
symbols.add(symbol);
138+
}
139+
} else {
140+
symbols.add(symbol);
141+
}
47142
}
48143
}
49144
return symbols;
@@ -60,4 +155,9 @@ public AnnotationQuery getAnnotationQuery() {
60155
public void setAnnotationQuery(AnnotationQuery annotationQuery) {
61156
this.annotationQuery = annotationQuery;
62157
}
158+
159+
@Override
160+
public void setQuery(String query) {
161+
this.query = query;
162+
}
63163
}

‎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)