Remove StringFileObject, FileManagerWrapper

11 files changed, 49 insertions, 350 deletions

diff --git a/src/main/java/org/javacs/CompileFocus.java b/src/main/java/org/javacs/CompileFocus.java
index 035de09..48b5469 100644
--- a/src/main/java/org/javacs/CompileFocus.java
+++ b/src/main/java/org/javacs/CompileFocus.java
@@ -64,7 +64,7 @@ public class CompileFocus {
                         parent.diags::add,
                         JavaCompilerService.options(parent.sourcePath, parent.classPath),
                         Collections.emptyList(),
-                        Collections.singletonList(new StringFileObject(contents, file)));
+                        List.of(new SourceFileObject(file, contents)));
     }
 
     /** Find the smallest element that includes the cursor */
diff --git a/src/main/java/org/javacs/FileManagerWrapper.java b/src/main/java/org/javacs/FileManagerWrapper.java
deleted file mode 100644
index 43e9f41..0000000
--- a/src/main/java/org/javacs/FileManagerWrapper.java
+++ /dev/null
@@ -1,302 +0,0 @@
-package org.javacs;
-
-import java.io.File;
-import java.io.IOException;
-import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.ServiceLoader;
-import java.util.Set;
-import java.util.function.Predicate;
-import java.util.stream.StreamSupport;
-import javax.tools.FileObject;
-import javax.tools.JavaFileObject;
-import javax.tools.StandardJavaFileManager;
-import javax.tools.StandardLocation;
-
-// TODO instead of forwarding to delegate, implement a from-scratch JavaFileManager that knows how to get open text from
-// FileStore
-class FileManagerWrapper implements StandardJavaFileManager {
-    private final StandardJavaFileManager delegate;
-
-    FileManagerWrapper(StandardJavaFileManager delegate) {
-        this.delegate = delegate;
-    }
-
-    private boolean notModuleInfo(JavaFileObject file) {
-        return file == null || !file.getName().endsWith("module-info.java");
-    }
-
-    private boolean notModuleInfo(File file) {
-        return file == null || !file.getName().endsWith("module-info.java");
-    }
-
-    private boolean notModuleInfo(Path file) {
-        return file == null || !file.getFileName().endsWith("module-info.java");
-    }
-
-    private JavaFileObject skipModuleInfo(JavaFileObject file) {
-        if (file == null) return null;
-        if (file.getName().endsWith("module-info.java")) return null;
-        else return file;
-    }
-
-    private FileObject skipModuleInfo(FileObject file) {
-        if (file == null) return null;
-        if (file.getName().endsWith("module-info.java")) return null;
-        else return file;
-    }
-
-    private <T> Iterable<T> filter(Iterable<T> in, Predicate<T> f) {
-        return StreamSupport.stream(in.spliterator(), false).filter(f)::iterator;
-    }
-
-    private Iterable<? extends JavaFileObject> removeModuleInfo(Iterable<? extends JavaFileObject> in) {
-        return filter(in, this::notModuleInfo);
-    }
-
-    private Iterable<JavaFileObject> removeModuleInfoInvariant(Iterable<JavaFileObject> in) {
-        return filter(in, this::notModuleInfo);
-    }
-
-    private Iterable<? extends File> removeModuleInfoFile(Iterable<? extends File> in) {
-        return filter(in, this::notModuleInfo);
-    }
-
-    private Iterable<? extends Path> removeModuleInfoPath(Iterable<? extends Path> in) {
-        return filter(in, this::notModuleInfo);
-    }
-
-    @Override
-    public boolean isSameFile(FileObject a, FileObject b) {
-        return delegate.isSameFile(a, b);
-    }
-
-    @Override
-    public Iterable<? extends JavaFileObject> getJavaFileObjectsFromFiles(Iterable<? extends File> files) {
-        return removeModuleInfo(delegate.getJavaFileObjectsFromFiles(files));
-    }
-
-    @Override
-    public Iterable<? extends JavaFileObject> getJavaFileObjectsFromPaths(Iterable<? extends Path> paths) {
-        return removeModuleInfo(delegate.getJavaFileObjectsFromPaths(paths));
-    }
-
-    @Override
-    public Iterable<? extends JavaFileObject> getJavaFileObjects(File... files) {
-        return removeModuleInfo(delegate.getJavaFileObjects(files));
-    }
-
-    @Override
-    public Iterable<? extends JavaFileObject> getJavaFileObjects(Path... paths) {
-        return removeModuleInfo(delegate.getJavaFileObjects(paths));
-    }
-
-    @Override
-    public Iterable<? extends JavaFileObject> getJavaFileObjectsFromStrings(Iterable<String> names) {
-        return removeModuleInfo(delegate.getJavaFileObjectsFromStrings(names));
-    }
-
-    @Override
-    public Iterable<? extends JavaFileObject> getJavaFileObjects(String... names) {
-        return removeModuleInfo(delegate.getJavaFileObjects(names));
-    }
-
-    @Override
-    public void setLocation(Location location, Iterable<? extends File> files) throws IOException {
-        delegate.setLocation(location, files);
-    }
-
-    @Override
-    public void setLocationFromPaths(Location location, Collection<? extends Path> paths) throws IOException {
-        delegate.setLocationFromPaths(location, paths);
-    }
-
-    @Override
-    public void setLocationForModule(Location location, String moduleName, Collection<? extends Path> paths)
-            throws IOException {
-        delegate.setLocationForModule(location, moduleName, paths);
-    }
-
-    @Override
-    public Iterable<? extends File> getLocation(Location location) {
-        return removeModuleInfoFile(delegate.getLocation(location));
-    }
-
-    @Override
-    public Iterable<? extends Path> getLocationAsPaths(Location location) {
-        return removeModuleInfoPath(delegate.getLocationAsPaths(location));
-    }
-
-    @Override
-    public Path asPath(FileObject file) {
-        return delegate.asPath(file);
-    }
-
-    @Override
-    public void setPathFactory(PathFactory f) {
-        delegate.setPathFactory(f);
-    }
-
-    @Override
-    public ClassLoader getClassLoader(Location location) {
-        return delegate.getClassLoader(location);
-    }
-
-    // Cache calls to list(...)
-    static class Key {
-        final Location location;
-        final String packageName;
-        final JavaFileObject.Kind kind;
-        final boolean recurse;
-
-        Key(Location location, String packageName, JavaFileObject.Kind kind, boolean recurse) {
-            this.location = location;
-            this.packageName = packageName;
-            this.kind = kind;
-            this.recurse = recurse;
-        }
-
-        @Override
-        public boolean equals(Object candidate) {
-            if (!(candidate instanceof Key)) return false;
-            var that = (Key) candidate;
-
-            return Objects.equals(this.location, that.location)
-                    && Objects.equals(this.packageName, that.packageName)
-                    && Objects.equals(this.kind, that.kind)
-                    && Objects.equals(this.recurse, that.recurse);
-        }
-
-        @Override
-        public int hashCode() {
-            return Objects.hash(location, packageName, kind, recurse);
-        }
-    }
-
-    // Store previous calls to list, because listing directories and .jar files is expensive
-    private Map<Key, List<JavaFileObject>> cache = new HashMap<>();
-
-    private List<JavaFileObject> loadCache(Key key) {
-        try {
-            var list = new ArrayList<JavaFileObject>();
-            var it =
-                    removeModuleInfoInvariant(
-                            delegate.list(key.location, key.packageName, Collections.singleton(key.kind), key.recurse));
-            for (var file : it) list.add(file);
-            return list;
-        } catch (IOException e) {
-            throw new RuntimeException(e);
-        }
-    }
-
-    @Override
-    public Iterable<JavaFileObject> list(
-            Location location, String packageName, Set<JavaFileObject.Kind> kinds, boolean recurse) throws IOException {
-        // If search source path, skip cache
-        // TODO does this actually do anything?
-        if (location == StandardLocation.SOURCE_PATH
-                || location == StandardLocation.MODULE_SOURCE_PATH
-                || location == StandardLocation.SOURCE_OUTPUT)
-            return removeModuleInfoInvariant(delegate.list(location, packageName, kinds, recurse));
-
-        // Search for each kind separately to improve cacheability
-        var result = new ArrayList<JavaFileObject>();
-        for (var kind : kinds) {
-            var list = cache.computeIfAbsent(new Key(location, packageName, kind, recurse), this::loadCache);
-            result.addAll(list);
-        }
-        return result;
-    }
-
-    @Override
-    public String inferBinaryName(Location location, JavaFileObject file) {
-        return delegate.inferBinaryName(location, file);
-    }
-
-    @Override
-    public boolean handleOption(String current, Iterator<String> remaining) {
-        return delegate.handleOption(current, remaining);
-    }
-
-    @Override
-    public boolean hasLocation(Location location) {
-        return delegate.hasLocation(location);
-    }
-
-    @Override
-    public JavaFileObject getJavaFileForInput(Location location, String className, JavaFileObject.Kind kind)
-            throws IOException {
-        return skipModuleInfo(delegate.getJavaFileForInput(location, className, kind));
-    }
-
-    @Override
-    public JavaFileObject getJavaFileForOutput(
-            Location location, String className, JavaFileObject.Kind kind, FileObject sibling) throws IOException {
-        return skipModuleInfo(delegate.getJavaFileForOutput(location, className, kind, sibling));
-    }
-
-    @Override
-    public FileObject getFileForInput(Location location, String packageName, String relativeName) throws IOException {
-        return skipModuleInfo(delegate.getFileForInput(location, packageName, relativeName));
-    }
-
-    @Override
-    public FileObject getFileForOutput(Location location, String packageName, String relativeName, FileObject sibling)
-            throws IOException {
-        return skipModuleInfo(delegate.getFileForOutput(location, packageName, relativeName, sibling));
-    }
-
-    @Override
-    public void flush() throws IOException {
-        delegate.flush();
-    }
-
-    @Override
-    public void close() throws IOException {
-        delegate.close();
-    }
-
-    @Override
-    public Location getLocationForModule(Location location, String moduleName) throws IOException {
-        return delegate.getLocationForModule(location, moduleName);
-    }
-
-    @Override
-    public Location getLocationForModule(Location location, JavaFileObject fo) throws IOException {
-        return delegate.getLocationForModule(location, fo);
-    }
-
-    @Override
-    public <S> ServiceLoader<S> getServiceLoader(Location location, Class<S> service) throws IOException {
-        return delegate.getServiceLoader(location, service);
-    }
-
-    @Override
-    public String inferModuleName(Location location) throws IOException {
-        return delegate.inferModuleName(location);
-    }
-
-    @Override
-    public Iterable<Set<Location>> listLocationsForModules(Location location) throws IOException {
-        return delegate.listLocationsForModules(location);
-    }
-
-    @Override
-    public boolean contains(Location location, FileObject fo) throws IOException {
-        if (fo.getName().endsWith("module-info.java")) return false;
-
-        return delegate.contains(location, fo);
-    }
-
-    @Override
-    public int isSupportedOption(String option) {
-        return delegate.isSupportedOption(option);
-    }
-}
diff --git a/src/main/java/org/javacs/JavaCompilerService.java b/src/main/java/org/javacs/JavaCompilerService.java
index 74bc502..23484e9 100644
--- a/src/main/java/org/javacs/JavaCompilerService.java
+++ b/src/main/java/org/javacs/JavaCompilerService.java
@@ -5,7 +5,6 @@ import com.sun.source.util.*;
 import java.io.File;
 import java.io.IOException;
 import java.net.URI;
-import java.nio.charset.Charset;
 import java.nio.file.*;
 import java.util.*;
 import java.util.function.Supplier;
@@ -28,7 +27,6 @@ public class JavaCompilerService {
     // Use the same file manager for multiple tasks, so we don't repeatedly re-compile the same files
     // TODO intercept files that aren't in the batch and erase method bodies so compilation is faster
     final StandardJavaFileManager fileManager;
-    static final boolean useSourceFileManager = true;
 
     public JavaCompilerService(
             Set<Path> sourcePath, Supplier<Set<Path>> allJavaFiles, Set<Path> classPath, Set<Path> docPath) {
@@ -54,11 +52,7 @@ public class JavaCompilerService {
         docSourcePath.addAll(docPath);
         this.docs = new Docs(docSourcePath);
         this.classPathClasses = Classes.classPathTopLevelClasses(classPath);
-        this.fileManager =
-                useSourceFileManager
-                        ? new SourceFileManager(sourcePath)
-                        : new FileManagerWrapper(
-                                compiler.getStandardFileManager(diags::add, null, Charset.defaultCharset()));
+        this.fileManager = new SourceFileManager(sourcePath);
         ;
     }
 
diff --git a/src/main/java/org/javacs/JavaLanguageServer.java b/src/main/java/org/javacs/JavaLanguageServer.java
index ea352f0..8a11e90 100644
--- a/src/main/java/org/javacs/JavaLanguageServer.java
+++ b/src/main/java/org/javacs/JavaLanguageServer.java
@@ -848,15 +848,16 @@ class JavaLanguageServer extends LanguageServer {
         for (var f : files) {
             var contents = FileStore.contents(f);
             var pruned = Pruner.prune(f, contents, name);
-            sources.add(new StringFileObject(pruned, f));
+            sources.add(new SourceFileObject(f, pruned));
         }
         return sources;
     }
 
+    // TODO pretty sure this is unnecessary now that SourceFileObject delegates to FileStore
     private List<JavaFileObject> latestText(Collection<URI> files) {
         var sources = new ArrayList<JavaFileObject>();
         for (var f : files) {
-            sources.add(new StringFileObject(FileStore.contents(f), f));
+            sources.add(new SourceFileObject(f));
         }
         return sources;
     }
diff --git a/src/main/java/org/javacs/ParseFile.java b/src/main/java/org/javacs/ParseFile.java
index a4c052d..0015416 100644
--- a/src/main/java/org/javacs/ParseFile.java
+++ b/src/main/java/org/javacs/ParseFile.java
@@ -465,7 +465,7 @@ public class ParseFile {
     private static final DocCommentTree EMPTY_DOC = makeEmptyDoc();
 
     private static DocCommentTree makeEmptyDoc() {
-        var file = new StringFileObject("/** */ class Foo { }", URI.create("file:///Foo.java"));
+        var file = new SourceFileObject(URI.create("file:///Foo.java"), "/** */ class Foo { }");
         var task = Parser.parseTask(file);
         var docs = DocTrees.instance(task);
         CompilationUnitTree root;
diff --git a/src/main/java/org/javacs/Pruner.java b/src/main/java/org/javacs/Pruner.java
index bf7203b..bda547d 100644
--- a/src/main/java/org/javacs/Pruner.java
+++ b/src/main/java/org/javacs/Pruner.java
@@ -107,9 +107,10 @@ class Pruner {
         return buffer.toString();
     }
 
+    // TODO can get rid of contents now that SourceFileObject references FileStore
     static String prune(URI file, String contents, int line, int character) {
         // Parse file
-        var task = Parser.parseTask(new StringFileObject(contents, file));
+        var task = Parser.parseTask(new SourceFileObject(file, contents));
         CompilationUnitTree root;
         try {
             root = task.parse().iterator().next();
@@ -124,6 +125,7 @@ class Pruner {
         return prune(root, pos, buffer, new long[] {cursor});
     }
 
+    // TODO can get rid of contents now that SourceFileObject references FileStore
     static String prune(URI file, String contents, String name) {
         // Find all occurrences of name in contents
         var list = new ArrayList<Long>();
@@ -137,7 +139,7 @@ class Pruner {
             offsets[i] = list.get(i);
         }
         // Parse file
-        var task = Parser.parseTask(new StringFileObject(contents, file));
+        var task = Parser.parseTask(new SourceFileObject(file, contents));
         CompilationUnitTree root;
         try {
             root = task.parse().iterator().next();
diff --git a/src/main/java/org/javacs/SourceFileObject.java b/src/main/java/org/javacs/SourceFileObject.java
index 261971f..24e7608 100644
--- a/src/main/java/org/javacs/SourceFileObject.java
+++ b/src/main/java/org/javacs/SourceFileObject.java
@@ -3,15 +3,32 @@ package org.javacs;
 import java.io.*;
 import java.net.URI;
 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;
 
 class SourceFileObject implements JavaFileObject {
+    /** path is the absolute path to this file on disk */
     final Path path;
+    /** contents is the text in this file, or null if we should use the text in FileStore */
+    final String contents;
+
+    SourceFileObject(URI uri) {
+        this(Paths.get(uri));
+    }
 
     SourceFileObject(Path path) {
+        this(path, null);
+    }
+
+    SourceFileObject(URI uri, String contents) {
+        this(Paths.get(uri), contents);
+    }
+
+    SourceFileObject(Path path, String contents) {
         this.path = path;
+        this.contents = contents;
     }
 
     @Override
@@ -63,6 +80,10 @@ class SourceFileObject implements JavaFileObject {
 
     @Override
     public InputStream openInputStream() throws IOException {
+        if (contents != null) {
+            var bytes = contents.getBytes();
+            return new ByteArrayInputStream(bytes);
+        }
         return FileStore.inputStream(path);
     }
 
@@ -73,11 +94,17 @@ class SourceFileObject implements JavaFileObject {
 
     @Override
     public Reader openReader(boolean ignoreEncodingErrors) throws IOException {
+        if (contents != null) {
+            return new StringReader(contents);
+        }
         return FileStore.bufferedReader(path);
     }
 
     @Override
     public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOException {
+        if (contents != null) {
+            return contents;
+        }
         return FileStore.contents(path);
     }
 
diff --git a/src/main/java/org/javacs/StringFileObject.java b/src/main/java/org/javacs/StringFileObject.java
deleted file mode 100644
index 683ddaf..0000000
--- a/src/main/java/org/javacs/StringFileObject.java
+++ /dev/null
@@ -1,22 +0,0 @@
-package org.javacs;
-
-import java.io.IOException;
-import java.net.URI;
-import javax.tools.SimpleJavaFileObject;
-
-class StringFileObject extends SimpleJavaFileObject {
-    private final String content;
-    private final URI path; // TODO rename
-
-    StringFileObject(String content, URI path) {
-        super(path, Kind.SOURCE);
-
-        this.content = content;
-        this.path = path;
-    }
-
-    @Override
-    public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOException {
-        return content;
-    }
-}
diff --git a/src/test/java/org/javacs/BenchmarkPruner.java b/src/test/java/org/javacs/BenchmarkPruner.java
index 4e9f10a..1daf169 100644
--- a/src/test/java/org/javacs/BenchmarkPruner.java
+++ b/src/test/java/org/javacs/BenchmarkPruner.java
@@ -15,12 +15,12 @@ import org.openjdk.jmh.annotations.*;
 @Fork(1)
 public class BenchmarkPruner {
     private static Path sourceRoot = Paths.get("src/main/java").normalize();
-    private static List<StringFileObject> files = files(false);
-    private static List<StringFileObject> pruned = files(true);
+    private static List<SourceFileObject> files = files(false);
+    private static List<SourceFileObject> pruned = files(true);
 
-    private static List<StringFileObject> files(boolean prune) {
+    private static List<SourceFileObject> files(boolean prune) {
         try {
-            var files = new ArrayList<StringFileObject>();
+            var files = new ArrayList<SourceFileObject>();
             var dir = Paths.get("src/main/java/org/javacs").normalize();
             var it = Files.list(dir).iterator();
             while (it.hasNext()) {
@@ -30,7 +30,7 @@ public class BenchmarkPruner {
                 if (prune) {
                     contents = Pruner.prune(file.toUri(), contents, "isWord");
                 }
-                files.add(new StringFileObject(contents, file.toUri()));
+                files.add(new SourceFileObject(file, contents));
             }
             return files;
         } catch (IOException e) {
diff --git a/src/test/java/org/javacs/JavaCompilerServiceTest.java b/src/test/java/org/javacs/JavaCompilerServiceTest.java
index b9419ad..a0618da 100644
--- a/src/test/java/org/javacs/JavaCompilerServiceTest.java
+++ b/src/test/java/org/javacs/JavaCompilerServiceTest.java
@@ -64,7 +64,7 @@ public class JavaCompilerServiceTest {
         return join.toString();
     }
 
-    private URI resourceUri(String resourceFile) {
+    static URI resourceUri(String resourceFile) {
         var root = JavaCompilerServiceTest.simpleProjectSrc();
         var file = root.resolve(resourceFile);
         return file.toUri();
diff --git a/src/test/java/org/javacs/PrunerTest.java b/src/test/java/org/javacs/PrunerTest.java
index 50e511f..9d649c2 100644
--- a/src/test/java/org/javacs/PrunerTest.java
+++ b/src/test/java/org/javacs/PrunerTest.java
@@ -1,45 +1,44 @@
 package org.javacs;
 
 import static org.hamcrest.Matchers.*;
-import static org.javacs.JavaCompilerServiceTest.contents;
+import static org.javacs.JavaCompilerServiceTest.*;
 import static org.junit.Assert.*;
 
-import java.net.URI;
 import org.junit.Test;
 
 public class PrunerTest {
 
     @Test
     public void pruneMethods() {
-        var actual = Pruner.prune(URI.create("/PruneMethods.java"), contents("PruneMethods.java"), 6, 19);
+        var actual = Pruner.prune(resourceUri("PruneMethods.java"), contents("PruneMethods.java"), 6, 19);
         var expected = contents("PruneMethods_erased.java");
         assertThat(actual, equalToIgnoringWhiteSpace(expected));
     }
 
     @Test
     public void pruneToEndOfBlock() {
-        var actual = Pruner.prune(URI.create("/PruneToEndOfBlock.java"), contents("PruneToEndOfBlock.java"), 4, 18);
+        var actual = Pruner.prune(resourceUri("PruneToEndOfBlock.java"), contents("PruneToEndOfBlock.java"), 4, 18);
         var expected = contents("PruneToEndOfBlock_erased.java");
         assertThat(actual, equalToIgnoringWhiteSpace(expected));
     }
 
     @Test
     public void pruneMiddle() {
-        var actual = Pruner.prune(URI.create("/PruneMiddle.java"), contents("PruneMiddle.java"), 4, 12);
+        var actual = Pruner.prune(resourceUri("PruneMiddle.java"), contents("PruneMiddle.java"), 4, 12);
         var expected = contents("PruneMiddle_erased.java");
         assertThat(actual, equalToIgnoringWhiteSpace(expected));
     }
 
     @Test
     public void pruneDot() {
-        var actual = Pruner.prune(URI.create("/PruneDot.java"), contents("PruneDot.java"), 3, 11);
+        var actual = Pruner.prune(resourceUri("PruneDot.java"), contents("PruneDot.java"), 3, 11);
         var expected = contents("PruneDot_erased.java");
         assertThat(actual, equalToIgnoringWhiteSpace(expected));
     }
 
     @Test
     public void pruneWords() {
-        var actual = Pruner.prune(URI.create("/PruneWords.java"), contents("PruneWords.java"), "word");
+        var actual = Pruner.prune(resourceUri("PruneWords.java"), contents("PruneWords.java"), "word");
         var expected = contents("PruneWords_erased.java");
         assertThat(actual, equalToIgnoringWhiteSpace(expected));
     }