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

Warn on one Parameter, Error on a different Parameter #6776

Closed
merlinstardust opened this issue Jul 27, 2016 · 4 comments
Closed

Warn on one Parameter, Error on a different Parameter #6776

merlinstardust opened this issue Jul 27, 2016 · 4 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion

Comments

@merlinstardust
Copy link

merlinstardust commented Jul 27, 2016

What version of ESLint are you using?
2.9.0
What problem are you trying to solve?
The initial issue that made me think of this was with the [https://github.com/eslint/eslint/blob/master/docs/rules/max-lines.md](max lines rule). What I'd like to do is have ESLint warn the developer if a file is over 50 lines and error if it is over 100.

This would allow us to have our own internal rule that no file should be over 50 lines but as with any rule there are exceptions, so we want to allow a true max limit of 100.

I'm aware that I can disable ESLint for a whole file but I don't want to do that and it goes the against the point having the two distinct rules in the first place

This same warning, then erroring could be applied to many rules: max-statements, max-depth, etc

My take on a solution:
I see a few possibilities for how this could go in a config file.

  1. Have warnLimit and errorLimit as keys for each applicable rule
  2. Allow each rule array to have third and fourth valus, so you could have "max-lines": ["warning", warnParams, "error", errorParams]
  3. Have warnRules and errorRules as top level config keys and allow for the same rule in each with different parameters
    • This would also have the benefit of not having to write error/2 or warning/1 for each role, but that's a different story

Apologies if this has already been requested. I did try searching for it but this isn't an easy one to search for.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jul 27, 2016
@alberto alberto added enhancement This change enhances an existing feature of ESLint core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jul 27, 2016
@alberto
Copy link
Member

alberto commented Jul 27, 2016

I think this has been requested in the past, yes, so I'd say it's not very likely to happen. For a start this should be done in a backwards compatible way, so if you want this to have any chance of being done, you should frame it taking that into account.

One workaround is having two different configs, one for warnings and another for errors and to run both.

@ilyavolodin
Copy link
Member

Another workaround is to clone this rule and re-name it, run both, one as a warning, and one as an error. ESLint doesn't support warning/errors from inside the rules, it's done in the core, base on the first parameter of the configuration value, so this is not possible to do in a way you are requesting.

Since there are two workaround, closing, as this is not something we could implement.

@merlinstardust
Copy link
Author

@alberto I believe my third option of warnRules and errorRules as top level configs should be backwards compatible, shouldn't it? You could have those or have the regular rules param.

@platinumazure
Copy link
Member

platinumazure commented Jul 28, 2016

@merlinpatt One problem with your suggested approach is that, with the current architecture, both rules would be completely independent from each other and a file with a violation of both the warning and error settings would actually generate both a warning and an error (e.g., if max-len warning is 50 and error setting is 100, a line of 101 characters triggers a warning AND an error). That's not a good user experience; but also, attempting to detect "duplicate" reports would be technically somewhat complex, and we're not sure that the gain for users is worth the complexity.

My suggestion (not representing the team here) for anyone who really wants to give this a shot is to start a new NodeJS project which consumes ESLint's CLIEngine, and create a tool which supports warning and error configurations, and then do one or more lint runs and resolve any duplicate reports and other issues that may crop up. That will give you an idea of what sort of changes we would need to make it core to support the use case there. Even if it becomes clear that we still can't support this in core, creating this tool means you have what you need. 😄

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion
Projects
None yet
Development

No branches or pull requests

5 participants