eslint: --strict for counting warnings as errors

Soliciting feedback on the idea before doing the work. My apologies if this is already doable, but I didn’t find anything thus far…

Scenario

Linting is ran before nodemon restarts, the build runs, etc.
ESLint ensures that any syntax issues cause an error, preventing unnecessary server restarts, builds, etc.
Warnings are available in the console for the developer to optionally clean up.
Code works, builds run, servers restart, but there’s no way to actually enforce standards without making everything incorrectly an error.

Solution – --strict.

A pre-push git hook can run eslint --strict and even warnings count as errors, preventing pushing.
Continuous integration tools can run in strict mode so that the build breaks when quality tests don’t pass.
The user can still work on a feature with warnings, since the most important thing is the feature working.

The gist is, quality should still be enforced somewhere in the process, but not be an impediment to development.

Thoughts?

About this issue

Original URL
State: closed
Created 9 years ago
Comments: 16 (7 by maintainers)

Most upvoted comments

--max-warnings can serve this purpose: --max-warnings=0 means that any warnings cause eslint to report an error.

+35

joshkel on May 17, 2016

Again, just don’t run the linter during development. Your one counterexample (syntax errors) can be solved by running a parser against the code as you write it, something many editors (including vim) will do for you.

michaelficarra on Apr 14, 2015

@michaelficarra As indicated in the Scenario above, when you’re working on a complex project, you’ll likely have console.log littered everywhere, several unused vars & things.

The linter also catches syntax & JSX errors, which is crucial to know before restarting several services/fixtures between changes.

I don’t want these in the final project output, but they’re a necessity for debugging and getting a feature working.

It’s unreasonable & costly to expect complex development to be 100% clean the whole time, which is why warnings shouldn’t impede development, while errors should. Similarly, warnings shouldn’t exist in the final, merged code, IMO.

ericclemmons on Apr 14, 2015

Old issue, but if anybody is looking for a solution, this is what we’re doing:

// .eslintrc.js
function isTruthy(value) {
  if (!value) return false;
  return ['1', 'true'].indexOf(value.toLowerCase()) >= 0;
}

// Warnings are errors in CI
var OFF = 'off';
var ERROR = 'error';
var WARNING = isTruthy(process.env.CI) ? ERROR : 'warn';

module.exports = {
  // ...
  "rules": {
    "comma-dangle": OFF,
    "eqeqeq": [WARNING, "allow-null"],
    "import/imports-first": OFF,
    "indent": [WARNING, 2, {"SwitchCase": 1}],
    "max-len": [WARNING, 100, 2],
    "no-console": [WARNING, {"allow": ["warn", "error"]}],
    "no-debugger": WARNING,
    "no-fallthrough": WARNING,
    "no-unreachable": WARNING,
    "no-unused-vars": [WARNING, {"vars": "all", "args": "none"}],
    "no-var": ERROR,
    "prefer-const": WARNING,
    "react/prop-types": [WARNING, {"ignore": ["className"]}],
    "semi": [WARNING, "never"],
  },
  // ...
}

rhys-vdw on Mar 15, 2017