improve accuracy of METHOD_CALL and CONSTRUCTOR_CALL

Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com>

.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

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<SymbolInformation> 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<SymbolInformation> 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);

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,34 +43,36 @@ public CustomASTVisitor(String query, SearchMatch match) {
         this.query = query.replaceAll("(?<!\\.)\\*", ".*");
         this.symbolMatches = false;
         this.match = match;
+        // depending on which location the query was for we only want to
+        // visit certain nodes
+        this.location = location;
     }
 
     /*
-     * When visiting AST nodes, it may happen that we visit more nodes
-     * than needed. We need to ensure that we are only visiting ones
-     * that are found in the given search match. I wrote this for methods
-     * where I observed that a node starts at the beginning of line whereas match
-     * starts at an offset within that line. However, both end on the same position.
-     * This could differ for other locations. In that case, change logic based
-     * on type of the node you get.
+     * When visiting AST nodes, it may happen that we visit more nodes than
+     * needed. We need to ensure that we are only visiting ones that are found
+     * in the given search match. I wrote this for methods / constructors where 
+     * I observed that node starts at the beginning of line whereas match starts 
+     * at an offset within that line. However, both end on the same position. This 
+     * could differ for other locations. In that case, change logic based on type of
+     * the node you get.
      */
     private boolean shouldVisit(ASTNode node) {
         return (this.match.getOffset() + this.match.getLength()) == 
             (node.getStartPosition() + node.getLength());
     }
 
     /* 
-     * This is to get information from a MethodInvocation
-     * used for METHOD_CALL and CONSTRUCTOR_CALL matches
-     * we only discard a match only when we can tell for sure
+     * 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
      * returning false stops further visits
     */
     @Override
     public boolean visit(MethodInvocation node) {
+        if (this.location != QueryLocation.METHOD_CALL || !this.shouldVisit(node)) {
+            return true;
+        }
         try {
-            if (!this.shouldVisit(node)) {
-                return true;
-            }
             IMethodBinding binding = node.resolveMethodBinding();
             if (binding != null) {
                 // get fqn of the method being called
@@ -67,18 +88,99 @@ public boolean visit(MethodInvocation node) {
                         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("error visiting MethodInvocation node: " + e);
-            // this is so that we fallback to old approach and we dont lose a result
+            // 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 ConstructorInvocation, used for CONSTRUCTOR_CALL
+     * we discard a match only when we can tell for sure. otherwise we accept
+     * returning false stops further visits
+    */
+    @Override
+    public boolean visit(ConstructorInvocation node) {
+        if (this.location != QueryLocation.CONSTRUCTOR_CALL || !this.shouldVisit(node)) {
+            return true;
+        }
+        try {
+            IMethodBinding binding = node.resolveConstructorBinding();
+            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(this.query)) {
+                        this.symbolMatches = true;
+                        return false;
+                    } else {
+                        logInfo("constructor fqn " + fullyQualifiedName + " did not match with " + query);
+                        return true;
+                    }
+                }
+            }
+            logInfo("failed to get accurate info for ConstructorInvocation, 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("error visiting ConstructorInvocation 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 ClassInstanceCreation, used for CONSTRUCTOR_CALL
+     * we discard a match only when we can tell for sure. otherwise we accept
+     * returning false stops further visits
+    */
+    @Override
+    public boolean visit(ClassInstanceCreation node) {
+        if (this.location != QueryLocation.CONSTRUCTOR_CALL || !this.shouldVisit(node)) {
+            return true;
+        }
+        try {
+            IMethodBinding binding = node.resolveConstructorBinding();
+            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(this.query)) {
+                        this.symbolMatches = true;
+                        return false;
+                    } else {
+                        logInfo("constructor fqn " + fullyQualifiedName + " did not match with " + query);
+                        return true;
+                    }
+                }
+            }
+            logInfo("failed to get accurate info for ClassInstanceCreation, 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("error visiting ConstructorInvocation node: " + e);
+            // this is so that we fallback and don't lose a match when we fail
+            this.symbolMatches = true;
+            return false;
+        }    
+    }
 
     public boolean symbolMatches() {
         return this.symbolMatches;

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

@@ -5,23 +5,21 @@
 import java.util.ArrayList;
 import java.util.List;
 
+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.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.Location;
 import org.eclipse.lsp4j.SymbolInformation;
 import org.eclipse.lsp4j.SymbolKind;
 
+import io.konveyor.tackle.core.internal.symbol.CustomASTVisitor.QueryLocation;
+
 public class MethodCallSymbolProvider implements SymbolProvider, WithQuery {
     private String query;
 
@@ -34,20 +32,29 @@ public List<SymbolInformation> 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);