Add setting "eslint.globalSeverity", Implements #468

[AshGrowem]

Add an eslint.globalSeverity setting. Addresses my feature request #468.

Just FYI, I added es7 to compilerOptions.lib in server/tsconfig.json to support the .includes() method I utilized on line 229 of eslintServer.ts

[msftclas]

All CLA requirements met.

[dbaeumer]

@ashrafhadden thanks for the PR. Looking through it I am wondering if the ESLint extension is the right place to do this. Wouldn't it make a lot more sense to have this in eslint itself? The problem will be that as soon as one of the ESLint tasks are run which spawn a separate eslint command line all these will be reported as errors again.

So far I was very relucent to add features to the ESLint extension that deviates the behavior from what you are seeing when running eslint in a terminal.

[AshGrowem]

@dbaeumer All good points! I figured I'd at least submit the PR since I had already hacked it on my local ESLint extension. Also this was one of my first major PR's so I did it for the experience as well. Thanks for looking over it at least! This was less intimidating for me to approach than the ESLint codebase, I think it's a lot bigger, but I'll definitely look into it! Thanks again for reviewing it! 🙏

[dbaeumer]

@ashrafhadden any objections to close the PR give my comments #561 (comment)

[AshGrowem]

@dbaeumer Nope, go ahead close! Thx!

[JoshuaKGoldberg]

@dbaeumer I don't understand your objection to this PR going in as is. Could you please elaborate?

Per #468, while it's common for settings files to set the default warning as "error" so that the CLI runs as errors, GUI users often prefer the errors to be visualized as warnings.

The TSLint extensions for VS Code support the feature with a tslint.alwaysShowRuleFailuresAsWarnings setting.

[justinbayctxs]

I'd really like to see this feature. I'm trying to move from TSLint given its deprecation, and I'm missing the alwaysShowRuleFailuresAsWarnings option. I liked having my TypeScript errors indicated with red squiggles and my lint errors indicated with green/yellow. It's all about the visual indicator. The rule could still even call out it's an error on hover, if desired.

I found some rationale relating to this feature being added to TSLint: https://github.com/microsoft/vscode-tslint/issues/199#issuecomment-299246484

[dbaeumer]

My objection is that you can also have a task that runs eslint over all files and in this case everything would be reported as an error again. Does anyone know how this got solved in TSLint?

[JoshuaKGoldberg]

@dbaeumer that concern is unrelated, no? This extension just shows a GUI on top of ESLint complaints. If a user runs a task that reports lint complaints, it's up to that task how it would want to report errors.

how this got solved in TSLint

https://marketplace.visualstudio.com/items?itemName=ms-vscode.vscode-typescript-tslint-plugin

A config flag in ms-vscode.vscode-typescript-tslint-plugin: tslint.alwaysShowRuleFailuresAsWarnings. "Always show rule failures as warnings, ignoring the severity configuration in the tslint.json configuration."

...so roughly what this rule option does, just TSLint is slightly less configurable.

[mrcljx]

@dbaeumer Maybe the naming of the setting is confusing?

The root issue as far as I understand (and also have) is that ESLint errors become indistinguishable from hard JS/TS syntax errors while working on a file - it's not about overriding the ESLint severity, but rather to tweak the mapping of an ESLint error levels to a visual VS Code diagnostic severity.

Different name and default values could be:

eslint.diagnosticSeverities.warn: "Warning"
eslint.diagnosticSeverities.error: "Error"

I personally would tweak that on my machine to be this:

eslint.diagnosticSeverities.warn: "Warning"
eslint.diagnosticSeverities.error: "Warning"

[mrcljx]

The TSLint setting name also hints at this being purely visual:

alwaysShowRuleFailuresAsWarnings (vs. for example: alwaysTreatRuleFailuresAsWarnings)

[AshGrowem]

I like @sirlantis's idea. Good combo of clear and flexible since it uses the name of the actual VSCode API responsible for visual Problems. I'm down to rewrite the PR if everyone agrees on an implementation. I will include a link to the DiagnosticSeverity documentation in the hover tooltip. Thoughts? @dbaeumer @JoshuaKGoldberg @justinbayctxs

    "eslint.diagnosticSeverity": {
        "off": "hint",
        "warn": "information",
	"error": "warning"
    },
    // exceptions to the above setting
    "eslint.diagnosticSeverity.exclude": ["semi", "no-console", "no-debugger"],

Possible Values: "hint", "information", "warning", "error"

[dbaeumer]

I try to explain myself again since I think I was not clear: VS Code has the ability to run tasks, scan the output and turn them into problems. If you have such a task there is no way to intercept the severity and to map it to different values. So in such cases problems might swap between error and warning (in the editor and the problems view) depending on whether the problem came from the task or from the ESLint plugin itself. This is why I stayed away from this so far.

[AshGrowem]

@dbaeumer Unless I'm misunderstanding, I don't see how this as an issue since the setting is only meant to target ESLint's problems, not VSCode's or any other source, thus "eslint.diagnosticSeverity". I will be writing a separate PR for this.

[dbaeumer]

@ashrafhadden the task contributed by the ESLint extension to lint all files in a workspave also creates problems owned by eslint. So there is a case where the task generates error problems which the ESlint reconciler generates as warning.

[AshGrowem]

@dbaeumer Ah okay, I understand now I just never used the ESLint task. Is there no way to modify/intercept the task problems from within this repo (vscode-eslint)?

[dbaeumer]

The only way to intercept them is by generating them in the plugin itself using the new CustomTask API in VS Code.

[Merott]

This is where the TSLint Language Service Plugin makes use of the configuration option alwaysShowRuleFailuresAsWarnings: https://github.com/microsoft/typescript-tslint-plugin/blob/1c0f74a52dbc38823d14c3a5c1b819a33015a2a9/src/plugin.ts#L386-L388

I don't have full understanding of how VSCode plugins work, but is there something different about ESLint that we can't follow the same pattern here?

[dbaeumer]

This is not about technical difficulties, this is about having the same feature set in the extension and the terminal. This is why I would like to have a solution that uses ESLint mechanisms since then the same output can be generated in the terminal and hence when used as a task insider VS Code.

Does anyone know if a ESLint plugin exists that changes the severity of all problems?

[Merott]

What I'm looking for (and I think anyone else who's used the alwaysShowRuleFailuresAsWarnings setting for tslint) is not to mess with the terminal output, but only to stop seeing red squiggly lines within the code editor for code style issues. That way, we can distinguish them from actual TypeScript errors that will fail to compile our code.

I agree that what this PR introduced was beyond the scope of this plugin. I think it tried to introduce a level of control that ESLint itself doesn't provide.

But, I think showing rule failures as warnings within the editor is a sensible option that could belong to the plugin itself. Any extra configuration options could be argued as meddling with ESLint's business.

I'm sorry if I'm still missing the point of your objection @dbaeumer.

[lolmaus]

I'm with @Merott here. My stand on this matter seems to be aligned with theirs:

My goal is to be able to visually distinguish between:

• errors that (potentially) break code execution
• violations of the code style guide that do not affect code execution in any way (and are likely to be fixed automatically with eslint --fix, e. g. on saving or in a pre-commit hook!)

My (incompetent) guess is that the easiest way to achieve this would be to have an option in the VS Code's ESLint extension that forces it to treat any errors reported by ESLint as warnings. A single checkbox that causes a simple override.

I believe, this feature request has nothing to do with the terminal output. I fail to imagine a use case where I would need to reduce ESLint errors to warnings in the terminal.

[dbaeumer]

VS Code has support to parse the output of tasks run in the terminal and generate errors from it. If you run such a task in which the terminal shows an ESLint problem as an error the editor will render the problem as an error and not as a warning (since the error is not going through the eslint extension code). Hence we are back to were we are right now.

This is why I think the best would be to have two .eslintrc config files were one depends on the other and the second one simple configures everything as a warning. Then you could even use that one when running a task in the terminal.

I for example usually do that with my TS compiler errors were I have relaxed rules when developing but tighter rules when compiling for production. But I didn't ask to have a special setting for that in the tool. I simply maintain two tsconfig.json files.

[Merott]

@dbaeumer I just checked, and the TSLint extension doesn't do anything about the output of the tasks. That doesn't bother me more than seeing lint errors mixed up with TS compile errors (while not using tasks), but I see your point, and not sure I can argue against it.

Since we don't have a better solution yet, I've decided to just hack the extension for my own use. If anyone's interested, here it is: https://github.com/Merott/vscode-eslint#always-show-rule-failures-as-warnings

[lolmaus]

@Merott Thank you! 🙏 🙇‍♂️

If you add a checkbox to extension settings, you could file your own PR...

[Merott]

@lolmaus you're welcome.

If you add a checkbox to extension settings, you could file your own PR...

@dbaeumer doesn't agree with this option being part of the extension, and a PR won't do any good until we have a solution we can all agree with.

Until then, I guess I'll just run my sed command every time the extension updates! 😅

[lolmaus]

So:

1. There's a problem that many VS Code users face.
2. There's a simple solution that efficiently fixes the problem.
3. It is rejected because this solution is not solving another problem (with the terminal) that nobody is actually facing.

Is this how it goes?

---

This is why I think the best would be to have two .eslintrc config files were one depends on the other and the second one simple configures everything as a warning. Then you could even use that one when running a task in the terminal.

Creating a separate eslint config in every project I'm opening, then configuring each project's workspace to use that config — is a ridiculous amount of effort.

It gets even worse when VCS is concerned. That config file has nothing to do with the codebase, so it has to be gitignored, which means that I need to commit a gitignore entry to every project I'm opening, which in turn means I have to rationalize the necessity of it to every project's maintainer.

And if I don't gitignore it, I'm prone to accidentally commiting the config itself.

You can't reasonably expect people to be doing all that.

All we want is for ESLint squiggles to be yellow! Can we please have a checkbox that does it?

[dbaeumer]

No this is not the case. The problem is that adding this will result in more requests having a more fine grained configuration. I used the terminal as an example to explain this. Take TypeScript ESlint as an example: there are quite some rules in TS ESLint for which even if I type I want to see them as an error and others (for example auto fixable rules) I want to see as a warning. So the next feature request will be that I have to support this as well (e.g. make everything a warning except these five rules, ....). This is what I want to avoid. So IMO the best solution would be if this can be implemented using normal eslint configuration / eslint plugin. Then the full power of ESLint configuration rules can be used to allow customization.

[lolmaus]

@dbaeumer I see your point, but I explained why manipulating ESLint configs is troublesome.

This could be fine if you're working on a single project, and either you're the owner, or the owner also happens to use VS Code (or at least is sympathetic to minor issues like this) and would accept a PR irrelevant to the project.

But there are lots of people who work on lots of projects every day. E. g. I've checked out a new codebase from GitHub just an hour ago. And I want ESLint squiggles to be yellow in it too, without me spending 20 minutes on wiring it together.

I want a global setting.

So the next feature request will be that I have to support this as well (e.g. make everything a warning except these five rules, ....)

Please don't take it as sarcasm, but that's a very reasonable thing to do! I honestly believe that it would be awesome to have exceptions from this global rule.

[jamesorlakin]

I agree with the majority here. While ESLint rules can be categorised into 'warning' and 'error', the vast majority of configurations I've seen use just 'error' almost exclusively. This also isn't something I necessarily have control over - whether it be through off-the-shelf configs such as Standard, or an existing mature project. Like others I also don't want to worry about fighting with my tooling when switching projects.

VS Code has support to parse the output of tasks run in the terminal and generate errors from it.

Aah good spot, I didn't know that was even a feature! I can't say I really see the point of it though. Why wouldn't I have an interactive report for open files contributed by an extension in such a case, especially a tool like a linter?

So the next feature request will be that I have to support this as well (e.g. make everything a warning except these five rules, ....)

Added maintenance burden isn't great no, but don't a lot of VS Code's features respect the spirit of customisation? I see few extra useful customisation options this extension could have above that. The effective rules are defined by the project configuration, how I want to see the linter within my IDE should be up to me, within reason... Of course a line will have to be drawn somewhere subject to code complexity and technical restraints.

---

@dbaeumer I kindly ask you to reconsider please. I propose we add a checkbox to override ESLint diagnostics to be seen as warnings as found in TSlint and #978 (which is based on standard/vscode-standard#108):

1. Based on the comments on this pull and linked issue, there is definitely community interest in having such an option.
2. We can document a known limitation or caveat around parsed errors from task output. I think the number of people utilising that functionality and being caught out by this proposed option would be very minimal. I imagine a quick sentence in the setting description would suffice.
3. As TSLint's downfall is coming, many users will eventually be migrating to this extension over time. We should try and match the featureset available in TSLint - which as far as I can tell is just this option really. With regard to future asks we could assess that another time? I believe the focus and justification should be on feature parity for an incoming wide userbase. XKCD 1172 springs to mind!
4. Comparing other extensions, the most popular one for JetBrains' IDEs also has such an option. The SublimeLinter has a much more complex configuration (which I guess is ultimately what you're trying to avoid, and I agree!) but may be able to achieve a similar result. Others I looked at provided relatively few options but I admit not looking in great detail.

I guess the ultimate solution would be for ESLint to have a native CLI option for this (and we tweak args in the extension) so it can work equally in all IDEs, or VS Code to have customisable Diagnostic types and colours. Both of these seem really far fetched; the user request here could be summed up to "make all linting yellow, red mean code broke". Can we not cave to that, even if it feels a bit irrational? I haven't seen anybody other than yourself opposing in this thread. (and I don't mean to highlight that in a disrespectful way)

Thank you. 🙂

[dbaeumer]

@jamesorlakin thanks for your thoughtful and considerate reply. It is not that I don't understand your desire to have that feature. But as you pointed out as well it is about maintaining a feature that has several problems and if added will cause more work in the long term:

• no consistent implementation inside VS Code (editor, terminal).
• does conceptually belong into the ESLint CLI / API. Have any of you every filed a issue against ESLint asking for this?
• only a small minority of vscode eslint users have ask for this so far. The feature request has 15 votes. I am not trying to say that all the other users of the extension are against it but it doesn't seem important for them. They happily work without having this feature.

And I personally don't understand the use case. Turning all ESLint errors into warnings is like turning ESLint off (you ignore all of them). I personally have the extension globally turned off and selectively turn it on in projects I actively work on and care about the warnings and errors. Can someone please explain why this is not a possible solution?

What I am willing to do is the following: merge a PR that adds one setting to globally overwrite the severity. I would also like to see:

• that someone that cares about this opens an issue against ESLint asking for the feature there as well.
• signs up maintaining the feature. If there are requests for a more fine grained configuration or bugs I will assign the issues to that person :-)

[lolmaus]

And I personally don't understand the use case. Turning all ESLint errors into warnings is like turning ESLint off (you ignore all of them).

There are two main factors:

1. The use case is to be able to visually distinguish errors that cause code to fail from formatting nit-picking, most of which is likely to be auto-fixed with eslint --fix.

   During development, I don't care about linting errors. I only need to fix them before committing.

   But at the same time, I still want to see them in the editor — in case I'm in the mood of fixing them right away, rather then revisiting the file later.

2. I want this to be consistent across all projects, as a default, without a need to set up a separate ESLint config file in every file.

[Merott]

only a small minority of vscode eslint users have ask for this so far.

This is only a problem for TypeScript users, because they're the ones who need to be able to distinguish typing errors from code style warnings. This is not like turning ESLint off. You don't ignore all of them, you just make them show up as warnings so you know that your code is still valid TypeScript, and will compile once you save the file. You can continue to test out your changes without worrying about those warnings, until the point you decide to commit and push your changes.

There are a lot of TSLint users who haven't yet migrated to ESLint. I have TypeScript projects right now that are still using TSLint, and I know they'd be a nightmare if we had to switch them to ESLint right now, because of this issue.

I personally have the extension globally turned off and selectively turn it on in projects I actively work on and care about the warnings and errors. Can someone please explain why this is not a possible solution?

I'm not sure why this needs explanation. If a project has ESLint configured, then turning off the ESLint extension doesn't make them go away. We still have to deal with them, and it's a lot easier to deal with them with the extension turned on. Why would you turn off the extension in a project that has ESLint configured?

[jamesorlakin]

Thanks @dbaeumer. I had a look at the ESLint repo and there have been a couple of relevant issues about this:

eslint/eslint#10346 - A similar request as here by the same author. VS Code warnings and errors have a different meaning to ESLint ones. It was decided that this isn't ESLint's responsibility as it's just reporting factually based on the configured ruleset. The takeaway was it's up to the IDE extension to interpret this and display it to the user as they wish.

I understand and agree with your points about the ESLint extension essentially lying to the user about results at that point if we were to implement this but, rather irrationally, I'm just a fan of yellow! When looking at the JetBrains plugins it seems the JetBrains IDEs have very granular options for severity. It's the limitation of VS Code's diagnostic display that sends us down the easier 'always make it a warning route' in order to differentiate language server problems.

One possible future proposal could be for VS Code to have configurable severity/colours for diagnostcs based on the contributing extension. However, this would really be out of my comfort zone! The recent rollout of semantic hightlighting at least indicates the colouring of items does matter in themes - maybe they would open to extension of diagnostic colours too?

Something like:

"workbench.colorCustomizations": {
    "[Monokai]": {
        "extensionOverrides": {
          "dbaeumer.vscode-eslint": {
            "diagnostic.error": "#FFC0CB",
            "diagnostic.warning": "#FFFF00"
          }
        },
        "sideBar.background": "#347890"
    }
}

I don't see this being an easy feat but this ideally would ensure the ESLint output matches what's seen in the problems panel in VS Code whilst still visually providing a difference for us yellow-fanatics.

[lolmaus]

this isn't ESLint's responsibility as it's just reporting factually based on the configured ruleset.

it's up to the IDE extension to interpret this and display it to the user as they wish.

👆 Exactly!

---

agree with your points about the ESLint extension essentially lying to the user about results at that point if we were to implement this

I don't agree. As you have said above, ESLint is simply reporting the facts.

I think, the matter will become easier to reason about if we make the following distinction:

There are two separate cases for ESLint:

— CI is responsible for not letting any formatting errors to be merged into main branches. Any linting issue is an error for CI.
— During development, syntax and typing errors are much more important for the developer because they require immediate attention, whereas formatting/styling issues can be dealt with later.

ESLint does not and should not be aware of visual preferences of the user. That's the concern of the IDE.

---

I'm now using @Merott's fork, and it makes development so much more ergonomic. E. g. without the proposed feature, I can't tell if this squiggle means my code is broken or simply unformatted:

With the proposed feature enabled, it's clear there's a logical error. When I fix the error, it looks like this:

It's now immediately visible that this error is minor, I can ignore it and switch to the browser and see the result, the app will not crash. The pre-commit hook will remove the lint automatically.

And I didn't need to create the second ESLint config file, negotiate with my teamlead for it to be committed, or apply --assume-unchanged to it and then risk it being committed by accident.

[Merott]

For anyone using a Mac and looking for a temporary patch, here it is:

cd ~/.vscode/extensions/dbaeumer.vscode-eslint-{VERSION}/server/out

# replace: switch(e){case 1:return r.DiagnosticSeverity.Warning;
# with: return r.DiagnosticSeverity.Warning;switch(e){case 1:return r.DiagnosticSeverity.Warning;
sed -i -e 's/switch(e){case 1:return r.DiagnosticSeverity.Warning;/return r.DiagnosticSeverity.Warning;switch(e){case 1:return r.DiagnosticSeverity.Warning;/g' ./eslintServer.js

If you're not on a Mac, I'm sure you can figure out how to adjust the above for your own system or apply the change by hand.

[dbaeumer]

Just for the record: I am only coding TypeScript and lint exclusively with this extension.

If I summaries the discussion: it is not about turning ESLint off for a project you open to read some code and you are not interested in any off the linting errors.

You basically want to define two different configurations. One for CLI and one for the IDE.

I can understand this since I do the same for the TS compiler. But I decided that I use two configuration sets for this to make it more clear and not IDE dependent (see tsconfig.json & tsconfig-watch.json in this repository: https://github.com/microsoft/vscode-languageserver-node.git).

Given my experience with this I highly doubt that having one switch that makes everything yellow will satisfy the needs especially in TypeScript where ESLint can validate based on symbol information as well. I think devs will immediately ask for a more fine grained configuration. I for example use the rule of no dangling Promises and this one is something devs might want to always see as an error and not as a warning even in the IDE.

I also understand that maintaining two files (like I do for TypeScript compiler) is a pain and I already thought about writing me a tool that generates the one from the other.

But I don't want to make this a VS Code settings problem either since the extension will end up supporting all ESLint config features in VS Code settings.

So here is another idea: since the extension uses the Node API (does not call the CLI) we could do the following:

• first call calculateConfigForFile
• then read a second (dev config file) that redefines some rules. We can use the same calculateConfigForFile method.
• merge the two files
• call the linter with the configuration.

So I slightly rephrase my offer from the last post:

What I am willing to do is the following: merge a PR that adds one setting to globally overwrite the severity. I would also like to see someone signing up maintaining the feature. Specially about requests for a more fine grained configuration. Anyone out there?

[lolmaus]

I also understand that maintaining two files (like I do for TypeScript compiler) is a pain and I already thought about writing me a tool that generates the one from the other.

That's like offering a painkiller instead of removing the source of pain. :(

---

devs will immediately ask for a more fine grained configuration

But I don't want to make this a VS Code settings problem either since the extension will end up supporting all ESLint config features in VS Code settings.

How bad can it really get?

I can imagine the following option:

Treat ESLint errors as warnings

🔘 Never
🔘 Only for the following properties:
⚫ For all properties except:

typescript-eslint/no-floating-promises

That would be extremely convenient.

A quick, efficient, frustration-free configuration without having to figure out how to generate the second config file.

As an IDE user working with many codebases, I don't want to even bother making a decision of where to store those generated configs. I just want ESLint squiggles to be yellow, why does it have to be that hard for end users? 😕

What I am willing to do is the following: merge a PR that adds one setting to globally overwrite the severity. I would also like to see someone signing up maintaining the feature. Specially about requests for a more fine grained configuration. Anyone out there?

Looks like @ashrafhadden's PR (this PR) already covers most of what I imagined above.

@dbaeumer Do you agree with this approach (apart from dependencies being checked in by accident and missing tests)?

[dbaeumer]

I will try to restate what I would like to see happening since I think I didn't express myself correctly

• a PR that I can merge that only adds an option to control the severity off all generated ESLint problems.
• a name or a group of names that is willing to work on another PR that adds a more configurable solution (regardless how it will look like) and that is willing to maintain it.

[dbaeumer]

Here is a starting point with no conflicts: https://github.com/microsoft/vscode-eslint/tree/dbaeumer/468

Setting name is eslint.rules.customizations (inspired by @jamesorlakin) which currently only allows a value:

{
    "*": "info" | "warn" | "error"
}

Setting already travels to the server (https://github.com/microsoft/vscode-eslint/blob/dbaeumer/468/server/src/eslintServer.ts#L209). Anyone willing to continue on this?

[jamesorlakin]

I've opened a PR based on what I think the agreed solution is. It seems fairly simple in retrospect, I'm just trying to think of what a more configurable solution would look like or achieve?

[dbaeumer]

@jamesorlakin thanks. Will have a look the next couple of days.

[dbaeumer]

#1164 got merged so I will close this one