Skip to content
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.

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

Issue #3238: Java 8 Grammar: annotations on arrays and varargs #7407

Merged
merged 1 commit into from Jan 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -170,8 +170,7 @@ public void setAllowLineBreaks(boolean allowLineBreaks) {
public void visitToken(DetailAST ast) {
final DetailAST whitespaceFollowedAst = getWhitespaceFollowedNode(ast);

if (whitespaceFollowedAst.getNextSibling() == null
|| whitespaceFollowedAst.getNextSibling().getType() != TokenTypes.ANNOTATIONS) {
if (shouldCheckWhitespaceAfter(whitespaceFollowedAst)) {
final int whitespaceColumnNo = getPositionAfter(whitespaceFollowedAst);
final int whitespaceLineNo = whitespaceFollowedAst.getLineNo();

Expand Down Expand Up @@ -206,6 +205,27 @@ private static DetailAST getWhitespaceFollowedNode(DetailAST ast) {
return whitespaceFollowedAst;
}

/**
* Returns whether whitespace after a visited node should be checked. For example, whitespace
* is not allowed between a type and an array declarator (returns true), except when there is
* an annotation in between the type and array declarator (returns false).
* @param ast the visited node
* @return true if whitespace after ast should be checked
*/
private static boolean shouldCheckWhitespaceAfter(DetailAST ast) {
boolean checkWhitespace = true;
final DetailAST sibling = ast.getNextSibling();
if (sibling != null) {
if (sibling.getType() == TokenTypes.ANNOTATIONS) {
checkWhitespace = false;
}
else if (sibling.getType() == TokenTypes.ARRAY_DECLARATOR) {
checkWhitespace = sibling.getFirstChild().getType() != TokenTypes.ANNOTATIONS;
}
}
return checkWhitespace;
}

/**
* Gets position after token (place of possible redundant whitespace).
* @param ast Node representing token.
Expand Down
23 changes: 17 additions & 6 deletions src/main/resources/com/puppycrawl/tools/checkstyle/grammar/java.g
Expand Up @@ -257,6 +257,14 @@ typeSpec[boolean addImagNode]
| builtInTypeSpec[addImagNode]
;

// A type specification for a variable length parameter is a type name with
// possible brackets afterwards that can end with annotations.
variableLengthParameterTypeSpec
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reference: LastFormalParameter which allows zero or more Annotation just before the ...

: (classOrInterfaceType[false] | builtInType)
rnveach marked this conversation as resolved.
Show resolved Hide resolved
({LA(1) == AT}? annotations | )
(lb:LBRACK^ {#lb.setType(ARRAY_DECLARATOR);} RBRACK ({LA(1) == AT}? annotations | ))*
;

// A class type specification is a class type with either:
// - possible brackets afterwards
// (which would make it an array type).
Expand Down Expand Up @@ -387,8 +395,8 @@ builtInTypeSpec[boolean addImagNode]
// A type name. which is either a (possibly qualified and parameterized)
// class name or a primitive (builtin) type
type
: classOrInterfaceType[false]
| builtInType
: ({LA(1) == AT}? annotations | )
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: type is only used in newExpression

reference: in the spec, PrimitiveType allows an annotation (similar to ClassOrInterfaceTypeToInstantiate)

PrimitiveType:
  {Annotation} NumericType
  {Annotation} boolean

but while checkstyle's classOrInterfaceType allows annotations, builtInType by itself doesn't.

My thinking was to allow the annotation here in type for either case, rather than update builtInType which is used in several other places, so hoping to minimize changes.

(classOrInterfaceType[false] | builtInType)
;

/** A declaration is the creation of a reference or primitive-type variable
Expand Down Expand Up @@ -893,9 +901,11 @@ variableDeclarator![AST mods, AST t]
{#variableDeclarator = #(#[VARIABLE_DEF,"VARIABLE_DEF"], mods, #(#[TYPE,"TYPE"],d), id, v);}
;

declaratorBrackets[AST typ]
: {#declaratorBrackets=typ;}
(lb:LBRACK^ {#lb.setType(ARRAY_DECLARATOR);} RBRACK)*
declaratorBrackets![AST typ]
: ({LA(1) == AT}? an:annotations | ) lb:LBRACK {#lb.setType(ARRAY_DECLARATOR);} rb:RBRACK
db:declaratorBrackets[#(lb, typ, an, rb)]
{#declaratorBrackets = #db;}
| {#declaratorBrackets = typ;}
esilkensen marked this conversation as resolved.
Show resolved Hide resolved
;

varInitializer
Expand Down Expand Up @@ -969,7 +979,7 @@ parameterDeclarationList
;

variableLengthParameterDeclaration!
: pm:parameterModifier t:typeSpec[false] td:ELLIPSIS IDENT
: pm:parameterModifier t:variableLengthParameterTypeSpec td:ELLIPSIS IDENT
pd:declaratorBrackets[#t]
{#variableLengthParameterDeclaration = #(#[PARAMETER_DEF,"PARAMETER_DEF"],
pm, #([TYPE,"TYPE"],pd), td, IDENT);}
Expand Down Expand Up @@ -1592,6 +1602,7 @@ newArrayDeclarator
warnWhenFollowAmbig = false;
}
:
({LA(1) == AT}? annotations | )
lb:LBRACK^ {#lb.setType(ARRAY_DECLARATOR);}
(expression)?
RBRACK
Expand Down
Expand Up @@ -68,6 +68,12 @@ public void testJava8ClassAstTree1() throws Exception {
getPath("InputRegressionJava8Class1.java"));
}

@Test
public void testJava8ClassAstTree2() throws Exception {
verifyAst(getPath("InputRegressionJava8Class2Ast.txt"),
getPath("InputRegressionJava8Class2.java"));
}

@Test
public void testInputSemicolonBetweenImports() throws Exception {
verifyAst(getPath("InputSemicolonBetweenImportsAst.txt"),
Expand Down
Expand Up @@ -132,4 +132,13 @@ public void testAnnotationInTypeParameters()
verify(checkConfig, getPath("InputAnnotations11.java"), expected);
}

@Test
public void testAnnotationOnVarargs()
throws Exception {
final DefaultConfiguration checkConfig =
createModuleConfig(MemberNameCheck.class);
final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verify(checkConfig, getPath("InputAnnotations12.java"), expected);
}

}
Expand Up @@ -9,6 +9,9 @@ public void testWithAnnotationInMiddle1(final char @AnnotationAfterTest [] a) {}
public void testWithAnnotationInMiddle2(final char@AnnotationAfterTest [] a) {}//Correct
public void testWithAnnotationInMiddle3(final char @AnnotationAfterTest[] a) {}//Correct
public void testWithAnnotationInMiddle4(final char@AnnotationAfterTest[]a) {}//Correct
public @AnnotationAfterTest String @AnnotationAfterTest [] testWithAnnotationInMiddle5() {
return new @AnnotationAfterTest String @AnnotationAfterTest [3];//Correct
}

@Target(ElementType.TYPE_USE)
@interface AnnotationAfterTest {
Expand Down
@@ -0,0 +1,38 @@
//start line index in expected file is 2
package com.puppycrawl.tools.checkstyle.grammar;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.Map;
import java.util.List;
import java.util.function.IntBinaryOperator;
import java.util.function.Predicate;
import java.util.function.Supplier;

public class InputRegressionJava8Class2 {
static class Inner1 { static class Inner2<V> { public void m() {} } }
static class Inner3<T> { public void m() {} }

public void m1(@MyAnnotation String @MyAnnotation ... vararg) {}
public String m2() @MyAnnotation [] @MyAnnotation [] { return null; }

public void instructions() {
// annotations
Map.@MyAnnotation Entry e;
String str = (@MyAnnotation String) "";
(new Inner3()).<@MyAnnotation String>m();
Object arr = new @MyAnnotation String @MyAnnotation [3];
for (String a @MyAnnotation [] : m2()) {}
Object arr2 = new @MyAnnotation int[3];
}
}

@Retention(RetentionPolicy.CLASS)
@Target({ ElementType.TYPE_USE })
@interface MyAnnotation {
}