[analyzer] [extension types] Implement 'precludes' rule to eliminate method/setter conflicts

[eernstg]

Edit, Nov 24 2023: This issue has been retargeted, and it is no longer about improving the diagnostic message for a certain method/setter conflict. Instead, the language was modified such that those conflicts do not exist any more, and this issue is now the implementation issue for the new rules.

As such, this is now the analyzer specific issue for #53717.

---

This is the analyzer specific issue for #53717. Consider the following example:

extension type E1(int i) {
  set m(_) {}
}

extension type E2(int i) implements E1 {
  void m() {}
}

void main() {
  E2(1).m = 10;
}

The analyzer reports the following error for the example, which is from #53717:

Class 'E2' can't define method 'm' and have field 'E1.m' with the same name.

This is confusing because there are no instance variable declarations named m anywhere in this program. It would probably be helpful to report something like "Class 'E2' can't define method 'm' and have a setter 'E1.m' with the same name".

(The language specification documents would say that the name of the setter is m= rather than m, but presumably the analyzer error messages consider setters to have names like m in general, without the =.)

[bwilkerson]

Yes, diagnostic messages do not include the = because users don't see the = anywhere in their code.

The rewrite lgtm.

[scheglov]

In the analyzer explicit getters and setters declare implicit fields.

[bwilkerson]

But in messages to users we should always refer to the explicit getter or setter. Referring to an implicit field is confusing because users don't think of there being an implicit field.

[eernstg]

Here's a proposal that allows methods and setters to shadow each other: dart-lang/language#3470.

If the language team prefers to do this (and change the specification such that the example in the original posting isn't an error) then there will be a need to change the analyzer accordingly. Sorry about the twisted path that this topic is following.

[scheglov]

https://dart-review.googlesource.com/c/sdk/+/337641

See also dart-lang/co19#2390

[eernstg]

Just to clarify: The language team did support the proposal in dart-lang/language#3470, and https://dart-review.googlesource.com/c/sdk/+/337641 implements the new rules.

(This means that this issue has been reinterpreted from "improve the diagnostic given when we have this kind of method/setter conflict" to "use the new rules based on the notion of 'precludes' to eliminate this kind of method/setter conflict". I've edited the original post in order to indicate this change.)

[eernstg]

@scheglov, I adjusted the labels based on the fact that the language has changed and this issue is associated with the implementation of the new rules. Please re-adjust the labels if there's a better way to do it.

[scheglov]

@eernstg is this valid code? Do we filter out precluded declarations first, or do we find the unique inherited declaration (report an error if is not unique) and see that it is precluded? FWIW, it is easier to check the unique setter/method to see if it is precluded by declared method/setter.

extension type E1(int it) {
  void foo() {}
}

extension type E2(int it) {
  int get foo => 0;
}

extension type E3(int it) implements E1, E2 {
  set foo(_) {}
}

[eernstg]

That's a good question!

The current wording in the feature specification makes it an error to have implements E1, E2 on an extension type that doesn't declare a member named foo, and there's no help from having the setter foo= because that's not taken into account at that location in the spec.

However, the rule about 'precludes' clearly specifies that E3 has the getter named foo, and not the method.

This means that the original motivation for making the conflict in the superinterface an error on E3 is gone.

See dart-lang/language#3486.