Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add no-extraneous-dependencies rule #241

Merged
merged 4 commits into from
Apr 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
## [Unreleased]
### Added
- [`no-named-as-default-member`] to `warnings` canned config
- add [`no-extraneous-dependencies`] rule

## [1.5.0] - 2016-04-18
### Added
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,12 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
* Ensure all imports appear before other statements ([`imports-first`])
* Report repeated import of the same module in multiple places ([`no-duplicates`])
* Report namespace imports ([`no-namespace`])
* Forbid the use of extraneous packages ([`no-extraneous-dependencies`])

[`imports-first`]: ./docs/rules/imports-first.md
[`no-duplicates`]: ./docs/rules/no-duplicates.md
[`no-namespace`]: ./docs/rules/no-namespace.md
[`no-extraneous-dependencies`]: ./docs/rules/no-extraneous-dependencies.md


## Installation
Expand Down
68 changes: 68 additions & 0 deletions docs/rules/no-extraneous-dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Forbid the use of extraneous packages

Forbid the import of external modules that are not declared in the `package.json`'s `dependencies` or `devDependencies`.
The closest parent `package.json` will be used. If no `package.json` is found, the rule will not lint anything.

### Options

This rule supports the following options:

`devDependencies`: If set to `false`, then the rule will show an error when `devDependencies` are imported. Defaults to `true`.

You can set the options like this:

```js
"import/no-extraneous-dependencies": ["error", {"devDependencies": false}]
```


## Rule Details

Given the following `package.json`:
```json
{
"name": "my-project",
"...": "...",
"dependencies": {
"builtin-modules": "^1.1.1",
"lodash.cond": "^4.2.0",
"lodash.find": "^4.2.0",
"pkg-up": "^1.0.0"
},
"devDependencies": {
"ava": "^0.13.0",
"eslint": "^2.4.0",
"eslint-plugin-ava": "^1.3.0",
"xo": "^0.13.0"
}
}
```


## Fail

```js
var _ = require('lodash');
import _ from 'lodash';

/* eslint import/no-extraneous-dependencies: ["error", {"devDependencies": false}] */
import test from 'ava';
var test = require('ava');
```


## Pass

```js
// Builtin and internal modules are fine
var path = require('path');
var foo = require('./foo');

import test from 'ava';
import find from 'lodash.find';
```


## When Not To Use It

If you do not have a `package.json` file in your project.
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,14 @@
"eslint": "2.x"
},
"dependencies": {
"builtin-modules": "^1.1.1",
"doctrine": "1.2.0",
"es6-map": "^0.1.3",
"es6-set": "^0.1.4",
"es6-symbol": "*",
"eslint-import-resolver-node": "^0.2.0",
"object-assign": "^4.0.1"
"lodash.cond": "^4.3.0",
"object-assign": "^4.0.1",
"pkg-up": "^1.0.0"
}
}
52 changes: 52 additions & 0 deletions src/core/importType.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import cond from 'lodash.cond'
import builtinModules from 'builtin-modules'
import { basename, join } from 'path'

import resolve from './resolve'

function constant(value) {
return () => value
}

function isBuiltIn(name) {
return builtinModules.indexOf(name) !== -1
}

const externalModuleRegExp = /^\w/
function isExternalModule(name, path) {
if (!externalModuleRegExp.test(name)) return false
return (!path || -1 < path.indexOf(join('node_modules', name)))
}

function isProjectModule(name, path) {
if (!externalModuleRegExp.test(name)) return false
return (path && -1 === path.indexOf(join('node_modules', name)))
}

function isRelativeToParent(name) {
return name.indexOf('../') === 0
}

const indexFiles = ['.', './', './index', './index.js']
function isIndex(name, path) {
if (path) return basename(path).split('.')[0] === 'index'
return indexFiles.indexOf(name) !== -1
}

function isRelativeToSibling(name) {
return name.indexOf('./') === 0
}

const typeTest = cond([
[isBuiltIn, constant('builtin')],
[isExternalModule, constant('external')],
[isProjectModule, constant('project')],
[isRelativeToParent, constant('parent')],
[isIndex, constant('index')],
[isRelativeToSibling, constant('sibling')],
[constant(true), constant('unknown')],
])

export default function resolveImportType(name, context) {
return typeTest(name, resolve(name, context))
}
8 changes: 8 additions & 0 deletions src/core/staticRequire.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// todo: merge with module visitor
export default function isStaticRequire(node) {
return node &&
node.callee.type === 'Identifier' &&
node.callee.name === 'require' &&
node.arguments.length === 1 &&
node.arguments[0].type === 'Literal'
}
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const rules = {
'no-amd': require('./rules/no-amd'),
'no-duplicates': require('./rules/no-duplicates'),
'imports-first': require('./rules/imports-first'),
'no-extraneous-dependencies': require('./rules/no-extraneous-dependencies'),

// metadata-based
'no-deprecated': require('./rules/no-deprecated'),
Expand Down
77 changes: 77 additions & 0 deletions src/rules/no-extraneous-dependencies.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import fs from 'fs'
import pkgUp from 'pkg-up'
import importType from '../core/importType'
import isStaticRequire from '../core/staticRequire'

function getDependencies(context) {
const filepath = pkgUp.sync(context.getFilename())
if (!filepath) {
return null
}

try {
const packageContent = JSON.parse(fs.readFileSync(filepath, 'utf8'))
return {
dependencies: packageContent.dependencies || {},
devDependencies: packageContent.devDependencies || {},
}
} catch (e) {
return null
}
}

function missingErrorMessage(packageName) {
return `'${packageName}' is not listed in the project's dependencies. ` +
`Run 'npm i -S ${packageName}' to add it`
}

function devDepErrorMessage(packageName) {
return `'${packageName}' is not listed in the project's dependencies, not devDependencies.`
}

function reportIfMissing(context, deps, allowDevDeps, node, name) {
if (importType(name, context) !== 'external') {
return
}
const packageName = name.split('/')[0]

if (deps.dependencies[packageName] === undefined) {
if (!allowDevDeps) {
context.report(node, devDepErrorMessage(packageName))
} else if (deps.devDependencies[packageName] === undefined) {
context.report(node, missingErrorMessage(packageName))
}
}
}

module.exports = function (context) {
const options = context.options[0] || {}
const allowDevDeps = options.devDependencies !== false
const deps = getDependencies(context)

if (!deps) {
return {}
}

// todo: use module visitor from module-utils core
return {
ImportDeclaration: function (node) {
reportIfMissing(context, deps, allowDevDeps, node, node.source.value)
},
CallExpression: function handleRequires(node) {
if (isStaticRequire(node)) {
reportIfMissing(context, deps, allowDevDeps, node, node.arguments[0].value)
}
},
}
}

module.exports.schema = [
{
'type': 'object',
'properties': {
'devDependencies': { 'type': 'boolean' },
},
'additionalProperties': false,
},
]
1 change: 0 additions & 1 deletion tests/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,4 @@ env:
mocha: true
rules:
no-unused-expressions: 0
quotes: [2, 'single', 'avoid-escape']
max-len: 0
1 change: 1 addition & 0 deletions tests/files/importType/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/* for importType test, just needs to exist */
14 changes: 13 additions & 1 deletion tests/files/package.json
Original file line number Diff line number Diff line change
@@ -1 +1,13 @@
{ "dummy": true }
{
"dummy": true,
"devDependencies": {
"eslint": "2.x"
},
"peerDependencies": {
"eslint": "2.x"
},
"dependencies": {
"lodash.cond": "^4.3.0",
"pkg-up": "^1.0.0"
}
}
60 changes: 60 additions & 0 deletions tests/src/core/importType.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { expect } from 'chai'
import * as path from 'path'

import importType from 'core/importType'

import { testContext } from '../utils'

describe('importType(name)', function () {
const context = testContext()

it("should return 'builtin' for node.js modules", function() {
expect(importType('fs', context)).to.equal('builtin')
expect(importType('path', context)).to.equal('builtin')
})

it("should return 'external' for non-builtin modules without a relative path", function() {
expect(importType('lodash', context)).to.equal('external')
expect(importType('async', context)).to.equal('external')
expect(importType('chalk', context)).to.equal('external')
expect(importType('foo', context)).to.equal('external')
expect(importType('lodash.find', context)).to.equal('external')
expect(importType('lodash/fp', context)).to.equal('external')
})

it("should return 'project' for non-builtins resolved outside of node_modules", function () {
const pathContext = testContext({ "import/resolver": { node: { paths: [ path.join(__dirname, '..', '..', 'files') ] } } })
expect(importType('importType', pathContext)).to.equal('project')
})

it("should return 'parent' for internal modules that go through the parent", function() {
expect(importType('../foo', context)).to.equal('parent')
expect(importType('../../foo', context)).to.equal('parent')
expect(importType('../bar/foo', context)).to.equal('parent')
})

it("should return 'sibling' for internal modules that are connected to one of the siblings", function() {
expect(importType('./foo', context)).to.equal('sibling')
expect(importType('./foo/bar', context)).to.equal('sibling')
})

describe("should return 'index' for sibling index file when", function() {
it("resolves", function() {
expect(importType('./importType', context)).to.equal('index')
expect(importType('./importType/', context)).to.equal('index')
expect(importType('./importType/index', context)).to.equal('index')
expect(importType('./importType/index.js', context)).to.equal('index')
})
it("doesn't resolve", function() {
expect(importType('.', context)).to.equal('index')
expect(importType('./', context)).to.equal('index')
expect(importType('./index', context)).to.equal('index')
expect(importType('./index.js', context)).to.equal('index')
})
})

it("should return 'unknown' for any unhandled cases", function() {
expect(importType('/malformed', context)).to.equal('unknown')
expect(importType(' foo', context)).to.equal('unknown')
})
})
Loading