Detect unused variables

6 files changed, 202 insertions, 2 deletions

diff --git a/src/main/java/org/javacs/JavaCompilerService.java b/src/main/java/org/javacs/JavaCompilerService.java
index 1646ecb..3d47052 100644
--- a/src/main/java/org/javacs/JavaCompilerService.java
+++ b/src/main/java/org/javacs/JavaCompilerService.java
@@ -137,28 +137,44 @@ public class JavaCompilerService {
     public List<Diagnostic<? extends JavaFileObject>> reportErrors(Collection<URI> uris) {
         LOG.info(String.format("Report errors in %d files...", uris.size()));
 
-        var options = options(sourcePath, classPath);
         // Construct list of sources
         var files = new ArrayList<File>();
         for (var p : uris) files.add(new File(p));
         var sources = fileManager.getJavaFileObjectsFromFiles(files);
+
         // Create task
+        var options = options(sourcePath, classPath);
         var task =
                 (JavacTask) compiler.getTask(null, fileManager, diags::add, options, Collections.emptyList(), sources);
+        var trees = Trees.instance(task);
+
         // Print timing information for optimization
         var profiler = new Profiler();
         task.addTaskListener(profiler);
+
         // Run compilation
         diags.clear();
+        Iterable<? extends CompilationUnitTree> roots;
         try {
+            roots = task.parse();
             task.analyze();
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
         profiler.print();
-
         LOG.info(String.format("...found %d errors", diags.size()));
 
+        // Check for unused privates
+        for (var r : roots) {
+            var warnUnused = new WarnUnused(task);
+            warnUnused.scan(r, null);
+            for (var unusedEl : warnUnused.notUsed()) {
+                var path = trees.getPath(unusedEl);
+                diags.add(
+                        new Warning(task, path, "unused", String.format("`%s` is not used", unusedEl.getSimpleName())));
+            }
+        }
+
         return Collections.unmodifiableList(new ArrayList<>(diags));
     }
 
diff --git a/src/main/java/org/javacs/WarnUnused.java b/src/main/java/org/javacs/WarnUnused.java
new file mode 100644
index 0000000..451a524
--- /dev/null
+++ b/src/main/java/org/javacs/WarnUnused.java
@@ -0,0 +1,63 @@
+package org.javacs;
+
+import com.sun.source.tree.*;
+import com.sun.source.util.*;
+import java.util.*;
+import javax.lang.model.element.*;
+
+class WarnUnused extends TreePathScanner<Void, Void> {
+    private final Trees trees;
+    private final Set<Element> declared = new HashSet<>(), used = new HashSet<>();
+
+    WarnUnused(JavacTask task) {
+        this.trees = Trees.instance(task);
+    }
+
+    Set<Element> notUsed() {
+        declared.removeAll(used);
+        return declared;
+    }
+
+    Element current() {
+        return trees.getElement(getCurrentPath());
+    }
+
+    boolean isPrivate(VariableTree t) {
+        return t.getModifiers().getFlags().contains(Modifier.PRIVATE);
+    }
+
+    boolean isLocal(VariableTree t) {
+        var parent = getCurrentPath().getParentPath().getLeaf();
+        return !(parent instanceof ClassTree);
+    }
+
+    boolean isPrivate(MethodTree t) {
+        return t.getModifiers().getFlags().contains(Modifier.PRIVATE);
+    }
+
+    @Override
+    public Void visitVariable(VariableTree t, Void __) {
+        if (isPrivate(t) || isLocal(t)) {
+            declared.add(current());
+        }
+        return super.visitVariable(t, null);
+    }
+
+    @Override
+    public Void visitIdentifier(IdentifierTree t, Void __) {
+        used.add(current());
+        return super.visitIdentifier(t, null);
+    }
+
+    @Override
+    public Void visitMemberSelect(MemberSelectTree t, Void __) {
+        used.add(current());
+        return super.visitMemberSelect(t, null);
+    }
+
+    @Override
+    public Void visitMemberReference(MemberReferenceTree t, Void __) {
+        used.add(current());
+        return super.visitMemberReference(t, null);
+    }
+}
diff --git a/src/main/java/org/javacs/Warning.java b/src/main/java/org/javacs/Warning.java
new file mode 100644
index 0000000..a173d40
--- /dev/null
+++ b/src/main/java/org/javacs/Warning.java
@@ -0,0 +1,70 @@
+package org.javacs;
+
+import com.sun.source.tree.LineMap;
+import com.sun.source.util.*;
+import java.util.Locale;
+import javax.tools.*;
+import javax.tools.JavaFileObject;
+
+class Warning implements Diagnostic<JavaFileObject> {
+
+    private final JavaFileObject source;
+    private final LineMap lines;
+    private final long start, end;
+    private final String code, message;
+
+    Warning(JavacTask task, TreePath path, String code, String message) {
+        this.source = path.getCompilationUnit().getSourceFile();
+        this.lines = path.getCompilationUnit().getLineMap();
+        var pos = Trees.instance(task).getSourcePositions();
+        this.start = pos.getStartPosition(path.getCompilationUnit(), path.getLeaf());
+        this.end = pos.getEndPosition(path.getCompilationUnit(), path.getLeaf());
+        this.code = code;
+        this.message = message;
+    }
+
+    @Override
+    public Kind getKind() {
+        return Kind.WARNING;
+    }
+
+    @Override
+    public JavaFileObject getSource() {
+        return source;
+    }
+
+    @Override
+    public long getPosition() {
+        return start;
+    }
+
+    @Override
+    public long getStartPosition() {
+        return start;
+    }
+
+    @Override
+    public long getEndPosition() {
+        return end;
+    }
+
+    @Override
+    public long getLineNumber() {
+        return lines.getLineNumber(start);
+    }
+
+    @Override
+    public long getColumnNumber() {
+        return lines.getColumnNumber(start);
+    }
+
+    @Override
+    public String getCode() {
+        return code;
+    }
+
+    @Override
+    public String getMessage(Locale locale) {
+        return message;
+    }
+}
diff --git a/src/test/java/org/javacs/LanguageServerFixture.java b/src/test/java/org/javacs/LanguageServerFixture.java
index f2da3b7..f9052a0 100644
--- a/src/test/java/org/javacs/LanguageServerFixture.java
+++ b/src/test/java/org/javacs/LanguageServerFixture.java
@@ -19,18 +19,26 @@ class LanguageServerFixture {
         return getJavaLanguageServer(DEFAULT_WORKSPACE_ROOT, diagnostic -> LOG.info(diagnostic.message));
     }
 
+    static JavaLanguageServer getJavaLanguageServer(Consumer<Diagnostic> onError) {
+        return getJavaLanguageServer(DEFAULT_WORKSPACE_ROOT, onError);
+    }
+
     static JavaLanguageServer getJavaLanguageServer(Path workspaceRoot, Consumer<Diagnostic> onError) {
         return getJavaLanguageServer(
                 workspaceRoot,
                 new LanguageClient() {
+                    @Override
                     public void publishDiagnostics(PublishDiagnosticsParams params) {
                         params.diagnostics.forEach(onError);
                     }
 
+                    @Override
                     public void showMessage(ShowMessageParams params) {}
 
+                    @Override
                     public void registerCapability(String method, JsonElement options) {}
 
+                    @Override
                     public void customNotification(String method, JsonElement params) {}
                 });
     }
diff --git a/src/test/java/org/javacs/WarningsTest.java b/src/test/java/org/javacs/WarningsTest.java
new file mode 100644
index 0000000..692e985
--- /dev/null
+++ b/src/test/java/org/javacs/WarningsTest.java
@@ -0,0 +1,36 @@
+package org.javacs;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.logging.Logger;
+import org.javacs.lsp.Diagnostic;
+import org.junit.Before;
+import org.junit.Test;
+
+public class WarningsTest {
+    protected static final Logger LOG = Logger.getLogger("main");
+
+    private static List<String> errors = new ArrayList<>();
+
+    protected static final JavaLanguageServer server =
+            LanguageServerFixture.getJavaLanguageServer(WarningsTest::onError);
+
+    private static void onError(Diagnostic error) {
+        var string = String.format("%s(%d)", error.code, error.range.start.line + 1);
+        errors.add(string);
+    }
+
+    @Before
+    public void setup() {
+        errors.clear();
+    }
+
+    @Test
+    public void unusedLocal() {
+        server.reportErrors(List.of(FindResource.uri("org/javacs/warn/Unused.java")));
+        assertThat(errors, hasItem("unused(5)"));
+    }
+}
diff --git a/src/test/test-project/workspace/src/org/javacs/warn/Unused.java b/src/test/test-project/workspace/src/org/javacs/warn/Unused.java
new file mode 100644
index 0000000..9b4f11c
--- /dev/null
+++ b/src/test/test-project/workspace/src/org/javacs/warn/Unused.java
@@ -0,0 +1,7 @@
+package org.javacs.warn;
+
+class Unused {
+    void test() {
+        int unusedLocal = 1;
+    }
+}
\ No newline at end of file