From ff019c785555dc07d62e577a0d0c78df05e81685 Mon Sep 17 00:00:00 2001 From: Michael Gumowski Date: Mon, 19 Jul 2021 18:28:06 +0200 Subject: [PATCH] SONARJAVA-3904 Fix highlighting (#3699) --- .../visitors/SyntaxHighlighterVisitor.java | 59 ++++++++++++++---- .../java/org/sonar/java/model/JParser.java | 9 ++- .../java/model/declaration/ClassTreeImpl.java | 16 ++++- .../plugins/java/api/tree/ClassTree.java | 8 +++ .../src/test/files/highlighter/Records.java | 9 +++ .../test/files/highlighter/SealedClass.java | 17 +++++ .../files/highlighter/SwitchExpression.java | 30 +++++++++ .../SyntaxHighlighterVisitorTest.java | 62 ++++++++++++++++--- .../sonar/java/model/JParserSemanticTest.java | 7 ++- 9 files changed, 194 insertions(+), 23 deletions(-) create mode 100644 java-frontend/src/test/files/highlighter/Records.java create mode 100644 java-frontend/src/test/files/highlighter/SealedClass.java create mode 100644 java-frontend/src/test/files/highlighter/SwitchExpression.java diff --git a/java-frontend/src/main/java/org/sonar/java/ast/visitors/SyntaxHighlighterVisitor.java b/java-frontend/src/main/java/org/sonar/java/ast/visitors/SyntaxHighlighterVisitor.java index b502f1cadf2..e5d8c9cadbe 100644 --- a/java-frontend/src/main/java/org/sonar/java/ast/visitors/SyntaxHighlighterVisitor.java +++ b/java-frontend/src/main/java/org/sonar/java/ast/visitors/SyntaxHighlighterVisitor.java @@ -25,6 +25,7 @@ import java.util.EnumMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import org.sonar.api.batch.sensor.highlighting.NewHighlighting; @@ -32,12 +33,17 @@ import org.sonar.java.SonarComponents; import org.sonar.java.ast.api.JavaKeyword; import org.sonar.java.ast.api.JavaRestrictedKeyword; +import org.sonar.java.model.ModifiersUtils; import org.sonar.java.model.declaration.ClassTreeImpl; import org.sonar.plugins.java.api.JavaFileScannerContext; import org.sonar.plugins.java.api.tree.AnnotationTree; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.Modifier; +import org.sonar.plugins.java.api.tree.ModifiersTree; import org.sonar.plugins.java.api.tree.SyntaxToken; import org.sonar.plugins.java.api.tree.SyntaxTrivia; import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.YieldStatementTree; public class SyntaxHighlighterVisitor extends SubscriptionVisitor { @@ -51,7 +57,7 @@ public class SyntaxHighlighterVisitor extends SubscriptionVisitor { public SyntaxHighlighterVisitor(SonarComponents sonarComponents) { this.sonarComponents = sonarComponents; - + keywords = Collections.unmodifiableSet(Arrays.stream(JavaKeyword.keywordValues()).collect(Collectors.toSet())); restrictedKeywords = Collections.unmodifiableSet(Arrays.stream(JavaRestrictedKeyword.restrictedKeywordValues()).collect(Collectors.toSet())); @@ -71,9 +77,19 @@ public SyntaxHighlighterVisitor(SonarComponents sonarComponents) { @Override public List nodesToVisit() { List list = new ArrayList<>(typesByKind.keySet()); - list.add(Tree.Kind.MODULE); list.add(Tree.Kind.TOKEN); list.add(Tree.Kind.TRIVIA); + // modules have their own set of restricted keywords + list.add(Tree.Kind.MODULE); + // 'yield' is a restricted keyword + list.add(Tree.Kind.YIELD_STATEMENT); + // 'record' is a restricted keyword + list.add(Tree.Kind.RECORD); + // sealed classes comes with restricted keyword 'permits', applying on classes and interfaces + list.add(Tree.Kind.CLASS); + list.add(Tree.Kind.INTERFACE); + // sealed classes comes with restricted modifiers 'sealed' and 'non-sealed', applying on classes and interfaces + list.add(Tree.Kind.MODIFIERS); return Collections.unmodifiableList(list); } @@ -88,15 +104,36 @@ public void scanFile(JavaFileScannerContext context) { @Override public void visitNode(Tree tree) { - if (tree.is(Tree.Kind.MODULE)) { - withinModule = true; - return; - } - if (tree.is(Tree.Kind.ANNOTATION)) { - AnnotationTree annotationTree = (AnnotationTree) tree; - highlight(annotationTree.atToken(), annotationTree.annotationType(), typesByKind.get(Tree.Kind.ANNOTATION)); - } else { - highlight(tree, typesByKind.get(tree.kind())); + switch (tree.kind()) { + case MODULE: + withinModule = true; + return; + case ANNOTATION: + AnnotationTree annotationTree = (AnnotationTree) tree; + highlight(annotationTree.atToken(), annotationTree.annotationType(), typesByKind.get(Tree.Kind.ANNOTATION)); + return; + case YIELD_STATEMENT: + // 'yield' is a 'restricted identifier' (JSL16, $3.9) only acting as keyword in a yield statement + Optional.ofNullable(((YieldStatementTree) tree).yieldKeyword()).ifPresent(yieldKeyword -> highlight(yieldKeyword, TypeOfText.KEYWORD)); + return; + case RECORD: + // 'record' is a 'restricted identifier' (JSL16, $3.9) only acting as keyword in a record declaration + highlight(((ClassTree) tree).declarationKeyword(), TypeOfText.KEYWORD); + return; + case CLASS: + case INTERFACE: + // 'permits' is a 'restricted identifier' (JSL16, $3.9) only acting as keyword in a class/interface declaration + Optional.ofNullable(((ClassTree) tree).permitsKeyword()).ifPresent(permitsKeyword -> highlight(permitsKeyword, TypeOfText.KEYWORD)); + return; + case MODIFIERS: + // 'sealed' and 'non-sealed' are 'restricted identifier' (JSL16, $3.9) only acting as keyword in a class declaration + ModifiersTree modifiers = (ModifiersTree) tree; + ModifiersUtils.findModifier(modifiers, Modifier.SEALED).ifPresent(modifier -> highlight(modifier, TypeOfText.KEYWORD)); + ModifiersUtils.findModifier(modifiers, Modifier.NON_SEALED).ifPresent(modifier -> highlight(modifier, TypeOfText.KEYWORD)); + return; + default: + highlight(tree, typesByKind.get(tree.kind())); + return; } } diff --git a/java-frontend/src/main/java/org/sonar/java/model/JParser.java b/java-frontend/src/main/java/org/sonar/java/model/JParser.java index 624dbdb42ca..8bb22dacc2e 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/JParser.java +++ b/java-frontend/src/main/java/org/sonar/java/model/JParser.java @@ -640,9 +640,14 @@ private ClassTreeImpl convertTypeDeclaration(TypeDeclaration e, ModifiersTreeImp .complete(modifiers, declarationKeyword, name) .completeTypeParameters(convertTypeParameters(e.typeParameters())); - if (e.getAST().isPreviewEnabled()) { + if (e.getAST().isPreviewEnabled() && !e.permittedTypes().isEmpty()) { // TODO final in Java 17? relates to sealed classes - convertSeparatedTypeList(e.permittedTypes(), t.permittedTypes()); + List permittedTypes = e.permittedTypes(); + InternalSyntaxToken permitsKeyword = firstTokenBefore((Type) permittedTypes.get(0), TerminalTokens.TokenNameRestrictedIdentifierpermits); + QualifiedIdentifierListTreeImpl classPermittedTypes = QualifiedIdentifierListTreeImpl.emptyList(); + + convertSeparatedTypeList(permittedTypes, classPermittedTypes); + t.completePermittedTypes(permitsKeyword, classPermittedTypes); } if (!e.isInterface() && e.getSuperclassType() != null) { diff --git a/java-frontend/src/main/java/org/sonar/java/model/declaration/ClassTreeImpl.java b/java-frontend/src/main/java/org/sonar/java/model/declaration/ClassTreeImpl.java index 52022fbeb25..eeb291eb051 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/declaration/ClassTreeImpl.java +++ b/java-frontend/src/main/java/org/sonar/java/model/declaration/ClassTreeImpl.java @@ -62,7 +62,9 @@ public class ClassTreeImpl extends JavaTree implements ClassTree { @Nullable private SyntaxToken implementsKeyword; private ListTree superInterfaces; - private final ListTree permittedTypes = QualifiedIdentifierListTreeImpl.emptyList(); + @Nullable + private SyntaxToken permitsKeyword; + private ListTree permittedTypes; @Nullable public ITypeBinding typeBinding; @@ -74,6 +76,7 @@ public ClassTreeImpl(Kind kind, SyntaxToken openBraceToken, List members, this.modifiers = ModifiersTreeImpl.emptyModifiers(); this.typeParameters = new TypeParameterListTreeImpl(); this.superInterfaces = QualifiedIdentifierListTreeImpl.emptyList(); + this.permittedTypes = QualifiedIdentifierListTreeImpl.emptyList(); } public ClassTreeImpl complete(ModifiersTreeImpl modifiers, SyntaxToken declarationKeyword, IdentifierTree name) { @@ -104,6 +107,12 @@ public ClassTreeImpl completeInterfaces(SyntaxToken keyword, QualifiedIdentifier return this; } + public ClassTreeImpl completePermittedTypes(SyntaxToken permitsKeyword, QualifiedIdentifierListTreeImpl permittedTypes) { + this.permitsKeyword = permitsKeyword; + this.permittedTypes = permittedTypes; + return this; + } + public ClassTreeImpl completeAtToken(InternalSyntaxToken atToken) { this.atToken = atToken; return this; @@ -151,6 +160,11 @@ public ListTree superInterfaces() { return superInterfaces; } + @Override + public SyntaxToken permitsKeyword() { + return permitsKeyword; + } + @Override public ListTree permittedTypes() { return permittedTypes; diff --git a/java-frontend/src/main/java/org/sonar/plugins/java/api/tree/ClassTree.java b/java-frontend/src/main/java/org/sonar/plugins/java/api/tree/ClassTree.java index d94e6309bc5..6edc6bc2016 100644 --- a/java-frontend/src/main/java/org/sonar/plugins/java/api/tree/ClassTree.java +++ b/java-frontend/src/main/java/org/sonar/plugins/java/api/tree/ClassTree.java @@ -86,6 +86,14 @@ public interface ClassTree extends StatementTree { ListTree superInterfaces(); + /** + * @since Java 15 + * @deprecated Preview Feature + */ + @Deprecated + @Nullable + SyntaxToken permitsKeyword(); + /** * @since Java 15 * @deprecated Preview Feature diff --git a/java-frontend/src/test/files/highlighter/Records.java b/java-frontend/src/test/files/highlighter/Records.java new file mode 100644 index 00000000000..e1355c86ff9 --- /dev/null +++ b/java-frontend/src/test/files/highlighter/Records.java @@ -0,0 +1,9 @@ +package org.foo; + +record Foo(int a, String s) { + static int record; + Foo { + assert a > 42; + assert s.length() > 42; + } +} diff --git a/java-frontend/src/test/files/highlighter/SealedClass.java b/java-frontend/src/test/files/highlighter/SealedClass.java new file mode 100644 index 00000000000..54614bc3265 --- /dev/null +++ b/java-frontend/src/test/files/highlighter/SealedClass.java @@ -0,0 +1,17 @@ +package org.foo; + +public class A { + public sealed interface Shape permits Circle, Rectangle, Square, Diamond { + default void foo(int non, int sealed) { + // bugs in ECJ - should compile without spaces + int permits = non - sealed; + } + + default void foo() { } + } + + public final class Circle implements Shape { } + public non-sealed class Rectangle implements Shape { } + public final class Square implements Shape { } + public record Diamond() implements Shape { } +} diff --git a/java-frontend/src/test/files/highlighter/SwitchExpression.java b/java-frontend/src/test/files/highlighter/SwitchExpression.java new file mode 100644 index 00000000000..46e099f2395 --- /dev/null +++ b/java-frontend/src/test/files/highlighter/SwitchExpression.java @@ -0,0 +1,30 @@ +package org.foo; + +public class A { + public boolean block() { + boolean yield = false; + return switch (Bool.random()) { + case TRUE -> { + System.out.println("Bool true"); + yield true; + } + case FALSE -> { + System.out.println("Bool false"); + yield false; + } + case FILE_NOT_FOUND -> { + var ex = new IllegalStateException("Ridiculous"); + throw ex; + } + default -> yield; + }; + } + + public enum Bool { + TRUE, FALSE, FILE_NOT_FOUND; + + public static Bool random() { + return TRUE; + } + } +} diff --git a/java-frontend/src/test/java/org/sonar/java/ast/visitors/SyntaxHighlighterVisitorTest.java b/java-frontend/src/test/java/org/sonar/java/ast/visitors/SyntaxHighlighterVisitorTest.java index dfabaf188ba..34e693126de 100644 --- a/java-frontend/src/test/java/org/sonar/java/ast/visitors/SyntaxHighlighterVisitorTest.java +++ b/java-frontend/src/test/java/org/sonar/java/ast/visitors/SyntaxHighlighterVisitorTest.java @@ -42,7 +42,6 @@ import org.sonar.java.classpath.ClasspathForMain; import org.sonar.java.classpath.ClasspathForTest; import org.sonar.java.model.JavaVersionImpl; -import org.sonar.plugins.java.api.JavaCheck; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -85,7 +84,7 @@ void parse_error() throws Exception { @ValueSource(strings = {"\n", "\r\n", "\r"}) void test_different_end_of_line(String eol) throws IOException { this.eol = eol; - InputFile inputFile = generateDefaultTestFile(); + InputFile inputFile = generateTestFile("src/test/files/highlighter/Example.java"); scan(inputFile); verifyHighlighting(inputFile); } @@ -192,13 +191,62 @@ void text_block() throws Exception { assertThatHasBeenHighlighted(componentKey, 8, 12, 10, 8, TypeOfText.STRING); } - private void scan(InputFile inputFile) { - JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(10), null, null, null, null, new JavaCheck[] {syntaxHighlighterVisitor}); - frontend.scan(Collections.singletonList(inputFile), Collections.emptyList(), Collections.emptyList()); + /** + * Java 15 + */ + @Test + void switch_expression() throws Exception { + this.eol = "\n"; + InputFile inputFile = generateTestFile("src/test/files/highlighter/SwitchExpression.java"); + scan(inputFile); + + String componentKey = inputFile.key(); + assertThatHasBeenHighlighted(componentKey, 9, 9, 9, 14, TypeOfText.KEYWORD); // yield true + assertThatHasBeenHighlighted(componentKey, 13, 9, 13, 14, TypeOfText.KEYWORD); // yield false + assertThatHasBeenHighlighted(componentKey, 19, 7, 19, 14, TypeOfText.KEYWORD); // default + assertThatHasNotBeenHighlighted(componentKey, 19, 18, 19, 23); // yield as identifier + } + + /** + * Java 16 + */ + @Test + void records() throws Exception { + this.eol = "\n"; + InputFile inputFile = generateTestFile("src/test/files/highlighter/Records.java"); + scan(inputFile); + + String componentKey = inputFile.key(); + assertThatHasBeenHighlighted(componentKey, 3, 1, 3, 7, TypeOfText.KEYWORD); // record + assertThatHasNotBeenHighlighted(componentKey, 4, 14, 4, 20); // record as identifier + } + + /** + * Java 16 (second preview) + */ + @Test + void sealed_classes() throws Exception { + this.eol = "\n"; + InputFile inputFile = generateTestFile("src/test/files/highlighter/SealedClass.java"); + scan(inputFile); + + String componentKey = inputFile.key(); + assertThatHasBeenHighlighted(componentKey, 4, 19, 4, 25, TypeOfText.KEYWORD); // sealed + assertThatHasNotBeenHighlighted(componentKey, 5, 35, 5, 41); // sealed as variable name + + assertThatHasBeenHighlighted(componentKey, 4, 33, 4, 40, TypeOfText.KEYWORD); // permits + assertThatHasNotBeenHighlighted(componentKey, 7, 11, 7, 18); // permits as variable name + + assertThatHasBeenHighlighted(componentKey, 14, 10, 14, 20, TypeOfText.KEYWORD); // non-sealed + // TODO fixme ECJ bug? should not require spaces + assertThatHasNotBeenHighlighted(componentKey, 7, 21, 7, 23); // non-sealed as expression + + assertThatHasBeenHighlighted(componentKey, 16, 10, 16, 16, TypeOfText.KEYWORD); // record } - private InputFile generateDefaultTestFile() throws IOException { - return generateTestFile("src/test/files/highlighter/Example.java"); + private void scan(InputFile inputFile) { + JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(), null, null, null, null, syntaxHighlighterVisitor); + frontend.scan(Collections.singletonList(inputFile), Collections.emptyList(), Collections.emptyList()); } private InputFile generateTestFile(String sourceFile) throws IOException { diff --git a/java-frontend/src/test/java/org/sonar/java/model/JParserSemanticTest.java b/java-frontend/src/test/java/org/sonar/java/model/JParserSemanticTest.java index affb6e9eff7..7a407cccace 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/JParserSemanticTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/JParserSemanticTest.java @@ -85,6 +85,7 @@ import org.sonar.plugins.java.api.tree.VariableTree; import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.java.model.assertions.TreeAssert.assertThat; import static org.sonar.java.model.assertions.TypeAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -1021,7 +1022,9 @@ void declaration_sealed_class() { assertThat(c.modifiers()).hasSize(1); ModifierKeywordTree m = (ModifierKeywordTree) c.modifiers().get(0); assertThat(m.modifier()).isEqualTo(Modifier.SEALED); - assertThat(m.firstToken().text()).isEqualTo("sealed"); + assertThat(m.firstToken()).is("sealed"); + + assertThat(c.permitsKeyword()).is("permits"); assertThat(c.permittedTypes()).hasSize(2); cu = test("non-sealed class Square extends Shape { }"); @@ -1029,7 +1032,7 @@ void declaration_sealed_class() { assertThat(c.modifiers()).hasSize(1); m = (ModifierKeywordTree) c.modifiers().get(0); assertThat(m.modifier()).isEqualTo(Modifier.NON_SEALED); - assertThat(c.modifiers().get(0).firstToken().text()).isEqualTo("non-sealed"); + assertThat(c.modifiers().get(0).firstToken()).is("non-sealed"); } /**