fix: pull in fix logic from normalize-package-data

This brings in the logic from `normalize-package-data` to be in this repo
instead.  It doesn't bring all of the logic, just the steps involved
with "fix".

There was some re-duplication of `bundleDependencies` that
`normalize-package-data` was doing that has been removed.  It was also
completely re-implementing the script fixing, which we already call as
part of "fix" so that was dropped.

lib/index.js

@@ -42,9 +42,7 @@ class PackageJson {
     'fixNameField',
     'fixVersionField',
     'fixRepositoryField',
-    'fixBinField',
     'fixDependencies',
-    'fixScriptsField',
     'devDependencies',
     'scriptpath',
   ])

lib/normalize.js

@@ -1,3 +1,4 @@
+const semver = require('semver')
 const fs = require('fs/promises')
 const { glob } = require('glob')
 const normalizePackageBin = require('npm-normalize-package-bin')
@@ -6,6 +7,27 @@ const legacyMakeWarning = require('normalize-package-data/lib/make_warning.js')
 const path = require('path')
 const log = require('proc-log')
 const git = require('@npmcli/git')
+const hostedGitInfo = require('hosted-git-info')
+
+function isCorrectlyEncodedName (spec) {
+  return !spec.match(/[/@\s+%:]/) &&
+    spec === encodeURIComponent(spec)
+}
+
+function isValidScopedPackageName (spec) {
+  if (spec.charAt(0) !== '@') {
+    return false
+  }
+
+  const rest = spec.slice(1).split('/')
+  if (rest.length !== 2) {
+    return false
+  }
+
+  return rest[0] && rest[1] &&
+    rest[0] === encodeURIComponent(rest[0]) &&
+    rest[1] === encodeURIComponent(rest[1])
+}
 
 // We don't want the `changes` array in here by default because this is a hot
 // path for parsing packuments during install.  So the calling method passes it
@@ -24,11 +46,47 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
 
   // name and version are load bearing so we have to clean them up first
   if (steps.includes('fixNameField') || steps.includes('normalizeData')) {
-    legacyFixer.fixNameField(data, { strict, allowLegacyCase })
+    if (!data.name && !strict) {
+      changes?.push('Missing "name" field was set to an empty string')
+      data.name = ''
+    } else {
+      if (typeof data.name !== 'string') {
+        throw new Error('name field must be a string.')
+      }
+      if (!strict) {
+        const name = data.name.trim()
+        if (data.name !== name) {
+          changes?.push(`Whitespace was trimmed from "name"`)
+          data.name = name
+        }
+      }
+
+      if (data.name.startsWith('.') ||
+        !(isValidScopedPackageName(data.name) || isCorrectlyEncodedName(data.name)) ||
+        (strict && (!allowLegacyCase) && data.name !== data.name.toLowerCase()) ||
+        data.name.toLowerCase() === 'node_modules' ||
+        data.name.toLowerCase() === 'favicon.ico') {
+        throw new Error('Invalid name: ' + JSON.stringify(data.name))
+      }
+    }
   }
 
   if (steps.includes('fixVersionField') || steps.includes('normalizeData')) {
-    legacyFixer.fixVersionField(data, strict)
+    // allow "loose" semver 1.0 versions in non-strict mode
+    // enforce strict semver 2.0 compliance in strict mode
+    const loose = !strict
+    if (!data.version) {
+      data.version = ''
+    } else {
+      if (!semver.valid(data.version, !loose)) {
+        throw new Error(`Invalid version: "${data.version}"`)
+      }
+      const version = semver.clean(data.version, !loose)
+      if (version !== data.version) {
+        changes?.push(`"version" was cleaned and set to "${version}"`)
+        data.version = version
+      }
+    }
   }
   // remove attributes that start with "_"
   if (steps.includes('_attributes')) {
@@ -84,11 +142,11 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
     if (data.dependencies &&
       data.optionalDependencies && typeof data.optionalDependencies === 'object') {
       for (const name in data.optionalDependencies) {
-        changes?.push(`optionalDependencies entry "${name}" was removed`)
+        changes?.push(`optionalDependencies."${name}" was removed`)
         delete data.dependencies[name]
       }
       if (!Object.keys(data.dependencies).length) {
-        changes?.push(`empty "optionalDependencies" was removed`)
+        changes?.push(`Empty "optionalDependencies" was removed`)
         delete data.dependencies
       }
     }
@@ -121,20 +179,21 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
   }
 
   // strip "node_modules/.bin" from scripts entries
+  // remove invalid scripts entries (non-strings)
   if (steps.includes('scripts') || steps.includes('scriptpath')) {
     const spre = /^(\.[/\\])?node_modules[/\\].bin[\\/]/
     if (typeof data.scripts === 'object') {
       for (const name in data.scripts) {
         if (typeof data.scripts[name] !== 'string') {
           delete data.scripts[name]
-          changes?.push(`invalid scripts entry "${name}" was removed`)
+          changes?.push(`Invalid scripts."${name}" was removed`)
         } else if (steps.includes('scriptpath')) {
           data.scripts[name] = data.scripts[name].replace(spre, '')
           changes?.push(`scripts entry "${name}" was fixed to remove node_modules/.bin reference`)
         }
       }
     } else {
-      changes?.push(`removed invalid "scripts"`)
+      changes?.push(`Removed invalid "scripts"`)
       delete data.scripts
     }
   }
@@ -320,19 +379,89 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
 
   // Some steps are isolated so we can do a limited subset of these in `fix`
   if (steps.includes('fixRepositoryField') || steps.includes('normalizeData')) {
-    legacyFixer.fixRepositoryField(data)
-  }
-
-  if (steps.includes('fixBinField') || steps.includes('normalizeData')) {
-    legacyFixer.fixBinField(data)
+    if (data.repositories) {
+      /* eslint-disable-next-line max-len */
+      changes?.push(`"repository" was set to the first entry in "repositories" (${data.repository})`)
+      data.repository = data.repositories[0]
+    }
+    if (data.repository) {
+      if (typeof data.repository === 'string') {
+        changes?.push('"repository" was changed from a string to an object')
+        data.repository = {
+          type: 'git',
+          url: data.repository,
+        }
+      }
+      if (data.repository.url) {
+        const hosted = hostedGitInfo.fromUrl(data.repository.url)
+        let r
+        if (hosted) {
+          if (hosted.getDefaultRepresentation() === 'shortcut') {
+            r = hosted.https()
+          } else {
+            r = hosted.toString()
+          }
+          if (r !== data.repository.url) {
+            changes?.push(`"repository.url" was normalized to "${r}"`)
+            data.repository.url = r
+          }
+        }
+      }
+    }
   }
 
   if (steps.includes('fixDependencies') || steps.includes('normalizeData')) {
-    legacyFixer.fixDependencies(data, strict)
-  }
+    // peerDependencies?
+    // devDependencies is meaningless here, it's ignored on an installed package
+    for (const type of ['dependencies', 'devDependencies', 'optionalDependencies']) {
+      if (data[type]) {
+        let secondWarning = true
+        if (typeof data[type] === 'string') {
+          changes?.push(`"${type}" was converted from a string into an object`)
+          data[type] = data[type].trim().split(/[\n\r\s\t ,]+/)
+          secondWarning = false
+        }
+        if (Array.isArray(data[type])) {
+          if (secondWarning) {
+            changes?.push(`"${type}" was converted from an array into an object`)
+          }
+          const o = {}
+          for (const d of data[type]) {
+            if (typeof d === 'string') {
+              const dep = d.trim().split(/(:?[@\s><=])/)
+              const dn = dep.shift()
+              const dv = dep.join('').replace(/^@/, '').trim()
+              o[dn] = dv
+            }
+          }
+          data[type] = o
+        }
+      }
+    }
+    // normalize-package-data used to put optional dependencies BACK into
+    // dependencies here, we no longer do this
 
-  if (steps.includes('fixScriptsField') || steps.includes('normalizeData')) {
-    legacyFixer.fixScriptsField(data)
+    for (const deps of ['dependencies', 'devDependencies']) {
+      if (deps in data) {
+        if (!data[deps] || typeof data[deps] !== 'object') {
+          changes?.push(`Removed invalid "${deps}"`)
+          delete data[deps]
+        } else {
+          for (const d in data[deps]) {
+            const r = data[deps][d]
+            if (typeof r !== 'string') {
+              changes?.push(`Removed invalid "${deps}.${d}"`)
+              delete data[deps][d]
+            }
+            const hosted = hostedGitInfo.fromUrl(data[deps][d])?.toString()
+            if (hosted && hosted !== data[deps][d]) {
+              changes?.push(`Normalized git reference to "${deps}.${d}"`)
+              data[deps][d] = hosted.toString()
+            }
+          }
+        }
+      }
+    }
   }
 
   if (steps.includes('normalizeData')) {

tap-snapshots/test/fix.js.test.cjs

@@ -0,0 +1,118 @@
+/* IMPORTANT
+ * This snapshot file is auto-generated, but designed for humans.
+ * It should be checked into source control and tracked carefully.
+ * Re-generate by setting TAP_SNAPSHOT=1 and running tests.
+ * Make sure to inspect the output below.  Do not ignore changes!
+ */
+'use strict'
+exports[`test/fix.js TAP with changes binRefs scoped name > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+]
+`
+
+exports[`test/fix.js TAP with changes fixDependencies array dependencies > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+  "\\"dependencies\\" was converted from an array into an object",
+]
+`
+
+exports[`test/fix.js TAP with changes fixDependencies false dependencies > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+  "Removed invalid \\"dependencies\\"",
+]
+`
+
+exports[`test/fix.js TAP with changes fixDependencies git dependency > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+  "Normalized git reference to \\"dependencies.npm\\"",
+]
+`
+
+exports[`test/fix.js TAP with changes fixDependencies non-string dependency > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+  "Removed invalid \\"dependencies.npm\\"",
+]
+`
+
+exports[`test/fix.js TAP with changes fixDependencies string dependencies > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+  "\\"dependencies\\" was converted from a string into an object",
+]
+`
+
+exports[`test/fix.js TAP with changes fixNameField scoped whitespace > must match snapshot 1`] = `
+Array [
+  "Whitespace was trimmed from \\"name\\"",
+  "\\"bundleDependencies\\" was removed",
+]
+`
+
+exports[`test/fix.js TAP with changes fixNameField unscoped whitespace > must match snapshot 1`] = `
+Array [
+  "Whitespace was trimmed from \\"name\\"",
+  "\\"bundleDependencies\\" was removed",
+]
+`
+
+exports[`test/fix.js TAP with changes fixRepositoryField full > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+  "\\"repository\\" was changed from a string to an object",
+  "\\"repository.url\\" was normalized to \\"git+https://github.com/npm/cli.git\\"",
+]
+`
+
+exports[`test/fix.js TAP with changes fixRepositoryField object no url > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+]
+`
+
+exports[`test/fix.js TAP with changes fixRepositoryField repositories array > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+  "\\"repository\\" was set to the first entry in \\"repositories\\" ([object Object])",
+  "\\"repository\\" was changed from a string to an object",
+]
+`
+
+exports[`test/fix.js TAP with changes fixRepositoryField shortcut > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+  "\\"repository\\" was changed from a string to an object",
+  "\\"repository.url\\" was normalized to \\"git+https://github.com/npm/cli.git\\"",
+]
+`
+
+exports[`test/fix.js TAP with changes fixVersionField none > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+]
+`
+
+exports[`test/fix.js TAP with changes fixVersionField unclean > must match snapshot 1`] = `
+Array [
+  "\\"version\\" was cleaned and set to \\"1.0.0\\"",
+  "\\"bundleDependencies\\" was removed",
+]
+`
+
+exports[`test/fix.js TAP with changes scriptpath non-object script entry > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+  "Invalid scripts.\\"test\\" was removed",
+]
+`
+
+exports[`test/fix.js TAP with changes scriptpath non-object scripts > must match snapshot 1`] = `
+Array [
+  "\\"bundleDependencies\\" was removed",
+  "Removed invalid \\"scripts\\"",
+]
+`

tap-snapshots/test/index.js.test.cjs

@@ -30,7 +30,7 @@ exports[`test/index.js TAP load custom formatting > should save back custom form
 {"name":"foo","version":"1.0.1","description":"Lorem ipsum dolor"}
 `
 
-exports[`test/index.js TAP load read, update content and write > should properly save contennt to a package.json 1`] = `
+exports[`test/index.js TAP load read, update content and write > should properly save content to a package.json 1`] = `
 {
         "name": "foo",
         "version": "1.0.1",