From a2ac8641630da7d2197cf2fc96cc32a3ea12b3a1 Mon Sep 17 00:00:00 2001 From: Jeroen Engels Date: Fri, 23 Sep 2016 19:50:34 +0200 Subject: [PATCH] Breaking - newline-after-import: Do not require empty line after inline require (#570) --- CHANGELOG.md | 2 ++ docs/rules/newline-after-import.md | 17 ++++++++-- src/rules/newline-after-import.js | 20 +++++++++++- tests/src/rules/newline-after-import.js | 43 ++++++++++++++++++------- 4 files changed, 66 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15a5bec7e..197995f54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Breaking - [`import/extensions` setting] defaults to `['.js']`. ([#306]) - [`import/ignore` setting] defaults to nothing, and ambiguous modules are ignored natively. This means importing from CommonJS modules will no longer be reported by [`default`], [`named`], or [`namespace`], regardless of `import/ignore`. ([#270]) +- [`newline-after-import`]: Removed need for an empty line after an inline `require` call ([#570]) ### Changed - `imports-first` is renamed to [`first`]. `imports-first` alias will continue to @@ -378,6 +379,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 +[#570]: https://github.com/benmosher/eslint-plugin-import/issues/570 [#567]: https://github.com/benmosher/eslint-plugin-import/issues/567 [#566]: https://github.com/benmosher/eslint-plugin-import/issues/566 [#545]: https://github.com/benmosher/eslint-plugin-import/issues/545 diff --git a/docs/rules/newline-after-import.md b/docs/rules/newline-after-import.md index 57aab454d..0ad2cba01 100644 --- a/docs/rules/newline-after-import.md +++ b/docs/rules/newline-after-import.md @@ -1,11 +1,9 @@ # newline-after-import -Reports if there's no new line after last import/require in group. +Enforces having an empty line after the last top-level import statement or require call. ## Rule Details -**NOTE**: In each of those examples you can replace `import` call with `require`. - Valid: ```js @@ -21,6 +19,13 @@ import { bar } from 'bar-lib' const FOO = 'BAR' ``` +```js +const FOO = require('./foo') +const BAR = require('./bar') + +const BAZ = 1 +``` + ...whereas here imports will be reported: ```js @@ -35,6 +40,12 @@ const FOO = 'BAR' import { bar } from 'bar-lib' ``` +```js +const FOO = require('./foo') +const BAZ = 1 +const BAR = require('./bar') +``` + ## When Not To Use It If you like to visually group module imports with its usage, you don't want to use this rule. diff --git a/src/rules/newline-after-import.js b/src/rules/newline-after-import.js index ca0286748..7adcbfcee 100644 --- a/src/rules/newline-after-import.js +++ b/src/rules/newline-after-import.js @@ -68,6 +68,14 @@ module.exports = { } } + let level = 0 + function incrementLevel() { + level++ + } + function decrementLevel() { + level-- + } + return { ImportDeclaration: function (node) { const { parent } = node @@ -83,7 +91,7 @@ module.exports = { }, CallExpression: function(node) { const scope = context.getScope() - if (isStaticRequire(node)) { + if (isStaticRequire(node) && level === 0) { const currentScope = scopes[scopeIndex] if (scope === currentScope.scope) { @@ -127,6 +135,16 @@ module.exports = { }) }) }, + FunctionDeclaration: incrementLevel, + FunctionExpression: incrementLevel, + ArrowFunctionExpression: incrementLevel, + BlockStatement: incrementLevel, + ObjectExpression: incrementLevel, + 'FunctionDeclaration:exit': decrementLevel, + 'FunctionExpression:exit': decrementLevel, + 'ArrowFunctionExpression:exit': decrementLevel, + 'BlockStatement:exit': decrementLevel, + 'ObjectExpression:exit': decrementLevel, } }, } diff --git a/tests/src/rules/newline-after-import.js b/tests/src/rules/newline-after-import.js index dd4d5a9c0..ed9d6de9b 100644 --- a/tests/src/rules/newline-after-import.js +++ b/tests/src/rules/newline-after-import.js @@ -1,7 +1,7 @@ import { RuleTester } from 'eslint' -const IMPORT_ERROR_MESSAGE = 'Expected empty line after import statement not followed by another import.'; -const REQUIRE_ERROR_MESSAGE = 'Expected empty line after require statement not followed by another require.'; +const IMPORT_ERROR_MESSAGE = 'Expected empty line after import statement not followed by another import.' +const REQUIRE_ERROR_MESSAGE = 'Expected empty line after require statement not followed by another require.' const ruleTester = new RuleTester() @@ -21,7 +21,6 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), { parserOptions: { ecmaVersion: 6 } , }, "function x(){ require('baz'); }", - "a(require('b'), require('c'), require('d'));", `function foo() { switch (renderData.modalViewKey) { @@ -100,7 +99,35 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), { { code: "var foo = require('foo-module');\n\nvar a = 123;\n\nvar bar = require('bar-lib');", parserOptions: { sourceType: 'module' } - } + }, + { + code: ` + function foo() { + var foo = require('foo'); + foo(); + } + `, + parserOptions: { sourceType: 'module' }, + }, + { + code: ` + if (true) { + var foo = require('foo'); + foo(); + } + `, + parserOptions: { sourceType: 'module' }, + }, + { + code: ` + function a() { + var assign = Object.assign || require('object-assign'); + var foo = require('foo'); + var bar = 42; + } + `, + parserOptions: { sourceType: 'module' }, + }, ], invalid: [ @@ -183,14 +210,6 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), { message: REQUIRE_ERROR_MESSAGE, } ] }, - { - code: "function a() {\nvar assign = Object.assign || require('object-assign');\nvar foo = require('foo');\nvar bar = 42; }", - errors: [ { - line: 3, - column: 1, - message: REQUIRE_ERROR_MESSAGE, - } ] - }, { code: "require('a');\nfoo(require('b'), require('c'), require('d'));\nrequire('d');\nvar foo = 'bar';", errors: [