From dedfb114b8138ac13e30799cb69f82b8267fdf05 Mon Sep 17 00:00:00 2001 From: kevin940726 Date: Sun, 5 Feb 2017 02:53:23 +0800 Subject: [PATCH 1/9] add allow glob for rule no-unassigned-import, fix #671 --- src/rules/no-unassigned-import.js | 32 +++++++++++++++-- tests/src/rules/no-unassigned-import.js | 47 +++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/rules/no-unassigned-import.js b/src/rules/no-unassigned-import.js index a5503f196..e7d5b1e3e 100644 --- a/src/rules/no-unassigned-import.js +++ b/src/rules/no-unassigned-import.js @@ -1,4 +1,6 @@ import isStaticRequire from '../core/staticRequire' +import path from 'path' +import minimatch from 'minimatch' function report(context, node) { context.report({ @@ -7,15 +9,40 @@ function report(context, node) { }) } +function testIsAllow(globs, filename, source) { + if (!Array.isArray(globs)) { + return false // default doens't allow any pattern + } + + 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) } }, @@ -33,6 +60,7 @@ module.exports = { 'devDependencies': { 'type': ['boolean', 'array'] }, 'optionalDependencies': { 'type': ['boolean', 'array'] }, 'peerDependencies': { 'type': ['boolean', 'array'] }, + 'allow': { 'type': 'array' }, }, 'additionalProperties': false, }, diff --git a/tests/src/rules/no-unassigned-import.js b/tests/src/rules/no-unassigned-import.js index 86248d459..b21668fa1 100644 --- a/tests/src/rules/no-unassigned-import.js +++ b/tests/src/rules/no-unassigned-import.js @@ -29,6 +29,38 @@ 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'] }], + }), ], invalid: [ test({ @@ -39,5 +71,20 @@ 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], + }), ], }) From 7d41745437ffc87ed21d183f072946bff0023163 Mon Sep 17 00:00:00 2001 From: kevin940726 Date: Sun, 5 Feb 2017 15:10:50 +0800 Subject: [PATCH 2/9] write doc, add two more tests --- docs/rules/no-unassigned-import.md | 22 ++++++++++++++++++++++ tests/src/rules/no-unassigned-import.js | 20 ++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/docs/rules/no-unassigned-import.md b/docs/rules/no-unassigned-import.md index 1b25b0169..324962767 100644 --- a/docs/rules/no-unassigned-import.md +++ b/docs/rules/no-unassigned-import.md @@ -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 usefull for some build environment (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 /src/app.js +import '../styles/app.css' +// {"allow": ["styles/*.css"]} ``` @@ -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 /src/app.js +import './styles/app.css' +import '../scripts/register.js' +// {"allow": ["src/styles/**", "**/scripts/*.js"]} ``` diff --git a/tests/src/rules/no-unassigned-import.js b/tests/src/rules/no-unassigned-import.js index b21668fa1..92b276999 100644 --- a/tests/src/rules/no-unassigned-import.js +++ b/tests/src/rules/no-unassigned-import.js @@ -61,6 +61,20 @@ ruleTester.run('no-unassigned-import', rule, { 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({ @@ -86,5 +100,11 @@ ruleTester.run('no-unassigned-import', rule, { 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], + }), ], }) From 2f690b4388552f36b07541c99cb82cab631f62cc Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Thu, 25 May 2017 14:42:18 -0400 Subject: [PATCH 3/9] update CI to build on Node 6+7 (#846) * update Travis to build on Node 6+7 * build against node 6+7 on windows --- .travis.yml | 2 +- appveyor.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 504c00e82..22918d364 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: node_js node_js: - - 4 - 6 + - 7 os: - linux diff --git a/appveyor.yml b/appveyor.yml index 288932b48..57adb8874 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,7 +1,7 @@ # Test against this version of Node.js environment: matrix: - - nodejs_version: "4" + - nodejs_version: "7" - nodejs_version: "6" # platform: From 28e1623dd09cf63717f1a1b2bd3ec3566c5fb106 Mon Sep 17 00:00:00 2001 From: Ivan Babak Date: Wed, 31 May 2017 06:27:40 -0700 Subject: [PATCH 4/9] eslint-module-utils: filePath in parserOptions (#840) * eslint-module-utils: filePath in parserOptions Refs https://github.com/benmosher/eslint-plugin-import/issues/839 * eslint-module-utils: Add tests for parserOptions Refs https://github.com/benmosher/eslint-plugin-import/issues/839 * eslint-module-utils: Reverted manual version bumps. Refs https://github.com/benmosher/eslint-plugin-import/issues/839 * Add sinon, replace eslint-module-utils test spy with sinon.spy * Fix CHANGELOG merge error * eslint-module-utils: Add more tests for parse (coverage 100%) * eslint-module-utils: In tests move require stub parser to the top. --- CHANGELOG.md | 3 ++- package.json | 1 + tests/src/core/parse.js | 36 +++++++++++++++++++++++++++++++ tests/src/core/parseStubParser.js | 4 ++++ utils/CHANGELOG.md | 8 +++++-- utils/parse.js | 4 ++++ 6 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 tests/src/core/parseStubParser.js diff --git a/CHANGELOG.md b/CHANGELOG.md index cc6f2c04e..246b82083 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +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 +- Add `filePath` into `parserOptions` passed to `parser` ([#839], thanks [@sompylasar]) ## [2.3.0] - 2017-05-18 ### Added diff --git a/package.json b/package.json index 0edaa58e0..60b53c3cd 100644 --- a/package.json +++ b/package.json @@ -69,6 +69,7 @@ "nyc": "^8.3.0", "redux": "^3.0.4", "rimraf": "2.5.2", + "sinon": "^2.3.2", "typescript": "^2.0.3", "typescript-eslint-parser": "^2.1.0" }, diff --git a/tests/src/core/parse.js b/tests/src/core/parse.js index 0793a70c2..2feea07ae 100644 --- a/tests/src/core/parse.js +++ b/tests/src/core/parse.js @@ -1,11 +1,14 @@ import * as fs from 'fs' import { expect } from 'chai' +import sinon from 'sinon' import parse from 'eslint-module-utils/parse' import { getFilename } from '../utils' describe('parse(content, { settings, ecmaFeatures })', function () { const path = getFilename('jsx.js') + const parseStubParser = require('./parseStubParser') + const parseStubParserPath = require.resolve('./parseStubParser') let content before((done) => @@ -21,4 +24,37 @@ describe('parse(content, { settings, ecmaFeatures })', function () { .not.to.throw(Error) }) + it('passes expected parserOptions to custom parser', function () { + const parseSpy = sinon.spy() + const parserOptions = { ecmaFeatures: { jsx: true } } + parseStubParser.parse = parseSpy + parse(path, content, { settings: {}, parserPath: parseStubParserPath, parserOptions: parserOptions }) + expect(parseSpy.callCount, 'custom parser to be called once').to.equal(1) + expect(parseSpy.args[0][0], 'custom parser to get content as its first argument').to.equal(content) + expect(parseSpy.args[0][1], 'custom parser to get an object as its second argument').to.be.an('object') + expect(parseSpy.args[0][1], 'custom parser to clone the parserOptions object').to.not.equal(parserOptions) + expect(parseSpy.args[0][1], 'custom parser to get ecmaFeatures in parserOptions which is a clone of ecmaFeatures passed in') + .to.have.property('ecmaFeatures') + .that.is.eql(parserOptions.ecmaFeatures) + .and.is.not.equal(parserOptions.ecmaFeatures) + expect(parseSpy.args[0][1], 'custom parser to get parserOptions.attachComment equal to true').to.have.property('attachComment', true) + expect(parseSpy.args[0][1], 'custom parser to get parserOptions.filePath equal to the full path of the source file').to.have.property('filePath', path) + }) + + it('should throw on context == null', function () { + expect(parse.bind(null, path, content, null)).to.throw(Error) + }) + + it('should throw on unable to resolve parserPath', function () { + expect(parse.bind(null, path, content, { settings: {}, parserPath: null })).to.throw(Error) + }) + + it('should take the alternate parser specified in settings', function () { + const parseSpy = sinon.spy() + const parserOptions = { ecmaFeatures: { jsx: true } } + parseStubParser.parse = parseSpy + expect(parse.bind(null, path, content, { settings: { 'import/parsers': { [parseStubParserPath]: [ '.js' ] } }, parserPath: null, parserOptions: parserOptions })).not.to.throw(Error) + expect(parseSpy.callCount, 'custom parser to be called once').to.equal(1) + }) + }) diff --git a/tests/src/core/parseStubParser.js b/tests/src/core/parseStubParser.js new file mode 100644 index 000000000..81daace43 --- /dev/null +++ b/tests/src/core/parseStubParser.js @@ -0,0 +1,4 @@ +// this stub must be in a separate file to require from parse via moduleRequire +module.exports = { + parse: function () {}, +} diff --git a/utils/CHANGELOG.md b/utils/CHANGELOG.md index e31196c69..241398a41 100644 --- a/utils/CHANGELOG.md +++ b/utils/CHANGELOG.md @@ -3,9 +3,13 @@ All notable changes to this module will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). -## v2 - 2016-11-07 +## [Unreleased] +### Added +- `parse` now additionally passes `filePath` to `parser` in `parserOptions` like `eslint` core does + +## v2.0.0 - 2016-11-07 ### Changed - `unambiguous` no longer exposes fast test regex ### Fixed -- `unambiguous.test()` regex is now properly in multiline mode \ No newline at end of file +- `unambiguous.test()` regex is now properly in multiline mode diff --git a/utils/parse.js b/utils/parse.js index c93417a61..671dc86c0 100644 --- a/utils/parse.js +++ b/utils/parse.js @@ -22,6 +22,10 @@ exports.default = function parse(path, content, context) { // always attach comments parserOptions.attachComment = true + // provide the `filePath` like eslint itself does, in `parserOptions` + // https://github.com/eslint/eslint/blob/3ec436ee/lib/linter.js#L637 + parserOptions.filePath = path + // require the parser relative to the main module (i.e., ESLint) const parser = moduleRequire(parserPath) From 95315e041f758ea925177736225ae2f931a25a8d Mon Sep 17 00:00:00 2001 From: kevin940726 Date: Sun, 5 Feb 2017 15:15:40 +0800 Subject: [PATCH 5/9] update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 246b82083..461303cdd 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 - 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 @@ -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 @@ -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 @@ -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 From 8f9b403f36e511498bcea40b8d21f26a4bbb33ad Mon Sep 17 00:00:00 2001 From: kevin940726 Date: Sun, 5 Feb 2017 16:58:49 +0800 Subject: [PATCH 6/9] fix typos, enforce type of array of strings in allow option --- docs/rules/no-unassigned-import.md | 2 +- src/rules/no-unassigned-import.js | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/rules/no-unassigned-import.md b/docs/rules/no-unassigned-import.md index 324962767..85f9b7c3a 100644 --- a/docs/rules/no-unassigned-import.md +++ b/docs/rules/no-unassigned-import.md @@ -10,7 +10,7 @@ This rule aims to remove modules with side-effects by reporting when a module is 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 usefull for some build environment (e.g. css-loader in webpack). +`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. diff --git a/src/rules/no-unassigned-import.js b/src/rules/no-unassigned-import.js index e7d5b1e3e..2b1499b34 100644 --- a/src/rules/no-unassigned-import.js +++ b/src/rules/no-unassigned-import.js @@ -11,7 +11,7 @@ function report(context, node) { function testIsAllow(globs, filename, source) { if (!Array.isArray(globs)) { - return false // default doens't allow any pattern + return false // default doesn't allow any patterns } let filePath @@ -60,7 +60,12 @@ module.exports = { 'devDependencies': { 'type': ['boolean', 'array'] }, 'optionalDependencies': { 'type': ['boolean', 'array'] }, 'peerDependencies': { 'type': ['boolean', 'array'] }, - 'allow': { 'type': 'array' }, + 'allow': { + 'type': 'array', + 'items': { + 'type': 'string', + }, + }, }, 'additionalProperties': false, }, From 3e291690e41883cb67d13b5bbde1b32850494943 Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Fri, 2 Jun 2017 09:54:34 -0400 Subject: [PATCH 7/9] bump v2.4.0 --- CHANGELOG.md | 8 +++++++- package.json | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 461303cdd..90fc0fe98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] + + +## [2.4.0] - 2017-06-02 ### 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]). @@ -448,6 +451,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 +[#839]: https://github.com/benmosher/eslint-plugin-import/issues/839 [#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 @@ -506,7 +510,8 @@ for info on changes for earlier releases. [#119]: https://github.com/benmosher/eslint-plugin-import/issues/119 [#89]: https://github.com/benmosher/eslint-plugin-import/issues/89 -[Unreleased]: https://github.com/benmosher/eslint-plugin-import/compare/v2.3.0...HEAD +[Unreleased]: https://github.com/benmosher/eslint-plugin-import/compare/v2.4.0...HEAD +[2.4.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.3.0...v2.4.0 [2.3.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.2.0...v2.3.0 [2.2.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.1.0...v2.2.0 [2.1.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.0.1...v2.1.0 @@ -586,4 +591,5 @@ for info on changes for earlier releases. [@giodamelio]: https://github.com/giodamelio [@ntdb]: https://github.com/ntdb [@ramasilveyra]: https://github.com/ramasilveyra +[@sompylasar]: https://github.com/sompylasar [@kevin940726]: https://github.com/kevin940726 diff --git a/package.json b/package.json index 60b53c3cd..c60e9b2ad 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-import", - "version": "2.3.0", + "version": "2.4.0", "description": "Import with sanity.", "engines": { "node": ">=4" From a3728d705ed1547f91e487dd6f7326bf67c52e9e Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Fri, 2 Jun 2017 09:57:28 -0400 Subject: [PATCH 8/9] bump eslint-module-utils to v2.1.0 --- package.json | 2 +- utils/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index c60e9b2ad..2a74867fe 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,7 @@ "debug": "^2.2.0", "doctrine": "1.5.0", "eslint-import-resolver-node": "^0.2.0", - "eslint-module-utils": "^2.0.0", + "eslint-module-utils": "^2.1.0", "has": "^1.0.1", "lodash.cond": "^4.3.0", "minimatch": "^3.0.3", diff --git a/utils/package.json b/utils/package.json index 0f76e24d9..bc234b534 100644 --- a/utils/package.json +++ b/utils/package.json @@ -1,6 +1,6 @@ { "name": "eslint-module-utils", - "version": "2.0.0", + "version": "2.1.0", "description": "Core utilities to support eslint-plugin-import and other module-related plugins.", "engines": { "node": ">=4" From 44ca1588d3817f8142aba0bfd345871f8d563ded Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Fri, 2 Jun 2017 10:03:05 -0400 Subject: [PATCH 9/9] update utils changelog --- utils/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/utils/CHANGELOG.md b/utils/CHANGELOG.md index 241398a41..02ed3e462 100644 --- a/utils/CHANGELOG.md +++ b/utils/CHANGELOG.md @@ -3,7 +3,10 @@ All notable changes to this module will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). -## [Unreleased] +## Unreleased + + +## v2.1.0 - 2017-06-02 ### Added - `parse` now additionally passes `filePath` to `parser` in `parserOptions` like `eslint` core does