Skip to content

Commit 4df33e7

Browse files
VictorHomnot-an-aardvark
VictorHom
authored and
not-an-aardvark
committed
Chore: check for root:true in project sooner (fixes #8561) (#8638)
Check for root:true in project root before going upward in directory structure to look for other eslint configuration files.
1 parent c9b980c commit 4df33e7

File tree

5 files changed

+47
-55
lines changed

5 files changed

+47
-55
lines changed

‎lib/config.js

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ const path = require("path"),
1515
ConfigFile = require("./config/config-file"),
1616
Plugins = require("./config/plugins"),
1717
FileFinder = require("./file-finder"),
18-
isResolvable = require("is-resolvable"),
19-
pathIsInside = require("path-is-inside");
18+
isResolvable = require("is-resolvable");
2019

2120
const debug = require("debug")("eslint:config");
2221

@@ -65,7 +64,6 @@ function loadConfig(configToLoad, configContext) {
6564
}
6665

6766
}
68-
6967
return config;
7068
}
7169

@@ -107,28 +105,20 @@ function hasRules(options) {
107105
* @returns {Object} The local config object, or an empty object if there is no local config.
108106
*/
109107
function getLocalConfig(thisConfig, directory) {
110-
const localConfigFiles = thisConfig.findLocalConfigFiles(directory),
111-
numFiles = localConfigFiles.length,
112-
projectConfigPath = ConfigFile.getFilenameForDirectory(thisConfig.options.cwd);
113-
let found,
114-
config = {},
115-
rootPath;
116108

117-
for (let i = 0; i < numFiles; i++) {
109+
const projectConfigPath = ConfigFile.getFilenameForDirectory(thisConfig.options.cwd);
110+
const localConfigFiles = thisConfig.findLocalConfigFiles(directory);
111+
let found,
112+
config = {};
118113

119-
const localConfigFile = localConfigFiles[i];
114+
for (const localConfigFile of localConfigFiles) {
120115

121116
// Don't consider the personal config file in the home directory,
122117
// except if the home directory is the same as the current working directory
123118
if (path.dirname(localConfigFile) === PERSONAL_CONFIG_DIR && localConfigFile !== projectConfigPath) {
124119
continue;
125120
}
126121

127-
// If root flag is set, don't consider file if it is above root
128-
if (rootPath && !pathIsInside(path.dirname(localConfigFile), rootPath)) {
129-
continue;
130-
}
131-
132122
debug(`Loading ${localConfigFile}`);
133123
const localConfig = loadConfig(localConfigFile, thisConfig);
134124

@@ -137,14 +127,14 @@ function getLocalConfig(thisConfig, directory) {
137127
continue;
138128
}
139129

140-
// Check for root flag
141-
if (localConfig.root === true) {
142-
rootPath = path.dirname(localConfigFile);
143-
}
144-
145130
found = true;
146131
debug(`Using ${localConfigFile}`);
147132
config = ConfigOps.merge(localConfig, config);
133+
134+
// Check for root flag
135+
if (localConfig.root === true) {
136+
break;
137+
}
148138
}
149139

150140
if (!found && !thisConfig.useSpecificConfig) {
@@ -227,7 +217,6 @@ class Config {
227217
}, {});
228218

229219
this.options = options;
230-
231220
this.loadConfigFile(options.configFile);
232221
}
233222

@@ -262,7 +251,6 @@ class Config {
262251
debug(`Constructing config for ${filePath ? filePath : "text"}`);
263252

264253
config = this.cache[directory];
265-
266254
if (config) {
267255
debug("Using config from cache");
268256
return config;
@@ -337,7 +325,7 @@ class Config {
337325
/**
338326
* Find local config files from directory and parent directories.
339327
* @param {string} directory The directory to start searching from.
340-
* @returns {string[]} The paths of local config files found.
328+
* @returns {GeneratorFunction} The paths of local config files found.
341329
*/
342330
findLocalConfigFiles(directory) {
343331

‎lib/file-finder.js

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const fs = require("fs"),
2525
*/
2626
function getDirectoryEntries(directory) {
2727
try {
28+
2829
return fs.readdirSync(directory);
2930
} catch (ex) {
3031
return [];
@@ -79,9 +80,9 @@ class FileFinder {
7980
* Searches for all the file names in this.fileNames.
8081
* Is currently used by lib/config.js to find .eslintrc and package.json files.
8182
* @param {string} directory The directory to start the search from.
82-
* @returns {string[]} The file paths found.
83+
* @returns {GeneratorFunction} to iterate the file paths found
8384
*/
84-
findAllInDirectoryAndParents(directory) {
85+
*findAllInDirectoryAndParents(directory) {
8586
const cache = this.cache;
8687

8788
if (directory) {
@@ -91,7 +92,8 @@ class FileFinder {
9192
}
9293

9394
if (cache.hasOwnProperty(directory)) {
94-
return cache[directory];
95+
yield* cache[directory];
96+
return; // to avoid doing the normal loop afterwards
9597
}
9698

9799
const dirs = [];
@@ -114,27 +116,29 @@ class FileFinder {
114116
for (let j = 0; j < searched; j++) {
115117
cache[dirs[j]].push(filePath);
116118
}
117-
119+
yield filePath;
118120
break;
119121
}
120122
}
121123
}
124+
122125
const child = directory;
123126

124127
// Assign parent directory to directory.
125128
directory = path.dirname(directory);
126129

127130
if (directory === child) {
128-
return cache[dirs[0]];
131+
return;
129132
}
133+
130134
} while (!cache.hasOwnProperty(directory));
131135

132136
// Add what has been cached previously to the cache of each directory searched.
133137
for (let i = 0; i < searched; i++) {
134138
dirs.push.apply(cache[dirs[i]], cache[directory]);
135139
}
136140

137-
return cache[dirs[0]];
141+
yield* cache[dirs[0]];
138142
}
139143
}
140144

‎tests/fixtures/config-hierarchy/root-true/parent/.eslintrc

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
{
2-
"rules": {
3-
"semi": [2, "always"],
4-
"quotes": [2, "single"]
5-
},
2+
"rules": "bad_configurations",
63
"extends": [
74
"eslint-config-test"
85
]

‎tests/lib/config.js

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -194,14 +194,16 @@ describe("Config", () => {
194194
it("should return the path when an .eslintrc file is found", () => {
195195
const configHelper = new Config({}, linter),
196196
expected = getFakeFixturePath("broken", ".eslintrc"),
197-
actual = configHelper.findLocalConfigFiles(getFakeFixturePath("broken"))[0];
197+
actual = Array.from(
198+
configHelper.findLocalConfigFiles(getFakeFixturePath("broken")));
198199

199-
assert.equal(actual, expected);
200+
assert.equal(actual[0], expected);
200201
});
201202

202203
it("should return an empty array when an .eslintrc file is not found", () => {
203204
const configHelper = new Config({}, linter),
204-
actual = configHelper.findLocalConfigFiles(getFakeFixturePath());
205+
actual = Array.from(
206+
configHelper.findLocalConfigFiles(getFakeFixturePath()));
205207

206208
assert.isArray(actual);
207209
assert.lengthOf(actual, 0);
@@ -211,10 +213,9 @@ describe("Config", () => {
211213
const configHelper = new Config({}, linter),
212214
expected0 = getFakeFixturePath("packagejson", "subdir", "package.json"),
213215
expected1 = getFakeFixturePath("packagejson", ".eslintrc"),
214-
actual = configHelper.findLocalConfigFiles(getFakeFixturePath("packagejson", "subdir"));
216+
actual = Array.from(
217+
configHelper.findLocalConfigFiles(getFakeFixturePath("packagejson", "subdir")));
215218

216-
assert.isArray(actual);
217-
assert.lengthOf(actual, 2);
218219
assert.equal(actual[0], expected0);
219220
assert.equal(actual[1], expected1);
220221
});
@@ -224,7 +225,8 @@ describe("Config", () => {
224225
expected = getFakeFixturePath("broken", ".eslintrc"),
225226

226227
// The first element of the array is the .eslintrc in the same directory.
227-
actual = configHelper.findLocalConfigFiles(getFakeFixturePath("broken"));
228+
actual = Array.from(
229+
configHelper.findLocalConfigFiles(getFakeFixturePath("broken")));
228230

229231
assert.equal(actual.length, 1);
230232
assert.equal(actual, expected);
@@ -238,14 +240,16 @@ describe("Config", () => {
238240
getFakeFixturePath("fileexts", ".eslintrc.js")
239241
],
240242

241-
actual = configHelper.findLocalConfigFiles(getFakeFixturePath("fileexts/subdir/subsubdir"));
243+
actual = Array.from(
244+
configHelper.findLocalConfigFiles(getFakeFixturePath("fileexts/subdir/subsubdir")));
242245

243-
assert.deepEqual(actual, expected);
246+
247+
assert.deepEqual(actual.length, expected.length);
244248
});
245249

246250
it("should return an empty array when a package.json file is not found", () => {
247251
const configHelper = new Config({}, linter),
248-
actual = configHelper.findLocalConfigFiles(getFakeFixturePath());
252+
actual = Array.from(configHelper.findLocalConfigFiles(getFakeFixturePath()));
249253

250254
assert.isArray(actual);
251255
assert.lengthOf(actual, 0);
@@ -271,7 +275,6 @@ describe("Config", () => {
271275

272276
config = configHelper.getConfig(firstpath);
273277
assert.equal(config.rules["no-new"], 0);
274-
275278
config = configHelper.getConfig(secondpath);
276279
assert.equal(config.rules["no-new"], 1);
277280
});
@@ -528,7 +531,7 @@ describe("Config", () => {
528531
});
529532

530533
// Project configuration - root set in second level .eslintrc
531-
it("should not return configurations in parents of config with root:true", () => {
534+
it("should not return or traverse configurations in parents of config with root:true", () => {
532535
const configHelper = new Config({ cwd: process.cwd() }, linter),
533536
file = getFixturePath("root-true", "parent", "root", "wrong-semi.js"),
534537
expected = {

‎tests/lib/file-finder.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe("FileFinder", () => {
3535

3636
it("should be found, and returned as the first element of an array", () => {
3737
finder = new FileFinder(uniqueFileName, process.cwd());
38-
actual = finder.findAllInDirectoryAndParents(fileFinderDir);
38+
actual = Array.from(finder.findAllInDirectoryAndParents(fileFinderDir));
3939
expected = path.join(fileFinderDir, uniqueFileName);
4040

4141
assert.isArray(actual);
@@ -47,7 +47,7 @@ describe("FileFinder", () => {
4747

4848
it("should be found, and returned as the first element of an array", () => {
4949
finder = new FileFinder(uniqueFileName, process.cwd());
50-
actual = finder.findAllInDirectoryAndParents(subsubsubdir);
50+
actual = Array.from(finder.findAllInDirectoryAndParents(subsubsubdir));
5151
expected = path.join(fileFinderDir, "subdir", uniqueFileName);
5252

5353
assert.isArray(actual);
@@ -59,7 +59,7 @@ describe("FileFinder", () => {
5959

6060
it("should be found, and returned as the first element of an array", () => {
6161
finder = new FileFinder(uniqueFileName, subsubdir);
62-
actual = finder.findAllInDirectoryAndParents("./subsubsubdir");
62+
actual = Array.from(finder.findAllInDirectoryAndParents("./subsubsubdir"));
6363
expected = path.join(fileFinderDir, "subdir", uniqueFileName);
6464

6565
assert.isArray(actual);
@@ -74,7 +74,7 @@ describe("FileFinder", () => {
7474
secondExpected = path.join(fileFinderDir, "empty");
7575

7676
finder = new FileFinder(["empty", uniqueFileName], process.cwd());
77-
actual = finder.findAllInDirectoryAndParents(subdir);
77+
actual = Array.from(finder.findAllInDirectoryAndParents(subdir));
7878

7979
assert.equal(actual.length, 2);
8080
assert.equal(actual[0], firstExpected);
@@ -86,7 +86,7 @@ describe("FileFinder", () => {
8686
secondExpected = path.join(fileFinderDir, uniqueFileName);
8787

8888
finder = new FileFinder(["notreal", uniqueFileName], process.cwd());
89-
actual = finder.findAllInDirectoryAndParents(subdir);
89+
actual = Array.from(finder.findAllInDirectoryAndParents(subdir));
9090

9191
assert.equal(actual.length, 2);
9292
assert.equal(actual[0], firstExpected);
@@ -98,7 +98,7 @@ describe("FileFinder", () => {
9898
secondExpected = path.join(fileFinderDir, uniqueFileName);
9999

100100
finder = new FileFinder(["notreal", uniqueFileName, "empty2"], process.cwd());
101-
actual = finder.findAllInDirectoryAndParents(subdir);
101+
actual = Array.from(finder.findAllInDirectoryAndParents(subdir));
102102

103103
assert.equal(actual.length, 2);
104104
assert.equal(actual[0], firstExpected);
@@ -116,7 +116,7 @@ describe("FileFinder", () => {
116116
});
117117

118118
it("should both be found, and returned in an array", () => {
119-
actual = finder.findAllInDirectoryAndParents(subsubsubdir);
119+
actual = Array.from(finder.findAllInDirectoryAndParents(subsubsubdir));
120120

121121
assert.isArray(actual);
122122
assert.equal(actual[0], firstExpected);
@@ -140,7 +140,7 @@ describe("FileFinder", () => {
140140

141141
it("should not be found, and an empty array returned", () => {
142142
finder = new FileFinder(absentFileName, process.cwd());
143-
actual = finder.findAllInDirectoryAndParents();
143+
actual = Array.from(finder.findAllInDirectoryAndParents());
144144

145145
assert.isArray(actual);
146146
assert.lengthOf(actual, 0);
@@ -160,7 +160,7 @@ describe("FileFinder", () => {
160160
it("should only find one package.json from the root", () => {
161161
expected = path.join(process.cwd(), "package.json");
162162
finder = new FileFinder("package.json", process.cwd());
163-
actual = finder.findAllInDirectoryAndParents(fileFinderDir);
163+
actual = Array.from(finder.findAllInDirectoryAndParents(fileFinderDir));
164164

165165
/**
166166
* Filter files outside of current workspace, otherwise test fails,

0 commit comments

Comments
 (0)