From e3eff4902df0cfaffac19ed4eab2b093338e0a87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Odin=20Dahlstr=C3=B6m?= Date: Mon, 23 Oct 2023 17:43:15 +0200 Subject: [PATCH] Fix wrongly encoded semantic tokens around class keyword MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Odin Dahlström --- .../semantictokens/SemanticTokensVisitor.java | 185 ++++++++++++++---- .../src/main/java/foo/Types.java | 11 +- .../handlers/SemanticTokensHandlerTest.java | 23 ++- 3 files changed, 176 insertions(+), 43 deletions(-) diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/semantictokens/SemanticTokensVisitor.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/semantictokens/SemanticTokensVisitor.java index 0d9488ec24..8e2fe73a8b 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/semantictokens/SemanticTokensVisitor.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/semantictokens/SemanticTokensVisitor.java @@ -16,6 +16,9 @@ import java.util.ArrayList; import java.util.List; +import org.eclipse.jdt.core.IJavaProject; +import org.eclipse.jdt.core.ITypeRoot; +import org.eclipse.jdt.core.JavaCore; import org.eclipse.jdt.core.JavaModelException; import org.eclipse.jdt.core.ToolFactory; import org.eclipse.jdt.core.compiler.IScanner; @@ -58,12 +61,13 @@ public class SemanticTokensVisitor extends ASTVisitor { private CompilationUnit cu; + private final IScanner scanner; private List tokens; - private static IScanner fScanner; public SemanticTokensVisitor(CompilationUnit cu) { super(true); this.cu = cu; + this.scanner = createScanner(cu); this.tokens = new ArrayList<>(); } @@ -460,34 +464,16 @@ public boolean visit(RecordDeclaration node) { public boolean visit(TypeDeclaration node) { acceptNode(node.getJavadoc()); acceptNodeList(node.modifiers()); - - // Using a scanner here to retrieve the class/interface keyword as there is no nice way to do so using ASTNodes - int typeKeywordStart = -1; - int typeKeywordEnd = -1; - try { - String contents = this.cu.getTypeRoot().getSource(); - final int shift = node.getStartPosition(); - IScanner scanner = getScanner(); - scanner.setSource(contents.toCharArray()); - scanner.resetTo(shift, shift + node.getLength()); - int token = 0; - while (token != ITerminalSymbols.TokenNameEOF && typeKeywordStart == -1) { - switch (token) { - case ITerminalSymbols.TokenNameinterface: - case ITerminalSymbols.TokenNameclass: - typeKeywordStart = scanner.getCurrentTokenStartPosition(); - typeKeywordEnd = scanner.getCurrentTokenEndPosition(); - default: - break; - } - token = getNextToken(scanner); + tokenizeGapBeforeTypeDeclarationName(node, (scannerToken, tokenOffset, tokenLength) -> { + switch (scannerToken) { + case ITerminalSymbols.TokenNameclass: + case ITerminalSymbols.TokenNameinterface: + addToken(tokenOffset, tokenLength, TokenType.MODIFIER, 0); + break; // "class" or "interface" keyword tokens + default: + break; // ignore other tokens } - } catch (JavaModelException e) { - // ignore - } - if (typeKeywordStart != -1) { - addToken(typeKeywordStart, typeKeywordEnd - typeKeywordStart + 1, TokenType.MODIFIER, 0); - } + }); acceptNode(node.getName()); acceptNodeList(node.typeParameters()); acceptNode(node.getSuperclassType()); @@ -499,23 +485,114 @@ public boolean visit(TypeDeclaration node) { return false; } - private static IScanner getScanner() { - if (fScanner == null) { - fScanner = ToolFactory.createScanner(true, false, false, true); + /** + * Tries to create an {@link IScanner} for the source of the given compilation unit. + * + * @param cu the compilation unit + * @return the scanner, or {@code null} if not available + */ + private IScanner createScanner(CompilationUnit cu) { + final ITypeRoot typeRoot = cu.getTypeRoot(); + if (typeRoot == null) { + return null; + } + final IJavaProject javaProject = typeRoot.getJavaProject(); + if (javaProject == null) { + return null; + } + final String source; + try { + source = typeRoot.getSource(); + } catch (JavaModelException e) { + return null; } - return fScanner; + if (source == null) { + return null; + } + + final String sourceLevel = javaProject.getOption(JavaCore.COMPILER_SOURCE, true); + final String complianceLevel = javaProject.getOption(JavaCore.COMPILER_COMPLIANCE, true); + final boolean enablePreview = JavaCore.ENABLED.equals(javaProject.getOption(JavaCore.COMPILER_PB_ENABLE_PREVIEW_FEATURES, true)); + + final IScanner scanner = ToolFactory.createScanner(false, false, false, sourceLevel, complianceLevel, enablePreview); + scanner.setSource(source.toCharArray()); + return scanner; } - private int getNextToken(IScanner scanner) { - int token = 0; - while (token == 0) { + private int getNextValidToken(IScanner scanner) { + while (true) { try { - token = scanner.getNextToken(); + return scanner.getNextToken(); } catch (InvalidInputException e) { // ignore } } - return token; + } + + /** + * Visitor for {@link IScanner} tokens. + */ + @FunctionalInterface + private interface ScannerTokenVisitor { + /** + * Visits the given scanner token. + * + * @param scannerToken the scanner token ID, see {@link ITerminalSymbols} + * @param tokenOffset the document offset of the scanner token + * @param tokenLength the length of the scanner token + */ + public void visit(int scannerToken, int tokenOffset, int tokenLength); + } + + /** + * Uses an {@link IScanner} (if available) to tokenize a source range in the document, + * and visits the scanner tokens in order of occurrence in the source range. + * + *

+ * NOTE: If semantic tokens are added by the visitor, the scan range MUST NOT intersect + * with any other range where semantic tokens can appear, in order to avoid overlapping semantic tokens. + *

+ * + * @param startPosition the (inclusive) start position of the scan range + * @param endPosition the (exclusive) end position of the scan range + * @param tokenVisitor the visitor to use for scanner tokens + */ + private void tokenizeWithScanner(int startPosition, int endPosition, ScannerTokenVisitor tokenVisitor) { + if (scanner == null) { + return; + } + scanner.resetTo(startPosition, endPosition - 1); // -1 because resetTo wants inclusive endPosition + for (int token = getNextValidToken(scanner); token != ITerminalSymbols.TokenNameEOF; token = getNextValidToken(scanner)) { + int tokenOffset = scanner.getCurrentTokenStartPosition(); + int tokenLength = scanner.getCurrentTokenEndPosition() - tokenOffset + 1; // +1 because getCurrentTokenEndPosition is inclusive + tokenVisitor.visit(token, tokenOffset, tokenLength); + } + } + + /** + * Uses an {@link IScanner} (if available) to tokenize the gap in the AST just before {@link TypeDeclaration#getName()}, + * and visits the scanner tokens in order of occurrence in the source range. + * + *

+ * NOTE: If semantic tokens are added by the visitor, the scan range MUST NOT intersect + * with any other range where semantic tokens can appear, in order to avoid overlapping semantic tokens. + *

+ * + * @param typeDeclaration the type declaration node + * @param tokenVisitor the visitor to use for scanner tokens + */ + private void tokenizeGapBeforeTypeDeclarationName(TypeDeclaration typeDeclaration, ScannerTokenVisitor tokenVisitor) { + // Try potentially nonexistent start positions, closest first + int gapBeforeNameStart = getEndPosition(typeDeclaration.modifiers()); + if (gapBeforeNameStart == -1) { + gapBeforeNameStart = getEndPosition(typeDeclaration.getJavadoc()); + } + // Fallback to closest known start position + if (gapBeforeNameStart == -1) { + gapBeforeNameStart = typeDeclaration.getStartPosition(); + } + int gapBeforeNameEnd = typeDeclaration.getName().getStartPosition(); + tokenizeWithScanner(gapBeforeNameStart, gapBeforeNameEnd, tokenVisitor); } /** @@ -587,6 +664,40 @@ private void acceptNode(ASTNode node) { } } + /** + * Gets the (exclusive) document end position of the given list of AST nodes. + * + *

+ * The caller is responsible for making sure that the element type + * of the given list is {@link ASTNode}. + *

+ * + * @param nodeList the list of AST nodes (may be {@code null}) + * @return the end position, or {@code -1} if unknown + */ + private int getEndPosition(List nodeList) { + if (nodeList != null && !nodeList.isEmpty()) { + ASTNode lastNode = (ASTNode) nodeList.get(nodeList.size() - 1); + return lastNode.getStartPosition() + lastNode.getLength(); + } else { + return -1; + } + } + + /** + * Gets the (exclusive) document end position of the given AST node. + * + * @param node the AST node (may be {@code null}) + * @return the end position, or {@code -1} if unknown + */ + private int getEndPosition(ASTNode node) { + if (node != null) { + return node.getStartPosition() + node.getLength(); + } else { + return -1; + } + } + /** * Helper method to recursively visit all the simple names of a {@link Name} node * using the specified {@link NodeVisitor}. diff --git a/org.eclipse.jdt.ls.tests/projects/maven/semantic-tokens/src/main/java/foo/Types.java b/org.eclipse.jdt.ls.tests/projects/maven/semantic-tokens/src/main/java/foo/Types.java index 28ecaff6b2..7dd8d14c9a 100644 --- a/org.eclipse.jdt.ls.tests/projects/maven/semantic-tokens/src/main/java/foo/Types.java +++ b/org.eclipse.jdt.ls.tests/projects/maven/semantic-tokens/src/main/java/foo/Types.java @@ -10,13 +10,20 @@ public class Types { public SomeClass> someClass; - class SomeClass { + /** + * class + */ + @SomeAnnotation(Types.class) + // class + public class SomeClass { T1 t1; T2 t2; } interface SomeInterface {} enum SomeEnum {} - @interface SomeAnnotation {} + @interface SomeAnnotation { + Class value(); + } record SomeRecord() {} } diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SemanticTokensHandlerTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SemanticTokensHandlerTest.java index 3cebd901f7..17b60274d9 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SemanticTokensHandlerTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SemanticTokensHandlerTest.java @@ -258,29 +258,44 @@ public void testSemanticTokens_Variables() throws JavaModelException { @Test public void testSemanticTokens_Types() throws JavaModelException { - TokenAssertionHelper.beginAssertion(getURI("Types.java"), "class", "interface", "enum", "annotation", "record", "typeParameter", "keyword") + TokenAssertionHelper.beginAssertion(getURI("Types.java"), "class", "interface", "enum", "annotation", "record", "typeParameter", "keyword", "modifier") + .assertNextToken("public", "modifier") + .assertNextToken("class", "modifier") .assertNextToken("Types", "class", "public", "declaration") + + .assertNextToken("public", "modifier") .assertNextToken("String", "class", "public", "readonly") + .assertNextToken("protected", "modifier") + .assertNextToken("final", "modifier") .assertNextToken("Class", "class", "public", "readonly", "generic") .assertNextToken("String", "class", "public", "readonly") .assertNextToken("class", "keyword") - .assertNextToken("SomeClass", "class", "generic") + .assertNextToken("public", "modifier") + .assertNextToken("SomeClass", "class", "public", "generic") .assertNextToken("String", "class", "public", "readonly", "typeArgument") - .assertNextToken("SomeClass", "class", "generic", "typeArgument") + .assertNextToken("SomeClass", "class", "public", "generic", "typeArgument") .assertNextToken("String", "class", "public", "readonly", "typeArgument") .assertNextToken("Integer", "class", "public", "readonly", "typeArgument") - .assertNextToken("SomeClass", "class", "generic", "declaration") + .assertNextToken("SomeAnnotation", "annotation", "static") + .assertNextToken("Types", "class", "public") + .assertNextToken("class", "keyword") + .assertNextToken("public", "modifier") + .assertNextToken("class", "modifier") + .assertNextToken("SomeClass", "class", "public", "generic", "declaration") .assertNextToken("T1", "typeParameter", "declaration") .assertNextToken("T2", "typeParameter", "declaration") .assertNextToken("T1", "typeParameter") .assertNextToken("T2", "typeParameter") + .assertNextToken("interface", "modifier") .assertNextToken("SomeInterface", "interface", "static", "declaration") .assertNextToken("SomeEnum", "enum", "static", "readonly", "declaration") .assertNextToken("SomeAnnotation", "annotation", "static", "declaration") + .assertNextToken("Class", "class", "public", "readonly", "generic") + .assertNextToken("record", "modifier") .assertNextToken("SomeRecord", "record", "static", "readonly", "declaration") .endAssertion(); }