chore: duplicate @typescript-eslint/ban-types locally to allow warning

[G-Rath]

Summary

Duplicates the @typescript-eslint/ban-types rule locally so that we can warn on types like Function that we want to ban, but are used too much in the codebase to be fixed all at once.

As mentioned in these comments.

Test plan

[SimenB]

Ah, nice! I'm not sure a million warnings in the console is very useful, though. Can we use a bunch of overrides entries instead that can be deleted as we clean them up, but still stop people from adding more to other files?

[G-Rath]

Sorry I've been a bit mentally drained, so slowly circling back around to all my open PRs.

I started doing this, and generated the file list:

[
  'e2e/babel-plugin-jest-hoist/__tests__/typescript.test.ts',
  'e2e/coverage-remapping/covered.ts',
  'packages/expect/src/jestMatchersObject.ts',
  'packages/expect/src/matchers.ts',
  'packages/expect/src/print.ts',
  'packages/expect/src/toThrowMatchers.ts',
  'packages/expect/src/types.ts',
  'packages/expect/src/utils.ts',
  'packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts',
  'packages/jest-config/src/Deprecated.ts',
  'packages/jest-core/src/ReporterDispatcher.ts',
  'packages/jest-core/src/TestScheduler.ts',
  'packages/jest-core/src/collectHandles.ts',
  'packages/jest-core/src/plugins/update_snapshots_interactive.ts',
  'packages/jest-environment/src/index.ts',
  'packages/jest-fake-timers/src/legacyFakeTimers.ts',
  'packages/jest-haste-map/src/index.ts',
  'packages/jest-haste-map/src/lib/FSEventsWatcher.ts',
  'packages/jest-jasmine2/src/errorOnPrivate.ts',
  'packages/jest-jasmine2/src/jasmine/SpyStrategy.ts',
  'packages/jest-jasmine2/src/jasmine/Suite.ts',
  'packages/jest-leak-detector/src/index.ts',
  'packages/jest-matcher-utils/src/index.ts',
  'packages/jest-mock/src/__tests__/index.test.ts',
  'packages/jest-mock/src/index.ts',
  'packages/jest-resolve/src/isBuiltinModule.ts',
  'packages/jest-snapshot/src/State.ts',
  'packages/jest-snapshot/src/index.ts',
  'packages/jest-snapshot/src/inline_snapshots.ts',
  'packages/jest-snapshot/src/printSnapshot.ts',
  'packages/jest-snapshot/src/types.ts',
  'packages/jest-types/src/Global.ts',
  'packages/jest-util/src/ErrorWithStack.ts',
  'packages/jest-util/src/convertDescriptorToString.ts',
  'packages/jest-validate/src/types.ts',
  'packages/jest-validate/src/validateCLIOptions.ts',
  'packages/jest-worker/src/Farm.ts',
  'packages/jest-worker/src/index.ts',
  'packages/pretty-format/src/index.ts',
  'packages/pretty-format/src/plugins/DOMCollection.ts'
]

But then I realised a better solution would be to stick /* eslint-disable local/ban-types-eventually */ at the top of all of those files, because then eslint-comments would cause linting to fail when a file no longer needs to be ignored for this rule.

What do you think?

[SimenB]

Agreed!

[G-Rath]

after rebasing:

/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts
  8:19  error  'local/ban-types-eventually' rule is disabled but never reported  eslint-comments/no-unused-disable

/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-config/src/Deprecated.ts
  8:19  error  'local/ban-types-eventually' rule is disabled but never reported  eslint-comments/no-unused-disable

/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-snapshot/src/State.ts
  8:19  error  'local/ban-types-eventually' rule is disabled but never reported  eslint-comments/no-unused-disable

/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-snapshot/src/inline_snapshots.ts
  8:19  error  'local/ban-types-eventually' rule is disabled but never reported  eslint-comments/no-unused-disable

/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-validate/src/types.ts
  8:19  error  'local/ban-types-eventually' rule is disabled but never reported  eslint-comments/no-unused-disable

/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-validate/src/validateCLIOptions.ts
  8:19  error  'local/ban-types-eventually' rule is disabled but never reported  eslint-comments/no-unused-disable

The system at work! ❤️

@SimenB I'll fix the above and then I think this is good to go? tbh I think this is probably a decent way to go about adding new eslint rules to codebases as big as ones like jest, as it strikes a nice balance between "time required to setup" and "time required to ensure all new code won't need to be changed later".

I saw that you've turned off prefer-rest-params & prefer-spread with a comment saying to turn them on at some point - do you want me to see how much work it'd be for those rules and maybe set them up in the same manner in a followup PR? (if its a small subset of files)

[SimenB]

@SimenB I'll fix the above and then I think this is good to go? tbh I think this is probably a decent way to go about adding new eslint rules to codebases as big as ones like jest, as it strikes a nice balance between "time required to setup" and "time required to ensure all new code won't need to be changed later".

Sounds like a great plan 👍

I saw that you've turned off prefer-rest-params & prefer-spread with a comment saying to turn them on at some point - do you want me to see how much work it'd be for those rules and maybe set them up in the same manner in a followup PR? (if its a small subset of files)

That would be lovely!

[SimenB]

@G-Rath would you be up for doing the same for @typescript-eslint/no-explicit-any here? Probably a billion violations, but still

[G-Rath]

ohhh yes for sure! that'll be a great one to have

[SimenB]

Wonderful, thanks! With ignoreRestArgs enabled I believe - at least at first. I've fought long and hard with the type system when trying to tighten test.each types and the rest args

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.