Skip to content

Commit

Permalink
Chore: improve coverage in lib/*.js (#9130)
Browse files Browse the repository at this point in the history 
* Chore: remove dead code path in ast-utils

This was checking to see if a function is the left child of an AssignmentExpression, which isn't possible.

* Chore: fix incorrect test in cli.js

This test was intended to verify that --print-config was disallowed with --stdin. However, the test didn't have an argument with --print-config, so the command failed option validation and never made it to the logic that it was supposed to test.

* Chore: avoid dead code branches in config logic

* Chore: Remove dead code in ignored-paths.js

* Chore: remove dead code path for environments

* Chore: Add istanbul ignore comment for sanity check error

* Chore: remove dead code from getRuleSeverity

* Chore: improve coverage of linter.markVariableAsUsed

* Chore: remove dead code branch in parserOptions handling

* Chore: improve coverage of linter.verifyAndFix

* Chore: improve coverage of RuleContext fix merging
  • Loading branch information
@not-an-aardvark
not-an-aardvark committed Aug 21, 2017
1 parent ff8c4bb commit 60c5148
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 57 deletions.
19 changes: 9 additions & 10 deletions lib/ast-utils.js
View file
Expand Up @@ -655,16 +655,15 @@ module.exports = {
// [Foo = function() { ... }] = a;
case "AssignmentExpression":
case "AssignmentPattern":
if (parent.right === node) {
if (parent.left.type === "MemberExpression") {
return false;
}
if (isAnonymous &&
parent.left.type === "Identifier" &&
startsWithUpperCase(parent.left.name)
) {
return false;
}
if (parent.left.type === "MemberExpression") {
return false;
}
if (
isAnonymous &&
parent.left.type === "Identifier" &&
startsWithUpperCase(parent.left.name)
) {
return false;
}
return true;

Expand Down
20 changes: 8 additions & 12 deletions lib/config.js
View file
Expand Up @@ -24,7 +24,7 @@ const debug = require("debug")("eslint:config");
// Constants
//------------------------------------------------------------------------------

const PERSONAL_CONFIG_DIR = os.homedir() || null;
const PERSONAL_CONFIG_DIR = os.homedir();
const SUBCONFIG_SEP = ":";

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -148,15 +148,13 @@ class Config {
getPersonalConfig() {
if (typeof this.personalConfig === "undefined") {
let config;
const filename = ConfigFile.getFilenameForDirectory(PERSONAL_CONFIG_DIR);

if (PERSONAL_CONFIG_DIR) {
const filename = ConfigFile.getFilenameForDirectory(PERSONAL_CONFIG_DIR);

if (filename) {
debug("Using personal config");
config = ConfigFile.load(filename, this);
}
if (filename) {
debug("Using personal config");
config = ConfigFile.load(filename, this);
}

this.personalConfig = config || null;
}

Expand Down Expand Up @@ -351,10 +349,8 @@ class Config {
config = ConfigOps.merge(config, { parser: this.parser });
}

// Step 4: Apply environments to the config if present
if (config.env) {
config = ConfigOps.applyEnvironments(config, this.linterContext.environments);
}
// Step 4: Apply environments to the config
config = ConfigOps.applyEnvironments(config, this.linterContext.environments);

this.configCache.setMergedConfig(vector, config);

Expand Down
2 changes: 0 additions & 2 deletions lib/ignored-paths.js
View file
Expand Up @@ -47,8 +47,6 @@ const DEFAULT_OPTIONS = {
* @returns {string} Path of ignore file or an empty string.
*/
function findFile(cwd, name) {
cwd = cwd || DEFAULT_OPTIONS.cwd;

const ignoreFilePath = path.resolve(cwd, name);

return fs.existsSync(ignoreFilePath) && fs.statSync(ignoreFilePath).isFile() ? ignoreFilePath : "";
Expand Down
43 changes: 16 additions & 27 deletions lib/linter.js
View file
Expand Up @@ -172,13 +172,11 @@ function addDeclaredGlobals(program, globalScope, config, envContext) {
Object.assign(declaredGlobals, builtin);

Object.keys(config.env).forEach(name => {
if (config.env[name]) {
const env = envContext.get(name),
environmentGlobals = env && env.globals;
const env = envContext.get(name),
environmentGlobals = env && env.globals;

if (environmentGlobals) {
Object.assign(declaredGlobals, environmentGlobals);
}
if (environmentGlobals) {
Object.assign(declaredGlobals, environmentGlobals);
}
});

Expand Down Expand Up @@ -531,6 +529,8 @@ function createStubRule(message) {
if (message) {
return createRuleModule;
}

/* istanbul ignore next */
throw new Error("No message passed to stub rule");

}
Expand Down Expand Up @@ -595,13 +595,7 @@ function stripUnicodeBOM(text) {
* @returns {number} 0, 1, or 2, indicating rule severity
*/
function getRuleSeverity(ruleConfig) {
if (typeof ruleConfig === "number") {
return ruleConfig;
} else if (Array.isArray(ruleConfig)) {
return ruleConfig[0];
}
return 0;

return Array.isArray(ruleConfig) ? ruleConfig[0] : ruleConfig;
}

/**
Expand Down Expand Up @@ -632,15 +626,15 @@ function getRuleOptions(ruleConfig) {
*/
function parse(text, config, filePath, messages) {

let parser,
parserOptions = {
loc: true,
range: true,
raw: true,
tokens: true,
comment: true,
filePath
};
let parser;
const parserOptions = Object.assign({}, config.parserOptions, {
loc: true,
range: true,
raw: true,
tokens: true,
comment: true,
filePath
});

try {
parser = require(config.parser);
Expand All @@ -658,11 +652,6 @@ function parse(text, config, filePath, messages) {
return null;
}

// merge in any additional parser options
if (config.parserOptions) {
parserOptions = Object.assign({}, config.parserOptions, parserOptions);
}

/*
* Check for parsing errors first. If there's a parsing error, nothing
* else can happen. However, a parsing error does not throw an error
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/cli.js
View file
Expand Up @@ -880,7 +880,7 @@ describe("cli", () => {
});

it("should error out when executing on text", () => {
const exitCode = cli.execute("--print-config", "foo = bar;");
const exitCode = cli.execute("--print-config=myFile.js", "foo = bar;");

assert.isTrue(log.info.notCalled);
assert.isTrue(log.error.calledOnce);
Expand Down
3 changes: 3 additions & 0 deletions tests/lib/config.js
View file
Expand Up @@ -881,6 +881,9 @@ describe("Config", () => {
};

assert.deepEqual(actual, expected);

// Ensure that the personal config is cached and isn't reloaded on every call
assert.strictEqual(config.getPersonalConfig(), config.getPersonalConfig());
});

it("should ignore the personal config if a local config was found", () => {
Expand Down
39 changes: 34 additions & 5 deletions tests/lib/linter.js
View file
Expand Up @@ -639,7 +639,7 @@ describe("Linter", () => {

linter.reset();
linter.on("Program:exit", () => {
linter.markVariableAsUsed("a");
assert.isTrue(linter.markVariableAsUsed("a"));

const scope = linter.getScope();

Expand All @@ -654,7 +654,7 @@ describe("Linter", () => {

linter.reset();
linter.on("ReturnStatement", () => {
linter.markVariableAsUsed("a");
assert.isTrue(linter.markVariableAsUsed("a"));

const scope = linter.getScope();

Expand All @@ -669,7 +669,7 @@ describe("Linter", () => {

linter.reset();
linter.on("ReturnStatement", () => {
linter.markVariableAsUsed("a");
assert.isTrue(linter.markVariableAsUsed("a"));
});
linter.on("Program:exit", () => {
const scope = linter.getScope();
Expand All @@ -689,7 +689,7 @@ describe("Linter", () => {
const globalScope = linter.getScope(),
childScope = globalScope.childScopes[0];

linter.markVariableAsUsed("a");
assert.isTrue(linter.markVariableAsUsed("a"));

assert.isTrue(getVariable(childScope, "a").eslintUsed);
assert.isUndefined(getVariable(childScope, "b").eslintUsed);
Expand All @@ -706,14 +706,25 @@ describe("Linter", () => {
const globalScope = linter.getScope(),
childScope = globalScope.childScopes[0];

linter.markVariableAsUsed("a");
assert.isTrue(linter.markVariableAsUsed("a"));

assert.isTrue(getVariable(childScope, "a").eslintUsed);
assert.isUndefined(getVariable(childScope, "b").eslintUsed);
});

linter.verify(code, { parserOptions: { sourceType: "module" } }, filename, true);
});

it("should return false if the given variable is not found", () => {
const code = "var a = 1, b = 2;";

linter.reset();
linter.on("Program:exit", () => {
assert.isFalse(linter.markVariableAsUsed("c"));
});

linter.verify(code, {}, filename, true);
});
});

describe("report()", () => {
Expand Down Expand Up @@ -3829,6 +3840,24 @@ describe("Linter", () => {

assert.strictEqual(fixResult.fixed, false);
});

it("stops fixing after 10 passes", () => {
linter.defineRule("add-spaces", context => ({
Program(node) {
context.report({
node,
message: "Add a space before this node.",
fix: fixer => fixer.insertTextBefore(node, " ")
});
}
}));

const fixResult = linter.verifyAndFix("a", { rules: { "add-spaces": "error" } });

assert.strictEqual(fixResult.fixed, true);
assert.strictEqual(fixResult.output, `${" ".repeat(10)}a`);
assert.strictEqual(fixResult.messages.length, 1);
});
});

describe("Edge cases", () => {
Expand Down
28 changes: 28 additions & 0 deletions tests/lib/rule-context.js
View file
Expand Up @@ -164,6 +164,34 @@ describe("RuleContext", () => {
mockESLint.verify();
});

it("should pass through fixes if only one is present", () => {
const mockESLint = sandbox.mock(eslint);

mockESLint.expects("getSourceCode").returns({ text: "var foo = 100;" });
mockESLint.expects("report").once().withArgs(
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match({
range: [10, 13],
text: "234"
})
);

ruleContext.report({
node: {},
loc: {},
message: "Message",
*fix(fixer) {
yield fixer.replaceTextRange([10, 13], "234");
}
});

mockESLint.verify();
});

it("should handle inserting BOM correctly.", () => {
const mockESLint = sandbox.mock(eslint);
Expand Down

0 comments on commit 60c5148

Please sign in to comment.