Add Code Narc to validate Groovy style

yahoo · Sep 19, 2017 · 223b3ac · 223b3ac
1 parent 706f8b5
commit 223b3ac
Show file tree

Hide file tree

Showing 17 changed files with 72 additions and 65 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,10 @@ Current
 
 ### Added:
 
+- [Add Code Narc to validate Groovy style](https://github.com/yahoo/fili/pull/420)
+    * Checkstyle is great, but it doesn't process Groovy. Code Narc is Checkstyle for Groovy, so we should totally use
+      it.
+
 - [Allow Webservice to Configure Metric Long Name](https://github.com/yahoo/fili/pull/492)
     * Logical metric needs more config-richness to not just configure metric name, but also metric long name,
       description, etc. MetricInstance is now created by accepting a LogicalMetricInfo which contains all these fields
@@ -43,7 +47,7 @@ Current
 
 - [Add Table-wide Availability](https://github.com/yahoo/fili/pull/414)
     * Add `availableIntervals` field to tables endpoint by union the availability for the logical table without taking
-    the TablesApiRequest into account.
+      the TablesApiRequest into account.
 
 - [Implement EtagCacheRequestHandler](https://github.com/yahoo/fili/pull/312)
     * Add `EtagCacheRequestHandler` that checks the cache for a matching eTag
@@ -52,7 +56,7 @@ Current
 
 - [Implement EtagCacheResponseProcessor](https://github.com/yahoo/fili/pull/311)
     * Add `EtagCacheResponseProcessor` that caches the results if appropriate after completing a query according to
-    etag value.
+      etag value.
 
 - [Add dimension dictionary to metric loader](https://github.com/yahoo/fili/pull/317)
     * Added a two argument version of `loadMetricDictionary` default method in `MetricLoader` interface that allows dimension

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/async/AsyncTestUtils.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/async/AsyncTestUtils.groovy
@@ -8,6 +8,7 @@ import static com.yahoo.bard.webservice.async.jobs.jobrows.DefaultJobField.STATU
 import static com.yahoo.bard.webservice.async.jobs.jobrows.DefaultJobField.USER_ID
 
 import com.yahoo.bard.webservice.application.JerseyTestBinder
+
 import com.yahoo.bard.webservice.util.GroovyTestUtils
 import com.yahoo.bard.webservice.util.JsonSlurper
 

diff --git a/...-core/src/test/groovy/com/yahoo/bard/webservice/async/jobs/stores/JobRowFilterSpec.groovy b/...-core/src/test/groovy/com/yahoo/bard/webservice/async/jobs/stores/JobRowFilterSpec.groovy
@@ -2,7 +2,6 @@
 // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
 package com.yahoo.bard.webservice.async.jobs.stores
 
-import com.yahoo.bard.webservice.async.jobs.stores.JobRowFilter
 import com.yahoo.bard.webservice.web.BadFilterException
 import com.yahoo.bard.webservice.web.FilterOperation
 

diff --git a/...test/groovy/com/yahoo/bard/webservice/data/dimension/impl/LuceneSearchProviderSpec.groovy b/...test/groovy/com/yahoo/bard/webservice/data/dimension/impl/LuceneSearchProviderSpec.groovy
@@ -15,7 +15,6 @@ import org.apache.lucene.store.FSDirectory
 
 import java.nio.file.Files
 import java.nio.file.Path
-
 /**
  * Specification for behavior specific to the LuceneSearchProvider
  */
@@ -239,24 +238,20 @@ class LuceneSearchProviderSpec extends SearchProviderSpec<LuceneSearchProvider>
         Throwable cause = null
 
         @Override
-        public void run() {
-            try {
-                DimensionRow dimensionRow3new = BardDimensionField.makeDimensionRow(
-                        keyValueStoreDimension,
-                        "kumquat",
-                        "this is still not an animal"
-                )
-                DimensionRow dimensionRow4 = BardDimensionField.makeDimensionRow(
-                        keyValueStoreDimension,
-                        "badger",
-                        "Badger badger badger badger mushroom mushroom badger badger badger"
-                )
-                keyValueStoreDimension.addDimensionRow(dimensionRow3new)
-                Thread.sleep(100)
-                keyValueStoreDimension.addDimensionRow(dimensionRow4)
-            } catch (Throwable t) {
-                cause = t
-            }
+        void run() {
+            DimensionRow dimensionRow3new = BardDimensionField.makeDimensionRow(
+                    keyValueStoreDimension,
+                    "kumquat",
+                    "this is still not an animal"
+            )
+            DimensionRow dimensionRow4 = BardDimensionField.makeDimensionRow(
+                    keyValueStoreDimension,
+                    "badger",
+                    "Badger badger badger badger mushroom mushroom badger badger badger"
+            )
+            keyValueStoreDimension.addDimensionRow(dimensionRow3new)
+            Thread.sleep(100)
+            keyValueStoreDimension.addDimensionRow(dimensionRow4)
         }
     }
 

diff --git a/...-core/src/test/groovy/com/yahoo/bard/webservice/data/metric/TemplateDruidQuerySpec.groovy b/...-core/src/test/groovy/com/yahoo/bard/webservice/data/metric/TemplateDruidQuerySpec.groovy
@@ -2,7 +2,6 @@
 // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
 package com.yahoo.bard.webservice.data.metric
 
-import com.yahoo.bard.webservice.data.time.ZonelessTimeGrain
 import com.yahoo.bard.webservice.druid.model.aggregation.Aggregation
 import com.yahoo.bard.webservice.druid.model.aggregation.CountAggregation
 import com.yahoo.bard.webservice.druid.model.aggregation.DoubleSumAggregation

diff --git a/...t/groovy/com/yahoo/bard/webservice/table/availability/VolatilityTableSelectionSpec.groovy b/...t/groovy/com/yahoo/bard/webservice/table/availability/VolatilityTableSelectionSpec.groovy
@@ -8,7 +8,6 @@ import static com.yahoo.bard.webservice.data.config.names.TestDruidTableName.MON
 import com.yahoo.bard.webservice.web.endpoints.BaseDataServletComponentSpec
 import com.yahoo.bard.webservice.web.endpoints.DataServlet
 
-import org.joda.time.DateTime
 import org.joda.time.Interval
 
 /**

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/util/FilterTokenizerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/util/FilterTokenizerSpec.groovy
@@ -2,8 +2,6 @@
 // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
 package com.yahoo.bard.webservice.util
 
-import com.yahoo.bard.webservice.util.FilterTokenizer
-
 import spock.lang.Specification
 import spock.lang.Unroll
 

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/filters/RateLimitFilterSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/filters/RateLimitFilterSpec.groovy
@@ -113,7 +113,7 @@ class RateLimitFilterSpec extends Specification {
                     break
 
                 default:
-                    assert false
+                    fail
             }
         }
     }
@@ -266,7 +266,7 @@ class RateLimitFilterSpec extends Specification {
                     fail.incrementAndGet()
                     break
                 default:
-                    assert false
+                    fail
             }
         }
     }

diff --git a/...c/test/groovy/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandlerSpec.groovy b/...c/test/groovy/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandlerSpec.groovy
@@ -186,13 +186,12 @@ class WeightCheckRequestHandlerSpec extends Specification {
         """
         JsonParser parser = new JsonFactory().createParser(weightResponse)
         JsonNode jsonResult = MAPPER.readTree(parser)
-        1 * next.handleRequest(context, request, groupByQuery, response)
 
         when:
         success.invoke(jsonResult)
 
         then:
-        1 == 1
+        1 * next.handleRequest(context, request, groupByQuery, response)
     }
 
     def "Test build and invoke success callback count too high"() {
@@ -217,15 +216,15 @@ class WeightCheckRequestHandlerSpec extends Specification {
         """
         JsonParser parser = new JsonFactory().createParser(weightResponse)
         JsonNode jsonResult = new ObjectMapper().readTree(parser)
-        0 * next.handleRequest(context, request, groupByQuery, response)
         HttpErrorCallback ec = Mock(HttpErrorCallback)
-        1 * response.getErrorCallback(groupByQuery) >> ec
-        1 * ec.dispatch(507, _, _)
+
         when:
         success.invoke(jsonResult)
 
         then:
-        1 == 1
+        0 * next.handleRequest(context, request, groupByQuery, response)
+        1 * response.getErrorCallback(groupByQuery) >> ec
+        1 * ec.dispatch(507, _, _)
     }
 
     def "Test build and invoke success callback invalid json"() {
@@ -250,15 +249,14 @@ class WeightCheckRequestHandlerSpec extends Specification {
         """
         JsonParser parser = new JsonFactory().createParser(weightResponse)
         JsonNode jsonResult = new ObjectMapper().readTree(parser)
-        0 * next.handleRequest(context, request, groupByQuery, response)
         FailureCallback fc = Mock(FailureCallback)
-        1 * response.getFailureCallback(groupByQuery) >> fc
-        1 * fc.dispatch(_)
 
         when:
         success.invoke(jsonResult)
 
         then:
-        1 == 1
+        0 * next.handleRequest(context, request, groupByQuery, response)
+        1 * response.getFailureCallback(groupByQuery) >> fc
+        1 * fc.dispatch(_)
     }
 }
diff --git a/.../src/test/groovy/com/yahoo/bard/webservice/web/handlers/workflow/DruidWorkflowSpec.groovy b/.../src/test/groovy/com/yahoo/bard/webservice/web/handlers/workflow/DruidWorkflowSpec.groovy
@@ -2,8 +2,6 @@
 // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
 package com.yahoo.bard.webservice.web.handlers.workflow
 
-import com.yahoo.bard.webservice.web.handlers.WeightCheckRequestHandler
-
 import static com.yahoo.bard.webservice.config.BardFeatureFlag.DRUID_CACHE
 import static com.yahoo.bard.webservice.config.BardFeatureFlag.DRUID_CACHE_V2
 import static com.yahoo.bard.webservice.config.BardFeatureFlag.QUERY_SPLIT

diff --git a/...oovy/com/yahoo/bard/webservice/web/responseprocessors/MappingResponseProcessorSpec.groovy b/...oovy/com/yahoo/bard/webservice/web/responseprocessors/MappingResponseProcessorSpec.groovy
@@ -52,9 +52,9 @@ class MappingResponseProcessorSpec extends Specification{
 
     MappingResponseProcessor buildSimpleMRP() {
         return new MappingResponseProcessor(apiRequest, objectMappers) {
-            public FailureCallback getFailureCallback(DruidAggregationQuery<?> query) {return null;}
-            public HttpErrorCallback getErrorCallback(DruidAggregationQuery<?> query) {return null;}
-            public void processResponse(JsonNode json, DruidAggregationQuery<?> query, LoggingContext metadata) {}
+            FailureCallback getFailureCallback(DruidAggregationQuery<?> query) {return null}
+            HttpErrorCallback getErrorCallback(DruidAggregationQuery<?> query) {return null}
+            void processResponse(JsonNode json, DruidAggregationQuery<?> query, LoggingContext metadata) {return}
         }
     }
 

diff --git a/...st/groovy/com/yahoo/wiki/webservice/web/endpoints/AutomaticDruidConfigLoadTaskSpec.groovy b/...st/groovy/com/yahoo/wiki/webservice/web/endpoints/AutomaticDruidConfigLoadTaskSpec.groovy
@@ -10,7 +10,7 @@ import com.fasterxml.jackson.databind.ObjectMapper
 
 import spock.lang.Specification
 
-public class AutomaticDruidConfigLoadTaskSpec extends Specification {
+class AutomaticDruidConfigLoaderSpec extends Specification {
     TestDruidWebService druidWebService
     DruidNavigator druidNavigator
     private String[] metrics = ["count", "added", "deleted", "delta", "user_unique"]

diff --git a/fili-sql/src/test/groovy/com/yahoo/bard/webservice/sql/DefaultSqlBackedClientSpec.groovy b/fili-sql/src/test/groovy/com/yahoo/bard/webservice/sql/DefaultSqlBackedClientSpec.groovy
@@ -34,7 +34,7 @@ import static com.yahoo.bard.webservice.sql.builders.Filters.or
 import static com.yahoo.bard.webservice.sql.builders.Filters.search
 import static com.yahoo.bard.webservice.sql.builders.Filters.select
 import static com.yahoo.bard.webservice.sql.builders.Havings.and
-import static com.yahoo.bard.webservice.sql.builders.Havings.equals
+import static com.yahoo.bard.webservice.sql.builders.Havings.equal
 import static com.yahoo.bard.webservice.sql.builders.Havings.gt
 import static com.yahoo.bard.webservice.sql.builders.Havings.lt
 import static com.yahoo.bard.webservice.sql.builders.Intervals.interval
@@ -293,8 +293,8 @@ class DefaultSqlBackedClientSpec extends Specification {
         HOUR      | [IS_NEW, IS_ROBOT] | null                           | and(gt(ADDED, 1), lt(ADDED, 1)) | 0
         HOUR      | [IS_ROBOT]         | null                           | null                            | 24 * 2
         DAY       | [IS_NEW, IS_ROBOT] | null                           | null                            | 4
-        HOUR      | [IS_NEW, IS_ROBOT] | null                           | equals(ADDED, 0)                | 0
-        HOUR      | [IS_NEW, IS_ROBOT] | search(COMMENT, FIRST_COMMENT) | equals(ADDED, 36)               | 1
+        HOUR      | [IS_NEW, IS_ROBOT] | null                           | equal(ADDED, 0)                 | 0
+        HOUR      | [IS_NEW, IS_ROBOT] | search(COMMENT, FIRST_COMMENT) | equal(ADDED, 36)                | 1
         HOUR      | []                 | null                           | gt(ADDED, 400000)               | 12
         HOUR      | []                 | null                           | null                            | 24
         DAY       | [PAGE, USER]       | null                           | null                            | 36565

diff --git a/fili-sql/src/test/groovy/com/yahoo/bard/webservice/sql/DruidQueryToSqlConverterSpec.groovy b/fili-sql/src/test/groovy/com/yahoo/bard/webservice/sql/DruidQueryToSqlConverterSpec.groovy
@@ -20,8 +20,6 @@ import static com.yahoo.bard.webservice.sql.builders.SimpleDruidQueryBuilder.get
 
 import com.yahoo.bard.webservice.database.Database
 import com.yahoo.bard.webservice.druid.model.orderby.LimitSpec
-import com.yahoo.bard.webservice.druid.model.orderby.OrderByColumn
-import com.yahoo.bard.webservice.druid.model.orderby.SortDirection
 import com.yahoo.bard.webservice.druid.model.query.DruidQuery
 import com.yahoo.bard.webservice.druid.model.query.Granularity
 import com.yahoo.bard.webservice.druid.model.query.GroupByQuery

diff --git a/fili-sql/src/test/groovy/com/yahoo/bard/webservice/sql/builders/Havings.groovy b/fili-sql/src/test/groovy/com/yahoo/bard/webservice/sql/builders/Havings.groovy
@@ -8,30 +8,28 @@ import com.yahoo.bard.webservice.druid.model.having.NotHaving
 import com.yahoo.bard.webservice.druid.model.having.NumericHaving
 import com.yahoo.bard.webservice.druid.model.having.OrHaving
 
-public class Havings {
-    public static AndHaving and(Having... havings) {
+class Havings {
+    static AndHaving and(Having... havings) {
         return new AndHaving(Arrays.asList(havings))
     }
 
-
-    public static OrHaving or(Having... havings) {
+    static OrHaving or(Having... havings) {
         return new OrHaving(Arrays.asList(havings))
     }
 
-
-    public static NotHaving not(Having havings) {
+    static NotHaving not(Having havings) {
         return new NotHaving(havings)
     }
 
-    public static NumericHaving equals(String name, Number value) {
+    static NumericHaving equal(String name, Number value) {
         return new NumericHaving(Having.DefaultHavingType.EQUAL_TO, name, value);
     }
 
-    public static NumericHaving gt(String name, Number value) {
+    static NumericHaving gt(String name, Number value) {
         return new NumericHaving(Having.DefaultHavingType.GREATER_THAN, name, value);
     }
 
-    public static NumericHaving lt(String name, Number value) {
+    static NumericHaving lt(String name, Number value) {
         return new NumericHaving(Having.DefaultHavingType.LESS_THAN, name, value);
     }
 }
diff --git a/fili-sql/src/test/groovy/com/yahoo/bard/webservice/sql/evaluator/HavingEvaluatorSpec.groovy b/fili-sql/src/test/groovy/com/yahoo/bard/webservice/sql/evaluator/HavingEvaluatorSpec.groovy
@@ -10,7 +10,7 @@ import static com.yahoo.bard.webservice.sql.builders.Aggregator.max
 import static com.yahoo.bard.webservice.sql.builders.Aggregator.min
 import static com.yahoo.bard.webservice.sql.builders.Aggregator.sum
 import static com.yahoo.bard.webservice.sql.builders.Havings.and
-import static com.yahoo.bard.webservice.sql.builders.Havings.equals
+import static com.yahoo.bard.webservice.sql.builders.Havings.equal
 import static com.yahoo.bard.webservice.sql.builders.Havings.gt
 import static com.yahoo.bard.webservice.sql.builders.Havings.lt
 import static com.yahoo.bard.webservice.sql.builders.Havings.or
@@ -79,12 +79,12 @@ class HavingEvaluatorSpec extends Specification {
                 toString();
         sql.contains(expectedHavingSql)
         where: "we have"
-        having                                                | aggregations                           | expectedHavingSql
-        gt(API + ADDED, ONE)                                  | [sum(API + ADDED)]                     | "HAVING SUM(`${ADDED}`) > 1"
-        lt(API + DELETED, ONE)                                | [sum(API + DELETED)]                   | "HAVING SUM(`${DELETED}`) < 1"
-        equals(API + DELETED, ONE)                            | [sum(API + DELETED), sum(API + ADDED)] | "HAVING SUM(`${DELETED}`) = 1"
-        or(equals(API + DELETED, ONE), lt(API + ADDED, TWO))  | [max(API + DELETED), min(API + ADDED)] | "HAVING MAX(`${DELETED}`) = 1 OR MIN(`${ADDED}`) < 2"
-        and(equals(API + DELETED, ONE), lt(API + ADDED, TWO)) | [max(API + DELETED), min(API + ADDED)] | "HAVING MAX(`${DELETED}`) = 1 AND MIN(`${ADDED}`) < 2"
+        having                                               | aggregations                           | expectedHavingSql
+        gt(API + ADDED, ONE)                                 | [sum(API + ADDED)]                     | "HAVING SUM(`${ADDED}`) > 1"
+        lt(API + DELETED, ONE)                               | [sum(API + DELETED)]                   | "HAVING SUM(`${DELETED}`) < 1"
+        equal(API + DELETED, ONE)                            | [sum(API + DELETED), sum(API + ADDED)] | "HAVING SUM(`${DELETED}`) = 1"
+        or(equal(API + DELETED, ONE), lt(API + ADDED, TWO))  | [max(API + DELETED), min(API + ADDED)] | "HAVING MAX(`${DELETED}`) = 1 OR MIN(`${ADDED}`) < 2"
+        and(equal(API + DELETED, ONE), lt(API + ADDED, TWO)) | [max(API + DELETED), min(API + ADDED)] | "HAVING MAX(`${DELETED}`) = 1 AND MIN(`${ADDED}`) < 2"
 
     }
 

diff --git a/pom.xml b/pom.xml
@@ -749,6 +749,26 @@
                     </execution>
                 </executions>
             </plugin>
+
+            <plugin>
+                <groupId>org.codehaus.mojo</groupId>
+                <artifactId>codenarc-maven-plugin</artifactId>
+                <version>0.18-1</version>
+                <configuration>
+                    <sourceDirectory>${project.basedir}</sourceDirectory>
+                    <maxPriority1Violations>0</maxPriority1Violations>
+                    <maxPriority2Violations>0</maxPriority2Violations>
+                    <maxPriority3Violations>0</maxPriority3Violations>
+                </configuration>
+                <executions>
+                    <execution>
+                        <phase>prepare-package</phase>
+                        <goals>
+                            <goal>codenarc</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
         </plugins>
     </build>
     <reporting>