[hibernate-dev] Checkstyle and ORM

[Emmanuel Bernard]

Note that custom checkstyle rule implementations does not fare well with IDE checkstyle rules enforcement. 
I can't make it reliably work on search and OGM. 

> On 16 mai 2015, at 16:11, Sanne Grinovero <sanne at hibernate.org> wrote:
> 
> One nice thing of checkstyle is that it's easily extended, for example
> in Search we added a couple of checks, like to ban double whitespaces:
> - https://github.com/hibernate/hibernate-search/blob/master/build-config/src/main/java/org/hibernate/checkstyle/checks/regexp/DoubleSpacesCheck.java
> 
> Or to ban specific import statements:
> - https://github.com/hibernate/hibernate-search/blob/master/build-config/src/main/java/org/hibernate/checkstyle/checks/regexp/IllegalImport.java
> 
> For example this is useful to iteratively move away from
> commons-annotations, like we ban its AssertionFailure to favor usage
> of a different AssertionFailure within Search:
> - https://github.com/hibernate/hibernate-search/blob/master/build-config/src/main/resources/checkstyle.xml#L172
> 
> But checkstyle isn't enough, I for one would love to ban auto-boxing
> from our main code (it's ok in tests) as I'd rather the syntax call
> explicit attention to it.
> Sometimes it's dangerous for performance reasons, or because of other
> subtle API compability reasons like the JBoss Logger upgrade.. but
> checktyle can't do such things, you need to add additional tools to
> the mix, like FindBugs.
> 
> Have a look at the above linked checkstyle.xml for more ideas, but
> sometimes I feel we've been too strict as it does fire back on quick
> prototyping: it's very annoying to have your build fail for a couple
> of style errors when you're deep in debugging some functionality..
> I'd rather stick to it though, as it can be disabled locally with
> appropriate flags and there finally are no more delays in pull request
> reviews because of whitespacing.
> 
> To make it slightly less annoying I've made a CheckStyle filter which
> allows us to enforce some selected rules only on non-test code:
> - https://github.com/hibernate/hibernate-search/blob/master/build-config/src/main/java/org/hibernate/checkstyle/filters/ExcludeTestPackages.java
> 
> We could similarly extend it to enforce a package-info.java for all
> non-test, non .impl packages?
> 
> One which I would strongly enforce is about having the right copyright
> headers. For example I've seen various hibernate-infinispan classes
> using a non-Hibernate template. The traditional header includes the
> year, which was sometimes problematic (like you'd copy an header in a
> new class and mark it with a 6 years old copyright); so now we're
> using a simplified copyright template which is shorter and has no
> mention years:
> - https://github.com/hibernate/hibernate-search/blob/master/engine/src/main/java/org/hibernate/search/analyzer/Discriminator.java#L1-L6
> (according to legal that's good enough)
> 
> -- Sanne
> 
> 
>> On 16 May 2015 at 00:19, Steve Ebersole <steve at hibernate.org> wrote:
>> FYI : https://hibernate.atlassian.net/browse/HHH-9803
>> 
>> Its not super high on my priority list, but I want to get to a point where
>> we can start to fail the build on "serious checkstyle regressions".  Part
>> of that is being more realistic with what is considered serious, and part
>> of that is fixing up code.
>> 
>> For example, we have quite a few warnings about spaces rather than tabs for
>> indentation.  That's a serious one to me.  There are quite a few "unused
>> import" warnings, again to me that's serious (it just looks sloppy).
>> 
>> A few I have already disabled.  Checking that there is a package-info.java
>> file for example.  I'd love to make a requirement that all API and SPI
>> packages have one.  But as far as I know that detailing is not available in
>> checkstyle, and I really dont overly care about package-info.java files for
>> internal packages.
>> 
>> These are just a few examples.  #909[1] (still running atm) is the first
>> build with my preliminary work here.  Let me know if there are any checks
>> anyone feels strongly about in one bucket or another.
>> 
>> 
>> [1] http://ci.hibernate.org/job/hibernate-orm-master-h2/909/
>> _______________________________________________
>> hibernate-dev mailing list
>> hibernate-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev