From de09dbe8b20867737a1724399e37ce54bc169798 Mon Sep 17 00:00:00 2001 From: Ian VanSchooten Date: Mon, 13 Mar 2017 23:03:23 -0700 Subject: [PATCH 1/2] Find closest package.json rather than home .eslintrc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #832 Previously, we would find the fallback `.eslintrc.*` config file in a user’s home directory, return that, and then potentially ignore it later because it’s a fallback. Only if there were _zero_ `.eslintrc.*` files would we ever look for a package.json. With this change, we will find a package.json file and check for an eslint configuration before traversing higher up the directory structure and potentially encountering a fallback in the user’s home root. It’s important that the `package.json` file come last in the array, so that we look for `.eslintrc` config files first. Otherwise, we may find a package.json file without an eslintConfig, ignore it, and not bother looking for a .eslintrc file in the same directory. This still has the problem that hitting a `package.json` file without an eslintConfig will prevent finding another config higher in the structure. That fix will come in the next commit. --- lib/worker-helpers.js | 17 +++++++++-------- spec/worker-helpers-spec.js | 5 +++++ src/worker-helpers.js | 17 +++++++++-------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/lib/worker-helpers.js b/lib/worker-helpers.js index 9a8f5368..ae5f26df 100644 --- a/lib/worker-helpers.js +++ b/lib/worker-helpers.js @@ -128,15 +128,16 @@ function getESLintInstance(fileDir, config, projectPath) { } function getConfigPath(fileDir) { - const configFile = (0, _atomLinter.findCached)(fileDir, ['.eslintrc.js', '.eslintrc.yaml', '.eslintrc.yml', '.eslintrc.json', '.eslintrc']); + const configFile = (0, _atomLinter.findCached)(fileDir, ['.eslintrc.js', '.eslintrc.yaml', '.eslintrc.yml', '.eslintrc.json', '.eslintrc', 'package.json']); if (configFile) { - return configFile; - } - - const packagePath = (0, _atomLinter.findCached)(fileDir, 'package.json'); - // eslint-disable-next-line import/no-dynamic-require - if (packagePath && Boolean(require(packagePath).eslintConfig)) { - return packagePath; + if (_path2.default.basename(configFile) === 'package.json') { + // eslint-disable-next-line import/no-dynamic-require + if (require(configFile).eslintConfig) { + return configFile; + } + } else { + return configFile; + } } return null; } diff --git a/spec/worker-helpers-spec.js b/spec/worker-helpers-spec.js index 70812734..31940f9f 100644 --- a/spec/worker-helpers-spec.js +++ b/spec/worker-helpers-spec.js @@ -141,6 +141,11 @@ describe('Worker Helpers', () => { const expectedPath = Path.join(fileDir, '.eslintrc.json') expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath) }) + it('finds package.json with an eslintConfig property', () => { + const fileDir = getFixturesPath(Path.join('configs', 'package-json')) + const expectedPath = Path.join(fileDir, 'package.json') + expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath) + }) }) describe('getRelativePath', () => { diff --git a/src/worker-helpers.js b/src/worker-helpers.js index 975cda1f..d50abad1 100644 --- a/src/worker-helpers.js +++ b/src/worker-helpers.js @@ -104,16 +104,17 @@ export function getESLintInstance(fileDir, config, projectPath) { export function getConfigPath(fileDir) { const configFile = findCached(fileDir, [ - '.eslintrc.js', '.eslintrc.yaml', '.eslintrc.yml', '.eslintrc.json', '.eslintrc' + '.eslintrc.js', '.eslintrc.yaml', '.eslintrc.yml', '.eslintrc.json', '.eslintrc', 'package.json' ]) if (configFile) { - return configFile - } - - const packagePath = findCached(fileDir, 'package.json') - // eslint-disable-next-line import/no-dynamic-require - if (packagePath && Boolean(require(packagePath).eslintConfig)) { - return packagePath + if (Path.basename(configFile) === 'package.json') { + // eslint-disable-next-line import/no-dynamic-require + if (require(configFile).eslintConfig) { + return configFile + } + } else { + return configFile + } } return null } From f7ef0a020755ecc7371fc71eae176cb02f64b133 Mon Sep 17 00:00:00 2001 From: Ian VanSchooten Date: Mon, 13 Mar 2017 23:07:05 -0700 Subject: [PATCH 2/2] Handle nested package.json files without eslintConfig Fixes #768 This continues traversing up the directory structure after finding a `package.json` file which does not contain an `eslintConfig`. --- lib/worker-helpers.js | 8 ++++++-- spec/fixtures/configs/package-json/nested/foo.js | 1 + spec/fixtures/configs/package-json/nested/package.json | 8 ++++++++ spec/worker-helpers-spec.js | 5 +++++ src/worker-helpers.js | 8 ++++++-- 5 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 spec/fixtures/configs/package-json/nested/foo.js create mode 100644 spec/fixtures/configs/package-json/nested/package.json diff --git a/lib/worker-helpers.js b/lib/worker-helpers.js index ae5f26df..a6b80e42 100644 --- a/lib/worker-helpers.js +++ b/lib/worker-helpers.js @@ -135,9 +135,13 @@ function getConfigPath(fileDir) { if (require(configFile).eslintConfig) { return configFile; } - } else { - return configFile; + // If we are here, we found a package.json without an eslint config + // in a dir without any other eslint config files + // (because 'package.json' is last in the call to findCached) + // So, keep looking from the parent directory + return getConfigPath(_path2.default.resolve(_path2.default.dirname(configFile), '..')); } + return configFile; } return null; } diff --git a/spec/fixtures/configs/package-json/nested/foo.js b/spec/fixtures/configs/package-json/nested/foo.js new file mode 100644 index 00000000..be92fda5 --- /dev/null +++ b/spec/fixtures/configs/package-json/nested/foo.js @@ -0,0 +1 @@ +var foo = 42; diff --git a/spec/fixtures/configs/package-json/nested/package.json b/spec/fixtures/configs/package-json/nested/package.json new file mode 100644 index 00000000..f68a5ea2 --- /dev/null +++ b/spec/fixtures/configs/package-json/nested/package.json @@ -0,0 +1,8 @@ +{ + "name": "test-fixture", + "version": "0.0.1", + "description": "", + "main": "foo.js", + "author": "", + "license": "" +} diff --git a/spec/worker-helpers-spec.js b/spec/worker-helpers-spec.js index 31940f9f..94a9f0d0 100644 --- a/spec/worker-helpers-spec.js +++ b/spec/worker-helpers-spec.js @@ -146,6 +146,11 @@ describe('Worker Helpers', () => { const expectedPath = Path.join(fileDir, 'package.json') expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath) }) + it('ignores package.json with no eslintConfig property', () => { + const fileDir = getFixturesPath(Path.join('configs', 'package-json', 'nested')) + const expectedPath = getFixturesPath(Path.join('configs', 'package-json', 'package.json')) + expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath) + }) }) describe('getRelativePath', () => { diff --git a/src/worker-helpers.js b/src/worker-helpers.js index d50abad1..e8dec11c 100644 --- a/src/worker-helpers.js +++ b/src/worker-helpers.js @@ -112,9 +112,13 @@ export function getConfigPath(fileDir) { if (require(configFile).eslintConfig) { return configFile } - } else { - return configFile + // If we are here, we found a package.json without an eslint config + // in a dir without any other eslint config files + // (because 'package.json' is last in the call to findCached) + // So, keep looking from the parent directory + return getConfigPath(Path.resolve(Path.dirname(configFile), '..')) } + return configFile } return null }