diff --git a/CHANGELOG.md b/CHANGELOG.md index f0a5a615e..af31210f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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!) @@ -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 @@ -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 diff --git a/README.md b/README.md index b396daf3b..fa746a991 100644 --- a/README.md +++ b/README.md @@ -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:** diff --git a/docs/rules/no-absolute-path.md b/docs/rules/no-absolute-path.md new file mode 100644 index 000000000..9a4be74f8 --- /dev/null +++ b/docs/rules/no-absolute-path.md @@ -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'); +``` diff --git a/src/core/importType.js b/src/core/importType.js index 02edc0234..5236b1d2a 100644 --- a/src/core/importType.js +++ b/src/core/importType.js @@ -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 @@ -46,6 +50,7 @@ function isRelativeToSibling(name) { } const typeTest = cond([ + [isAbsolute, constant('absolute')], [isBuiltIn, constant('builtin')], [isExternalModule, constant('external')], [isScoped, constant('external')], diff --git a/src/index.js b/src/index.js index 52d5668c5..6d2a5d22c 100644 --- a/src/index.js +++ b/src/index.js @@ -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'), diff --git a/src/rules/no-absolute-path.js b/src/rules/no-absolute-path.js new file mode 100644 index 000000000..08c8b80fe --- /dev/null +++ b/src/rules/no-absolute-path.js @@ -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) + } + }, + } +} diff --git a/tests/src/core/importType.js b/tests/src/core/importType.js index 28a8d7f4e..610916912 100644 --- a/tests/src/core/importType.js +++ b/tests/src/core/importType.js @@ -8,6 +8,12 @@ import { testContext } from '../utils' describe('importType(name)', function () { const context = testContext() + it("should return 'absolute' for node.js modules", 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') @@ -47,7 +53,7 @@ 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') @@ -55,7 +61,8 @@ describe('importType(name)', function () { }) 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') }) diff --git a/tests/src/rules/no-absolute-path.js b/tests/src/rules/no-absolute-path.js new file mode 100644 index 000000000..88951672b --- /dev/null +++ b/tests/src/rules/no-absolute-path.js @@ -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], + }), + ], +})