Merge branch 'gh188_codenarc'

Add support for checking Groovy code by CodeNarc.

Unfortunately, I have to put a lot of @SuppressWarnings annotations to suppress some checks that I'm
not interesting in. It turned out that I can't suppress them in codenarc.properties file because
doesn't support it (gleclaire/codenarc-maven-plugin#14).

Also I found one false positive in CodeNarc itself (CodeNarc/CodeNarc#176)
that I also suppressed.

Fix #188

Author: php-coder

pom.xml

@@ -378,6 +378,8 @@
 		<cglib.version>2.2.2</cglib.version>
 		<checkstyle.plugin.version>2.17</checkstyle.plugin.version>
 		<clean.plugin.version>3.0.0</clean.plugin.version>
+		<codenarc.plugin.version>0.22-1</codenarc.plugin.version>
+		<codenarc.version>0.25.2</codenarc.version>
 
 		<!-- Redefine default value from spring-boot-dependencies (https://github.com/spring-projects/spring-boot/blob/v1.4.1.RELEASE/spring-boot-dependencies/pom.xml) -->
 		<commons-dbcp.version>1.4</commons-dbcp.version>
@@ -778,6 +780,24 @@
 				</executions>
 			</plugin>
 
+			<!--
+			     Usage:
+			     mvn codenarc:codenarc (checks the sources and generates report in target/site/codenarc.html
+			-->
+			<plugin>
+				<groupId>org.codehaus.mojo</groupId>
+				<artifactId>codenarc-maven-plugin</artifactId>
+				<version>${codenarc.plugin.version}</version>
+				<configuration>
+					<codeNarcVersion>${codenarc.version}</codeNarcVersion>
+					<groovyVersion>${groovy.version}</groovyVersion>
+					<sourceDirectory>src/test/groovy</sourceDirectory>
+					<rulesetfiles>
+						rulesets/basic.xml,rulesets/braces.xml,rulesets/concurrency.xml,rulesets/convention.xml,rulesets/design.xml,rulesets/dry.xml,rulesets/enhanced.xml,rulesets/exceptions.xml,rulesets/formatting.xml,rulesets/generic.xml,rulesets/groovyism.xml,rulesets/imports.xml,rulesets/jdbc.xml,rulesets/logging.xml,rulesets/naming.xml,rulesets/security.xml,rulesets/serialization.xml,rulesets/unnecessary.xml,rulesets/unused.xml
+					</rulesetfiles>
+				</configuration>
+			</plugin>
+			
 			<plugin>
 				<groupId>org.codehaus.mojo</groupId>
 				<artifactId>native2ascii-maven-plugin</artifactId>

src/main/scripts/ci/check-build-and-verify.sh

@@ -9,6 +9,7 @@ fi
 
 CS_FAIL=
 PMD_FAIL=
+CODENARC_FAIL=
 LICENSE_FAIL=
 POM_FAIL=
 BOOTLINT_FAIL=
@@ -20,6 +21,11 @@ VERIFY_FAIL=
 if [ "$RUN_ONLY_INTEGRATION_TESTS" = 'no' ]; then
 	mvn --batch-mode checkstyle:check -Dcheckstyle.violationSeverity=warning >cs.log 2>&1 || CS_FAIL=yes
 	mvn --batch-mode pmd:check >pmd.log 2>&1 || PMD_FAIL=yes
+	mvn --batch-mode codenarc:codenarc \
+		-Dcodenarc.maxPriority1Violations=0 \
+		-Dcodenarc.maxPriority2Violations=0 \
+		-Dcodenarc.maxPriority3Violations=0 \
+		>codenarc.log 2>&1 || CODENARC_FAIL=yes
 	mvn --batch-mode license:check >license.log 2>&1 || LICENSE_FAIL=yes
 	mvn --batch-mode sortpom:verify -Dsort.verifyFail=stop >pom.log || POM_FAIL=yes
 	find src -type f -name '*.html' | xargs bootlint >bootlint.log 2>&1 || BOOTLINT_FAIL=yes
@@ -46,6 +52,7 @@ echo
 if [ "$RUN_ONLY_INTEGRATION_TESTS" = 'no' ]; then
 	print_status "$CS_FAIL"       'Run CheckStyle'
 	print_status "$PMD_FAIL"      'Run PMD'
+	print_status "$CODENARC_FAIL" 'Run CodeNarc'
 	print_status "$LICENSE_FAIL"  'Check license headers'
 	print_status "$POM_FAIL"      'Check sorting of pom.xml'
 	print_status "$BOOTLINT_FAIL" 'Run bootlint'
@@ -61,6 +68,7 @@ echo
 if [ "$RUN_ONLY_INTEGRATION_TESTS" = 'no' ]; then
 	print_log cs.log        'Run CheckStyle'
 	print_log pmd.log       'Run PMD'
+	print_log codenarc.log  'Run CodeNarc'
 	print_log license.log   'Check license headers'
 	print_log pom.log       'Check sorting of pom.xml'
 	print_log bootlint.log  'Run bootlint'
@@ -71,8 +79,8 @@ fi
 
 print_log verify.log   'Run integration tests'
 
-rm -f cs.log pmd.log license.log bootlint.log jasmine.log validator.log test.log verify.log
+rm -f cs.log pmd.log codenarc.log license.log bootlint.log jasmine.log validator.log test.log verify.log
 
-if [ -n "$CS_FAIL$PMD_FAIL$LICENSE_FAIL$POM_FAIL$BOOTLINT_FAIL$JASMINE_FAIL$HTML_FAIL$TEST_FAIL$VERIFY_FAIL" ]; then
+if [ -n "$CS_FAIL$PMD_FAIL$CODENARC_FAIL$LICENSE_FAIL$POM_FAIL$BOOTLINT_FAIL$JASMINE_FAIL$HTML_FAIL$TEST_FAIL$VERIFY_FAIL" ]; then
 	exit 1
 fi

src/test/groovy/ru/mystamps/web/service/CategoryServiceImplTest.groovy

@@ -27,13 +27,14 @@ import ru.mystamps.web.dao.dto.LinkEntityDto
 import ru.mystamps.web.tests.DateUtils
 import ru.mystamps.web.util.SlugUtils
 
+@SuppressWarnings(['ClassJavadoc', 'MethodName', 'NoDef', 'NoTabCharacter', 'TrailingWhitespace'])
 class CategoryServiceImplTest extends Specification {
 
-	private AddCategoryForm form
-	private Integer userId = 123
+	private final Integer userId = 123
+	private final CategoryDao categoryDao = Mock()
+	private final CategoryService service = new CategoryServiceImpl(categoryDao)
 
-	private CategoryDao categoryDao = Mock()
-	private CategoryService service = new CategoryServiceImpl(categoryDao)
+	private AddCategoryForm form
 
 	def setup() {
 		form = new AddCategoryForm()
@@ -92,6 +93,7 @@ class CategoryServiceImplTest extends Specification {
 			actualSlug == expectedSlug
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "add() should pass English category name to dao"() {
 		given:
 			String expectedCategoryName = 'Animals'
@@ -105,6 +107,7 @@ class CategoryServiceImplTest extends Specification {
 			}) >> 20
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "add() should pass Russian category name to dao"() {
 		given:
 			String expectedCategoryName = 'Животные'
@@ -127,9 +130,10 @@ class CategoryServiceImplTest extends Specification {
 			thrown IllegalArgumentException
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "add() should pass slug to dao"() {
 		given:
-			String name = "-foo123 test_"
+			String name = '-foo123 test_'
 		and:
 			String slug = SlugUtils.slugify(name)
 		and:
@@ -143,6 +147,7 @@ class CategoryServiceImplTest extends Specification {
 			}) >> 40
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "add() should assign created at to current date"() {
 		when:
 			service.add(form, userId)
@@ -153,6 +158,7 @@ class CategoryServiceImplTest extends Specification {
 			}) >> 50
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "add() should assign updated at to current date"() {
 		when:
 			service.add(form, userId)
@@ -163,6 +169,7 @@ class CategoryServiceImplTest extends Specification {
 			}) >> 60
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "add() should assign created by to user"() {
 		given:
 			Integer expectedUserId = 10
@@ -175,6 +182,7 @@ class CategoryServiceImplTest extends Specification {
 			}) >> 70
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "add() should assign updated by to user"() {
 		given:
 			Integer expectedUserId = 20
@@ -207,6 +215,7 @@ class CategoryServiceImplTest extends Specification {
 	}
 
 	@Unroll
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "findAllAsLinkEntities(String) should pass language '#expectedLanguage' to dao"(String expectedLanguage) {
 		when:
 			service.findAllAsLinkEntities(expectedLanguage)
@@ -238,6 +247,7 @@ class CategoryServiceImplTest extends Specification {
 			null | _
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "findOneAsLinkEntity() should pass arguments to dao"() {
 		given:
 			String expectedSlug = 'people'
@@ -288,6 +298,7 @@ class CategoryServiceImplTest extends Specification {
 			thrown IllegalArgumentException
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "countCategoriesOf() should pass arguments to dao"() {
 		given:
 			Integer expectedCollectionId = 10
@@ -320,6 +331,7 @@ class CategoryServiceImplTest extends Specification {
 			result == 2L
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "countByName() should pass category name to dao in lowercase"() {
 		when:
 			service.countByName('Sport')
@@ -350,6 +362,7 @@ class CategoryServiceImplTest extends Specification {
 			result == 2L
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "countByNameRu() should pass category name to dao in lowercase"() {
 		when:
 			service.countByNameRu('Спорт')
@@ -371,6 +384,7 @@ class CategoryServiceImplTest extends Specification {
 			thrown IllegalArgumentException
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "countAddedSince() should invoke dao, pass argument and return result from dao"() {
 		given:
 			Date expectedDate = new Date()
@@ -398,6 +412,7 @@ class CategoryServiceImplTest extends Specification {
 			thrown IllegalArgumentException
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "getStatisticsOf() should pass arguments to dao"() {
 		given:
 			Integer expectedCollectionId = 15

src/test/groovy/ru/mystamps/web/service/CollectionServiceImplTest.groovy

@@ -26,8 +26,10 @@ import ru.mystamps.web.dao.dto.CollectionInfoDto
 import ru.mystamps.web.dao.dto.UrlEntityDto
 import ru.mystamps.web.util.SlugUtils
 
+@SuppressWarnings(['ClassJavadoc', 'MethodName', 'NoDef', 'NoTabCharacter', 'TrailingWhitespace'])
 class CollectionServiceImplTest extends Specification {
-	private CollectionDao collectionDao = Mock()
+	
+	private final CollectionDao collectionDao = Mock()
 
 	private CollectionService service
 
@@ -39,27 +41,31 @@ class CollectionServiceImplTest extends Specification {
 	// Tests for createCollection()
 	//
 
+	@SuppressWarnings('FactoryMethodName')
 	def "createCollection() should throw exception when owner id is null"() {
 		when:
 			service.createCollection(null, 'test-owner-login')
 		then:
 			thrown IllegalArgumentException
 	}
 
+	@SuppressWarnings('FactoryMethodName')
 	def "createCollection() should throw exception when owner login is null"() {
 		when:
 			service.createCollection(123, null)
 		then:
 			thrown IllegalArgumentException
 	}
 
+	@SuppressWarnings('FactoryMethodName')
 	def "createCollection() should throw exception when owner login can't be converted to slug"() {
 		when:
 			service.createCollection(123, '')
 		then:
 			thrown IllegalArgumentException
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'FactoryMethodName', 'UnnecessaryReturnKeyword'])
 	def "createCollection() should pass owner id to dao"() {
 		given:
 			Integer expectedOwnerId = 123
@@ -72,6 +78,7 @@ class CollectionServiceImplTest extends Specification {
 			}) >> 100
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'FactoryMethodName', 'UnnecessaryReturnKeyword'])
 	def "createCollection() should pass slugified owner login to dao"() {
 		given:
 			String ownerLogin = 'Test User'
@@ -104,6 +111,7 @@ class CollectionServiceImplTest extends Specification {
 			thrown IllegalArgumentException
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "addToCollection() should pass arguments to dao"() {
 		given:
 			Integer expectedUserId = 123
@@ -138,7 +146,7 @@ class CollectionServiceImplTest extends Specification {
 			thrown IllegalArgumentException
 	}
 
-	
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "removeFromCollection() should find collection by user id"() {
 		given:
 			Integer expectedUserId = 123
@@ -151,13 +159,14 @@ class CollectionServiceImplTest extends Specification {
 			}) >> TestObjects.createUrlEntityDto()
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "removeFromCollection() should remove series from collection"() {
 		given:
 			Integer expectedSeriesId = 456
 		and:
 			UrlEntityDto url = TestObjects.createUrlEntityDto()
 		and:
-			Integer expectedCollectionId = url.getId()
+			Integer expectedCollectionId = url.id
 		and:
 			collectionDao.findCollectionUrlEntityByUserId(_ as Integer) >> url
 		when:
@@ -205,6 +214,7 @@ class CollectionServiceImplTest extends Specification {
 			0 * collectionDao.isSeriesInUserCollection(_ as Integer, _ as Integer)
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "isSeriesInCollection() should pass arguments to dao"() {
 		given:
 			Integer expectedUserId = 123
@@ -257,6 +267,7 @@ class CollectionServiceImplTest extends Specification {
 			0        | _
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "findRecentlyCreated() should pass arguments to dao"() {
 		given:
 			int expectedQuantity = 4
@@ -280,6 +291,7 @@ class CollectionServiceImplTest extends Specification {
 			thrown IllegalArgumentException
 	}
 
+	@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
 	def "findBySlug() should invoke dao, pass argument and return result from dao"() {
 		given:
 			String expectedSlug = 'cuba'