From c89a5187264bd93c85c5cdb3060d289a72399aeb Mon Sep 17 00:00:00 2001 From: George Fraser Date: Wed, 2 Jan 2019 23:24:55 -0800 Subject: Allow virtual references and definitions --- src/main/java/org/javacs/CompileBatch.java | 190 +++++++++++++-------- src/main/java/org/javacs/CompileFile.java | 68 -------- src/main/java/org/javacs/JavaCompilerService.java | 12 ++ src/main/java/org/javacs/JavaLanguageServer.java | 62 ++----- src/main/java/org/javacs/Profiler.java | 1 + src/test/java/org/javacs/FindReferencesTest.java | 6 + src/test/java/org/javacs/GotoTest.java | 48 +++--- .../java/org/javacs/JavaCompilerServiceTest.java | 43 ----- .../src/org/javacs/example/GotoImplementation.java | 16 ++ 9 files changed, 202 insertions(+), 244 deletions(-) create mode 100644 src/test/test-project/workspace/src/org/javacs/example/GotoImplementation.java (limited to 'src') diff --git a/src/main/java/org/javacs/CompileBatch.java b/src/main/java/org/javacs/CompileBatch.java index 48a1536..60cd9a8 100644 --- a/src/main/java/org/javacs/CompileBatch.java +++ b/src/main/java/org/javacs/CompileBatch.java @@ -7,28 +7,21 @@ import java.net.URI; import java.nio.file.Files; import java.nio.file.Paths; import java.nio.file.attribute.FileTime; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.logging.Logger; import java.util.stream.Collectors; import javax.lang.model.element.*; -import javax.tools.Diagnostic; -import javax.tools.JavaFileObject; -import org.javacs.lsp.*; +import javax.lang.model.util.*; +import javax.tools.*; +import org.javacs.lsp.Range; public class CompileBatch { private final JavaCompilerService parent; private final ReportProgress progress; private final JavacTask task; private final Trees trees; + private final Elements elements; + private final Types types; private final List roots; CompileBatch(JavaCompilerService parent, Collection files, ReportProgress progress) { @@ -36,6 +29,8 @@ public class CompileBatch { this.progress = progress; this.task = batchTask(parent, files); this.trees = Trees.instance(task); + this.elements = task.getElements(); + this.types = task.getTypes(); this.roots = new ArrayList(); // Print timing information for optimization var profiler = new Profiler(); @@ -103,18 +98,126 @@ public class CompileBatch { throw new RuntimeException("File " + uri + " isn't in batch " + roots); } - public Optional path(Element e) { - return Optional.ofNullable(trees.getPath(e)); + public List definitions(Element el) { + LOG.info(String.format("Search for definitions of `%s` in %d files...", el, roots.size())); + + var refs = new ArrayList(); + class FindDefinitions extends TreePathScanner { + boolean sameSymbol(Element found) { + return el.equals(found); + } + + boolean isSubMethod(Element found) { + if (!(el instanceof ExecutableElement)) return false; + if (!(found instanceof ExecutableElement)) return false; + var superMethod = (ExecutableElement) el; + var subMethod = (ExecutableElement) found; + var subType = (TypeElement) subMethod.getEnclosingElement(); + return elements.overrides(subMethod, superMethod, subType); + } + + boolean isSubType(Element found) { + if (!(el instanceof TypeElement)) return false; + if (!(found instanceof TypeElement)) return false; + var superType = (TypeElement) el; + var subType = (TypeElement) found; + return types.isSubtype(subType.asType(), superType.asType()); + } + + void check(TreePath from) { + var found = trees.getElement(from); + var match = sameSymbol(found) || isSubMethod(found) || isSubType(found); + if (match) refs.add(from); + } + + @Override + public Void visitClass(ClassTree t, Void __) { + check(getCurrentPath()); + return super.visitClass(t, null); + } + + @Override + public Void visitMethod(MethodTree t, Void __) { + check(getCurrentPath()); + return super.visitMethod(t, null); + } + + @Override + public Void visitVariable(VariableTree t, Void __) { + check(getCurrentPath()); + return super.visitVariable(t, null); + } + } + var finder = new FindDefinitions(); + for (var r : roots) { + finder.scan(r, null); + ; + } + return refs; } public List references(Element to) { LOG.info(String.format("Search for references to `%s` in %d files...", to, roots.size())); - var result = new ArrayList(); - for (var f : roots) { - result.addAll(referencesToElement(f, to)); + var refs = new ArrayList(); + class FindReferences extends TreePathScanner { + boolean sameSymbol(Element found) { + return to.equals(found); + } + + boolean isSuperMethod(Element found) { + if (!(to instanceof ExecutableElement)) return false; + if (!(found instanceof ExecutableElement)) return false; + var subMethod = (ExecutableElement) to; + var subType = (TypeElement) subMethod.getEnclosingElement(); + var superMethod = (ExecutableElement) found; + return elements.overrides(subMethod, superMethod, subType); + } + + boolean isSuperType(Element found) { + if (!(to instanceof TypeElement)) return false; + if (!(found instanceof TypeElement)) return false; + var subType = (TypeElement) to; + var superType = (TypeElement) found; + return types.isSubtype(subType.asType(), superType.asType()); + } + + void check(TreePath from) { + var found = trees.getElement(from); + var match = sameSymbol(found) || isSuperMethod(found) || isSuperType(found); + if (match) refs.add(from); + } + + @Override + public Void visitMemberReference(MemberReferenceTree t, Void __) { + check(getCurrentPath()); + return super.visitMemberReference(t, null); + } + + @Override + public Void visitMemberSelect(MemberSelectTree t, Void __) { + check(getCurrentPath()); + return super.visitMemberSelect(t, null); + } + + @Override + public Void visitIdentifier(IdentifierTree t, Void __) { + check(getCurrentPath()); + return super.visitIdentifier(t, null); + } + + @Override + public Void visitNewClass(NewClassTree t, Void __) { + check(getCurrentPath()); + return super.visitNewClass(t, null); + } } - return result; + var finder = new FindReferences(); + for (var r : roots) { + finder.scan(r, null); + ; + } + return refs; } public Map countReferences() { @@ -160,61 +263,12 @@ public class CompileBatch { return Objects.equals(Objects.toString(left, ""), Objects.toString(right, "")); } - private boolean sameSymbol(Element from, Element to) { - return to != null - && from != null - && toStringEquals(to.getEnclosingElement(), from.getEnclosingElement()) - && toStringEquals(to, from); - } - private static boolean isField(Element to) { if (!(to instanceof VariableElement)) return false; var field = (VariableElement) to; return field.getEnclosingElement() instanceof TypeElement; } - private List referencesToElement(CompilationUnitTree root, Element to) { - var trees = Trees.instance(task); - var results = new ArrayList(); - class FindReferencesElement extends TreePathScanner { - void check(TreePath from) { - var found = trees.getElement(from); - if (sameSymbol(found, to)) { - results.add(from); - } - } - - @Override - public Void visitMemberReference(MemberReferenceTree t, Void __) { - check(getCurrentPath()); - return super.visitMemberReference(t, null); - } - - @Override - public Void visitMemberSelect(MemberSelectTree t, Void __) { - check(getCurrentPath()); - return super.visitMemberSelect(t, null); - } - - @Override - public Void visitIdentifier(IdentifierTree t, Void __) { - check(getCurrentPath()); - return super.visitIdentifier(t, null); - } - - @Override - public Void visitNewClass(NewClassTree t, Void __) { - check(getCurrentPath()); - return super.visitNewClass(t, null); - } - } - new FindReferencesElement().scan(root, null); - LOG.info( - String.format( - "...found %d references in %s", results.size(), Parser.fileName(root.getSourceFile().toUri()))); - return results; - } - static List index(Trees trees, CompilationUnitTree root) { var refs = new ArrayList(); class IndexFile extends TreePathScanner { diff --git a/src/main/java/org/javacs/CompileFile.java b/src/main/java/org/javacs/CompileFile.java index 7d7802c..35a685a 100644 --- a/src/main/java/org/javacs/CompileFile.java +++ b/src/main/java/org/javacs/CompileFile.java @@ -2,12 +2,8 @@ package org.javacs; import com.sun.source.tree.*; import com.sun.source.util.*; -import java.io.File; import java.io.IOException; import java.net.URI; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -16,7 +12,6 @@ import java.util.Objects; import java.util.Optional; import java.util.function.Predicate; import java.util.logging.Logger; -import java.util.regex.Pattern; import javax.lang.model.element.*; import org.javacs.lsp.*; @@ -292,68 +287,5 @@ public class CompileFile { }; } - public Optional declaringFile(Element e) { - var top = topLevelDeclaration(e); - if (!top.isPresent()) return Optional.empty(); - return findDeclaringFile(top.get()); - } - - private Optional topLevelDeclaration(Element e) { - if (e == null) return Optional.empty(); - var parent = e; - TypeElement result = null; - while (parent.getEnclosingElement() != null) { - if (parent instanceof TypeElement) result = (TypeElement) parent; - parent = parent.getEnclosingElement(); - } - return Optional.ofNullable(result); - } - - private boolean containsTopLevelDeclaration(Path file, String simpleClassName) { - var find = Pattern.compile("\\b(class|interface|enum) +" + simpleClassName + "\\b"); - try (var lines = Files.newBufferedReader(file)) { - var line = lines.readLine(); - while (line != null) { - if (find.matcher(line).find()) return true; - line = lines.readLine(); - } - } catch (IOException e) { - throw new RuntimeException(e); - } - return false; - } - - /** Find the file `e` was declared in */ - private Optional findDeclaringFile(TypeElement e) { - var name = e.getQualifiedName().toString(); - var lastDot = name.lastIndexOf('.'); - var packageName = lastDot == -1 ? "" : name.substring(0, lastDot); - var className = name.substring(lastDot + 1); - // First, look for a file named [ClassName].java - var packagePath = Paths.get(packageName.replace('.', File.separatorChar)); - var publicClassPath = packagePath.resolve(className + ".java"); - for (var root : parent.sourcePath) { - var absPath = root.resolve(publicClassPath); - if (Files.exists(absPath) && containsTopLevelDeclaration(absPath, className)) { - return Optional.of(absPath.toUri()); - } - } - // Then, look for a secondary declaration in all java files in the package - var isPublic = e.getModifiers().contains(Modifier.PUBLIC); - if (!isPublic) { - for (var root : parent.sourcePath) { - var absDir = root.resolve(packagePath); - try { - var foundFile = - Files.list(absDir).filter(f -> containsTopLevelDeclaration(f, className)).findFirst(); - if (foundFile.isPresent()) return foundFile.map(Path::toUri); - } catch (IOException err) { - throw new RuntimeException(err); - } - } - } - return Optional.empty(); - } - private static final Logger LOG = Logger.getLogger("main"); } diff --git a/src/main/java/org/javacs/JavaCompilerService.java b/src/main/java/org/javacs/JavaCompilerService.java index bba914c..86ef333 100644 --- a/src/main/java/org/javacs/JavaCompilerService.java +++ b/src/main/java/org/javacs/JavaCompilerService.java @@ -200,8 +200,20 @@ public class JavaCompilerService { return Parser.containsWord(file, name); } + public List potentialDefinitions(Element to) { + // TODO only methods and types can have multiple definitions + // TODO reduce number of files we need to check by parsing and eliminating more cases + return matchesName(to); + } + // TODO should probably cache this public List potentialReferences(Element to) { + // TODO only methods and types can have multiple definitions + // TODO reduce number of files we need to check by parsing and eliminating more cases + return matchesName(to); + } + + private List matchesName(Element to) { LOG.info(String.format("Find potential references to `%s`...", to)); // Check all files on source path diff --git a/src/main/java/org/javacs/JavaLanguageServer.java b/src/main/java/org/javacs/JavaLanguageServer.java index e9ebb18..a51e655 100644 --- a/src/main/java/org/javacs/JavaLanguageServer.java +++ b/src/main/java/org/javacs/JavaLanguageServer.java @@ -772,56 +772,28 @@ class JavaLanguageServer extends LanguageServer { return List.of(); } - // Figure out what file toEl is declared in - LOG.info(String.format("...looking for definition of `%s`", toEl.get())); - // TODO this should include overrides of toEl - var toUri = hoverCache.declaringFile(toEl.get()); - if (!toUri.isPresent()) { - LOG.info(String.format("...couldn't find declaring file, giving up")); - return List.of(); - } - if (!isJavaFile(toUri.get())) { - LOG.info(String.format("...declaring file %s isn't a .java file", toUri)); - return List.of(); - } + // Compile all files that *might* contain definitions of fromEl + var toFiles = compiler.potentialDefinitions(toEl.get()); + if (toFiles.isEmpty()) return List.of(); + var batch = compiler.compileBatch(toFiles); - // Compile fromUri and toUri together - Optional toRange; - if (toUri.get().equals(fromUri)) { - LOG.info("...definition is in the same file, using cached compilation"); + // Find fromEl again, so that we have an Element from the current batch + var fromElAgain = batch.element(fromUri, fromLine, fromColumn).get(); - var toPath = hoverCache.path(toEl.get()); - if (!toPath.isPresent()) { - LOG.warning(String.format("...couldn't locate `%s` in %s", toEl, toUri.get())); - return List.of(); - } - toRange = hoverCache.range(toPath.get()); - if (!toRange.isPresent()) { - LOG.info(String.format("...couldn't find `%s` in %s", toPath.get(), toUri)); - return List.of(); - } - } else { - LOG.info( - String.format( - "...compiling %s and %s together", Parser.fileName(fromUri), Parser.fileName(toUri.get()))); - - var both = Map.of(fromUri, contents(fromUri).content, toUri.get(), contents(toUri.get()).content); - var batch = compiler.compileBatch(both); - var toElAgain = batch.element(fromUri, fromLine, fromColumn).get(); - var toPath = batch.path(toElAgain); - if (!toPath.isPresent()) { - LOG.warning(String.format("...couldn't locate `%s` in %s", toEl, toUri.get())); - return List.of(); - } - toRange = batch.range(toPath.get()); + // Find all definitions of fromElAgain + var toTreePaths = batch.definitions(fromElAgain); + var result = new ArrayList(); + for (var path : toTreePaths) { + var toUri = path.getCompilationUnit().getSourceFile().toUri(); + var toRange = batch.range(path); if (!toRange.isPresent()) { - LOG.info(String.format("...couldn't find `%s` in %s", toPath.get(), toUri)); - return List.of(); + LOG.warning(String.format("Couldn't locate `%s`", path.getLeaf())); + continue; } + var from = new Location(toUri, toRange.get()); + result.add(from); } - - var to = new Location(toUri.get(), toRange.get()); - return List.of(to); + return result; } class Progress implements ReportProgress, AutoCloseable { diff --git a/src/main/java/org/javacs/Profiler.java b/src/main/java/org/javacs/Profiler.java index 43c0b80..7285732 100644 --- a/src/main/java/org/javacs/Profiler.java +++ b/src/main/java/org/javacs/Profiler.java @@ -36,6 +36,7 @@ class Profiler implements TaskListener { var s = elapsed.getSeconds() + elapsed.getNano() / 1000.0 / 1000.0 / 1000.0; lines.add(String.format("%s: %.3fs", k, s)); } + // TODO log names if n is small LOG.info(String.format("Compiled %d files: %s", files.size(), lines)); } diff --git a/src/test/java/org/javacs/FindReferencesTest.java b/src/test/java/org/javacs/FindReferencesTest.java index 9ce21c7..e728cc9 100644 --- a/src/test/java/org/javacs/FindReferencesTest.java +++ b/src/test/java/org/javacs/FindReferencesTest.java @@ -36,6 +36,12 @@ public class FindReferencesTest { assertThat(items("/org/javacs/example/GotoOther.java", 6, 30), not(empty())); } + @Test + public void findInterfaceReference() { + assertThat( + items("/org/javacs/example/GotoImplementation.java", 14, 26), contains("GotoImplementation.java(5)")); + } + @Test public void findConstructorReferences() { assertThat(items("/org/javacs/example/ConstructorRefs.java", 4, 10), contains("ConstructorRefs.java(9)")); diff --git a/src/test/java/org/javacs/GotoTest.java b/src/test/java/org/javacs/GotoTest.java index 7ad39ea..ac5d1cc 100644 --- a/src/test/java/org/javacs/GotoTest.java +++ b/src/test/java/org/javacs/GotoTest.java @@ -10,6 +10,7 @@ import org.javacs.lsp.*; import org.junit.Ignore; import org.junit.Test; +// TODO change :n to (n) public class GotoTest { private static final String file = "/org/javacs/example/Goto.java"; private static final String defaultConstructorFile = "/org/javacs/example/GotoDefaultConstructor.java"; @@ -18,91 +19,91 @@ public class GotoTest { public void localVariable() { var suggestions = doGoto(file, 10, 9); - assertThat(suggestions, contains("Goto.java:5")); + assertThat(suggestions, hasItem("Goto.java:5")); } @Test public void defaultConstructor() { var suggestions = doGoto(defaultConstructorFile, 5, 46); - assertThat(suggestions, contains("GotoDefaultConstructor.java:3")); + assertThat(suggestions, hasItem("GotoDefaultConstructor.java:3")); } @Test public void constructor() { var suggestions = doGoto(file, 11, 21); - assertThat(suggestions, contains("Goto.java:3")); + assertThat(suggestions, hasItem("Goto.java:3")); } @Test public void className() { var suggestions = doGoto(file, 16, 9); - assertThat(suggestions, contains("Goto.java:3")); + assertThat(suggestions, hasItem("Goto.java:3")); } @Test public void staticField() { var suggestions = doGoto(file, 13, 22); - assertThat(suggestions, contains("Goto.java:36")); + assertThat(suggestions, hasItem("Goto.java:36")); } @Test public void field() { var suggestions = doGoto(file, 14, 22); - assertThat(suggestions, contains("Goto.java:37")); + assertThat(suggestions, hasItem("Goto.java:37")); } @Test public void staticMethod() { var suggestions = doGoto(file, 16, 14); - assertThat(suggestions, contains("Goto.java:38")); + assertThat(suggestions, hasItem("Goto.java:38")); } @Test public void method() { var suggestions = doGoto(file, 17, 14); - assertThat(suggestions, contains("Goto.java:41")); + assertThat(suggestions, hasItem("Goto.java:41")); } @Test public void staticMethodReference() { var suggestions = doGoto(file, 19, 27); - assertThat(suggestions, contains("Goto.java:38")); + assertThat(suggestions, hasItem("Goto.java:38")); } @Test public void methodReference() { var suggestions = doGoto(file, 20, 27); - assertThat(suggestions, contains("Goto.java:41")); + assertThat(suggestions, hasItem("Goto.java:41")); } @Test public void otherStaticMethod() { var suggestions = doGoto(file, 29, 25); - assertThat(suggestions, contains(startsWith("GotoOther.java:"))); + assertThat(suggestions, hasItem(startsWith("GotoOther.java:"))); } @Test public void otherMethod() { var suggestions = doGoto(file, 30, 18); - assertThat(suggestions, contains(startsWith("GotoOther.java:"))); + assertThat(suggestions, hasItem(startsWith("GotoOther.java:"))); } @Test public void otherCompiledFile() { var suggestions = doGoto(file, 29, 25); - assertThat(suggestions, contains(startsWith("GotoOther.java:"))); + assertThat(suggestions, hasItem(startsWith("GotoOther.java:"))); } @Test @@ -110,7 +111,7 @@ public class GotoTest { public void typeParam() { var suggestions = doGoto(file, 46, 12); - assertThat(suggestions, contains("Goto.java:3")); + assertThat(suggestions, hasItem("Goto.java:3")); } @Test @@ -125,18 +126,25 @@ public class GotoTest { public void gotoOverload() { String file = "/org/javacs/example/GotoOverload.java"; - assertThat(doGoto(file, 7, 12), contains("GotoOverload.java:4")); - assertThat(doGoto(file, 8, 12), contains("GotoOverload.java:12")); - assertThat(doGoto(file, 9, 12), contains("GotoOverload.java:16")); + assertThat(doGoto(file, 7, 12), hasItem("GotoOverload.java:4")); + assertThat(doGoto(file, 8, 12), hasItem("GotoOverload.java:12")); + assertThat(doGoto(file, 9, 12), hasItem("GotoOverload.java:16")); } @Test public void gotoOverloadInOtherFile() { String file = "/org/javacs/example/GotoOverloadInOtherFile.java"; - assertThat(doGoto(file, 5, 25), contains("GotoOverload.java:4")); - assertThat(doGoto(file, 6, 25), contains("GotoOverload.java:12")); - assertThat(doGoto(file, 7, 25), contains("GotoOverload.java:16")); + assertThat(doGoto(file, 5, 25), hasItem("GotoOverload.java:4")); + assertThat(doGoto(file, 6, 25), hasItem("GotoOverload.java:12")); + assertThat(doGoto(file, 7, 25), hasItem("GotoOverload.java:16")); + } + + @Test + public void gotoImplementation() { + String file = "/org/javacs/example/GotoImplementation.java"; + + assertThat(doGoto(file, 5, 18), hasItems("GotoImplementation.java:9", "GotoImplementation.java:14")); } private static final JavaLanguageServer server = LanguageServerFixture.getJavaLanguageServer(); diff --git a/src/test/java/org/javacs/JavaCompilerServiceTest.java b/src/test/java/org/javacs/JavaCompilerServiceTest.java index 0785acb..c947bde 100644 --- a/src/test/java/org/javacs/JavaCompilerServiceTest.java +++ b/src/test/java/org/javacs/JavaCompilerServiceTest.java @@ -226,49 +226,6 @@ public class JavaCompilerServiceTest { assertThat(names, hasItem("concurrent")); } - /* - @Test - public void gotoDefinition() { - var def = - compiler.definition("/GotoDefinition.java"; - assertTrue(def.isPresent()); - - var t = def.get(); - var unit = t.getCompilationUnit(); - assertThat(unit.getSourceFile().getName(), endsWith("GotoDefinition.java")); - - var trees = compiler.trees(); - var pos = trees.getSourcePositions(); - var lines = unit.getLineMap(); - long start = pos.getStartPosition(unit, t.getLeaf()); - long line = lines.getLineNumber(start); - assertThat(line, equalTo(6L)); - } - */ - - @Test - public void references() { - var file = "GotoDefinition.java"; - var to = compiler.compileFocus(resourceUri(file), contents(file), 6, 13).element(); - var possible = compiler.potentialReferences(to); - assertThat( - "GotoDefinition.java can have references to itself", - possible, - hasItem(hasToString(endsWith("/GotoDefinition.java")))); - - var batch = compiler.compileBatch(possible); - var refs = batch.references(to); - var stringify = new ArrayList(); - for (var r : refs) { - var uri = r.getCompilationUnit().getSourceFile().toUri(); - var fileName = Paths.get(uri).getFileName(); - var range = batch.range(r).get(); - stringify.add(String.format("%s:%d", fileName, range.start.line + 1)); - } - assertThat(stringify, hasItem("GotoDefinition.java:3")); - assertThat(stringify, not(hasItem("GotoDefinition.java:6"))); - } - @Test public void overloads() { var uri = resourceUri("Overloads.java"); diff --git a/src/test/test-project/workspace/src/org/javacs/example/GotoImplementation.java b/src/test/test-project/workspace/src/org/javacs/example/GotoImplementation.java new file mode 100644 index 0000000..e1a0798 --- /dev/null +++ b/src/test/test-project/workspace/src/org/javacs/example/GotoImplementation.java @@ -0,0 +1,16 @@ +package org.javacs.example; + +class GotoImplementation { + void main(IGoto i) { + i.virtualMethod(); + } + + interface IGoto { + void virtualMethod(); + } + + class CGoto implements IGoto { + @Override + public void virtualMethod() { } + } +} \ No newline at end of file -- cgit v1.2.3