addressbook-level4 icon indicating copy to clipboard operation
addressbook-level4 copied to clipboard

Checkstyle: add more rules to match coding standard

Open eugenepeh opened this issue 7 years ago • 3 comments

Currently several rules of the java coding standard stated in the https://oss-generic.github.io/process/codingStandards/CodingStandard-Java.html are not being enforced. Issue #283 has summarized a couple of them.

Those enforceable rules should be enforced.

Phase 1 : Complete

Rules that are outright English related are not enforceable as checkstyle does not check for English. Some, also, may not be enforceable as checkstyle currently does not have such checks yet. (Do let me know if anyone knows alternative to implement those check that I thought to be not possible)

Adding enforceable rules stated in Java Coding Convention from oss-generic. (https://oss-generic.github.io/process/codingStandards/CodingStandard-Java.html)

Current Progress (Java Coding Convention from oss-generic) : https://docs.google.com/document/d/19Ca176H0m7vCc29a_rxq2l8_TOjHkBYzuAHNxcUEAFY/edit?usp=sharing

Legend : Strikeout - checked / implemented / done / unneccessary Red - unlikely to be possible Yellow - currently/may not enforceable Green - to be done Cyan - done and ready to be merged

Phase 2 : Complete

As Java Coding Convention from oss-generic states that use the Google Java Style Guide for any topics not covered in this document. Since google has its own checkstyle configuration file, we can simply compare and collect those useful but unused in our configuration file.

Adding rules, that is not covered in oss java coding convention but beneficial, from google checkstyle.

Current Progress (Google's checkstyle) : (https://docs.google.com/document/d/1o3DeKTUxv2zH5j0RtvMXSVTBa0kbGBKWiE_qgtchbZ4/edit?usp=sharing)

Legend : Red - indicates the module does not exists on the our checkstyle Yellow - indicates the module exists on the our checkstyle but has different properties Green - indicates that the module exists on the our checkstyle and has the same/stricter properties Underlined - implementing in progress and waiting to be merge Strikeout - not in the coding convention / may not useful in our cause

Phase 3: Ongoing

Keep in sync with Checkstyle updates for checks that can covers all our other coding standards

eugenepeh avatar Mar 06 '17 04:03 eugenepeh

Hi Prof. Damith @damithc , Currently we have about 41 classes and 84 public methods (excluding single statement methods and those with specific annotation such as @Override) missing header comments, and most are self explanatory.

This rule is enforceable using JavadocType and JavadocMethod. However, it will require header comment for all classes, public methods (including trivial / self explanatory). Should we still strictly enforce (with a severe error) this rule below?

However, you MUST write header comments for all classes, public methods, and all non-trivial private methods. The code, even if it is self-explanatory, can only tell the reader HOW the code works, not WHAT the code is supposed to do.

eugenepeh avatar Apr 04 '17 11:04 eugenepeh

I feel that for this one a warning should suffice? Unless we are ok with putting suppressWarning or //CHECKSTYLE:OFF all around these method, which may make it a bit disorganized.

yamgent avatar Apr 04 '17 12:04 yamgent

We can start with a warning. Hopefully, we can fix the warnings incrementally until we can enforce the rule fully. Also, we can exempt getters and setters.

damithc avatar Apr 05 '17 09:04 damithc