Classify lintish diagnostics as warnings, add treatWarningsAsErrors

[weswigham]

Fixes #13408

We already had the infrastructure to do this in pretty much the entire stack (heck, pretty output even has configured yellow text for warnings, even though in no circumstances would we have seen them before). All that was missing was the ability to toggle warnings as errors and the initial listing of the diagnostics as warnings.

@mhegazy As a feature, I assume this is too late for 2.6?

[weswigham]

This isn't strictly related to the test, but when I went to remove it, I considered that it is useful in verifying that we're not reporting all errors as warnings.

[Jessidhia]

Nice that it defaults to the safer setting (true); but how do you disable it via command-line arguments? --no-treatWarningsAsErrors?

[weswigham]

--treatWarningsAsErrors false, same as our other boolean flags.

[weswigham]

I think I've gotten all the places where the warning v. error distinction matters changed. Also:

Note the green underline instead of red. Already works in vscode, too. We really did plumb this through almost everywhere already.

[RyanCavanaugh]

What's the rule for whether something is a warning or not?

[weswigham]

Based on the disccussion in #18894, a new flag --treatWraningsAsErrorrs will be added, defaults to true. The servirty of the style checks done by the compiler will all be set to Warrning. here is the list of configurations to control these messages :
--noUnusedLocals
--noUnusedParameters
--noImplicitReturns
--noFallthroughCasesInSwitch
--allowUnusedLabels
--allowUnreachableCode

For future errors... Based on this pattern, non-semantic errors which aren't runtime errors but are code smells.

[alexeagle]

I didn't check, but does the tsc output order the errors last? That is very important when a build has a wall of warnings, so I don't have to scroll around my terminal buffer or CI log.

Also, I think it's desirable that the treatWarningsAsErrors applies to the exit code and on the command line, but not in the editor.

[mhegazy]

Also, I think it's desirable that the treatWarningsAsErrors applies to the exit code and on the command line, but not in the editor.

I am not sure i understand this statement. can you elaborate?

[weswigham]

Also, I think it's desirable that the treatWarningsAsErrors applies to the exit code and on the command line, but not in the editor.

Wait, do you mean always still red underlines in the editor, but still allow the exit code to be 0? Or the opposite?

[weswigham]

I didn't check, but does the tsc output order the errors last? That is very important when a build has a wall of warnings, so I don't have to scroll around my terminal buffer or CI log.

No, right now warnings are mixed with errors, sorted by file position. Is it always desirable to split them up?

Actually rustc only outputs warnings when there are no errors - is that more desirable? (Then again, #deny in rust also only changes the exit code - warnings are still only reported once all errors are resolved... And in rust a program with errors is never compiled, with TS, even with errors you get output...)

[timc13]

any movement here?

[alexeagle]

@timc13 see the design notes linked above:

Conclusion: hold off on dealing with warnings, understand how to improve TSLint, think about how to champion community tooling better by default for projects.

[RyanCavanaugh]

@weswigham can we close for now?

[weswigham]

We're not actively considering it right now, and I imagine the next time we decide to talk about this we'll waffle just as much, so sure. 😛

The real question is will you close the original issue: #13408