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

Support // @ts-nocheck for lint like checks like --noUnusedLocals #15953

Closed
egamma opened this issue May 19, 2017 · 8 comments
Closed

Support // @ts-nocheck for lint like checks like --noUnusedLocals #15953

egamma opened this issue May 19, 2017 · 8 comments
Labels
Suggestion An idea for TypeScript

Comments

@egamma
Copy link
Member

egamma commented May 19, 2017

TypeScript has compiler options for several lint style checks like noUnusedLocals. For such checks it can be desirable to turn them of for a specifc location in the code. Now that there is support for // @ts-nocheck comments, this comment should also be support for the lint style options TS supports.

@mhegazy
Copy link
Contributor

mhegazy commented May 19, 2017

From the compiler perspective there is no difference between the errors that the --noUnusedLocals enable and any other errors.

We have talked about disabling errors multiple times and never reached a conclusion to add such support. we believe that all TS errors are warnings by definition (do not block emit), and that disabling some would leave your program in an inconsistent state and would negatively impact the whole experience. Moreover we believe that there are already in place language constructs that enable fixing the errors instead of silencing them.

That said, I know that features like --noUnusedLocals, --noUnusedParameters, --noImplicitFallThrough, and noImplicitReturn are rather style checks and not just type checks. but they are opt-in, and users can just switch them off all together and maybe rely on a linter rule (which tslint has removed, but i would argue can put back). and in the case of --noUnusedParameters an _ prefix disables the checking all together.

One alternative here is for these style options (--noUnusedLocals, --noUnusedParameters, --noImplicitFallThrough, and noImplicitReturn), their value could be true | false | "warn" to control the error severity.

Similar discussion can be found at #9448

@ghost
Copy link

ghost commented May 19, 2017

There's currently no way to fix fallthrough though; maybe we should recognize a // falls through comment like tslint does.

@evmar
Copy link
Contributor

evmar commented May 20, 2017

@alexeagle has strong opinions in this area.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@alexeagle
Copy link
Contributor

At Google, we've spent a lot of time working through how we present the "severity" of findings from compilers and linters. We have a paper hopefully publishing soon in CACM (I can forward a copy if you're interested).

Basically, we found that developers ignore existing warnings in the code, then that warning blindness extends to new warnings. To combat this, we only present newly introduced warnings. Compilers don't know what has changed, because it would be a layering violation for them to read info from the VCS. Therefore we found that compilers should never produce "warnings" - more precisely, if the compiler "accepts" the program, it should not produce any additional output asking developers to take some optional action.

We turned off all warnings from all compilers (I worked on the Java one quite a bit) and re-implemented the ones we care about as errors, or else left them for a linter. We're doing the same for TypeScript now.

A linter is different - you can hook it up later in the developer workflow, where you do have context from the VCS, and can show only the newly introduced things.

So I believe TypeScript should avoid non-fatal diagnostics (and unused variable was a mistake).

I would love to discuss it further!

@mhegazy
Copy link
Contributor

mhegazy commented May 24, 2017

(I can forward a copy if you're interested).

please do.

@alexeagle
Copy link
Contributor

alexeagle commented May 24, 2017 via email

@danielweck
Copy link

+1

@mhegazy mhegazy added Suggestion An idea for TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Aug 17, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Oct 9, 2017

closed in favor of #13408. please see #13408 (comment) for the current action plan.

@mhegazy mhegazy closed this as completed Oct 9, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants