no-unsafe-call: ban calling Function about typescript-eslint HOT 14 OPEN

This would probably be better as a part of no-unsafe-call. Eg ban all calls of a Function typed value, just like it bans all calls of an any typed values

from typescript-eslint.

The problem with doing that is that it requires type info to handle the typeof usage.

no-uppercase-wrapper-types will be pure syntactic.
no-unsafe-call is already type-aware.
So it makes sense (IMO) to split the checks as we don't want to gate a Function ban behind type-info.

from typescript-eslint.

I don't think there's a case where you want to be unsafe in every case. What kind of codebase would say "yes I always want to allow unsafely calling Function"? I think that kind of user wouldn't even use the rule to begin with.

I think no option is fine - people can use a disable comment to opt-out on a case-by-case basis.

from typescript-eslint.

I don't see how Function is any more or less safe than any. You either ban both or neither

from typescript-eslint.

My thinking would be this: most people are going to just enable the rule with the defaults and do no more - we have learned that the majority of users don't want to read docs and granularly configure things and instead want sensible defaults.

So adding an option would likely be unused as we'd default it to true. Most users wouldn't read the docs and decide they want the rule on but with just the Function check only.

I definitely understand your reasoning - but I think that it's just not going to be a usecase people want (and thus unnecessary branching for us).

I'm not saying we never add an option! If people ask for it then we should. But I think shipping without it is completely okay.

from typescript-eslint.

I think I'm ok with that, too. After a little experimentation i think that's safe as far as passing the narrowed function, too, since

declare const looselyTypedFunction: Function;
// TS error, not assignable.
const strictlyTypedFunction: () => void = looselyTypedFunction;

// ditto for passing as a parameter to functions
declare function callsWithAConcreteCallSignature(f: () => void): void;
// TS error
callsWithAConcreteCallSignature(looselyTypedFunction)

So I think Function is only unsafe when directly called, which would get caught with no-unsafe-call

from typescript-eslint.

I guess the downside is just that I could see wanting no-unsafe-call disabled but still wanting the sneaky Function call safety

from typescript-eslint.

💡🧠 completely different direction: what if we instead made a rule like no-unsafe-function-type as a part of #9102? It could be aimed at catching any case where the unsafe Function type creeps in:

• : Function type annotations
• typeof type narrowing on an unknown

Function is already different from the other types banned in #9102's current new no-uppercase-wrapper-types in that it's not a class wrapper around a primitive. And thanks to this issue we can see there's this second difference around typeof narrowing too. That makes me suspect we should have a dedicated rule for the Function type rather than piecemeal it across multiple other somewhat-related rules (no-uppercase-wrapper-types, no-unsafe-call).

from typescript-eslint.

So is our conclusion to build this as a part of no-unsafe-call instead? Should this be an option or just enabled by default?

from typescript-eslint.

My hot take would be to just put both the existing behavior of no-unsafe-call and the Function safety under separate options in order to accomodate #9108 (comment). I don't feel strongly though

from typescript-eslint.

@bradzacher

I don't think there's a case where you want to be unsafe in every case. What kind of codebase would say "yes I always want to allow unsafely calling Function"? I think that kind of user wouldn't even use the rule to begin with.

100% agree.

I think no option is fine - people can use a disable comment to opt-out on a case-by-case basis.

However, I don't think this conclusion follows. It does make sense to want to allow calling any (i.e. people who don't currently use the rule at all) and ban calling Function.

from typescript-eslint.

I'd agree. I don't see why you'd ban one without the other. Both are very clear and present vectors for unsafety and they are really the same thing.

from typescript-eslint.

I agree that there is zero difference in safety. But, there is a difference in ergonomics and developer intention. Banning using any as any is very unwieldy for when you want to just do some intentional yolo coding, but the typeof function laxness is just sneaky unsafety that looks safe, not an intentional opt-out from type safety.

from typescript-eslint.

I'm not saying we never add an option! If people ask for it then we should. But I think shipping without it is completely okay.

Ah, gotcha. I fully agree with this! 🙂

from typescript-eslint.