Refactor the plugin code:

- Change the packages layout
- Clean and simplify some code
- Change the way exec runner works. Use as many mutable data structures as possible
- Remove all the unnecessary after project evaluation's hook usages in tasks (issue #39)

Author: bsautel

src/main/kotlin/com/github/gradle/node/NodeExtension.kt

@@ -5,9 +5,7 @@ import org.gradle.api.Project
 import java.io.File
 import kotlin.properties.Delegates
 
-@Suppress("MemberVisibilityCanBePrivate", "unused") // Extension object; properties may be configured in build scripts
 open class NodeExtension(project: Project) {
-
     private val cacheDir = File(project.projectDir, ".gradle")
     var workDir = File(cacheDir, "nodejs")
     var npmWorkDir = File(cacheDir, "npm")

src/main/kotlin/com/github/gradle/node/NodePlugin.kt

@@ -1,40 +1,32 @@
 package com.github.gradle.node
 
-import com.github.gradle.node.npm.NpmInstallTask
-import com.github.gradle.node.npm.NpmSetupTask
-import com.github.gradle.node.npm.NpmTask
-import com.github.gradle.node.npm.NpxTask
+import com.github.gradle.node.npm.task.NpmInstallTask
+import com.github.gradle.node.npm.task.NpmSetupTask
+import com.github.gradle.node.npm.task.NpmTask
+import com.github.gradle.node.npm.task.NpxTask
+import com.github.gradle.node.task.NodeSetupTask
 import com.github.gradle.node.task.NodeTask
-import com.github.gradle.node.task.SetupTask
 import com.github.gradle.node.variant.VariantBuilder
-import com.github.gradle.node.yarn.YarnInstallTask
-import com.github.gradle.node.yarn.YarnSetupTask
-import com.github.gradle.node.yarn.YarnTask
+import com.github.gradle.node.yarn.task.YarnInstallTask
+import com.github.gradle.node.yarn.task.YarnSetupTask
+import com.github.gradle.node.yarn.task.YarnTask
 import org.gradle.api.Plugin
 import org.gradle.api.Project
 import org.gradle.kotlin.dsl.create
 
 class NodePlugin : Plugin<Project> {
-
     private lateinit var project: Project
-    private lateinit var config: NodeExtension
-
-    private lateinit var setupTask: SetupTask
-    private lateinit var npmSetupTask: NpmSetupTask
-    private lateinit var yarnSetupTask: YarnSetupTask
+    private lateinit var nodeExtension: NodeExtension
 
     override fun apply(project: Project) {
         this.project = project
-        this.config = NodeExtension.create(project)
+        this.nodeExtension = NodeExtension.create(project)
         addGlobalTypes()
         addTasks()
         addNpmRule()
         addYarnRule()
         this.project.afterEvaluate {
-            config.variant = VariantBuilder(config).build()
-            configureSetupTask()
-            configureNpmSetupTask()
-            configureYarnSetupTask()
+            nodeExtension.variant = VariantBuilder(nodeExtension).build()
         }
     }
 
@@ -45,16 +37,16 @@ class NodePlugin : Plugin<Project> {
         addGlobalTaskType(YarnTask::class.java)
     }
 
+    private fun addGlobalTaskType(type: Class<*>) {
+        project.extensions.extraProperties[type.simpleName] = type
+    }
+
     private fun addTasks() {
         project.tasks.create(NpmInstallTask.NAME, NpmInstallTask::class)
         project.tasks.create(YarnInstallTask.NAME, YarnInstallTask::class)
-        setupTask = project.tasks.create(SetupTask.NAME, SetupTask::class)
-        npmSetupTask = project.tasks.create(NpmSetupTask.NAME, NpmSetupTask::class)
-        yarnSetupTask = project.tasks.create(YarnSetupTask.NAME, YarnSetupTask::class)
-    }
-
-    private fun addGlobalTaskType(type: Class<*>) {
-        project.extensions.extraProperties[type.simpleName] = type
+        project.tasks.create(NodeSetupTask.NAME, NodeSetupTask::class)
+        project.tasks.create(NpmSetupTask.NAME, NpmSetupTask::class)
+        project.tasks.create(YarnSetupTask.NAME, YarnSetupTask::class)
     }
 
     private fun addNpmRule() { // note this rule also makes it possible to specify e.g. "dependsOn npm_install"
@@ -85,18 +77,6 @@ class NodePlugin : Plugin<Project> {
         }
     }
 
-    private fun configureSetupTask() {
-        setupTask.isEnabled = config.download
-    }
-
-    private fun configureNpmSetupTask() {
-        npmSetupTask.configureVersion(config.npmVersion)
-    }
-
-    private fun configureYarnSetupTask() {
-        yarnSetupTask.configureVersion(config.yarnVersion)
-    }
-
     companion object {
         const val NODE_GROUP = "Node"
     }

src/main/kotlin/com/github/gradle/node/exec/ExecConfiguration.kt

@@ -0,0 +1,14 @@
+package com.github.gradle.node.exec
+
+import org.gradle.process.ExecSpec
+import java.io.File
+
+internal data class ExecConfiguration(
+        val executable: String,
+        val args: List<String> = listOf(),
+        var additionalBinPath: String? = null,
+        val environment: Map<String, String> = mapOf(),
+        val workingDir: File? = null,
+        val ignoreExitValue: Boolean = false,
+        val execOverrides: (ExecSpec.() -> Unit)? = null
+)

src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt

@@ -1,79 +1,40 @@
 package com.github.gradle.node.exec
 
 import com.github.gradle.node.NodeExtension
-import com.github.gradle.node.variant.Variant
-import groovy.lang.Closure
 import org.gradle.api.Project
-import org.gradle.api.tasks.Input
-import org.gradle.api.tasks.Internal
-import org.gradle.kotlin.dsl.getByType
 import org.gradle.kotlin.dsl.invoke
-import org.gradle.process.ExecResult
-import org.gradle.process.ExecSpec
 import java.io.File
-import java.util.*
 
-abstract class ExecRunner(
-        @get:Internal protected var project: Project
-) {
-
-    @get:Internal
-    protected val ext: NodeExtension = NodeExtension[project]
-    @get:Internal
-    protected val variant: Variant by lazy { ext.variant }
-
-    @get:Input
-    val environment = LinkedHashMap<String, String>()
-    @get:Input
-    var ignoreExitValue = false
-    @get:Internal
-    var workingDir: File? = null
-    @get:Internal
-    var arguments: MutableList<String> = ArrayList()
-    @get:Internal
-    var execOverrides: (ExecSpec.() -> Unit)? = null
-
-    fun execute(): ExecResult {
-        return doExecute()
-    }
-
-    protected fun run(exec: String, runArgs: List<String>): ExecResult {
-        return project.exec {
-            executable = exec
-            args = runArgs
-            environment = computeExecEnvironment()
-            isIgnoreExitValue = ignoreExitValue
-            workingDir = computeWorkingDir()
-            execOverrides?.invoke(this)
+internal class ExecRunner {
+    fun execute(project: Project, execConfiguration: ExecConfiguration) {
+        project.exec {
+            executable = execConfiguration.executable
+            args = execConfiguration.args
+            environment = computeEnvironment(execConfiguration)
+            isIgnoreExitValue = execConfiguration.ignoreExitValue
+            workingDir = computeWorkingDir(project, execConfiguration)
+            execConfiguration.execOverrides?.invoke(this)
         }
     }
 
-    private fun computeWorkingDir(): File? {
-        val computedWorkingDir = workingDir ?: project.extensions.getByType(NodeExtension::class).nodeModulesDir
-        computedWorkingDir.mkdirs()
-        return computedWorkingDir
-    }
-
-    private fun computeExecEnvironment(): Map<String, String> {
-        val execEnvironment = linkedMapOf<String, String>()
+    private fun computeEnvironment(execConfiguration: ExecConfiguration): Map<String, String> {
+        val execEnvironment = mutableMapOf<String, String>()
         execEnvironment += System.getenv()
-        execEnvironment += environment
-        val path = computeAdditionalBinPath()
-        if (path.isNotBlank()) {
+        execEnvironment += execConfiguration.environment
+        if (execConfiguration.additionalBinPath != null) {
             // Take care of Windows environments that may contain "Path" OR "PATH" - both existing
             // possibly (but not in parallel as of now)
-            val envPathName = if (execEnvironment["Path"] != null) "Path" else "PATH"
-            execEnvironment[envPathName] = path + File.pathSeparator + execEnvironment[envPathName]
+            val pathEnvironmentVariableName = if (execEnvironment["Path"] != null) "Path" else "PATH"
+            val actualPath = execEnvironment[pathEnvironmentVariableName]
+            execEnvironment[pathEnvironmentVariableName] =
+                    "${execConfiguration.additionalBinPath}${File.pathSeparator}${actualPath}"
         }
         return execEnvironment
     }
 
-    // Configurable; Groovy support
-    fun setExecOverrides(execOverrides: Closure<ExecSpec>) {
-        this.execOverrides = { execOverrides.invoke(this) }
+    private fun computeWorkingDir(project: Project, execConfiguration: ExecConfiguration): File? {
+        val workingDir = execConfiguration.workingDir ?: NodeExtension[project].nodeModulesDir
+        workingDir.mkdirs()
+        return workingDir
     }
-
-    protected abstract fun computeAdditionalBinPath(): String
-
-    protected abstract fun doExecute(): ExecResult
 }

src/main/kotlin/com/github/gradle/node/exec/NodeExecConfiguration.kt

@@ -0,0 +1,12 @@
+package com.github.gradle.node.exec
+
+import org.gradle.process.ExecSpec
+import java.io.File
+
+internal data class NodeExecConfiguration(
+        val command: List<String> = listOf(),
+        val environment: Map<String, String> = mapOf(),
+        val workingDir: File? = null,
+        val ignoreExitValue: Boolean = false,
+        val execOverrides: (ExecSpec.() -> Unit)? = null
+)

src/main/kotlin/com/github/gradle/node/exec/NodeExecRunner.kt

@@ -1,16 +1,22 @@
 package com.github.gradle.node.exec
 
+import com.github.gradle.node.NodeExtension
 import org.gradle.api.Project
-import org.gradle.process.ExecResult
 
-class NodeExecRunner(project: Project) : ExecRunner(project) {
-
-    override fun doExecute(): ExecResult {
-        val exec = if (ext.download) variant.nodeExec else "node"
-        return run(exec, arguments)
+internal class NodeExecRunner {
+    fun execute(project: Project, nodeExecConfiguration: NodeExecConfiguration) {
+        val execConfiguration = buildExecConfiguration(project, nodeExecConfiguration)
+        val execRunner = ExecRunner()
+        execRunner.execute(project, execConfiguration)
     }
 
-    override fun computeAdditionalBinPath(): String {
-        return if (ext.download) variant.nodeBinDir.absolutePath else ""
+    private fun buildExecConfiguration(project: Project, nodeExecConfiguration: NodeExecConfiguration): ExecConfiguration {
+        val nodeExtension = NodeExtension[project]
+        val variant = nodeExtension.variant
+        val executable = if (nodeExtension.download) variant.nodeExec else "node"
+        val additionalBinPath = if (nodeExtension.download) variant.nodeBinDir.absolutePath else null
+        return ExecConfiguration(executable, nodeExecConfiguration.command, additionalBinPath,
+                nodeExecConfiguration.environment, nodeExecConfiguration.workingDir,
+                nodeExecConfiguration.ignoreExitValue, nodeExecConfiguration.execOverrides)
     }
 }