Hello, ClebertAfter review, we have some minor objections to your code, could you, please, resolve them?1.String names[] = getAnnotationParameters(aAST);for (String p : names) {if (requiredParameters.contains(p)) {found++;}}I think, it would be better to calculate set of actually present required parameters and then just compare it with desired set with equals().Like this:if(!getRequiredAnnotationParameters(aAST).equals(this.requiredParameters)) {log(...)}
So, it would be no need to count required annotation parameters.
2. There should be no variable names like 'p', 'text' or 'found'. Every variable name should exactly explain what is it for.e.g.:p --> annotationParameterNametext --> requiredParameterfound --> foundRequiredParametersCount
3. Please format your code with our Formatter for Checkstyle and check it with our 'Checkstyle for Checkstyle' configuration. See more details at https://github.com/sevntu-checkstyle/sevntu.checkstyle/wiki/Java-code-Formater-and-Checkstyle-configuration-for-development
4. As far as I remember there is a Guava dependency in Checkstyle. So, you could use Guava's Joiner to let codeIterator<String> iter = requiredParameters.iterator();while (iter.hasNext()) {String text = iter.next();propertiesText.append(text);if (iter.hasNext()) {propertiesText.append(",");}}look like this:Joiner.on(", ").join(requiredParameters);
With such a custom build, you could start using your check before we will merge it to main repo and it appear in next releases.
Hello, Clebert!
Sorry, for delay,
1. Please remove redundant return at ForbidAnnotationCheck.java, line 25 as it has appeared after your changes.
2. There is a minor typo in Check JavaDoc:
Marks a given parameter as required for a giveN annotation
3. Please update Joiner.on(",") to Joiner.on(", ") to let printed error be better formatted.
4. From /eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/annotation/checkstyle-metadata.properties:
RequiredParameterForAnnotation.name = Required Parameters on Annotation
RequiredParameterForAnnotation.desc = Enforce the use of configured parameters on a given annotation
ForbidAnnotation.annotationName = The name of the annotation where we are enforcing the parameter
ForbidAnnotation.requiredParameters = Set of parameters that are required on the configured annotation. They can be specified on any order and you may use additional parameters, but these parameters need to be filled.
please update 'ForbidAnnotation' to 'RequiredParameterForAnnotation' in order to let description for check properties be visible in plugin's UI.
Also, please update description for 'requiredParameters' property in order to contain words about several required annotations could be written separated by comma.
Ans also, just curious: what app generates ".metadata" files you've added to .gitignore?
--
You received this message because you are subscribed to a topic in the Google Groups "Sevntu Checkstyle" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sevntu-checkstyle/YibCclEYexs/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sevntu-checkst...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Hello, Clebert
--
You received this message because you are subscribed to a topic in the Google Groups "Sevntu Checkstyle" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sevntu-checkstyle/YibCclEYexs/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sevntu-checkst...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
String requiredParametersAsString = Joiner.on(", ").join( |
missingParameters); |
log(aAST, MSG_KEY, this.annotationName, |
requiredParametersAsString); |
--
You received this message because you are subscribed to a topic in the Google Groups "Sevntu Checkstyle" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sevntu-checkstyle/YibCclEYexs/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sevntu-checkst...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Do you need help?
Just checking a progress.
--
You received this message because you are subscribed to a topic in the Google Groups "Sevntu Checkstyle" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sevntu-checkstyle/YibCclEYexs/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sevntu-checkst...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
For example if | |||||||
* you enforce the parameter "description" on @EJB, you are still free to use | |||||||
* beanName if you like. Please move examples to the bottom and do that with code example. In first paragraph we need to explain all by words only. 11) > value="ThePropertyName,ThePropertyName2,...ThePropertyNameN"/> </module> all examples of config should follow with java code example , please redo that example. 12) >
|
--
You received this message because you are subscribed to the Google Groups "Sevntu Checkstyle" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sevntu-checkstyle+unsubscribe@googlegroups.com.