fix(node): Default to NPM if there is no indication for any Node manager

This restores the previous behavior before f99e2ed. Slightly improve some code comments while at it. Signed-off-by: Sebastian Schuberth <[email protected]>
oss-review-toolkit · Dec 6, 2023 · 849f987 · 849f987
1 parent f898d75
commit 849f987
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 11 deletions.
diff --git a/plugins/package-managers/node/src/main/kotlin/utils/NpmDetection.kt b/plugins/package-managers/node/src/main/kotlin/utils/NpmDetection.kt
@@ -72,7 +72,7 @@ class NpmDetection(private val definitionFiles: Collection<File>) {
         definitionFiles.filter { file ->
             val projectDir = file.parentFile
 
-            // Try to clearly determine the package manager from specific files.
+            // Try to clearly determine the package manager from files specific to it.
             val managersFromFiles = projectDirManagers[projectDir].orEmpty()
             when {
                 manager !in managersFromFiles -> return@filter false
@@ -82,26 +82,25 @@ class NpmDetection(private val definitionFiles: Collection<File>) {
                 }
             }
 
-            // Fall back to determining the package manager from workspaces.
+            // There is ambiguity when only looking at the files, so also look at any workspaces to clearly determine
+            // the package manager.
             val managersFromWorkspaces = getWorkspaceRoots(projectDir).mapNotNull {
                 projectDirManagers[it]
             }.flatten()
 
-            when {
-                manager !in managersFromWorkspaces -> return@filter false
-                managersFromWorkspaces.size == 1 -> {
-                    logger.info { "Skipping '$file' as it is part of an implicitly handled $manager workspace." }
-                    return@filter false
+            if (managersFromWorkspaces.isNotEmpty()) {
+                logger.info {
+                    "Skipping '$file' as it is part of a workspace implicitly handled by $managersFromWorkspaces."
                 }
+                return@filter false
             }
 
+            // Looking at the workspaces did not bring any clarity, so assume the package manager is NPM.
             logger.warn {
-                val managers = managersFromFiles + managersFromWorkspaces
-                "Ignoring '$file' directory as the package manager is unclear. It could be any of " +
-                    "${managers.joinToString()}."
+                "Any of $managersFromFiles could be the package manager for '$file'. Assuming it is an NPM project."
             }
 
-            false
+            manager == NodePackageManager.NPM
         }
 }
 

diff --git a/plugins/package-managers/node/src/test/kotlin/utils/NpmDetectionTest.kt b/plugins/package-managers/node/src/test/kotlin/utils/NpmDetectionTest.kt
@@ -158,6 +158,7 @@ class NpmDetectionTest : WordSpec({
             val filteredFiles = NpmDetection(definitionFiles).filterApplicable(NPM)
 
             filteredFiles.map { it.toRelativeString(projectDir) } shouldContainExactlyInAnyOrder listOf(
+                "npm/no-lockfile/package.json",
                 "npm/node-modules/package.json",
                 "npm/package-lock/package.json",
                 "npm/shrinkwrap/package.json",