Skip to content

Commit

Permalink
Fix #529: Add rule no-unassigned-import
Browse files Browse the repository at this point in the history
  • Loading branch information
jfmengels authored and benmosher committed Sep 28, 2016
1 parent 1bd0650 commit dc3609f
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com).

## [Unreleased]
### Added
- Added [`no-unassigned-import`] rule ([#529])

## 2.0.0 - WIP
### Added
Expand Down Expand Up @@ -339,6 +341,7 @@ for info on changes for earlier releases.
[`no-internal-modules`]: ./docs/rules/no-internal-modules.md
[`no-dynamic-require`]: ./docs/rules/no-dynamic-require.md
[`no-webpack-loader-syntax`]: ./docs/rules/no-webpack-loader-syntax.md
[`no-unassigned-import`]: ./docs/rules/no-unassigned-import.md

[#586]: https://github.com/benmosher/eslint-plugin-import/pull/586
[#578]: https://github.com/benmosher/eslint-plugin-import/pull/578
Expand Down Expand Up @@ -394,6 +397,7 @@ for info on changes for earlier releases.
[#566]: https://github.com/benmosher/eslint-plugin-import/issues/566
[#545]: https://github.com/benmosher/eslint-plugin-import/issues/545
[#530]: https://github.com/benmosher/eslint-plugin-import/issues/530
[#529]: https://github.com/benmosher/eslint-plugin-import/issues/529
[#519]: https://github.com/benmosher/eslint-plugin-import/issues/519
[#507]: https://github.com/benmosher/eslint-plugin-import/issues/507
[#478]: https://github.com/benmosher/eslint-plugin-import/issues/478
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
* Enforce a newline after import statements ([`newline-after-import`])
* Prefer a default export if module exports a single name ([`prefer-default-export`])
* Limit the maximum number of dependencies a module can have. ([`max-dependencies`])
* Forbid unassigned imports. ([`no-unassigned-import`])

[`first`]: ./docs/rules/first.md
[`no-duplicates`]: ./docs/rules/no-duplicates.md
Expand All @@ -83,6 +84,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
[`newline-after-import`]: ./docs/rules/newline-after-import.md
[`prefer-default-export`]: ./docs/rules/prefer-default-export.md
[`max-dependencies`]: ./docs/rules/max-dependencies.md
[`no-unassigned-import`]: ./docs/rules/no-unassigned-import.md

## Installation

Expand Down
37 changes: 37 additions & 0 deletions docs/rules/no-unassigned-import.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Forbid unassigned imports

With both CommonJS' `require` and the ES6 modules' `import` syntax, it is possible to import a module but not to use its result. This can be done explicitly by not assigning the module to as variable. Doing so can mean either of the following things:
- The module is imported but not used
- The module has side-effects (like [`should`](https://www.npmjs.com/package/should)). Having side-effects, makes it hard to know whether the module is actually used or can be removed. It can also make it harder to test or mock parts of your application.

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

## Fail

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


## Pass

```js
import _ from 'foo'
import _, {foo} from 'foo'
import _, {foo as bar} from 'foo'
import {foo as bar} from 'foo'
import * as _ from 'foo'

const _ = require('foo')
const {foo} = require('foo')
const {foo: bar} = require('foo')
const [a, b] = require('foo')
const _ = require('foo')

// Module is not assigned, but it is used
bar(require('foo'))
require('foo').bar
require('foo').bar()
require('foo')()
```
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const rules = {
'prefer-default-export': require('./rules/prefer-default-export'),
'no-dynamic-require': require('./rules/no-dynamic-require'),
'unambiguous': require('./rules/unambiguous'),
'no-unassigned-import': require('./rules/no-unassigned-import'),

// metadata-based
'no-deprecated': require('./rules/no-deprecated'),
Expand Down
41 changes: 41 additions & 0 deletions src/rules/no-unassigned-import.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import isStaticRequire from '../core/staticRequire'

function report(context, node) {
context.report({
node,
message: 'Imported module should be assigned',
})
}

function create(context) {
return {
ImportDeclaration(node) {
if (node.specifiers.length === 0) {
report(context, node)
}
},
ExpressionStatement(node) {
if (node.expression.type === 'CallExpression' && isStaticRequire(node.expression)) {
report(context, node.expression)
}
},
}
}

module.exports = {
create,
meta: {
docs: {},
schema: [
{
'type': 'object',
'properties': {
'devDependencies': { 'type': ['boolean', 'array'] },
'optionalDependencies': { 'type': ['boolean', 'array'] },
'peerDependencies': { 'type': ['boolean', 'array'] },
},
'additionalProperties': false,
},
],
},
}
43 changes: 43 additions & 0 deletions tests/src/rules/no-unassigned-import.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { test } from '../utils'
import * as path from 'path'

import { RuleTester } from 'eslint'

const ruleTester = new RuleTester()
, rule = require('rules/no-unassigned-import')

const error = {
ruleId: 'no-unassigned-import',
message: 'Imported module should be assigned'
}

ruleTester.run('no-unassigned-import', rule, {
valid: [
test({ code: 'import _ from "lodash"'}),
test({ code: 'import _, {foo} from "lodash"'}),
test({ code: 'import _, {foo as bar} from "lodash"'}),
test({ code: 'import {foo as bar} from "lodash"'}),
test({ code: 'import * as _ from "lodash"'}),
test({ code: 'import _ from "./"'}),
test({ code: 'const _ = require("lodash")'}),
test({ code: 'const {foo} = require("lodash")'}),
test({ code: 'const {foo: bar} = require("lodash")'}),
test({ code: 'const [a, b] = require("lodash")'}),
test({ code: 'const _ = require("lodash")'}),
test({ code: 'const _ = require("./")'}),
test({ code: 'foo(require("lodash"))'}),
test({ code: 'require("lodash").foo'}),
test({ code: 'require("lodash").foo()'}),
test({ code: 'require("lodash")()'}),
],
invalid: [
test({
code: 'import "lodash"',
errors: [error],
}),
test({
code: 'require("lodash")',
errors: [error],
}),
],
})

0 comments on commit dc3609f

Please sign in to comment.