[explicit-function-return-type] Allow implicit void functions

[jorgegonzalez]

It would be nice if this rule had an option to ignore errors for void functions not explicitly marked as such.

Currently:

// Invalid: Missing return type on function
function hello() {
	console.log('Hello');
}

After:

{
	"rules": {
		"typescript/explicit-function-return-type": [
			"error",
			{
				"allowVoid": true
			}
		]
	}
}

// Valid
function hello() {
	console.log('Hello');
}

// Also valid
function something(props, text): void {
	props.text = text;
	return;
}

[armano2]

I think we should do it in different way, if there is no type specified, we should check if return type if consistent:
if all paths has or not have return like: consistent-return eslint rune does it.

it can be easily accomplished with code path

[bradzacher]

There's four cases you have to test on each path:

• no return at all
• return;
• return undefined;
• x = () => void; return x();
  repl

All three are the same type-wise.
The last one is dubious, but is technically correct.
There has been talk of adding rules/features which statically analyse types, so this could potentially be a case for that...

The question is - what is the motive behind this rule?
IMO there are two motives:

1. documentation - it's easier to read the return type than it is to read the entire function to determine it's return type.
2. type soundness - you know that your return types are correctly inferred, and there are no invalid cases accidentally introduced. This is a great help esp if you don't have strict null checks turned on:

function foo(param : string) { // return type = string
  return 'string' + param;
}
foo('').length // runtime safe! 

// modify code

// strict return type = string | null
// non-strict return type = string
function foo(param : string) {
  if (!string) {
    return null;
  }

  return 'string' + param;
}
foo('').length // non-strict, compile-time safe, but breaks at runtime

repl - use the options to turn strict nulls on/off

So the question is - why strictly enforce return type on void?
Does disabling the check on void returns benefit the rule?

1. documentation

• not explicitly stating the return type is void is ambiguous, so it breaks motivation 1.
• "did their linter not run and they forgot to put the return type?"

2. type soundness

• by not requiring voids, we still implicitly have the check for void (because if you add/remove a return value to/from a void function, the lint warning will throw because of the change in return type).
• this doesn't break motivation 1, as it still ensures the same type soundness.

So the question is this - is the saving of 5-7 characters per void function and breaking of motivation (1) worth the code complexity that will be required to do function path and return type analysis?

[armano2]

why did you close this one? this issue is not resolved yet :>

[jorgegonzalez]

I understood that you didn't want to implement it ... and I've personally gotten used to explicitly typing void

[armano2]

currently we will have to add a lot of logic for this but, when type analysis will be available in parser we will have access to it directly.

I will like to implement this feature, but not before we are going to update parser to v21.*

[jorgegonzalez]

In that case, forgive me for closing.

[davidje13]

@armano2

I will like to implement this feature, but not before we are going to update parser to v21.*

Is there a separate issue for tracking this? I couldn't find anything.

[bradzacher]

@davidje13 type information has been available to rules for quite a while.

---

Looking at this enhancement again, I think that whilst it's easy to do, it goes against the intention of the rule. We don't want a special case for void. It's 4 characters, and not including it creates a lot of inconsistencies (which this rule is attempting to stop).

[davidje13]

@bradzacher my main interest in this is for Jasmine/Jest testing. To give you an idea of what that looks like:

describe('some thing', (): void => {
  describe('some method', (): void => {
    it('does stuff', (): void => {
    });
    it('does more stuff', (): void => {
    });
    it('does something asynchronously', async (): Promise<void> => {
      await foobar();
    });
  });
});

Clearly this is very verbose (especially for the async block). Allowing implicit void returns (and Promise<void> in the async case) would be hugely beneficial.

Currently I am setting 'allowExpressions': true, but this is not ideal because it means there are many other places I may be getting an unexpected return type. Essentially I'm currently forced to abuse another flag to get behaviour which is close to but not actually what I need, causing me to miss issues I'd like to catch.

[davidje13]

An alternative fix which would work for my use-case is extra logic if a function is defined inline as a parameter to another (typed) function: if the return type could implicitly match the expected return type, and it can be verified that the implementation does indeed conform to it, omitting an explicit return type seems logical. Presumably the same logic could apply if a lambda is assigned to a typed variable.

In the above case, because describe and it expect (approximately) a () => (void | Promise<void>), the given lambdas can be checked against that requirement. This makes it clear that explicit return types are just noise in these cases (and this applies to more than just testing APIs)

[bradzacher]

I direct you to either have a look at the rule's documentation, or read the collapsed comments where another user had the same problem you have.
allowTypedFunctionExpressions will do what you want.

---

For tests specifically, you could use eslint overrides to disable the rule. Considering tests aren't production code, it isn't a problem to have more relaxed linting.

[davidje13]

@bradzacher in theory that seems to be what I'm looking for, though I'm observing the same behaviour as thisissami. Presumably it's a bug; if I can narrow it down I'll open a new issue.

I'm already using eslint overrides for relaxing rules for tests, but I still have helpers, etc. which I'd like to have some certainty around.

[davidje13]

@bradzacher I have created #679 which demonstrates this not working in a minimal case.

[thisissami]

@davidje13 on my computer, all it took was a VScode reload for this to work properly.

[davidje13]

@thisissami that's weird considering the feature was apparently never implemented (see the PR for the issue I raised); are you sure you didn't set another option or disable the rule entirely somehow?

[thisissami]

@davidje13 i have the following in my .eslintrc.js:

    "@typescript-eslint/explicit-function-return-type": [
      "warn",
      {
        allowExpressions: true,
        allowTypedFunctionExpressions: true,
      }
    ],

When I first chimed in this thread, I had first set the rules as per immediately above - and was getting warnings on all my jest describe blocks. When I next opened VSCode, all the warnings were gone.

However, doing some digging now - it's the allowExpressions: true that makes the warnings go away - not the allowTypedFunctionExpressions. Sounds like there is indeed a bug!

[tomnar]

Man I wish this feature existed. 90% of my functions are void - it would be very nice way of telling me "hey, this function actually returns something, is this the expected type?". But adding void, void, void everywhere makes it harder to read/write and adds no value.

[johannesjo]

Can't believe that this is closed....

@davidje13 type information has been available to rules for quite a while.

Looking at this enhancement again, I think that whilst it's easy to do, it goes against the intention of the rule. We don't want a special case for void. It's 4 characters, and not including it creates a lot of inconsistencies (which this rule is attempting to stop).

The way I see it, it's just a sensible default to save oneself a lot of time not an inconsistency.

The way it is, I am so annoyed by having to write void all the time that I disable the rule completely, which is really not ideal neither. Because void and non void functions are often conceptually different it even makes more sense to have a strong visual separation and personally I think that:

function bla() {}
function bla(): string {} 

is easier to distinguish then:

function bla(): void {}
function bla(): string {} 

It wouldn't have to be a default, just an optional rule.

[bradzacher]

We won't be building type information into the rule because:

• many users don't use type information, so adding it to an existing rule severely limits the userbase that can use a rule.
• adding it to an existing rule is a breaking change, which needs a major version bump.
• type information is created lazily by typescript, which means that each new piece of type information consumed comes with a non-trival performance overhead. So we opt for static analysis of the AST where possible.
• we don't like to conditionally consume type information in a rule because it creates a confusing experience for users; a rule might work in their setup until they turn on a certain option. This creates maintenance burden for the maintainers, who volunteer their time to support this project.

If this is something that you few would find valuable, this code is released under an MIT licence, so you're welcome to create, say, eslint-plugin-explicit-function-return-type, and create a new version of the rule which supports your use case.