Thanks for the PR,@phiresky!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitorsper day.

Also, since prettier does not support this syntax yet, the autoformatter currently strips the types from the docs etc on auto-format, which is pretty unhandy.

Codecov Report

Merging#2202 intomaster willdecrease coverage by0.01%.
The diff coverage is84.61%.

@@            Coverage Diff             @@##           master    #2202      +/-   ##==========================================- Coverage   93.11%   93.10%   -0.02%==========================================  Files         283      284       +1       Lines        9061     9074      +13       Branches     2483     2487       +4     ==========================================+ Hits         8437     8448      +11- Misses        301      302       +1- Partials      323      324       +1

Flags with carried forward coverage won't be shown.Click here to find out more.

Can we have an option on this that enforcesunknown as the argument type in this PR? Or should that be discussed in a seperate issue?

I ideally wantcatch (e) andcatch (e: any) to be errors.

Can we have an option on this that enforcesunknown as the argument type in this PR? Or should that be discussed in a seperate issue?

@tom-sherman, doesn't this PR already cover that?
https://github.com/typescript-eslint/typescript-eslint/pull/2202/files#diff-edf249e9f9f8af86a2cf0ed9bb7c9a3dR42-R48

It does cover that, but I've defaulted it to false since I think you'd usually want to use theno-explicit-any rule for that since it covers all of those cases.

@bradzacher

doesn't this PR already cover that?

You're right, I missed that part - nice!

I made a few updates to fix the CI, but for some reason github isn't picking up the changes.
I committed to your master branch which this PR is tracking - so the changes are there, but unsure why GH isn't adding them to the PR.

Feel free to take a look and merge master into your branch as well.

I committed to your master branch which this PR is tracking

You were able to push to a different users master branch?

I committed to your master branch which this PR is tracking - so the changes are there,

Wait, what? I did not know that was possible and I didn't add change any access permissions. Maybe this is a gihubt bug since github itself didn't even notice the repo change happened, and now the "update from master" button is broken?

Regardless, I've merged upstream master into my branch again so now github has picked up the changes again.

Never mind, I checked theAllow edits by maintainers checkbox when I created the PR. And github picking it up is probably just because it was down (again) earlier today.

You were able to push to a different users master branch?

@WORMSS As phiresky mentioned - GH has a setting on PRs which allows maintainers of the project you're PRing to to commit to the remote branch you want to merge.

This permission is what enables the "update from master" button to work on the UI, but it also lets a maintainer do things like quick edit files in the UI. A keen observer also might note that this ofc means you can checkout the branch locally, make whatever changes, and push the changes.

I usually avoid doing anything beyond updating from master because it:

• removes agency and ownership from the contributor
• is a pain as their local branch will be out of sync with their remote

I usually prefer to rely upon the contributor owning e2e and making the changes that I request, but sometimes I use it in the following cases:

• If I have a minor one-line change (i.e. a documentation change), that's not worth RCing and bothering the contributor with.
• If I want to get something in for a release, I sometimes just make changes myself because I don't know when , and comment with what I did and why.

probably just because it was down (again) earlier today

Yeah, it looks like it was just a problem with github. Seems to be working now.

okay as for the failing tests here - I think we should probably look to split out the 4.0.0 upgrade as a separate PR now.

It looks like 4.0.0 has changed hownull types are represented in addition to the other changes.

Before, the TS parser just outputNullKeyword, but now it outputsLiteralType > NullKeyword, which is a breaking change that we need to "undo".

I'll submit a PR this week to bump the version and fix all of the issues, and then we can rebase this PR to add the rule + support for type annotations on catch clause variables.

What about thrown Promises, should it be included in this rule too?

async function foo() {}foo().catch((err) => console.error(err));            ---- Disallow usage of the implicit `any` type in catch clauses

No - thatcatch is a function call - so it is handled by things like theno-unsafe-* ruleset.
This rule is specifically about enforcing usage of the new TS4.0 synax.