Passes tests but still having live issues

9 files changed, 114 insertions, 212 deletions

diff --git a/src/main/java/org/javacs/CompileFile.java b/src/main/java/org/javacs/CompileFile.java
index 7269edc..b0cae2c 100644
--- a/src/main/java/org/javacs/CompileFile.java
+++ b/src/main/java/org/javacs/CompileFile.java
@@ -32,8 +32,7 @@ public class CompileFile {
         var profiler = new Profiler();
         task.addTaskListener(profiler);
         try {
-            var it = task.parse().iterator();
-            this.root = it.hasNext() ? it.next() : null; // TODO something better than null when no class is present
+            this.root = task.parse().iterator().next();
             // The results of task.analyze() are unreliable when errors are present
             // You can get at `Element` values using `Trees`
             task.analyze();
diff --git a/src/main/java/org/javacs/CompileFocus.java b/src/main/java/org/javacs/CompileFocus.java
index 8ee8273..035de09 100644
--- a/src/main/java/org/javacs/CompileFocus.java
+++ b/src/main/java/org/javacs/CompileFocus.java
@@ -43,8 +43,7 @@ public class CompileFocus {
         var profiler = new Profiler();
         task.addTaskListener(profiler);
         try {
-            var it = task.parse().iterator();
-            this.root = it.hasNext() ? it.next() : null; // TODO something better than null when no class is present
+            this.root = task.parse().iterator().next();
             // The results of task.analyze() are unreliable when errors are present
             // You can get at `Element` values using `Trees`
             task.analyze();
diff --git a/src/main/java/org/javacs/FileStore.java b/src/main/java/org/javacs/FileStore.java
index 25fdbe8..7c5e2ad 100644
--- a/src/main/java/org/javacs/FileStore.java
+++ b/src/main/java/org/javacs/FileStore.java
@@ -1,7 +1,10 @@
 package org.javacs;
 
 import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
+import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.net.URI;
@@ -9,9 +12,12 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.time.Instant;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import org.javacs.lsp.DidChangeTextDocumentParams;
@@ -22,7 +28,42 @@ import org.javacs.lsp.TextDocumentContentChangeEvent;
 class FileStore {
 
     private static final Map<URI, VersionedContent> activeDocuments = new HashMap<>();
-    private static final Map<Path, Instant> modified = new HashMap<>();
+
+    /**
+     * modified[file] is the modified time of file. modified contains .java files and directories on the source path. If
+     * modified contains a directory, it contains all the .java files in that directory. modified is sorted, so you can
+     * list all the .java files in a directory by doing modified.tailSet(directory)
+     */
+    private static final TreeMap<Path, Instant> modified = new TreeMap<>();
+
+    /** Needed in tests */
+    static void reset() {
+        modified.clear();
+        activeDocuments.clear();
+    }
+
+    static List<Path> list(Path dir, String packageName) {
+        // If we've never indexed dir, index it now
+        if (!modified.containsKey(dir)) {
+            LOG.info(String.format("Indexing %s for the first time", dir));
+            try {
+                modified.put(dir, Instant.EPOCH);
+                Files.walk(dir).filter(SourcePath::isJavaFile).forEach(FileStore::readModifiedFromDisk);
+            } catch (IOException e) {
+                throw new RuntimeException(e);
+            }
+        }
+        // Walk through a section of index, starting at dir and ending when we find the next dir
+        var afterDir = modified.tailMap(dir, false).keySet();
+        var packageDir = dir.resolve(packageName.replace('.', File.separatorChar));
+        var list = new ArrayList<Path>();
+        for (var file : afterDir) {
+            if (!SourcePath.isJavaFile(file)) continue;
+            if (!file.startsWith(dir)) break;
+            if (file.startsWith(packageDir)) list.add(file);
+        }
+        return list;
+    }
 
     static Instant modified(Path file) {
         // If we've never checked before, look up modified time on disk
@@ -114,6 +155,37 @@ class FileStore {
         }
     }
 
+    static String contents(Path file) {
+        return contents(file.toUri());
+    }
+
+    static InputStream inputStream(Path file) {
+        var uri = file.toUri();
+        if (activeDocuments.containsKey(uri)) {
+            var string = activeDocuments.get(uri).content;
+            var bytes = string.getBytes();
+            return new ByteArrayInputStream(bytes);
+        }
+        try {
+            return Files.newInputStream(file);
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    static BufferedReader bufferedReader(Path file) {
+        var uri = file.toUri();
+        if (activeDocuments.containsKey(uri)) {
+            var string = activeDocuments.get(uri).content;
+            return new BufferedReader(new StringReader(string));
+        }
+        try {
+            return Files.newBufferedReader(file);
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
     static BufferedReader lines(Path file) {
         try {
             return Files.newBufferedReader(file);
diff --git a/src/main/java/org/javacs/JarFileObject.java b/src/main/java/org/javacs/JarFileObject.java
deleted file mode 100644
index a88d01c..0000000
--- a/src/main/java/org/javacs/JarFileObject.java
+++ /dev/null
@@ -1,101 +0,0 @@
-package org.javacs;
-
-import java.io.*;
-import java.net.URI;
-import java.nio.charset.Charset;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.jar.JarEntry;
-import java.util.jar.JarFile;
-import javax.lang.model.element.Modifier;
-import javax.lang.model.element.NestingKind;
-import javax.tools.JavaFileObject;
-
-class JarFileObject implements JavaFileObject {
-    final Path jar;
-    final JarEntry entry;
-
-    JarFileObject(Path jar, JarEntry entry) {
-        this.jar = jar;
-        this.entry = entry;
-    }
-
-    @Override
-    public Kind getKind() {
-        var name = entry.getName().toString();
-        return SourceFileObject.kindFromExtension(name);
-    }
-
-    @Override
-    public boolean isNameCompatible(String simpleName, Kind kind) {
-        var relative = Paths.get(simpleName.replace('.', '/'));
-        var absolute = Paths.get(entry.getName());
-        return absolute.endsWith(relative);
-    }
-
-    @Override
-    public NestingKind getNestingKind() {
-        return null;
-    }
-
-    @Override
-    public Modifier getAccessLevel() {
-        return null;
-    }
-
-    @Override
-    public URI toUri() {
-        var entryName = entry.getName();
-        var jarURI = jar.toUri().normalize();
-        var separator = entryName.startsWith("/") ? "!" : "!/";
-        return URI.create("jar:" + jarURI + separator + entryName);
-    }
-
-    @Override
-    public String getName() {
-        return jar + "(" + entry.getName() + ")";
-    }
-
-    @Override
-    public InputStream openInputStream() throws IOException {
-        return new JarFile(jar.toFile()).getInputStream(entry);
-    }
-
-    @Override
-    public OutputStream openOutputStream() throws IOException {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public Reader openReader(boolean ignoreEncodingErrors) throws IOException {
-        return new InputStreamReader(openInputStream());
-    }
-
-    @Override
-    public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOException {
-        var bytes = openInputStream().readAllBytes();
-        return new String(bytes, Charset.forName("UTF-8"));
-    }
-
-    @Override
-    public Writer openWriter() throws IOException {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public long getLastModified() {
-        return entry.getLastModifiedTime().toMillis();
-    }
-
-    @Override
-    public boolean delete() {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public boolean equals(Object other) {
-        if (other.getClass() != JarFileObject.class) return false;
-        var that = (JarFileObject) other;
-        return this.jar.equals(that.jar) && this.entry.getName().equals(that.entry.getName());
-    }
-}
diff --git a/src/main/java/org/javacs/JavaCompilerService.java b/src/main/java/org/javacs/JavaCompilerService.java
index 80a1f4d..74bc502 100644
--- a/src/main/java/org/javacs/JavaCompilerService.java
+++ b/src/main/java/org/javacs/JavaCompilerService.java
@@ -56,7 +56,7 @@ public class JavaCompilerService {
         this.classPathClasses = Classes.classPathTopLevelClasses(classPath);
         this.fileManager =
                 useSourceFileManager
-                        ? new SourceFileManager(sourcePath, classPath)
+                        ? new SourceFileManager(sourcePath)
                         : new FileManagerWrapper(
                                 compiler.getStandardFileManager(diags::add, null, Charset.defaultCharset()));
         ;
@@ -70,10 +70,8 @@ public class JavaCompilerService {
     static List<String> options(Set<Path> sourcePath, Set<Path> classPath) {
         var list = new ArrayList<String>();
 
-        if (!useSourceFileManager) {
-            Collections.addAll(list, "-classpath", joinPath(classPath));
-            Collections.addAll(list, "-sourcepath", joinPath(sourcePath));
-        }
+        Collections.addAll(list, "-classpath", joinPath(classPath));
+        Collections.addAll(list, "-sourcepath", joinPath(sourcePath));
         // Collections.addAll(list, "-verbose");
         Collections.addAll(list, "-proc:none");
         Collections.addAll(list, "-g");
@@ -196,6 +194,7 @@ public class JavaCompilerService {
             }
         }
         // TODO hint fields that could be final
+        // TODO hint unused exception
 
         return Collections.unmodifiableList(new ArrayList<>(diags));
     }
diff --git a/src/main/java/org/javacs/ParseFile.java b/src/main/java/org/javacs/ParseFile.java
index 2f54a0d..a4c052d 100644
--- a/src/main/java/org/javacs/ParseFile.java
+++ b/src/main/java/org/javacs/ParseFile.java
@@ -32,8 +32,7 @@ public class ParseFile {
         var profiler = new Profiler();
         task.addTaskListener(profiler);
         try {
-            var it = task.parse().iterator();
-            this.root = it.hasNext() ? it.next() : null; // TODO something better than null when no class is present
+            this.root = task.parse().iterator().next();
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
diff --git a/src/main/java/org/javacs/SourceFileManager.java b/src/main/java/org/javacs/SourceFileManager.java
index 0d8b792..e457b40 100644
--- a/src/main/java/org/javacs/SourceFileManager.java
+++ b/src/main/java/org/javacs/SourceFileManager.java
@@ -9,32 +9,31 @@ import java.nio.charset.Charset;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.*;
-import java.util.function.Predicate;
-import java.util.jar.JarFile;
+import java.util.logging.Logger;
 import java.util.stream.Stream;
 import javax.tools.*;
 
 class SourceFileManager implements StandardJavaFileManager {
     private final StandardJavaFileManager delegate = createDelegateFileManager();
+    private final Set<Path> sourcePath;
 
-    private final Set<Path> sourcePath, classPath;
-
-    SourceFileManager(Set<Path> sourcePath, Set<Path> classPath) {
+    SourceFileManager(Set<Path> sourcePath) {
         this.sourcePath = sourcePath;
-        this.classPath = classPath;
     }
 
     private static StandardJavaFileManager createDelegateFileManager() {
         var compiler = ServiceLoader.load(JavaCompiler.class).iterator().next();
-        return compiler.getStandardFileManager(__ -> {}, null, Charset.defaultCharset());
+        return compiler.getStandardFileManager(SourceFileManager::logError, null, Charset.defaultCharset());
+    }
+
+    private static void logError(Diagnostic<?> error) {
+        LOG.warning(error.getMessage(null));
     }
 
     @Override
     public ClassLoader getClassLoader(Location location) {
         var thisClassLoader = getClass().getClassLoader();
-        if (location == StandardLocation.CLASS_PATH) {
-            return new URLClassLoader(urls(classPath), thisClassLoader);
-        } else if (location == StandardLocation.SOURCE_PATH) {
+        if (location == StandardLocation.SOURCE_PATH) {
             return new URLClassLoader(urls(sourcePath), thisClassLoader);
         } else {
             return thisClassLoader;
@@ -57,10 +56,11 @@ class SourceFileManager implements StandardJavaFileManager {
     @Override
     public Iterable<JavaFileObject> list(
             Location location, String packageName, Set<JavaFileObject.Kind> kinds, boolean recurse) throws IOException {
-        if (location == StandardLocation.CLASS_PATH) {
-            return list(classPath, this::isClassPathFile);
-        } else if (location == StandardLocation.SOURCE_PATH) {
-            return list(sourcePath, this::isJavaSource);
+        if (location == StandardLocation.SOURCE_PATH) {
+            var dirs = sourcePath.stream();
+            var files = dirs.flatMap(dir -> FileStore.list(dir, packageName).stream());
+            var filter = files.map(this::asJavaFileObject).filter(this::isJavaSource);
+            return filter::iterator;
         } else {
             return delegate.list(location, packageName, kinds, recurse);
         }
@@ -70,16 +70,6 @@ class SourceFileManager implements StandardJavaFileManager {
         return file.getName().endsWith(".java");
     }
 
-    private boolean isClassPathFile(JavaFileObject file) {
-        var name = file.getName();
-        return name.endsWith(".class") || name.endsWith(".jar");
-    }
-
-    private Iterable<JavaFileObject> list(Set<Path> dirs, Predicate<JavaFileObject> filter) {
-        var stream = dirs.stream().flatMap(this::list).flatMap(this::asJavaFileObjects).filter(filter);
-        return stream::iterator;
-    }
-
     private Stream<Path> list(Path dir) {
         try {
             if (!Files.exists(dir)) return Stream.of();
@@ -89,19 +79,8 @@ class SourceFileManager implements StandardJavaFileManager {
         }
     }
 
-    private Stream<JavaFileObject> asJavaFileObjects(Path file) {
-        var fileName = file.getFileName().toString();
-        if (fileName.endsWith(".jar")) {
-            JarFile jar;
-            try {
-                jar = new JarFile(file.toFile());
-            } catch (IOException e) {
-                throw new RuntimeException(e);
-            }
-            return jar.stream().map(entry -> new JarFileObject(file, entry));
-        } else {
-            return Stream.of(new SourceFileObject(file));
-        }
+    private JavaFileObject asJavaFileObject(Path file) {
+        return new SourceFileObject(file);
     }
 
     @Override
@@ -109,13 +88,6 @@ class SourceFileManager implements StandardJavaFileManager {
         if (location == StandardLocation.SOURCE_PATH) {
             var source = (SourceFileObject) file;
             return sourceFileBinaryName(sourcePath, source);
-        } else if (location == StandardLocation.CLASS_PATH && file instanceof SourceFileObject) {
-            var source = (SourceFileObject) file;
-            return sourceFileBinaryName(classPath, source);
-        } else if (location == StandardLocation.CLASS_PATH && file instanceof JarFileObject) {
-            var jarFile = (JarFileObject) file;
-            var relativePath = jarFile.entry.getName();
-            return binaryName(relativePath);
         } else {
             return delegate.inferBinaryName(location, file);
         }
@@ -148,20 +120,18 @@ class SourceFileManager implements StandardJavaFileManager {
 
     @Override
     public boolean handleOption(String current, Iterator<String> remaining) {
-        return false;
+        return delegate.handleOption(current, remaining);
     }
 
     @Override
     public boolean hasLocation(Location location) {
-        return location == StandardLocation.CLASS_PATH
-                || location == StandardLocation.SOURCE_PATH
-                || delegate.hasLocation(location);
+        return location == StandardLocation.SOURCE_PATH || delegate.hasLocation(location);
     }
 
     @Override
     public JavaFileObject getJavaFileForInput(Location location, String className, JavaFileObject.Kind kind)
             throws IOException {
-        if (location == StandardLocation.SOURCE_PATH || location == StandardLocation.CLASS_PATH) {
+        if (location == StandardLocation.SOURCE_PATH) {
             var relative = className.replace('.', '/') + kind.extension;
             return findFileForInput(location, relative);
         }
@@ -170,7 +140,7 @@ class SourceFileManager implements StandardJavaFileManager {
 
     @Override
     public FileObject getFileForInput(Location location, String packageName, String relativeName) throws IOException {
-        if (location == StandardLocation.SOURCE_PATH || location == StandardLocation.CLASS_PATH) {
+        if (location == StandardLocation.SOURCE_PATH) {
             var relative = relativeName;
             if (!packageName.isEmpty()) {
                 relative = packageName.replace('.', '/') + '/' + relative;
@@ -181,32 +151,10 @@ class SourceFileManager implements StandardJavaFileManager {
     }
 
     private JavaFileObject findFileForInput(Location location, String relative) {
-        if (location == StandardLocation.CLASS_PATH) {
-            for (var root : classPath) {
-                if (Files.isDirectory(root)) {
-                    var absolute = root.resolve(relative);
-                    if (Files.exists(absolute)) {
-                        return new SourceFileObject(root);
-                    }
-                } else if (root.getFileName().toString().endsWith(".jar")) {
-                    JarFile jar;
-                    try {
-                        jar = new JarFile(root.toFile());
-                    } catch (IOException e) {
-                        throw new RuntimeException(e);
-                    }
-                    var entry = jar.getJarEntry(relative);
-                    if (entry != null) {
-                        return new JarFileObject(root, entry);
-                    }
-                }
-            }
-        } else if (location == StandardLocation.SOURCE_PATH) {
-            for (var root : sourcePath) {
-                var absolute = root.resolve(relative);
-                if (Files.exists(absolute)) {
-                    return new SourceFileObject(root);
-                }
+        for (var root : sourcePath) {
+            var absolute = root.resolve(relative);
+            if (Files.exists(absolute)) {
+                return new SourceFileObject(absolute);
             }
         }
         return null;
@@ -232,7 +180,7 @@ class SourceFileManager implements StandardJavaFileManager {
 
     @Override
     public int isSupportedOption(String option) {
-        return -1;
+        return delegate.isSupportedOption(option);
     }
 
     @Override
@@ -263,17 +211,9 @@ class SourceFileManager implements StandardJavaFileManager {
 
     @Override
     public boolean contains(Location location, FileObject file) throws IOException {
-        if (file instanceof SourceFileObject && location == StandardLocation.SOURCE_PATH) {
+        if (location == StandardLocation.SOURCE_PATH) {
             var source = (SourceFileObject) file;
             return contains(sourcePath, source);
-        } else if (file instanceof JarFileObject) {
-            var jarFile = (JarFileObject) file;
-            for (var jar : classPath) {
-                if (jarFile.jar.equals(jar)) {
-                    return true;
-                }
-            }
-            return false;
         } else {
             return delegate.contains(location, file);
         }
@@ -324,4 +264,6 @@ class SourceFileManager implements StandardJavaFileManager {
     public Iterable<? extends File> getLocation(Location location) {
         throw new UnsupportedOperationException();
     }
+
+    private static final Logger LOG = Logger.getLogger("main");
 }
diff --git a/src/main/java/org/javacs/SourceFileObject.java b/src/main/java/org/javacs/SourceFileObject.java
index 4310a8a..261971f 100644
--- a/src/main/java/org/javacs/SourceFileObject.java
+++ b/src/main/java/org/javacs/SourceFileObject.java
@@ -2,9 +2,7 @@ package org.javacs;
 
 import java.io.*;
 import java.net.URI;
-import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import javax.lang.model.element.Modifier;
 import javax.lang.model.element.NestingKind;
 import javax.tools.JavaFileObject;
@@ -40,8 +38,7 @@ class SourceFileObject implements JavaFileObject {
 
     @Override
     public boolean isNameCompatible(String simpleName, Kind kind) {
-        var relative = Paths.get(simpleName.replace('.', '/'));
-        return path.endsWith(relative);
+        return path.getFileName().toString().equals(simpleName + kind.extension);
     }
 
     @Override
@@ -66,7 +63,7 @@ class SourceFileObject implements JavaFileObject {
 
     @Override
     public InputStream openInputStream() throws IOException {
-        return Files.newInputStream(path);
+        return FileStore.inputStream(path);
     }
 
     @Override
@@ -76,12 +73,12 @@ class SourceFileObject implements JavaFileObject {
 
     @Override
     public Reader openReader(boolean ignoreEncodingErrors) throws IOException {
-        return Files.newBufferedReader(path);
+        return FileStore.bufferedReader(path);
     }
 
     @Override
     public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOException {
-        return Files.readString(path);
+        return FileStore.contents(path);
     }
 
     @Override
@@ -91,11 +88,7 @@ class SourceFileObject implements JavaFileObject {
 
     @Override
     public long getLastModified() {
-        try {
-            return Files.getLastModifiedTime(path).toMillis();
-        } catch (IOException e) {
-            throw new RuntimeException(e);
-        }
+        return FileStore.modified(path).toEpochMilli();
     }
 
     @Override
diff --git a/src/main/java/org/javacs/WarnUnused.java b/src/main/java/org/javacs/WarnUnused.java
index b3a2237..18d352e 100644
--- a/src/main/java/org/javacs/WarnUnused.java
+++ b/src/main/java/org/javacs/WarnUnused.java
@@ -38,7 +38,7 @@ class WarnUnused extends TreePathScanner<Void, Void> {
     boolean isLocal(VariableTree t) {
         var parent = getCurrentPath().getParentPath().getLeaf();
         return !(parent instanceof ClassTree)
-                && !(parent instanceof MethodTree)
+                && !(parent instanceof MethodTree) // TODO hint for unused parameters
                 && !(parent instanceof LambdaExpressionTree);
     }