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

Fix: --quiet no longer fixes warnings (fixes #8675) #8858

Merged
merged 2 commits into from
Jul 4, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 12 additions & 1 deletion lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@ const debug = require("debug")("eslint:cli");
// Helpers
//------------------------------------------------------------------------------

/**
* Predicate function for whether or not to apply fixes in quiet mode.
* If a message is a warning, do not apply a fix.
* @param {LintResult} lintResult The lint result.
* @returns {boolean} True if the lint message is an error (and thus should be
* autofixed), false otherwise.
*/
function quietFixPredicate(lintResult) {
return Boolean(lintResult && lintResult.severity === 2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't this be return lintResult.severity === 2? Is lintResult ever null?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably should never be null. Normally I favor defensive coding to be safe; but since this could be a tight path (running this function on every lint message), I am happy to shave off those cycles. I'll update tonight (US).

}

/**
* Translates the CLI options into the options expected by the CLIEngine.
* @param {Object} cliOptions The CLI options to translate.
Expand All @@ -52,7 +63,7 @@ function translateOptions(cliOptions) {
cache: cliOptions.cache,
cacheFile: cliOptions.cacheFile,
cacheLocation: cliOptions.cacheLocation,
fix: cliOptions.fix,
fix: cliOptions.fix && (cliOptions.quiet ? quietFixPredicate : true),
allowInlineConfig: cliOptions.inlineConfig
};
}
Expand Down
12 changes: 12 additions & 0 deletions tests/bin/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ describe("bin/eslint.js", () => {
const tempFilePath = `${fixturesPath}/temp.js`;
const startingText = fs.readFileSync(`${fixturesPath}/left-pad.js`).toString();
const expectedFixedText = fs.readFileSync(`${fixturesPath}/left-pad-expected.js`).toString();
const expectedFixedTextQuiet = fs.readFileSync(`${fixturesPath}/left-pad-expected-quiet.js`).toString();

beforeEach(() => {
fs.writeFileSync(tempFilePath, startingText);
Expand All @@ -127,6 +128,17 @@ describe("bin/eslint.js", () => {
return Promise.all([exitCodeAssertion, outputFileAssertion]);
});

it("has exit code 0, fixes errors in a file, and does not report or fix warnings if --quiet and --fix are used", () => {
const child = runESLint(["--fix", "--quiet", "--no-eslintrc", "--no-ignore", tempFilePath]);
const exitCodeAssertion = assertExitCode(child, 0);
const stdoutAssertion = getStdout(child).then(stdout => assert.strictEqual(stdout, ""));
const outputFileAssertion = awaitExit(child).then(() => {
assert.strictEqual(fs.readFileSync(tempFilePath).toString(), expectedFixedTextQuiet);
});

return Promise.all([exitCodeAssertion, stdoutAssertion, outputFileAssertion]);
});

it("has exit code 1 and fixes a file if not all rules can be fixed", () => {
const child = runESLint(["--fix", "--no-eslintrc", "--no-ignore", "--rule", "max-len: [2, 10]", tempFilePath]);
const exitCodeAssertion = assertExitCode(child, 1);
Expand Down
33 changes: 33 additions & 0 deletions tests/fixtures/autofix-integration/left-pad-expected-quiet.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/* eslint dot-notation: 2 */
/* eslint indent: [2, 2] */
/* eslint no-extra-parens: 2 */
/* eslint no-implicit-coercion: 2 */
/* eslint no-whitespace-before-property: 2 */
/* eslint quotes: [2, "single"] */
/* eslint semi: 2 */
/* eslint semi-spacing: 2 */
/* eslint space-before-function-paren: 2 */
/* eslint space-before-blocks: 1 */

/*
* left-pad@0.0.3
* WTFPL https://github.com/stevemao/left-pad/blob/aff6d744155a70b81f09effb8185a1564f348462/COPYING
*/

module.exports = leftpad;

function leftpad (str, len, ch){
str = String(str);

var i = -1;

ch || (ch = ' ');
len = len - str.length;


while (++i < len) {
str = ch + str;
}

return str;
}
1 change: 1 addition & 0 deletions tests/fixtures/autofix-integration/left-pad-expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
/* eslint semi: 2 */
/* eslint semi-spacing: 2 */
/* eslint space-before-function-paren: 2 */
/* eslint space-before-blocks: 1 */

/*
* left-pad@0.0.3
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/autofix-integration/left-pad.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
/* eslint semi: 2 */
/* eslint semi-spacing: 2 */
/* eslint space-before-function-paren: 2 */
/* eslint space-before-blocks: 1 */

/*
* left-pad@0.0.3
Expand All @@ -15,7 +16,7 @@

module.exports = (leftpad)

function leftpad(str, len, ch) {
function leftpad(str, len, ch){
str = ("" + str);

var i = -1 ;
Expand Down
4 changes: 2 additions & 2 deletions tests/lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ describe("cli", () => {

});

it("should rewrite files when in fix mode and quiet mode", () => {
it("should provide fix predicate and rewrite files when in fix mode and quiet mode", () => {

const report = {
errorCount: 0,
Expand All @@ -822,7 +822,7 @@ describe("cli", () => {
};

// create a fake CLIEngine to test with
const fakeCLIEngine = sandbox.mock().withExactArgs(sinon.match({ fix: true }));
const fakeCLIEngine = sandbox.mock().withExactArgs(sinon.match({ fix: sinon.match.func }));

fakeCLIEngine.prototype = leche.fake(CLIEngine.prototype);
sandbox.stub(fakeCLIEngine.prototype, "executeOnFiles").returns(report);
Expand Down