From 328b86d8d7d0dbffab0c45b5cb84f53a6b0aef91 Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Fri, 26 Aug 2016 16:28:53 +0100
Subject: [PATCH] JAL-1854 two more rules, readme notes

---
 resources/checkstyle/README.txt     |   30 ++++++++++++++++++------
 resources/checkstyle/checkstyle.xml |   43 ++++++++++++++++++++++++++++++-----
 2 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/resources/checkstyle/README.txt b/resources/checkstyle/README.txt
index 7ad1268..1c2aced 100644
--- a/resources/checkstyle/README.txt
+++ b/resources/checkstyle/README.txt
@@ -23,7 +23,7 @@ Config
 		checkstyle-suppress.xml : rules to exclude certain checks / files
 		import-control.xml      : package import rules
 	
-	Checkstyle error messages can be customised. I've done this for TypeName as an example.
+	Checkstyle error messages can be customised. See TypeName for an example.
 
 How to use checkstyle
 ---------------------
@@ -33,9 +33,8 @@ How to use checkstyle
 		- notice CheckstyleNature gets added to the .project file
 		- don't commit this file unless we all agree to!
 		- Checkstyle will run as you recompile changed code
-		- checking the whole project can be slow and may hang - not recommended for now
 
-	Option 2: on selected code
+	Option 2: on demand on selected code
 		- right-click on a class or package and Checkstyle | Check code with checkstyle
 
 Checkstyle rules
@@ -46,10 +45,23 @@ Checkstyle rules
 	- which rules to use
 	- what naming and layout standards to apply
 	- settings for complexity metrics
-	- whether any rules should report error instead of warning  
+	- whether any rules should report an error instead of a warning  
 	
-Gotchas
--------
+Suppressing findings
+--------------------
+	If there are warnings you judge it ok to suppress, your options are
+	(from most global to most local scope):
+	- remove the rule entirely
+	- adjust its properties
+	- add an entry in checkstyle-suppress.xml to skip the file for the rule
+	- add comments around the reported source lines
+		// CHECKSTYLE.OFF: RuleName
+		source code here
+		// CHECKSTYLE.ON: RuleName
+	The suppression should be as localised as possible, to avoid false negatives.
+	
+Tips
+----
 	Sometimes checkstyle needs a kick before it will refresh its findings.
 	A whitespace edit in checkstyle.xml usually does this. There may be better ways.
 	
@@ -62,4 +74,8 @@ Gotchas
 	that, carefully check / back out / redo any recent changes to its config.
 	
 	Putting <!-- XML comments --> inside a checkstyle <module> causes it to be ignored!
-	
\ No newline at end of file
+	
+	If a rule doesn't behave as you expected, read its documentation carefully, including
+	the use and default value of any properties.
+	
+	To highlight a single rule's findings, you could temporarily raise its severity to error.
diff --git a/resources/checkstyle/checkstyle.xml b/resources/checkstyle/checkstyle.xml
index 4189a21..423f698 100644
--- a/resources/checkstyle/checkstyle.xml
+++ b/resources/checkstyle/checkstyle.xml
@@ -175,14 +175,30 @@
 		<module name="MultipleVariableDeclarations"/>
 		
 		<!-- 
+			Don't have more than one statement on a line
+			(code formatting on save may enforce this anyway)
+		 -->
+		<module name="OneStatementPerLine"/>
+
+		<!-- 
+			Declare variables close to their point of first use 
+			(doesn't handle variables used inside loops very well) 
+		-->
+		<module name="VariableDeclarationUsageDistance">
+			<property name="allowedDistance" value="5" />
+			<message key="variable.declaration.usage.distance.extend"
+				value="Distance between declaration of ''{0}'' and its first use is {1}, suggested maximum is {2}. Consider moving, or make final if it may not be modified." />
+		</module>
+	
+		<!-- 
 			Only use blocks within control statements 
 		-->
-		<module name="AvoidNestedBlocks"/>
-		
+		<module name="AvoidNestedBlocks" />
+
 		<!-- 
 			Require at least a comment within a block. 
-			Note this will accept auto-generated // TODO comments,
-			(but they should be flagged up by the TodoComment rule)
+			Note this will accept auto-generated // TODO comments, 
+			(but they should be flagged up by the TodoComment rule) 
 		-->
 		<module name="EmptyBlock">
 			<property name="option" value="text"/>
@@ -334,7 +350,8 @@
 	<!-- ****************************** -->
 
 		<!-- 
-			Check for use of factory method rather than constructor for specified classes 
+			Check for use of factory method rather than constructor for specified classes
+			e.g. Boolean.valueOf(true) rather than new Boolean(true)
 		-->
 		<module name="IllegalInstantiation">
 			<property name="classes" value="java.lang.Boolean"/>
@@ -446,7 +463,13 @@
 		</module>
 		
 		<!-- 
-			Checks that clone() invokes super.clone() 
+			Don't override clone - it never works! 
+		 -->
+		<module name="NoClone"/>
+		
+		<!-- 
+			Checks that clone() invokes super.clone()
+			(for classes that break the NoClone rule)
 		 -->
 		<module name="SuperClone"/>
 		
@@ -472,6 +495,14 @@
 			Checks that exceptions are immutable (have only final fields)
 		 -->
 		<module name="MutableException"/>
+		
+		<!-- 
+		 	A general rule to check for source text tokens that shouldn't be there
+		 	see http://checkstyle.sourceforge.net/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html
+		 -->
+		<module name="IllegalToken">
+		   <property name="tokens" value="LITERAL_ASSERT"/>
+		</module>
 
 	<!-- ****************************** -->
 	<!--           COMPLEXITY           -->
-- 
1.7.10.2