Make sure type-aware lint rules' test cases are type-error-free

[Josh-Cena]

Suggestion

We should add an extra type-checking step to rule-tester when testing type-aware lint rules. Right now, we have some test cases that don't pass, most notably because they have undeclared variables (which become any). However we don't guarantee linting behavior when there are type errors (because the result is likely any, which either created new lint errors or masks actual lint errors). Such type errors may be hard to catch in more complex situations.

Brad mentioned there may be a perf penalty, but it would be useful to experiment nonetheless.

[JoshuaKGoldberg]

I like this. But also per #8231 we'd need a way to mark a test as intentionally having an "error" type.

[auvred]

I sent a draft just to check how many tests contain semantic TS errors: #8351

There is really lot of them...

Test Suites: 51 failed, 129 passed, 180 total
Tests:       1643 failed, 1 skipped, 27628 passed, 29272 total

Some tests, such as dot-notation.test.ts, contain errors in almost every case..

https://github.com/typescript-eslint/typescript-eslint/pull/8351/files#diff-93cb24cbe290e1f30e97aaf0631ade9bd3751820d1f0bef8ecb4a0532243e2bd

So if we decide we want to fix all of them, that's a huge git diff (maybe we can split it into several prs)

---

Also, I'm not sure how best to handle cases with fixers. I suppose we'd like to be able to write both types of test cases:

1. We expect code to contain a certain ts error. But the output shouldn't contain ts errors
2. We expect code to contain a certain ts error. And the output should contain ts error

---

Any thoughts on this? I just want to make sure we want to fix all the failed tests before I start fixing the 1643 cases 😄

[bradzacher]

The issue is that there are a lot of tests where we don't actually care if there is a type error because the code is testing a pure syntactic code path.

For example - dot-notation only cares about types specifically in the case of private member access. So all of those tests with errors are moot - whether or not they have errors they still accurately model the 99.9% case of not being member access on a class.

So if we were to build this we would need a way to easily opt out, IMO. It's not worth us writing type-correct test cases for every single case, IMO, as that would involve multi-line tests where a single-line would have been sufficient.

[JoshuaKGoldberg]

1643 failed

😭.

...although, hey, 1643 / 29272 is ~5.6%. I suppose that's an 'A' grade...? 😂

a way to easily opt out, IMO

I'm now wondering whether it should be opt in or opt out... We the typescript-eslint team are very good at doing this right. But I don't know that we'd want to have that assumption hold by default for not-us folks. And even we've had slips in.

I like how #8351 has an ignoreTsErrors explicitly set. That feels good to me.

Having it key on specific error codes is a little trickier given that they aren't stable version-to-version. That'll likely cause pain down the line the next time TypeScript shuffles around error codes in a version. Which happens once in a while. And is a pity because in theory I do really like the idea of needing to specify which errors are allowed. 😞

split it into several prs

Agreed - for any big change like this I'm a favor of splitting out

[bradzacher]

given that they aren't stable version-to-version

They're a lot more stable than the TS team says they are.

[JoshuaKGoldberg]

You know, Jake Bailey recently asked me out of curiosity what my next big ask for the TS team is... maybe finally getting around to asking for stable error categories is up there.

[Josh-Cena]

I think any rule whose tested code path interacts with the TS compiler should be error-free. For dot-notation, this means any case that involves a computed access and one of the custom options. a[0]; on itself shouldn't be "valid" because the type we retrieve from TS has no semantic meaning. However, those test cases whose code path do not involve type information, I think we can opt out, though I would accept a PR that makes them all "valid". My idea is that our rule tester should have a way to register some global declarations for a single test file, such as declare a: Record<string, unknown>.

[bradzacher]

The difficult thing is that the only way to "register" types is via code. So that means injecting code into the test cases. The complicated thing is that will show up in the test runner output then which can be confusing.

That also adds the complexity of needing to ensure the test case doesn't declare the same variables that are "pre-declared"