Checkstyle for Vaadin #9065

* Validates that all java files contains a license file and fails the build if a license is missing
* For now runs a lot of checks with a severity less than error to avoid failing the build
* Outputs test results to TeamCity for reporting

Change-Id: I24ca9cd0646a4bc530ef3a183eef1bc4addc555d

15 files changed, 371 insertions, 25 deletions

diff --git a/all/build.xml b/all/build.xml
index e352cbbc07..d5a484a902 100644
--- a/all/build.xml
+++ b/all/build.xml
@@ -104,8 +104,10 @@
 	<target name="clean">
 		<antcall target="common.clean" />
 	</target>
-
-	<target name="tests">
+	<target name="checkstyle">
+		<!-- Checkstyle is handled by all separate modules -->
+	</target>
+	<target name="tests" depends="checkstyle">
 		<!-- No tests for this zip.. -->
 	</target>
 
diff --git a/build.xml b/build.xml
index 50a4f4463f..306b169bc7 100644
--- a/build.xml
+++ b/build.xml
@@ -37,7 +37,9 @@
 		<delete dir="${ivy.cache.dir}/com.carrotsearch" />
 
 	</target>
-
+	<target name="checkstyle" depends="buildorder">
+		<subant buildpathref="build-path" target="checkstyle"/>
+	</target>
 	<target name="tests" depends="buildorder">
 		<property name="war.file" location="result/artifacts/${vaadin.version}/vaadin-uitest/vaadin-uitest-${vaadin.version}.war" />
 		<parallel>
diff --git a/buildhelpers/build.xml b/buildhelpers/build.xml
index d50e024ce4..e3e7b434d6 100644
--- a/buildhelpers/build.xml
+++ b/buildhelpers/build.xml
@@ -28,7 +28,13 @@
 		<antcall target="common.clean" />
 	</target>
 
-	<target name="tests">
+	<target name="checkstyle">
+		<antcall target="common.checkstyle">
+			<param name="cs.src" location="src" />
+		</antcall>
+	</target>
+
+	<target name="tests" depends="checkstyle">
 		<!--<antcall target="common.tests.run" />-->
 		<echo>WHAT? No JUnit tests for ${module.name}!</echo>
 	</target>
diff --git a/checkstyle/header b/checkstyle/header
new file mode 100644
index 0000000000..511f77e5ba
--- /dev/null
+++ b/checkstyle/header
@@ -0,0 +1,15 @@
+/*
+ * Copyright 2000-2013 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
\ No newline at end of file
diff --git a/checkstyle/vaadin-checkstyle.xml b/checkstyle/vaadin-checkstyle.xml
new file mode 100644
index 0000000000..3e23370a23
--- /dev/null
+++ b/checkstyle/vaadin-checkstyle.xml
@@ -0,0 +1,257 @@
+<?xml version="1.0"?>

+<!DOCTYPE module PUBLIC

+          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"

+          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

+

+<!-- Checkstyle configuration for Vaadin that is based on the the sun coding

+	conventions from:

+	- the Java Language Specification at http://java.sun.com/docs/books/jls/second_edition/html/index.html

+	- the Sun Code Conventions at http://java.sun.com/docs/codeconv/

+	- the Javadoc guidelines at http://java.sun.com/j2se/javadoc/writingdoccomments/index.html

+	- the JDK Api documentation http://java.sun.com/j2se/docs/api/index.html

+	- some best practices Vaadin specific changes:

+	- Removed DoubleCheckedLocking because it doesn't work in CheckStyle 5.6

+	-> http://www.smartics.eu/bugzilla/show_bug.cgi?id=593

+	- Modified HiddenField Check to allow field shadowing on Constructor, Setter

+	and Abstract Method parameters.

+	- Modified StaticVariableName Check format for PUBLIC variables from ^[a-z][a-zA-Z0-9]*$

+	to ^[A-Z_]*$ . Others (protected, package and private) still have ^[a-z][a-zA-Z0-9]*$

+	- Modified the severity of the following Checks from error to info:

+	• JavadocPackage (checks for package-info.java)

+	• JavadocType (class and interface declarations, scope private)

+	• JavadocMethod (method declarations, scope private)

+	• JavadocVariable (variable declarations, scope private)

+	• JavadocStyle (Javadocs are "well formed")

+	- Modified the severity of the following Checks from error to warning because

+	not so critical:

+	• LineLenght (the default value is 80 which is also used in formatter, but

+	i.e. member declarations are not wrapped onto next line)

+	• RedundantModifier (i.e. using public in interface method declarations)

+	• RedundantThrows (causes unnecessary fails when can't get class information)

+	• MethodLength (default maxLength is 150)

+	• ParameterNumber (default maxLength is 7)

+	• EmptyBlock (if-else statements have some blocks if x -> no action)

+	• UpperEll (should use L instead of l with long)

+	• TodoComment (not serious)

+	• WhitespaceAroundCheck (expects whitespace around some operators)

+	NOTE other checks are also warning but should be error.

+	- Modified the severity of the following Checks from error to ignore:

+	• FinalParameters (method parameters can be modified)

+	• VisibilityModifier (i.e. in state classes public members are allowed)

+	• DesignForExtension (this design is not used)

+	• FileLength (bad design to have files with over 2000 lines? see VScrollTable)

+	• MagicNumber (MagicNumbers like error codes are used, but could just ignore

+	this in some classes)

+	• AvoidInlineConditionals ( you like these ? ignore : error ) -->

+

+<module name="Checker">

+	<!-- If you set the basedir property below, then all reported file names

+		will be relative to the specified directory. See http://checkstyle.sourceforge.net/5.x/config.html#Checker

+		<property name="basedir" value="${basedir}"/> -->

+

+	<!-- Checks that a package-info.java file exists for each package. -->

+	<!-- See http://checkstyle.sf.net/config_javadoc.html#JavadocPackage -->

+	<module name="JavadocPackage">

+		<property name="severity" value="info" />

+	</module>

+

+	<!-- Checks whether files end with a new line. -->

+	<!-- See http://checkstyle.sf.net/config_misc.html#NewlineAtEndOfFile -->

+	<module name="NewlineAtEndOfFile">

+		<property name="severity" value="warning" />

+	</module>

+

+

+	<!-- Checks that property files contain the same keys. -->

+	<!-- See http://checkstyle.sf.net/config_misc.html#Translation -->

+	<module name="Translation">

+		<property name="severity" value="warning" />

+	</module>

+

+	<!-- Checks for Size Violations. -->

+	<!-- See http://checkstyle.sf.net/config_sizes.html -->

+	<module name="FileLength">

+		<property name="severity" value="ignore" />

+	</module>

+

+	<!-- Checks for whitespace -->

+	<!-- See http://checkstyle.sf.net/config_whitespace.html -->

+	<module name="FileTabCharacter">

+		<property name="severity" value="warning" />

+	</module>

+

+	<!-- Checks for Headers -->

+	<!-- See http://checkstyle.sf.net/config_header.html -->

+	<module name="Header">

+		<property name="headerFile" value="${checkstyle.header.file}" />

+		<property name="fileExtensions" value="java" />

+	</module>

+

+	<module name="TreeWalker">

+		<property name="severity" value="warning" />

+

+		<!-- Custom checks to check serializable, neither is working because checkstyle

+			can't load the classes. <module name="com.vaadin.checks.CheckClassesSerializable"/>

+			<module name="com.vaadin.checks.IsSerializableClassCheck"/> -->

+

+		<!-- Checks for Javadoc comments. -->

+		<!-- See http://checkstyle.sf.net/config_javadoc.html -->

+		<module name="JavadocMethod">

+			<property name="severity" value="info" />

+		</module>

+		<module name="JavadocType">

+			<property name="severity" value="info" />

+		</module>

+		<module name="JavadocVariable">

+			<property name="severity" value="info" />

+		</module>

+		<module name="JavadocStyle">

+			<property name="severity" value="info" />

+		</module>

+

+		<!-- Checks for Naming Conventions. -->

+		<!-- See http://checkstyle.sf.net/config_naming.html -->

+		<module name="ConstantName" />

+		<module name="LocalFinalVariableName" />

+		<module name="LocalVariableName" />

+		<module name="MemberName" />

+		<module name="MethodName" />

+		<module name="PackageName" />

+		<module name="ParameterName" />

+		<module name="StaticVariableName">

+			<property name="applyToPublic" value="false" />

+		</module>

+		<module name="StaticVariableName">

+			<property name="applyToPublic" value="true" />

+			<property name="applyToProtected" value="false" />

+			<property name="applyToPackage" value="false" />

+			<property name="applyToPrivate" value="false" />

+			<property name="format" value="^[A-Z_]*$" />

+		</module>

+		<module name="TypeName" />

+

+

+		<!-- Checks for imports -->

+		<!-- See http://checkstyle.sf.net/config_import.html -->

+		<module name="AvoidStarImport" />

+		<module name="IllegalImport" /> <!-- defaults to sun.* packages -->

+		<module name="RedundantImport" />

+		<module name="UnusedImports" />

+

+

+		<!-- Checks for Size Violations. -->

+		<!-- See http://checkstyle.sf.net/config_sizes.html -->

+		<module name="LineLength">

+			<property name="severity" value="warning" />

+		</module>

+		<module name="MethodLength">

+			<property name="severity" value="warning" />

+		</module>

+		<module name="ParameterNumber">

+			<property name="severity" value="warning" />

+		</module>

+

+

+		<!-- Checks for whitespace -->

+		<!-- See http://checkstyle.sf.net/config_whitespace.html -->

+		<module name="EmptyForIteratorPad" />

+		<module name="GenericWhitespace" />

+		<module name="MethodParamPad" />

+		<module name="NoWhitespaceAfter" />

+		<module name="NoWhitespaceBefore" />

+		<module name="OperatorWrap" />

+		<module name="ParenPad" />

+		<module name="TypecastParenPad" />

+		<module name="WhitespaceAfter" />

+		<module name="WhitespaceAround" />

+		<!-- Check for trailing white space in Java code -->

+		<module name="RegexpSinglelineJava">

+			<!-- Ensure no whitespace at the end of line, excluding comments -->

+			<property name="format" value="\s+$" />

+			<property name="minimum" value="0" />

+			<property name="maximum" value="0" />

+			<property name="message" value="Java code has trailing white space." />

+			<property name="severity" value="warning" />

+			<property name="ignoreComments" value="true" />

+		</module>

+

+		<!-- Modifier Checks -->

+		<!-- See http://checkstyle.sf.net/config_modifiers.html -->

+		<module name="ModifierOrder" />

+		<module name="RedundantModifier">

+			<property name="severity" value="warning" />

+		</module>

+

+

+		<!-- Checks for blocks. You know, those {}'s -->

+		<!-- See http://checkstyle.sf.net/config_blocks.html -->

+		<module name="AvoidNestedBlocks" />

+		<module name="EmptyBlock">

+			<property name="severity" value="warning" />

+		</module>

+		<module name="LeftCurly" />

+		<module name="NeedBraces" />

+		<module name="RightCurly" />

+

+

+		<!-- Checks for common coding problems -->

+		<!-- See http://checkstyle.sf.net/config_coding.html -->

+		<module name="AvoidInlineConditionals">

+			<property name="severity" value="ignore" />

+		</module>

+		<module name="EmptyStatement" />

+		<module name="EqualsHashCode" />

+		<module name="HiddenField">

+			<property name="ignoreConstructorParameter" value="true" />

+			<property name="ignoreSetter" value="true" />

+			<property name="ignoreAbstractMethods" value="true" />

+		</module>

+		<module name="IllegalInstantiation" />

+		<module name="InnerAssignment" />

+		<module name="MagicNumber">

+			<property name="severity" value="ignore" />

+		</module>

+		<module name="MissingSwitchDefault" />

+		<module name="RedundantThrows">

+			<property name="severity" value="warning" />

+		</module>

+		<module name="SimplifyBooleanExpression" />

+		<module name="SimplifyBooleanReturn" />

+

+		<!-- Checks for class design -->

+		<!-- See http://checkstyle.sf.net/config_design.html -->

+		<module name="DesignForExtension">

+			<property name="severity" value="ignore" />

+		</module>

+		<module name="FinalClass" />

+		<module name="HideUtilityClassConstructor" />

+		<module name="InterfaceIsType" />

+		<module name="VisibilityModifier">

+			<property name="severity" value="ignore" />

+		</module>

+

+

+		<!-- Miscellaneous other checks. -->

+		<!-- See http://checkstyle.sf.net/config_misc.html -->

+		<module name="ArrayTypeStyle" />

+		<module name="FinalParameters">

+			<property name="severity" value="ignore" />

+		</module>

+		<module name="TodoComment">

+			<property name="severity" value="warning" />

+		</module>

+		<module name="UpperEll">

+			<property name="severity" value="warning" />

+		</module>

+		

+		<!--  Check for System.err/out.println -->

+        <module name="RegexpSinglelineJava">

+            <property name="format" value="System\.out\.println"/>

+        </module>

+        <module name="RegexpSinglelineJava">

+            <property name="format" value="System\.err\.println"/>

+        </module>

+

+	</module>

+

+</module>

diff --git a/client-compiled/build.xml b/client-compiled/build.xml
index d9f3da6271..d1f3eda674 100644
--- a/client-compiled/build.xml
+++ b/client-compiled/build.xml
@@ -124,7 +124,10 @@
 		<antcall target="common.clean" />
 	</target>
 
-	<target name="tests">
+	<target name="checkstyle">
+		<echo>No java files in module</echo>
+	</target>
+	<target name="tests" depends="checkstyle">
 		<!--<antcall target="common.tests.run" />-->
 		<echo>WHAT? No tests for ${module.name}!</echo>
 	</target>
diff --git a/client-compiler/build.xml b/client-compiler/build.xml
index 95544708c1..9d4e5c403c 100644
--- a/client-compiler/build.xml
+++ b/client-compiler/build.xml
@@ -52,7 +52,13 @@ gwt.svnrev=${git.revision}</echo>
 		<antcall target="common.clean" />
 	</target>
 
-	<target name="tests">
+	<target name="checkstyle">
+		<antcall target="common.checkstyle">
+			<param name="cs.src" location="src" />
+		</antcall>
+	</target>
+
+	<target name="tests" depends="checkstyle">
 		<!--<antcall target="common.tests.run" />-->
 		<echo>WHAT? No tests for ${module.name}!</echo>
 	</target>
diff --git a/client/build.xml b/client/build.xml
index 26f92ee33f..d2946be6e0 100644
--- a/client/build.xml
+++ b/client/build.xml
@@ -61,7 +61,13 @@
 		<antcall target="common.clean" />
 	</target>
 
-	<target name="tests">
+	<target name="checkstyle">
+		<antcall target="common.checkstyle">
+			<param name="cs.src" location="src" />
+		</antcall>
+	</target>
+
+	<target name="tests" depends="checkstyle">
 		<antcall target="common.tests.run" />
 	</target>
 
diff --git a/common.xml b/common.xml
index 8df71cc177..e96037032b 100644
--- a/common.xml
+++ b/common.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<project name="common" basedir="." default="" xmlns:ivy="antlib:org.apache.ivy.ant" xmlns:antcontrib="antlib:net.sf.antcontrib">
+<project name="common" basedir="." default="" xmlns:ivy="antlib:org.apache.ivy.ant" xmlns:antcontrib="antlib:net.sf.antcontrib" xmlns:cs="antlib:com.puppycrawl.tools.checkstyle">
 
 	<tstamp>
 		<format property="build.date" pattern="yyyy-MM-dd" />
@@ -350,4 +350,23 @@
 		<ivy:publish conf="${conf}" resolver="local-maven" overwrite="true">
 		</ivy:publish>
 	</target>
+
+	<!-- Checkstyle conf -->
+	<property name="cs.dir" location="${vaadin.basedir}/checkstyle" />
+	<property name="cs.xml" location="${cs.dir}/vaadin-checkstyle.xml" />
+	<property name="cs.header-file" location="${cs.dir}/header" />
+	<taskdef resource="checkstyletask.properties" uri="antlib:com.puppycrawl.tools.checkstyle" classpathref="taskdefs.classpath" />
+
+	<target name="checkstyle">
+		<fail unless="result.dir" message="No result.dir parameter given" />
+		<fail unless="cs.src" message="No cs.src parameter given" />
+		<property name="result.dir.full" location="${result.dir}"/>
+		<mkdir dir="${result.dir}" />
+		<echo>##teamcity[importData type='checkstyle' path='${result.dir.full}/checkstyle-errors.xml']</echo>
+		<cs:checkstyle config="${cs.xml}" failOnViolation="true">
+			<fileset dir="${cs.src}" includes="**/*.java" />
+			<formatter type="xml" toFile="${result.dir}/checkstyle-errors.xml"/>
+			<property key="checkstyle.header.file" file="${cs.header-file}" />
+		</cs:checkstyle>
+	</target>
 </project>
diff --git a/ivy-taskdefs.xml b/ivy-taskdefs.xml
index ccc4d75bcf..64d5ec1be1 100644
--- a/ivy-taskdefs.xml
+++ b/ivy-taskdefs.xml
@@ -1,18 +1,24 @@
 
-<ivy-module version="2.0" 
+<ivy-module version="2.0"
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-	xsi:noNamespaceSchemaLocation= "http://ant.apache.org/ivy/schemas/ivy.xsd">
-    
-    <info organisation="com.vaadin" module="vaadin"/>
-    <configurations>
-   		<conf name="taskdefs" description="Ant task definitions" visibility="private" />
+	xsi:noNamespaceSchemaLocation="http://ant.apache.org/ivy/schemas/ivy.xsd">
+
+	<info organisation="com.vaadin" module="vaadin" />
+	<configurations>
+		<conf name="taskdefs" description="Ant task definitions"
+			visibility="private" />
 	</configurations>
-    <publications />        
+	<publications />
 	<dependencies>
-   		<!-- Ant tasks -->   	
-   		<dependency org="ant-contrib" name="ant-contrib" rev="1.0b3" conf="taskdefs ->master"/>
-   		<dependency org="org.apache.maven" name="maven-ant-tasks" rev="2.0.10" conf="taskdefs ->master"/>
-   		<dependency org="com.googlecode.jarjar" name="jarjar" rev="1.3" conf="taskdefs ->master"/>
+		<!-- Ant tasks -->
+		<dependency org="ant-contrib" name="ant-contrib" rev="1.0b3"
+			conf="taskdefs ->master" />
+		<dependency org="org.apache.maven" name="maven-ant-tasks"
+			rev="2.0.10" conf="taskdefs ->master" />
+		<dependency org="com.googlecode.jarjar" name="jarjar" rev="1.3"
+			conf="taskdefs ->master" />
+		<dependency org="com.puppycrawl.tools" name="checkstyle"
+			rev="5.6" />
 	</dependencies>
 
 </ivy-module>
diff --git a/server/build.xml b/server/build.xml
index 6c61b7dc75..253d09a45b 100644
--- a/server/build.xml
+++ b/server/build.xml
@@ -43,7 +43,13 @@
 		<antcall target="common.clean" />
 	</target>
 
-	<target name="tests">
+	<target name="checkstyle">
+		<antcall target="common.checkstyle">
+			<param name="cs.src" location="src" />
+		</antcall>
+	</target>
+
+	<target name="tests" depends="checkstyle">
 		<antcall target="common.tests.run" />
 	</target>
 
diff --git a/shared/build.xml b/shared/build.xml
index 4cba76c85a..06a27b0ef9 100644
--- a/shared/build.xml
+++ b/shared/build.xml
@@ -47,8 +47,14 @@
 	<target name="clean">
 		<antcall target="common.clean" />
 	</target>
+	
+	<target name="checkstyle">
+		<antcall target="common.checkstyle">
+			<param name="cs.src" location="src" />
+		</antcall>
+	</target>
 
-	<target name="tests">
+	<target name="tests" depends="checkstyle">
 		<!--<antcall target="common.tests.run" />-->
 		<echo>WHAT? No tests for ${module.name}!</echo>
 	</target>
diff --git a/theme-compiler/build.xml b/theme-compiler/build.xml
index 1f9cddebd7..885cb6e589 100644
--- a/theme-compiler/build.xml
+++ b/theme-compiler/build.xml
@@ -43,7 +43,13 @@
 		<antcall target="common.clean" />
 	</target>
 
-	<target name="tests">
+	<target name="checkstyle">
+		<antcall target="common.checkstyle">
+			<param name="cs.src" location="src" />
+		</antcall>
+	</target>
+
+	<target name="tests" depends="checkstyle">
 		<antcall target="common.tests.run" />
 	</target>
 
diff --git a/themes/build.xml b/themes/build.xml
index a504d585dd..6e9e3c9951 100644
--- a/themes/build.xml
+++ b/themes/build.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
 
-<project name="vaadin-client-compiler" basedir="." default="publish-local" xmlns:ivy="antlib:org.apache.ivy.ant">
+<project name="vaadin-themes" basedir="." default="publish-local" xmlns:ivy="antlib:org.apache.ivy.ant">
 	<description>
 		Themes compiled to CSS 
 	</description>
@@ -96,7 +96,10 @@
 		<antcall target="common.clean" />
 	</target>
 
-	<target name="tests">
+	<target name="checkstyle">
+		<echo>No java files in module</echo>
+	</target>
+	<target name="tests" depends="checkstyle">
 		<!--<antcall target="common.tests.run" />-->
 		<echo>WHAT? No tests for ${module.name}!</echo>
 	</target>
diff --git a/uitest/build.xml b/uitest/build.xml
index 8d5c1b28f9..ee2b2c6124 100644
--- a/uitest/build.xml
+++ b/uitest/build.xml
@@ -133,7 +133,10 @@
 	<target name="clean">
 		<antcall target="common.clean" />
 	</target>
-	<target name="tests">
+	<target name="checkstyle">
+		<echo>Checkstyle is disabled for uitest for now</echo>
+	</target>
+	<target name="tests" depends="checkstyle">
 		<!--<antcall target="common.tests.run" />-->
 		<echo>WHAT? No JUnit tests for ${module.name}!</echo>
 	</target>