Skip to content

Commit

Permalink
Report "unused ..." diagnostics (eclipse-jdt#623)
Browse files Browse the repository at this point in the history 
* Report diagnostics for the unused imports & private members & local variables
  • Loading branch information
@testforstephen @akurtakov
testforstephen authored and akurtakov committed Aug 16, 2024
1 parent 086c099 commit 3f38435
Show file tree
Hide file tree
Showing 6 changed files with 588 additions and 13 deletions.
79 changes: 79 additions & 0 deletions org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacCompilationUnitResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import javax.lang.model.element.TypeElement;
import javax.tools.Diagnostic;
import javax.tools.DiagnosticListener;
import javax.tools.JavaFileManager;
Expand All @@ -42,6 +43,7 @@
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.Signature;
import org.eclipse.jdt.core.WorkingCopyOwner;
import org.eclipse.jdt.core.compiler.CategorizedProblem;
import org.eclipse.jdt.core.compiler.CharOperation;
import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.core.compiler.InvalidInputException;
Expand All @@ -57,6 +59,7 @@
import org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment;
import org.eclipse.jdt.internal.compiler.lookup.PackageBinding;
import org.eclipse.jdt.internal.compiler.problem.DefaultProblemFactory;
import org.eclipse.jdt.internal.compiler.util.Util;
import org.eclipse.jdt.internal.core.CancelableNameEnvironment;
import org.eclipse.jdt.internal.core.JavaModelManager;
Expand All @@ -65,18 +68,25 @@
import org.eclipse.jdt.internal.core.util.BindingKeyParser;
import org.eclipse.jdt.internal.javac.JavacProblemConverter;
import org.eclipse.jdt.internal.javac.JavacUtils;
import org.eclipse.jdt.internal.javac.UnusedProblemFactory;
import org.eclipse.jdt.internal.javac.UnusedTreeScanner;

import com.sun.source.tree.ClassTree;
import com.sun.source.tree.CompilationUnitTree;
import com.sun.source.tree.Tree;
import com.sun.source.util.JavacTask;
import com.sun.source.util.TaskEvent;
import com.sun.source.util.TaskListener;
import com.sun.tools.javac.api.JavacTool;
import com.sun.tools.javac.api.MultiTaskListener;
import com.sun.tools.javac.code.Symbol.PackageSymbol;
import com.sun.tools.javac.file.JavacFileManager;
import com.sun.tools.javac.parser.JavadocTokenizer;
import com.sun.tools.javac.parser.Scanner;
import com.sun.tools.javac.parser.ScannerFactory;
import com.sun.tools.javac.parser.Tokens.Comment.CommentStyle;
import com.sun.tools.javac.parser.Tokens.TokenKind;
import com.sun.tools.javac.tree.JCTree.JCClassDecl;
import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.DiagnosticSource;
Expand Down Expand Up @@ -470,6 +480,7 @@ private Map<org.eclipse.jdt.internal.compiler.env.ICompilationUnit, CompilationU
Context context = new Context();
Map<org.eclipse.jdt.internal.compiler.env.ICompilationUnit, CompilationUnit> result = new HashMap<>(sourceUnits.length, 1.f);
Map<JavaFileObject, CompilationUnit> filesToUnits = new HashMap<>();
final UnusedProblemFactory unusedProblemFactory = new UnusedProblemFactory(new DefaultProblemFactory(), compilerOptions);
var problemConverter = new JavacProblemConverter(compilerOptions, context);
DiagnosticListener<JavaFileObject> diagnosticListener = diagnostic -> {
findTargetDOM(filesToUnits, diagnostic).ifPresent(dom -> {
Expand All @@ -488,6 +499,63 @@ public void finished(TaskEvent e) {
if (e.getCompilationUnit() instanceof JCCompilationUnit u) {
problemConverter.registerUnit(e.getSourceFile(), u);
}

if (e.getKind() == TaskEvent.Kind.ANALYZE) {
final JavaFileObject file = e.getSourceFile();
final CompilationUnit dom = filesToUnits.get(file);
if (dom == null) {
return;
}

final TypeElement currentTopLevelType = e.getTypeElement();
UnusedTreeScanner<Void, Void> scanner = new UnusedTreeScanner<>() {
@Override
public Void visitClass(ClassTree node, Void p) {
if (node instanceof JCClassDecl classDecl) {
/**
* If a Java file contains multiple top-level types, it will
* trigger multiple ANALYZE taskEvents for the same compilation
* unit. Each ANALYZE taskEvent corresponds to the completion
* of analysis for a single top-level type. Therefore, in the
* ANALYZE task event listener, we only visit the class and nested
* classes that belong to the currently analyzed top-level type.
*/
if (Objects.equals(currentTopLevelType, classDecl.sym)
|| !(classDecl.sym.owner instanceof PackageSymbol)) {
return super.visitClass(node, p);
} else {
return null; // Skip if it does not belong to the currently analyzed top-level type.
}
}

return super.visitClass(node, p);
}
};
final CompilationUnitTree unit = e.getCompilationUnit();
try {
scanner.scan(unit, null);
} catch (Exception ex) {
ILog.get().error("Internal error when visiting the AST Tree. " + ex.getMessage(), ex);
}

List<CategorizedProblem> unusedProblems = scanner.getUnusedPrivateMembers(unusedProblemFactory);
if (!unusedProblems.isEmpty()) {
addProblemsToDOM(dom, unusedProblems);
}

List<CategorizedProblem> unusedImports = scanner.getUnusedImports(unusedProblemFactory);
List<? extends Tree> topTypes = unit.getTypeDecls();
int typeCount = topTypes.size();
// Once all top level types of this Java file have been resolved,
// we can report the unused import to the DOM.
if (typeCount <= 1) {
addProblemsToDOM(dom, unusedImports);
} else if (typeCount > 1 && topTypes.get(typeCount - 1) instanceof JCClassDecl lastType) {
if (Objects.equals(currentTopLevelType, lastType.sym)) {
addProblemsToDOM(dom, unusedImports);
}
}
}
}
});
// must be 1st thing added to context
Expand Down Expand Up @@ -602,6 +670,17 @@ public boolean visit(Javadoc javadoc) {
return result;
}

private void addProblemsToDOM(CompilationUnit dom, Collection<CategorizedProblem> problems) {
IProblem[] previous = dom.getProblems();
IProblem[] newProblems = Arrays.copyOf(previous, previous.length + problems.size());
int start = previous.length;
for (CategorizedProblem problem : problems) {
newProblems[start] = problem;
start++;
}
dom.setProblems(newProblems);
}

private Optional<CompilationUnit> findTargetDOM(Map<JavaFileObject, CompilationUnit> filesToUnits, Object obj) {
if (obj == null) {
return Optional.empty();
Expand Down
32 changes: 32 additions & 0 deletions org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/JavacCompilationResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@

package org.eclipse.jdt.internal.javac;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
import java.util.stream.Stream;

import org.eclipse.jdt.core.compiler.CategorizedProblem;
import org.eclipse.jdt.internal.compiler.CompilationResult;
import org.eclipse.jdt.internal.compiler.env.ICompilationUnit;

Expand All @@ -26,6 +29,8 @@ public class JavacCompilationResult extends CompilationResult {
private Set<String> javacSimpleNameReferences = new TreeSet<>();
private Set<String> javacRootReferences = new TreeSet<>();
private boolean isMigrated = false;
private List<CategorizedProblem> unusedMembers = null;
private List<CategorizedProblem> unusedImports = null;

public JavacCompilationResult(ICompilationUnit compilationUnit) {
this(compilationUnit, 0, 0, Integer.MAX_VALUE);
Expand Down Expand Up @@ -65,4 +70,31 @@ public void migrateReferenceInfo() {
this.javacQualifiedReferences.clear();
this.isMigrated = true;
}

public void setUnusedImports(List<CategorizedProblem> newUnusedImports) {
this.unusedImports = newUnusedImports;
}

public void addUnusedMembers(List<CategorizedProblem> problems) {
if (this.unusedMembers == null) {
this.unusedMembers = new ArrayList<>();
}

this.unusedMembers.addAll(problems);
}

public List<CategorizedProblem> getAdditionalProblems() {
if (this.unusedMembers == null && this.unusedImports == null) {
return null;
}

List<CategorizedProblem> problems = new ArrayList<>();
if (this.unusedImports != null) {
problems.addAll(this.unusedImports);
}
if (this.unusedMembers != null) {
problems.addAll(this.unusedMembers);
}
return problems;
}
}
19 changes: 13 additions & 6 deletions org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/JavacCompiler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.eclipse.core.resources.IResource;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.compiler.CategorizedProblem;
import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.internal.compiler.CompilationResult;
import org.eclipse.jdt.internal.compiler.Compiler;
Expand All @@ -53,11 +54,13 @@

public class JavacCompiler extends Compiler {
JavacConfig compilerConfig;
IProblemFactory problemFactory;

public JavacCompiler(INameEnvironment environment, IErrorHandlingPolicy policy, CompilerConfiguration compilerConfig,
ICompilerRequestor requestor, IProblemFactory problemFactory) {
super(environment, policy, compilerConfig.compilerOptions(), requestor, problemFactory);
this.compilerConfig = JavacConfig.createFrom(compilerConfig);
this.problemFactory = problemFactory;
}

@Override
Expand Down Expand Up @@ -105,7 +108,7 @@ public void compile(ICompilationUnit[] sourceUnits) {
.collect(Collectors.groupingBy(this::computeOutputDirectory));

// Register listener to intercept intermediate results from Javac task.
JavacTaskListener javacListener = new JavacTaskListener(this.compilerConfig, outputSourceMapping);
JavacTaskListener javacListener = new JavacTaskListener(this.compilerConfig, outputSourceMapping, this.problemFactory);
MultiTaskListener mtl = MultiTaskListener.instance(javacContext);
mtl.add(javacListener);

Expand Down Expand Up @@ -167,20 +170,24 @@ public int errorCount() {
for (int i = 0; i < sourceUnits.length; i++) {
ICompilationUnit in = sourceUnits[i];
CompilationResult result = new CompilationResult(in, i, sourceUnits.length, Integer.MAX_VALUE);
List<IProblem> problems = new ArrayList<>();
if (javacListener.getResults().containsKey(in)) {
result = javacListener.getResults().get(in);
((JavacCompilationResult) result).migrateReferenceInfo();
result.unitIndex = i;
result.totalUnitsKnown = sourceUnits.length;
List<CategorizedProblem> additionalProblems = ((JavacCompilationResult) result).getAdditionalProblems();
if (additionalProblems != null && !additionalProblems.isEmpty()) {
problems.addAll(additionalProblems);
}
}

if (javacProblems.containsKey(in)) {
JavacProblem[] problems = javacProblems.get(in).toArray(new JavacProblem[0]);
result.problems = problems; // JavaBuilder is responsible
// for converting the problems
// to IMarkers
result.problemCount = problems.length;
problems.addAll(javacProblems.get(in));
}
// JavaBuilder is responsible for converting the problems to IMarkers
result.problems = problems.toArray(new CategorizedProblem[0]);
result.problemCount = problems.size();
this.requestor.acceptResult(result);
if (result.compiledTypes != null) {
for (Object type : result.compiledTypes.values()) {
Expand Down
25 changes: 18 additions & 7 deletions org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/JavacTaskListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
import javax.tools.JavaFileObject;

import org.eclipse.core.resources.IContainer;
import org.eclipse.core.runtime.ILog;
import org.eclipse.jdt.internal.compiler.ClassFile;
import org.eclipse.jdt.internal.compiler.IProblemFactory;
import org.eclipse.jdt.internal.compiler.env.ICompilationUnit;

import com.sun.source.tree.ClassTree;
Expand All @@ -35,7 +37,6 @@
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.util.TaskEvent;
import com.sun.source.util.TaskListener;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.code.Symbol.PackageSymbol;
Expand All @@ -52,6 +53,7 @@
public class JavacTaskListener implements TaskListener {
private Map<ICompilationUnit, IContainer> sourceOutputMapping = new HashMap<>();
private Map<ICompilationUnit, JavacCompilationResult> results = new HashMap<>();
private UnusedProblemFactory problemFactory;
private static final Set<String> PRIMITIVE_TYPES = new HashSet<String>(Arrays.asList(
"byte",
"short",
Expand All @@ -63,7 +65,9 @@ public class JavacTaskListener implements TaskListener {
"boolean"
));

public JavacTaskListener(JavacConfig config, Map<IContainer, List<ICompilationUnit>> outputSourceMapping) {
public JavacTaskListener(JavacConfig config, Map<IContainer, List<ICompilationUnit>> outputSourceMapping,
IProblemFactory problemFactory) {
this.problemFactory = new UnusedProblemFactory(problemFactory, config.compilerOptions());
for (Entry<IContainer, List<ICompilationUnit>> entry : outputSourceMapping.entrySet()) {
IContainer currentOutput = entry.getKey();
entry.getValue().forEach(cu -> sourceOutputMapping.put(cu, currentOutput));
Expand All @@ -84,9 +88,9 @@ public void finished(TaskEvent e) {
final Map<Symbol, ClassFile> visitedClasses = new HashMap<Symbol, ClassFile>();
final Set<ClassSymbol> hierarchyRecorded = new HashSet<>();
final TypeElement currentTopLevelType = e.getTypeElement();
TreeScanner scanner = new TreeScanner() {
UnusedTreeScanner<Void, Void> scanner = new UnusedTreeScanner<>() {
@Override
public Object visitClass(ClassTree node, Object p) {
public Void visitClass(ClassTree node, Void p) {
if (node instanceof JCClassDecl classDecl) {
/**
* If a Java file contains multiple top-level types, it will
Expand Down Expand Up @@ -116,7 +120,7 @@ public Object visitClass(ClassTree node, Object p) {
}

@Override
public Object visitIdentifier(IdentifierTree node, Object p) {
public Void visitIdentifier(IdentifierTree node, Void p) {
if (node instanceof JCIdent id
&& id.sym instanceof TypeSymbol typeSymbol) {
String qualifiedName = typeSymbol.getQualifiedName().toString();
Expand All @@ -126,7 +130,7 @@ public Object visitIdentifier(IdentifierTree node, Object p) {
}

@Override
public Object visitMemberSelect(MemberSelectTree node, Object p) {
public Void visitMemberSelect(MemberSelectTree node, Void p) {
if (node instanceof JCFieldAccess field) {
if (field.sym != null &&
!(field.type instanceof MethodType || field.type instanceof UnknownType)) {
Expand Down Expand Up @@ -221,7 +225,14 @@ private void recordTypeHierarchy(ClassSymbol classSymbol) {
};

final CompilationUnitTree unit = e.getCompilationUnit();
scanner.scan(unit, null);
try {
scanner.scan(unit, null);
} catch (Exception ex) {
ILog.get().error("Internal error when visiting the AST Tree. " + ex.getMessage(), ex);
}

result.addUnusedMembers(scanner.getUnusedPrivateMembers(this.problemFactory));
result.setUnusedImports(scanner.getUnusedImports(this.problemFactory));
}
}

Expand Down
Loading

0 comments on commit 3f38435

Please sign in to comment.