Analyzer should warn if setter contains a return with a value

[munificent]

Analyzer currently gives me a friendly warning in:

void set bar(value) {
  return "bad";
}

[warning] The return type 'String' is not a 'void', as defined by the method 'bar' (temp.dart, line 2, col 10)

If I annotate that to any other type, it also correctly tells me I'm doing something wrong:

String set bar(value) {
  return "bad";
}

[warning] The return type of the setter must be 'void' (temp.dart, line 1, col 1)

In fact, it won't even let me use dynamic:

dynamic set bar(value) {
  return "bad";
}

[warning] The return type of the setter must be 'void' (temp.dart, line 1, col 1)

So it, really, clearly, obvious, undoubtedly, has a void return type. Even if I want to jam a dynamic in there to shut it up, it won't let me. Given that, if I omit the type:

set bar(value) {
  return "bad";
}

I should still get a warning on the return. The type of the body is void so the return "bad"; is clearly wrong.

Right now, the looser behavior when the type is omitted means you get better static analysis if you add the "void". That encourages people to adopt a pointlessly verbose style. Can we still be smart even if the "void" is omitted so I can tell people they don't have to add it?

[bwilkerson]

Section 10.3 of the specification states:

If no return type is specified, the return type of the setter is dynamic.

and

It is a static warning if a setter declares a return type other than void.

So by omitting the return type you get a return type you can't explicitly state. That seems a bit strange.

[zoechi]

This set "behavior" is especially annoying with shorthand functions (=>) implicit return. I think the spec should be changed that a setter can't have a return type, implicit return is ignored and explicit return with a value is a warning.

[kasperpeulen]

totally agree with @zoechi here

[alan-knight]

I quite like being able to use shorthand functions for setters, which are typically short. So ignoring an implicit return but flagging an explicit one might be reasonable. I'd be annoyed if it started flagging shorthand functions in setters. So I guess I agree with @zoechi too

[srawlins]

The analyzer now reports an error for the omitted void case:

$ dartanalyzer --version
dartanalyzer version 2.0.0-dev.63.0
$ cat 25228.dart 
set bar(value) {
  return "bad";
}
$ dartanalyzer 25228.dart 
Analyzing 25228.dart...
  error • The return type 'String' isn't a 'void', as defined by the method 'bar' at 25228.dart:2:10 • return_of_invalid_type
1 error found.