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

Support allow option in no-unassigned-import, fix #671 #737

Merged
merged 4 commits into from
Jun 1, 2017
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
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],
}),
],
})