Skip to content

Commit

Permalink
Add no-absolute-path rule (fixes #530) (#538)
Browse files Browse the repository at this point in the history
* Add `no-absolute-path` rule (fixes #530)

* fix typo
  • Loading branch information
jfmengels authored and benmosher committed Sep 1, 2016
1 parent 28dbed8 commit 79f3f83
Show file tree
Hide file tree
Showing 8 changed files with 155 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
- Added an `allow` option to [`no-nodejs-modules`] to allow exceptions ([#452], [#509]).
- Added [`no-absolute-path`] rule ([#530], [#538])

### Fixed
- [`no-named-as-default-member`] Allow default import to have a property named "default" ([#507]+[#508], thanks [@jquense] for both!)
Expand Down Expand Up @@ -293,7 +294,9 @@ for info on changes for earlier releases.
[`no-mutable-exports`]: ./docs/rules/no-mutable-exports.md
[`prefer-default-export`]: ./docs/rules/prefer-default-export.md
[`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md
[`no-absolute-path`]: ./docs/rules/no-absolute-path.md

[#538]: https://github.com/benmosher/eslint-plugin-import/pull/538
[#509]: https://github.com/benmosher/eslint-plugin-import/pull/509
[#508]: https://github.com/benmosher/eslint-plugin-import/pull/508
[#503]: https://github.com/benmosher/eslint-plugin-import/pull/503
Expand Down Expand Up @@ -334,6 +337,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

[#530]: https://github.com/benmosher/eslint-plugin-import/issues/530
[#507]: https://github.com/benmosher/eslint-plugin-import/issues/507
[#478]: https://github.com/benmosher/eslint-plugin-import/issues/478
[#456]: https://github.com/benmosher/eslint-plugin-import/issues/456
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
* Ensure a default export is present, given a default import. ([`default`])
* Ensure imported namespaces contain dereferenced properties as they are dereferenced. ([`namespace`])
* Restrict which files can be imported in a given folder ([`no-restricted-paths`])
* Forbid import of modules using absolute paths ([`no-absolute-path`])

[`no-unresolved`]: ./docs/rules/no-unresolved.md
[`named`]: ./docs/rules/named.md
[`default`]: ./docs/rules/default.md
[`namespace`]: ./docs/rules/namespace.md
[`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md
[`no-absolute-path`]: ./docs/rules/no-absolute-path.md

**Helpful warnings:**

Expand Down
27 changes: 27 additions & 0 deletions docs/rules/no-absolute-path.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Forbid import of modules using absolute paths

Node.js allows the import of modules using an absolute path such as `/home/xyz/file.js`. That is a bad practice as it ties the code using it to your computer, and therefore makes it unusable in packages distributed on `npm` for instance.

## Rule Details

### Fail

```js
import f from '/foo';
import f from '/some/path';

var f = require('/foo');
var f = require('/some/path');
```

### Pass

```js
import _ from 'lodash';
import foo from 'foo';
import foo from './foo';

var _ = require('lodash');
var foo = require('foo');
var foo = require('./foo');
```
5 changes: 5 additions & 0 deletions src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ function constant(value) {
return () => value
}

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

export function isBuiltIn(name, settings) {
const extras = (settings && settings['import/core-modules']) || []
return builtinModules.indexOf(name) !== -1 || extras.indexOf(name) > -1
Expand Down Expand Up @@ -46,6 +50,7 @@ function isRelativeToSibling(name) {
}

const typeTest = cond([
[isAbsolute, constant('absolute')],
[isBuiltIn, constant('builtin')],
[isExternalModule, constant('external')],
[isScoped, constant('external')],
Expand Down
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const rules = {
'no-duplicates': require('./rules/no-duplicates'),
'imports-first': require('./rules/imports-first'),
'no-extraneous-dependencies': require('./rules/no-extraneous-dependencies'),
'no-absolute-path': require('./rules/no-absolute-path'),
'no-nodejs-modules': require('./rules/no-nodejs-modules'),
'order': require('./rules/order'),
'newline-after-import': require('./rules/newline-after-import'),
Expand Down
21 changes: 21 additions & 0 deletions src/rules/no-absolute-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import importType from '../core/importType'
import isStaticRequire from '../core/staticRequire'

function reportIfMissing(context, node, name) {
if (importType(name, context) === 'absolute') {
context.report(node, 'Do not import modules using an absolute path')
}
}

module.exports = function (context) {
return {
ImportDeclaration: function handleImports(node) {
reportIfMissing(context, node, node.source.value)
},
CallExpression: function handleRequires(node) {
if (isStaticRequire(node)) {
reportIfMissing(context, node, node.arguments[0].value)
}
},
}
}
11 changes: 9 additions & 2 deletions tests/src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ import { testContext } from '../utils'
describe('importType(name)', function () {
const context = testContext()

it("should return 'absolute' for paths starting with a /", function() {
expect(importType('/', context)).to.equal('absolute')
expect(importType('/path', context)).to.equal('absolute')
expect(importType('/some/path', context)).to.equal('absolute')
})

it("should return 'builtin' for node.js modules", function() {
expect(importType('fs', context)).to.equal('builtin')
expect(importType('path', context)).to.equal('builtin')
Expand Down Expand Up @@ -47,15 +53,16 @@ describe('importType(name)', function () {
expect(importType('./importType/index.js', context)).to.equal('sibling')
})

describe("should return 'index' for sibling index file", function() {
it("should return 'index' for sibling index file", 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('@malformed', context)).to.equal('unknown')
expect(importType(' /malformed', context)).to.equal('unknown')
expect(importType(' foo', context)).to.equal('unknown')
})

Expand Down
86 changes: 86 additions & 0 deletions tests/src/rules/no-absolute-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { test } from '../utils'

import { RuleTester } from 'eslint'

const ruleTester = new RuleTester()
, rule = require('rules/no-absolute-path')

const error = {
ruleId: 'no-absolute-path',
message: 'Do not import modules using an absolute path',
}

ruleTester.run('no-absolute-path', rule, {
valid: [
test({ code: 'import _ from "lodash"'}),
test({ code: 'import find from "lodash.find"'}),
test({ code: 'import foo from "./foo"'}),
test({ code: 'import foo from "../foo"'}),
test({ code: 'import foo from "foo"'}),
test({ code: 'import foo from "./"'}),
test({ code: 'import foo from "@scope/foo"'}),
test({ code: 'var _ = require("lodash")'}),
test({ code: 'var find = require("lodash.find")'}),
test({ code: 'var foo = require("./foo")'}),
test({ code: 'var foo = require("../foo")'}),
test({ code: 'var foo = require("foo")'}),
test({ code: 'var foo = require("./")'}),
test({ code: 'var foo = require("@scope/foo")'}),
test({
code: 'import events from "events"',
options: [{
allow: ['events'],
}],
}),
test({
code: 'import path from "path"',
options: [{
allow: ['path'],
}],
}),
test({
code: 'var events = require("events")',
options: [{
allow: ['events'],
}],
}),
test({
code: 'var path = require("path")',
options: [{
allow: ['path'],
}],
}),
test({
code: 'import path from "path";import events from "events"',
options: [{
allow: ['path', 'events'],
}],
}),
],
invalid: [
test({
code: 'import f from "/foo"',
errors: [error],
}),
test({
code: 'import f from "/foo/path"',
errors: [error],
}),
test({
code: 'import f from "/some/path"',
errors: [error],
}),
test({
code: 'var f = require("/foo")',
errors: [error],
}),
test({
code: 'var f = require("/foo/path")',
errors: [error],
}),
test({
code: 'var f = require("/some/path")',
errors: [error],
}),
],
})

0 comments on commit 79f3f83

Please sign in to comment.