[Request] use constants for linter error levels

[braco]

My team has forked this project a couple times because linting rules are mostly 2/error instead of 1/warning, so it obscures genuine errors.

Something like this:

<Foo b={} a={} /> // props sorted wrong

Should not have the same severity as a critical bug! This adds a ton of mental overhead to every line when red dots are popping up everywhere: "will this code throw an error or is there some minor formatting issue?"

Needing to be so precious with every line slows down development when you may just be testing some additions or wading into third party code.

Doubly with autofix enabled, Atom currently fixes a ton of formatting inconsistencies on save, I can safely ignore some warnings now.

However, I understand your logic in #853 – when you need to enforce absolute consistency in your team, saying "you can't proceed without fixing this niggle" is important. I would argue that should go in a commit hook or similar, but...

why not use a custom constant to define the error levels? This would allow the linting logic to be more flexible:

{ rule: airbnb.ERROR }

[ljharb]

Everything is a genuine error, and yes, that most certainly should have the same severity as a critical bug. No formatting issues are minor, because they can obscure serious bugs and architectural issues in the code that are otherwise unlintable. In other words, you DO need to be so precious with every line - and if that slows down development, so be it: speed is not more important than correctness and rigor.

If you have errors autofixed on save, and you don't want to see them, can you not just hit ⌘-S or ctrl-S and remove them?

I'm leaving this open because it is an interesting idea to use environment variables to override each rule category's severity - it would not be more granular than "one env var per file", but that would indeed make it easy to override some or all of the groups of rules' severities, without having to fork and/or replicate the config.

[braco]

@ljharb: thanks for the discussion.

The logic you're using in that first link is a bit like the brown M&M test – if you suck at this part, you probably suck at other parts. That's a good rule of thumb, but that could also be enforced by rejecting PR/pushes with any warnings, among other things. In the meantime, rephrasing "warning, this might be obscuring an error" as "this is an error" is an extreme style in itself and broadly impacts everyone that uses this popular and thoughtful ruleset. (My team doesn't check up any code with linter warnings, for example, but the in-editor distinction is kept)

Formatting is of utmost importance, but there is a difference between "this should be fixed" and "this code will not compile". It's easy to come up with some silly examples, like errors from a trailing space inside a comment block sitting next to errors from a missing comma inside an object init. I guess there's probably an argument in here for a protected eslint level or editor differentiation for compile errors...

Regarding saving to preemptively autofix formatting: it doesn't fix everything yet, and if you've got some form of hot reload/autobuild going, you may be loading a critically broken script. That's why I would scan for red-dot errors first.

I think much of this comes down to workflow preferences, and the constant would go a long way into making the package more flexible. Thanks for considering, and for all your output here in general.

[ljharb]

The M&M clause is a very apt analogy, yes :-)

Things that might be obscuring errors, are themselves errors. I totally understand there are occasionally workflows where fast iteration matters more than correctness/style, but those aren't the majority, and it's better to inconvenience those people, and risk that they're slower, then to risk that the majority has more brittle code.

One concern I've heard about the env var approach is ease of configurability in editors, which is where most people would probably prefer the flexibility. Those interested in this configurability can help by commenting with ways that demonstrate how to configure env vars for linting in popular editors/IDEs :-)

[callumlocke]

@ljharb:
Everything is a genuine error

Totally agree this is the right default for eslint-config-airbnb.

But we have special requirements. We want to import all the Airbnb rules as warnings, and then selectively turn a handful of rules back into hard errors. We are a mixed ability team in a newsroom, and we rely on code contributions from journalist-developers (which usually then get cleaned up by more 'expert' JS devs). We need to be flexible; hard enforcement of all rules would be a non-starter for us, but this shouldn't rule us out from being able to improve our coding standard over time via gradual adoption of Airbnb's eslint config. We want to gradually teach people code quality via in-editor lint highlighting. And we also want to enforce a small handful of rules as hard errors with a pre-commit-hook. So ESLint's two-tier severity system is a perfect fit for us.

Currently though, there's no clean way to grab all of Airbnb's rules and convert them to warnings. We're doing something like this, which is pretty messy and breaks unpredictably whenever this repo's structure changes:

// eslintrc.js

const rules = Object.assign({},
  // this must be manually kept in sync with the eslint-config-airbnb repo
  require('eslint-config-airbnb-base/rules/es6').rules,
  require('eslint-config-airbnb-base/rules/best-practices').rules,
  require('eslint-config-airbnb-base/rules/style').rules,
  require('eslint-config-airbnb-base/rules/errors').rules,
  require('eslint-config-airbnb-base/rules/variables').rules
);

const config = {
  extends: 'airbnb',
  parser: 'babel-eslint',
  plugins: ['babel'],
  // convert all rules into warnings
  rules: Object.keys(rules).reduce((last, curr) => {
    let rule = rules[curr];
    if (Array.isArray(rule) && (rule[0] === 2 || rule[0] === 'error')) {
      rule[0] = 1;
    } else if (rule === 2 || rule === 'error') {
      rule = 1;
    }

    last[curr] = rule;
    return last;
  }, {})
};

// make a few hard errors
const overrides = {
  // ...
};

Object.assign(config.rules, overrides);
module.exports = config;

This is what I really want:

# eslintrc.yml

extends: airbnb/soft # like airbnb, but all rules are warnings
rules:
  # ...a few hard errors

[ljharb]

@callumlocke Rather than an entry point, the current suggestion I'm entertaining would involve still extending "airbnb", but setting an environment variable in your npm run-script as part of your eslint command.