Skip to content
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

[no-explicit-any] Allow safe usages of any #1071

Closed
OliverJAsh opened this issue Oct 11, 2019 · 18 comments
Closed

[no-explicit-any] Allow safe usages of any #1071

OliverJAsh opened this issue Oct 11, 2019 · 18 comments
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

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Oct 11, 2019

Repro

{
  "rules": {
    "@typescript-eslint/no-explicit-any": 2
  }
}
type Component<P> = (props: P) => unknown;

type Props = { foo: string; }

declare const MyComponent: Component<Props>;

declare function withBar<T extends Component<any>>(
    component: T,
): Component<Parameters<T>[0] & { bar: string }>;

const NewComponent = withBar(MyComponent);

Expected Result
No error

Actual Result
Error

Additional Info
You might think we should change any to unknown, to fix this error:

-declare function withBar<T extends Component<any>>(
+declare function withBar<T extends Component<unknown>>(

… however, if we do that, the function call withBar will fail under strictFunctionTypes:

/*
Argument of type 'Component<Props>' is not assignable to parameter of type 'Component<unknown>'.
  Type 'unknown' is not assignable to type 'Props'.
*/
const NewComponent = withBar(MyComponent);

For this reason, I believe this is a valid and safe place to use any.

The rule no-explicit-any does exactly what it says on the tin—it prevents all explicit usages of any. Perhaps what I am asking for is a separate rule—one that only disallows unsafe usages of any—although I would question why you would want to disallow all explicit usages including safe ones.

Versions

package version
@typescript-eslint/eslint-plugin 2.2.0
@typescript-eslint/parser 2.2.0
TypeScript 3.6.3
ESLint 5.16.0
node 12.8.1
npm 6.10.2
@OliverJAsh OliverJAsh added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Oct 11, 2019
@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for maintainers to take a look labels Oct 11, 2019
@bradzacher
Copy link
Member

-declare function withBar<T extends Component<any>>(
+declare function withBar<T extends Component<never>>(

repl

you should pretty much never have to use any, as never and unknown should be able to cover.

unknown is the (safe) supertype of all types - it is the set of all possible types.
never is a subtype of all types - it is the empty set.

Another way to think of it:
Every type is assignable to unknown type, but unknown is not assignable to any type.
Inversely, never is assignable to any type, but no types are assignable to never.

@karol-majewski
Copy link

karol-majewski commented Oct 11, 2019

@bradzacher how would you rewrite this definition so it doesn't use any?

import * as React from 'react';

interface Props<T> {
    elements: T[]
}

declare class MyComponent<T> extends React.Component<Props<T>> {};

declare function debug<T extends React.ComponentType<any>>(Component: T): T;

debug(MyComponent);

TypeScript Playground

@bradzacher
Copy link
Member

Hmmm.. Digging into it, from what I can see is the answer is that you can't.
The solutions I came up with butt into problems like microsoft/TypeScript#30650
It appears that typescript does not permit the generics to be carried through like you'd expect. I'd call this a bug in TS, but it appears the TS team calls it a limitation.

There are some rather complex interactions in the react types and react type resolution logic, so it's not entirely surprising.

If you're working with components with generics on the props, then you will have to use any from what I can see.

declare function debug<TProps>(Component: React.ComponentType<TProps>): typeof Component;

const Res1 = debug(MyComponent); // typeof === React.ComponentType<Props<unknown>> 😭

An alternate option is to defer typing to the caller of your HOC, but this is pretty ugly and not a good solution:

declare function debug<TProps>(comp: React.ComponentType<TProps>): never;

const Res1: typeof MyComponent = debug(MyComponent);
const Res2 = debug(MyComponent);

let x = <Res1 elements={['']} />
// works fine

let y = <Res2 elements={['']} />
// JSX element type 'Res2' does not have any construct or call signatures.

Another alternate option is to drop the type restraint, but

declare function debug<T>(comp: T): T;

const Res1: typeof MyComponent = debug(MyComponent);

I would note that back when I was doing react typescript on the regular, JSX + generics was very much an unsupported use case. Since then support has changed a lot (you can explicitly use generics in JSX now, for example), so my knowledge is limited in this regard.
There may be a way to make it work that I don't know.

@bradzacher
Copy link
Member

although I would question why you would want to disallow all explicit usages including safe ones

The problem is that there isn't really any "safe" anys. Or rather it's very, very hard to detect when one is acceptably safe.

I suspect that a rule that detected these edge cases where it's "acceptable" to use any would first require type information, and second would require some very complex logic.

For example, in a different usecase, the debug function provided above could be very, very unsafe:

function debug<T extends Array<any>>(comp: T): T {
    comp.push('');
    comp.push(1);
    comp.push(false);

    return comp;
}

interface Props<T> { element: T[] }
const array = new Array<Props<string>>();
const out = debug(array);
// typeof out === Array<Props<String>>
// but we can see that it clearly no longer matches that signature do to the presence of the values added

repl

What are the differences? between the two.
Well aside from the obvious that one uses Array and the other uses React.ComponentType, there aren't that many.
Defining these exact differences will actually be pretty complex to code up, and I suspect a lot of them come down to "react type semantics" (because React has its own typing semantics thanks to things like LibraryManagedAttributes, which changes a lot of things).

If someone wants to take the time to research, and write such a rule, happy to accept a PR!
But my gut is that the time and effort required to do that well is not really worth the tradeoff when you can just eslint-disable-next-lineing the few times you create a HOC.

@bradleyayers
Copy link
Contributor

How about this?

declare function debug<T>(Component: React.ComponentType<T>): typeof Component;

@karol-majewski
Copy link

karol-majewski commented Oct 14, 2019

For example, in a different usecase, the debug function provided above could be very, very unsafe:

This function would be made both simpler and type-safe if we defined it like this:

function debug<T>(comp: T[]): T[] {
    comp.push('');
    comp.push(1);
    comp.push(false);

    return comp;
}

This wouldn't allow any of the push operations because T is not known in advance.

This signature is saying "give me an array of anything and I will produce an array of anything in return", which is precisely what the code in your example does:

function debug<T extends Array<any>>(comp: T): T {

whereas this one is saying "give me an array of something and I will give you an array of the same something in return", which is a stronger promise.

function debug<T>(comp: T[]): T[] {

How about this?

declare function debug<T>(Component: React.ComponentType<T>): typeof Component;

This signature won't work when props are a union.

import * as React from 'react';

interface LinkProps {
    href: string;
}

interface ButtonProps {
    onClick: React.UIEventHandler<HTMLButtonElement>;
}

type Props = LinkProps | ButtonProps;

declare class Button extends React.Component<Props> {}

declare function debug<T>(Component: React.ComponentType<T>): typeof Component;

debug(Button); // Compile-time error

@karol-majewski
Copy link

karol-majewski commented Oct 14, 2019

Use cases for any

Type guards

When validating user input, unknown makes it cumbersome to check for existence of a property. TypeScript won't let you access the property because it might not exist. One has to use a type assertion, which isn't making things any safer.

interface ErrorLike {
    message: string;
}

function isErrorLike(candidate: unknown): candidate is ErrorLike {
    return (
        typeof candidate === 'object' &&
        object !== null &&
        // 'message' in candidate;                             // Can't do it
        // typeof candidate.message === 'string';              // Can't do it
        (typeof (candidate as ErrorLike).message === 'string') // Could have just used `any`
    )
}

User-defined type guards are unsafe by design — runtime type checking is one of TypeScript's no-goals. The programmer is required to take full responsibility for whatever is happening inside a user-defined type guard. Because we can't rely on the type system to help us there, it's fair to say a more relaxed position towards any is justified.

When it doesn't improve anything

Consider a function that wraps a method from the standard library.

const isEqual = (left: any, right: any): boolean =>
  Object.is(left, right);

Because Object.is accepts any (just like console.log does), restricting the signature of isEqual doesn't add value.

Conditional types

When using the infer keyword, we often care about a single type parameter. If a type accepts more than one, we can simply disregard the part we are not interested in.

type ValueOf<T> =
  T extends Record<any, infer U>
    ? U
    : never

This is a way of saying "for as long as it's a Record, I will give you its value". T be indexed by whatever. We are only interested in the right-hand side. There is nothing to gain by replacing any with a more precise type. We simply don't care, and any indicates that lack of interest.


These are a few examples I leave in case anyone really wants to add some heuristics to this rule. I totally agree it's better to use something other than any when possible or just disable the rule locally if you know what you're doing! 👍

@bradzacher
Copy link
Member

bradzacher commented Oct 14, 2019

declare function debug<T>(Component: React.ComponentType<T>): typeof Component;

This case also doesn't work for the nested generics problem, as mentioned in my comment.
Also, sorry @bradleyayers that people keep on tagging you instead of me... I've filed a ticket with github to see if they can improve the tag autocomplete sorting algorithm.


This function would be made both simpler and type-safe if we defined it like this

It would indeed, I was writing an intentionally bad function signature so that I could highlight the point that it's really, really hard to detect a "safe" usage.

You could also add readonly / ReadonlyArray, and have it similarly type-safe.


I found a nice solution for the unknown -> object problem in this comment, you can actually achieve the type guard nicely without any, and without the pain of unknown:

interface ErrorLike {
    message: string;
}

function isObjectLike(candidate: unknown): candidate is Record<string, unknown> {
  return (
    typeof candidate === 'object' &&
    candidate !== null
  );
}

function isErrorLike(candidate: unknown): candidate is ErrorLike {
  return (
    isObjectLike(candidate) &&
    typeof candidate.message === 'string'
  );
}

repl

The unknown -> object problem is a huge problem with unknown right now, and it really needs to be addressed within typescript itself.


const isEqual1 = (left: unknown, right: unknown): boolean =>
    Object.is(left, right);
isEqual1({ a: 1 }, { b: 2 });

const isEqual2 = <TL, TR>(left: TL, right: TR): boolean =>
    Object.is(left, right);
isEqual2({ a: 1 }, { b: 2 });

repl

Just because the underlying API uses any, doesn't mean you're restrained to using any.
In fact, it using any frees you up to add stricter typings if you'd like.


this particular case you can solve by using the correct union:

type ValueOf<T> =
  T extends Record<string | number | symbol, infer U>
    ? U
    : never;

type Foo1 = ValueOf<Record<string, number>>; // typeof === number
type Foo2 = ValueOf<Record<number, number>>; // typeof === number
type Foo3 = ValueOf<Record<symbol, number>>; // typeof === number

type Foo4 = ValueOf<{ a: 1, b: 2 }>; // typeof === 1 | 2
type Foo5 = ValueOf<{ a: string, b: number }>; // typeof === string | number
type Foo6 = ValueOf<{ a: 1, b: 2, c: boolean, d: object }>; // typeof === boolean | object | 1 | 2

Though this is definitely one of the cases, where any becomes more and more of a valid use case.
The case specifically being: generic types with complex generic constraints when used in complex situations.
It's hard to gauge the exact tradeoff between type safety and brevity. And it's also hard to gauge exactly when the any influences the output.

In the ValueOf type, it's clear that the any has no influence on the actual outcome, but say you're instead dealing with a very strange type:

type WeirdRecord<TKey, TValue> = Record<TKey, TValue | TKey>;

Looking at the call site of the type, you wouldn't be able to tell that providing any influences more than just the key, and similarly, it would be hard to tell from a programmatic POV as well.

This is why I say that it's a very complex problem to solve.


As an aside, I think that in future, this rule would be okay to relax more, when this rule is introduced: #791
Relaxing this rule whilst having a rule which helps catch accidental anys would be the best experience IMO.

@karol-majewski
Copy link

I found a nice solution for the unknown -> object problem in this comment, you can actually achieve the type guard nicely without any, and without the pain of unknown:

That's very nice! Thank you for sharing.

this particular case you can solve by using the correct union:

In this specific example, we don't even need conditional types, we can achieve the same result by defining ValueOf as type ValueOf<T> = T[keyof T];. This works for Record, but does not work for other types utilizing multiple type parameters. I used any to demonstrate my point, which is we don't always care about all of them.

This is why I say that it's a very complex problem to solve.

And I agree! I don't believe it makes sense to try and "solve" it statically. It's better if a programmer uses their best judgment.

@bradzacher bradzacher added enhancement: plugin rule option New rule option for an existing eslint-plugin rule and removed awaiting response Issues waiting for a reply from the OP or another party labels Oct 15, 2019
@Lonli-Lokli
Copy link

What about usage with Angular?
There are some cases which might be valid as well, eg HttpResponse

@bradzacher
Copy link
Member

Thinking about this more - it's really hard to define what a "safe" any is such that it can be statically analysed and detected.

Using any in a type parameter instantiation position in the extends clause of a type parameter declaration is not always safe, as it may leak an any in certain cases, eg:

declare class Foo<T> {
    prop: T;
}

function foo<T extends Foo<any>>(arg: T) {
    arg.prop.a.b.c.d; // prop is type any
}
foo(new Foo<string>()) // will error at runtime

The only place that I can think of where an any is "safe" would be in a type parameter instantiation position in the extends clause of a type parameter declaration where that type parameter declaration belongs to a type, eg:

  • a function signature (no body allowed)
  • a type/interface declaration

Even then, I'm not entirely sure that it's 100% safe to do that.

I think that it would take a lot of TS experience (or rather, specific knowledge of this issue) to understand why no-explicit-any isn't reporting in these very specific case.

Why is this a problem? If an engineer cannot understand why the rule doesn't flag a specific case, then it causes two things:

  • they think it's a bug, and will file an issue here.
    • this creates noise and maintenance overhead for maintainers.
    • 99% of the time people don't read the docs, so likely explicitly documenting this case wouldn't help here.
  • they lose confidence in the rule and by extension the linter.

As such, I think that this proposal should ultimately be rejected.
I think that it's a rare-enough case that either: using unknown / never is good enough, or using any with a disable comment explaining why it's safe is a much better alternative.

@luokuning
Copy link

/*
Argument of type 'Component<Props>' is not assignable to parameter of type 'Component<unknown>'.
  Type 'unknown' is not assignable to type 'Props'.
*/
const NewComponent = withBar(MyComponent);

I was wondering why the error message is Type 'unknown' is not assignable to type 'Props' instead of Type 'Props' is not assignable to type 'unknown'? 😅

@paztis
Copy link

paztis commented Jul 22, 2020

is it possible to add a parameter ignoreArrays to allow any[] only ?
I'm trying to enable this rule on a big project but it causes too many issues, and if we can try to first correct the simple any, then the any arrays, it will be nicer.

@paztis
Copy link

paztis commented Jul 22, 2020

even I don't knoz what to pass instead of the any inside the decorators, in the proxy, ...
get: (target: any, prop: any, receiver: any) => { ... }

These cases are blocking me to adopt this rule, and les developers use any everywhere...
More flexibility will be better with few options

@ccheraa
Copy link

ccheraa commented Jul 23, 2020

This is the situation I found my self in:
if I use any[] I get Unexpected any. Specify a different type.eslint@typescript-eslint/no-explicit-any
if I use anything else I get A mixin class must have a constructor with a single rest parameter of type 'any[]'.ts(2545)

export function makeVariableContent<T extends { new(...args: any[]): BaseNode }>(constructor: T): T  {
  return class extends constructor implements VariableContentNode {
    /******/
  }
}

Maybe we can allow any used in a single rest parameter of type 'any[]' in constructors.

@bradzacher
Copy link
Member

is it possible to add a parameter ignoreArrays to allow any[] only ?

Nope.
Adding an option to help with adoption may sound like a good idea, but it reduces the overall safety of the rule.
Any option we add lives for the life of the rule, so it has to make sense in the context of the rule.

If you have specific requirements for your codebase, it would be pretty trivial to fork the rule temporarily to handle your migration use case, and then delete the fork once your migration is complete.

@bradzacher
Copy link
Member

I'm going to close this issue. This is not possible to action, as there's no way to detect a "safe" any

See my previous comment.
#1071 (comment)

@bradzacher bradzacher added the wontfix This will not be worked on label Jul 23, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2020
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

8 participants