fix: ensure that npm dependencies retain their "production" grouping

pkg/lockfile/fixtures/npm/same-package-different-groups.v1.json

@@ -0,0 +1,81 @@
+{
+ "requires": true,
+ "lockfileVersion": 1,
+ "dependencies": {
+ "ajv": {
+ "version": "5.5.2",
+ "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
+ "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
+ "dev": true,
+ "requires": {
+ "co": "^4.6.0",
+ "fast-deep-equal": "^1.0.0",
+ "fast-json-stable-stringify": "^2.0.0",
+ "json-schema-traverse": "^0.3.0"
+ }
+ },
+ "eslint": {
+ "version": "1.2.3",
+ "dev": true,
+ "dependencies": {
+ "ajv": {
+ "version": "5.5.2",
+ "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
+ "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
+ "dev": true,
+ "requires": {
+ "co": "^4.6.0",
+ "fast-deep-equal": "^1.0.0",
+ "fast-json-stable-stringify": "^2.0.0",
+ "json-schema-traverse": "^0.3.0"
+ },
+ "dependencies": {
+ "ajv": {
+ "version": "5.5.2",
+ "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
+ "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
+ "optional": true,
+ "requires": {
+ "co": "^4.6.0",
+ "fast-deep-equal": "^1.0.0",
+ "fast-json-stable-stringify": "^2.0.0",
+ "json-schema-traverse": "^0.3.0"
+ }
+ }
+ }
+ }
+ }
+ },
+ "table": {
+ "version": "1.0.0",
+ "dependencies": {
+ "ajv": {
+ "version": "5.5.2",
+ "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
+ "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
+ "requires": {
+ "co": "^4.6.0",
+ "fast-deep-equal": "^1.0.0",
+ "fast-json-stable-stringify": "^2.0.0",
+ "json-schema-traverse": "^0.3.0"
+ },
+ "dependencies": {
+ "ajv": {
+ "version": "5.5.2",
+ "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
+ "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
+ "dev": true,
+ "optional": true,
+ "requires": {
+ "co": "^4.6.0",
+ "fast-deep-equal": "^1.0.0",
+ "fast-json-stable-stringify": "^2.0.0",
+ "json-schema-traverse": "^0.3.0"
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+}

pkg/lockfile/fixtures/npm/same-package-different-groups.v2.json

@@ -0,0 +1,78 @@
+{
+ "name": "my-library",
+ "lockfileVersion": 2,
+ "requires": true,
+ "packages": {
+ "": {
+ "dependencies": {},
+ "devDependencies": {}
+ },
+ "node_modules/ajv": {
+ "version": "5.5.2",
+ "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
+ "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
+ "dev": true,
+ "dependencies": {
+ "co": "^4.6.0",
+ "fast-deep-equal": "^1.0.0",
+ "fast-json-stable-stringify": "^2.0.0",
+ "json-schema-traverse": "^0.3.0"
+ }
+ },
+ "node_modules/eslint": {
+ "version": "1.2.3",
+ "dev": true
+ },
+ "node_modules/eslint/node_modules/ajv": {
+ "version": "5.5.2",
+ "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
+ "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
+ "dev": true,
+ "dependencies": {
+ "co": "^4.6.0",
+ "fast-deep-equal": "^1.0.0",
+ "fast-json-stable-stringify": "^2.0.0",
+ "json-schema-traverse": "^0.3.0"
+ }
+ },
+ "node_modules/eslint/node_modules/ajv/node_modules/ajv": {
+ "version": "5.5.2",
+ "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
+ "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
+ "optional": true,
+ "dependencies": {
+ "co": "^4.6.0",
+ "fast-deep-equal": "^1.0.0",
+ "fast-json-stable-stringify": "^2.0.0",
+ "json-schema-traverse": "^0.3.0"
+ }
+ },
+ "node_modules/table": {
+ "version": "1.0.0"
+ },
+ "node_modules/table/node_modules/ajv": {
+ "version": "5.5.2",
+ "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
+ "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
+ "dependencies": {
+ "co": "^4.6.0",
+ "fast-deep-equal": "^1.0.0",
+ "fast-json-stable-stringify": "^2.0.0",
+ "json-schema-traverse": "^0.3.0"
+ }
+ },
+ "node_modules/table/node_modules/ajv/node_modules/ajv": {
+ "version": "5.5.2",
+ "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
+ "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
+ "devOptional": true,
+ "dependencies": {
+ "co": "^4.6.0",
+ "fast-deep-equal": "^1.0.0",
+ "fast-json-stable-stringify": "^2.0.0",
+ "json-schema-traverse": "^0.3.0"
+ }
+ }
+ },
+ "dependencies": {}
+}

pkg/lockfile/parse-npm-lock-v1_test.go

@@ -415,3 +415,36 @@ func TestParseNpmLock_v1_OptionalPackage(t *testing.T) {
  },
  })
 }
+
+func TestParseNpmLock_v1_SamePackageDifferentGroups(t *testing.T) {
+ t.Parallel()
+
+ packages, err := lockfile.ParseNpmLock("fixtures/npm/same-package-different-groups.v1.json")
+
+ if err != nil {
+ t.Errorf("Got unexpected error: %v", err)
+ }
+
+ expectPackages(t, packages, []lockfile.PackageDetails{
+ {
+ Name: "eslint",
+ Version: "1.2.3",
+ Ecosystem: lockfile.NpmEcosystem,
+ CompareAs: lockfile.NpmEcosystem,
+ DepGroups: []string{"dev"},
+ },
+ {
+ Name: "table",
+ Version: "1.0.0",
+ Ecosystem: lockfile.NpmEcosystem,
+ CompareAs: lockfile.NpmEcosystem,
+ DepGroups: nil,
+ },
+ {
+ Name: "ajv",
+ Version: "5.5.2",
+ Ecosystem: lockfile.NpmEcosystem,
+ CompareAs: lockfile.NpmEcosystem,
+ },
+ })
+}

pkg/lockfile/parse-npm-lock-v2_test.go

@@ -409,3 +409,35 @@ func TestParseNpmLock_v2_OptionalPackage(t *testing.T) {
  },
  })
 }
+
+func TestParseNpmLock_v2_SamePackageDifferentGroups(t *testing.T) {
+ t.Parallel()
+
+ packages, err := lockfile.ParseNpmLock("fixtures/npm/same-package-different-groups.v2.json")
+
+ if err != nil {
+ t.Errorf("Got unexpected error: %v", err)
+ }
+
+ expectPackages(t, packages, []lockfile.PackageDetails{
+ {
+ Name: "eslint",
+ Version: "1.2.3",
+ Ecosystem: lockfile.NpmEcosystem,
+ CompareAs: lockfile.NpmEcosystem,
+ DepGroups: []string{"dev"},
+ },
+ {
+ Name: "table",
+ Version: "1.0.0",
+ Ecosystem: lockfile.NpmEcosystem,
+ CompareAs: lockfile.NpmEcosystem,
+ },
+ {
+ Name: "ajv",
+ Version: "5.5.2",
+ Ecosystem: lockfile.NpmEcosystem,
+ CompareAs: lockfile.NpmEcosystem,
+ },
+ })
+}

pkg/lockfile/parse-npm-lock.go

@@ -8,6 +8,7 @@ import (
  "strings"
 
  "golang.org/x/exp/maps"
+ "golang.org/x/exp/slices"
 )
 
 type NpmLockDependency struct {
@@ -49,6 +50,39 @@ type NpmLockfile struct {
 
 const NpmEcosystem Ecosystem = "npm"
 
+type npmPackageDetailsMap map[string]PackageDetails
+
+// mergeNpmDepsGroups handles merging the dependency groups of packages within the
+// NPM ecosystem, since they can appear multiple times in the same dependency tree
+//
+// the merge happens almost as you'd expect, except that if either given packages
+// belong to no groups, then that is the result since it indicates the package
+// is implicitly a production dependency.
+func mergeNpmDepsGroups(a, b PackageDetails) []string {
+ // if either group includes no groups, then the package is in the "production" group
+ if len(a.DepGroups) == 0 || len(b.DepGroups) == 0 {
+ return nil
+ }
+
+ combined := make([]string, 0, len(a.DepGroups)+len(b.DepGroups))
+ combined = append(combined, a.DepGroups...)
+ combined = append(combined, b.DepGroups...)
+
+ slices.Sort(combined)
+
+ return slices.Compact(combined)
+}
+
+func (pdm npmPackageDetailsMap) add(key string, details PackageDetails) {
+ existing, ok := pdm[key]
+
+ if ok {
+ details.DepGroups = mergeNpmDepsGroups(existing, details)
+ }
+
+ pdm[key] = details
+}
+
 func (dep NpmLockDependency) depGroups() []string {
  if dep.Dev && dep.Optional {
  return []string{"dev", "optional"}
@@ -64,7 +98,7 @@ func (dep NpmLockDependency) depGroups() []string {
 }
 
 func parseNpmLockDependencies(dependencies map[string]NpmLockDependency) map[string]PackageDetails {
- details := map[string]PackageDetails{}
+ details := npmPackageDetailsMap{}
 
  for name, detail := range dependencies {
  if detail.Dependencies != nil {
@@ -98,14 +132,14 @@ func parseNpmLockDependencies(dependencies map[string]NpmLockDependency) map[str
  }
  }
 
- details[name+"@"+version] = PackageDetails{
+ details.add(name+"@"+version, PackageDetails{
  Name: name,
  Version: finalVersion,
  Ecosystem: NpmEcosystem,
  CompareAs: NpmEcosystem,
  Commit: commit,
  DepGroups: detail.depGroups(),
- }
+ })
  }
 
  return details
@@ -137,7 +171,7 @@ func (pkg NpmLockPackage) depGroups() []string {
 }
 
 func parseNpmLockPackages(packages map[string]NpmLockPackage) map[string]PackageDetails {
- details := map[string]PackageDetails{}
+ details := npmPackageDetailsMap{}
 
  for namePath, detail := range packages {
  if namePath == "" {
@@ -159,14 +193,14 @@ func parseNpmLockPackages(packages map[string]NpmLockPackage) map[string]Package
  finalVersion = commit
  }
 
- details[finalName+"@"+finalVersion] = PackageDetails{
+ details.add(finalName+"@"+finalVersion, PackageDetails{
  Name: finalName,
  Version: detail.Version,
  Ecosystem: NpmEcosystem,
  CompareAs: NpmEcosystem,
  Commit: commit,
  DepGroups: detail.depGroups(),
- }
+ })
  }
 
  return details

pkg/lockfile/parse-nuget-lock.go

@@ -43,7 +43,9 @@ func parseNuGetLock(lockfile NuGetLockfile) ([]PackageDetails, error) {
  // its dependencies, there might be different or duplicate dependencies
  // between frameworks
  for _, dependencies := range lockfile.Dependencies {
- maps.Copy(details, parseNuGetLockDependencies(dependencies))
+ for name, detail := range parseNuGetLockDependencies(dependencies) {
+ details[name] = detail
+ }
  }
 
  return maps.Values(details), nil