This is an automated email from the ASF dual-hosted git repository. neilcsmith pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/netbeans.git
The following commit(s) were added to refs/heads/master by this push: new c8ead04 Separating unused element detection out of the semantic highlighter; adding a separate hint for unused elements. new 5c5c23e Merge pull request #2207 from jlahoda/unused-hint c8ead04 is described below commit c8ead044e5e6ab836e2996d3b42f2bc943868e7b Author: Jan Lahoda <jlah...@netbeans.org> AuthorDate: Sun Jun 21 09:01:41 2020 +0200 Separating unused element detection out of the semantic highlighter; adding a separate hint for unused elements. --- .../base/semantic/SemanticHighlighterBase.java | 284 +----------- .../java/editor/base/semantic/UnusedDetector.java | 486 +++++++++++++++++++++ .../java/editor/base/semantic/Utilities.java | 17 + .../base/semantic/DetectorTest/test89356.pass | 2 +- .../DetectorTest/testConstructorUsedBySuper1.pass | 4 +- .../DetectorTest/testConstructorUsedBySuper2.pass | 4 +- .../DetectorTest/testLambdaAndFunctionType.pass | 4 +- .../semantic/data/ConstructorUsedBySuper1.java | 2 +- .../semantic/data/ConstructorUsedBySuper2.java | 2 +- .../java/editor/base/semantic/DetectorTest.java | 6 +- .../java/editor/base/semantic/TestBase.java | 48 +- .../editor/base/semantic/UnusedDetectorTest.java | 377 ++++++++++++++++ .../netbeans/modules/java/hints/bugs/Unused.java | 85 ++++ .../modules/java/hints/bugs/UnusedTest.java | 53 +++ 14 files changed, 1078 insertions(+), 296 deletions(-) diff --git a/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/SemanticHighlighterBase.java b/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/SemanticHighlighterBase.java index 00d3687..00e115d 100644 --- a/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/SemanticHighlighterBase.java +++ b/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/SemanticHighlighterBase.java @@ -18,15 +18,10 @@ */ package org.netbeans.modules.java.editor.base.semantic; -import com.sun.source.tree.ArrayTypeTree; -import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ClassTree; import com.sun.source.tree.CompilationUnitTree; -import com.sun.source.tree.CompoundAssignmentTree; -import com.sun.source.tree.EnhancedForLoopTree; import com.sun.source.tree.ExportsTree; import com.sun.source.tree.IdentifierTree; -import com.sun.source.tree.LambdaExpressionTree; import com.sun.source.tree.LiteralTree; import com.sun.source.tree.MemberReferenceTree; import com.sun.source.tree.MemberSelectTree; @@ -44,13 +39,11 @@ import com.sun.source.tree.UsesTree; import com.sun.source.tree.VariableTree; import com.sun.source.util.SourcePositions; import com.sun.source.util.TreePath; -import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.EnumSet; import java.util.HashMap; -import java.util.HashSet; import java.util.IdentityHashMap; import java.util.List; import java.util.Map; @@ -63,13 +56,9 @@ import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.Modifier; -import javax.lang.model.element.TypeElement; -import javax.lang.model.type.TypeKind; -import javax.lang.model.type.TypeMirror; import javax.swing.text.Document; import org.netbeans.api.java.lexer.JavaTokenId; import org.netbeans.api.java.source.CompilationInfo; -import org.netbeans.api.java.source.ElementHandle; import org.netbeans.api.java.source.JavaParserResultTask; import org.netbeans.api.java.source.JavaSource.Phase; import org.netbeans.api.java.source.TreeUtilities; @@ -77,15 +66,14 @@ import org.netbeans.api.java.source.support.CancellableTreePathScanner; import org.netbeans.api.lexer.PartType; import org.netbeans.api.lexer.Token; import org.netbeans.api.lexer.TokenHierarchy; -//import org.netbeans.modules.editor.NbEditorUtilities; import org.netbeans.modules.java.editor.base.imports.UnusedImports; import org.netbeans.modules.java.editor.base.semantic.ColoringAttributes.Coloring; +import org.netbeans.modules.java.editor.base.semantic.UnusedDetector.UnusedDescription; import org.netbeans.modules.parsing.spi.Parser.Result; import org.netbeans.modules.parsing.spi.Scheduler; import org.netbeans.modules.parsing.spi.SchedulerEvent; import org.netbeans.modules.parsing.spi.TaskIndexingMode; import org.openide.filesystems.FileUtil; -import org.openide.util.Exceptions; import org.openide.util.Pair; @@ -152,95 +140,6 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { protected abstract boolean process(CompilationInfo info, final Document doc); - /** - * Signatures of Serializable methods. - */ - private static final Set<String> SERIALIZABLE_SIGNATURES = new HashSet<>(Arrays.asList(new String[] { - "writeObject(Ljava/io/ObjectOutputStream;)V", - "readObject(Ljava/io/ObjectInputStream;)V", - "readResolve()Ljava/lang/Object;", - "writeReplace()Ljava/lang/Object;", - "readObjectNoData()V", - })); - - /** - * Also returns true on error / undecidable situation, so the filtering - * will probably accept serial methods and will not mark them as unused, if - * the class declaration is errneous. - * - * @param info the compilation context - * @param e the class member (the enclosing element will be tested) - * @return true, if in serializable/externalizable or unknown - */ - private static boolean isInSerializableOrExternalizable(CompilationInfo info, Element e) { - Element encl = e.getEnclosingElement(); - if (encl == null || !encl.getKind().isClass()) { - return true; - } - TypeMirror m = encl.asType(); - if (m == null || m.getKind() != TypeKind.DECLARED) { - return true; - } - Element serEl = info.getElements().getTypeElement("java.io.Serializable"); // NOI18N - Element extEl = info.getElements().getTypeElement("java.io.Externalizable"); // NOI18N - if (serEl == null || extEl == null) { - return true; - } - if (info.getTypes().isSubtype(m, serEl.asType())) { - return true; - } - if (info.getTypes().isSubtype(m, extEl.asType())) { - return true; - } - return false; - } - - private static Field signatureAccessField; - - /** - * Hack to get signature out of ElementHandle - there's no API method for that - */ - private static String _getSignatureHack(ElementHandle<ExecutableElement> eh) { - try { - if (signatureAccessField == null) { - try { - Field f = ElementHandle.class.getDeclaredField("signatures"); // NOI18N - f.setAccessible(true); - signatureAccessField = f; - } catch (NoSuchFieldException | SecurityException ex) { - // ignore - return ""; // NOI18N - } - } - String[] signs = (String[])signatureAccessField.get(eh); - if (signs == null || signs.length != 3) { - return ""; // NOI18N - } else { - return signs[1] + signs[2]; - } - } catch (IllegalArgumentException | IllegalAccessException ex) { - return ""; // NOI18N - } - } - - /** - * Checks if the method is specified by Serialization API and the class - * extends Serializable/Externalizable. Unused methods defined in API spec - * should not be marked as unused. - * - * @param info compilation context - * @param method the method - * @return true, if the method is from serialization API and should not be reported - */ - private boolean isSerializationMethod(CompilationInfo info, ExecutableElement method) { - if (!isInSerializableOrExternalizable(info, method)) { - return false; - } - ElementHandle<ExecutableElement> eh = ElementHandle.create(method); - String sign = _getSignatureHack(eh); - return SERIALIZABLE_SIGNATURES.contains(sign); - } - protected boolean process(CompilationInfo info, final Document doc, ErrorDescriptionSetter setter) { DetectorVisitor v = new DetectorVisitor(info, doc, cancel); @@ -277,6 +176,9 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { } } + Map<Element, List<UnusedDescription>> element2Unused = UnusedDetector.findUnused(info) //XXX: unnecessarily ugly + .stream() + .collect(Collectors.groupingBy(ud -> ud.unusedElement)); for (Element decl : v.type2Uses.keySet()) { if (cancel.get()) return true; @@ -287,23 +189,9 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { if (u.spec == null) continue; - if (u.type.contains(UseTypes.DECLARATION) && Utilities.isPrivateElement(decl)) { - if ((decl.getKind().isField() && !isSerialSpecField(info, decl)) || isLocalVariableClosure(decl)) { - if (!hasAllTypes(uses, EnumSet.of(UseTypes.READ, UseTypes.WRITE))) { - u.spec.add(ColoringAttributes.UNUSED); - } - } - - if ((decl.getKind() == ElementKind.CONSTRUCTOR && !decl.getModifiers().contains(Modifier.PRIVATE)) || decl.getKind() == ElementKind.METHOD) { - if (!(hasAllTypes(uses, EnumSet.of(UseTypes.EXECUTE)) || isSerializationMethod(info, (ExecutableElement)decl))) { - u.spec.add(ColoringAttributes.UNUSED); - } - } - - if (decl.getKind().isClass() || decl.getKind().isInterface()) { - if (!hasAllTypes(uses, EnumSet.of(UseTypes.CLASS_USE))) { - u.spec.add(ColoringAttributes.UNUSED); - } + if (u.declaration) { + if (element2Unused.containsKey(decl)) { + u.spec.add(ColoringAttributes.UNUSED); } } @@ -336,23 +224,6 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { return false; } - - private boolean hasAllTypes(List<Use> uses, EnumSet<UseTypes> types) { - for (Use u : uses) { - if (types.isEmpty()) { - return true; - } - - types.removeAll(u.type); - } - - return types.isEmpty(); - } - - private enum UseTypes { - READ, WRITE, EXECUTE, DECLARATION, CLASS_USE, MODULE_USE; - } - private static Coloring collection2Coloring(Collection<ColoringAttributes> attr) { Coloring c = ColoringAttributes.empty(); @@ -383,41 +254,20 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { LOCAL_VARIABLES.contains(el.getKind()); } - /** Detects static final long SerialVersionUID - * @return true if element is final static long serialVersionUID - */ - private static boolean isSerialSpecField(CompilationInfo info, Element el) { - if (el.getModifiers().contains(Modifier.FINAL) - && el.getModifiers().contains(Modifier.STATIC)) { - - if (!isInSerializableOrExternalizable(info, el)) { - return false; - } - if (info.getTypes().getPrimitiveType(TypeKind.LONG).equals(el.asType()) - && el.getSimpleName().toString().equals("serialVersionUID")) { - return true; - } - if (el.getSimpleName().contentEquals("serialPersistentFields")) { - return true; - } - } - return false; - } - private static class Use { - private Collection<UseTypes> type; + private boolean declaration; private TreePath tree; private Collection<ColoringAttributes> spec; - public Use(Collection<UseTypes> type, TreePath tree, Collection<ColoringAttributes> spec) { - this.type = type; + public Use(boolean declaration, TreePath tree, Collection<ColoringAttributes> spec) { + this.declaration = declaration; this.tree = tree; this.spec = spec; } @Override public String toString() { - return "Use: " + type; + return "Use: " + spec; } } @@ -582,23 +432,6 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { return null; } - private Element toRecordComponent(Element el) { - if (el == null ||el.getKind() != ElementKind.FIELD) { - return el; - } - TypeElement owner = (TypeElement) el.getEnclosingElement(); - if (!"RECORD".equals(owner.getKind().name())) { - return el; - } - for (Element encl : owner.getEnclosedElements()) { - if (TreeShims.isRecordComponent(encl.getKind()) && - encl.getSimpleName().equals(el.getSimpleName())) { - return encl; - } - } - return el; - } - private static final Set<Kind> LITERALS = EnumSet.of(Kind.BOOLEAN_LITERAL, Kind.CHAR_LITERAL, Kind.DOUBLE_LITERAL, Kind.FLOAT_LITERAL, Kind.INT_LITERAL, Kind.LONG_LITERAL, Kind.STRING_LITERAL); private void handlePossibleIdentifier(TreePath expr, boolean declaration) { @@ -621,7 +454,7 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { return ; } - decl = decl == null ? toRecordComponent(info.getTrees().getElement(expr)) : decl; + decl = decl == null ? Utilities.toRecordComponent(info.getTrees().getElement(expr)) : decl; ElementKind declKind = decl != null ? decl.getKind() : null; boolean isDeclType = decl != null && @@ -689,98 +522,23 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { } if (c != null) { - Collection<UseTypes> type = EnumSet.noneOf(UseTypes.class); - - if (isDeclType) { - if (!declaration) { - type.add(UseTypes.CLASS_USE); + if (decl.getKind() == ElementKind.CONSTRUCTOR && !declaration) { + if (info.getElements().isDeprecated(decl.getEnclosingElement())) { + c.add(ColoringAttributes.DEPRECATED); } - } else if (decl.getKind().isField() || isLocalVariableClosure(decl)) { - if (!declaration) { - while (true) { - if (parent.getLeaf().getKind() == Kind.POSTFIX_DECREMENT || - parent.getLeaf().getKind() == Kind.POSTFIX_INCREMENT || - parent.getLeaf().getKind() == Kind.PREFIX_DECREMENT || - parent.getLeaf().getKind() == Kind.PREFIX_INCREMENT) { - type.add(UseTypes.WRITE); - currentPath = parent; - parent = currentPath.getParentPath(); - continue; - } - if (CompoundAssignmentTree.class.isAssignableFrom(parent.getLeaf().getKind().asInterface()) && - ((CompoundAssignmentTree) parent.getLeaf()).getVariable() == currentPath.getLeaf()) { - type.add(UseTypes.WRITE); - currentPath = parent; - parent = currentPath.getParentPath(); - continue; - } - break; - } - if (parent.getLeaf().getKind() == Kind.ASSIGNMENT && - ((AssignmentTree) parent.getLeaf()).getVariable() == currentPath.getLeaf()) { - type.add(UseTypes.WRITE); - } else if (parent.getLeaf().getKind() != Kind.EXPRESSION_STATEMENT) { - type.add(UseTypes.READ); - } - } else if (decl.getKind() == ElementKind.PARAMETER) { - Element method = decl.getEnclosingElement(); - - type.add(UseTypes.WRITE); - - if (parent.getLeaf().getKind() == Kind.LAMBDA_EXPRESSION && - ((LambdaExpressionTree) parent.getLeaf()).getParameters().contains(currentPath.getLeaf())) { -// type.add(UseTypes.READ); - } else if (method.getModifiers().contains(Modifier.ABSTRACT) || method.getModifiers().contains(Modifier.NATIVE) || !method.getModifiers().contains(Modifier.PRIVATE)) { - type.add(UseTypes.READ); - } - } else if (decl.getKind().isField() || decl.getKind() == ElementKind.EXCEPTION_PARAMETER || decl.getKind() == BINDING_VARIABLE) { - type.add(UseTypes.WRITE); - } else if (parent.getLeaf().getKind() == Kind.ENHANCED_FOR_LOOP && - ((EnhancedForLoopTree) parent.getLeaf()).getVariable() == currentPath.getLeaf()) { - type.add(UseTypes.WRITE); - } else { - VariableTree vt = (VariableTree) currentPath.getLeaf(); - - if (vt.getInitializer() != null) { - type.add(UseTypes.WRITE); - } - } - } else if (decl.getKind() == ElementKind.METHOD) { - if (!declaration) { - type.add(UseTypes.EXECUTE); - } - } else if (decl.getKind() == ElementKind.CONSTRUCTOR) { - if (!declaration) { - if (info.getElements().isDeprecated(decl.getEnclosingElement())) { - c.add(ColoringAttributes.DEPRECATED); - } - type.add(UseTypes.EXECUTE); - } - } else if (TreeShims.isRecordComponent(toRecordComponent(decl).getKind())) { - if (declaration) { - type.add(UseTypes.READ); - type.add(UseTypes.WRITE); - } - } - if (declaration) { - type.add(UseTypes.DECLARATION); } - addUse(decl, type, expr, c); + addUse(decl, declaration, expr, c); } } - private void addUse(Element decl, Collection<UseTypes> useTypes, TreePath t, Collection<ColoringAttributes> c) { - if (decl == recursionDetector) { - useTypes.remove(UseTypes.EXECUTE); //recursive execution is not use - } - + private void addUse(Element decl, boolean declaration, TreePath t, Collection<ColoringAttributes> c) { List<Use> uses = type2Uses.get(decl); if (uses == null) { type2Uses.put(decl, uses = new ArrayList<Use>()); } - Use u = new Use(useTypes, t, c); + Use u = new Use(declaration, t, c); uses.add(u); } @@ -924,7 +682,7 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { if ("super".equals(ident) || "this".equals(ident)) { //NOI18N Element resolved = info.getTrees().getElement(getCurrentPath()); - addUse(resolved, EnumSet.of(UseTypes.EXECUTE), null, null); + addUse(resolved, false, null, null); } } @@ -1076,7 +834,7 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { Element clazz = info.getTrees().getElement(tp); if (clazz != null) { - addUse(clazz, EnumSet.of(UseTypes.CLASS_USE), null, null); + addUse(clazz, false, null, null); } scan(tree.getEnclosingExpression(), null); @@ -1133,7 +891,7 @@ public abstract class SemanticHighlighterBase extends JavaParserResultTask { private boolean isRecordComponent(Tree member) { Element el = info.getTrees().getElement(new TreePath(getCurrentPath(), member)); - return el != null && TreeShims.isRecordComponent(toRecordComponent(el).getKind()); + return el != null && TreeShims.isRecordComponent(Utilities.toRecordComponent(el).getKind()); } @Override diff --git a/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/UnusedDetector.java b/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/UnusedDetector.java new file mode 100644 index 0000000..a458ba5 --- /dev/null +++ b/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/UnusedDetector.java @@ -0,0 +1,486 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.java.editor.base.semantic; + +import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.CompoundAssignmentTree; +import com.sun.source.tree.EnhancedForLoopTree; +import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.MemberReferenceTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.NewClassTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.TreePath; +import com.sun.source.util.TreePathScanner; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.Modifier; +import javax.lang.model.element.TypeElement; +import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeMirror; +import org.netbeans.api.java.source.CompilationInfo; +import org.netbeans.api.java.source.ElementHandle; +import org.netbeans.api.java.source.support.ErrorAwareTreePathScanner; + +/** + * + * @author lahvac + */ +public class UnusedDetector { + + public static class UnusedDescription { + public final Element unusedElement; + public final TreePath unusedElementPath; + public final UnusedReason reason; + + public UnusedDescription(Element unusedElement, TreePath unusedElementPath, UnusedReason reason) { + this.unusedElement = unusedElement; + this.unusedElementPath = unusedElementPath; + this.reason = reason; + } + + } + + public enum UnusedReason { + NOT_WRITTEN_READ("neither read or written to"), + NOT_WRITTEN("never written to"), //makes sense? + NOT_READ("never read"), + NOT_USED("never used"); + private final String text; + + private UnusedReason(String text) { + this.text = text; + } + + } + + public static List<UnusedDescription> findUnused(CompilationInfo info) { + List<UnusedDescription> cached = (List<UnusedDescription>) info.getCachedValue(UnusedDetector.class); + if (cached != null) { + return cached; + } + + UnusedVisitor uv = new UnusedVisitor(info); + uv.scan(info.getCompilationUnit(), null); + List<UnusedDescription> result = new ArrayList<>(); + for (Entry<Element, TreePath> e : uv.element2Declaration.entrySet()) { + Element el = e.getKey(); + TreePath declaration = e.getValue(); + Set<UseTypes> uses = uv.useTypes.getOrDefault(el, Collections.emptySet()); + boolean isPrivate = el.getModifiers().contains(Modifier.PRIVATE); //TODO: effectivelly private! + if (isLocalVariableClosure(el) || (el.getKind().isField() && isPrivate)) { + if (!isSerialSpecField(info, el)) { + boolean isWritten = uses.contains(UseTypes.WRITTEN); + boolean isRead = uses.contains(UseTypes.READ); + if (!isWritten && !isRead) { + result.add(new UnusedDescription(el, declaration, UnusedReason.NOT_WRITTEN_READ)); + } else if (!isWritten) { + result.add(new UnusedDescription(el, declaration, UnusedReason.NOT_WRITTEN)); + } else if (!isRead) { + result.add(new UnusedDescription(el, declaration, UnusedReason.NOT_READ)); + } + } + } else if ((el.getKind() == ElementKind.CONSTRUCTOR || el.getKind() == ElementKind.METHOD) && isPrivate) { + if (!isSerializationMethod(info, (ExecutableElement)el) && !uses.contains(UseTypes.USED)) { + result.add(new UnusedDescription(el, declaration, UnusedReason.NOT_USED)); + } + } else if ((el.getKind().isClass() || el.getKind().isInterface()) && isPrivate) { + if (!uses.contains(UseTypes.USED)) { + result.add(new UnusedDescription(el, declaration, UnusedReason.NOT_USED)); + } + } + } + + info.putCachedValue(UnusedDetector.class, result, CompilationInfo.CacheClearPolicy.ON_CHANGE); + + return result; + } + + /** Detects static final long SerialVersionUID + * @return true if element is final static long serialVersionUID + */ + private static boolean isSerialSpecField(CompilationInfo info, Element el) { + if (el.getModifiers().contains(Modifier.FINAL) + && el.getModifiers().contains(Modifier.STATIC)) { + + if (!isInSerializableOrExternalizable(info, el)) { + return false; + } + if (info.getTypes().getPrimitiveType(TypeKind.LONG).equals(el.asType()) + && el.getSimpleName().toString().equals("serialVersionUID")) { + return true; + } + if (el.getSimpleName().contentEquals("serialPersistentFields")) { + return true; + } + } + return false; + } + + /** + * Also returns true on error / undecidable situation, so the filtering + * will probably accept serial methods and will not mark them as unused, if + * the class declaration is errneous. + * + * @param info the compilation context + * @param e the class member (the enclosing element will be tested) + * @return true, if in serializable/externalizable or unknown + */ + private static boolean isInSerializableOrExternalizable(CompilationInfo info, Element e) { + Element encl = e.getEnclosingElement(); + if (encl == null || !encl.getKind().isClass()) { + return true; + } + TypeMirror m = encl.asType(); + if (m == null || m.getKind() != TypeKind.DECLARED) { + return true; + } + Element serEl = info.getElements().getTypeElement("java.io.Serializable"); // NOI18N + Element extEl = info.getElements().getTypeElement("java.io.Externalizable"); // NOI18N + if (serEl == null || extEl == null) { + return true; + } + if (info.getTypes().isSubtype(m, serEl.asType())) { + return true; + } + if (info.getTypes().isSubtype(m, extEl.asType())) { + return true; + } + return false; + } + + private static Field signatureAccessField; + + /** + * Hack to get signature out of ElementHandle - there's no API method for that + */ + private static String _getSignatureHack(ElementHandle<ExecutableElement> eh) { + try { + if (signatureAccessField == null) { + try { + Field f = ElementHandle.class.getDeclaredField("signatures"); // NOI18N + f.setAccessible(true); + signatureAccessField = f; + } catch (NoSuchFieldException | SecurityException ex) { + // ignore + return ""; // NOI18N + } + } + String[] signs = (String[])signatureAccessField.get(eh); + if (signs == null || signs.length != 3) { + return ""; // NOI18N + } else { + return signs[1] + signs[2]; + } + } catch (IllegalArgumentException | IllegalAccessException ex) { + return ""; // NOI18N + } + } + + /** + * Checks if the method is specified by Serialization API and the class + * extends Serializable/Externalizable. Unused methods defined in API spec + * should not be marked as unused. + * + * @param info compilation context + * @param method the method + * @return true, if the method is from serialization API and should not be reported + */ + private static boolean isSerializationMethod(CompilationInfo info, ExecutableElement method) { + if (!isInSerializableOrExternalizable(info, method)) { + return false; + } + ElementHandle<ExecutableElement> eh = ElementHandle.create(method); + String sign = _getSignatureHack(eh); + return SERIALIZABLE_SIGNATURES.contains(sign); + } + + /** + * Signatures of Serializable methods. + */ + private static final Set<String> SERIALIZABLE_SIGNATURES = new HashSet<>(Arrays.asList(new String[] { + "writeObject(Ljava/io/ObjectOutputStream;)V", + "readObject(Ljava/io/ObjectInputStream;)V", + "readResolve()Ljava/lang/Object;", + "writeReplace()Ljava/lang/Object;", + "readObjectNoData()V", + })); + + private static final Set<ElementKind> LOCAL_VARIABLES = EnumSet.of( + ElementKind.LOCAL_VARIABLE, ElementKind.RESOURCE_VARIABLE, + ElementKind.EXCEPTION_PARAMETER); + private static final ElementKind BINDING_VARIABLE; + + static { + ElementKind bindingVariable; + try { + LOCAL_VARIABLES.add(bindingVariable = ElementKind.valueOf(TreeShims.BINDING_VARIABLE)); + } catch (IllegalArgumentException ex) { + bindingVariable = null; + } + BINDING_VARIABLE = bindingVariable; + } + + private static boolean isLocalVariableClosure(Element el) { + return el.getKind() == ElementKind.PARAMETER || + LOCAL_VARIABLES.contains(el.getKind()); + } + + private enum UseTypes { + READ, WRITTEN, USED; + } + + private static final class UnusedVisitor extends ErrorAwareTreePathScanner<Void, Void> { + + private final Map<Element, Set<UseTypes>> useTypes = new HashMap<>(); + private final Map<Element, TreePath> element2Declaration = new HashMap<>(); + private final CompilationInfo info; + private ExecutableElement recursionDetector; + + public UnusedVisitor(CompilationInfo info) { + this.info = info; + } + + @Override + public Void visitIdentifier(IdentifierTree node, Void p) { + handleUse(); + return super.visitIdentifier(node, p); + } + + @Override + public Void visitMemberSelect(MemberSelectTree node, Void p) { + handleUse(); + return super.visitMemberSelect(node, p); + } + + @Override + public Void visitMemberReference(MemberReferenceTree node, Void p) { + handleUse(); + return super.visitMemberReference(node, p); + } + + @Override + public Void visitNewClass(NewClassTree node, Void p) { + handleUse(); + return super.visitNewClass(node, p); + } + + private void handleUse() { + Element el = info.getTrees().getElement(getCurrentPath()); + + if (el == null) { + return ; + } + + boolean isPrivate = el.getModifiers().contains(Modifier.PRIVATE); //TODO: effectivelly private! + + if (isLocalVariableClosure(el) || (el.getKind().isField() && isPrivate)) { + TreePath effectiveUse = getCurrentPath(); + boolean isWrite = false; + boolean isRead = false; + + OUTER: while (true) { + TreePath parent = effectiveUse.getParentPath(); + + switch (parent.getLeaf().getKind()) { + case ASSIGNMENT: + AssignmentTree at = (AssignmentTree) parent.getLeaf(); + if (at.getVariable() == effectiveUse.getLeaf()) { + isWrite = true; + } else if (at.getExpression() == effectiveUse.getLeaf()) { + isRead = true; + } + break OUTER; + case AND_ASSIGNMENT: case DIVIDE_ASSIGNMENT: + case LEFT_SHIFT_ASSIGNMENT: case MINUS_ASSIGNMENT: + case MULTIPLY_ASSIGNMENT: case OR_ASSIGNMENT: + case PLUS_ASSIGNMENT: case REMAINDER_ASSIGNMENT: + case RIGHT_SHIFT_ASSIGNMENT: case UNSIGNED_RIGHT_SHIFT_ASSIGNMENT: + case XOR_ASSIGNMENT: + CompoundAssignmentTree cat = (CompoundAssignmentTree) parent.getLeaf(); + if (cat.getVariable() == effectiveUse.getLeaf()) { + //check if the results of compound assignment is used: + effectiveUse = parent; + break; + } + //use on the right hand side of the compound assignment - consider as read + isRead = true; + break OUTER; + case EXPRESSION_STATEMENT: + break OUTER; + default: + isRead = true; + break OUTER; + } + } + + if (isWrite) { + addUse(el, UseTypes.WRITTEN); + } + + if (isRead) { + addUse(el, UseTypes.READ); + } + } else if (isPrivate) { + if (el.getKind() != ElementKind.METHOD || recursionDetector != el) + addUse(el, UseTypes.USED); + } + } + + private void addUse(Element el, UseTypes type) { + useTypes.computeIfAbsent(el, x -> EnumSet.noneOf(UseTypes.class)).add(type); + } + + @Override + public Void visitClass(ClassTree node, Void p) { + ExecutableElement prevRecursionDetector = recursionDetector; + + try { + recursionDetector = null; + + handleDeclaration(getCurrentPath()); + return super.visitClass(node, p); + } finally { + recursionDetector = prevRecursionDetector; + } + } + + @Override + public Void visitVariable(VariableTree node, Void p) { + handleDeclaration(getCurrentPath()); + return super.visitVariable(node, p); + } + + @Override + public Void visitMethod(MethodTree node, Void p) { + ExecutableElement prevRecursionDetector = recursionDetector; + + try { + Element el = info.getTrees().getElement(getCurrentPath()); + recursionDetector = (el != null && el.getKind() == ElementKind.METHOD) ? (ExecutableElement) el : null; + handleDeclaration(getCurrentPath()); + return super.visitMethod(node, p); + } finally { + recursionDetector = prevRecursionDetector; + } + } + + @Override + public Void scan(Tree tree, Void p) { + if (tree != null && TreeShims.BINDING_PATTERN.equals(tree.getKind().name())) { + handleDeclaration(new TreePath(getCurrentPath(), tree)); + } + return super.scan(tree, p); + } + + private void handleDeclaration(TreePath path) { + Element el = info.getTrees().getElement(path); + + if (el == null) { + return ; + } + + element2Declaration.put(el, path); + + if (el.getKind() == ElementKind.PARAMETER) { + addUse(el, UseTypes.WRITTEN); + boolean read = true; + Tree parent = path.getParentPath().getLeaf(); + if (parent.getKind() == Kind.METHOD) { + MethodTree method = (MethodTree) parent; + Set<Modifier> mods = method.getModifiers().getFlags(); + if (method.getParameters().contains(path.getLeaf()) && + mods.contains(Modifier.PRIVATE) && !mods.contains(Modifier.ABSTRACT) && + !mods.contains(Modifier.NATIVE)) { + read = false; + } + } + if (read) { + addUse(el, UseTypes.READ); + } + } else if (el.getKind() == ElementKind.EXCEPTION_PARAMETER) { + //Ignore unread caught exceptions. There are valid reasons why it + //could be unused; and there should be a separate hint checking if + //it makes sense to no use it: + addUse(el, UseTypes.READ); + addUse(el, UseTypes.WRITTEN); + } else if (el.getKind() == BINDING_VARIABLE) { + addUse(el, UseTypes.WRITTEN); + } else if (el.getKind() == ElementKind.LOCAL_VARIABLE) { + Tree parent = path.getParentPath().getLeaf(); + if (parent.getKind() == Kind.ENHANCED_FOR_LOOP && + ((EnhancedForLoopTree) parent).getVariable() == path.getLeaf()) { + addUse(el, UseTypes.WRITTEN); + } + } else if (TreeShims.isRecordComponent(Utilities.toRecordComponent(el).getKind())) { + addUse(el, UseTypes.READ); + addUse(el, UseTypes.WRITTEN); + } else if (el.getKind().isField()) { + addUse(el, UseTypes.WRITTEN); + } else if (el.getKind() == ElementKind.CONSTRUCTOR && + el.getModifiers().contains(Modifier.PRIVATE) && + ((ExecutableElement) el).getParameters().isEmpty()) { + //check if this constructor prevent initalization of "utility" class, + //in which case, it is not "unused": + TypeElement encl = (TypeElement) el.getEnclosingElement(); + TypeElement jlObject = info.getElements().getTypeElement("java.lang.Object"); + boolean utility = !encl.getModifiers().contains(Modifier.ABSTRACT) && + encl.getInterfaces().isEmpty() && + (jlObject == null || info.getTypes().isSameType(encl.getSuperclass(), jlObject.asType())); + for (Element sibling : el.getEnclosingElement().getEnclosedElements()) { + if (sibling.getKind() == ElementKind.CONSTRUCTOR && !sibling.equals(el)) { + utility = false; + break; + } else if ((sibling.getKind().isField() || sibling.getKind() == ElementKind.METHOD) && + !sibling.getModifiers().contains(Modifier.STATIC)) { + utility = false; + break; + } + } + if (utility) { + addUse(el, UseTypes.USED); + } + } + + if (path.getLeaf().getKind() == Kind.VARIABLE) { + VariableTree vt = (VariableTree) path.getLeaf(); + if (vt.getInitializer() != null) { + addUse(el, UseTypes.WRITTEN); + } + } + } + } +} diff --git a/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/Utilities.java b/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/Utilities.java index e741427..bce68e1 100644 --- a/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/Utilities.java +++ b/java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/Utilities.java @@ -56,6 +56,7 @@ import javax.lang.model.element.Name; import javax.tools.Diagnostic; import com.sun.source.tree.ModifiersTree; +import javax.lang.model.element.TypeElement; import org.netbeans.api.java.lexer.JavaTokenId; import org.netbeans.api.java.source.TreeUtilities; import org.netbeans.api.lexer.Token; @@ -722,5 +723,21 @@ public class Utilities { return LOCAL_ELEMENT_KINDS.contains(el.getKind()) || el.getModifiers().contains(Modifier.PRIVATE); } + public static Element toRecordComponent(Element el) { + if (el == null ||el.getKind() != ElementKind.FIELD) { + return el; + } + TypeElement owner = (TypeElement) el.getEnclosingElement(); + if (!"RECORD".equals(owner.getKind().name())) { + return el; + } + for (Element encl : owner.getEnclosedElements()) { + if (TreeShims.isRecordComponent(encl.getKind()) && + encl.getSimpleName().equals(el.getSimpleName())) { + return encl; + } + } + return el; + } } diff --git a/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/test89356.pass b/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/test89356.pass index 82e4188..d660bf4 100644 --- a/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/test89356.pass +++ b/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/test89356.pass @@ -2,4 +2,4 @@ [PUBLIC, CLASS, DECLARATION], 4:13-4:34 [PUBLIC, INTERFACE], 4:46-4:58 [STATIC, PRIVATE, FIELD, DECLARATION], 5:30-5:46 -[PRIVATE, CONSTRUCTOR, DECLARATION], 7:12-7:33 +[PRIVATE, CONSTRUCTOR, UNUSED, DECLARATION], 7:12-7:33 diff --git a/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testConstructorUsedBySuper1.pass b/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testConstructorUsedBySuper1.pass index cb4b9b5..d86206d 100644 --- a/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testConstructorUsedBySuper1.pass +++ b/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testConstructorUsedBySuper1.pass @@ -2,7 +2,7 @@ [PRIVATE, CONSTRUCTOR, DECLARATION], 4:12-4:35 [STATIC, PUBLIC, CLASS, DECLARATION], 8:24-8:32 [PUBLIC, CLASS], 8:41-8:64 -[PRIVATE, CONSTRUCTOR, DECLARATION], 9:16-9:24 +[PUBLIC, CONSTRUCTOR, DECLARATION], 9:15-9:23 [STATIC, PUBLIC, CLASS], 13:22-13:30 [STATIC, PUBLIC, METHOD, DECLARATION], 13:31-13:37 -[PRIVATE, CONSTRUCTOR], 14:23-14:31 +[PUBLIC, CONSTRUCTOR], 14:23-14:31 diff --git a/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testConstructorUsedBySuper2.pass b/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testConstructorUsedBySuper2.pass index cb4b9b5..d86206d 100644 --- a/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testConstructorUsedBySuper2.pass +++ b/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testConstructorUsedBySuper2.pass @@ -2,7 +2,7 @@ [PRIVATE, CONSTRUCTOR, DECLARATION], 4:12-4:35 [STATIC, PUBLIC, CLASS, DECLARATION], 8:24-8:32 [PUBLIC, CLASS], 8:41-8:64 -[PRIVATE, CONSTRUCTOR, DECLARATION], 9:16-9:24 +[PUBLIC, CONSTRUCTOR, DECLARATION], 9:15-9:23 [STATIC, PUBLIC, CLASS], 13:22-13:30 [STATIC, PUBLIC, METHOD, DECLARATION], 13:31-13:37 -[PRIVATE, CONSTRUCTOR], 14:23-14:31 +[PUBLIC, CONSTRUCTOR], 14:23-14:31 diff --git a/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testLambdaAndFunctionType.pass b/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testLambdaAndFunctionType.pass index ca70386..3a44978 100644 --- a/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testLambdaAndFunctionType.pass +++ b/java/java.editor.base/test/unit/data/goldenfiles/org/netbeans/modules/java/editor/base/semantic/DetectorTest/testLambdaAndFunctionType.pass @@ -15,7 +15,7 @@ [PUBLIC, CLASS], 9:40-9:46 [PARAMETER, DECLARATION], 9:47-9:49 [PUBLIC, CLASS], 9:51-9:57 -[PARAMETER, UNUSED, DECLARATION], 9:58-9:60 +[PARAMETER, DECLARATION], 9:58-9:60 [PARAMETER], 9:73-9:75 [PUBLIC, METHOD], 9:76-9:85 [PUBLIC, CLASS], 10:8-10:19 @@ -24,7 +24,7 @@ [PUBLIC, CLASS], 10:40-10:46 [PARAMETER, DECLARATION], 10:47-10:49 [PUBLIC, CLASS], 10:51-10:57 -[PARAMETER, UNUSED, DECLARATION], 10:58-10:60 +[PARAMETER, DECLARATION], 10:58-10:60 [PARAMETER], 10:65-10:67 [PUBLIC, METHOD], 10:68-10:77 [PUBLIC, CLASS], 11:8-11:19 diff --git a/java/java.editor.base/test/unit/data/org/netbeans/modules/java/editor/base/semantic/data/ConstructorUsedBySuper1.java b/java/java.editor.base/test/unit/data/org/netbeans/modules/java/editor/base/semantic/data/ConstructorUsedBySuper1.java index 172bbc5..eb0bdee 100644 --- a/java/java.editor.base/test/unit/data/org/netbeans/modules/java/editor/base/semantic/data/ConstructorUsedBySuper1.java +++ b/java/java.editor.base/test/unit/data/org/netbeans/modules/java/editor/base/semantic/data/ConstructorUsedBySuper1.java @@ -7,7 +7,7 @@ public class ConstructorUsedBySuper1 { } public static class Subclass extends ConstructorUsedBySuper1 { - private Subclass() { + public Subclass() { } diff --git a/java/java.editor.base/test/unit/data/org/netbeans/modules/java/editor/base/semantic/data/ConstructorUsedBySuper2.java b/java/java.editor.base/test/unit/data/org/netbeans/modules/java/editor/base/semantic/data/ConstructorUsedBySuper2.java index 7b99680..55bcc15 100644 --- a/java/java.editor.base/test/unit/data/org/netbeans/modules/java/editor/base/semantic/data/ConstructorUsedBySuper2.java +++ b/java/java.editor.base/test/unit/data/org/netbeans/modules/java/editor/base/semantic/data/ConstructorUsedBySuper2.java @@ -7,7 +7,7 @@ public class ConstructorUsedBySuper2 { } public static class Subclass extends ConstructorUsedBySuper2 { - private Subclass() { + public Subclass() { super(); } diff --git a/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/DetectorTest.java b/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/DetectorTest.java index 03cd5e6..726b65c 100644 --- a/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/DetectorTest.java +++ b/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/DetectorTest.java @@ -515,7 +515,7 @@ public class DetectorTest extends TestBase { //OK, no RELEASE_14, skip tests return ; } - setSourceLevel("14"); + enablePreview(); performTest("Record", "public record Test(String s) {}\n" + "class T {\n" + @@ -542,8 +542,8 @@ public class DetectorTest extends TestBase { } catch (IllegalArgumentException ex) { //OK, no RELEASE_14, skip tests return; - } - setSourceLevel("14"); + } + enablePreview(); performTest("Records", "public class Records {\n" + " public interface Super {}\n" + diff --git a/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/TestBase.java b/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/TestBase.java index 1ef26b6..6164663 100644 --- a/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/TestBase.java +++ b/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/TestBase.java @@ -167,26 +167,7 @@ public abstract class TestBase extends NbTestCase { protected void performTest(Input input, final Performer performer, boolean doCompileRecursively, Validator validator) throws Exception { SourceUtilsTestUtil.prepareTest(new String[] {"org/netbeans/modules/java/editor/resources/layer.xml"}, new Object[] { new MIMEResolverImpl(), - new CompilerOptionsQueryImplementation() { - @Override - public CompilerOptionsQueryImplementation.Result getOptions(FileObject file) { - if (testSourceFO == file) { - return new CompilerOptionsQueryImplementation.Result() { - @Override - public List<? extends String> getArguments() { - return extraOptions; - } - - @Override - public void addChangeListener(ChangeListener listener) {} - - @Override - public void removeChangeListener(ChangeListener listener) {} - }; - } - return null; - } - } + new CompilerOptionsQueryImplementationImpl() }); FileObject scratch = SourceUtilsTestUtil.makeScratchDir(this); @@ -263,7 +244,10 @@ public abstract class TestBase extends NbTestCase { } protected void performTest(String fileName, String code, Performer performer, boolean doCompileRecursively, String... expected) throws Exception { - SourceUtilsTestUtil.prepareTest(new String[] {"org/netbeans/modules/java/editor/resources/layer.xml"}, new Object[] {new MIMEResolverImpl()}); + SourceUtilsTestUtil.prepareTest(new String[] {"org/netbeans/modules/java/editor/resources/layer.xml"}, new Object[] { + new MIMEResolverImpl(), + new CompilerOptionsQueryImplementationImpl() + }); FileObject scratch = SourceUtilsTestUtil.makeScratchDir(this); FileObject cache = scratch.createFolder("cache"); @@ -426,4 +410,26 @@ public abstract class TestBase extends NbTestCase { protected interface Validator { public void validate(String actual) throws Exception; } + + private class CompilerOptionsQueryImplementationImpl implements CompilerOptionsQueryImplementation { + + @Override + public CompilerOptionsQueryImplementation.Result getOptions(FileObject file) { + if (testSourceFO == file || testSourceFO.getParent() == file) { + return new CompilerOptionsQueryImplementation.Result() { + @Override + public List<? extends String> getArguments() { + return extraOptions; + } + + @Override + public void addChangeListener(ChangeListener listener) {} + + @Override + public void removeChangeListener(ChangeListener listener) {} + }; + } + return null; + } + } } diff --git a/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/UnusedDetectorTest.java b/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/UnusedDetectorTest.java new file mode 100644 index 0000000..e7bc89b --- /dev/null +++ b/java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/UnusedDetectorTest.java @@ -0,0 +1,377 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.java.editor.base.semantic; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.stream.Collectors; +import javax.lang.model.SourceVersion; +import javax.swing.text.Document; +import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.assertNotNull; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import static org.junit.Assert.*; +import org.netbeans.api.java.lexer.JavaTokenId; +import org.netbeans.api.java.source.CompilationController; +import org.netbeans.api.java.source.CompilationInfo; +import org.netbeans.api.java.source.JavaSource; +import org.netbeans.api.java.source.SourceUtilsTestUtil; +import org.netbeans.api.java.source.TestUtilities; +import org.netbeans.api.java.source.Task; +import org.netbeans.api.lexer.Language; +import org.netbeans.junit.NbTestCase; +import org.openide.cookies.EditorCookie; +import org.openide.filesystems.FileObject; +import org.openide.filesystems.FileUtil; +import org.openide.loaders.DataObject; + +/** + * + * @author lahvac + */ +public class UnusedDetectorTest extends NbTestCase { + + public UnusedDetectorTest(String name) { + super(name); + } + + @Test + public void testUnusedMethod() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public class Test {\n" + + " private void unusedMethod() {}\n" + + "}\n", + "3:unusedMethod:NOT_USED"); + } + + @Test + public void testUnusedParameters() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public abstract class Test {\n" + + " private void method(int unused) {}\n" + + " public void api(String notUnused1) {\n" + + " method(0);\n" + + " l(notUnused2 -> {});\n" + + " EffectivellyPrivate p = x -> x;\n" + + " nativeMethod(p.abstractMethod(0));\n" + + " }\n" + + " {\n" + + " l(notUnused3 -> {});\n" + + " }\n" + + " public String t1 = l(notUnused4 -> {});\n" + + " static {\n" + + " l(notUnused5 -> {});\n" + + " }\n" + + " public static String t2 = l(notUnused6 -> {});\n" + + " private native void nativeMethod(int notUnused7);\n" + + " private interface EffectivellyPrivate {\n" + + " int abstractMethod(int notUnused8);\n" + + " }\n" + + " private native brokenMethod(int notUnused9);\n" + + " public void l(I i) { }\n" + + " interface I {\n" + + " public String run(int i);\n" + + " }\n" + + "}\n", + "3:unused:NOT_READ", + "22:<init>:NOT_USED"); + } + + @Test + public void testMethodRecursion() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public class Test {\n" + + " private void unusedRecursive() {\n" + + " unusedRecursive();\n" + + " delay(Test::unusedRecursive);\n" + + " delay(() -> unusedRecursive());\n" + + " }\n" + + " private void used() {}\n" + + " public void use() { delay(Test::used); }\n" + + " public void delay(Runnable r) {}\n" + + "}\n", + "3:unusedRecursive:NOT_USED"); + } + + @Test + public void testForEachLoop() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public class Test {\n" + + " public void test(String[] args) {\n" + + " for (String a : args) {\n" + + " System.err.println(a);\n" + + " }\n" + + " }\n" + + "}\n"); + } + + @Test + public void testCompoundAssignment() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public class Test {\n" + + " public int compound(int i) {\n" + + " int unused = 0;\n" + + " unused += 9;\n" + + " int used = 0;\n" + + " compound(used += 9);\n" + + " int unused2 = 0;\n" + + " int used2 = 0;\n" + + " unused2 += used2;\n" + + " int unused3 = 0;\n" + + " int u1 = unused3 += 4;\n" + + " int unused4 = 0;\n" + + " int u2 = 0;\n" + + " u2 = unused4 += 4;\n" + + " return u1 + u2;\n" + + " }\n" + + "}\n", + "4:unused:NOT_READ", + "8:unused2:NOT_READ"); + } + + @Test + public void testBindingPatterns() throws Exception { + try { + SourceVersion.valueOf("RELEASE_14"); //NOI18N + } catch (IllegalArgumentException ex) { + //OK, no RELEASE_14, skip tests + return; + } + sourceLevel = "14"; + performTest("test/Test.java", + "package test;\n" + + "public class Test {\n" + + " public void compound(Object o1, Object o2) {\n" + + " if (o1 instanceof String s1) {\n" + + " return s1;\n" + + " }\n" + + " if (o2 instanceof String s2) {\n" + + " return null;\n" + + " }\n" + + " return null;\n" + + " }\n" + + "}\n", + "7:s2:NOT_READ"); + } + + @Test + public void testRecord() throws Exception { + try { + SourceVersion.valueOf("RELEASE_14"); //NOI18N + } catch (IllegalArgumentException ex) { + //OK, no RELEASE_14, skip tests + return; + } + sourceLevel = "14"; + performTest("test/Test.java", + "package test;\n" + + "public record Test(int i, long j) {\n" + + " private void unused() {}\n" + + "}\n", + "3:unused:NOT_USED"); + } + + @Test + public void testEnumConstructor() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public enum E {\n" + + " A,\n" + + " B(1);\n" + + " E() {}\n" + + " E(int i) { System.err.println(s); }\n" + + " E(String s) { System.err.println(s); }\n" + + "}\n", + "7:<init>:NOT_USED"); + } + + @Test + public void testConstructor() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public class Test {\n" + + " private Test() {}\n" + + " public static Test test() { return new Test(); }\n" + + "}\n"); + } + + @Test + public void testCaught() throws Exception { + //Ignore unread caught exceptions. There are valid reasons why it + //could be unused; and there should be a separate hint checking if + //it makes sense to no use it: + performTest("test/Test.java", + "package test;\n" + + "public class Test {\n" + + " public static Test test() {\n" + + " try {" + + " test();\n" + + " } catch (RuntimeException ex) {\n" + + " }\n" + + " }\n" + + "}\n"); + } + + @Test + public void testPrivateConstructorUtilityClass1() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public class Test {\n" + + " private Test() {}\n" + + " public static void test() {\n" + + " }\n" + + "}\n"); + } + + @Test + public void testPrivateConstructorNotUtilityClass2() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public class Test {\n" + + " private Test() {}\n" + + " public Test(int i) {}\n" + + " }\n" + + "}\n", + "3:<init>:NOT_USED"); + } + + @Test + public void testPrivateConstructorNotUtilityClass3() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public abstract class Test {\n" + + " private Test() {}\n" + + " }\n" + + "}\n", + "3:<init>:NOT_USED"); + } + + @Test + public void testPrivateConstructorNotUtilityClass4() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public class Test extends Exception {\n" + + " private Test() {}\n" + + " }\n" + + "}\n", + "3:<init>:NOT_USED"); + } + + @Test + public void testPrivateConstructorNotUtilityClass5() throws Exception { + performTest("test/Test.java", + "package test;\n" + + "public class Test implements I {\n" + + " private Test() {}\n" + + " }\n" + + "}\n" + + "interface I {}\n" + + "}\n", + "3:<init>:NOT_USED"); + } + + protected String sourceLevel = "1.8"; + + protected void performTest(String fileName, String code, String... expected) throws Exception { + SourceUtilsTestUtil.prepareTest(new String[] {}, new Object[] {new TestBase.MIMEResolverImpl()}); + + FileObject scratch = SourceUtilsTestUtil.makeScratchDir(this); + FileObject cache = scratch.createFolder("cache"); + + File wd = getWorkDir(); + File testSource = new File(wd, "test/" + fileName + ".java"); + + testSource.getParentFile().mkdirs(); + TestUtilities.copyStringToFile(testSource, code); + + FileObject testSourceFO = FileUtil.toFileObject(testSource); + + assertNotNull(testSourceFO); + + if (sourceLevel != null) { + SourceUtilsTestUtil.setSourceLevel(testSourceFO, sourceLevel); + } + + File testBuildTo = new File(wd, "test-build"); + + testBuildTo.mkdirs(); + + FileObject srcRoot = FileUtil.toFileObject(testSource.getParentFile()); + SourceUtilsTestUtil.prepareTest(srcRoot,FileUtil.toFileObject(testBuildTo), cache); + + final Document doc = getDocument(testSourceFO); + + JavaSource source = JavaSource.forFileObject(testSourceFO); + + assertNotNull(source); + + final CountDownLatch l = new CountDownLatch(1); + + source.runUserActionTask(new Task<CompilationController>() { + public void run(CompilationController parameter) { + try { + parameter.toPhase(JavaSource.Phase.UP_TO_DATE); + + Set<String> result = UnusedDetector.findUnused(parameter) + .stream() + .map(ud -> parameter.getCompilationUnit().getLineMap().getLineNumber(parameter.getTrees().getSourcePositions().getStartPosition(ud.unusedElementPath.getCompilationUnit(), ud.unusedElementPath.getLeaf())) + ":" + ud.unusedElement.getSimpleName() + ":" + ud.reason.name()) + .collect(Collectors.toSet()); + assertEquals(new HashSet<>(Arrays.asList(expected)), result); + } catch (IOException e) { + e.printStackTrace(); + } finally { + l.countDown(); + } + } + }, true); + + l.await(); + } + + private final Document getDocument(FileObject file) throws IOException { + DataObject od = DataObject.find(file); + EditorCookie ec = (EditorCookie) od.getCookie(EditorCookie.class); + + if (ec != null) { + Document doc = ec.openDocument(); + + doc.putProperty(Language.class, JavaTokenId.language()); + doc.putProperty("mimeType", "text/x-java"); + + return doc; + } else { + return null; + } + } +} diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/bugs/Unused.java b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/Unused.java new file mode 100644 index 0000000..cc87f67 --- /dev/null +++ b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/Unused.java @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.java.hints.bugs; + +import com.sun.source.tree.Tree.Kind; +import java.util.List; +import java.util.stream.Collectors; +import javax.lang.model.element.ElementKind; +import org.netbeans.modules.java.editor.base.semantic.UnusedDetector; +import org.netbeans.modules.java.editor.base.semantic.UnusedDetector.UnusedDescription; +import org.netbeans.spi.editor.hints.ErrorDescription; +import org.netbeans.spi.java.hints.ErrorDescriptionFactory; +import org.netbeans.spi.java.hints.Hint; +import org.netbeans.spi.java.hints.HintContext; +import org.netbeans.spi.java.hints.TriggerTreeKind; +import org.openide.util.NbBundle.Messages; + +/** + * + * @author lahvac + */ +@Hint(displayName = "#DN_org.netbeans.modules.java.hints.bugs.Unused", description = "#DESC_org.netbeans.modules.java.hints.bugs.Unused", category="bugs", options=Hint.Options.QUERY, suppressWarnings="unused") +@Messages({ + "DN_org.netbeans.modules.java.hints.bugs.Unused=Unused Element", + "DESC_org.netbeans.modules.java.hints.bugs.Unused=Detects and reports unused variables, methods and classes" +}) +public class Unused { + + @TriggerTreeKind(Kind.COMPILATION_UNIT) + public static List<ErrorDescription> unused(HintContext ctx) { + return UnusedDetector.findUnused(ctx.getInfo()) + .stream() + .map(ud -> convertUnused(ctx, ud)) + .filter(err -> err != null) + .collect(Collectors.toList()); + } + + @Messages({ + "# {0} - variable name", + "ERR_NeitherReadOrWritten=Variable {0} is neither read or written to", + "# {0} - variable name", + "ERR_NotWritten=Variable {0} is never written to", + "# {0} - variable name", + "ERR_NotRead=Variable {0} is never read", + "# {0} - element name", + "ERR_NotUsed={0} is never used", + "ERR_NotUsedConstructor=Constructor is never used", + }) + private static ErrorDescription convertUnused(HintContext ctx, UnusedDescription ud) { + //TODO: switch expression candidate! + String name = ud.unusedElement.getSimpleName().toString(); + String message; + switch (ud.reason) { + case NOT_WRITTEN_READ: message = Bundle.ERR_NeitherReadOrWritten(name); break; + case NOT_WRITTEN: message = Bundle.ERR_NotWritten(name); break; + case NOT_READ: message = Bundle.ERR_NotRead(name); break; + case NOT_USED: + if (ud.unusedElement.getKind() == ElementKind.CONSTRUCTOR) { + message = Bundle.ERR_NotUsedConstructor(); + } else { + message = Bundle.ERR_NotUsed(name); + } + break; + default: + throw new IllegalStateException("Unknown unused type: " + ud.reason); + } + return ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath, message); + } +} diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/UnusedTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/UnusedTest.java new file mode 100644 index 0000000..2b41dba --- /dev/null +++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/UnusedTest.java @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.java.hints.bugs; + +import org.netbeans.junit.NbTestCase; +import org.netbeans.modules.java.hints.test.api.HintTest; + +/** + * + * @author lahvac + */ +public class UnusedTest extends NbTestCase { + + public UnusedTest(String name) { + super(name); + } + + public void testUnused() throws Exception { + HintTest + .create() + .input("package test;\n" + + "public class Test {\n" + + " private class UnusedClass {}\n" + + " private void unusedMethod() {}\n" + + " private int unusedField;\n" + + " private void test(int unusedParam) {}\n" + + " public void test2() {test(1);}\n" + + " private Test() {}\n" + + "}\n") + .run(Unused.class) + .assertWarnings("2:18-2:29:verifier:" + Bundle.ERR_NotUsed("UnusedClass"), + "3:17-3:29:verifier:" + Bundle.ERR_NotUsed("unusedMethod"), + "4:16-4:27:verifier:" + Bundle.ERR_NotRead("unusedField"), + "5:26-5:37:verifier:" + Bundle.ERR_NotRead("unusedParam"), + "7:12-7:16:verifier:" + Bundle.ERR_NotUsedConstructor()); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@netbeans.apache.org For additional commands, e-mail: commits-h...@netbeans.apache.org For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists