Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🐛 check accuracy of TypeReferenceMatch & MethodReferenceMatch #91

Merged
merged 3 commits into from
Mar 28, 2024
Merged

🐛 check accuracy of TypeReferenceMatch & MethodReferenceMatch #91

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
643849a
:bug: check accuracy of TypeReferenceMatch
pranavgaikwad Mar 27, 2024
7bcb27e
:bug: fix accuracy of method call searches
pranavgaikwad Mar 28, 2024
4fa8b8c
:bug: minor fix
pranavgaikwad Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...dle.core/src/main/java/io/konveyor/tackle/core/internal/SampleDelegateCommandHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@ private static SearchPattern getPatternSingleQuery(int location, String query) t
case 7:
case 9:
return SearchPattern.createPattern(query, IJavaSearchConstants.TYPE, IJavaSearchConstants.REFERENCES, pattern);
case 2:
case 2:
if (query.contains(".")) {
return SearchPattern.createPattern(query, IJavaSearchConstants.METHOD, IJavaSearchConstants.QUALIFIED_REFERENCE, SearchPattern.R_PATTERN_MATCH | SearchPattern.R_ERASURE_MATCH);
}
// Switched back to referenced
return SearchPattern.createPattern(query, IJavaSearchConstants.METHOD, IJavaSearchConstants.REFERENCES, SearchPattern.R_PATTERN_MATCH | SearchPattern.R_ERASURE_MATCH);
case 3:
Expand Down
24 changes: 0 additions & 24 deletions ...e.core/src/main/java/io/konveyor/tackle/core/internal/SymbolInformationTypeRequestor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,29 +63,6 @@ public void acceptSearchMatch(SearchMatch match) throws CoreException {

}

var e = (IJavaElement) match.getElement();
if (shouldCheckAccuracy(e)) {

if ((!this.query.contains("?") && !this.query.contains("*")) && match.getAccuracy() == SearchMatch.A_INACCURATE) {

//TODO: This is a hack, this will give use some clue of what we are looking at, if the search is exact then this should match
// I don't love this, but seems to be the right way
logInfo("attempting: " + e.getHandleIdentifier());
// Adding specific case for annotations, they will always be inaccurrate.
if (!e.getHandleIdentifier().contains(query) && !(this.symbolKind == 4 || this.symbolKind == 5 || this.symbolKind == 1 || this.symbolKind == 3)) {
logInfo("exact match is looking for accurate results" + match);
return;
}
}

if ((this.query.contains("?") && (this.query.contains("(") || this.query.contains(")"))) && match.getAccuracy() == SearchMatch.A_INACCURATE) {
if(!this.query.contains("*")) {
logInfo("exact match is looking for accurate results " + match);
return;
}
}
}

SymbolProvider symbolProvider = SymbolProviderResolver.resolve(this.symbolKind, match);
if (symbolProvider instanceof WithQuery) {
((WithQuery) symbolProvider).setQuery(this.query);
Expand All @@ -97,7 +74,6 @@ public void acceptSearchMatch(SearchMatch match) throws CoreException {
logInfo("getting match: " + match + "with provider: " + symbolProvider);
List<SymbolInformation> symbols = Optional.ofNullable(symbolProvider.get(match)).orElse(new ArrayList<>());
this.symbols.addAll(symbols);

}

public List<SymbolInformation> getSymbols() {
Expand Down
12 changes: 2 additions & 10 deletions ....core/src/main/java/io/konveyor/tackle/core/internal/symbol/MethodCallSymbolProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

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.search.MethodReferenceMatch;
import org.eclipse.jdt.core.search.SearchMatch;
import org.eclipse.lsp4j.SymbolInformation;
Expand All @@ -22,16 +24,6 @@ public List<SymbolInformation> get(SearchMatch match) {
// For Method Calls we will need to do the local variable trick
try {
MethodReferenceMatch m = (MethodReferenceMatch) match;

// Default to filter to only accurate matches
var filterOut = m.getAccuracy() != SearchMatch.A_ACCURATE;
if (query.contains("*")) {
filterOut = false;
}

if (filterOut) {
return symbols;
}
IMethod e = (IMethod) m.getElement();
SymbolInformation symbol = new SymbolInformation();
symbol.setName(e.getElementName());
Expand Down
87 changes: 80 additions & 7 deletions ...bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/TypeSymbolProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,95 @@
import java.util.ArrayList;
import java.util.List;

import org.eclipse.core.runtime.URIUtil;
import org.eclipse.jdt.core.IClassFile;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IImportDeclaration;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.IPackageDeclaration;
import org.eclipse.jdt.core.search.SearchMatch;
import org.eclipse.jdt.core.search.TypeDeclarationMatch;
import org.eclipse.jdt.core.search.TypeParameterDeclarationMatch;
import org.eclipse.jdt.core.search.TypeParameterReferenceMatch;
import org.eclipse.jdt.core.search.TypeReferenceMatch;
import org.eclipse.lsp4j.Location;
import org.eclipse.lsp4j.SymbolInformation;
import org.eclipse.lsp4j.SymbolKind;

public class TypeSymbolProvider implements SymbolProvider {
public class TypeSymbolProvider implements SymbolProvider, WithQuery {
private String query;

@Override
public List<SymbolInformation> get(SearchMatch match) {
SymbolKind k = convertSymbolKind((IJavaElement) match.getElement());
List<SymbolInformation> symbols = new ArrayList<>();
// For Method Calls we will need to do the local variable trick
if (!(match instanceof TypeReferenceMatch ||
match instanceof TypeDeclarationMatch ||
match instanceof TypeParameterDeclarationMatch ||
match instanceof TypeParameterReferenceMatch)) {
return null;
if (!(match instanceof TypeReferenceMatch ||
match instanceof TypeDeclarationMatch ||
match instanceof TypeParameterDeclarationMatch ||
match instanceof TypeParameterReferenceMatch)) {
return null;
}
// TypeReferenceMatch often are inaccurate in that if a pattern is a.b.C we get
// matches for references of C no matter whether they are in package a.b or not
// we try to confirm whether C we are getting belongs to package a.b with checks:
// first, we check if the file belongs to package a.b
// second, we check if CompilationUnit has package declaration of a.b
// third, we check if CompilationUnit has explicit import of a.b.C or a.b.*
if (match instanceof TypeReferenceMatch && this.query.contains(".")) {
try {
String qualification = "";
int dotIndex = this.query.lastIndexOf('.');
if (dotIndex > 0) {
qualification = this.query.substring(0, dotIndex);
}
var element = (IJavaElement) match.getElement();
ICompilationUnit compilationUnit = (ICompilationUnit) element
.getAncestor(IJavaElement.COMPILATION_UNIT);
if (compilationUnit == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my own understanding when does this happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understood it correctly, this happens when match is in a .class file. In that case, the compilation unit needs to be loaded into memory. Fortunately, the class file doesn't need to be decompiled....we can somehow get imports, package declaration, methods etc just from the class file handle

IClassFile cls = (IClassFile) element.getAncestor(IJavaElement.CLASS_FILE);
if (cls != null) {
// TODO: make sure following doesn't affect performance
compilationUnit = cls.becomeWorkingCopy(null, null, null);
}
}
boolean isAccurate = false;
Location location = getLocation((IJavaElement) match.getElement(), match);
// if the file is in the same package as the query
// there's a high chance its an accurate match
if (qualification != "" && location.getUri().contains(qualification.replaceAll(".", "/"))) {
isAccurate = true;
}
if (compilationUnit != null && !isAccurate) {
// if the file contains package declaration that matches the query, then type
// can be referenced without its fully qualified name
for (IPackageDeclaration packageDecl : compilationUnit.getPackageDeclarations()) {
if (qualification != "" && packageDecl.getElementName().matches(qualification)) {
isAccurate = true;
}
}
// if the file contains explicit imports for the fully qualified name
// or a .* import with partial qualified name then type must be accurate
if (isAccurate) {
for (IImportDeclaration importDecl : compilationUnit.getImports()) {
String importElement = importDecl.getElementName();
if (importElement.matches(this.query)) {
isAccurate = true;
}
if (qualification != "" &&
importElement.replaceAll(".*", "").matches(qualification)) {
isAccurate = true;
}
}
}
}
if (!isAccurate) {
return null;
}
} catch (Exception e) {
logInfo("failed to determine accuracy of TypeReferenceMatch accepting.." + match);
}
}
try {
var mod = (IJavaElement) match.getElement();
SymbolInformation symbol = new SymbolInformation();
Expand All @@ -34,7 +102,6 @@ public List<SymbolInformation> get(SearchMatch match) {
symbol.setContainerName(mod.getParent().getElementName());
symbol.setLocation(getLocation((IJavaElement) match.getElement(), match));
symbols.add(symbol);


} catch (Exception e) {
logInfo("Unable to convert for TypeSymbolProvider: " + e);
Expand All @@ -43,4 +110,10 @@ public List<SymbolInformation> get(SearchMatch match) {

return symbols;
}

@Override
public void setQuery(String query) {
// TODO Auto-generated method stub
this.query = query;
}
}
Loading