maxvetrenko / checkstyle

Checkstyle is a development tool to help programmers write Java code that adheres to a coding standard. By default it supports the Sun Code Conventions, but is highly configurable. It can be invoked with an ANT task and a command line program.
http://checkstyle.sourceforge.net/
GNU Lesser General Public License v2.1
0 stars 0 forks source link

New Check: AnnotationLocation #19

Closed maxvetrenko closed 10 years ago

maxvetrenko commented 10 years ago

http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s4.8.5-annotations

These line breaks do not constitute line-wrappin, so the indentation level is not increased. Example:

@Override
@Nullable
public String getNameIfPresent() { ... }
rdiachenko commented 10 years ago

We have to cover the following cases:

  1. Annotations applying to a class, method, constructor or field should appear immediately after the documentation block
  2. Multiple annotations applying to a class, method or constructor should be separated by a new line
maxvetrenko commented 10 years ago

Agree with Ruslan.

romani commented 10 years ago
  1. - agree.
  2. it is not a rule - see example "@Partial @Mock DataLoader loader;" in GoogeStyle , it is not a rule, it is up to user.
rdiachenko commented 10 years ago

@Partial @Mock DataLoader loader;

According to a GoogleStyle it is an exception which is applied only to fields. We can use both: @Partial @Mock DataLoader loader; and @Partial @Mock DataLoader loader;

What GoogleStyle says about fields:

but in this case, multiple annotations (possibly parameterized) may be listed on the same line

What GoogleStyle says about class, method or constructor:

each annotation is listed on a line of its own (that is, one annotation per line)

Roman, let's discuss it via Skype if you're disagree

maxvetrenko commented 10 years ago

You forgot about single parameterless annotation. It may instead appear together with the first line of the signature, for example: @Override public int hashCode() { ... } I'm ready to discuss it with you.

romani commented 10 years ago

my bad - I call back my point "2.". But we have to allow single no argument annotation for method as Max posted above.

maxvetrenko commented 10 years ago
  1. Выполнить поиск аннотаций.
  2. Если количество 1, то проверить, есть ли у нее параметры: а. Параметры есть. Проверить, не находится ли она на одной строке с MethodDef, CtorDef, EnumDef, InterfaceDef или ClassDef. Если находится, то ОШИБКА. б. Параметров нет. Перейти к следующей аннотации
  3. Если количество больше 1, то выполнить проверку, нет ли двух или аннотации и MethodDef, CtorDef, EnumDef, InterfaceDef или ClassDef на одной строке. Если есть, то ОШИБКА.
  4. Если аннотации относятся к филдам или локальным переменным - НИЧЕГО НЕ ДЕЛАТЬ
rdiachenko commented 10 years ago

1)

Проверить indentation. Если он отличается от indentation MethodDef, CtorDef или ClassDef - ОШИБКА Indentation: если Indentation аннотации и Variable_Def отличается, то ОШИБКА.

Why do we have to check the indentation? I don't think it is a job of this check

2) Don't forget about enums and interfaces

maxvetrenko commented 10 years ago

@romani

<module name="AnnotationIndentation"/>

maxvetrenko commented 10 years ago

@rdiachenko

Why do we have to check the indentation? I don't think it is a job of this check

I agree with you. We shouldn't check Indentation. I have updated issue: https://github.com/maxvetrenko/checkstyle/issues/26

Don't forget about enums and interfaces

Done

romani commented 10 years ago

I do not like that implementation, i do not see how you diff location on annotation for methods and fields separately. Check should be named - AnnotationLocation, as we do not check indentation only, we mostly focused location( same line or separate line). Please provide me links who do indentation verification of annotation. That check should be desined to have two options:

  • allowSamelineSingleParrametterlessAnnotation
  • allowSamelineaParametrizedAnnotation

User will create few intances of configuration- for fileds, for methods, for class, ...

maxvetrenko commented 10 years ago

@romani I redesigned Check, now it separately checks fields annotations and else annotations. checkAnnotations() and checkFieldAnnotations() validate both cases. Also was added allowSamelineParametrizedAnnotation and allowSameLineSingleParrametterlessAnnotation options.