Skip to content

Enable noFallthroughCasesInSwitch - #19109

Closed
Deleted user (ghost) wants to merge 1 commit into
masterfrom
noFallthroughCasesInSwitch
Closed

Deleted user (ghost) wants to merge 1 commit into
masterfrom
noFallthroughCasesInSwitch

Conversation

@ghost

Copy link
Copy Markdown

Since we have // @ts-ignore comments now, we don't need to use the lint rule for this any more.

@ghost
ghost requested a review from Wesley Wigham (weswigham) October 11, 2017 20:24
@weswigham

Copy link
Copy Markdown
Member

I dislike this, since @ts-ignore squelches all errors on the following line. If we remove one of these syntax kinds for some reason (or just introduce a typo), that use site will not be marked as an error after the removal (or typo introduction). I dislike the collateral damage this causes in terms of potential safety losses - this feels way too clunky.

@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Oct 12, 2017 •

Copy link
Copy Markdown
Member

. If we remove one of these syntax kinds for some reason

Precisely - we actually can detect cases when an enum will never match in a switch/case given accurate types, which you don't want to ignore.

@Jessidhia

Copy link
Copy Markdown

This goes back to the problem of not being able to specify which specific error is ignored by // @ts-ignore; it's just a blanket ignore all things marker.

Is there no provision for, say, a // fallthrough or // fallsthrough comment in the lint rule? I believe at least the eslint rule that checks for this supports such comments.

@ghost

Deleted user (ghost) commented Oct 12, 2017 •

Copy link
Copy Markdown
Author

OK, let's wait on #9448 to be fixed first.
@Kovensky the tslint rule supports // falls through comments, but the TypeScript implementation does not.

@ghost ghost closed this Oct 12, 2017
@ghost
ghost deleted the noFallthroughCasesInSwitch branch October 12, 2017 14:42
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants