feat(kotlin): Adding detekt static code analysis for Kotlin modules (s…

…pinnaker#450) * feat(kotlin): Adding detekt static code analysis * chore(kotlin): Apply detekt fixes
jonsie · Jan 16, 2020 · 4252e34 · 4252e34
1 parent 1141373
commit 4252e34
Show file tree

Hide file tree

Showing 14 changed files with 89 additions and 28 deletions.
diff --git a/.detekt.yml b/.detekt.yml
@@ -0,0 +1,7 @@
+style:
+  ThrowsCount:
+    active: false
+
+formatting:
+  ParameterListWrapping:
+    active: false
diff --git a/build.gradle b/build.gradle
@@ -35,6 +35,7 @@ buildscript {
     }
     classpath("com.netflix.nebula:nebula-kotlin-plugin:1.3.50")
     classpath("org.jetbrains.kotlin:kotlin-allopen:1.3.50")
+    classpath("io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.2.2")
   }
 }
 

diff --git a/gradle/kotlin.gradle b/gradle/kotlin.gradle
@@ -16,6 +16,7 @@
 
 apply plugin: "nebula.kotlin"
 apply plugin: "kotlin-spring"
+apply plugin: "io.gitlab.arturbosch.detekt"
 
 dependencies {
   testImplementation "org.junit.jupiter:junit-jupiter-api"
@@ -31,6 +32,9 @@ dependencies {
   testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine"
   testRuntimeOnly "org.junit.vintage:junit-vintage-engine"
   testRuntimeOnly "org.jetbrains.spek:spek-junit-platform-engine"
+
+
+  detektPlugins "io.gitlab.arturbosch.detekt:detekt-formatting:1.2.2"
 }
 
 test {
@@ -45,3 +49,17 @@ compileKotlin {
     jvmTarget = "1.8"
   }
 }
+
+detekt {
+  parallel = true
+  config = files("$rootDir/.detekt.yml")
+  buildUponDefaultConfig = true
+  reports {
+    xml {
+      enabled = false
+    }
+    txt {
+      enabled = false
+    }
+  }
+}
diff --git a/...ions/src/main/kotlin/com/netflix/spinnaker/kork/expressions/ExpressionFunctionProvider.kt b/...ions/src/main/kotlin/com/netflix/spinnaker/kork/expressions/ExpressionFunctionProvider.kt
@@ -87,14 +87,21 @@ interface ExpressionFunctionProvider : ExtensionPoint {
     val parameters: List<FunctionParameter>,
     val documentation: FunctionDocumentation?
   ) {
-    @Deprecated("Please use the overload with description", replaceWith = ReplaceWith("FunctionDefinition(name, \"\", functionParameters)"))
+    @Deprecated(
+      "Please use the overload with description",
+      replaceWith = ReplaceWith("FunctionDefinition(name, \"\", functionParameters)"))
     constructor(name: String, vararg functionParameters: FunctionParameter) :
       this(name, "", listOf(*functionParameters), null)
 
     constructor(name: String, description: String, vararg functionParameters: FunctionParameter) :
       this(name, description, listOf(*functionParameters), null)
 
-    constructor(name: String, description: String, documentation: FunctionDocumentation, vararg functionParameters: FunctionParameter) :
+    constructor(
+      name: String,
+      description: String,
+      documentation: FunctionDocumentation,
+      vararg functionParameters: FunctionParameter
+    ) :
       this(name, description, listOf(*functionParameters), documentation)
   }
 
@@ -131,7 +138,8 @@ interface ExpressionFunctionProvider : ExtensionPoint {
    * This is used by deck to display in-line docs for a SpEL function
    *
    * @param usage example usage, e.g. "#stage('bake in us-east').hasSucceeded"
-   * @param description explanation of the usage sample, markdown supported e.g. "checks if the bake stage has completed successfully"
+   * @param description explanation of the usage sample, markdown supported
+   *  e.g. "checks if the bake stage has completed successfully"
    */
   data class FunctionUsageExample(
     val usage: String,

diff --git a/kork-plugins/src/main/kotlin/com/netflix/spinnaker/kork/plugins/SpinnakerPluginManager.kt b/kork-plugins/src/main/kotlin/com/netflix/spinnaker/kork/plugins/SpinnakerPluginManager.kt
@@ -24,8 +24,8 @@ import com.netflix.spinnaker.kork.plugins.loaders.SpinnakerJarPluginLoader
 import org.pf4j.CompoundPluginLoader
 import org.pf4j.DefaultPluginManager
 import org.pf4j.ExtensionFactory
-import org.pf4j.PluginLoader
 import org.pf4j.PluginDescriptorFinder
+import org.pf4j.PluginLoader
 import org.pf4j.PluginStatusProvider
 import org.pf4j.PluginWrapper
 import java.nio.file.Path
@@ -46,11 +46,11 @@ open class SpinnakerPluginManager(
 
   private val springExtensionFactory: ExtensionFactory = SpringExtensionFactory(this, configResolver)
 
-  private inner class ExtensionFactoryDelegate() : ExtensionFactory {
+  private inner class ExtensionFactoryDelegate : ExtensionFactory {
     override fun <T : Any?> create(extensionClass: Class<T>?): T = springExtensionFactory.create(extensionClass)
   }
 
-  private inner class PluginStatusProviderDelegate() : PluginStatusProvider {
+  private inner class PluginStatusProviderDelegate : PluginStatusProvider {
     override fun disablePlugin(pluginId: String?) = statusProvider.disablePlugin(pluginId)
 
     override fun isPluginDisabled(pluginId: String?): Boolean = statusProvider.isPluginDisabled(pluginId)

diff --git a/kork-plugins/src/main/kotlin/com/netflix/spinnaker/kork/plugins/SpringExtensionFactory.kt b/kork-plugins/src/main/kotlin/com/netflix/spinnaker/kork/plugins/SpringExtensionFactory.kt
@@ -102,6 +102,7 @@ class SpringExtensionFactory(
   /**
    * Load an extension config, provided a set of [ConfigCoordinates].
    */
+  @Suppress("ThrowsCount")
   private fun loadConfig(extension: Any, coordinates: ConfigCoordinates) {
     if (extension !is ConfigurableExtension<*>) {
       return
@@ -127,15 +128,18 @@ class SpringExtensionFactory(
             } catch (se: SecurityException) {
               throw SystemException(se)
             } catch (iae: IllegalAccessException) {
-              throw SystemException("Could not access setConfiguration method on extension: ${extension.javaClass.name}", iae)
+              throw SystemException(
+                "Could not access setConfiguration method on extension: ${extension.javaClass.name}", iae)
             } catch (iae: IllegalArgumentException) {
-              throw SystemException("Configuration on extension appears to be invalid: ${extension.javaClass.name}", iae)
+              throw SystemException(
+                "Configuration on extension appears to be invalid: ${extension.javaClass.name}", iae)
             } catch (ite: InvocationTargetException) {
-              throw SystemException("Failed to invoke setConfiguration on extension: ${extension.javaClass.name}", ite)
+              throw SystemException(
+                "Failed to invoke setConfiguration on extension: ${extension.javaClass.name}", ite)
             }
           }
-          ?: throw SystemException(
-            "Could not find configuration class '${resolvedType.getGeneric(0)}' for extension: ${extension.javaClass.name}")
+          ?: throw SystemException("Could not find configuration class " +
+            "'${resolvedType.getGeneric(0)}' for extension: ${extension.javaClass.name}")
       }
   }
 
@@ -147,7 +151,7 @@ class SpringExtensionFactory(
   private fun <T> createWithoutSpring(extensionClass: Class<T>): T? {
     try {
       return extensionClass.newInstance()
-    } catch (e: Exception) {
+    } catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
       log.error(e.message, e)
     }
     return null

diff --git a/.../main/kotlin/com/netflix/spinnaker/kork/plugins/config/SpringEnvironmentConfigResolver.kt b/.../main/kotlin/com/netflix/spinnaker/kork/plugins/config/SpringEnvironmentConfigResolver.kt
@@ -32,6 +32,7 @@ import org.slf4j.LoggerFactory
 import org.springframework.core.env.ConfigurableEnvironment
 import org.springframework.core.env.EnumerablePropertySource
 import java.lang.reflect.ParameterizedType
+import java.lang.RuntimeException
 import java.util.Properties
 
 /**
@@ -99,7 +100,7 @@ class SpringEnvironmentConfigResolver(
       throw IntegrationException("Failed reading extension config: Input appears invalid", pe)
     } catch (me: JsonMappingException) {
       throw IntegrationException("Failed reading extension config: Could not map provided config to expected shape", me)
-    } catch (e: Exception) {
+    } catch (@Suppress("TooGenericExceptionCaught") e: RuntimeException) {
       throw SystemException("Failed resolving extension config for an unexpected reason", e)
     }
   }

diff --git a/...rc/main/kotlin/com/netflix/spinnaker/kork/plugins/loaders/SpinnakerDefaultPluginLoader.kt b/...rc/main/kotlin/com/netflix/spinnaker/kork/plugins/loaders/SpinnakerDefaultPluginLoader.kt
@@ -34,7 +34,8 @@ class SpinnakerDefaultPluginLoader(
 
   override fun createPluginClassLoader(pluginPath: Path, pluginDescriptor: PluginDescriptor): PluginClassLoader {
     if (pluginDescriptor !is SpinnakerPluginDescriptor) {
-      log.debug("Descriptor for ${pluginDescriptor.pluginId} is not SpinnakerPluginDescriptor: Falling back to default behavior")
+      log.debug("Descriptor for ${pluginDescriptor.pluginId} is not SpinnakerPluginDescriptor: " +
+        "Falling back to default behavior")
       return super.createPluginClassLoader(pluginPath, pluginDescriptor)
     }
 

diff --git a/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/JooqToSpringExceptionTransformer.kt b/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/JooqToSpringExceptionTransformer.kt
@@ -25,10 +25,11 @@ class JooqToSpringExceptionTransformer : DefaultExecuteListener() {
   override fun exception(ctx: ExecuteContext) {
     if (ctx.sqlException() != null) {
       val dialect = ctx.configuration().dialect()
-      val translator = if (dialect != null)
+      val translator = if (dialect != null) {
         SQLErrorCodeSQLExceptionTranslator(dialect.name)
-      else
+      } else {
         SQLStateSQLExceptionTranslator()
+      }
       ctx.exception(translator.translate("jOOQ", ctx.sql(), ctx.sqlException()))
     }
   }

diff --git a/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/ConnectionPoolProperties.kt b/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/ConnectionPoolProperties.kt
@@ -18,6 +18,7 @@ package com.netflix.spinnaker.kork.sql.config
 import org.jooq.SQLDialect
 import java.util.concurrent.TimeUnit
 
+@Suppress("MagicNumber")
 data class ConnectionPoolProperties(
   var dialect: SQLDialect = SQLDialect.MYSQL,
   var jdbcUrl: String? = null,

diff --git a/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/DefaultSqlConfiguration.kt b/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/DefaultSqlConfiguration.kt
@@ -40,10 +40,10 @@ import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration
 import org.springframework.boot.autoconfigure.jdbc.metadata.DataSourcePoolMetadataProvidersConfiguration
 import org.springframework.boot.context.properties.EnableConfigurationProperties
 import org.springframework.context.annotation.Bean
+import org.springframework.context.annotation.Conditional
 import org.springframework.context.annotation.Configuration
 import org.springframework.context.annotation.DependsOn
 import org.springframework.context.annotation.Import
-import org.springframework.context.annotation.Conditional
 import org.springframework.jdbc.datasource.DataSourceTransactionManager
 import org.springframework.jdbc.datasource.TransactionAwareDataSourceProxy
 import sun.net.InetAddressCachePolicy
@@ -78,6 +78,7 @@ class DefaultSqlConfiguration {
   fun secondaryLiquibase(properties: SqlProperties): SpringLiquibase =
     SpringLiquibaseProxy(properties.secondaryMigration)
 
+  @Suppress("ReturnCount", "ThrowsCount")
   @DependsOn("liquibase")
   @Bean
   fun dataSource(dataSourceFactory: DataSourceFactory, properties: SqlProperties): DataSource {
@@ -174,8 +175,14 @@ class DefaultSqlConfiguration {
 
   @Bean(destroyMethod = "close")
   @Conditional(SecondaryPoolDialectCondition::class)
-  fun secondaryJooq(connectionProvider: DataSourceConnectionProvider, sqlProperties: SqlProperties): DSLContext {
-    val secondaryPool: ConnectionPoolProperties = sqlProperties.connectionPools.filter { !it.value.default }.values.first()
+  fun secondaryJooq(
+    connectionProvider: DataSourceConnectionProvider,
+    sqlProperties: SqlProperties
+  ): DSLContext {
+    val secondaryPool: ConnectionPoolProperties = sqlProperties.connectionPools
+      .filter { !it.value.default }
+      .values
+      .first()
     val secondaryJooqConfig: DefaultConfiguration = DefaultConfiguration().apply {
       set(*DefaultExecuteListenerProvider.providers(
         JooqToSpringExceptionTransformer(),
@@ -224,6 +231,7 @@ private fun forceInetAddressCachePolicy() {
 
 class InetAddressOverrideFailure(cause: Throwable) : IllegalStateException(cause)
 
+@Suppress("ThrowsCount")
 private fun validateDefaultTargetDataSources(targets: Collection<TargetDataSource>) {
   if (targets.isEmpty()) {
     throw BeanCreationException("At least one connection pool must be configured")

diff --git a/...ql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/SecondaryPoolDialectCondition.kt b/...ql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/SecondaryPoolDialectCondition.kt
@@ -18,11 +18,11 @@ package com.netflix.spinnaker.kork.sql.config
 
 import org.springframework.boot.autoconfigure.condition.ConditionOutcome
 import org.springframework.boot.autoconfigure.condition.SpringBootCondition
-import org.springframework.context.annotation.ConditionContext
-import org.springframework.core.type.AnnotatedTypeMetadata
-import org.springframework.boot.context.properties.source.ConfigurationPropertyName
 import org.springframework.boot.context.properties.bind.Bindable
 import org.springframework.boot.context.properties.bind.Binder
+import org.springframework.boot.context.properties.source.ConfigurationPropertyName
+import org.springframework.context.annotation.ConditionContext
+import org.springframework.core.type.AnnotatedTypeMetadata
 
 class SecondaryPoolDialectCondition : SpringBootCondition() {
 
@@ -39,9 +39,18 @@ class SecondaryPoolDialectCondition : SpringBootCondition() {
       return false
     }
 
-    val defaultPool: ConnectionPoolProperties = sqlProperties.connectionPools.filter { it.value.default }.values.first()
-    val secondaryPool: ConnectionPoolProperties = sqlProperties.connectionPools.filter { !it.value.default }.values.first()
+    val defaultPool: ConnectionPoolProperties = sqlProperties.connectionPools.first(default = true)
+    val secondaryPool: ConnectionPoolProperties = sqlProperties.connectionPools.first(default = false)
 
     return defaultPool.dialect != secondaryPool.dialect
   }
+
+  private fun MutableMap<String, ConnectionPoolProperties>.first(default: Boolean): ConnectionPoolProperties =
+    filter {
+      if (default) {
+        it.value.default
+      } else {
+        !it.value.default
+      }
+    }.values.first()
 }
diff --git a/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/SqlProperties.kt b/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/config/SqlProperties.kt
@@ -35,10 +35,11 @@ data class SqlProperties(
       }
       return connectionPool!!
     }
-    if (connectionPools.size == 1) {
-      return connectionPools.values.first()
+    return if (connectionPools.size == 1) {
+      connectionPools.values.first()
+    } else {
+      connectionPools.values.first { it.default }
     }
-    return connectionPools.values.first { it.default }
   }
 }
 

diff --git a/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/health/SqlHealthProvider.kt b/kork-sql/src/main/kotlin/com/netflix/spinnaker/kork/sql/health/SqlHealthProvider.kt
@@ -34,6 +34,7 @@ class SqlHealthProvider(
 
   private val log = LoggerFactory.getLogger(javaClass)
 
+  @Suppress("VariableNaming")
   internal val _enabled = AtomicBoolean(false)
   private val _healthException: AtomicReference<Exception> = AtomicReference()
 
@@ -66,7 +67,7 @@ class SqlHealthProvider(
         }
       }
       unhealthyCounter.set(0)
-    } catch (e: Exception) {
+    } catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
       _healthException.set(e)
       healthyCounter.set(0)
       unhealthyCounter.incrementAndGet().also { unhealthyCount ->
-Original file line number
+Diff line change
@@ Expand Up / @@ -35,6 +35,7 @@ buildscript { @@
         }
         classpath("com.netflix.nebula:nebula-kotlin-plugin:1.3.50")
         classpath("org.jetbrains.kotlin:kotlin-allopen:1.3.50")
+        classpath("io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.2.2")
       }
     }
@@ Expand Down @@