diff --git a/CHANGELOG.md b/CHANGELOG.md index 6299f12a5c..01b1f61ab6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ## [Unreleased] ### Added -- Ignore type imports for named rule ([#931], thanks [@mattijsbliek]) +- Ignore type imports for [`named`] rule ([#931], thanks [@mattijsbliek]) - Add documentation for [`no-useless-path-segments`] rule ([#1068], thanks [@manovotny]) @@ -539,6 +539,7 @@ for info on changes for earlier releases. [#912]: https://github.com/benmosher/eslint-plugin-import/pull/912 [#1058]: https://github.com/benmosher/eslint-plugin-import/issues/1058 +[#931]: https://github.com/benmosher/eslint-plugin-import/issues/931 [#886]: https://github.com/benmosher/eslint-plugin-import/issues/886 [#863]: https://github.com/benmosher/eslint-plugin-import/issues/863 [#842]: https://github.com/benmosher/eslint-plugin-import/issues/842 @@ -714,3 +715,4 @@ for info on changes for earlier releases. [@klimashkin]: https://github.com/klimashkin [@lukeapage]: https://github.com/lukeapage [@manovotny]: https://github.com/manovotny +[@mattijsbliek]: https://github.com/mattijsbliek diff --git a/README.md b/README.md index 238c5347a1..541c58296e 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a * Forbid webpack loader syntax in imports ([`no-webpack-loader-syntax`]) * Forbid a module from importing itself ([`no-self-import`]) * Forbid a module from importing a module with a dependency path back to itself ([`no-cycle`]) -* Prevent unnecessary path segemnts in import and require statements ([`no-useless-path-segments`]) +* Prevent unnecessary path segments in import and require statements ([`no-useless-path-segments`]) [`no-unresolved`]: ./docs/rules/no-unresolved.md [`named`]: ./docs/rules/named.md @@ -361,6 +361,7 @@ settings: [`eslint_d`]: https://www.npmjs.com/package/eslint_d [`eslint-loader`]: https://www.npmjs.com/package/eslint-loader + ## SublimeLinter-eslint SublimeLinter-eslint introduced a change to support `.eslintignore` files diff --git a/docs/rules/no-extraneous-dependencies.md b/docs/rules/no-extraneous-dependencies.md index 069dab0cec..5c3542ebd4 100644 --- a/docs/rules/no-extraneous-dependencies.md +++ b/docs/rules/no-extraneous-dependencies.md @@ -35,6 +35,13 @@ Also there is one more option called `packageDir`, this option is to specify the "import/no-extraneous-dependencies": ["error", {"packageDir": './some-dir/'}] ``` +It may also be an array of multiple paths, to support monorepos or other novel project +folder layouts: + +```js +"import/no-extraneous-dependencies": ["error", {"packageDir": ['./some-dir/', './root-pkg']}] +``` + ## Rule Details Given the following `package.json`: diff --git a/package.json b/package.json index 860f822a67..0a8fe957be 100644 --- a/package.json +++ b/package.json @@ -16,15 +16,11 @@ ], "scripts": { "watch": "cross-env NODE_PATH=./src mocha --watch --compilers js:babel-register --recursive tests/src", - "cover": "gulp pretest && cross-env NODE_PATH=./lib istanbul cover --dir reports/coverage _mocha tests/lib/ -- --recursive -R progress", "pretest": "linklocal", "posttest": "eslint ./src", "test": "cross-env BABEL_ENV=test NODE_PATH=./src nyc -s mocha -R dot --recursive tests/src -t 5s", "test-compiled": "npm run prepublish && NODE_PATH=./lib mocha --compilers js:babel-register --recursive tests/src", - "coverage-report": "npm t && nyc report --reporter html", "test-all": "npm test && for resolver in ./resolvers/*; do cd $resolver && npm test && cd ../..; done", - "ci-test": "eslint ./src && gulp pretest && cross-env NODE_PATH=./lib istanbul cover --report lcovonly --dir reports/coverage _mocha tests/lib/ -- --recursive --reporter dot", - "debug": "cross-env NODE_PATH=./lib mocha debug --recursive --reporter dot tests/lib/", "prepublish": "gulp prepublish", "coveralls": "nyc report --reporter lcovonly && cat ./coverage/lcov.info | coveralls" }, @@ -48,12 +44,12 @@ }, "homepage": "https://github.com/benmosher/eslint-plugin-import", "devDependencies": { - "babel-eslint": "next", - "babel-plugin-istanbul": "^2.0.1", + "babel-eslint": "8.0.x", + "babel-plugin-istanbul": "^4.1.6", "babel-preset-es2015-argon": "latest", - "babel-register": "6.24.1", + "babel-register": "^6.26.0", "babylon": "6.15.0", - "chai": "^3.4.0", + "chai": "^3.5.0", "coveralls": "^3.0.0", "cross-env": "^4.0.0", "eslint": "2.x - 4.x", @@ -62,18 +58,16 @@ "eslint-import-resolver-webpack": "file:./resolvers/webpack", "eslint-module-utils": "file:./utils", "eslint-plugin-import": "2.x", - "eslint-plugin-typescript": "^0.8.1", "gulp": "^3.9.0", "gulp-babel": "6.1.2", - "istanbul": "^0.4.0", "linklocal": "^2.6.0", - "mocha": "^3.1.2", - "nyc": "^8.3.0", + "mocha": "^3.5.3", + "nyc": "^11.7.1", "redux": "^3.0.4", "rimraf": "^2.6.2", "sinon": "^2.3.2", "typescript": "^2.6.2", - "typescript-eslint-parser": "^12.0.0" + "typescript-eslint-parser": "^15.0.0" }, "peerDependencies": { "eslint": "2.x - 4.x" @@ -95,6 +89,10 @@ "babel-register" ], "sourceMap": false, - "instrument": false + "instrument": false, + "include": [ + "src/", + "resolvers/" + ] } } diff --git a/resolvers/node/package.json b/resolvers/node/package.json index c5859833cc..ceebe40d77 100644 --- a/resolvers/node/package.json +++ b/resolvers/node/package.json @@ -7,7 +7,8 @@ "index.js" ], "scripts": { - "test": "nyc mocha" + "test": "nyc mocha", + "coveralls": "nyc report --reporter lcovonly && cd ../.. && coveralls < ./resolvers/node/coverage/lcov.info" }, "repository": { "type": "git", @@ -32,7 +33,13 @@ }, "devDependencies": { "chai": "^3.5.0", + "coveralls": "^3.0.0", "mocha": "^3.5.3", - "nyc": "^10.3.2" + "nyc": "^11.7.1" + }, + "nyc": { + "exclude": [ + "test/" + ] } } diff --git a/resolvers/webpack/index.js b/resolvers/webpack/index.js index e1ffe70d17..1a39d92a1b 100644 --- a/resolvers/webpack/index.js +++ b/resolvers/webpack/index.js @@ -1,10 +1,9 @@ var findRoot = require('find-root') , path = require('path') - , get = require('lodash.get') + , get = require('lodash/get') + , isEqual = require('lodash/isEqual') , find = require('array-find') , interpret = require('interpret') - // not available on 0.10.x - , isAbsolute = path.isAbsolute || require('is-absolute') , fs = require('fs') , coreLibs = require('node-libs-browser') , resolve = require('resolve') @@ -56,7 +55,7 @@ exports.resolve = function (source, file, settings) { if (!configPath || typeof configPath === 'string') { // see if we've got an absolute path - if (!configPath || !isAbsolute(configPath)) { + if (!configPath || !path.isAbsolute(configPath)) { // if not, find ancestral package.json and use its directory as base for the path packageDir = findRoot(path.resolve(file)) if (!packageDir) throw new Error('package not found above ' + file) @@ -105,7 +104,8 @@ exports.resolve = function (source, file, settings) { } // otherwise, resolve "normally" - var resolveSync = createResolveSync(configPath, webpackConfig) + var resolveSync = getResolveSync(configPath, webpackConfig) + try { return { found: true, path: resolveSync(path.dirname(file), source) } } catch (err) { @@ -114,6 +114,24 @@ exports.resolve = function (source, file, settings) { } } +var MAX_CACHE = 10 +var _cache = [] +function getResolveSync(configPath, webpackConfig) { + var cacheKey = { configPath: configPath, webpackConfig: webpackConfig } + var cached = find(_cache, function (entry) { return isEqual(entry.key, cacheKey) }) + if (!cached) { + cached = { + key: cacheKey, + value: createResolveSync(configPath, webpackConfig) + } + // put in front and pop last item + if (_cache.unshift(cached) > MAX_CACHE) { + _cache.pop() + } + } + return cached.value +} + function createResolveSync(configPath, webpackConfig) { var webpackRequire , basedir = null @@ -316,7 +334,7 @@ function findConfigPath(configPath, packageDir) { }) // see if we've got an absolute path - if (!isAbsolute(configPath)) { + if (!path.isAbsolute(configPath)) { configPath = path.join(packageDir, configPath) } } else { diff --git a/resolvers/webpack/package.json b/resolvers/webpack/package.json index 9cbce0d479..2ec1dec565 100644 --- a/resolvers/webpack/package.json +++ b/resolvers/webpack/package.json @@ -5,7 +5,8 @@ "main": "index.js", "scripts": { "test": "nyc mocha -t 5s", - "report": "nyc report --reporter=html" + "report": "nyc report --reporter=html", + "coveralls": "nyc report --reporter lcovonly && cd ../.. && coveralls < ./resolvers/webpack/coverage/lcov.info" }, "files": [ "index.js", @@ -35,8 +36,7 @@ "find-root": "^1.1.0", "has": "^1.0.1", "interpret": "^1.0.0", - "is-absolute": "^0.2.3", - "lodash.get": "^4.4.2", + "lodash": "^4.17.4", "node-libs-browser": "^1.0.0 || ^2.0.0", "resolve": "^1.4.0", "semver": "^5.3.0" @@ -46,11 +46,17 @@ "webpack": ">=1.11.0" }, "devDependencies": { - "babel-plugin-istanbul": "^4.1.5", - "babel-preset-es2015-argon": "^0.1.0", + "babel-plugin-istanbul": "^4.1.6", + "babel-preset-es2015-argon": "latest", "babel-register": "^6.26.0", - "chai": "^3.4.1", - "mocha": "^2.3.3", - "nyc": "^7.0.0" + "chai": "^3.5.0", + "coveralls": "^3.0.0", + "mocha": "^3.5.3", + "nyc": "^11.7.1" + }, + "nyc": { + "exclude": [ + "test/" + ] } } diff --git a/src/rules/no-anonymous-default-export.js b/src/rules/no-anonymous-default-export.js index 491783bf67..34128a914a 100644 --- a/src/rules/no-anonymous-default-export.js +++ b/src/rules/no-anonymous-default-export.js @@ -4,6 +4,7 @@ */ import docsUrl from '../docsUrl' +import has from 'has' const defs = { ArrayExpression: { @@ -65,7 +66,7 @@ const schemaProperties = Object.keys(defs) const defaults = Object.keys(defs) .map((key) => defs[key]) .reduce((acc, def) => { - acc[def.option] = def.hasOwnProperty('default') ? def.default : false + acc[def.option] = has(def, 'default') ? def.default : false return acc }, {}) diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index bb684e4480..9d51018e9a 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -1,5 +1,6 @@ import path from 'path' import fs from 'fs' +import { isArray, isEmpty } from 'lodash' import readPkgUp from 'read-pkg-up' import minimatch from 'minimatch' import resolve from 'eslint-module-utils/resolve' @@ -7,24 +8,66 @@ import importType from '../core/importType' import isStaticRequire from '../core/staticRequire' import docsUrl from '../docsUrl' +function hasKeys(obj = {}) { + return Object.keys(obj).length > 0 +} + +function extractDepFields(pkg) { + return { + dependencies: pkg.dependencies || {}, + devDependencies: pkg.devDependencies || {}, + optionalDependencies: pkg.optionalDependencies || {}, + peerDependencies: pkg.peerDependencies || {}, + } +} + function getDependencies(context, packageDir) { + let paths = [] try { - const packageContent = packageDir - ? JSON.parse(fs.readFileSync(path.join(packageDir, 'package.json'), 'utf8')) - : readPkgUp.sync({cwd: context.getFilename(), normalize: false}).pkg + const packageContent = { + dependencies: {}, + devDependencies: {}, + optionalDependencies: {}, + peerDependencies: {}, + } - if (!packageContent) { - return null + if (!isEmpty(packageDir)) { + if (!isArray(packageDir)) { + paths = [path.resolve(packageDir)] + } else { + paths = packageDir.map(dir => path.resolve(dir)) + } } - return { - dependencies: packageContent.dependencies || {}, - devDependencies: packageContent.devDependencies || {}, - optionalDependencies: packageContent.optionalDependencies || {}, - peerDependencies: packageContent.peerDependencies || {}, + if (!isEmpty(paths)) { + // use rule config to find package.json + paths.forEach(dir => { + Object.assign(packageContent, extractDepFields( + JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf8')) + )) + }) + } else { + // use closest package.json + Object.assign( + packageContent, + extractDepFields( + readPkgUp.sync({cwd: context.getFilename(), normalize: false}).pkg + ) + ) + } + + if (![ + packageContent.dependencies, + packageContent.devDependencies, + packageContent.optionalDependencies, + packageContent.peerDependencies, + ].some(hasKeys)) { + return null } + + return packageContent } catch (e) { - if (packageDir && e.code === 'ENOENT') { + if (!isEmpty(paths) && e.code === 'ENOENT') { context.report({ message: 'The package.json file could not be found.', loc: { line: 0, column: 0 }, @@ -66,9 +109,8 @@ function reportIfMissing(context, deps, depsOptions, node, name) { } const resolved = resolve(name, context) - if (!resolved) { - return - } + if (!resolved) { return } + const splitName = name.split('/') const packageName = splitName[0][0] === '@' ? splitName.slice(0, 2).join('/') @@ -124,7 +166,7 @@ module.exports = { 'devDependencies': { 'type': ['boolean', 'array'] }, 'optionalDependencies': { 'type': ['boolean', 'array'] }, 'peerDependencies': { 'type': ['boolean', 'array'] }, - 'packageDir': { 'type': 'string' }, + 'packageDir': { 'type': ['string', 'array'] }, }, 'additionalProperties': false, }, diff --git a/tests/files/monorepo/package.json b/tests/files/monorepo/package.json new file mode 100644 index 0000000000..3ed889ddf5 --- /dev/null +++ b/tests/files/monorepo/package.json @@ -0,0 +1,6 @@ +{ + "private": true, + "devDependencies": { + "left-pad": "^1.2.0" + } +} diff --git a/tests/files/monorepo/packages/nested-package/package.json b/tests/files/monorepo/packages/nested-package/package.json new file mode 100644 index 0000000000..615c0a2342 --- /dev/null +++ b/tests/files/monorepo/packages/nested-package/package.json @@ -0,0 +1,6 @@ +{ + "name": "nested-monorepo-pkg", + "dependencies": { + "react": "^16.0.0" + } +} diff --git a/tests/files/node_modules/left-pad b/tests/files/node_modules/left-pad new file mode 120000 index 0000000000..dbbbe75d2d --- /dev/null +++ b/tests/files/node_modules/left-pad @@ -0,0 +1 @@ +not-a-dependency \ No newline at end of file diff --git a/tests/files/node_modules/react b/tests/files/node_modules/react new file mode 120000 index 0000000000..dbbbe75d2d --- /dev/null +++ b/tests/files/node_modules/react @@ -0,0 +1 @@ +not-a-dependency \ No newline at end of file diff --git a/tests/src/core/getExports.js b/tests/src/core/getExports.js index 096ef533a6..3423fe3e11 100644 --- a/tests/src/core/getExports.js +++ b/tests/src/core/getExports.js @@ -311,6 +311,7 @@ describe('ExportMap', function () { }) context('alternate parsers', function () { + const configs = [ // ['string form', { 'typescript-eslint-parser': '.ts' }], ['array form', { 'typescript-eslint-parser': ['.ts', '.tsx'] }], @@ -326,6 +327,7 @@ describe('ExportMap', function () { let imports before('load imports', function () { + this.timeout(20000) // takes a long time :shrug: imports = ExportMap.get('./typescript.ts', context) }) diff --git a/tests/src/rules/no-extraneous-dependencies.js b/tests/src/rules/no-extraneous-dependencies.js index a8817931b1..381b392cbf 100644 --- a/tests/src/rules/no-extraneous-dependencies.js +++ b/tests/src/rules/no-extraneous-dependencies.js @@ -15,6 +15,8 @@ const packageFileWithSyntaxErrorMessage = (() => { } })() const packageDirWithFlowTyped = path.join(__dirname, '../../files/with-flow-typed') +const packageDirMonoRepoRoot = path.join(__dirname, '../../files/monorepo') +const packageDirMonoRepoWithNested = path.join(__dirname, '../../files/monorepo/packages/nested-package') ruleTester.run('no-extraneous-dependencies', rule, { valid: [ @@ -75,8 +77,46 @@ ruleTester.run('no-extraneous-dependencies', rule, { options: [{packageDir: packageDirWithFlowTyped}], parser: 'babel-eslint', }), + test({ + code: 'import react from "react";', + options: [{packageDir: packageDirMonoRepoWithNested}], + }), + test({ + code: 'import leftpad from "left-pad";', + options: [{packageDir: [packageDirMonoRepoWithNested, packageDirMonoRepoRoot]}], + }), + test({ + code: 'import leftpad from "left-pad";', + options: [{packageDir: packageDirMonoRepoRoot}], + }), ], invalid: [ + test({ + code: 'import "not-a-dependency"', + filename: path.join(packageDirMonoRepoRoot, 'foo.js'), + options: [{packageDir: packageDirMonoRepoRoot }], + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: '\'not-a-dependency\' should be listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it', + }], + }), + test({ + code: 'import "not-a-dependency"', + filename: path.join(packageDirMonoRepoWithNested, 'foo.js'), + options: [{packageDir: packageDirMonoRepoRoot}], + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: '\'not-a-dependency\' should be listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it', + }], + }), + test({ + code: 'import "not-a-dependency"', + options: [{packageDir: packageDirMonoRepoRoot}], + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: '\'not-a-dependency\' should be listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it', + }], + }), test({ code: 'import "not-a-dependency"', errors: [{ @@ -197,5 +237,31 @@ ruleTester.run('no-extraneous-dependencies', rule, { message: 'The package.json file could not be parsed: ' + packageFileWithSyntaxErrorMessage, }], }), - ], + test({ + code: 'import leftpad from "left-pad";', + filename: path.join(packageDirMonoRepoWithNested, 'foo.js'), + options: [{packageDir: packageDirMonoRepoWithNested}], + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: "'left-pad' should be listed in the project's dependencies. Run 'npm i -S left-pad' to add it", + }], + }), + test({ + code: 'import react from "react";', + filename: path.join(packageDirMonoRepoRoot, 'foo.js'), + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: "'react' should be listed in the project's dependencies. Run 'npm i -S react' to add it", + }], + }), + test({ + code: 'import react from "react";', + filename: path.join(packageDirMonoRepoWithNested, 'foo.js'), + options: [{packageDir: packageDirMonoRepoRoot}], + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: "'react' should be listed in the project's dependencies. Run 'npm i -S react' to add it", + }], + }), + ] }) diff --git a/tests/src/utils.js b/tests/src/utils.js index 144969f5b0..fe04d684e2 100644 --- a/tests/src/utils.js +++ b/tests/src/utils.js @@ -4,7 +4,7 @@ import path from 'path' import 'babel-eslint' export function testFilePath(relativePath) { - return path.join(process.cwd(), './tests/files', relativePath) + return path.join(process.cwd(), './tests/files', relativePath) } export const FILENAME = testFilePath('foo.js') @@ -31,7 +31,7 @@ export function getFilename(file) { /** * to be added as valid cases just to ensure no nullable fields are going - * to crash at runtinme + * to crash at runtime * @type {Array} */ export const SYNTAX_CASES = [