From 85c04dffd73567cf4d084810ab8ddeb213873ac4 Mon Sep 17 00:00:00 2001 From: Pranav Gaikwad Date: Fri, 5 Jul 2024 13:17:44 -0400 Subject: [PATCH] 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 | 59 ++++++++ 5 files changed, 236 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..4047f3f 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,60 @@ 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) { + 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); + } + // for a query, java.io.paths.File*, query qualification is java.io.paths + if (queryQualification != "") { + // an import can be java.io.paths.* or java.io.* + // in that case, importQualification is java.io.paths or java.io + if (importElement.contains("*") && + queryQualification.startsWith(importQualification)) { + return true; + } + // import can be absolute like java.io.path.FileReader + if (query.matches(importElement)) { + return true; + } + } + } + } catch (Exception e) { + logInfo("unable to determine accuracy of the match"); + } + } + return false; + } }