GeezerVison

I'd like to try this in a playground:

TypeScript Bot (@typescript-bot) perf test
TypeScript Bot (@typescript-bot) pack this

Heya Orta Therox (@orta), I've started to run the perf test suite on this PR at eb5610a. You can monitor the build here. It should now contribute to this PR's status checks.

Update: The results are in!

Heya Orta Therox (@orta), I've started to run the tarball bundle task on this PR at eb5610a. You can monitor the build here. It should now contribute to this PR's status checks.

Orta Therox (@orta)
The results of the perf run you requested are in!

Re that On-Demand Benchmark test failure: Monaco check time went up 7.14% (line link). but emit time decreased by almost as much (31s out of 36s), and the total time went up only by 0.57%:

│ Monaco - node (v8.9.0, x86)                                                                     │
├─────────────┬────────────────────┬────────────────────┬───────────────────┬──────────┬──────────┤
│ Memory used │ 203,369k (± 0.02%) │ 204,511k (± 0.01%) │ +1,141k (+ 0.56%) │ 204,463k │ 204,576k │
│ Parse Time  │    1.60s (± 0.35%) │    1.61s (± 0.72%) │  +0.01s (+ 0.56%) │    1.59s │    1.63s │
│ Bind Time   │    0.76s (± 1.35%) │    0.76s (± 0.77%) │  -0.00s (- 0.26%) │    0.74s │    0.77s │
│ Check Time  │    5.08s (± 1.78%) │    5.45s (± 1.22%) │  +0.36s (+ 7.14%) │    5.22s │    5.57s │
│ Emit Time   │    3.21s (± 2.34%) │    2.90s (± 3.30%) │  -0.31s (- 9.66%) │    2.83s │    3.28s │
├─────────────┼────────────────────┼────────────────────┼───────────────────┼──────────┼──────────┤
│ Total Time  │   10.65s (± 0.33%) │   10.71s (± 0.56%) │  +0.06s (+ 0.57%) │   10.59s │   10.87s │

...so this feels like it's not a serious performance degradation, and this PR is still valid? 😁

This is great!

Josh Ghoulberg 👻 (@JoshuaKGoldberg) I've also been toying around with implementing this. However, I was making @ts-ignore and @ts-expect-error pragmas instead of adding a new concept (Comment Directive).

I added the @ts-expect-error as a new pragma (and was going to add @ts-ignore too):

        "ts-expect-error": {
            kind: PragmaKindFlags.AnySingleLine
        }

And then I parse it in processCommentPragmas in parser.ts:

        iterateAllCommentRanges(sourceText, (range) => {
            const comment = sourceText.substring(range.pos, range.end);
            extractPragmas(pragmas, range, comment, PragmaKindFlags.AnySingleLine);
        });

(iterateAllCommentRanges is a new function I added to scanner.ts)

While I couldn't find a formal definition of "pragmas" in programming languages, in some programming languages, a "pragma" can be in places other than just the top of the file (https://dlang.org/spec/pragma.html).

Here's my in progress diff: https://gist.github.com/davidgomes/5cb71342b6d2cf95ffa05a767eb802c7

I’d love to have something more powerful:

//@ts-expect-error 2322 "Type 'number' is not assignable to type 'never'."

Maybe with the option to omit trailing characters of the message and/or the error number. The former could be achieved via regular expressions:

//@ts-expect-error 2322 /^Type 'number' is not assignable to type 'never'\.$/

Rationale: It’s not only a problem to expect errors where there aren’t any, but also to expect the wrong kind of error.

Re-running perf tests, I'm not really convinced this could add 7% after reading the PR

TypeScript Bot (@typescript-bot) perf test
TypeScript Bot (@typescript-bot) pack this

Heya Orta Therox (@orta), I've started to run the perf test suite on this PR at 7a5ef8a. You can monitor the build here.

Update: The results are in!

Heya Orta Therox (@orta), I've started to run the tarball bundle task on this PR at 7a5ef8a. You can monitor the build here.

Hey Orta Therox (@orta), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/67018/artifacts?artifactName=tgz&fileId=5E21999D0FB04B583BB8D7FCD8E00648D55FEC3D17EA25E3229AC512A97BB1D902&fileName=/typescript-3.9.0-insiders.20200303.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

Orta Therox (@orta)
The results of the perf run you requested are in!

These numbers make more sense to me - basically everything is varying in the 0.x range on a plus or minus which is basically just normal noise in perf tests.

This would make it possible to deprecate $ExpectError in dtslint.

It would also reduce noise in IDEs, as $ExpectError still results in the error being shown in the IDE.

OK, this looks good to go - thanks!

I don't know if it's in TypeScript, or in vscode, but we should also get // @ts-expect-error to show up in the autocomplete in comments too, but I'm good for that to be another PR.

The playground doesn't show any of those options.

👍 It's in vscode, in a list of directives: microsoft/vscode#92093

I love this! Does this mean we can use @ts-expect instead of the tslint expect rule and $ExpectError?

I think so, yep

As someone else already mentioned, I second that it would be nice to be able to specify exactly what error to expect (by its id/code etc). I've only used ts-ignore a few times but have already ran into the problem where I think I'm ignoring one error just to have it transform into another completely unrelated error however, unknowingly, since this transformation happened in "the meantime" while the ts-ignore directive was still active/in-place the whole time. So the new error that the old error "transformed into" went completely unnoticed from what I remember (until I removed the directive of course, which was when I found out about the change). Now I try to just avoid ts-ignore altogether because of such potential situations that may arise because it's just not worth taking that kind of a risk imo.

related to Tyler D (@Tyler1205) 's comment I would like to be able to specify number of errors, and that might be easier to handle.

related to Tyler D (@Tyler1205) 's comment I would like to be able to specify number of errors, and that might be easier to handle.

I agree, this is really needed as you can never know if you are blocking more than 1 error, or the wrong kind of error