Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: add a fuzzer to detect bugs in core rules #8422

Merged
merged 4 commits into from Jul 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -5,6 +5,7 @@ build/
npm-debug.log
.DS_Store
tmp/
debug/
.idea
jsdoc/
versions.json
Expand Down
49 changes: 41 additions & 8 deletions Makefile.js
Expand Up @@ -51,6 +51,7 @@ const OPEN_SOURCE_LICENSES = [
const NODE = "node ", // intentional extra space
NODE_MODULES = "./node_modules/",
TEMP_DIR = "./tmp/",
DEBUG_DIR = "./debug/",
BUILD_DIR = "./build/",
DOCS_DIR = "../eslint.github.io/docs",
SITE_DIR = "../eslint.github.io/",
Expand All @@ -62,7 +63,7 @@ const NODE = "node ", // intentional extra space

// Files
MAKEFILE = "./Makefile.js",
JS_FILES = "\"lib/**/*.js\" \"conf/**/*.js\" \"bin/**/*.js\"",
JS_FILES = "\"lib/**/*.js\" \"conf/**/*.js\" \"bin/**/*.js\" \"tools/**/*.js\"",
JSON_FILES = find("conf/").filter(fileType("json")),
MARKDOWN_FILES_ARRAY = find("docs/").concat(ls(".")).filter(fileType("md")),
TEST_FILES = getTestFilePatterns(),
Expand All @@ -86,16 +87,12 @@ const NODE = "node ", // intentional extra space
* @private
*/
function getTestFilePatterns() {
const testLibPath = "tests/lib/",
testTemplatesPath = "tests/templates/",
testBinPath = "tests/bin/";

return ls(testLibPath).filter(pathToCheck => test("-d", testLibPath + pathToCheck)).reduce((initialValue, currentValues) => {
return ls("tests/lib/").filter(pathToCheck => test("-d", `tests/lib/${pathToCheck}`)).reduce((initialValue, currentValues) => {
if (currentValues !== "rules") {
initialValue.push(`"${testLibPath + currentValues}/**/*.js"`);
initialValue.push(`"tests/lib/${currentValues}/**/*.js"`);
}
return initialValue;
}, [`"${testLibPath}rules/**/*.js"`, `"${testLibPath}*.js"`, `"${testTemplatesPath}*.js"`, `"${testBinPath}**/*.js"`]).join(" ");
}, ["tests/lib/rules/**/*.js", "tests/lib/*.js", "tests/templates/*.js", "tests/bin/**/*.js", "tests/tools/*.js"]).join(" ");
}

/**
Expand Down Expand Up @@ -543,6 +540,42 @@ target.lint = function() {
}
};

target.fuzz = function() {
const fuzzerRunner = require("./tools/fuzzer-runner");
const fuzzResults = fuzzerRunner.run({ amount: process.env.CI ? 1000 : 300 });

if (fuzzResults.length) {
echo(`The fuzzer reported ${fuzzResults.length} error${fuzzResults.length === 1 ? "" : "s"}.`);

const formattedResults = JSON.stringify({ results: fuzzResults }, null, 4);

if (process.env.CI) {
echo("More details can be found below.");
echo(formattedResults);
} else {
if (!test("-d", DEBUG_DIR)) {
mkdir(DEBUG_DIR);
}

let fuzzLogPath;
let fileSuffix = 0;

// To avoid overwriting any existing fuzzer log files, append a numeric suffix to the end of the filename.
do {
fuzzLogPath = path.join(DEBUG_DIR, `fuzzer-log-${fileSuffix}.json`);
fileSuffix++;
} while (test("-f", fuzzLogPath));

formattedResults.to(fuzzLogPath);

// TODO: (not-an-aardvark) Create a better way to isolate and test individual fuzzer errors from the log file
echo(`More details can be found in ${fuzzLogPath}.`);
}

exit(1);
}
};

target.test = function() {
target.lint();
target.checkRuleFiles();
Expand Down
2 changes: 2 additions & 0 deletions package.json
Expand Up @@ -10,6 +10,7 @@
"scripts": {
"test": "node Makefile.js test",
"lint": "node Makefile.js lint",
"fuzz": "node Makefile.js fuzz",
"release": "node Makefile.js release",
"ci-release": "node Makefile.js ciRelease",
"alpharelease": "node Makefile.js prerelease -- alpha",
Expand Down Expand Up @@ -83,6 +84,7 @@
"eslint-plugin-eslint-plugin": "^0.7.2",
"eslint-plugin-node": "^5.0.0",
"eslint-release": "^0.10.1",
"eslump": "1.6.0",
"esprima": "^3.1.3",
"esprima-fb": "^15001.1001.0-dev-harmony-fb",
"istanbul": "^0.4.5",
Expand Down
258 changes: 258 additions & 0 deletions tests/tools/eslint-fuzzer.js
@@ -0,0 +1,258 @@
"use strict";

const assert = require("chai").assert;
const eslint = require("../..");
const espree = require("espree");
const sinon = require("sinon");
const configRule = require("../../lib/config/config-rule");

describe("eslint-fuzzer", function() {
let fakeRule, fuzz;

/*
* These tests take awhile because isolating which rule caused an error requires running eslint up to hundreds of
* times, one rule at a time.
*/
this.timeout(15000); // eslint-disable-line no-invalid-this

const linter = new eslint.Linter();
const coreRules = linter.getRules();
const fixableRuleNames = Array.from(coreRules)
.filter(rulePair => rulePair[1].meta && rulePair[1].meta.fixable)
.map(rulePair => rulePair[0]);
const CRASH_BUG = new TypeError("error thrown from a rule");

// A comment to disable all core fixable rules
const disableFixableRulesComment = `// eslint-disable-line ${fixableRuleNames.join(",")}`;

before(() => {
const realCoreRuleConfigs = configRule.createCoreRuleConfigs();

// Make sure the config generator generates a config for "test-fuzzer-rule"
sinon.stub(configRule, "createCoreRuleConfigs").returns(Object.assign(realCoreRuleConfigs, { "test-fuzzer-rule": [2] }));

// Create a closure around `fakeRule` so that tests can reassign it and have the changes take effect.
linter.defineRule("test-fuzzer-rule", Object.assign(context => fakeRule(context), { meta: { fixable: "code" } }));

fuzz = require("../../tools/eslint-fuzzer");
});

after(() => {
linter.reset();
configRule.createCoreRuleConfigs.restore();
});

describe("when running in crash-only mode", () => {
describe("when a rule crashes on the given input", () => {
it("should report the crash with a minimal config", () => {
fakeRule = () => ({
Program() {
throw CRASH_BUG;
}
});

const results = fuzz({ count: 1, codeGenerator: () => "foo", checkAutofixes: false, linter });

assert.strictEqual(results.length, 1);
assert.strictEqual(results[0].type, "crash");
assert.strictEqual(results[0].text, "foo");
assert.deepEqual(results[0].config.rules, { "test-fuzzer-rule": 2 });
assert.strictEqual(results[0].error, CRASH_BUG.stack);
});
});

describe("when no rules crash", () => {
it("should return an empty array", () => {
fakeRule = () => ({});

assert.deepEqual(fuzz({ count: 1, codeGenerator: () => "foo", checkAutofixes: false, linter }), []);
});
});
});

describe("when running in crash-and-autofix mode", () => {
const INVALID_SYNTAX = "this is not valid javascript syntax";
let expectedSyntaxError;

try {
espree.parse(INVALID_SYNTAX);
} catch (err) {
expectedSyntaxError = err;
}

describe("when a rule crashes on the given input", () => {
it("should report the crash with a minimal config", () => {
fakeRule = () => ({
Program() {
throw CRASH_BUG;
}
});

const results = fuzz({ count: 1, codeGenerator: () => "foo", checkAutofixes: false, linter });

assert.strictEqual(results.length, 1);
assert.strictEqual(results[0].type, "crash");
assert.strictEqual(results[0].text, "foo");
assert.deepEqual(results[0].config.rules, { "test-fuzzer-rule": 2 });
assert.strictEqual(results[0].error, CRASH_BUG.stack);
});
});

describe("when a rule's autofix produces valid syntax", () => {
it("does not report any errors", () => {

// Replaces programs that start with "foo" with "bar"
fakeRule = context => ({
Program(node) {
if (context.getSourceCode().text.startsWith("foo")) {
context.report({
node,
message: "no foos allowed",
fix: fixer => fixer.replaceText(node, `bar ${disableFixableRulesComment}`)
});
}
}
});

const results = fuzz({
count: 1,

/*
* To ensure that no other rules produce a different autofix and mess up the test, add a big disable
* comment for all core fixable rules.
*/
codeGenerator: () => `foo ${disableFixableRulesComment}`,
checkAutofixes: true,
linter
});

assert.deepEqual(results, []);
});
});

describe("when a rule's autofix produces invalid syntax on the first pass", () => {
it("reports an autofix error with a minimal config", () => {

// Replaces programs that start with "foo" with invalid syntax
fakeRule = context => ({
Program(node) {
const sourceCode = context.getSourceCode();

if (sourceCode.text.startsWith("foo")) {
context.report({
node,
message: "no foos allowed",
fix: fixer => fixer.replaceTextRange([0, sourceCode.text.length], INVALID_SYNTAX)
});
}
}
});

const results = fuzz({
count: 1,
codeGenerator: () => `foo ${disableFixableRulesComment}`,
checkAutofixes: true,
linter
});

assert.strictEqual(results.length, 1);
assert.strictEqual(results[0].type, "autofix");
assert.strictEqual(results[0].text, `foo ${disableFixableRulesComment}`);
assert.deepEqual(results[0].config.rules, { "test-fuzzer-rule": 2 });
assert.deepEqual(results[0].error, {
ruleId: null,
fatal: true,
severity: 2,
source: INVALID_SYNTAX,
message: `Parsing error: ${expectedSyntaxError.message}`,
line: expectedSyntaxError.lineNumber,
column: expectedSyntaxError.column
});
});
});

describe("when a rule's autofix produces invalid syntax on the second pass", () => {
it("reports an autofix error with a minimal config and the text from the second pass", () => {
const intermediateCode = `bar ${disableFixableRulesComment}`;

// Replaces programs that start with "foo" with invalid syntax
fakeRule = context => ({
Program(node) {
const sourceCode = context.getSourceCode();

if (sourceCode.text.startsWith("foo") || sourceCode.text.startsWith("bar")) {
context.report({
node,
message: "no foos allowed",
fix(fixer) {
return fixer.replaceTextRange(
[0, sourceCode.text.length],
sourceCode.text === intermediateCode ? INVALID_SYNTAX : intermediateCode
);
}
});
}
}
});

const results = fuzz({
count: 1,
codeGenerator: () => `foo ${disableFixableRulesComment}`,
checkAutofixes: true,
linter
});

assert.strictEqual(results.length, 1);
assert.strictEqual(results[0].type, "autofix");
assert.strictEqual(results[0].text, intermediateCode);
assert.deepEqual(results[0].config.rules, { "test-fuzzer-rule": 2 });
assert.deepEqual(results[0].error, {
ruleId: null,
fatal: true,
severity: 2,
source: INVALID_SYNTAX,
message: `Parsing error: ${expectedSyntaxError.message}`,
line: expectedSyntaxError.lineNumber,
column: expectedSyntaxError.column
});
});
});

describe("when a rule crashes on the second autofix pass", () => {
it("reports a crash error with a minimal config", () => {

// Replaces programs that start with "foo" with invalid syntax
fakeRule = context => ({
Program(node) {
const sourceCode = context.getSourceCode();

if (sourceCode.text.startsWith("foo")) {
context.report({
node,
message: "no foos allowed",
fix: fixer => fixer.replaceText(node, "bar")
});
} else if (sourceCode.text.startsWith("bar")) {
throw CRASH_BUG;
}
}
});

const results = fuzz({
count: 1,
codeGenerator: () => `foo ${disableFixableRulesComment}`,
checkAutofixes: true,
linter
});

assert.strictEqual(results.length, 1);
assert.strictEqual(results[0].type, "crash");

// TODO: (not-an-aardvark) It might be more useful to output the intermediate code here.
assert.strictEqual(results[0].text, `foo ${disableFixableRulesComment}`);
assert.deepEqual(results[0].config.rules, { "test-fuzzer-rule": 2 });
assert.strictEqual(results[0].error, CRASH_BUG.stack);
});
});
});
});