Skip to content

Commit

Permalink
Merge pull request #737 from kevin940726/master
Browse files Browse the repository at this point in the history
[New] Support `allow` option in `no-unassigned-import`, fix #671
  • Loading branch information
ljharb authored Jun 1, 2017
2 parents 28e1623 + 8f9b403 commit ea9c92c
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 2 deletions.
4 changes: 4 additions & 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
- Add `filePath` into `parserOptions` passed to `parser` ([#839], thanks [@sompylasar])
- Add `allow` option to [`no-unassigned-import`] to allow for files that match the globs ([#671], [#737], thanks [@kevin940726]).

## [2.3.0] - 2017-05-18
### Added
Expand Down Expand Up @@ -390,6 +391,7 @@ for info on changes for earlier releases.
[`no-anonymous-default-export`]: ./docs/rules/no-anonymous-default-export.md

[#742]: https://github.com/benmosher/eslint-plugin-import/pull/742
[#737]: https://github.com/benmosher/eslint-plugin-import/pull/737
[#712]: https://github.com/benmosher/eslint-plugin-import/pull/712
[#685]: https://github.com/benmosher/eslint-plugin-import/pull/685
[#680]: https://github.com/benmosher/eslint-plugin-import/pull/680
Expand Down Expand Up @@ -446,6 +448,7 @@ for info on changes for earlier releases.
[#157]: https://github.com/benmosher/eslint-plugin-import/pull/157
[#314]: https://github.com/benmosher/eslint-plugin-import/pull/314

[#671]: https://github.com/benmosher/eslint-plugin-import/issues/671
[#660]: https://github.com/benmosher/eslint-plugin-import/issues/660
[#653]: https://github.com/benmosher/eslint-plugin-import/issues/653
[#627]: https://github.com/benmosher/eslint-plugin-import/issues/627
Expand Down Expand Up @@ -583,3 +586,4 @@ for info on changes for earlier releases.
[@giodamelio]: https://github.com/giodamelio
[@ntdb]: https://github.com/ntdb
[@ramasilveyra]: https://github.com/ramasilveyra
[@kevin940726]: https://github.com/kevin940726
22 changes: 22 additions & 0 deletions docs/rules/no-unassigned-import.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,24 @@ With both CommonJS' `require` and the ES6 modules' `import` syntax, it is possib

This rule aims to remove modules with side-effects by reporting when a module is imported but not assigned.

### Options

This rule supports the following option:

`allow`: An Array of globs. The files that match any of these patterns would be ignored/allowed by the linter. This can be useful for some build environments (e.g. css-loader in webpack).

Note that the globs start from the where the linter is executed (usually project root), but not from each file that includes the source. Learn more in both the pass and fail examples below.


## Fail

```js
import 'should'
require('should')

// In <PROJECT_ROOT>/src/app.js
import '../styles/app.css'
// {"allow": ["styles/*.css"]}
```


Expand All @@ -34,4 +47,13 @@ bar(require('foo'))
require('foo').bar
require('foo').bar()
require('foo')()

// With allow option set
import './style.css' // {"allow": ["**/*.css"]}
import 'babel-register' // {"allow": ["babel-register"]}

// In <PROJECT_ROOT>/src/app.js
import './styles/app.css'
import '../scripts/register.js'
// {"allow": ["src/styles/**", "**/scripts/*.js"]}
```
37 changes: 35 additions & 2 deletions src/rules/no-unassigned-import.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import isStaticRequire from '../core/staticRequire'
import path from 'path'
import minimatch from 'minimatch'

function report(context, node) {
context.report({
Expand All @@ -7,15 +9,40 @@ function report(context, node) {
})
}

function testIsAllow(globs, filename, source) {
if (!Array.isArray(globs)) {
return false // default doesn't allow any patterns
}

let filePath

if (source[0] !== '.' && source[0] !== '/') { // a node module
filePath = source
} else {
filePath = path.resolve(path.dirname(filename), source) // get source absolute path
}

return globs.find(glob => (
minimatch(filePath, glob) ||
minimatch(filePath, path.join(process.cwd(), glob))
)) !== undefined
}

function create(context) {
const options = context.options[0] || {}
const filename = context.getFilename()
const isAllow = source => testIsAllow(options.allow, filename, source)

return {
ImportDeclaration(node) {
if (node.specifiers.length === 0) {
if (node.specifiers.length === 0 && !isAllow(node.source.value)) {
report(context, node)
}
},
ExpressionStatement(node) {
if (node.expression.type === 'CallExpression' && isStaticRequire(node.expression)) {
if (node.expression.type === 'CallExpression' &&
isStaticRequire(node.expression) &&
!isAllow(node.expression.arguments[0].value)) {
report(context, node.expression)
}
},
Expand All @@ -33,6 +60,12 @@ module.exports = {
'devDependencies': { 'type': ['boolean', 'array'] },
'optionalDependencies': { 'type': ['boolean', 'array'] },
'peerDependencies': { 'type': ['boolean', 'array'] },
'allow': {
'type': 'array',
'items': {
'type': 'string',
},
},
},
'additionalProperties': false,
},
Expand Down
67 changes: 67 additions & 0 deletions tests/src/rules/no-unassigned-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,52 @@ ruleTester.run('no-unassigned-import', rule, {
test({ code: 'require("lodash").foo'}),
test({ code: 'require("lodash").foo()'}),
test({ code: 'require("lodash")()'}),
test({
code: 'import "app.css"',
options: [{ 'allow': ['**/*.css'] }],
}),
test({
code: 'import "app.css";',
options: [{ 'allow': ['*.css'] }],
}),
test({
code: 'import "./app.css"',
options: [{ 'allow': ['**/*.css'] }],
}),
test({
code: 'import "foo/bar"',
options: [{ 'allow': ['foo/**'] }],
}),
test({
code: 'import "foo/bar"',
options: [{ 'allow': ['foo/bar'] }],
}),
test({
code: 'import "../dir/app.css"',
options: [{ 'allow': ['**/*.css'] }],
}),
test({
code: 'import "../dir/app.js"',
options: [{ 'allow': ['**/dir/**'] }],
}),
test({
code: 'require("./app.css")',
options: [{ 'allow': ['**/*.css'] }],
}),
test({
code: 'import "babel-register"',
options: [{ 'allow': ['babel-register'] }],
}),
test({
code: 'import "./styles/app.css"',
options: [{ 'allow': ['src/styles/**'] }],
filename: path.join(process.cwd(), 'src/app.js'),
}),
test({
code: 'import "../scripts/register.js"',
options: [{ 'allow': ['src/styles/**', '**/scripts/*.js'] }],
filename: path.join(process.cwd(), 'src/app.js'),
}),
],
invalid: [
test({
Expand All @@ -39,5 +85,26 @@ ruleTester.run('no-unassigned-import', rule, {
code: 'require("lodash")',
errors: [error],
}),
test({
code: 'import "./app.css"',
options: [{ 'allow': ['**/*.js'] }],
errors: [error],
}),
test({
code: 'import "./app.css"',
options: [{ 'allow': ['**/dir/**'] }],
errors: [error],
}),
test({
code: 'require("./app.css")',
options: [{ 'allow': ['**/*.js'] }],
errors: [error],
}),
test({
code: 'import "./styles/app.css"',
options: [{ 'allow': ['styles/*.css'] }],
filename: path.join(process.cwd(), 'src/app.js'),
errors: [error],
}),
],
})

0 comments on commit ea9c92c

Please sign in to comment.