Checkstyle and Java. Will automatic code inspection help?

Recently, articles on automation and optimization of code-review, code inspections and other ways of keeping the code of large projects in decent form (for example, article 1 , article 2 , article 3 ) have noticeably become more frequent on Habré.

I think regular readers have already met previous posts ( first , second ), where my colleagues talked about using the Checkstyle java library and shared our joint experience on its expansion.
')
The latest work of our team on writing additional checks (checks) for Checkstyle is included in the new assembly of our add-on for Eclipse Checkstyle-plugin Eclipse-cs ( sevntu-checkstyle 1.5.3 , 4.09.2012).

We continue experiments to automate the search for errors at the stage of writing Java code. We made sure that it is easy to write your own checks for Checkstyle - or rather, quite simply! It’s harder to write a check that is truly unique and useful in development (and even harder is to agree later on with all members of the development team about sharing a new check). In this post I will try to describe the latest achievements of our team in the direction of developing new checks for Checkstyle. Also below, I would like to highlight a bit the principle of operation, usefulness and limitations of the Checkstyle library, the Eclipse Checkstyle-plugin and our addition to it.

Development requires a lot of effort and attention from the programmer. Time does not stand still: customer's wishes change, projects get fat, and the program code is safely forgotten by the developer and becomes obsolete with time. We have to work together to structure the code and try to always keep it in a decent condition. All possible means are used for this: code review, refactoring, measurement of the “characteristics” of the code in parrots by metrics, etc. Such checks and reorganizations of the code take a lot of time and nerve cells from developers. On the other hand, there is a positive point: the presence of regular code-review in the company of developers leads to the emergence and consolidation of general rules for writing the "good" and "correct" code. Most of these rules then become the standards generally accepted in the company and often the first thing that new employees are taught in the company is corporate rules and standards for writing code.

A lot of books have been written on the subject of writing “good” and “safe” code, for example, Joshua Bloch's “Effective Java” and “Java Puzzlers”. These books contain many years of accumulated experience of “correct” programming, and most of the tips from these books are also worthy of becoming corporate standards.

But no matter how universal and good the standards and rules of “competent” programming, it is practically impossible to study and remember them all and keep them in memory constantly, and, most importantly, do not forget to constantly apply them in practice. Moreover, even knowing by heart all the rules for writing "good" code, the programmer always has the right to make mistakes (or his own vision of the rules-standards). A novice developer can write "bad" or unsafe code simply out of ignorance, and an experienced programmer can easily forget about the standards, hurrying due to lack of time.

Non-observance of standards and "experience of ancestors" can manage various consequences. If the rules relating to the style of the code are not followed, the code simply becomes harder to understand. This is fixable, but in most cases the developer simply does not have the desire to return to the long-written code and refactor it in accordance with all standards. The ancient principles of programming like “Why, if everything works fine” and “Who doesn’t like it, fix it” =) work) The errors that remain in the code are sometimes difficult to detect, especially in large projects. Here, the developer must come to the aid of the Checkstyle library, which indicates errors and inconsistencies with pre-configured programming standards, reflecting not only the place, but also the cause of the problem in each problematic section of the code.

Basically, Checkstyle is used to help the developer follow the code style and identify potentially dangerous situations in it. Checkstyle can also be used to force , in a soft (or not very soft) form, to prompt the Java developer how to write code that conforms to certain standards and rules. Almost everything can be referred to as “standards and rules”: from the method of placing brackets and indents to potentially dangerous situations. Moreover, the set of “problem” areas and situations checked by Checkstyle corresponds to the long-established and generally accepted rules of “good form” in Java - programming.

Having found errors / style problems in the code, Checkstyle may behave differently, depending on the settings. With the “soft” settings, the library will only warn about errors present, and with the strictest, it will fail the build until all the Checkstyle warnings in the project are corrected.

Checkstyle comes with a couple of already established configurations, one of which is set up in accordance with the Sun Code Conventions. This makes it convenient to use the out-of-box Checkstyle However, for anyone who is seriously and for a long time going to use Checkstyle, I would advise you to create your own configuration and, including setting up checks one by one, gradually come to the appropriate settings. This will help to get acquainted with the library's capabilities and to be sure that each included check is really useful for you. In addition, if you are a novice Java developer, setting up Checkstyle from scratch may well be a good experience for you to learn the “good taste” of Java programming.

It is also possible to add your own checks to Checkstyle for specific needs. Our team has long been engaged in writing checks and adding them to Checkstyle, with many of the ready-made checks we constantly use in development. Example: in our company it is considered a bad tone to create a java.lang.NullpointerException object. In order to make sure that NPE is not instantiated in any project, we added our own check to the Eclipse Checkstyle plug-in test suite, which prohibits creating objects of some specified class ( ForbidInstantiationCheck ) and configured it so that the check “swears” only NPE instantiation.

It is very helpful in writing your own checks and improving Checkstyle that the code of the Checkstyle engine and the checks themselves are written qualitatively and understandably (not a single check is added to the library officially until Checkstyle itself passes with very “fascist” settings ). There is a simple opportunity to extend Checkstyle capabilities by adding your own checks written in Java or modifying existing ones. It is easy to write your own check and add it to Checkstyle. To do this, write the code of the new check, following the instructions on the Checkstyle project site and our wiki and add it to the latest Checkstyle assembly using our add-on . The result will be a reassembled Checkstyle jar archive, which can be fed to the Checkstyle plug-in for the IDE, Maven Checkstyle plug-in, etc. Among existing checks, it is easy to find a good example for writing your own, so even a novice programmer without much experience with Java can write a check for Checkstyle.

Like any software product, Checkstyle has its pros and cons. The advantages are simple: convenience and flexibility in customization, support for a large number of different rules (checks), the ability to write your own checks and ease of embedding them into the library.

I believe that the main drawback of the library is that all Checkstyle checks cannot go beyond the limits of a single Java file. Also during the checks, Checkstyle omits all comments, but this, in my opinion, is not fatal.

Checking Java code is always performed by Checktyle as follows:

1. Based on the LL grammars used in Checkstyle ANTLR engine, a Java code parser is generated that conforms to the specification of the Java version used.
2. Using this parser, the Java code is represented as abstract parse trees (Abstract Synthax Tree or AST) - one tree per Java file. Each received syntactic tree has approximately the following structure:
   
3. Each vertex of each tree is processed by all included checks, in the code of which the corresponding vertex type is indicated as input data. The processing order of the vertices of the syntacical tree corresponds to the real order of their following when reading the code: from left to right and from top to bottom.
4. For all errors found, the corresponding warnings are generated.

A little bit about our supplement

During the two-year practice, our team of several people became well acquainted with the capabilities of Checkstyle and expanded the number of checks that exist in Checkstyle. Most of them we brought in a separate project called "SevNTU-Checkstyle". (For reference: SevNTU - Sevastopol National Technical University, most of our team are former or current students of this university, supervised by Java developers of the Sevastopol office of the company “Reveredata LLC”. The essence of our project is that it allows you to add a set of checks Checkstyle new checks (and test their work in real time on real projects) without having to send each check to the owners of the project Checkstyle, expecting it to be accepted into the official set of checks As we have seen over time, the founders of the Checkstyle project are very busy people and, moreover, they don’t share some quality code ideas with us (as they say, each monastery has its own charter). Therefore, in most cases they don’t respond to new proposals and fixes in their own patch tracker , and sometimes even qualitatively tested checks in Checkstyle do not add.In general, now for those who want to expand the capabilities of Checkstyle, or change its checks to fit their needs, this is no longer a hindrance.

About a year ago (11/09/11) our project became part of the Eclipse Checkstyle plug-in, becoming an “extension” for Eclipse-cs . Installing it, like any other add-ons for Eclipse, can be easily done via update-site. The installation process consists of two simple points:

1. Open Eclipse.
2. We drive in the Help -> “Install New Software” URL of our update site: eclipse-cs.sourceforge.net/update/ and put both daws, i.e. - we choose to install the Checkstyle-plugin itself and our addition to it:

In addition, you can install our add-on (and Eclipse-cs itself) through the Eclipse Marketplace , without manually inserting links.

After restarting Eclipse and creating a new Checkstyle configuration (Window -> Preferences -> Checkstyle -> New ...) a group with 25 of our additional checks will appear in the Checkstyle settings:


Most of our 25 checks, included in the supplement, do not look at the “code style” checks, but look for potential dangers in it (Although, of course, this is true only when the problem can be clearly identified using Checkstyle: without bytecode analysis and checking simultaneously multiple java files).

A brief description of each check in the appendix is ​​on our page , so I would like to dwell in more detail only on some of them, more precisely, those whose code I wrote or edited personally:

ChildBlockLengthCheck - a check that checks the percentage of the size of nested statements (compares the number of lines occupied by the body of statements). This check displays a warning if the body size of the nested statement is more than N% of the size of the statement lying a higher level. A check displays warnings only if the nested statement occupies more than m lines. This is useful when you need to find blocks that do not fit completely on the screen and take up too many lines of the parent block.

AvoidModifiersForTypesCheck - allows you to prohibit the use of one or more of the 4 types of keywords: static, final, transient, volatile for objects of specified types. As parameters, this check for each of the 4 keywords allows you to set 4 separate regular expressions that define the format of the name of the type you are looking for.

Example of using AvoidModifiersForTypesCheck: ULC graphics library objects (ULCComponents) should not be static: link . Therefore, using Checkstyle, it is very reasonable to prohibit the assignment of the keyword “static” to all objects whose class name begins, for example, with “ULC”.

CauseParameterInExceptionCheck - allows you to force the developer to specify Exception or Throwable as a parameter of at least one constructor of any class that represents its own exception (constructor).

Classes representing their own exceptions are checked by class name (using a regular expression that is specified as an input parameter).

Also in the check there is the possibility of ignoring some classes by their name - again using a regular expression.

ForbidsCertainImportsCheck - a check that prohibits importing certain packages / types by a regular expression (this check is similar to the corresponding Sonar check).

ForbidInstantiationCheck - a check that prohibits instantiation of objects of a given type. Example of use: suppose we need to prohibit the creation of a java.lang.NullPointerException object at the level of the whole project (in our company, the creation of an NPE object is considered a bad tone). As parameters, the check accepts a comma-separated list of qualified-paths to classes that are prohibited from instantiation:

«java.lang.NullPointerException, your.custom.Class, etc.»

ReturnCountExtendedCheck is a check that checks the number of return keywords in methods and constructors. The check has flexible ignoring settings, it can be used not only to prohibit writing methods with more than n “return” operators. This check can "count" only those of the operators "return", which:

• do not lie within the first m lines of the method / constructor (such “return” operators often serve to “quickly” exit the construction when checking the method parameters to null, or the expected output value, or ...
• lie on the nesting level of operators greater than k (such “return” s can serve to exit from complex structures and cannot be counted)
• not in small size methods. If a method fits on the screen, then it is often quite acceptable for it to use a large number of “return” statements without losing the readability of the code.

This check also allows you to ignore some of the methods by their names using a regular expression.

AbbreviationAsWordInNameCheck - a check that checks class names for the number of consecutive capital letters. Those. - this check is intended to prohibit the naming of Java classes like “MDCContextListener”, suggesting that the name be changed to “MdcContextListener”. Such a check is needed not only to standardize the class names. It is useful that with the help of input parameters you can set a check for the names of those abbreviations which will be exceptions.

OverridableMethodInCtorCheck — A check that prohibits calling overridden methods from constructors (a check similar to that on NetBeans). As you know, in Java, calling an overridable method from a constructor can cause non-obvious program behavior (the result of a method when calling such a constructor may differ from what was expected) (for an example, look at this discussion of a problem on StackOverflow). Also calling an unredefined method that calls another, this time already redefined method - in this case, the “strange” behavior of the program can also start.Therefore, the redefinition is considered a “good tone” in the Java code methods called from constructors (as well as from the clone () and readObject () methods, which work as constructors). For this, non-static methods called from constructors (as well as from clone () and readObject ()) are assigned the keyword final or scope modifier private.

As we have seen in the process of writing checks, there are quite a few situations in Java code that are difficult or even impossible to verify without directly analyzing bytecode while simultaneously opening several files in memory. Checkstyle cannot do this, so I can say that our OverridableMethodInCtorCheck is far from perfect (mainly due to Checkstyle restrictions). In general, I would advise you to look for a quality check on the call of redefined methods from the constructor not in Checkstyle, but in PMD or FindBugs, which can work with the compiled byte-code.

PS I hope you find the Eclipse Checkstyle plugin useful and our addition to it. The set of checks written by us is well covered with tests and tested by us on our own projects. However, if you find a bug or have ideas for new checks, feel free to create an issue in our github repositories , we will try to consider them as quickly as possible.

That's all, thank you for your attention)