Improve typedef for callback used as parameter

[VincentLanglet]

PR checklist

• Addresses an existing issue: Inconsistency of typedef:parameter with lambdas #227 ; arrow-call-signature Not working with arrow function missing type definition #2000
• Includes tests

Overview of change:

Typedef arrow-call-signature now return error for

someFunc(() => {});

Is there anything you'd like reviewers to focus on?

Actually, with all typedef rules actives.

const A = () => {} // error (return type)
someFunc(() => {}); // no error
someFunc((n) => {}); // error (param type)
setTimeout(function() {}, 1000); // error (return type)

After the PR, error everywhere ; for consistency.

Here #2000 (comment), a suggestion is a child-arrow-call-signature. That's a possibility, but we have to (for consistency) add a child-arrow-parameter too, which is also a suggestion here #813.
We, then, have to decide before if we want also a child-parameter and a child-call-signature.

I can do it if you prefer.
Personally I don't think all these options are necessary. The typedef is a style rule for people who want to type the code (more like a documentation) even if it's not necessary thanks to inference. If you don't want to be redundant and infer as much type you can, just use the noImplicitAny option in tsconfig.

CHANGELOG.md entry:

[enhancement] typedef rule arrow-call-signature option is more consistent in reporting errors on lambdas and will flag more violations that were missed in the previous rule implementation.

[JoshuaKGoldberg]

The code looks good to me (nice clean implementation!), but since this is a breaking change, I think we'd need to wait until 6.0 to merge in.

/cc @adidahiya

[VincentLanglet]

@JoshuaKGoldberg
The question "Is it breaking change or is it bug fix" is in a way kinda debatable.

There was no consistency in this example

const A = () => {} // error (return type)
someFunc(() => {}); // no error
someFunc((n) => {}); // error (param type)
setTimeout(function() {}, 1000); // error (return type)

So you can think it was a bug to not detect the error for callback return type.

[VincentLanglet]

@JoshuaKGoldberg @adidahiya Is it a breaking change or a bug fix ? I think it's debatable.

When is the v6 supposed to be released ?
If it's in a long time, an option can do the work while waiting

[JoshuaKGoldberg]

breaking change or a bug fix

Both? 😄

Even if this is considered a bug, it's a pretty big change. If a project already enables the rule under the expectation that scenario of arrow lambdas passed to methods shouldn't be flagged, then it starts flagging them, it'd become more difficult to migrate to the next TSLint version (even if this is desirable).

[adidahiya]

yes this is a breaking change and if you want to merge this it needs to be under a new option. I suggest naming the option check-call-signatures

[adidahiya]

why does the failure range extend to the =? that seems a little off

[VincentLanglet]

In const foo = n => n+1 location.pos = 11
in const foo = (n => n+1) location.pos = 13

I didn't touch this part

[adidahiya]

same here, feels strange to include the ( in the failure range

[VincentLanglet]

It's made for (n) => n+1
.....................~~~ [expected arrow-call-signature to have a typedef]

The Node is only n.
But the code use this.addFailure(location.pos - 1, location.end + 1, failure);
I didn't touch this part

[VincentLanglet]

@JoshuaKGoldberg @adidahiya I agree it will be easier with something like arrow-callback-call-signature option. But it does not make any sense having an option for callback in arrow function, and not in standard function, or an option for callback return type in arrow function, and not parameter type in callback function.

A bug fix is always a breaking change. Before the rule had false positive, now it correctly find the errors.

But if you prefer, you can wait for the next major version. But in this case I have a question. When is it planned ? There is already a lot of Breaking changes waiting for it.
https://github.com/palantir/tslint/pulls?q=is%3Aopen+is%3Apr+label%3A%22Type%3A+Breaking+Change%22

[JoshuaKGoldberg]

@VincentLanglet great question! 😄 just filed #4811 to discuss.

[VincentLanglet]

@JoshuaKGoldberg Great ! Is this PR ready for 6.0.0 version ?

[JoshuaKGoldberg]

Deferring to @adidahiya for review now that we're ready for breaking changes (#4811).

[adidahiya]

thanks for the PR @VincentLanglet!