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

Checkstyle parsing fails for complex annotations supported by Lombok #4633

Closed
denizarsan opened this issue Jul 6, 2017 · 9 comments
Closed
Labels

Comments

@denizarsan
Copy link

denizarsan commented Jul 6, 2017

$ wget -q https://projectlombok.org/downloads/lombok-1.16.4.jar
$ javac -cp lombok-1.16.4.jar C.java
$ cat C.java

// simplified from https://github.com/TheLQ/pircbotx/blob/406a0f98a94693a2c8037d82f450e1400bfe767d/src/main/java/org/pircbotx/hooks/events/PrivateMessageEvent.java
class C extends D { // D added just to compile with two annotations
    @lombok.Getter(onMethod = @__(@Override, @Deprecated)) // need two (different) annotations!
    int i;
}
abstract class D { abstract int getI(); } // D added just to compile

$ cat config.xml

<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.3//EN" "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
<module name="Checker">
  <module name="TreeWalker">
    <module name="LineLength"/> <!-- can be any check that requires parsing -->
  </module>
</module>

$ java -jar checkstyle-8.0-all.jar -c config.xml C.java

Starting audit...
/tmp/C.java:3:44: expecting RPAREN, found ','
/tmp/C.java:3:57: unexpected token: )
com.puppycrawl.tools.checkstyle.api.CheckstyleException: Exception was thrown while processing C.java
        at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:295)
        at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:213)
        at com.puppycrawl.tools.checkstyle.Main.runCheckstyle(Main.java:430)
        at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:362)
        at com.puppycrawl.tools.checkstyle.Main.main(Main.java:177)
Caused by: com.puppycrawl.tools.checkstyle.api.CheckstyleException: MismatchedTokenException occurred during the analysis of file C.java.
        at com.puppycrawl.tools.checkstyle.TreeWalker.processFiltered(TreeWalker.java:199)
        at com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck.process(AbstractFileSetCheck.java:78)
        at com.puppycrawl.tools.checkstyle.Checker.processFile(Checker.java:316)
        at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:286)
        ... 4 more
Caused by: /tmp/C.java:5:1: expecting EOF, found '}'
        at antlr.Parser.match(Parser.java:211)
        at com.puppycrawl.tools.checkstyle.grammars.GeneratedJavaRecognizer.compilationUnit(GeneratedJavaRecognizer.java:211)
        at com.puppycrawl.tools.checkstyle.TreeWalker.parse(TreeWalker.java:446)
        at com.puppycrawl.tools.checkstyle.TreeWalker.processFiltered(TreeWalker.java:179)
        ... 7 more
Checkstyle ends with 1 errors.

The parsing problems that eventually lead to the exception start from @__(@Override, @Deprecated), which the Lombok annotation processor can process fine, without requiring @__({@Override, @Deprecated}).

@rnveach
Copy link
Member

rnveach commented Jul 6, 2017

@denizarsan I am getting the following errors in eclipse:

Description	Resource	Path	Location	Type
Syntax error on token ",", ( expected	C.java	/checkstyle/src/main/java	line 3	Java Problem
Syntax error, insert ")" to complete Modifiers	C.java	/checkstyle/src/main/java	line 3	Java Problem
The type C must implement the inherited abstract method D.getI()	C.java	/checkstyle/src/main/java	line 2	Java Problem

This error is valid and shows the file isn't compilable: The type C must implement the inherited abstract method D.getI()

Please update your example so it is compilable. Verify output with javac.

@denizarsan
Copy link
Author

@rnveach You will need to add lombok-1.16.4.jar to Eclipse to have the annotations work. I am able to compile it through the command line.

@rnveach
Copy link
Member

rnveach commented Jul 6, 2017

Edit: I see now lombok will generate the getter for me with this annotation.
I will test this again locally.

@romani
Copy link
Member

romani commented Jul 7, 2017

We should support all code that compiled by javac , except for know limitations like code in UTF

@romani
Copy link
Member

romani commented Jul 14, 2017

if such code is rare case you can skip files from chekstyle consideration by http://checkstyle.sourceforge.net/config_filefilters.html#BeforeExecutionExclusionFileFilter

@rnveach
Copy link
Member

rnveach commented Feb 18, 2018

Parsing of example passes when , @Deprecated is removed.
So parser's failure is centered around it not recognizing/understanding it is a comma-delimited annotation list.
I think issue resides in annotationMemberValueInitializer or it's children.

Parsing will pass if {} is added around the 2 inner annotations.

@lombok.Getter(onMethod = @__({@Override, @Deprecated}))

Even lombok's guide recommends to use it this way.

To apply multiple annotations, use @__({@Annotation1, @Annotation2}).

https://projectlombok.org/features/experimental/onX

By using a non-existent annotation @__ it cannot determine it is bogus (it might be created by an annotation processor) and will not give an error right away. That gives Lombok the time to do its work and remove the @__ from the code.

https://stackoverflow.com/a/38059047/1016482
The annotation doesn't exist, so it's structure can't be determined.

@rnveach
Copy link
Member

rnveach commented Feb 18, 2018

@denizarsan @romani My recommendation is we should close this issue as the style of code being written cannot be parsed by javac at all naturally. Users are abusing lombok's pre-compile override to write non-standard JLS code.

Here is my example:

$ cat TestClass.java
package lombok;

import static java.lang.annotation.ElementType.CONSTRUCTOR;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.LOCAL_VARIABLE;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.ElementType.TYPE;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

public class Test {
	@AnnotationTest(value = { @AnnotationTest1, @AnnotationTest1 })
	public void m53() {
	}

	@AnnotationTest({ @AnnotationTest1, @AnnotationTest1 })
	public void m54() {
	}
	
	@AnnotationTest(@AnnotationTest1, @AnnotationTest1)
	public void m55() {
	}
}

@Target({ TYPE, FIELD, METHOD, PARAMETER, CONSTRUCTOR, LOCAL_VARIABLE })
@Retention(RetentionPolicy.SOURCE)
@interface AnnotationTest {
	AnnotationTest1[] value();
}

@Target({ TYPE, FIELD, METHOD, PARAMETER, CONSTRUCTOR, LOCAL_VARIABLE })
@Retention(RetentionPolicy.SOURCE)
@interface AnnotationTest1 {
}

$ javac TestClass.java
TestClass.java:23: error: annotation values must be of the form 'name=value'
	@AnnotationTest(@AnnotationTest1, @AnnotationTest1)
	                ^
TestClass.java:23: error: annotation values must be of the form 'name=value'
	@AnnotationTest(@AnnotationTest1, @AnnotationTest1)

$ javac -version
javac 1.8.0_151

The first 2 forms compile fine. The last one doesn't.

@rnveach rnveach removed the approved label Feb 18, 2018
@romani
Copy link
Member

romani commented Feb 19, 2018

we support only what javac support, user need to exclude such files from validation by Chekstyle, http://checkstyle.sourceforge.net/config_filefilters.html#BeforeExecutionExclusionFileFilter.

@romani romani closed this as completed Feb 19, 2018
@rnveach
Copy link
Member

rnveach commented Feb 20, 2018

user need to exclude such files from validation by Chekstyle,

All that is needed is to fix Java code to be JLS compliant.
Like I showed in my example, only the {}s are missing.

Code should be:

@lombok.Getter(onMethod = @__({ @Override, @Deprecated }))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants