diff --git a/lib/commands/install.js b/lib/commands/install.js index 71f4229bb2566..72dcbe547eb8d 100644 --- a/lib/commands/install.js +++ b/lib/commands/install.js @@ -6,6 +6,7 @@ const pacote = require('pacote') const checks = require('npm-install-checks') const reifyFinish = require('../utils/reify-finish.js') const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const validateProjectStructure = require('../utils/validate-project.js') class Install extends ArboristWorkspaceCmd { static description = 'Install a package' @@ -104,6 +105,21 @@ class Install extends ArboristWorkspaceCmd { const forced = this.npm.config.get('force') const scriptShell = this.npm.config.get('script-shell') || undefined + // Add validation for non-global installs with no args + if (!isGlobalInstall && args.length === 0) { + try { + validateProjectStructure(this.npm.prefix) + } catch (err) { + if (err.code === 'ENOPROJECT') { + throw Object.assign( + new Error(err.message), + { code: err.code } + ) + } + throw err + } + } + // be very strict about engines when trying to update npm itself const npmInstall = args.find(arg => arg.startsWith('npm@') || arg === 'npm') if (isGlobalInstall && npmInstall) { diff --git a/lib/commands/uninstall.js b/lib/commands/uninstall.js index f9baebe3bc2e2..d076c4811e983 100644 --- a/lib/commands/uninstall.js +++ b/lib/commands/uninstall.js @@ -3,6 +3,7 @@ const pkgJson = require('@npmcli/package-json') const reifyFinish = require('../utils/reify-finish.js') const completion = require('../utils/installed-shallow.js') const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const validateProjectStructure = require('../utils/validate-project.js') class Uninstall extends ArboristWorkspaceCmd { static description = 'Remove a package' @@ -18,6 +19,21 @@ class Uninstall extends ArboristWorkspaceCmd { } async exec (args) { + // Add validation for non-global uninstalls + if (!this.npm.global) { + try { + validateProjectStructure(this.npm.prefix) + } catch (err) { + if (err.code === 'ENOPROJECT') { + throw Object.assign( + new Error(err.message), + { code: err.code } + ) + } + throw err + } + } + if (!args.length) { if (!this.npm.global) { throw new Error('Must provide a package name to remove') diff --git a/lib/utils/validate-project.js b/lib/utils/validate-project.js new file mode 100644 index 0000000000000..6a3c535de576e --- /dev/null +++ b/lib/utils/validate-project.js @@ -0,0 +1,26 @@ +const fs = require('node:fs') +const path = require('node:path') + +// Validates that a package.json exists in the target directory +function validateProjectStructure (prefix) { + const projectPath = prefix || process.cwd() + const packageJsonPath = path.join(projectPath, 'package.json') + + // Check if directory exists when --prefix is used + if (prefix && !fs.existsSync(projectPath)) { + const err = new Error(`Dir "${projectPath}" does not exist. Run "npm init" first.`) + err.code = 'ENOPROJECT' + throw err + } + + // Check for package.json + if (!fs.existsSync(packageJsonPath)) { + const err = new Error('No package.json found. Run "npm init" to create a new package.') + err.code = 'ENOPROJECT' + throw err + } + + return true +} + +module.exports = validateProjectStructure diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js index a4d9c06129ec0..f7ea4cdcaabc8 100644 --- a/test/lib/commands/install.js +++ b/test/lib/commands/install.js @@ -717,3 +717,47 @@ t.test('devEngines', async t => { t.ok(!output.includes('EBADDEVENGINES')) }) }) + +t.test('package.json validation error handling', async t => { + await t.test('handles ENOPROJECT errors', async t => { + const { npm } = await loadMockNpm(t, { + prefixDir: {}, + mocks: { + '{LIB}/utils/validate-project.js': () => { + const err = new Error('Custom error') + err.code = 'ENOPROJECT' + throw err + }, + }, + }) + await t.rejects( + npm.exec('install', []), + { + code: 'ENOPROJECT', + message: 'Custom error', + }, + 'should preserve error code and message' + ) + }) + + await t.test('handles non-ENOPROJECT errors', async t => { + const { npm } = await loadMockNpm(t, { + prefixDir: {}, + mocks: { + '{LIB}/utils/validate-project.js': () => { + const err = new Error('Different error') + err.code = 'EDIFFERENT' + throw err + }, + }, + }) + await t.rejects( + npm.exec('install', []), + { + code: 'EDIFFERENT', + message: 'Different error', + }, + 'should throw through other errors unchanged' + ) + }) +}) diff --git a/test/lib/commands/uninstall.js b/test/lib/commands/uninstall.js index a1ef1745d0f55..ef6be4f0904dd 100644 --- a/test/lib/commands/uninstall.js +++ b/test/lib/commands/uninstall.js @@ -86,6 +86,14 @@ t.test('remove single installed lib', async t => { t.test('remove multiple installed libs', async t => { const { uninstall, prefix } = await mockNpm(t, { prefixDir: { + 'package.json': JSON.stringify({ + name: 'test-rm-multiple-lib', + version: '1.0.0', + dependencies: { + a: '*', + b: '*', + }, + }), node_modules: { a: { 'package.json': JSON.stringify({ @@ -139,18 +147,8 @@ t.test('remove multiple installed libs', async t => { await uninstall(['b']) - t.throws(() => fs.statSync(a), 'should have removed a package from nm') t.throws(() => fs.statSync(b), 'should have removed b package from nm') -}) - -t.test('no args local', async t => { - const { uninstall } = await mockNpm(t) - - await t.rejects( - uninstall([]), - /Must provide a package name to remove/, - 'should throw package name required error' - ) + t.ok(fs.statSync(a), 'should not have removed a package from nm') }) t.test('no args global', async t => { @@ -200,3 +198,87 @@ t.test('non ENOENT error reading from localPrefix package.json', async t => { 'should throw non ENOENT error' ) }) + +t.test('package.json validation', async t => { + await t.test('no package.json in local uninstall', async t => { + const { uninstall } = await mockNpm(t, { + prefixDir: {}, // empty directory + }) + await t.rejects( + uninstall(['some-package']), + { + code: 'ENOPROJECT', + message: 'No package.json found. Run "npm init" to create a new package.', + }, + 'should throw ENOPROJECT error' + ) + }) + + await t.test('no package.json in local uninstall', async t => { + const { uninstall } = await mockNpm(t, { + prefixDir: {}, // empty directory + }) + await t.rejects( + uninstall(['some-package']), + { + code: 'ENOPROJECT', + message: 'No package.json found. Run "npm init" to create a new package.', + } + ) + }) +}) + +t.test('validation error handling', async t => { + const { uninstall } = await mockNpm(t, { + prefixDir: {}, + mocks: { + '{LIB}/utils/validate-project.js': () => { + throw new Error('Generic error') + }, + }, + }) + await t.rejects( + uninstall(['some-package']), + /Generic error/, + 'should throw through non-ENOPROJECT errors' + ) +}) + +t.test('no args with package name validation', async t => { + const { uninstall } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'test-pkg', + version: '1.0.0', + }), + }, + }) + await t.rejects( + uninstall([]), + { + message: 'Must provide a package name to remove', + }, + 'should throw correct error message' + ) +}) + +t.test('handles non-ENOPROJECT validation errors', async t => { + const { uninstall } = await mockNpm(t, { + prefixDir: {}, + mocks: { + '{LIB}/utils/validate-project.js': () => { + const err = new Error('Different error') + err.code = 'EDIFFERENT' + throw err + }, + }, + }) + await t.rejects( + uninstall(['some-package']), + { + code: 'EDIFFERENT', + message: 'Different error', + }, + 'should throw through other errors unchanged' + ) +}) diff --git a/test/lib/commands/validate-project.js b/test/lib/commands/validate-project.js new file mode 100644 index 0000000000000..bc0d175885d12 --- /dev/null +++ b/test/lib/commands/validate-project.js @@ -0,0 +1,47 @@ +const t = require('tap') + +// Mock fs.existsSync to control file existence checks +const mockFs = { + existsSync: () => true, +} + +const validate = t.mock('../../../lib/utils/validate-project.js', { + 'node:fs': mockFs, +}) + +t.test('validate project structure', async t => { + t.test('returns true when package.json exists', async t => { + mockFs.existsSync = () => true + t.equal(validate('/some/path'), true, 'should validate successfully') + }) + + t.test('uses cwd() when no prefix provided', async t => { + mockFs.existsSync = () => true + t.equal(validate(), true, 'should validate successfully with default path') + }) + + t.test('throws ENOPROJECT when directory does not exist', async t => { + mockFs.existsSync = () => false + t.throws( + () => validate('/non-existent-dir'), + { + code: 'ENOPROJECT', + message: 'Dir "/non-existent-dir" does not exist. Run "npm init" to begin.', + }, + 'should throw correct error for missing directory' + ) + }) + + t.test('throws ENOPROJECT when package.json is missing', async t => { + // Directory exists but package.json doesn't + mockFs.existsSync = (p) => !p.endsWith('package.json') + t.throws( + () => validate('/some/path'), + { + code: 'ENOPROJECT', + message: 'No package.json found. Run "npm init" to create a new package.', + }, + 'should throw correct error for missing package.json' + ) + }) +})