Skip to content

Conversation

@Busyrev
Copy link
Contributor

@BusyrevBusyrev commented Feb 3, 2018

Fixes#21197
Special syntax available now:

//@ts-ignore TS2349 

to ignore only "Cannot invoke an expression whose type lacks a call signature." error.
Comma separated list supported, trailing comma is supported too:

//@ts-ignore TS2349, TS2552 

If no error code specified, it ignores all errors.

@Busyrev
Copy link
ContributorAuthor

sorry for linting issues, fixing them

@Jessidhia
Copy link

I actually suggested this in the original PR, but the author said he didn't like the idea because you can't tell what is being ignored by just the error code.


namespacets{
constignoreDiagnosticCommentRegEx=/(^\s*$)|(^\s*\/\/\/?\s*(@ts-ignore)?)/;
constignoreDiagnosticCommentRegEx=/(^\s*$)|(^\s*\/\/\/?\s*(@ts-ignore)?(?:\s+((?:(?:TS\d+),\s*)*(?:TS\d+)?))?)/;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ? in (?:TS\d+)? is redundant (causes an extra backtrack into a 0-length match, which is already allowed by the ? in the outer non-capturing match)

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, thanks, I'll remove it later

@Busyrev
Copy link
ContributorAuthor

Busyrev commented Feb 5, 2018

Error codes are public now, they printed berfore error message in error report. Imho blindly ignore everything is very dangerous and not good idea. Ignoring is always not good idea, but additional safety is not superfluous, we are at TypeScript repository, right?

@Busyrev
Copy link
ContributorAuthor

Another suggestion:
I think it would be good idea to show error code in IDE, not only in text report. And it possible to show hint on rollover, like method signature or type information, when pointing on error code.

@lmcarreiro
Copy link

lmcarreiro commented Feb 15, 2018

Don't you have to split the provided string at the , (or /\s*,\s*/g) before use the indexOf?

We have cases like this:

TS9002 Only identifiers/qualified-names with optional type arguments are currently supported in a class 'extends' clause.

and

TS90020 Initialize property '{0}' in the constructor
TS90021 Initialize static property '{0}'
...
TS90029 ...

In this case, if we have a TS9002 and a comment // @ts-ignore TS90021, it would execute "TS90021".indexOf("TS9002") !== -1 and would return true instead of false

@Busyrev
Copy link
ContributorAuthor

@lmcarreiro yes, you are right. I`ll fix it

@lmcarreiro
Copy link

Your regex doesn't match error codes with spaces before comma:
// @ts-ignore TS2349 , TS2552

@mhegazy
Copy link
Contributor

Thanks @Busyrev for the contribution. We have had another heated discussion about this topic in the last language design meeting (see notes in #22011). The conclusion is we could not reach consensuses whether this feature should be added or not. It boils down to concerns about using error numbers, and how they do not add to expressiveness in a position like such. It is a discussion we have once every quarter or so, including when we first added support for ignoring errors. I am sure we will come to it again in the near future.
That all said, I do not think we would be able to take this PR in the time being. thanks again for the contribution and sorry for the delay.

@Busyrev
Copy link
ContributorAuthor

@mhegazy no problem, I can use my own fork :). For discussion I think that short text error idents, like in tslint, would be better. But someone should create them.

@weswigham
Copy link
Member

weswigham commented Feb 28, 2018

For discussion I think that short text error idents, like in tslint, would be better. But someone should create them.

👍 Thank you 😍 This is one if the points we keep going back to 😉

@mhegazy
Copy link
Contributor

closing for now.

@mhegazymhegazy closed this Mar 2, 2018
@microsoftmicrosoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for freeto 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.

5 participants

@Busyrev@Jessidhia@lmcarreiro@mhegazy@weswigham