CFE and analyzer disagree on getter/setter consistency requirements

[leafpetersen]

The CFE accepts this code with no errors:

class A {
  String operator+(int s) => null;
  void set x(String s) {}
  A get x => this;
}
void main() async {
  var a = new A();
  a.x++;
}

The analyzer rejects it with the following error messages:

  error • The return type of getter 'x' is 'A' which isn't assignable to the type 'String' of its setter 'x' at /Users/leafp/tmp/test1.dart:10:9 • mismatched_getter_and_setter_types
  error • The argument type 'int' can't be assigned to the parameter type 'String' at /Users/leafp/tmp/test1.dart:27:3 • argument_type_not_assignable

@eernstg Can you verify which behavior is consistent with the spec and then re-assign to the appropriate area and team?

cc @stereotype441 @kmillikin

[lrhn]

The current specification says:

It is a static warning if a class has
a setter named \code{$v$=} with argument type $T$ and
a getter named $v$ with return type $S$,
and $S$ may not be assigned to $T$.

With that, the first error should be a warning.
The second error seems spurious, the argument to the setter has type String as required. If you replace a.x++ with the equivalent a.x += 1, the error goes away.

The getter/setter-type-incompatibility issue was one of the warnings-turned-errors-turned-warnings-again, but if the analyzer has successfully made it an error, maybe we can keep it as an error.

[leafpetersen]

It does seem to be an error currently, so we could consider keeping it.

[eernstg]

Issues #36126 and #36127 report on the fact that both the analyzer and DDC report errors where the spec says 'static warning'. The vm and dart2js do not report anything, which would be fine if it's a warning.

So one way to settle this is to say (1) all those warnings which are currently reported as errors by the analyzer and DDC remain errors (and we adjust the spec for that again), and then (2) the common front end needs to report those errors as well.

@leafpetersen, @lrhn, @munificent, can you support that?

[leafpetersen]

LGTM.

[lrhn]

LGTM.

[eernstg]

This has been implemented today.