dart-lang / language

Design of the Dart language
Other
2.66k stars 205 forks source link

Allow static and instance members to have the same name. #1711

Open lrhn opened 3 years ago

lrhn commented 3 years ago

We currently do not allow two members declared in the same class to have the same name, even if one is static and the other is an instance member. However, we gladly allow you to declare a static member and an extension member with the same name. That provides a perverse incentive to declare instance methods as extensions methods in order to allow a static member with the same name. (If we ever allow static extension methods, #723, it'll probably switch the other way, with instance methods declared in the class, and extension static members declared on the side.)

There is no resolution issue when doing qualified invocations. An invocation of C.foo() is sufficiently different from new C().foo() that we can always see whether we want to call a static or instance member. It's always a bug to invoke it in the wrong way (unlike, say, Java where you can invoke static members through instances by invoking the member on their static type, like a poor man's non-virtual methods).

The only situation where there is a potential conflict is with unqualified invocations, foo(), where the function is declared in the lexical scope. In that case, we should be able to either

The first item is probably the safest, and most readable (aka. least risk of misreading).

Conflicts will be rare, and they should be clearly visible in the lexical scope that they affect, so it's not going to be breaking code at a distance if you add a second declaration with the same name,

(I was just adding an extension method, and documenting that it was an extension method to avoid a static-member name conflict, and realized that the incentives are wrong here. We're not protecting people by preventing name clashes, we're urging them to circumvent it.)

Levi-Lesches commented 3 years ago

I think this would be great! I recently did a side-project where I needed to have an instance and static method do the same exact thing (instance delegated to the static one). I ended up using a slightly different name for the static method, but it would've been easier to have them share a name instead of remembering which name was static vs instance.

Prefer the instance method in instance methods and the static method in static methods.

Hm, in the context of the static method, shouldn't it be unambiguous? The user can't call an instance method from a static call, so it should be presumed to be the static method in a static context. The only conflict then is in instance members.

I'm in favor of preferring this.foo(), and having to mark TypeName.foo() when necessary. It feels compatible with the natural assumption Dart programmers have of "not including this will still refer to instance members anyway."

munificent commented 3 years ago

SGTM 100%.

lrhn commented 3 years ago

We'd still need to prevent statics and constructors from having the same name. I'd prefer going with "lexical access to contested name is an error" and require you to do this.foo or ClassNam.foo to access the two contesting declarations from inside the same class. (It's only access through an unqualified identifier inside the lexical scope which has any chance of conflict, all external or qualified access is unambiguous).

Picking either of them as default seems spurious (picking the instance member gives errors in all static contexts, picking the static one is likely confusing in instance contexts), and having different defaults for static vs instance contexts is likely even more confusing. (final x = foo(); is a static context, late final x = foo(); is an instance context, users will go "waaa?").

It might work to have the static as default in static contexts and an error in instance contexts. At least things don't silently change behavior when you make something an instance context.

leafpetersen commented 3 years ago

I'm fine with this change.

munificent commented 3 years ago

(final x = foo(); is a static context, late final x = foo(); is an instance context, users will go "waaa?").

Yeah, that is a real footgun.

However, if we make it an error to have a collision, then it means adding an instance method to a base class can be a breaking change to subclasses if they happen to have a static member with the same name. It's obviously already a breaking change if they already have an instance member with that name, so I suppose this is no worse.

Levi-Lesches commented 3 years ago

However, if we make it an error to have a collision, then it means adding an instance method to a base class can be a breaking change to subclasses if they happen to have a static member with the same name.

If I understand this correctly, isn't it already "breaking"? Just logic-wise, not at compile time.

class Person {
  /// List of activity groups this person is part of, like gyms, clubs, institutions, etc.
  List<Group> get groups => [];
}

class Adult extends Person {
  /// There are 16 personality types, at least according to Google
  static numberOfGroups = 16;  

  /// Whether this person feels like they belong in that group
  bool feelsLikePersonalityGroup(int groupNumber) => true;  // humans are weird
  void categorize() {  // yay, tribalism
    for (int index = 0; index < numberOfGroups; index++) {
      if (this.feelsLikePersonalityType(index)) print("I am an $index");
    }
  }
}

If we add an instance property to Person:

class Person {
  int get numberOfGroups => this.groups.length;
}

Now the code in Adult.categorize is all messed up. A compile-time error here would improve things, I think. Of course, this example uses contrived naming because in general, having a property of a class that applies just as much to the whole (static) as the individual (instance) isn't super common. But I guess it can still happen, and in any case, having a clear message from the compiler when there's a possible issue would be helpful.

lrhn commented 3 years ago

However, if we make it an error to have a collision, then it means adding an instance method to a base class can be a breaking change to subclasses if they happen to have a static member with the same name.

If I understand this correctly, isn't it already "breaking"? Just logic-wise, not at compile time.

It's currently a breaking compile-time error. You are not allowed to declare a static member with the same name as a declared or inherited instance member.

It is a compile-time error if $C$
declares a static member with basename $n$ and
the interface of $C$ has an instance member with basename $n$.

That's one of the rules we'd be removing to allow statics and instance members with the same name.

If we do that, then no matter how we handle scope conflicts, adding an instance member to a superclass won't introduce a new scope conflict in a subclass!

The instance member is inherited, it is not in the lexical scope, so local unqualified references will still see the locally declared static member. Qualified accesses never have any problem, unqualified accesses are based on the lexical scope in the current class. The only way to get a lexical-scope conflict is to declare both an instance member and a static member in the same class, and then refer to that name with an unqualified identifier. That requires a local change to the class to introduce, and can be fixed by another local change to that class.

eernstg commented 3 years ago

@lrhn, do you have reason to want this relaxation? We got one motivational example from @Levi-Lesches:

I ended up using a slightly different name for the static method, but it would've been easier to have them share a name instead of remembering which name was static vs instance.

However, I think there is a trade-off: It is in some ways more convenient to use the same name for two different things (we avoid inventing two slightly different names just to avoid the name clash), but it is in other ways more convenient to use different names for different things, because the reader of the code then won't have to run a little snippet of the analyzer in their head, in order to discover which meaning is applicable in each case.

A good example of optimizing for allowing many name clashes is Java (and you can make it a lot worse using static overloading):

class Clash {
  int Clash;
  void Clash(Object o) {
    Clash Clash = Clash(Clash.this.Clash(new Clash()));
  }
  static Clash Clash(Clash Clash) { return Clash; }
  public static void main(String[] args) {
    new Clash().Clash("Clash");
  }
}

Apart from the considerations about the code readability, we would need to introduce a scope distinction (such that instance members and static members are declared in different scopes). Alternatively, we'd have to introduce a highly abnormal rule saying that, just in this case, it's no problem to have multiple declarations in the same scope with the same name. In how many other cases do we want to do a similar thing? How do we make sure that the associated disambiguation rules remain simple and reader-friendly?

lrhn commented 3 years ago

I was originally adding an extension method instead of an instance method, because the instance method caused a conflict with a static, and the extension method doesn't. So, there are cases where you do want the same name, and because most uses are qualified, those names are not in the same mental context, even if they are in the same lexical scope. (For example, you would be able to have both List.sort(list, compare) and list.sort(compare), both of which are reasonable naming.

So, it is occasionally useful, and we already provide a workaround through extension methods, so we might as well allow the direct declaration too.

we would need to introduce a scope distinction (such that instance members and static members are declared in different scopes)

Not necessarily. But we would allow two declarations with the same name in the same scope, if one is static/constructor and the other is instance. It would be a "conflict", in precisely the same way as a conflicted import name: It's an error to refer to that name in that scope., but it's there for lexical lookup purposes. If you don't look it up, there is no error. An explicit C.foo or C().foo would check only for static declarations or only for instance declarations, and would not get a conflict.

munificent commented 3 years ago

@lrhn, do you have reason to want this relaxation?

I have occasionally felt like the most natural name for some static member would collide with an existing instance one. It's not super common, but it's happened.

Levi-Lesches commented 3 years ago

I wonder, though, if making this possible will open up more oppurtunities for collisions, where people would make the wrong choice. We all know it's an error to have a name collision, so we avoid them as much as possible, and only sit back and reflect when we truly feel we need them. But in a world without those errors, we'd all have much more liberty, and that might encourage using the same name when really two different names would be the better choice. How come some of the most difficult problems revolve around thinking of good variable names? đŸ˜‚

I'm probably going too deep, and I don't think I'll ever see code like @eernstg's sample of Java, but just thinking out loud.

lrhn commented 3 years ago

The only collision this approach would open up for is a static and an instance member declared in the same class, and accessed by an unqualified identifier inside that class. For all other uses, there is no conflict.

If we make such an access an unconditional error, no-one will make the wrong choice by accident. They'll have to write either this.x or ClassName.x explicitly. (And they'll know exactly why: Because they asked for it.)

LuisMiguelSS commented 11 months ago

Sadly, no update on this yet