Do we bundle setters and getters?

[eernstg]

The scope rules in Dart include a couple of elements which are specifically dealing with the names of getters and setters, but these rules do not seem to be a perfect fit for any underlying principles.

This issue is a proposal to say "yes", we do consider the names id and id= to be related, we perform lexical lookups for both, and we insist that if we found one of them at a specific level then we must use the other one from the same level. For instance, if an instance getter id is in scope and we have id = e then it must use an instance setter id=, it cannot be a static setter nor a library setter.

In particular, lookups for an identifier reference are specified in terms of the basenames of declarations (it was spelled out as id or id= because that was written before the term basename was introduced):

Let d be the innermost declaration in the enclosing lexical scope whose
name is id or id=

This means that x + 1 below must fail, because there is no getter named x:

// Example 1.

int x = 42;

class C {
  set x(value) {}
  int get foo => x + 1; // Error.
}

This happens because d is the setter, and the only matching case is the one which proceeds to desugar x into this.x (statically and dynamically). If the setter is static then it matches a different case, but we get the same error.

In any case, the underlying principle would be that a getter/setter pair is treated as a single entity (whose name is the basename and the name of the getter) and that entity is capable of shadowing other declarations with the same basename. And this is true even in the case where we only have a lone getter or a lone setter as the lexically nearest declaration.

However, unqualified function invocations do not get the same treatment (as stated here):

If there exists a lexically visible declaration named id, let D_id
be the innermost such declaration.

Note that this text has remained unchanged since 2013 when the language specification was added to the SDK repository, except that the declaration was named $f_{id}$.

So the following must not fail, it will look up x the top-level method and ignore the instance setter:

// Example 2.

int x() => 42;

class C {
  set x(value) {}
  int get foo => x() + 1; // OK, calls top-level function.
}

For an assignment v = e, we have the current text which is based on a lookup of the name v and accepting the case where it is a variable, or the name v= (which will be a setter); so this will allow a local getter to be ignored:

// Example 3.

int x;

class C {
  String get x => "";
  void foo() => x = 2;
}

An older version of this text (6d58ce9bfef) did bundle getters and setters for the dynamic semantics:

Let $d$ be the innermost declaration whose name is $v$ or $v=$, if it exists.

But the static analysis was not very specific:

It is a compile-time error if the static type of $e$ may not be assigned
to the static type of $v$.
The static type of the expression $v$ \code{=} $e$ is the static type of $e$.

That's all. This omits such cases as (1) v is not in the lexical scope (which would be the case when v is an inherited variable, and we should say that we proceed to check compile-time errors for this.v = e), (2) d is an instance getter, and there is no setter (at any level), etc.

For consistency we should reintroduce the bundling of getters and setters also for assignments, preserving the broader coverage of cases that we have in the current text.

However, tools (as of 9dcd7267bacd0924e2bd17a8519e74d0ea480e94) disagree on this topic: With example 1, dartanalyzer reports no issues, but dart reports that there is no getter x. With example 2, dartanalyzer reports no issues, but dart reports that there is no method x. With example 3, both dartanalyzer and dart report that there is no setter x.

We should decide on an underlying principle and then apply that principle consistently. This could be:

Every lookup for an identifier id finds the lexically nearest declaration with basename id, if any.

This principle is consistent with the behavior that we currently see from the common front end (dart and dart2js).

It would introduce some new errors with the analyzer, and in that sense it would be a breaking change, but given that both dart and dart2js are already reporting these errors we have previously considered such changes to be non-breaking "in practice".

So, @lrhn, @leafpetersen, @munificent, WDYT?

[eernstg]

Note that #504 contains further considerations about name clashes based on basenames, in the context of exports and imports. A conclusion from there could be to have something similar to the following rules:

If a library L has a public declaration with basename id then it shadows all re-exported declarations with basename id.

Let D1 and D2 be two declarations re-exported by a library L by exports of distinct libraries L1 and L2. It is a compile-time error if D1 and D2 have the same name, unless they are the same declaration; it is a compile-time error D1 and D2 have the same basename, unless they are declared in the same library.

If a library L has a top-level declaration with basename id then it shadows all imports of a declaration with basename id.

Let D1 and D2 be two declarations with basename id imported by a library L from distinct libraries L1 and L2 (so L has no top-level declaration named id). It is a compile-time error if D1 and D2 have the same name, unless they are the same declaration; it is a compile-time error D1 and D2 have the same basename, unless they are declared in the same library.

We have to say these things slightly differently ('shadow' isn't well-defined, and L1 won't re-export that declaration when it's an error), but you get the gist.

[lrhn]

I think that bundling is a good and sound approach, and we should do it.

We should also treat getter/setter declarations as one thing in all other settings (like the elsewhere mentioned shadowing of exports, and probably also whether a non-platform library import shadows a platform library import).

[leafpetersen]

I would be very happy to treat them uniformly in this way. We would need to understand carefully how breaking this would be. If all of the resulting changes are in places where our tools are already inconsistent, then I think it's completely reasonable to make the change in our preferred direction. Otherwise, we'll need to evaluate the breakage carefully.

Perhaps a good start would be to get a set of co19 tests together that cover all of these cases, then evaluate the current behavior on those test cases?

[leafpetersen]

I wrote some extension method tests to verify that extension methods match at least the class semantics: https://dart-review.googlesource.com/c/sdk/+/112925 .

[leafpetersen]

The tests also include tests for the case that the local extension defines a setter (respectively getter) and another extension defines a getter (respectively setter) or method with the same base name. I'm assuming we want the same error there, but please validate (and see if there are more errors that we should be specifying now that I've missed).

We should update the informal specification to make it clear that these are errors.

[eernstg]

We adopted the bundling consistently for lexical lookups, #548. For imports/exports we're considering, #534, whether it needs to be considered a breaking change.

[eernstg]

The bundling for imports/exports was adopted into the language specification in #1161.