Repository: mockito/mockito
Issue: Review usage of linter during build
Labels: refactoring

Given the amount of stylistic changes lately in primarily @bric3 PRs, I think we should consider installing a linter to make sure the style is enforced project wide. In my opinion this linter should be extremely lenient, just enforce some very simple rules.

We can look into configuring Checkstyle such that it checks a handful of properties. We can always later decide to make the linter more strict.

Issue comments:
I have to push the code style decided a few months ago, maybe gradle can generate a section with the mockito style for IDEA (and eclipse and netbeans)
Absolutely great idea. Perhaps it's worth waiting for Mockito3 before we do that to avoid merging problems?

Pull request review comments:
Don't we also need the dev dependency for checkstyle?
Not sure what you mean by this. Could you please elaborate?

@TimvdLippe, we don't need any extra dependency. checkstyle plugin is a part of Gradle distribution. Good question!
@mureinik, do we need separate checker configurations per submodule? I'd rather us have a single consistent checkstyle rule for all java code.
Can you write a bit of xml comment here that we added the validation to make the git blame / git diff workflows easier? I suspect this rule can be annoying to engineers when the builds start failing (including me ;). It's nice to see the reminder in the code why we decided to apply those rules.
@szczepiq  I tried to reduce the scope of the change as much as possible, but you're right - this is a bad idea, and we should use one conf file for the entire project. I'll update the PR.
@szczepiq will do, thanks for the idea.
Is it necessary to have a stylesheet ?
It makes searching for errors a lot easier. Else it would be thousands of rows with 0 information
Yup, what @TimvdLippe said. Actually, he authored this style sheet and asked me to include it in the patch.

Patch that resolved the issue:
diff --git a/build.gradle b/build.gradle
index 2f85ad0e54..f5ab5b4fbd 100644
--- a/build.gradle
+++ b/build.gradle
@@ -35,6 +35,10 @@ allprojects {
         options.warnings = false
         options.encoding = 'UTF-8'
     }
+    apply plugin: 'checkstyle'
+    checkstyle {
+       configFile = rootProject.file('config/checkstyle/checkstyle.xml')
+    }
 }
 
 configurations {
diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml
new file mode 100644
index 0000000000..fe1ca7645e
--- /dev/null
+++ b/config/checkstyle/checkstyle.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0"?>
+<!DOCTYPE module PUBLIC
+    "-//Puppy Crawl//DTD Check Configuration 1.2//EN"
+    "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
+<module name="Checker">
+    <!--
+     Different editors and IDEs may insert or remove trailing
+     whitespaces at the end of source lines or newline characyers
+     at the end of source files.
+
+     This changes have no actual meaning, but may cause visual spam
+     when [re]viewing diffs.
+     Having checkstyles rules to strictly enforce the defacto
+     standards of the project helps avoid such issues and make the
+     review process more efficient.
+
+     See also https://github.com/mockito/mockito/issues/903
+     -->
+    <module name="NewlineAtEndOfFile"/>
+    <module name="RegexpSingleline">
+        <!-- \s matches whitespace character, $ matches end of line. -->
+        <property name="format" value="\s+$"/>
+        <property name="message" value="Line has trailing spaces."/>
+    </module>
+</module>
+
diff --git a/config/checkstyle/checkstyle.xsl b/config/checkstyle/checkstyle.xsl
new file mode 100644
index 0000000000..c9337f4d95
--- /dev/null
+++ b/config/checkstyle/checkstyle.xsl
@@ -0,0 +1,207 @@
+<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0">
+	<xsl:output method="html" indent="yes"/>
+	<xsl:decimal-format decimal-separator="." grouping-separator="," />
+
+	<xsl:key name="files" match="file" use="@name" />
+
+	<!-- Checkstyle XML Style Sheet by Rolf Wojtech <rolf@wojtech.de>                   -->
+	<!-- (based on checkstyle-noframe-sorted.xsl by Stephane Bailliez                   -->
+	<!--  <sbailliez@apache.org> and sf-patch 1721291 by Leo Liang)                     -->
+	<!-- Changes: 																								                      -->
+	<!--  * Outputs seperate columns for error/warning/info                             -->
+	<!--  * Sorts primarily by #error, secondarily by #warning, tertiary by #info       -->
+	<!--  * Compatible with windows path names (converts '\' to '/' for html anchor)    -->
+	<!--                                                                                -->
+	<!-- Part of the Checkstyle distribution found at http://checkstyle.sourceforge.net -->
+	<!-- Usage (generates checkstyle_report.html):                                      -->
+	<!--    <checkstyle failonviolation="false" config="${check.config}">               -->
+	<!--      <fileset dir="${src.dir}" includes="**/*.java"/>                          -->
+	<!--      <formatter type="xml" toFile="${doc.dir}/checkstyle_report.xml"/>         -->
+	<!--    </checkstyle>                                                               -->
+	<!--    <style basedir="${doc.dir}" destdir="${doc.dir}"                            -->
+	<!--            includes="checkstyle_report.xml"                                    -->
+	<!--            style="${doc.dir}/checkstyle-noframes-severity-sorted.xsl"/>        -->
+
+	<xsl:template match="checkstyle">
+		<html>
+			<head>
+				<style type="text/css">
+					.bannercell {
+					border: 0px;
+					padding: 0px;
+					}
+					body {
+					margin-left: 10;
+					margin-right: 10;
+					font:normal 80% arial,helvetica,sanserif;
+					background-color:#FFFFFF;
+					color:#000000;
+					}
+					.a td {
+					background: #efefef;
+					}
+					.b td {
+					background: #fff;
+					}
+					th, td {
+					text-align: left;
+					vertical-align: top;
+					}
+					th {
+					font-weight:bold;
+					background: #ccc;
+					color: black;
+					}
+					table, th, td {
+					font-size:100%;
+					border: none
+					}
+					table.log tr td, tr th {
+
+					}
+					h2 {
+					font-weight:bold;
+					font-size:140%;
+					margin-bottom: 5;
+					}
+					h3 {
+					font-size:100%;
+					font-weight:bold;
+					background: #525D76;
+					color: white;
+					text-decoration: none;
+					padding: 5px;
+					margin-right: 2px;
+					margin-left: 2px;
+					margin-bottom: 0;
+					}
+				</style>
+			</head>
+			<body>
+				<a name="top"></a>
+				<!-- jakarta logo -->
+				<table border="0" cellpadding="0" cellspacing="0" width="100%">
+					<tr>
+						<td class="bannercell" rowspan="2">
+							<!--a href="http://jakarta.apache.org/">
+							<img src="http://jakarta.apache.org/images/jakarta-logo.gif" alt="http://jakarta.apache.org" align="left" border="0"/>
+							</a-->
+						</td>
+						<td class="text-align:right"><h2>CheckStyle Audit</h2></td>
+					</tr>
+					<tr>
+						<td class="text-align:right">Designed for use with <a href='http://checkstyle.sourceforge.net/'>CheckStyle</a> and <a href='http://jakarta.apache.org'>Ant</a>.</td>
+					</tr>
+				</table>
+				<hr size="1"/>
+
+				<!-- Summary part -->
+				<xsl:apply-templates select="." mode="summary"/>
+				<hr size="1" width="100%" align="left"/>
+
+				<xsl:if test="count(descendant::file[error]) + count(descendant::file[warning]) + count(descendant::file[info]) > 0">
+					<!-- Package List part -->
+					<xsl:apply-templates select="." mode="filelist"/>
+					<hr size="1" width="100%" align="left"/>
+
+					<!-- For each package create its part -->
+					<xsl:apply-templates select="file[@name and generate-id(.) = generate-id(key('files', @name))][count(error) + count(warning) + count(info) > 0]" />
+					<hr size="1" width="100%" align="left"/>
+				</xsl:if>
+			</body>
+		</html>
+	</xsl:template>
+
+
+
+	<xsl:template match="checkstyle" mode="filelist">
+		<h3>Files</h3>
+		<table class="log" border="0" cellpadding="5" cellspacing="2" width="100%">
+			<tr>
+				<th>Name</th>
+				<th>Errors</th>
+				<th>Warnings</th>
+				<th>Infos</th>
+			</tr>
+			<xsl:for-each select="file[@name and generate-id(.) = generate-id(key('files', @name))][count(error) + count(warning) + count(info) > 0]">
+
+				<!-- Sort method 1: Primary by #error, secondary by #warning, tertiary by #info -->
+				<xsl:sort data-type="number" order="descending" select="count(key('files', @name)/error[@severity='error'])"/>
+				<xsl:sort data-type="number" order="descending" select="count(key('files', @name)/error[@severity='warning'])"/>
+				<xsl:sort data-type="number" order="descending" select="count(key('files', @name)/error[@severity='info'])"/>
+
+				<!-- Sort method 1: Sum(#error+#info+#warning) (uncomment to use, comment method 1)  -->
+				<!--
+				<xsl:sort data-type="number" order="descending" select="count(key('files', @name)/error)"/>
+				-->
+
+				<xsl:variable name="errorCount" select="count(key('files', @name)/error[@severity='error'])"/>
+				<xsl:variable name="warningCount" select="count(key('files', @name)/error[@severity='warning'])"/>
+				<xsl:variable name="infoCount" select="count(key('files', @name)/error[@severity='info'])"/>
+
+				<tr>
+					<xsl:call-template name="alternated-row"/>
+					<td><a href="#f-{translate(@name,'\','/')}"><xsl:value-of select="@name"/></a></td>
+					<td><xsl:value-of select="$errorCount"/></td>
+					<td><xsl:value-of select="$warningCount"/></td>
+					<td><xsl:value-of select="$infoCount"/></td>
+				</tr>
+			</xsl:for-each>
+		</table>
+	</xsl:template>
+
+
+	<xsl:template match="file">
+		<a name="f-{translate(@name,'\','/')}"></a>
+		<h3>File <xsl:value-of select="@name"/></h3>
+
+		<table class="log" border="0" cellpadding="5" cellspacing="2" width="100%">
+			<tr>
+				<th>Severity</th>
+				<th>Error Description</th>
+				<th>Line</th>
+			</tr>
+			<xsl:for-each select="key('files', @name)/error">
+				<xsl:sort data-type="number" order="ascending" select="@line"/>
+				<tr>
+					<xsl:call-template name="alternated-row"/>
+					<td><xsl:value-of select="@severity"/></td>
+					<td><xsl:value-of select="@message"/></td>
+					<td><xsl:value-of select="@line"/></td>
+				</tr>
+			</xsl:for-each>
+		</table>
+		<a href="#top">Back to top</a>
+	</xsl:template>
+
+
+	<xsl:template match="checkstyle" mode="summary">
+		<h3>Summary</h3>
+		<xsl:variable name="fileCount" select="count(file[@name and generate-id(.) = generate-id(key('files', @name))])"/>
+		<xsl:variable name="errorCount" select="count(file/error[@severity='error'])"/>
+		<xsl:variable name="warningCount" select="count(file/error[@severity='warning'])"/>
+		<xsl:variable name="infoCount" select="count(file/error[@severity='info'])"/>
+		<table class="log" border="0" cellpadding="5" cellspacing="2" width="100%">
+			<tr>
+				<th>Files</th>
+				<th>Errors</th>
+				<th>Warnings</th>
+				<th>Infos</th>
+			</tr>
+			<tr>
+				<xsl:call-template name="alternated-row"/>
+				<td><xsl:value-of select="$fileCount"/></td>
+				<td><xsl:value-of select="$errorCount"/></td>
+				<td><xsl:value-of select="$warningCount"/></td>
+				<td><xsl:value-of select="$infoCount"/></td>
+			</tr>
+		</table>
+	</xsl:template>
+
+	<xsl:template name="alternated-row">
+		<xsl:attribute name="class">
+			<xsl:if test="position() mod 2 = 1">a</xsl:if>
+			<xsl:if test="position() mod 2 = 0">b</xsl:if>
+		</xsl:attribute>
+	</xsl:template>
+</xsl:stylesheet>
diff --git a/gradle/java-library.gradle b/gradle/java-library.gradle
index 9c34a86fea..4eb9814528 100644
--- a/gradle/java-library.gradle
+++ b/gradle/java-library.gradle
@@ -28,5 +28,13 @@ test {
     }
 }
 
+tasks.withType(Checkstyle) {
+    reports {
+        xml.enabled false
+        html.enabled true
+        html.stylesheet resources.text.fromFile("$rootDir/config/checkstyle/checkstyle.xsl")
+    }
+}
+
 assert project.hasProperty("artifactName") : "Please configure 'ext.artifactName' for project that wants to use 'java-library.gradle' plugin"
-archivesBaseName = project.ext.artifactName
\ No newline at end of file
+archivesBaseName = project.ext.artifactName
diff --git a/src/main/java/org/mockito/internal/stubbing/answers/DoesNothing.java b/src/main/java/org/mockito/internal/stubbing/answers/DoesNothing.java
index 81e85144ee..bc625fb1bc 100644
--- a/src/main/java/org/mockito/internal/stubbing/answers/DoesNothing.java
+++ b/src/main/java/org/mockito/internal/stubbing/answers/DoesNothing.java
@@ -17,13 +17,13 @@ public class DoesNothing implements Answer<Object>, ValidableAnswer, Serializabl
     private static final long serialVersionUID = 4840880517740698416L;
 
     private static final DoesNothing SINGLETON = new DoesNothing();
-    
+
     private DoesNothing() {}
-    
+
     public static DoesNothing doesNothing(){
         return SINGLETON;
     }
-    
+
     @Override
     public Object answer(InvocationOnMock invocation){
         return null;


Test patch:
diff --git a/src/test/java/org/mockito/internal/stubbing/answers/DoesNothingTest.java b/src/test/java/org/mockito/internal/stubbing/answers/DoesNothingTest.java
index 06f950e2ba..f3c83ffad0 100644
--- a/src/test/java/org/mockito/internal/stubbing/answers/DoesNothingTest.java
+++ b/src/test/java/org/mockito/internal/stubbing/answers/DoesNothingTest.java
@@ -16,7 +16,7 @@
 import org.mockitousage.IMethods;
 
 public class DoesNothingTest   {
-    
+
     private IMethods mock;
     private Invocation invocation_Void;
     private Invocation invocation_void;
@@ -25,17 +25,17 @@ public class DoesNothingTest   {
     @Before
     public void init(){
         mock = mock(IMethods.class);
-        
+
         mock.voidMethod();
         invocation_Void = getLastInvocation();
-        
+
         mock.voidReturningMethod();
         invocation_void = getLastInvocation();
-        
+
         mock.simpleMethod();
         invocation_String = getLastInvocation();
     }
-    
+
     @Test
     public void answer_returnsNull() throws Throwable {
         assertThat(doesNothing().answer(invocation_Void)).isNull();
@@ -52,11 +52,11 @@ public void validateFor_nonVoidReturnType_shouldFail()   {
     public void validateFor_voidReturnType_shouldPass()   {
         doesNothing().validateFor(invocation_void);
     }
-    
+
     @Test
     public void validateFor_voidObjectReturnType() throws Throwable {
         doesNothing().validateFor(invocation_Void);
     }
-    
-    
+
+
 }
diff --git a/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java b/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java
index 6d4a1ab5db..d683605392 100644
--- a/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java
+++ b/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java
@@ -76,31 +76,31 @@ public void should_know_method_is_declared_on_interface() throws Exception {
         assertThat(new InvocationInfo(new InvocationBuilder().method(iAmAbstract()).toInvocation()).isDeclaredOnInterface()).isFalse();
         assertThat(new InvocationInfo(new InvocationBuilder().method("voidMethod").toInvocation()).isDeclaredOnInterface()).isTrue();
     }
-    
+
     @Test
     public void isVoid_invocationOnVoidMethod_returnTrue(){
         mock(IMethods.class).voidMethod();
-        
+
         InvocationInfo voidMethod = new InvocationInfo(getLastInvocation());
-        
+
         assertThat(voidMethod.isVoid()).isTrue();
     }
-    
+
     @Test
     public void isVoid_invocationOnVoidReturningMethod_returnTrue(){
         mock(IMethods.class).voidReturningMethod();
-        
+
         InvocationInfo voidRetuningMethod = new InvocationInfo(getLastInvocation());
-        
+
         assertThat(voidRetuningMethod.isVoid()).isTrue();
     }
-    
+
     @Test
     public void isVoid_invocationNonVoidMethod_returnFalse(){
         mock(IMethods.class).simpleMethod();
-        
+
         InvocationInfo stringReturningMethod = new InvocationInfo(getLastInvocation());
-        
+
         assertThat(stringReturningMethod.isVoid()).isFalse();
     }
 
@@ -110,7 +110,7 @@ abstract class TheAbstract {
         }
         return TheAbstract.class.getDeclaredMethod("iAmAbstract");
     }
-    
+
     private Method iAmNotAbstract() throws NoSuchMethodException {
         abstract class TheNotAbstract {
             void iAmNotAbstract() {};
diff --git a/src/test/java/org/mockitousage/IMethods.java b/src/test/java/org/mockitousage/IMethods.java
index f360475bec..2d5663115c 100644
--- a/src/test/java/org/mockitousage/IMethods.java
+++ b/src/test/java/org/mockitousage/IMethods.java
@@ -187,7 +187,7 @@ public interface IMethods {
     String toString(String foo);
 
     void voidMethod();
-    
+
     Void voidReturningMethod();
 
     String forList(List<String> list);