From 13ca6a231e39ee6c30bcd3c5c66c9c2e71b1efad Mon Sep 17 00:00:00 2001 From: Pranav Gaikwad Date: Tue, 25 Jun 2024 17:49:12 -0400 Subject: [PATCH 1/8] :bug: improve accuracy of MethodReference matches Signed-off-by: Pranav Gaikwad --- .../symbol/MethodCallSymbolProvider.java | 58 +++++++++++++++++-- .../core/internal/symbol/SymbolProvider.java | 9 +++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java index 7e62ac0..8577107 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java @@ -5,10 +5,18 @@ import java.util.ArrayList; import java.util.List; +import org.eclipse.jdt.core.ICompilationUnit; import org.eclipse.jdt.core.IJavaElement; import org.eclipse.jdt.core.IMethod; import org.eclipse.jdt.core.IType; import org.eclipse.jdt.core.ITypeRoot; +import org.eclipse.jdt.core.dom.AST; +import org.eclipse.jdt.core.dom.ASTParser; +import org.eclipse.jdt.core.dom.ASTVisitor; +import org.eclipse.jdt.core.dom.CompilationUnit; +import org.eclipse.jdt.core.dom.IMethodBinding; +import org.eclipse.jdt.core.dom.ITypeBinding; +import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.search.MethodReferenceMatch; import org.eclipse.jdt.core.search.SearchMatch; import org.eclipse.lsp4j.SymbolInformation; @@ -25,12 +33,50 @@ public List get(SearchMatch match) { try { MethodReferenceMatch m = (MethodReferenceMatch) match; IMethod e = (IMethod) m.getElement(); - SymbolInformation symbol = new SymbolInformation(); - symbol.setName(e.getElementName()); - symbol.setKind(convertSymbolKind(e)); - symbol.setContainerName(e.getParent().getElementName()); - symbol.setLocation(getLocation(e, match)); - symbols.add(symbol); + if (this.query.contains(".")) { + ICompilationUnit unit = e.getCompilationUnit(); + ASTParser astParser = ASTParser.newParser(AST.getJLSLatest()); + astParser.setSource(unit); + astParser.setResolveBindings(true); + CompilationUnit cu = (CompilationUnit) astParser.createAST(null); + cu.accept(new ASTVisitor() { + // we are only doing this for MethodInvocation right now + // look into MethodDeclaration if needed + public boolean visit(MethodInvocation node) { + try { + IMethodBinding binding = node.resolveMethodBinding(); + if (binding != null) { + // get fqn of the method being called + ITypeBinding declaringClass = binding.getDeclaringClass(); + if (declaringClass != null) { + String fullyQualifiedName = declaringClass.getQualifiedName() + "." + binding.getName(); + // match fqn with query pattern + if (fullyQualifiedName.matches(getCleanedQuery(query))) { + SymbolInformation symbol = new SymbolInformation(); + symbol.setName(e.getElementName()); + symbol.setKind(convertSymbolKind(e)); + symbol.setContainerName(e.getParent().getElementName()); + symbol.setLocation(getLocation(e, match)); + symbols.add(symbol); + } else { + logInfo("fqn " + fullyQualifiedName + " did not match with " + query); + } + } + } + } catch (Exception e) { + logInfo("error determining accuracy of match: " + e); + } + return true; + } + }); + } else { + SymbolInformation symbol = new SymbolInformation(); + symbol.setName(e.getElementName()); + symbol.setKind(convertSymbolKind(e)); + symbol.setContainerName(e.getParent().getElementName()); + symbol.setLocation(getLocation(e, match)); + symbols.add(symbol); + } } catch (Exception e) { logInfo("unable to convert for variable: " + e); } diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java index 382574b..e3331f2 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java @@ -167,4 +167,13 @@ private static void setPosition(Position position, int[] coords) { position.setLine(coords[0]); position.setCharacter(coords[1]); } + + /* + * When comparing query pattern with an actual found java element's fqn + * we need to make sure that * that are not preceded with a . are replaced + * by .* so that java regex works as expected on them + */ + default String getCleanedQuery(String query) { + return query.replaceAll("(? Date: Wed, 26 Jun 2024 09:46:24 -0400 Subject: [PATCH 2/8] improve accuracy of constructor call Signed-off-by: Pranav Gaikwad --- .../symbol/ConstructorCallSymbolProvider.java | 53 ++++++++++++++++++- .../symbol/MethodCallSymbolProvider.java | 15 ++---- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java index 2b016b1..670da70 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java @@ -6,14 +6,24 @@ import java.util.List; import org.eclipse.core.runtime.CoreException; +import org.eclipse.jdt.core.ICompilationUnit; import org.eclipse.jdt.core.IMethod; +import org.eclipse.jdt.core.dom.AST; +import org.eclipse.jdt.core.dom.ASTParser; +import org.eclipse.jdt.core.dom.ASTVisitor; +import org.eclipse.jdt.core.dom.CompilationUnit; +import org.eclipse.jdt.core.dom.IMethodBinding; +import org.eclipse.jdt.core.dom.ITypeBinding; +import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.search.MethodReferenceMatch; import org.eclipse.jdt.core.search.SearchMatch; import org.eclipse.jdt.internal.core.JavaElement; import org.eclipse.lsp4j.SymbolInformation; import org.eclipse.lsp4j.SymbolKind; -public class ConstructorCallSymbolProvider implements SymbolProvider { +public class ConstructorCallSymbolProvider implements SymbolProvider, WithQuery { + public String query; + @Override public List get(SearchMatch match) throws CoreException { List symbols = new ArrayList<>(); @@ -32,11 +42,50 @@ public List get(SearchMatch match) throws CoreException { } symbol.setContainerName(mod.getParent().getElementName()); symbol.setLocation(getLocation(mod, match)); - symbols.add(symbol); + if (this.query.contains(".")) { + ICompilationUnit unit = mod.getCompilationUnit(); + ASTParser astParser = ASTParser.newParser(AST.getJLSLatest()); + astParser.setSource(unit); + astParser.setResolveBindings(true); + CompilationUnit cu = (CompilationUnit) astParser.createAST(null); + cu.accept(new ASTVisitor() { + // we are only doing this for MethodInvocation right now + // look into MethodDeclaration if needed + public boolean visit(MethodInvocation node) { + try { + IMethodBinding binding = node.resolveMethodBinding(); + if (binding != null) { + // get fqn of the method being called + ITypeBinding declaringClass = binding.getDeclaringClass(); + if (declaringClass != null) { + String fullyQualifiedName = declaringClass.getQualifiedName() + "." + binding.getName(); + // match fqn with query pattern + if (fullyQualifiedName.matches(getCleanedQuery(query))) { + symbols.add(symbol); + } else { + logInfo("fqn " + fullyQualifiedName + " did not match with " + query); + } + } + } + } catch (Exception e) { + logInfo("error determining accuracy of match: " + e); + } + return true; + } + }); + } else { + symbols.add(symbol); + } } catch (Exception e) { logInfo("unable to get constructor: " + e); return null; } return symbols; } + + @Override + public void setQuery(String query) { + // TODO Auto-generated method stub + this.query = query; + } } diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java index 8577107..52bb007 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java @@ -33,6 +33,11 @@ public List get(SearchMatch match) { try { MethodReferenceMatch m = (MethodReferenceMatch) match; IMethod e = (IMethod) m.getElement(); + SymbolInformation symbol = new SymbolInformation(); + symbol.setName(e.getElementName()); + symbol.setKind(convertSymbolKind(e)); + symbol.setContainerName(e.getParent().getElementName()); + symbol.setLocation(getLocation(e, match)); if (this.query.contains(".")) { ICompilationUnit unit = e.getCompilationUnit(); ASTParser astParser = ASTParser.newParser(AST.getJLSLatest()); @@ -52,11 +57,6 @@ public boolean visit(MethodInvocation node) { String fullyQualifiedName = declaringClass.getQualifiedName() + "." + binding.getName(); // match fqn with query pattern if (fullyQualifiedName.matches(getCleanedQuery(query))) { - SymbolInformation symbol = new SymbolInformation(); - symbol.setName(e.getElementName()); - symbol.setKind(convertSymbolKind(e)); - symbol.setContainerName(e.getParent().getElementName()); - symbol.setLocation(getLocation(e, match)); symbols.add(symbol); } else { logInfo("fqn " + fullyQualifiedName + " did not match with " + query); @@ -70,11 +70,6 @@ public boolean visit(MethodInvocation node) { } }); } else { - SymbolInformation symbol = new SymbolInformation(); - symbol.setName(e.getElementName()); - symbol.setKind(convertSymbolKind(e)); - symbol.setContainerName(e.getParent().getElementName()); - symbol.setLocation(getLocation(e, match)); symbols.add(symbol); } } catch (Exception e) { From 67c2bb8334595093df00a309fae64e3e41b1cb8a Mon Sep 17 00:00:00 2001 From: Pranav Gaikwad Date: Wed, 26 Jun 2024 12:40:12 -0400 Subject: [PATCH 3/8] :ghost: improve logic / structure a bit Signed-off-by: Pranav Gaikwad --- .../symbol/ConstructorCallSymbolProvider.java | 30 ++----- .../internal/symbol/CustomASTVisitor.java | 86 +++++++++++++++++++ .../symbol/MethodCallSymbolProvider.java | 30 ++----- .../core/internal/symbol/SymbolProvider.java | 9 -- 4 files changed, 96 insertions(+), 59 deletions(-) create mode 100644 java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java index 670da70..1bf8cdb 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java @@ -48,31 +48,11 @@ public List get(SearchMatch match) throws CoreException { astParser.setSource(unit); astParser.setResolveBindings(true); CompilationUnit cu = (CompilationUnit) astParser.createAST(null); - cu.accept(new ASTVisitor() { - // we are only doing this for MethodInvocation right now - // look into MethodDeclaration if needed - public boolean visit(MethodInvocation node) { - try { - IMethodBinding binding = node.resolveMethodBinding(); - if (binding != null) { - // get fqn of the method being called - ITypeBinding declaringClass = binding.getDeclaringClass(); - if (declaringClass != null) { - String fullyQualifiedName = declaringClass.getQualifiedName() + "." + binding.getName(); - // match fqn with query pattern - if (fullyQualifiedName.matches(getCleanedQuery(query))) { - symbols.add(symbol); - } else { - logInfo("fqn " + fullyQualifiedName + " did not match with " + query); - } - } - } - } catch (Exception e) { - logInfo("error determining accuracy of match: " + e); - } - return true; - } - }); + CustomASTVisitor visitor = new CustomASTVisitor(query, match); + cu.accept(visitor); + if (visitor.symbolMatches()) { + symbols.add(symbol); + } } else { symbols.add(symbol); } diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java new file mode 100644 index 0000000..7251e70 --- /dev/null +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java @@ -0,0 +1,86 @@ +package io.konveyor.tackle.core.internal.symbol; + +import org.eclipse.jdt.core.dom.ASTVisitor; +import org.eclipse.jdt.core.dom.ASTNode; +import org.eclipse.jdt.core.dom.IMethodBinding; +import org.eclipse.jdt.core.dom.ITypeBinding; +import org.eclipse.jdt.core.dom.MethodInvocation; +import org.eclipse.jdt.core.search.SearchMatch; + +import static org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin.logInfo; + +public class CustomASTVisitor extends ASTVisitor { + private String query; + private SearchMatch match; + private boolean symbolMatches; + + + public CustomASTVisitor(String query, SearchMatch match) { + /* + * When comparing query pattern with an actual java element's fqn + * we need to make sure that * not preceded with a . are replaced + * by .* so that java regex works as expected on them + */ + this.query = query.replaceAll("(? get(SearchMatch match) { astParser.setSource(unit); astParser.setResolveBindings(true); CompilationUnit cu = (CompilationUnit) astParser.createAST(null); - cu.accept(new ASTVisitor() { - // we are only doing this for MethodInvocation right now - // look into MethodDeclaration if needed - public boolean visit(MethodInvocation node) { - try { - IMethodBinding binding = node.resolveMethodBinding(); - if (binding != null) { - // get fqn of the method being called - ITypeBinding declaringClass = binding.getDeclaringClass(); - if (declaringClass != null) { - String fullyQualifiedName = declaringClass.getQualifiedName() + "." + binding.getName(); - // match fqn with query pattern - if (fullyQualifiedName.matches(getCleanedQuery(query))) { - symbols.add(symbol); - } else { - logInfo("fqn " + fullyQualifiedName + " did not match with " + query); - } - } - } - } catch (Exception e) { - logInfo("error determining accuracy of match: " + e); - } - return true; - } - }); + CustomASTVisitor visitor = new CustomASTVisitor(query, match); + cu.accept(visitor); + if (visitor.symbolMatches()) { + symbols.add(symbol); + } } else { symbols.add(symbol); } diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java index e3331f2..382574b 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java @@ -167,13 +167,4 @@ private static void setPosition(Position position, int[] coords) { position.setLine(coords[0]); position.setCharacter(coords[1]); } - - /* - * When comparing query pattern with an actual found java element's fqn - * we need to make sure that * that are not preceded with a . are replaced - * by .* so that java regex works as expected on them - */ - default String getCleanedQuery(String query) { - return query.replaceAll("(? Date: Fri, 5 Jul 2024 13:17:44 -0400 Subject: [PATCH 4/8] improve accuracy of METHOD_CALL and CONSTRUCTOR_CALL Signed-off-by: Pranav Gaikwad --- .github/workflows/global-ci.yaml | 26 ++-- .../symbol/ConstructorCallSymbolProvider.java | 36 +++-- .../internal/symbol/CustomASTVisitor.java | 134 +++++++++++++++--- .../symbol/MethodCallSymbolProvider.java | 37 +++-- .../core/internal/symbol/SymbolProvider.java | 66 +++++++++ 5 files changed, 243 insertions(+), 56 deletions(-) diff --git a/.github/workflows/global-ci.yaml b/.github/workflows/global-ci.yaml index 84f8882..04d9ed3 100644 --- a/.github/workflows/global-ci.yaml +++ b/.github/workflows/global-ci.yaml @@ -18,14 +18,17 @@ jobs: else echo "::set-output name=ref::refs/pull/$PULL_REQUEST_NUMBER/merge" fi + - name: checkout uses: actions/checkout@v3 + - name: Checkout tools repo uses: actions/checkout@v3 with: repository: konveyor/analyzer-lsp path: analyzer-lsp ref: "${{ steps.extract_analyzer_pull_request_number.outputs.ref }}" + - uses: actions/checkout@v3 with: fetch-depth: 0 @@ -35,24 +38,23 @@ jobs: - name: Build bundle base image run: | - docker build -t quay.io/konveyor/jdtls-server-base:latest . - - name: build analyzer-lsp Dockerfile - run: | - docker build -f analyzer-lsp/Dockerfile -t quay.io/konveyor/analyzer-lsp:latest analyzer-lsp - docker tag quay.io/konveyor/analyzer-lsp:latest analyzer-lsp - - name: Build addon and save image - working-directory: tackle2-addon-analyzer + podman build -t quay.io/konveyor/jdtls-server-base:latest . + + - name: build java provider and save image + working-directory: analyzer-lsp run: | - IMG=quay.io/konveyor/tackle2-addon-analyzer:latest make image-podman - podman save -o /tmp/tackle2-addon-analyzer.tar quay.io/konveyor/tackle2-addon-analyzer:latest + make build-java-provider + podman tag java-provider quay.io/konveyor/java-external-provider:latest + podman save -o /tmp/java-provider.tar quay.io/konveyor/java-external-provider:latest + - name: Upload image as artifact uses: actions/upload-artifact@v3 with: - name: tackle2-addon-analyzer - path: /tmp/tackle2-addon-analyzer.tar + name: java-provider + path: /tmp/java-provider.tar retention-days: 1 e2e: needs: build-addon uses: konveyor/ci/.github/workflows/global-ci.yml@main with: - component_name: tackle2-addon-analyzer + component_name: java-provider diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java index 1bf8cdb..6fa9fee 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/ConstructorCallSymbolProvider.java @@ -6,21 +6,22 @@ import java.util.List; import org.eclipse.core.runtime.CoreException; +import org.eclipse.jdt.core.IClassFile; import org.eclipse.jdt.core.ICompilationUnit; +import org.eclipse.jdt.core.IJavaElement; import org.eclipse.jdt.core.IMethod; import org.eclipse.jdt.core.dom.AST; import org.eclipse.jdt.core.dom.ASTParser; -import org.eclipse.jdt.core.dom.ASTVisitor; import org.eclipse.jdt.core.dom.CompilationUnit; -import org.eclipse.jdt.core.dom.IMethodBinding; -import org.eclipse.jdt.core.dom.ITypeBinding; -import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.search.MethodReferenceMatch; import org.eclipse.jdt.core.search.SearchMatch; import org.eclipse.jdt.internal.core.JavaElement; +import org.eclipse.lsp4j.Location; import org.eclipse.lsp4j.SymbolInformation; import org.eclipse.lsp4j.SymbolKind; +import io.konveyor.tackle.core.internal.symbol.CustomASTVisitor.QueryLocation; + public class ConstructorCallSymbolProvider implements SymbolProvider, WithQuery { public String query; @@ -32,6 +33,7 @@ public List get(SearchMatch match) throws CoreException { MethodReferenceMatch m = (MethodReferenceMatch) match; var mod = (IMethod) m.getElement(); SymbolInformation symbol = new SymbolInformation(); + Location location = getLocation(mod, match); symbol.setName(mod.getElementName()); // If the search match is for a constructor, the enclosing element may not be a constructor. if (m.isConstructor()) { @@ -41,17 +43,25 @@ public List get(SearchMatch match) throws CoreException { return null; } symbol.setContainerName(mod.getParent().getElementName()); - symbol.setLocation(getLocation(mod, match)); + symbol.setLocation(location); if (this.query.contains(".")) { ICompilationUnit unit = mod.getCompilationUnit(); - ASTParser astParser = ASTParser.newParser(AST.getJLSLatest()); - astParser.setSource(unit); - astParser.setResolveBindings(true); - CompilationUnit cu = (CompilationUnit) astParser.createAST(null); - CustomASTVisitor visitor = new CustomASTVisitor(query, match); - cu.accept(visitor); - if (visitor.symbolMatches()) { - symbols.add(symbol); + if (unit == null) { + IClassFile cls = (IClassFile) ((IJavaElement) mod).getAncestor(IJavaElement.CLASS_FILE); + if (cls != null) { + unit = cls.becomeWorkingCopy(null, null, null); + } + } + if (this.queryQualificationMatches(this.query, 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.CONSTRUCTOR_CALL); + cu.accept(visitor); + if (visitor.symbolMatches()) { + symbols.add(symbol); + } } } else { symbols.add(symbol); diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java index 7251e70..33568b4 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/CustomASTVisitor.java @@ -1,6 +1,8 @@ package io.konveyor.tackle.core.internal.symbol; 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.ASTNode; import org.eclipse.jdt.core.dom.IMethodBinding; import org.eclipse.jdt.core.dom.ITypeBinding; @@ -9,13 +11,30 @@ import static org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin.logInfo; +/* + * SearchEngine we use often gives us more matches than needed when + * query contains a * and/or contains a fqn. e.g. java.io.paths.get* + * This class exists to help us get accurate results for such queries + * For different type of symbols we get from a match, we try to + * get fqn of that symbol and ensure it matches the given query + * (pgaikwad): if you can, please make the visit() functions DRYer + */ public class CustomASTVisitor extends ASTVisitor { private String query; private SearchMatch match; private boolean symbolMatches; + private QueryLocation location; + /* + * we re-use this same class for different locations in a query + * we should only be visiting nodes specific to a location, not all + */ + public enum QueryLocation { + METHOD_CALL, + CONSTRUCTOR_CALL, + } - public CustomASTVisitor(String query, SearchMatch match) { + public CustomASTVisitor(String query, SearchMatch match, QueryLocation location) { /* * When comparing query pattern with an actual java element's fqn * we need to make sure that * not preceded with a . are replaced @@ -24,16 +43,19 @@ public CustomASTVisitor(String query, SearchMatch match) { this.query = query.replaceAll("(? get(SearchMatch match) { MethodReferenceMatch m = (MethodReferenceMatch) match; IMethod e = (IMethod) m.getElement(); SymbolInformation symbol = new SymbolInformation(); + Location location = getLocation((IJavaElement) match.getElement(), match); symbol.setName(e.getElementName()); symbol.setKind(convertSymbolKind(e)); symbol.setContainerName(e.getParent().getElementName()); - symbol.setLocation(getLocation(e, match)); + symbol.setLocation(location); if (this.query.contains(".")) { ICompilationUnit unit = e.getCompilationUnit(); - ASTParser astParser = ASTParser.newParser(AST.getJLSLatest()); - astParser.setSource(unit); - astParser.setResolveBindings(true); - CompilationUnit cu = (CompilationUnit) astParser.createAST(null); - CustomASTVisitor visitor = new CustomASTVisitor(query, match); - cu.accept(visitor); - if (visitor.symbolMatches()) { - symbols.add(symbol); + if (unit == null) { + IClassFile cls = (IClassFile) ((IJavaElement) e).getAncestor(IJavaElement.CLASS_FILE); + if (cls != null) { + unit = cls.becomeWorkingCopy(null, null, null); + } + } + if (this.queryQualificationMatches(this.query, 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.METHOD_CALL); + cu.accept(visitor); + if (visitor.symbolMatches()) { + symbols.add(symbol); + } } } else { symbols.add(symbol); diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java index 382574b..7eac7c5 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java @@ -12,12 +12,15 @@ import org.eclipse.jdt.core.IClassFile; import org.eclipse.jdt.core.ICompilationUnit; import org.eclipse.jdt.core.IField; +import org.eclipse.jdt.core.IImportDeclaration; import org.eclipse.jdt.core.IJavaElement; import org.eclipse.jdt.core.IMethod; import org.eclipse.jdt.core.IOpenable; +import org.eclipse.jdt.core.IPackageDeclaration; import org.eclipse.jdt.core.IType; import org.eclipse.jdt.core.JavaModelException; import org.eclipse.jdt.core.SourceRange; +import org.eclipse.jdt.core.search.MethodReferenceMatch; import org.eclipse.jdt.core.search.SearchMatch; import org.eclipse.jdt.ls.core.internal.JDTUtils; import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin; @@ -167,4 +170,67 @@ private static void setPosition(Position position, int[] coords) { position.setLine(coords[0]); position.setCharacter(coords[1]); } + + /* + * Given a query, class and location of a Match, tells whether CompilationUnit of the match + * matches the qualification part of the query. For example, if the query is `konveyor.io.Util.get*`, + * qualification means `konveyor.io.Util`. This is so that we can improve accuracy of a match of + * queries that are looking for FQNs. For query `konveyor.io.Util.get*`, returns true if either one is true: + * 1. match is found in the package `konveyor.io` or class `konveyor.io.Util` + * 2. the compilation unit imports package `konveyor.io.Util` or `konveyor.io.*` + * 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) { + String queryQualification = ""; + int dotIndex = query.lastIndexOf('.'); + if (dotIndex > 0) { + // for a query, java.io.paths.File*, queryQualification is java.io.paths + queryQualification = query.substring(0, dotIndex); + } + // check if the match was found in the same package as the query was looking for + if (queryQualification != "" && location.getUri().contains(queryQualification.replaceAll(".", "/"))) { + return true; + } + if (unit != null) { + try { + // check if the package declaration on the unit matches query + for (IPackageDeclaration packageDecl : unit.getPackageDeclarations()) { + if (queryQualification != "" && packageDecl.getElementName().matches(queryQualification)) { + return true; + } + } + for (IImportDeclaration importDecl : unit.getImports()) { + String importElement = importDecl.getElementName(); + String importQualification = ""; + int importDotIndex = query.lastIndexOf('.'); + if (importDotIndex > 0) { + importQualification = query.substring(0, dotIndex); + } + // import can be absolute like java.io.paths.FileReader + if (query.matches(importElement)) { + return true; + } + if (importElement.matches(query)) { + return true; + } + // an import can be java.io.paths.* or java.io.* + if (importElement.contains("*")) { + if (queryQualification != "") { + // query is java.io.paths.File*, import is java.io.paths.* + if (queryQualification.startsWith(importQualification)) { + return true; + } + if (importElement.replaceAll(".*", "").matches(queryQualification)) { + return true; + } + } + } + } + } catch (Exception e) { + logInfo("unable to determine accuracy of the match"); + } + } + return false; + } } From f9b679c48a6046f3909aeba899e3350ae11a9732 Mon Sep 17 00:00:00 2001 From: Pranav Gaikwad Date: Mon, 8 Jul 2024 12:27:22 -0400 Subject: [PATCH 5/8] :bug: add a way to specify api tests ref Signed-off-by: Pranav Gaikwad --- .github/workflows/global-ci.yaml | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/.github/workflows/global-ci.yaml b/.github/workflows/global-ci.yaml index 04d9ed3..76c680d 100644 --- a/.github/workflows/global-ci.yaml +++ b/.github/workflows/global-ci.yaml @@ -6,17 +6,26 @@ jobs: build-addon: name: Build tackle2-addon-analyzer runs-on: ubuntu-20.04 + outputs: + api_tests_ref: ${{ steps.extract_info.outputs.API_TESTS_REF }} strategy: fail-fast: false steps: - name: Extract pull request number from PR description - id: extract_analyzer_pull_request_number + id: extract_info run: | - PULL_REQUEST_NUMBER=$(echo "${{ github.event.pull_request.body }}" | grep -oP '[A|a]nalyzer.?[P|p][R|r]: \K\d+' || true) - if [ -z "$PULL_REQUEST_NUMBER" ]; then - echo "::set-output name=ref::main" + ANALYZER_PR=$(echo "${{ github.event.pull_request.body }}" | grep -oP '[A|a]nalyzer.?[P|p][R|r]: \K\d+' || true) + if [ -z "$ANALYZER_PR" ]; then + echo "ANALYZER_REF=${GITHUB_BASE_REF}" >> $GITHUB_OUTPUT else - echo "::set-output name=ref::refs/pull/$PULL_REQUEST_NUMBER/merge" + echo "ANALYZER_REF=refs/pull/$ANALYZER_PR/merge" >> $GITHUB_OUTPUT + fi + + API_TESTS_PR=$(echo "${{ github.event.pull_request.body }}" | grep -oP '[A|a]pi *[T|t]ests *[P|p][R|r]: \K\d+' || true) + if [ -z "$API_TESTS_PR" ]; then + echo "API_TESTS_REF=${GITHUB_BASE_REF}" >> $GITHUB_OUTPUT + else + echo "API_TESTS_REF=refs/pull/$API_TESTS_PR/merge" >> $GITHUB_OUTPUT fi - name: checkout @@ -27,7 +36,7 @@ jobs: with: repository: konveyor/analyzer-lsp path: analyzer-lsp - ref: "${{ steps.extract_analyzer_pull_request_number.outputs.ref }}" + ref: "${{ steps.extract_info.outputs.ANALYZER_REF }}" - uses: actions/checkout@v3 with: @@ -58,3 +67,4 @@ jobs: uses: konveyor/ci/.github/workflows/global-ci.yml@main with: component_name: java-provider + api_tests_ref: "${{ needs.build-addon.outputs.api_tests_ref }}" From b46a3bc1cb1c4280aaa790ca658c8b2d67531c55 Mon Sep 17 00:00:00 2001 From: Pranav Gaikwad Date: Tue, 9 Jul 2024 14:39:58 -0400 Subject: [PATCH 6/8] :bug: fix query that ends in a * Signed-off-by: Pranav Gaikwad --- .../io/konveyor/tackle/core/internal/symbol/SymbolProvider.java | 1 + 1 file changed, 1 insertion(+) diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java index 7eac7c5..620cc8d 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java @@ -182,6 +182,7 @@ private static void setPosition(Position position, int[] coords) { * 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) { + query = query.replaceAll("(? 0) { From 1fa6d30238b0c02d75810e95781db79b720826bb Mon Sep 17 00:00:00 2001 From: Shawn Hurley Date: Tue, 9 Jul 2024 14:38:36 -0400 Subject: [PATCH 7/8] Adding query replace to make java regex in qualification match, bumping LS Signed-off-by: Shawn Hurley --- Dockerfile | 2 +- .../konveyor/tackle/core/internal/symbol/SymbolProvider.java | 2 +- java-analyzer-bundle.tp/java-analyzer-bundle.target | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Dockerfile b/Dockerfile index e6e5169..813e0eb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,6 @@ FROM registry.access.redhat.com/ubi9/ubi AS jdtls-download WORKDIR /jdtls -RUN curl -s -o jdtls.tar.gz https://download.eclipse.org/jdtls/milestones/1.16.0/jdt-language-server-1.16.0-202209291445.tar.gz &&\ +RUN curl -s -o jdtls.tar.gz https://download.eclipse.org/jdtls/milestones/1.36.0/jdt-language-server-1.36.0-202405301306.tar.gz &&\ tar -xvf jdtls.tar.gz --no-same-owner &&\ chmod 755 /jdtls/bin/jdtls &&\ rm -rf jdtls.tar.gz diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java index 7eac7c5..15e0ad4 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java @@ -20,7 +20,6 @@ import org.eclipse.jdt.core.IType; import org.eclipse.jdt.core.JavaModelException; import org.eclipse.jdt.core.SourceRange; -import org.eclipse.jdt.core.search.MethodReferenceMatch; import org.eclipse.jdt.core.search.SearchMatch; import org.eclipse.jdt.ls.core.internal.JDTUtils; import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin; @@ -192,6 +191,7 @@ default boolean queryQualificationMatches(String query, ICompilationUnit unit, L if (queryQualification != "" && location.getUri().contains(queryQualification.replaceAll(".", "/"))) { return true; } + query = query.replaceAll("(? - + @@ -61,7 +61,7 @@ - + From 0ec991d901e3c4033467fad982006f77887e5700 Mon Sep 17 00:00:00 2001 From: Pranav Gaikwad Date: Tue, 9 Jul 2024 14:50:05 -0400 Subject: [PATCH 8/8] update Signed-off-by: Pranav Gaikwad --- .../io/konveyor/tackle/core/internal/symbol/SymbolProvider.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java index 46c513f..f69ad34 100644 --- a/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java +++ b/java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/SymbolProvider.java @@ -192,7 +192,6 @@ default boolean queryQualificationMatches(String query, ICompilationUnit unit, L if (queryQualification != "" && location.getUri().contains(queryQualification.replaceAll(".", "/"))) { return true; } - query = query.replaceAll("(?