Rules do not notify user on invalid config options (Also, inconsistent failing)

[jonvuri]

This is kind of two issues but if the first part is agreed upon and fixed it also fixes the second:

1. Rules do not currently notify users when they are passed invalid configs. If a rule takes arguments "always" and "never", it won't complain when it's passed [2, "fonzie"]. Ideally this would cause an error along the lines of "invalid configuration for rule 'blah'", since if the user explicitly passed something but it wasn't a valid option, they'd pretty much always want to know about that and correct it before continuing. A warning would be alright too, but to me an error seems more appropriate. A warning has the chance of being skimmed over, so that the incorrect config will just sit there silently and cause confusion later on.
2. Rules act differently when given an invalid config. For instance, when you pass this file:

var color = "purple";

To eslint --reset with config:

{
  "rules": {
    "semi": [2, "nevr"]
  }
}

It will silently fail (to meet the intent of the user, that is), returning no linting errors as if the option were the default of [2, "always"]. It will also return linting errors if you remove the semicolon.

However, if you pass this file:

var palette = {color: "purple"};

To eslint --reset with config:

{
  "rules": {
    "space-in-brackets": [2, "alwas"]
  }
}

It will also silently fail to return any errors. However, it also won't return any errors for { color: "purple" }! Unlike "semi", which just branches of off !== "never", this rule branches off of === "never" and === "always". If the config argument is neither of those two, the rule will run but never actually report anything.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[nzakas]

This is an interesting problem. I think there's a fundamental question here: is an incorrect configuration a fatal error not? As in, should a misconfiguration prevent prevent you from linting?

If the answer is that it is a fatal error, the question becomes how to add a config validation phase before starting to lint. If otherwise, then what has the responsibility of validating and reporting?

I don't know the answer right now, need to think about it some more.

[goloroden]

Just some thoughts, maybe they are helpful (if not, feel free to ignore them):

When you are using a tool such as ESLint, it's because you care about rules for your code and you want to make sure that these rules are followed. What's important is that you are strict about these rules: They are mandatory, not just nice-to-have. Otherwise you wouldn't use a tool to verify this.

Now, if rule configuration errors don't pop up - what does that mean? Basically it gives you a wrong feeling of security: You think that everything is alright, but in fact it isn't. It's like an empty catch clause.

So, let's compare the reasons why a rule may be configured wrongly:

• It may just be a typo. If so, I would actually expect ESLint to tell me, so that I can fix it.
• It may be because of a breaking change, and the option set is not available any more. Again, I would like to know about this.
• It may be because of … uhm, yeah, why?

To be true, there aren't any other cases that come to my mind, and I bet that even if there are, those two reasons are the most widely spread ones for wrongly configured rules.

So what's the benefit in ignoring errors here? IMHO there is none, and hence a wrong configuration in fact is a fatal error that requires me to pay attention. If I don't, this leads the usage of ESLint ad absurdum.

Just my 2 cents…

[puzrin]

I think, incorrect rule options is a fatal error. As simple solution - it's possible to throw exception from rule code. A bit dirty, but will work and does not require architecture changes.

For more robust solution it's possible to export validate property, that can be function or json schema (depending on complexity).

[jonvuri]

I think it's a lot easier allow rules to throw exceptions or to pass them an error callback than it is to try to externally validate them.

[nzakas]

@jrajav not necessarily. Something like a JSON schema could potentially make sense.

As it is, I'd like to postpone digging too deep until 0.9.0 timeframe.

[ilyavolodin]

I was thinking about this issue, and I see only 2 solutions (given that I think it's extremely important to notify the user that configuration for the rule is incorrect):

• rules themselves should export a list of options with expected values for each
• have central json file that contains schema for all rules

First option probably will have a very significant performance hit and require some significant memoization, and second will be harder to maintain. In first case, eslint.js should probably call validate function before calling into the rule. In the second - config.js should validate current configuration against schema.
This should also be available for plugins as well (if you go with option 2 then it should be another property that is exported by plugin).

[nzakas]

It also depends on how we want to use that information. For instance, we could require that rules validate their own options and throw errors for invalid options - that would mean the possible options wouldn't need to be exported. If we ask them to export some sort of schema, that means validation happens outside the rule, and that schema can also be used elsewhere.

The question is really: whose responsibility is it to validate options?

[ilyavolodin]

Having rules validate options will significantly complicate rules implementation. Rules are currently responsible only for one thing, validating AST and reporting errors. Ideally, I think we would want something like optionator but for configuration.

[puzrin]

IMHO it's rule's responsibility to define validator, because such functionality should be isolated and located in the same place. Something like this:

module.exports = function rule_implementation(...) { ... };

// Optional
module.exports.validadator = /* function OR json schema */

That's convenient for developpers, backward compatible & testable.

[ilyavolodin]

What I mean is that I think ESLint itself should be running validation, does validation comes from rules or from global schema is a different question.

[nzakas]

I think it has to come from rules so that plugins can also have their options validated.

[btmills]

We talked about exporting configuration schemas from rules in order to enable fuzzing in #1568. There are some JSON schema examples for a few rules in that thread. I'd agree with @nzakas that rules should export their own configuration schema of some sort (JSON or otherwise).

[ilyavolodin]

I think it's perfectly reasonable for the rule to expose JSON with validation schema, I don't think each rule should have the code to validate options passed into it. I think that code should be centralized.
I'm also concerned about performance hit if each rule is going to supply it's own validation schema. You would need some complicated caching to not verify options multiple times per rule.

[nzakas]

Yeah, the overhead of validating is a concern. We would have to revalidate in every directory site to configuration cascading.

[puzrin]

Yeah, the overhead of validating is a concern. We would have to revalidate in every directory site to configuration cascading.

That's not expensive, because you need to validate each new params combination only once, total count is not big, and validation fn is cheap.

[nzakas]

It depends on a lot of factors. Maybe not expensive for a single file, but we have users with projects that have hundreds and thousands of files and folders. Adding an extra few milliseconds here and there adds up quickly in that scenario.

[puzrin]

Even if you have zillions of configs, number of different params combinations for each rule is very limited. When you memoise those, validation become just a lookup. That's not milliseconds, that's microseconds.

[nzakas]

You're arguing theoretical performance of nonexistent code. I'm saying we need to be careful doing this as it would be irresponsible to add in validation without considering the performance impact. I won't believe that it's negligible until I see a solution working.

If you think you know of a fast way to do this, we accept pull requests. :)

[DylanPiercey]

Why do validations have to be a performamce issue?

The way I see it there should be an option on the CLI to validate the rules.

eslint --verify-rules

That way the rules can handle there own validation but only if the environment requests it.

[nzakas]

Config validation can't be done purely as a separate step because most people will never do that.

[DylanPiercey]

@nzakas
I agree, but it's much better than omitting config validation because of performance.

[nzakas]

We aren't omitting validation due to performance. This issue is open, which means it's waiting to be implemented. My only point was that it must be done in such a way that it doesn't measurably negatively affect performance when rules are properly configured.

[nzakas]

Validation was released in 0.23.0