Skip to content

AnnotationsUseStyle yields warning on "({})" of Array types #28

Closed
romani opened this Issue Oct 26, 2013 · 9 comments

2 participants

@romani
Checkstyle member
romani commented Oct 26, 2013

Created: 2012-07-31
Creator: Bjoern Kimminich
SF issue: 678 (https://sourceforge.net/p/checkstyle/bugs/678/)

There should be an exceptional handling of the meta-annotation @Target({}) in the AnnotationsUseStyle check. Currently it is reported as "Annotations cannot have closing parenthesis" when using the closingParens:never option. As this is a meta-exception it should be excluded from the validation.

http://checkstyle.sourceforge.net/config_annotation.html#AnnotationUseStyle

http://docs.oracle.com/javase/7/docs/api/java/lang/annotation/Target.html

Problem with treatment of Arrays, as Checkstyle does not have ability to know type of classes the declared in other files, we could rely only on user to privide us list of Annotations that have Array as values. Default value should be "Target" , option name is closingParensIgnoreList, that contains list of Annotation names separated by coma.

We still come to problem full name usage in code "@java.lang.annotation.Target({})", we can ask user to provide full name, additional analys of imports will be required, to distinguish Target from java.lang and from users Annotaton.

Additionally we need to put on web side description from JavaDoc, do the same for Eclipse CS plugin. We need to add description with code example what does closingParens mean, as took we awile to understand a reason of option.

package test;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
@Retention(value=RetentionPolicy.RUNTIME)
//@SomeAnnotation({ElementType.FIELD, ElementType.METHOD})
@SomeAnnotation({})
//@Target({ElementType.FIELD, ElementType.METHOD})
@Target({})
public @interface MyAnn {

}
package test;

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

@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.ANNOTATION_TYPE)
public @interface SomeAnnotation  {
    String[] value();
}

Example of error if annotation is "@Target()" or "@Target":

sh-4.3# javac HelloWorld.java                                                                                                                                    
HelloWorld.java:10: error: annotation @Target is missing a default value for the element 'value'                                                                 
@Target()                                                                                                                                                
^                                                                                                                                                               
1 error  
@alexkravin alexkravin was assigned by romani Mar 12, 2015
@alexkravin

Please be more concrete as I only understood that right now Check warns on empty {}
What kind of option should be there? What user has to specify?
It will be very convenient if you'll show an example

@romani
Checkstyle member
romani commented Mar 15, 2015

I updated description a bit but please continue investigation why some annotations could be used without Parens and other (like Target) do not.

that will make us understand what option should be created ...

@alexkravin

Thank you for description.

If there's any param in annotation it would be required until default value is provided in annotation definition, e.g.:

Target, Retention annotations:

@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.ANNOTATION_TYPE)
public @interface Retention {
    RetentionPolicy value(); // is required in annotation declaration
}

@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.ANNOTATION_TYPE)
public @interface Target {
    ElementType[] value(); // is required in annotation declaration
}

Spring's annotations example:

@Target({ElementType.CONSTRUCTOR, ElementType.FIELD, ElementType.METHOD, ElementType.ANNOTATION_TYPE})
@Retention(RetentionPolicy.RUNTIME)
@Documented
public @interface Autowired {

    /**
     * Declares whether the annotated dependency is required.
     * <p>Defaults to {@code true}.
     */
    boolean required() default true; // default value is provided

}

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Documented
public @interface Component {

    /**
     * The value may indicate a suggestion for a logical component name,
     * to be turned into a Spring bean in case of an autodetected component.
     * @return the suggested component name, if any
     */
    String value() default ""; // default value is provided

}

So params are optional and you don't need to write @Autowired(required=true) as it's so by default

@romani
Checkstyle member
romani commented Mar 17, 2015

So Check have to deal with Annotations that do not have default values,
new option - annnotaionsWihtoutDefault. It will store canonical names of annotations. Please review all annotations from http://docs.oracle.com/javase/7/docs/api/java/lang/annotation/package-summary.html and make a default value for option form some of that annotations.

@alexkravin

I can't reproduce the problem.

Input:

@Target({})
public @interface myAnn {

}

...

checkConfig.addAttribute("closingParens", "NEVER"); // as it's described in original issue

No violations

@romani
Checkstyle member
romani commented Mar 17, 2015

Please provide UT and provide explanation why and where Check manage that

@alexkravin alexkravin added a commit to alexkravin/checkstyle that referenced this issue Mar 17, 2015
@alexkravin alexkravin Annotation Use Style Check, issue #28 44c8e2d
@alexkravin

See UT and input in referencing commit.

Looks like this if:

else if (ClosingParens.NEVER == this.parens
            && !ast.branchContains(TokenTypes.EXPR)
            && !ast.branchContains(TokenTypes.ANNOTATION_MEMBER_VALUE_PAIR)
            && !ast.branchContains(TokenTypes.ANNOTATION_ARRAY_INIT)
            && parenExists)
        {
            this.log(ast.getLineNo(), MSG_KEY_ANNOTATION_PARENS_PRESENT);
        }

Determines cases without violations

@romani
Checkstyle member
romani commented Mar 17, 2015

please do PR with that UT, looks like issue is resolved.

@alexkravin

Done

@romani romani closed this Mar 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.