From 3b8cb13d2ff42f0ece8e99c2e73c1f9f7c44f207 Mon Sep 17 00:00:00 2001
From: Pranav Gaikwad <pgaikwad@redhat.com>
Date: Tue, 2 Jul 2024 22:45:59 -0400
Subject: [PATCH] :bug: improve accuracy of MethodReference matches (#97)

* :bug: improve accuracy of MethodReference matches

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

* improve accuracy of constructor call

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

* :ghost: improve logic / structure a bit

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

---------

Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com>
---
 .../symbol/ConstructorCallSymbolProvider.java | 33 ++++++-
 .../internal/symbol/CustomASTVisitor.java     | 86 +++++++++++++++++++
 .../symbol/MethodCallSymbolProvider.java      | 23 ++++-
 3 files changed, 139 insertions(+), 3 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 2b016b1..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
@@ -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<SymbolInformation> get(SearchMatch match) throws CoreException {
         List<SymbolInformation> symbols = new ArrayList<>();
@@ -32,11 +42,30 @@ public List<SymbolInformation> 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);
+                CustomASTVisitor visitor = new CustomASTVisitor(query, match);
+                cu.accept(visitor);
+                if (visitor.symbolMatches()) {
+                    symbols.add(symbol);
+                }
+            } 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/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("(?<!\\.)\\*", ".*");
+        this.symbolMatches = false;
+        this.match = match;
+    }
+
+    /*
+     * 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.
+     */
+    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
+     * returning false stops further visits
+    */
+    @Override
+    public boolean visit(MethodInvocation node) {
+        try {
+            if (!this.shouldVisit(node)) {
+                return true;
+            }
+            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(this.query)) {
+                        this.symbolMatches = true;
+                        return false;
+                    } else {
+                        logInfo("method fqn " + fullyQualifiedName + " did not match with " + query);
+                        return true;
+                    }
+                }
+            } 
+            // 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.symbolMatches = true;
+            return false;
+        }
+    }
+
+    public boolean symbolMatches() {
+        return this.symbolMatches;
+    }
+}
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..0e1d625 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;
@@ -30,7 +38,20 @@ public List<SymbolInformation> get(SearchMatch match) {
             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);
+                CustomASTVisitor visitor = new CustomASTVisitor(query, match);
+                cu.accept(visitor);
+                if (visitor.symbolMatches()) {
+                    symbols.add(symbol);
+                }
+            } else {
+                symbols.add(symbol);
+            }
         } catch (Exception e) {
             logInfo("unable to convert for variable: " + e);
         }