Skip to content

Enhancement: [no-explicit-any] Option to disable rule for generic constraints (revisited) #7751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
4 tasks done
kasper573 opened this issue Oct 14, 2023 · 4 comments
Closed
4 tasks done
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin wontfix This will not be worked on

Comments

@kasper573
Copy link

kasper573 commented Oct 14, 2023

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/no-explicit-any/

Description

I'd like to revisit this issue for adding an option to the no-explicit-any rule that allows the use of any as generic constraint. That issue was seemingly closed by a bot, so I'm not even sure if that issue reached a real consensus. Also, I've written a few code examples that I think add some value to the discussion, which is why I'm opening this issue even if there's technically another existing (closed) issue.

Disclaimer: Before anyone mentions it, I'm aware that this PR in the typescript repo exists and would solve all problems below if merged, but It's been around since 2018 and it's unsure when or if it is going to get merged, so I still think this discussion is worth having.

Example 1: Non-exported types in 3rd party packages

When integrating with 3rd party typescript packages, it's unfortunately common for packages to not expose all the types the consumer may need. While this is a fault in those packages, I think we have to be pragmatic and acknowledge that this is a common enough problem to justify a workaround.

Let's imagine the package example-package that doesn't export a generic constraint type:

type Generics = { a: unknown; b: unknown; c: unknown; };
export type Foo<G extends Generics> = {
  generics: G;
}

This is what it would look like to consume this package:

Strategy 1: Using never as generic constraint

Using never makes the constraint too specific and the function now only accepts Foo<never>

import { Foo } from "example-package";

declare function useNever<T extends Foo<never>>(arg: T): void;
useNever({ generics: {a: 123, b: 123, c: 123} });
//            ^ Type '{ a: number; b: number; c: number; }' is not assignable to type 'never'.(2322)

Strategy 2: Using unknown as generic constraint

Using unknown requires us to reconstruct the type used for the generic constraint while using unknown everywhere.

declare function useUnknown<T extends Foo<{a: unknown, b: unknown, c: unknown}>>(arg: T): void;

This actually works. Neither Typescript or typescript-eslint yields errors, but this is unnecessarily verbose when all I want is to tell typescript that useUnknown should accept any form of Foo. It's also brittle, since I have to redefine implementation details of the 3rd party package. If the package refactors their internal type definitions, I will have to refactor my code.

Strategy 3: Using any as generic constraint

Finally let's try using any. It gives us the best developer experience. It applies the generic constraint we want, is terse and not brittle, and without yielding any typescript errors. However, unfortunately, typescript-eslint complains if you have no-explicit-any enabled.

declare function useAny<T extends Foo<any>>(arg: T): void;

Example 2: The same problem can still occur even if you own all the type definitions

Let's say we have this builder pattern:

type GenericKeys = "a" | "b" | "c"
interface Generics<
  A extends unknown[],
  B extends unknown[],
  C extends unknown[]
> {
  a: A,
  b: B,
  c: C,
}

declare class Builder<Values extends Generics<unknown[], unknown[], unknown[]>> {
  readonly values: Readonly<Values>;
  add <AddTo extends GenericKeys, Added>(): Builder<{
    [K in GenericKeys]: K extends AddTo ? [...Values[K], Added] : Existing[K]
  }>
}

const builder = new Builder<{a: [], b: [], c: []}>()
  .add<"a", number>()
  .add<"a", string>()
  .add<"b", boolean>()
  .add<"b", null>()

// ^ Builder<{ a: [number, string]; b: [boolean, null]; c: []; }>

Now we want to define some functions that accepts generic variants of this Builder type. Observe that if we try the same strategies as in Example 1, we'll experience the exact same problems:

Strategy 1: Using never as generic constraint

declare function useNever<B extends Builder<never>>(builder: B): void;
useNever(builder);
// ^ Argument of type 'Builder<{ a: [number, string]; b: [boolean, null]; c: []; }>' is not assignable to parameter of type 'Builder<never>'

Strategy 2: Using unknown as generic constraint

Once again, this works, but is as verbose and brittle as mentioned in Example 1.

declare function useUnknown<B extends Builder<Generics<unknown[], unknown[], unknown[]>>>(builder: B): void;
useUnknown(builder)

Strategy 3: Using any as generic constraint

Once again, any as generic constraint is the winner:

declare function useAny<T extends Foo<any>>(arg: T): void;

eslint test cases

Fail

// Using `any` outside of generic constraints should still be an error
const foo: any = 123;
function fn (arg: any) { }

Pass

// But using `any` inside a generic constraint should be valid
function fn<T extends any>(arg: T) { }
@kasper573 kasper573 added enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Oct 14, 2023
@bradzacher
Copy link
Member

That issue was seemingly closed by a bot, so I'm not even sure if that issue reached a real consensus.

No, it was locked by a bot.
It was closed by me. Was my closing comment ambiguous?

The comments after closing were all people suggesting example of why you 100% had to use any, followed by us countering with ways to explicitly avoid the any.


For example 1 there exists a much, much simpler option - patch the package to export the type. Modern package managers support patching out of the box - so it's trivial to locally create a patch for your version and then create a PR for the package from that patch.

This avoids the need for any shenanigans or type gymnastics.


For example two - it depends what your goal is. If your goal is to have correct code - then strategy 2 is the correct option.

You have also skipped over an even better solution. You have clearly defined a type where the generics are optional - hence you're happy to pass any to "shut the checker up".

So if the generics are optional define them as such.

interface Generics<
  A extends unknown[] = unknown[],
  B extends unknown[] = unknown[],
  C extends unknown[] = unknown[]
> {
  a: A,
  b: B,
  c: C,
}

class Builder<Values extends Generics = Generics> {...}

declare function useBuilder<B extends Builder>(builder: B): void;

Now you no longer need to define any generics and can pass everything through concisely whilst still having the option to pass generics if required.

@bradzacher
Copy link
Member

bradzacher commented Oct 14, 2023

You have to be really careful with any. It's very easy to create constraints that allow much wider code than you intended.

In your specific example of "a builder type that only uses the generic as a mapped type" - sure there's a lot of implicit safety built in specifically in the way that things are structured.

However if you enable such an option you're opening the door to simpler usecases in your codebase and you can introduce unsafe code very easily.

For example your first example is COMPLETELY UNSAFE.

useAny({
  generics: 1,
// ts does not error here even though 1 clearly does not satisfy the generic object constraint.
});

PLAYGROUND

It is not safe to just enable the use of any in generic constraints. And as such you should only use it very, very sparingly. This usecase is covered - use a disable comment to expliclty opt-out of the rule in those specific cases and provide reviewers with context as to why using any in that specific case is safe.

@bradzacher bradzacher closed this as not planned Won't fix, can't repro, duplicate, stale Oct 14, 2023
@bradzacher bradzacher added wontfix This will not be worked on and removed triage Waiting for team members to take a look labels Oct 14, 2023
@kasper573
Copy link
Author

thanks @bradzacher That final playground is a super example of why even in generic constraints we shouldn't be using any. TIL! Thanks for the quick response and great insight!

@JoshuaKGoldberg
Copy link
Member

Relevant for the exported types point: #7670

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants