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

False positive in AnnotationUseStyle for version 8.17 #6446

Closed
davidburstromspotify opened this issue Feb 21, 2019 · 10 comments

Comments

Projects
None yet
3 participants
@davidburstromspotify
Copy link

commented Feb 21, 2019

https://checkstyle.org/config_annotation.html#AnnotationUseStyle
https://checkstyle.org/property_types.html#closingParens

/var/tmp $ javac Example.java

/var/tmp $ cat config.xml
<?xml version="1.0"?>
<!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="AnnotationUseStyle" />
    </module>
</module>

/var/tmp $ cat Example.java
@Anno(@Value)
public class Example {
}

@interface Anno {
    Value value();
}

@interface Value {
}

/var/tmp $ RUN_LOCALE="-Duser.language=en -Duser.country=US"
/var/tmp $ java $RUN_LOCALE -jar checkstyle-8.17-all.jar -c config.xml Example.java
Starting audit...
[ERROR] /var/tmp/Example.java:1: Annotation cannot have closing parenthesis. [AnnotationUseStyle]
Audit done.
Checkstyle ends with 1 errors.

I don't expect an error, as the closing parenthesis is for the outer annotation.


@davidburstromspotify

This comment has been minimized.

Copy link
Author

commented Feb 21, 2019

I should mention this is not an issue in 8.16.

@rnveach

This comment has been minimized.

Copy link
Member

commented Feb 21, 2019

Violation is happening.

I should mention this is not an issue in 8.16.

@davidburstromspotify This statement is not true for me. I get regression going all the way back to 5.0 when the check was created.

Here is 8.16:

$ cat TestClass.java
@Anno(@Value)
public class Example {
}

@interface Anno {
    Value value();
}

@interface Value {
}

$ cat TestConfig.xml
<?xml version="1.0"?>
<!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="AnnotationUseStyle" />
    </module>
</module>

$ java -jar checkstyle-8.16-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:1: Annotation cannot have closing parenthesis. [AnnotationUseStyle]
Audit done.
Checkstyle ends with 1 errors.
@davidburstromspotify

This comment has been minimized.

Copy link
Author

commented Feb 21, 2019

@rnveach Thank you for pointing that out.

Upon further investigation, the scenario that worked in 8.16 but broke in 8.17 was
@Anno(value = @Value)

@romani romani added the approved label Feb 22, 2019

@romani

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

@davidburstromspotify, just curious, what for such code (annotation inside annotation) might be used ?
Can you share with us some other non simple usage of annotations? We will extend our test base. I really surprised that it is possible/compilable.

@rnveach

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

@romani Are both cases false positives or is it just the 2nd one?

@Anno(@Value)
@Anno(value = @Value)

I have a feeling this issue was created because of pitest removals.

@davidburstromspotify

This comment has been minimized.

Copy link
Author

commented Feb 22, 2019

For example, this can be used to supply test data, like @TestData(@AccessPoint) where @AccessPoint has a default value for the AP, as opposed to @TestData(@AccessPoint("127.0.01"))

@romani

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

On default config of Check:

@Anno(@Value) // no violation
@Anno(value = @Value) //no violation
@Anno() // violation
@Anno(@Value()) // violation
@Anno(value = @Value()) //violation
@davidburstromspotify

This comment has been minimized.

Copy link
Author

commented Feb 22, 2019

That looks like what we want, yes.

@rnveach

This comment has been minimized.

Copy link
Member

commented Mar 10, 2019

@Anno(@Value) // no violation

This produced a violation even in 8.16 before breaking change came in.
I will move this to a new issue.

@romani

This comment has been minimized.

Copy link
Member

commented Mar 10, 2019

Fix is merged

@romani romani closed this Mar 10, 2019

Vantuz added a commit to Vantuz/checkstyle that referenced this issue Apr 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.