Skip to content

Unused in package hint: optimize used-in-package scenario #8524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Unused in package hint: optimize used-in-package scenario #8524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d638711
Unused in package hint: optimize used-in-package scenario
mbien May 23, 2025
1b6f8e1
Language level cleanup in SemanticHighlighterBase and friends.
mbien May 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 35 additions & 76 deletions ...itor.base/src/org/netbeans/modules/java/editor/base/semantic/SemanticHighlighterBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,14 @@
*
* @author Jan Lahoda
*/
@SuppressWarnings("AccessingNonPublicFieldOfAnotherObject")
public abstract class SemanticHighlighterBase extends JavaParserResultTask<Result> {

public static final String JAVA_INLINE_HINT_PARAMETER_NAME = "javaInlineHintParameterName"; //NOI18N
public static final String JAVA_INLINE_HINT_CHAINED_TYPES = "javaInlineHintChainedTypes"; //NOI18N
public static final String JAVA_INLINE_HINT_VAR_TYPE = "javaInlineHintVarType"; //NOI18N

private AtomicBoolean cancel = new AtomicBoolean();
private final AtomicBoolean cancel = new AtomicBoolean();

protected SemanticHighlighterBase() {
super(Phase.RESOLVED, TaskIndexingMode.ALLOWED_DURING_SCAN);
Expand Down Expand Up @@ -128,11 +129,8 @@ private static boolean verifyDocument(final Document doc) {
}

final boolean[] tokenSequenceNull = new boolean[1];
doc.render(new Runnable() {
@Override
public void run() {
tokenSequenceNull[0] = (TokenHierarchy.get(doc).tokenSequence() == null);
}
doc.render(() -> {
tokenSequenceNull[0] = (TokenHierarchy.get(doc).tokenSequence() == null);
});
return !tokenSequenceNull[0];
}
Expand Down Expand Up @@ -265,17 +263,7 @@ private static boolean isLocalVariableClosure(Element el) {
LOCAL_VARIABLES.contains(el.getKind());
}

private static class Use {
private boolean declaration;
private TreePath tree;
private Collection<ColoringAttributes> spec;

public Use(boolean declaration, TreePath tree, Collection<ColoringAttributes> spec) {
this.declaration = declaration;
this.tree = tree;
this.spec = spec;
}

private record Use(boolean declaration, TreePath tree, Collection<ColoringAttributes> spec) {
@Override
public String toString() {
return "Use: " + spec;
Expand All @@ -284,35 +272,32 @@ public String toString() {

private static class DetectorVisitor extends CancellableTreePathScanner<Void, Void> {

private final org.netbeans.api.java.source.CompilationInfo info;
private final Document doc;
private final CompilationInfo info;
private final Settings settings;
private Map<Element, List<Use>> type2Uses;
private Map<Tree, List<Token>> tree2Tokens;
private List<Token> contextKeywords;
private List<Pair<int[], Coloring>> extraColoring;
private Map<Integer, String> preText;
private TokenList tl;
private final TokenList tl;
private long memberSelectBypass = -1;
private SourcePositions sourcePositions;
private final SourcePositions sourcePositions;
private ExecutableElement recursionDetector;

private DetectorVisitor(org.netbeans.api.java.source.CompilationInfo info, final Document doc, Settings settings, AtomicBoolean cancel) {
private DetectorVisitor(CompilationInfo info, Document doc, Settings settings, AtomicBoolean cancel) {
super(cancel);

this.info = info;
this.doc = doc;
this.settings = settings;
type2Uses = new HashMap<Element, List<Use>>();
tree2Tokens = new IdentityHashMap<Tree, List<Token>>();
type2Uses = new HashMap<>();
tree2Tokens = new IdentityHashMap<>();
contextKeywords = new ArrayList<>();
extraColoring = new ArrayList<>();
preText = new HashMap<>();

tl = new TokenList(info, doc, cancel);

this.sourcePositions = info.getTrees().getSourcePositions();
// this.pos = pos;
}

private void firstIdentifier(String name) {
Expand Down Expand Up @@ -401,7 +386,7 @@ private void addModifiers(Element decl, Collection<ColoringAttributes> c) {
}

private Collection<ColoringAttributes> getMethodColoring(ExecutableElement mdecl) {
Collection<ColoringAttributes> c = new ArrayList<ColoringAttributes>();
Collection<ColoringAttributes> c = new ArrayList<>();

addModifiers(mdecl, c);

Expand All @@ -414,7 +399,7 @@ private Collection<ColoringAttributes> getMethodColoring(ExecutableElement mdecl
}

private Collection<ColoringAttributes> getVariableColoring(Element decl) {
Collection<ColoringAttributes> c = new ArrayList<ColoringAttributes>();
Collection<ColoringAttributes> c = new ArrayList<>();

addModifiers(decl, c);

Expand Down Expand Up @@ -499,38 +484,32 @@ private void handlePossibleIdentifier(TreePath expr, boolean declaration, Elemen
c = getVariableColoring(decl);
}

if (decl instanceof ExecutableElement) {
c = getMethodColoring((ExecutableElement) decl);
if (decl instanceof ExecutableElement exec) {
c = getMethodColoring(exec);
}

if (decl.getKind() == ElementKind.MODULE) {
c = new ArrayList<ColoringAttributes>();
c = new ArrayList<>();
c.add(ColoringAttributes.MODULE);
}

if (isDeclType) {
c = new ArrayList<ColoringAttributes>();

c = new ArrayList<>();
addModifiers(decl, c);

switch (decl.getKind()) {
case CLASS: c.add(ColoringAttributes.CLASS); break;
case INTERFACE: c.add(ColoringAttributes.INTERFACE); break;
case ANNOTATION_TYPE: c.add(ColoringAttributes.ANNOTATION_TYPE); break;
case ENUM: c.add(ColoringAttributes.ENUM); break;
default:
if (decl.getKind().name().contentEquals("RECORD")) {
c.add(ColoringAttributes.RECORD);
}
break;
case CLASS -> c.add(ColoringAttributes.CLASS);
case INTERFACE -> c.add(ColoringAttributes.INTERFACE);
case ANNOTATION_TYPE -> c.add(ColoringAttributes.ANNOTATION_TYPE);
case ENUM -> c.add(ColoringAttributes.ENUM);
case RECORD -> c.add(ColoringAttributes.RECORD);
}
}

if (declaration) {
if (c == null) {
c = new ArrayList<ColoringAttributes>();
c = new ArrayList<>();
}

c.add(ColoringAttributes.DECLARATION);
}

Expand All @@ -545,15 +524,8 @@ private void handlePossibleIdentifier(TreePath expr, boolean declaration, Elemen
}

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(declaration, t, c);

uses.add(u);
type2Uses.computeIfAbsent(decl, k -> new ArrayList<>())
.add(new Use(declaration, t, c));
}

@Override
Expand Down Expand Up @@ -993,7 +965,7 @@ record = true;
//XXX:????
scan(tree.getTypeParameters(), null);
if (record) {
scan(tree.getMembers().stream().filter(m -> isRecordComponent(m)).collect(Collectors.toList()), null);
scan(tree.getMembers().stream().filter(m -> isRecordComponent(m)).toList(), null);
}
scan(tree.getExtendsClause(), null);
scan(tree.getImplementsClause(), null);
Expand All @@ -1015,7 +987,7 @@ record = true;
recursionDetector = null;

if (record) {
scan(tree.getMembers().stream().filter(m -> !isRecordComponent(m)).collect(Collectors.toList()), null);
scan(tree.getMembers().stream().filter(m -> !isRecordComponent(m)).toList(), null);
} else {
scan(tree.getMembers(), null);
}
Expand Down Expand Up @@ -1055,7 +1027,7 @@ public Void visitLiteral(LiteralTree node, Void p) {
String tokenText = t.text().toString();
String[] lines = tokenText.split("\n");
int indent = Arrays.stream(lines, 1, lines.length)
.filter(l -> !l.trim().isEmpty())
.filter(l -> !l.isBlank())
.mapToInt(this::leadingIndent)
.min()
.orElse(0);
Expand All @@ -1080,7 +1052,7 @@ public Void visitLiteral(LiteralTree node, Void p) {

@Override
public Void scan(Tree tree, Void p) {
if (tree != null && "YIELD".equals(tree.getKind().name())) {
if (tree != null && tree.getKind() == Kind.YIELD) {
tl.moveToOffset(sourcePositions.getStartPosition(info.getCompilationUnit(), tree));
Token t = firstIdentifierToken("yield"); //NOI18N
if (t != null) {
Expand Down Expand Up @@ -1170,26 +1142,13 @@ public static interface ErrorDescriptionSetter {
public void setColorings(Document doc, Map<Token, Coloring> colorings);
}

public static class Settings {
private static final Map<String, Boolean> DEFAULT_VALUES;
public record Settings(boolean javaInlineHintParameterName, boolean javaInlineHintChainedTypes, boolean javaInlineHintVarType) {

static {
Map<String, Boolean> defaultValuesBuilder = new HashMap<>();
defaultValuesBuilder.put(JAVA_INLINE_HINT_PARAMETER_NAME, true);
defaultValuesBuilder.put(JAVA_INLINE_HINT_CHAINED_TYPES, false);
defaultValuesBuilder.put(JAVA_INLINE_HINT_VAR_TYPE, false);
DEFAULT_VALUES = Collections.unmodifiableMap(defaultValuesBuilder);
}

public final boolean javaInlineHintParameterName;
public final boolean javaInlineHintChainedTypes;
public final boolean javaInlineHintVarType;

public Settings(boolean javaInlineHintParameterName, boolean javaInlineHintChainedTypes, boolean javaInlineHintVarType) {
this.javaInlineHintParameterName = javaInlineHintParameterName;
this.javaInlineHintChainedTypes = javaInlineHintChainedTypes;
this.javaInlineHintVarType = javaInlineHintVarType;
}
private static final Map<String, Boolean> DEFAULT_VALUES = Map.of(
JAVA_INLINE_HINT_PARAMETER_NAME, true,
JAVA_INLINE_HINT_CHAINED_TYPES, false,
JAVA_INLINE_HINT_VAR_TYPE, false
);

public static Settings getDefault() {
Preferences preferences = NbPreferences.root().node("/org/netbeans/modules/java/editor/InlineHints/default");
Expand Down
77 changes: 64 additions & 13 deletions java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/UnusedDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.sun.source.util.TreePath;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Field;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -80,6 +81,7 @@
*
* @author lahvac
*/
@SuppressWarnings("AccessingNonPublicFieldOfAnotherObject")
public class UnusedDetector {

public record UnusedDescription(Element unusedElement, TreePath unusedElementPath, boolean packagePrivate, UnusedReason reason) {}
Expand Down Expand Up @@ -328,35 +330,39 @@ public boolean isDependencies() {
}
});
switch (el.getKind()) {
case FIELD:
case FIELD -> {
typeElement = info.getElementUtilities().enclosingTypeElement(el);
searchKinds = EnumSet.of(ClassIndex.SearchKind.FIELD_REFERENCES);
break;
case METHOD:
case CONSTRUCTOR:
}
case METHOD, CONSTRUCTOR -> {
typeElement = info.getElementUtilities().enclosingTypeElement(el);
searchKinds = EnumSet.of(ClassIndex.SearchKind.METHOD_REFERENCES);
break;
case ANNOTATION_TYPE:
case CLASS:
case ENUM:
case INTERFACE:
}
case ANNOTATION_TYPE, CLASS, ENUM, INTERFACE -> {
List<? extends TypeElement> topLevelElements = info.getTopLevelElements();
if (topLevelElements.size() == 1 && topLevelElements.get(0) == el) {
return false;
}
typeElement = (TypeElement) el;
searchKinds = EnumSet.of(ClassIndex.SearchKind.TYPE_REFERENCES);
break;

default:
}
default -> {
return true;
}
}

// check if previous runs already found a usage in package-local files
FileObject fileObject = info.getFileObject();
ElementHandle<Element> eh = ElementHandle.create(el);
if (PkgScanResultCache.getCachedUsedInPkg(fileObject, eh)) {
return false;
}

// scan package for usage
ClassIndex classIndex = getCachedClassIndex(fileObject, info);
Set<FileObject> res = classIndex.getResources(ElementHandle.create(typeElement), searchKinds, scope);

if (res != null) {
ElementHandle<Element> eh = ElementHandle.create(el);
for (FileObject fo : res) {
try {
if (Boolean.TRUE.equals(cancel.call())) {
Expand Down Expand Up @@ -386,6 +392,7 @@ public Void scan(Tree tree, Element p) {
}.scan(new TreePath(cc.getCompilationUnit()), el);
}, true);
if (found.get()) {
PkgScanResultCache.markFoundInFile(fileObject, fo, ElementHandle.create(el));
return false;
}
}
Expand Down Expand Up @@ -421,6 +428,50 @@ private static ClassIndex getCachedClassIndex(FileObject fileObject, Compilation
return classIndex;
}

/*
* Remembers if an Element is used in a package-local source file to avoid having to
* repeat the search.
*
* Invalidates the whole cache if the source file of the current CompilationInfo becomes a different file.
* Invalidates a single cached value if the file was (externally) modified in the meantime.
* The not-used case is not stored to keep the logic simple.
*/
private static class PkgScanResultCache {

private static FileObject lastSource;
private static Map<ElementHandle<Element>, Usage> cache = new HashMap<>();

private record Usage(FileObject inFile, Instant markedTime) {}

private static void markFoundInFile(FileObject source, FileObject pkgLocalFile, ElementHandle<Element> signature) {
if (source == pkgLocalFile) {
throw new IllegalArgumentException();
}
lastSource = source;
cache.put(signature, new Usage(pkgLocalFile, Instant.now()));
}

private static boolean getCachedUsedInPkg(FileObject root, ElementHandle<Element> signature) {
if (lastSource == null) {
return false;
}
if (lastSource != root) {
lastSource = null;
cache = new HashMap<>();
return false;
}
Usage usage = cache.get(signature);
if (usage == null) {
return false;
}
if (usage.inFile().lastModified().toInstant().isAfter(usage.markedTime())) {
cache.remove(signature);
return false;
}
return true;
}
}

private static List<UsedDetector> collectUsedDetectors(CompilationInfo info) {
List<UsedDetector> detectors = new ArrayList<>();
for (UsedDetector.Factory factory : Lookup.getDefault().lookupAll(UsedDetector.Factory.class)) {
Expand Down
9 changes: 0 additions & 9 deletions ...base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/UnusedDetectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
Expand All @@ -29,17 +28,9 @@
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;
Expand Down
Loading
Loading