[explicit-function-return-type] Add fixer

[typescript-eslint]

[explicit-function-return-type] Add fixer

[bradzacher]

I never really returned to think about this in greater detail.
We do not want to add a fixer in this rule for a number of reasons, such as:

(1) it forces the rule to need type information.

Note: this is the primary reason that this rule will never receive a fixer.

There is no other way to automatically add the return type without type information.
Many of our users do not use type information for various reasons, so requiring type information in this rule is a major breaking change and it restricts the possible users of the rule unnecessarily.

(2) a fixer won't work well beyond primitive and named types

For example, if you return an object type return { a: { b: { c: 1 } } }, then an automatic fixer will add a lot of verbose code, when there is likely a better type to use. Similarly union types may have aliases, etc.

(3) The spirit of the rule is for engineers to think clearly and establish explicit contracts with the functions they write.

Automatically adding the return type goes against the spirit of the rule, esp when many users have "autofix on save" turned on. Theres little difference to just using the inferred return type with a fixer, as the engineer won't ever have to think about it.

---

I understand that there may be some convenience to having a fixer for those that are adding the rule to a codebase, but a fixer isn't just a codemod tool - it lives on, and runs forever with the rule.

It's for these reasons like this that rules like no-unused-vars don't have fixers either - it might be good to cleanup a codebase, but it's not good during development.

[htho]

I started implementing this. https://github.com/htho/typescript-eslint/commit/bd865d8fe66062f1ea09e17731a5abca679b8225

But I have some issues:

1. In order to get the return type, I use type.getCallSignatures() If there is a known return Type, getCallSignatures returns an array with two identical signatures. Why does this happen, and is there a better way to get the return type?
2. I don't know how to use the fixer in a way that the annotation is placed at the correct position. I want the : string annotation to appear right after the closing bracket: function f(): string {return ""} But I don't know what to pass to fixer.insertTextBefore: or fixter.insertTextAfter return fixer.insertTextAfter(?????????, : ${returnTypes[0]} );

I'd appreciate any hints.

[bradzacher]

For (2) - https://eslint.org/docs/developer-guide/working-with-rules#applying-fixes
insertTextAfter(nodeOrToken, text)

You need to use the source code object context.getSourceCode() to find the token you want to insert text after.

---

For (1)
could you please show me a repro in https://ts-ast-viewer.com/

One note is that I don't think we should ever replace an existing type annotation.
IMO that defeats the purpose of the rule - which is to help inform you of the changes you make to function contracts.
A lot of users use eslint's fix on save functionality, which would mean you'd pretty much be unable to see the error.
Also a lot of users don't use IDE plugins, and rely upon cli, but they run eslint --fix when they lint, so then they would never see an error.

To clarify - definitely think adding missing return type annotations would be awesome, as there's no contract to break there!

[htho]

Thank you for the hints.

The code is as simple as:

class X {
    public getSomeString() {
        return "";
    }
    public getSomeObject() {
        return {foo: 1}
    }
}

After setting the "Show Internals" option on, in the AST-Viewer, there is only one Item in the callSignatures field of the type.
Since this is a field and not the result of getCallSignatures() I am not sure if they are the same.

To clarify - definitely think adding missing return type annotations would be awesome, as there's no contract to break there!

Of course this is only about missing return type annotations.

[bradzacher]

It looks like the getCallSignatures method doesn't just fetch the callSignatures field, it also does some stuff to resolve types, but it looks like it doesn't dedupe or anything.

I'd just spend some time stepping through the function in your debugger so you can interrogate where the additional signatures from.

[astoilkov]

In our codebase, we have 1300 errors from this rule. If someone implements this I would probably save days. Any status on this would be awesome so I can know if I should start working on this.

Thanks in advance.

[bradzacher]

Nobody is working on it. Feel free to put up a PR.

Some things ahead of time:

• The fixer will need to be behind an option, or else it's a breaking change.
• It's fine to fix simple types (boolean, string, etc), type references (Foo, Bar, etc), and array types (foo[]).
• Unions and raw object types can be added as a suggestion fixer, as I don't think they should be auto-fixed due to their verbosity, etc. Also chances are the user has a type reference they want to use, but hadn't asserted the return type to.
• Similarly intersections should be a suggestion fixer, as they are not always intentional, or correct:

  // this should probably be an async function with return type Promise<boolean>,
  // but an auto-fixer would just fix to Promise<boolean> | boolean
  function foo() {
    if (random) { return  Promise.resolve(true) }
    return true;
  }

[astoilkov]

Unfortunately, I don't understand the codebase. It will take me a lot more time to get familiar with it than to go through 1300 errors.