From cbdd480642d66a6db1748b640474902dba6679c1 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 31 May 2023 16:49:28 +0200 Subject: [PATCH 1/3] refactor(Npm): Move some functions to top-level Move private functions that to not require `this`-context out of the class for a better overview. Signed-off-by: Sebastian Schuberth --- .../node/src/main/kotlin/Npm.kt | 76 +++++++++---------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/plugins/package-managers/node/src/main/kotlin/Npm.kt b/plugins/package-managers/node/src/main/kotlin/Npm.kt index cb30e04fd4b72..86ec89756d312 100644 --- a/plugins/package-managers/node/src/main/kotlin/Npm.kt +++ b/plugins/package-managers/node/src/main/kotlin/Npm.kt @@ -261,33 +261,6 @@ open class Npm( } } - private fun isValidNodeModulesDirectory(rootModulesDir: File, modulesDir: File?): Boolean { - if (modulesDir == null) return false - - var currentDir: File = modulesDir - while (currentDir != rootModulesDir) { - if (currentDir.name != "node_modules") { - return false - } - - currentDir = currentDir.parentFile.parentFile - if (currentDir.name.startsWith("@")) { - currentDir = currentDir.parentFile - } - } - - return true - } - - private fun nodeModulesDirForPackageJson(packageJson: File): File? { - var modulesDir = packageJson.parentFile.parentFile - if (modulesDir.name.startsWith("@")) { - modulesDir = modulesDir.parentFile - } - - return modulesDir.takeIf { it.name == "node_modules" } - } - /** * Construct a [Package] by parsing its _package.json_ file and - if applicable - querying additional * content via the `npm view` command. The result is a [Pair] with the raw identifier and the new package. @@ -549,17 +522,6 @@ open class Npm( ) } - private fun findDependencyModuleDir(dependencyName: String, searchModuleDirs: List): List { - searchModuleDirs.forEachIndexed { index, moduleDir -> - // Note: resolve() also works for scoped dependencies, e.g. dependencyName = "@x/y" - val dependencyModuleDir = moduleDir.resolve("node_modules/$dependencyName") - if (dependencyModuleDir.isDirectory) { - return listOf(dependencyModuleDir) + searchModuleDirs.subList(index, searchModuleDirs.size) - } - } - return emptyList() - } - private fun parseProject(packageJson: File): Project { logger.debug { "Parsing project info from '$packageJson'." } @@ -686,3 +648,41 @@ open class Npm( return ProcessCapture(workingDir, command(workingDir), subcommand, *options.toTypedArray()) } } + +private fun findDependencyModuleDir(dependencyName: String, searchModuleDirs: List): List { + searchModuleDirs.forEachIndexed { index, moduleDir -> + // Note: resolve() also works for scoped dependencies, e.g. dependencyName = "@x/y" + val dependencyModuleDir = moduleDir.resolve("node_modules/$dependencyName") + if (dependencyModuleDir.isDirectory) { + return listOf(dependencyModuleDir) + searchModuleDirs.subList(index, searchModuleDirs.size) + } + } + return emptyList() +} + +private fun isValidNodeModulesDirectory(rootModulesDir: File, modulesDir: File?): Boolean { + if (modulesDir == null) return false + + var currentDir: File = modulesDir + while (currentDir != rootModulesDir) { + if (currentDir.name != "node_modules") { + return false + } + + currentDir = currentDir.parentFile.parentFile + if (currentDir.name.startsWith("@")) { + currentDir = currentDir.parentFile + } + } + + return true +} + +private fun nodeModulesDirForPackageJson(packageJson: File): File? { + var modulesDir = packageJson.parentFile.parentFile + if (modulesDir.name.startsWith("@")) { + modulesDir = modulesDir.parentFile + } + + return modulesDir.takeIf { it.name == "node_modules" } +} From 986248c53b1f80aaeb7c3c2e9dc1d4d20d8689b1 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 31 May 2023 17:03:28 +0200 Subject: [PATCH 2/3] refactor(Npm): Make `mapLinesToIssues()` a top-level extension function Split the large function to ease maintenance and increase overview. Signed-off-by: Sebastian Schuberth --- .../node/src/main/kotlin/Npm.kt | 117 +++++++++--------- 1 file changed, 58 insertions(+), 59 deletions(-) diff --git a/plugins/package-managers/node/src/main/kotlin/Npm.kt b/plugins/package-managers/node/src/main/kotlin/Npm.kt index 86ec89756d312..731cb51acb61b 100644 --- a/plugins/package-managers/node/src/main/kotlin/Npm.kt +++ b/plugins/package-managers/node/src/main/kotlin/Npm.kt @@ -572,67 +572,13 @@ open class Npm( val lines = process.stderr.lines() val issues = mutableListOf() - fun mapLinesToIssues(prefix: String, severity: Severity) { - val ignorablePrefixes = setOf("code ", "errno ", "path ", "syscall ") - val singleLinePrefixes = setOf("deprecated ", "skipping integrity check for git dependency ") - val minSecondaryPrefixLength = 5 - - val issueLines = lines.mapNotNull { line -> - line.withoutPrefix(prefix)?.takeUnless { ignorablePrefixes.any { prefix -> it.startsWith(prefix) } } - } - - var commonPrefix: String - var previousPrefix = "" - - val collapsedLines = issueLines.distinct().fold(mutableListOf()) { messages, line -> - if (messages.isEmpty()) { - // The first line is always added including the prefix. The prefix will be removed later. - messages += line - } else { - // Find the longest common prefix that ends with space. - commonPrefix = line.commonPrefixWith(messages.last()) - if (!commonPrefix.endsWith(' ')) { - // Deal with prefixes being used on their own as separators. - commonPrefix = if ("$commonPrefix " == previousPrefix) { - "$commonPrefix " - } else { - commonPrefix.dropLastWhile { it != ' ' } - } - } - - if (commonPrefix !in singleLinePrefixes && commonPrefix.length >= minSecondaryPrefixLength) { - // Do not drop the whole prefix but keep the space when concatenating lines. - messages[messages.size - 1] += line.drop(commonPrefix.length - 1).trimEnd() - previousPrefix = commonPrefix - } else { - // Remove the prefix from previously added message start. - messages[messages.size - 1] = messages.last().removePrefix(previousPrefix).trimStart() - messages += line - } - } - - messages - } - - if (collapsedLines.isNotEmpty()) { - // Remove the prefix from the last added message start. - collapsedLines[collapsedLines.size - 1] = collapsedLines.last().removePrefix(previousPrefix).trimStart() - } - - collapsedLines.forEach { line -> - // Skip any footer as a whole. - if (line == "A complete log of this run can be found in:") return - - issues += Issue( - source = managerName, - message = line, - severity = severity - ) - } + lines.groupLines("npm WARN ").mapTo(issues) { + Issue(source = managerName, message = it, severity = Severity.WARNING) } - mapLinesToIssues("npm WARN ", Severity.WARNING) - mapLinesToIssues("npm ERR! ", Severity.ERROR) + lines.groupLines("npm ERR! ").mapTo(issues) { + Issue(source = managerName, message = it, severity = Severity.ERROR) + } return issues } @@ -686,3 +632,56 @@ private fun nodeModulesDirForPackageJson(packageJson: File): File? { return modulesDir.takeIf { it.name == "node_modules" } } + +private fun List.groupLines(prefix: String): List { + val ignorablePrefixes = setOf("code ", "errno ", "path ", "syscall ") + val singleLinePrefixes = setOf("deprecated ", "skipping integrity check for git dependency ") + val minSecondaryPrefixLength = 5 + + val issueLines = mapNotNull { line -> + line.withoutPrefix(prefix)?.takeUnless { ignorablePrefixes.any { prefix -> it.startsWith(prefix) } } + } + + var commonPrefix: String + var previousPrefix = "" + + val collapsedLines = issueLines.distinct().fold(mutableListOf()) { messages, line -> + if (messages.isEmpty()) { + // The first line is always added including the prefix. The prefix will be removed later. + messages += line + } else { + // Find the longest common prefix that ends with space. + commonPrefix = line.commonPrefixWith(messages.last()) + if (!commonPrefix.endsWith(' ')) { + // Deal with prefixes being used on their own as separators. + commonPrefix = if ("$commonPrefix " == previousPrefix) { + "$commonPrefix " + } else { + commonPrefix.dropLastWhile { it != ' ' } + } + } + + if (commonPrefix !in singleLinePrefixes && commonPrefix.length >= minSecondaryPrefixLength) { + // Do not drop the whole prefix but keep the space when concatenating lines. + messages[messages.size - 1] += line.drop(commonPrefix.length - 1).trimEnd() + previousPrefix = commonPrefix + } else { + // Remove the prefix from previously added message start. + messages[messages.size - 1] = messages.last().removePrefix(previousPrefix).trimStart() + messages += line + } + } + + messages + } + + if (collapsedLines.isNotEmpty()) { + // Remove the prefix from the last added message start. + collapsedLines[collapsedLines.size - 1] = collapsedLines.last().removePrefix(previousPrefix).trimStart() + } + + return collapsedLines.takeWhile { + // Skip any footer as a whole. + it != "A complete log of this run can be found in:" + } +} From a8e43bfd8741b896ecdf896f4a41ee1756a9b2d7 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 31 May 2023 17:07:26 +0200 Subject: [PATCH 3/3] refactor(Npm): Rename a few `groupLines()` variables for clarity Signed-off-by: Sebastian Schuberth --- plugins/package-managers/node/src/main/kotlin/Npm.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/package-managers/node/src/main/kotlin/Npm.kt b/plugins/package-managers/node/src/main/kotlin/Npm.kt index 731cb51acb61b..7dc2ef26d0dff 100644 --- a/plugins/package-managers/node/src/main/kotlin/Npm.kt +++ b/plugins/package-managers/node/src/main/kotlin/Npm.kt @@ -633,13 +633,13 @@ private fun nodeModulesDirForPackageJson(packageJson: File): File? { return modulesDir.takeIf { it.name == "node_modules" } } -private fun List.groupLines(prefix: String): List { - val ignorablePrefixes = setOf("code ", "errno ", "path ", "syscall ") +private fun List.groupLines(marker: String): List { + val ignorableLinePrefixes = setOf("code ", "errno ", "path ", "syscall ") val singleLinePrefixes = setOf("deprecated ", "skipping integrity check for git dependency ") - val minSecondaryPrefixLength = 5 + val minCommonPrefixLength = 5 val issueLines = mapNotNull { line -> - line.withoutPrefix(prefix)?.takeUnless { ignorablePrefixes.any { prefix -> it.startsWith(prefix) } } + line.withoutPrefix(marker)?.takeUnless { ignorableLinePrefixes.any { prefix -> it.startsWith(prefix) } } } var commonPrefix: String @@ -661,7 +661,7 @@ private fun List.groupLines(prefix: String): List { } } - if (commonPrefix !in singleLinePrefixes && commonPrefix.length >= minSecondaryPrefixLength) { + if (commonPrefix !in singleLinePrefixes && commonPrefix.length >= minCommonPrefixLength) { // Do not drop the whole prefix but keep the space when concatenating lines. messages[messages.size - 1] += line.drop(commonPrefix.length - 1).trimEnd() previousPrefix = commonPrefix