Skip to content

Commit

Permalink
Chore: check for root:true in project sooner (fixes #8561) (#8638)
Browse files Browse the repository at this point in the history 
Check for root:true in project root before going upward in directory
structure to look for other eslint configuration files.
  • Loading branch information
@VictorHom @not-an-aardvark
VictorHom authored and not-an-aardvark committed May 31, 2017
1 parent c9b980c commit 4df33e7
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 55 deletions.
36 changes: 12 additions & 24 deletions lib/config.js
View file
Expand Up @@ -15,8 +15,7 @@ const path = require("path"),
ConfigFile = require("./config/config-file"),
Plugins = require("./config/plugins"),
FileFinder = require("./file-finder"),
isResolvable = require("is-resolvable"),
pathIsInside = require("path-is-inside");
isResolvable = require("is-resolvable");

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

Expand Down Expand Up @@ -65,7 +64,6 @@ function loadConfig(configToLoad, configContext) {
}

}

return config;
}

Expand Down Expand Up @@ -107,28 +105,20 @@ function hasRules(options) {
* @returns {Object} The local config object, or an empty object if there is no local config.
*/
function getLocalConfig(thisConfig, directory) {
const localConfigFiles = thisConfig.findLocalConfigFiles(directory),
numFiles = localConfigFiles.length,
projectConfigPath = ConfigFile.getFilenameForDirectory(thisConfig.options.cwd);
let found,
config = {},
rootPath;

for (let i = 0; i < numFiles; i++) {
const projectConfigPath = ConfigFile.getFilenameForDirectory(thisConfig.options.cwd);
const localConfigFiles = thisConfig.findLocalConfigFiles(directory);
let found,
config = {};

const localConfigFile = localConfigFiles[i];
for (const localConfigFile of localConfigFiles) {

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

// If root flag is set, don't consider file if it is above root
if (rootPath && !pathIsInside(path.dirname(localConfigFile), rootPath)) {
continue;
}

debug(`Loading ${localConfigFile}`);
const localConfig = loadConfig(localConfigFile, thisConfig);

Expand All @@ -137,14 +127,14 @@ function getLocalConfig(thisConfig, directory) {
continue;
}

// Check for root flag
if (localConfig.root === true) {
rootPath = path.dirname(localConfigFile);
}

found = true;
debug(`Using ${localConfigFile}`);
config = ConfigOps.merge(localConfig, config);

// Check for root flag
if (localConfig.root === true) {
break;
}
}

if (!found && !thisConfig.useSpecificConfig) {
Expand Down Expand Up @@ -227,7 +217,6 @@ class Config {
}, {});

this.options = options;

this.loadConfigFile(options.configFile);
}

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

config = this.cache[directory];

if (config) {
debug("Using config from cache");
return config;
Expand Down Expand Up @@ -337,7 +325,7 @@ class Config {
/**
* Find local config files from directory and parent directories.
* @param {string} directory The directory to start searching from.
* @returns {string[]} The paths of local config files found.
* @returns {GeneratorFunction} The paths of local config files found.
*/
findLocalConfigFiles(directory) {

Expand Down
16 changes: 10 additions & 6 deletions lib/file-finder.js
View file
Expand Up @@ -25,6 +25,7 @@ const fs = require("fs"),
*/
function getDirectoryEntries(directory) {
try {

return fs.readdirSync(directory);
} catch (ex) {
return [];
Expand Down Expand Up @@ -79,9 +80,9 @@ class FileFinder {
* Searches for all the file names in this.fileNames.
* Is currently used by lib/config.js to find .eslintrc and package.json files.
* @param {string} directory The directory to start the search from.
* @returns {string[]} The file paths found.
* @returns {GeneratorFunction} to iterate the file paths found
*/
findAllInDirectoryAndParents(directory) {
*findAllInDirectoryAndParents(directory) {
const cache = this.cache;

if (directory) {
Expand All @@ -91,7 +92,8 @@ class FileFinder {
}

if (cache.hasOwnProperty(directory)) {
return cache[directory];
yield* cache[directory];
return; // to avoid doing the normal loop afterwards
}

const dirs = [];
Expand All @@ -114,27 +116,29 @@ class FileFinder {
for (let j = 0; j < searched; j++) {
cache[dirs[j]].push(filePath);
}

yield filePath;
break;
}
}
}

const child = directory;

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

if (directory === child) {
return cache[dirs[0]];
return;
}

} while (!cache.hasOwnProperty(directory));

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

return cache[dirs[0]];
yield* cache[dirs[0]];
}
}

Expand Down
5 changes: 1 addition & 4 deletions tests/fixtures/config-hierarchy/root-true/parent/.eslintrc
View file
@@ -1,8 +1,5 @@
{
"rules": {
"semi": [2, "always"],
"quotes": [2, "single"]
},
"rules": "bad_configurations",
"extends": [
"eslint-config-test"
]
Expand Down
27 changes: 15 additions & 12 deletions tests/lib/config.js
View file
Expand Up @@ -194,14 +194,16 @@ describe("Config", () => {
it("should return the path when an .eslintrc file is found", () => {
const configHelper = new Config({}, linter),
expected = getFakeFixturePath("broken", ".eslintrc"),
actual = configHelper.findLocalConfigFiles(getFakeFixturePath("broken"))[0];
actual = Array.from(
configHelper.findLocalConfigFiles(getFakeFixturePath("broken")));

assert.equal(actual, expected);
assert.equal(actual[0], expected);
});

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

assert.isArray(actual);
assert.lengthOf(actual, 0);
Expand All @@ -211,10 +213,9 @@ describe("Config", () => {
const configHelper = new Config({}, linter),
expected0 = getFakeFixturePath("packagejson", "subdir", "package.json"),
expected1 = getFakeFixturePath("packagejson", ".eslintrc"),
actual = configHelper.findLocalConfigFiles(getFakeFixturePath("packagejson", "subdir"));
actual = Array.from(
configHelper.findLocalConfigFiles(getFakeFixturePath("packagejson", "subdir")));

assert.isArray(actual);
assert.lengthOf(actual, 2);
assert.equal(actual[0], expected0);
assert.equal(actual[1], expected1);
});
Expand All @@ -224,7 +225,8 @@ describe("Config", () => {
expected = getFakeFixturePath("broken", ".eslintrc"),

// The first element of the array is the .eslintrc in the same directory.
actual = configHelper.findLocalConfigFiles(getFakeFixturePath("broken"));
actual = Array.from(
configHelper.findLocalConfigFiles(getFakeFixturePath("broken")));

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

actual = configHelper.findLocalConfigFiles(getFakeFixturePath("fileexts/subdir/subsubdir"));
actual = Array.from(
configHelper.findLocalConfigFiles(getFakeFixturePath("fileexts/subdir/subsubdir")));

assert.deepEqual(actual, expected);

assert.deepEqual(actual.length, expected.length);
});

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

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

config = configHelper.getConfig(firstpath);
assert.equal(config.rules["no-new"], 0);

config = configHelper.getConfig(secondpath);
assert.equal(config.rules["no-new"], 1);
});
Expand Down Expand Up @@ -528,7 +531,7 @@ describe("Config", () => {
});

// Project configuration - root set in second level .eslintrc
it("should not return configurations in parents of config with root:true", () => {
it("should not return or traverse configurations in parents of config with root:true", () => {
const configHelper = new Config({ cwd: process.cwd() }, linter),
file = getFixturePath("root-true", "parent", "root", "wrong-semi.js"),
expected = {
Expand Down
18 changes: 9 additions & 9 deletions tests/lib/file-finder.js
View file
Expand Up @@ -35,7 +35,7 @@ describe("FileFinder", () => {

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

/**
* Filter files outside of current workspace, otherwise test fails,
Expand Down

0 comments on commit 4df33e7

Please sign in to comment.