Replaced read-pkg-up with cache-aware package locator

package.json

@@ -87,8 +87,7 @@
     "eslint-module-utils": "^2.1.1",
     "has": "^1.0.1",
     "lodash": "^4.17.4",
-    "minimatch": "^3.0.3",
-    "read-pkg-up": "^2.0.0"
+    "minimatch": "^3.0.3"
   },
   "nyc": {
     "require": [

src/core/CachedPackageLocator.js

@@ -0,0 +1,61 @@
+import path from 'path'
+import os from 'os'
+import fs from 'fs'
+
+export default class CachedPackageLocator {
+  constructor() {
+    this.store = {}
+  }
+
+  readUpSync(context, dirname, immediate, reduce) {
+    const locations = []
+    do {
+      const location = path.join(dirname, 'package.json')
+
+      if (this.store[location]) {
+        return this.store[location]
+      }
+
+      locations.push(location)
+      if (this.store[location] === null) {
+        continue
+      }
+
+      try {
+        this.store[location] = reduce(
+          JSON.parse(fs.readFileSync(location, 'utf8'))
+        )
+
+        if (this.store[location]) {
+          return this.store[location]
+        }
+      } catch (err) {
+        if (err.code === 'ENOENT') {
+          this.store[location] = null
+
+          if (immediate) {
+            context.report({
+              message: 'Could not find package.json file: ' + location,
+              loc: { line: 0, column: 0 },
+            })
+            return
+          }
+        } else if (err instanceof SyntaxError) {
+          context.report({
+            message: 'Could not parse package.json file: ' + err.message + ': ' + location,
+            loc: { line: 0, column: 0 },
+          })
+          return
+        } else {
+          // dont swallow unknown error
+          throw err
+        }
+      }
+    } while (dirname !== (dirname = path.dirname(dirname)))
+
+    context.report({
+      message: `Could not find package.json files: ${os.EOL}${locations.join(os.EOL)}`,
+      loc: { line: 0, column: 0 },
+    })
+  }
+}

src/rules/no-extraneous-dependencies.js

@@ -1,44 +1,28 @@
 import path from 'path'
-import fs from 'fs'
-import readPkgUp from 'read-pkg-up'
 import minimatch from 'minimatch'
 import resolve from 'eslint-module-utils/resolve'
 import importType from '../core/importType'
 import isStaticRequire from '../core/staticRequire'
+import CachedPackageLocator from '../core/CachedPackageLocator'
 import docsUrl from '../docsUrl'
 
-function getDependencies(context, packageDir) {
-  try {
-    const packageContent = packageDir
-      ? JSON.parse(fs.readFileSync(path.join(packageDir, 'package.json'), 'utf8'))
-      : readPkgUp.sync({cwd: context.getFilename(), normalize: false}).pkg
+const packageLocator = new CachedPackageLocator()
 
-    if (!packageContent) {
-      return null
-    }
-
-    return {
-      dependencies: packageContent.dependencies || {},
-      devDependencies: packageContent.devDependencies || {},
-      optionalDependencies: packageContent.optionalDependencies || {},
-      peerDependencies: packageContent.peerDependencies || {},
-    }
-  } catch (e) {
-    if (packageDir && e.code === 'ENOENT') {
-      context.report({
-        message: 'The package.json file could not be found.',
-        loc: { line: 0, column: 0 },
-      })
-    }
-    if (e.name === 'JSONError' || e instanceof SyntaxError) {
-      context.report({
-        message: 'The package.json file could not be parsed: ' + e.message,
-        loc: { line: 0, column: 0 },
-      })
-    }
+function notEmpty(obj) {
+  return Object.keys(obj).length
+}
 
-    return null
+function reducePackage({
+  dependencies = {},
+  devDependencies = {},
+  peerDependencies = {},
+  optionalDependencies = {},
+} = {}) {
+  if ([dependencies, devDependencies, peerDependencies, optionalDependencies].some(notEmpty)) {
+    return { dependencies, devDependencies, peerDependencies, optionalDependencies }
   }
+
+  return null
 }
 
 function missingErrorMessage(packageName) {
@@ -131,10 +115,15 @@ module.exports = {
     ],
   },
 
-  create: function (context) {
+  create(context) {
     const options = context.options[0] || {}
     const filename = context.getFilename()
-    const deps = getDependencies(context, options.packageDir)
+    const deps = packageLocator.readUpSync(
+      context,
+      options.packageDir || path.dirname(context.getFilename()),
+      typeof options.packageDir !== 'undefined',
+      reducePackage
+    )
 
     if (!deps) {
       return {}
@@ -148,10 +137,10 @@ module.exports = {
 
     // todo: use module visitor from module-utils core
     return {
-      ImportDeclaration: function (node) {
+      ImportDeclaration(node) {
         reportIfMissing(context, deps, depsOptions, node, node.source.value)
       },
-      CallExpression: function handleRequires(node) {
+      CallExpression(node) {
         if (isStaticRequire(node)) {
           reportIfMissing(context, deps, depsOptions, node, node.arguments[0].value)
         }

tests/src/core/CachedPackageLocator.js

@@ -0,0 +1,212 @@
+import fs from 'fs'
+import sinon from 'sinon'
+import { expect } from 'chai'
+
+import CachedPackageLocator from '../../../src/core/CachedPackageLocator'
+
+function notEmpty(obj) {
+  return Object.keys(obj).length
+}
+
+function reduce({
+  dependencies = {},
+  devDependencies = {},
+  peerDependencies = {},
+  optionalDependencies = {},
+} = {}) {
+  if ([dependencies, devDependencies, peerDependencies, optionalDependencies].some(notEmpty)) {
+    return { dependencies, devDependencies, peerDependencies, optionalDependencies }
+  }
+
+  return null
+}
+
+describe('CachedPackageLocator.readUpSync()', function () {
+  let sandbox
+  let packageLocator
+
+  const withUnexpectedTokenStr = '{{"name":"foo"}'
+  const withUnexpectedEndStr = '{"name":"foo"'
+  const withDeps = {
+    dependencies: { 'prop-types': '~15.0.0' },
+    devDependencies: { 'webpack': '^2.0.0' },
+    peerDependencies: { 'react': '>=15.0.0' },
+    optionalDependencies: { 'fs-events': '*' },
+  }
+  const withDepsStr = JSON.stringify(withDeps)
+  const withDepsExtraFieldsStr = JSON.stringify(
+    Object.assign({}, withDeps, {
+      name: 'not-needed',
+      description: 'foo bar',
+    })
+  )
+  const context = {
+    report: sinon.spy(),
+  }
+
+  before(function () {
+    sandbox = sinon.sandbox.create()
+    sandbox.stub(fs, 'readFileSync').reset()
+  })
+
+  after(function () {
+    sandbox.restore()
+  })
+
+  beforeEach(function () {
+    fs.readFileSync.throws({ code: 'ENOENT' })
+    packageLocator = new CachedPackageLocator()
+  })
+
+  afterEach(function () {
+    context.report.reset()
+    sandbox.reset()
+  })
+
+  it('should not repeat fs.readFileSync on stored locations', function () {
+    fs.readFileSync.withArgs('/a/package.json').returns(withDepsStr)
+
+    expect(packageLocator.readUpSync(context, '/a/b', false, reduce))
+      .to.deep.equal(withDeps)
+    sinon.assert.callCount(fs.readFileSync, 2)
+    expect(packageLocator.readUpSync(context, '/a', false, reduce))
+      .to.deep.equal(withDeps)
+    sinon.assert.callCount(fs.readFileSync, 2)
+    expect(packageLocator.store).to.deep.equal({
+      '/a/b/package.json': null,
+      '/a/package.json': withDeps,
+    })
+
+    expect(packageLocator.readUpSync(context, '/x', false, reduce)).to.be.undefined
+    sinon.assert.callCount(fs.readFileSync, 4)
+    expect(packageLocator.readUpSync(context, '/x', false, reduce)).to.be.undefined
+    sinon.assert.callCount(fs.readFileSync, 4)
+    expect(packageLocator.store).to.deep.equal({
+      '/x/package.json': null,
+      '/a/b/package.json': null,
+      '/a/package.json': withDeps,
+      '/package.json': null,
+    })
+
+    expect(packageLocator.readUpSync(context, '/x/y/z', false, reduce)).to.be.undefined
+    sinon.assert.callCount(fs.readFileSync, 6)
+    expect(packageLocator.readUpSync(context, '/x/y/z', false, reduce)).to.be.undefined
+    sinon.assert.callCount(fs.readFileSync, 6)
+    expect(packageLocator.store).to.deep.equal({
+      '/x/y/z/package.json': null,
+      '/x/y/package.json': null,
+      '/x/package.json': null,
+      '/a/b/package.json': null,
+      '/a/package.json': withDeps,
+      '/package.json': null,
+    })
+
+    expect(packageLocator.readUpSync(context, '/x/w', false, reduce)).to.be.undefined
+    sinon.assert.callCount(fs.readFileSync, 7)
+    expect(packageLocator.readUpSync(context, '/x/w', false, reduce)).to.be.undefined
+    sinon.assert.callCount(fs.readFileSync, 7)
+
+    expect(packageLocator.store).to.deep.equal({
+      '/x/y/z/package.json': null,
+      '/x/y/package.json': null,
+      '/x/w/package.json': null,
+      '/x/package.json': null,
+      '/a/b/package.json': null,
+      '/a/package.json': withDeps,
+      '/package.json': null,
+    })
+  })
+
+  it('should only store and return dependency fields', function () {
+    fs.readFileSync.withArgs('/package.json').returns(withDepsExtraFieldsStr)
+    expect(packageLocator.readUpSync(context, '/', false, reduce))
+      .to.deep.equal(withDeps)
+    expect(packageLocator.store).to.deep.equal({
+      '/package.json': withDeps,
+    })
+    sinon.assert.calledOnce(fs.readFileSync)
+  })
+
+  it('should locate first available', function () {
+    fs.readFileSync.withArgs('/a/b/package.json').returns(withDepsStr)
+    expect(packageLocator.readUpSync(context, '/a/b', false, reduce))
+      .to.deep.equal(withDeps)
+    expect(packageLocator.store).to.deep.equal({
+      '/a/b/package.json': withDeps,
+    })
+    sinon.assert.notCalled(context.report)
+  })
+
+  it('should locate last available', function () {
+    fs.readFileSync.withArgs('/package.json').returns(withDepsStr)
+    expect(packageLocator.readUpSync(context, '/a/b/c/d/e/f', false, reduce))
+      .to.deep.equal(withDeps)
+    expect(packageLocator.store).to.deep.equal({
+      '/a/b/c/d/e/f/package.json': null,
+      '/a/b/c/d/e/package.json': null,
+      '/a/b/c/d/package.json': null,
+      '/a/b/c/package.json': null,
+      '/a/b/package.json': null,
+      '/a/package.json': null,
+      '/package.json': withDeps,
+    })
+    sinon.assert.notCalled(context.report)
+  })
+
+  it('should store package.json with empty deps as null', function () {
+    fs.readFileSync.withArgs('/package.json').returns('{}')
+    expect(packageLocator.readUpSync(context, '/', false, reduce))
+      .to.be.undefined
+    expect(packageLocator.store).to.deep.equal({
+      '/package.json': null,
+    })
+    sinon.assert.calledOnce(context.report)
+  })
+
+  it('should not store JSON.parse failures', function () {
+    fs.readFileSync
+      .withArgs('/package.json').returns(withDepsStr)
+      .withArgs('/a/package.json').returns(withUnexpectedTokenStr)
+      .withArgs('/a/b/package.json').returns(withUnexpectedEndStr)
+    expect(packageLocator.readUpSync(context, '/a', false))
+      .to.be.undefined
+    expect(packageLocator.store).to.be.empty
+    expect(packageLocator.readUpSync(context, '/a/b/c/d', false))
+      .to.be.undefined
+    expect(packageLocator.store).to.deep.equal({
+      '/a/b/c/d/package.json': null,
+      '/a/b/c/package.json': null,
+    })
+    sinon.assert.callCount(context.report, 2)
+  })
+
+  it('should store failed locations as null', function () {
+    expect(packageLocator.readUpSync(context, '/does/not/exist', false))
+      .to.be.undefined
+    expect(packageLocator.store).to.deep.equal({
+      '/does/not/exist/package.json': null,
+      '/does/not/package.json': null,
+      '/does/package.json': null,
+      '/package.json': null,
+    })
+    sinon.assert.calledOnce(context.report)
+  })
+
+  it('immediate=true should halt on first failed location', function () {
+    expect(packageLocator.readUpSync(context, '/does/not/exist', true))
+      .to.be.undefined
+    expect(packageLocator.store).to.deep.equal({
+      '/does/not/exist/package.json': null
+    })
+    sinon.assert.calledOnce(context.report)
+  })
+
+  it('should throw unknown errors', function () {
+    fs.readFileSync.throws(new Error('Some unknown error'))
+    expect(() => {
+      packageLocator.readUpSync(context, '/does/not/exist', true)
+    }).to.throw('Some unknown error')
+    expect(packageLocator.store).to.empty
+    sinon.assert.notCalled(context.report)
+  })
+})