Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add severity of errors #345

Closed
lachvoj opened this issue Mar 12, 2015 · 17 comments
Closed

Add severity of errors #345

lachvoj opened this issue Mar 12, 2015 · 17 comments

Comments

@lachvoj
Copy link

lachvoj commented Mar 12, 2015

Please add severity of errors into rules to distinguish if it's functionality issue or just code readability issue.

@Lexicality
Copy link

I support this - I would very much like to be able to fail the build due to class-name violations, but not comment-format.
It'd be great if you could do it in the same way as ESLint where you can specify off, warning or error when activating a rule in the config.

@readme42
Copy link

👍 this would be great!

@jkillian
Copy link
Contributor

👍 I like this idea as well

@pgatilov
Copy link

pgatilov commented Nov 5, 2015

👍 please!

@jkillian
Copy link
Contributor

jkillian commented Nov 5, 2015

Related to #629 perhaps. Rules could have a default severity that could be overriden in tslint.json

@JoshuaKGoldberg
Copy link
Contributor

+1, this would be great

Would it be feasible to add a "warnings"option to tslint.jsons?

Edit: alternately, it would be cool to expand the tslint.json format to allow for more complex settings:

{
    "defaults": {
        "severity": "error"
    },
    "rules": {
        "radix": true,
        "semicolon": {
            "rule": "always",
            "severity": "warning"
        }
    }
}

...in the future, if some sort of options like auto-fixing were to be added, they could go in there too?

@jkillian
Copy link
Contributor

jkillian commented Jun 13, 2016

I'm thinking about implementing this soon, I like the idea of each rule taking an object as configuration. (This is also very nice code-wise, since rules only accept a boolean or array right now, we can implement this feature with no backwards-compat breakage.)

I like a format similar to what's specified above:

"rules": {
  "align": {
    "mode": "off",
    "options": ["parameters", "arguments"]
  }
}

"mode": could be either "off", "warn", "error", or "on". If "on", the default level of warning would be used.

Open questions:

  • Should we stick with the ESLint way of things and also allow 0, 1, 2, and 3 instead of "off", "warn", "error", "on", etc?
  • Do we want an "on" option at all?
  • I like the name "mode" because it's easy to type, but should we pick a different name?

I also like how this proposal makes it easy to add in additional options for things like auto-fixing, as mentioned above.

This would probable be implemented in two steps:

  1. Adding in the code to handle object configurations for rules.
  2. Writing the code to make different modes act differently (exit status, etc)

@Lexicality
Copy link

I'm fairly sure the number severities in ESLint are backwards compatibility only. Unless you use the same rule format (eg an array rather than an object) there's not much point I would say.
Perhaps "level" instead of mode?

@jkillian
Copy link
Contributor

Realized #629 is a better issue to discuss rule configuration schema issues, so moved some comments over there!

@adidahiya
Copy link
Contributor

shipped in 5.0

@ErikCupal
Copy link

@adidahiya Sorry if I missed something, but how to turn all errors back to warnings in version 5.0? I tried this so far..

{
    "defaults": {
        "severity": "warning"
    },
    ...
}

@adidahiya
Copy link
Contributor

@ErikCupal "defaultSeverity": "warning". see https://palantir.github.io/tslint/usage/configuration/

@nchen63
Copy link
Contributor

nchen63 commented Apr 17, 2017

defaultSeverity only applies to rules in the current file and doesn't affect rules in the recommended config. Right now we don't have a way to reset the inherited severity levels besides overriding the severity one by one.

@adidahiya
Copy link
Contributor

@nchen63 ah, I didn't realize that. I think it would be more intuitive for defaultSeverity to be a global setting that applies to all rules that have been loaded -- it should override any base configurations

@emilio-martinez
Copy link

@adidahiya I agree. I've faced this issue myself where a bunch of rules are throwing an error instead of a warning for "defaultSeverity": "warning", i.e., I thought that was meant to be global. Is there a place where that specific issue is being tracked? (since this is closed, I mean)

@adidahiya
Copy link
Contributor

@emilio-martinez yes, #2569 (linked right above your comment)

@emilio-martinez
Copy link

@adidahiya cool. Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants