Skip to content

Commit

Permalink
Breaking: Error thrown/printed if no config found (fixes #5987) (#6538)
Browse files Browse the repository at this point in the history 
* Breaking: Error thrown/printed if no config found (fixes #5987)

* Edit message per feedback

I added @IanVS' suggestion of including the directory and ancestors-- let me know if it's too long as a result.

* Fix block comment to avoid using JSDoc

* Add tests to ensure no errors when configured

* Do not throw error if rules are specified

* Reorganizing unit tests for lib/config.js

* Reorganize code in lib/config

* Fixing JSDoc issues and unclear code
  • Loading branch information
@platinumazure @nzakas
platinumazure authored and nzakas committed Jun 28, 2016
1 parent 18663d4 commit e0d4b19
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 54 deletions.
46 changes: 41 additions & 5 deletions lib/config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function loadConfig(configToLoad) {

/**
* Get personal config object from ~/.eslintrc.
* @returns {Object} the personal config object (empty object if there is no personal config)
* @returns {Object} the personal config object (null if there is no personal config)
* @private
*/
function getPersonalConfig() {
Expand All @@ -87,7 +87,16 @@ function getPersonalConfig() {
}
}

return config || {};
return config || null;
}

/**
* Determine if rules were explicitly passed in as options.
* @param {Object} options The options used to create our configuration.
* @returns {boolean} True if rules were passed in as options, false otherwise.
*/
function hasRules(options) {
return options.rules && Object.keys(options.rules).length > 0;
}

/**
Expand All @@ -105,7 +114,8 @@ function getLocalConfig(thisConfig, directory) {
localConfigFiles = thisConfig.findLocalConfigFiles(directory),
numFiles = localConfigFiles.length,
rootPath,
projectConfigPath = ConfigFile.getFilenameForDirectory(thisConfig.options.cwd);
projectConfigPath = ConfigFile.getFilenameForDirectory(thisConfig.options.cwd),
personalConfig;

for (i = 0; i < numFiles; i++) {

Expand Down Expand Up @@ -140,8 +150,34 @@ function getLocalConfig(thisConfig, directory) {
config = ConfigOps.merge(localConfig, config);
}

// Use the personal config file if there are no other local config files found.
return found || thisConfig.useSpecificConfig ? config : ConfigOps.merge(config, getPersonalConfig());
if (!found && !thisConfig.useSpecificConfig) {

/*
* - Is there a personal config in the user's home directory? If so,
* merge that with the passed-in config.
* - Otherwise, if no rules were manually passed in, throw and error.
* - Note: This function is not called if useEslintrc is false.
*/
personalConfig = getPersonalConfig();

if (personalConfig) {
config = ConfigOps.merge(config, personalConfig);
} else if (!hasRules(thisConfig.options)) {

// No config file, no manual configuration, and no rules, so error.
var noConfigError = new Error("No ESLint configuration found.");

noConfigError.messageTemplate = "no-config-found";
noConfigError.messageData = {
directory: directory,
filesExamined: localConfigFiles
};

throw noConfigError;
}
}

return config;
}

//------------------------------------------------------------------------------
Expand Down
7 changes: 7 additions & 0 deletions messages/no-config-found.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
ESLint couldn't find a configuration file. To set up a configuration file for this project, please run:

eslint --init

ESLint looked for configuration files in <%= directory %> and its ancestors.

If you think you already have a configuration file or if you need more help, please stop by the ESLint chat room: https://gitter.im/eslint/eslint
3 changes: 3 additions & 0 deletions tests/fixtures/source-code-util/.eslintrc.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
// This file is only to ensure no error is thrown due to lack of config.
}
163 changes: 114 additions & 49 deletions tests/lib/config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,17 @@ describe("Config", function() {
return path.join.apply(path, args);
}

/**
* Mocks the current CWD path
* @param {string} fakeCWDPath - fake CWD path
* @returns {void}
* @private
*/
function mockCWDResponse(fakeCWDPath) {
sandbox.stub(process, "cwd")
.returns(fakeCWDPath);
}

// copy into clean area so as not to get "infected" by this project's .eslintrc files
before(function() {
fixtureDir = os.tmpdir() + "/eslint/fixtures";
Expand Down Expand Up @@ -251,13 +262,15 @@ describe("Config", function() {
assert.equal(config.rules["no-new"], 1);
});

it("should return a default config when an invalid path is given", function() {
it("should throw an error when an invalid path is given", function() {
var configPath = path.resolve(__dirname, "..", "fixtures", "configurations", "foobaz", ".eslintrc");
var configHelper = new Config({cwd: process.cwd()});

sandbox.stub(fs, "readdirSync").throws(new Error());

assert.isObject(configHelper.getConfig(configPath));
assert.throws(function() {
configHelper.getConfig(configPath);
}, "No ESLint configuration found.");
});

it("should throw error when a configuration file doesn't exist", function() {
Expand Down Expand Up @@ -780,7 +793,6 @@ describe("Config", function() {
});

describe("personal config file within home directory", function() {
var getCwd;

/**
* Returns the path inside of the fixture directory.
Expand Down Expand Up @@ -812,19 +824,7 @@ describe("Config", function() {
});
}

/**
* Mocks the current CWD path
* @param {string} fakeCWDPath - fake CWD path
* @returns {undefined}
* @private
*/
function mockCWDResponse(fakeCWDPath) {
getCwd = sinon.stub(process, "cwd");
getCwd.returns(fakeCWDPath);
}

afterEach(function() {
getCwd.restore();
mockFs.restore();
});

Expand Down Expand Up @@ -904,12 +904,11 @@ describe("Config", function() {
assert.deepEqual(actual, expected);
});

it("should have an empty config if no local config and no personal config was found", function() {
var projectPath = getFakeFixturePath("personal-config", "project-without-config"),
homePath = getFakeFixturePath("personal-config", "folder-does-not-exist"),
filePath = getFakeFixturePath("personal-config", "project-without-config", "foo.js");
it("should still load the project config if the current working directory is the same as the home folder", function() {
var projectPath = getFakeFixturePath("personal-config", "project-with-config"),
filePath = getFakeFixturePath("personal-config", "project-with-config", "subfolder", "foo.js");

var StubbedConfig = proxyquire("../../lib/config", { "user-home": homePath });
var StubbedConfig = proxyquire("../../lib/config", { "user-home": projectPath });

mockPersonalConfigFileSystem();
mockCWDResponse(projectPath);
Expand All @@ -921,13 +920,70 @@ describe("Config", function() {
env: {},
globals: {},
parser: undefined,
rules: {}
rules: {
"project-level-rule": 2,
"subfolder-level-rule": 2
}
};

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

describe("when no local or personal config is found", function() {

/**
* Returns the path inside of the fixture directory.
* @returns {string} The path inside the fixture directory.
* @private
*/
function getFakeFixturePath() {
var args = Array.prototype.slice.call(arguments);

args.unshift("config-hierarchy");
args.unshift("fixtures");
args.unshift("eslint");
args.unshift(process.cwd());
return path.join.apply(path, args);
}

/**
* Mocks the file system for personal-config files
* @returns {undefined}
* @private
*/
function mockPersonalConfigFileSystem() {
mockFs({
eslint: {
fixtures: {
"config-hierarchy": DIRECTORY_CONFIG_HIERARCHY
}
}
});
}

afterEach(function() {
mockFs.restore();
});

it("should have an empty config if no local config was found and ~/package.json contains no eslintConfig section", function() {
it("should throw an error if no local config and no personal config was found", function() {
var projectPath = getFakeFixturePath("personal-config", "project-without-config"),
homePath = getFakeFixturePath("personal-config", "folder-does-not-exist"),
filePath = getFakeFixturePath("personal-config", "project-without-config", "foo.js");

var StubbedConfig = proxyquire("../../lib/config", { "user-home": homePath });

mockPersonalConfigFileSystem();
mockCWDResponse(projectPath);

var config = new StubbedConfig({ cwd: process.cwd() });

assert.throws(function() {
config.getConfig(filePath);
}, "No ESLint configuration found");
});

it("should throw an error if no local config was found and ~/package.json contains no eslintConfig section", function() {
var projectPath = getFakeFixturePath("personal-config", "project-without-config"),
homePath = getFakeFixturePath("personal-config", "home-folder-with-packagejson"),
filePath = getFakeFixturePath("personal-config", "project-without-config", "foo.js");
Expand All @@ -937,42 +993,51 @@ describe("Config", function() {
mockPersonalConfigFileSystem();
mockCWDResponse(projectPath);

var config = new StubbedConfig({ cwd: process.cwd() }),
actual = config.getConfig(filePath),
expected = {
parserOptions: {},
env: {},
globals: {},
parser: undefined,
rules: {}
};
var configHelper = new StubbedConfig({ cwd: process.cwd() });

assert.deepEqual(actual, expected);
assert.throws(function() {
configHelper.getConfig(filePath);
}, "No ESLint configuration found");
});

it("should still load the project config if the current working directory is the same as the home folder", function() {
var projectPath = getFakeFixturePath("personal-config", "project-with-config"),
filePath = getFakeFixturePath("personal-config", "project-with-config", "subfolder", "foo.js");
it("should not throw an error if no local config and no personal config was found but useEslintrc is false", function() {
var projectPath = getFakeFixturePath("personal-config", "project-without-config"),
homePath = getFakeFixturePath("personal-config", "folder-does-not-exist"),
filePath = getFakeFixturePath("personal-config", "project-without-config", "foo.js");

var StubbedConfig = proxyquire("../../lib/config", { "user-home": projectPath });
var StubbedConfig = proxyquire("../../lib/config", { "user-home": homePath });

mockPersonalConfigFileSystem();
mockCWDResponse(projectPath);

var config = new StubbedConfig({ cwd: process.cwd() }),
actual = config.getConfig(filePath),
expected = {
parserOptions: {},
env: {},
globals: {},
parser: undefined,
rules: {
"project-level-rule": 2,
"subfolder-level-rule": 2
}
};
var config = new StubbedConfig({
cwd: process.cwd(),
useEslintrc: false
});

assert.deepEqual(actual, expected);
assert.doesNotThrow(function() {
config.getConfig(filePath);
}, "No ESLint configuration found");
});

it("should not throw an error if no local config and no personal config was found but rules are specified", function() {
var projectPath = getFakeFixturePath("personal-config", "project-without-config"),
homePath = getFakeFixturePath("personal-config", "folder-does-not-exist"),
filePath = getFakeFixturePath("personal-config", "project-without-config", "foo.js");

var StubbedConfig = proxyquire("../../lib/config", { "user-home": homePath });

mockPersonalConfigFileSystem();
mockCWDResponse(projectPath);

var config = new StubbedConfig({
cwd: process.cwd(),
rules: { quotes: [2, "single"] }
});

assert.doesNotThrow(function() {
config.getConfig(filePath);
}, "No ESLint configuration found");
});
});
});
Expand Down

0 comments on commit e0d4b19

Please sign in to comment.