Skip to content

Commit

Permalink
SONARJAVA-3904 Fix highlighting (#3699)
Browse files Browse the repository at this point in the history
  • Loading branch information
Wohops authored Jul 19, 2021
1 parent ace9f4c commit ff019c7
Show file tree
Hide file tree
Showing 9 changed files with 194 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,25 @@
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;
import org.sonar.api.batch.sensor.highlighting.TypeOfText;
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 {

Expand All @@ -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()));

Expand All @@ -71,9 +77,19 @@ public SyntaxHighlighterVisitor(SonarComponents sonarComponents) {
@Override
public List<Tree.Kind> nodesToVisit() {
List<Tree.Kind> 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);
}

Expand All @@ -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;
}
}

Expand Down
9 changes: 7 additions & 2 deletions java-frontend/src/main/java/org/sonar/java/model/JParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ public class ClassTreeImpl extends JavaTree implements ClassTree {
@Nullable
private SyntaxToken implementsKeyword;
private ListTree<TypeTree> superInterfaces;
private final ListTree<TypeTree> permittedTypes = QualifiedIdentifierListTreeImpl.emptyList();
@Nullable
private SyntaxToken permitsKeyword;
private ListTree<TypeTree> permittedTypes;
@Nullable
public ITypeBinding typeBinding;

Expand All @@ -74,6 +76,7 @@ public ClassTreeImpl(Kind kind, SyntaxToken openBraceToken, List<Tree> 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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -151,6 +160,11 @@ public ListTree<TypeTree> superInterfaces() {
return superInterfaces;
}

@Override
public SyntaxToken permitsKeyword() {
return permitsKeyword;
}

@Override
public ListTree<TypeTree> permittedTypes() {
return permittedTypes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ public interface ClassTree extends StatementTree {

ListTree<TypeTree> superInterfaces();

/**
* @since Java 15
* @deprecated Preview Feature
*/
@Deprecated
@Nullable
SyntaxToken permitsKeyword();

/**
* @since Java 15
* @deprecated Preview Feature
Expand Down
9 changes: 9 additions & 0 deletions java-frontend/src/test/files/highlighter/Records.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.foo;

record Foo(int a, String s) {
static int record;
Foo {
assert a > 42;
assert s.length() > 42;
}
}
17 changes: 17 additions & 0 deletions java-frontend/src/test/files/highlighter/SealedClass.java
Original file line number Diff line number Diff line change
@@ -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 { }
}
30 changes: 30 additions & 0 deletions java-frontend/src/test/files/highlighter/SwitchExpression.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -1021,15 +1022,17 @@ 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 { }");
c = (ClassTreeImpl) cu.types().get(0);
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");
}

/**
Expand Down

0 comments on commit ff019c7

Please sign in to comment.