Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do implementers have to implement private members? #1006

Open
wytesk133 opened this issue Jun 4, 2020 · 10 comments
Open

Do implementers have to implement private members? #1006

wytesk133 opened this issue Jun 4, 2020 · 10 comments
Labels
request Requests to resolve a particular developer problem

Comments

@wytesk133
Copy link

On DartPad, the following code does not compile

class A {
  int thisIsPublic;
  int _thisIsPrivate;
}

class B implements A {
  @override
  int thisIsPublic;
}

The error is The non-abstract class 'B' is missing implementations for these members: - A._thisIsPrivate.

Do implementers have to implement private members?

@wytesk133 wytesk133 added the request Requests to resolve a particular developer problem label Jun 4, 2020
@renefloor
Copy link

The dart docs says so yes:

// A person. The implicit interface contains greet().
class Person {
  // In the interface, but visible only in this library.
  final _name;

(https://dart.dev/guides/language/language-tour#implicit-interfaces)

Note "visible only in this library", is you do class B implements A in another library (or even just another file) you don't have to implement the private members.

@pedromassango
Copy link

Thanks. Those details are very important to keep in mind

@ghost
Copy link

ghost commented Jun 4, 2020

This approach doesn't seem safe to me. Consider:

library a;

import 'b.dart';

void main() {
  B()._thisIsPrivate; // NoSuchMethodError: Class 'B' has no instance getter '_thisIsPrivate'.
}

abstract class A {
  int thisIsPublic;
  int _thisIsPrivate;
}
library b;

import 'a.dart';

class B implements A {
  @override
  int thisIsPublic;
}

Now is too late to change this, but just for discussion matters, why not demanding implementations from different libraries to implement private members? Of course it would prevent implementations of classes with private members, but isn't safety more important?

@eernstg
Copy link
Member

eernstg commented Jun 4, 2020

@wytesk133 wrote:

Do implementers have to implement private members?

Yes, the basic rule is that it is an error if a concrete subtype does not have an implementation of a member, private or public.

However, Dart used to be a much more dynamically checked language than it is today, and the treatment of missing private members across different libraries is a corner of the language where sound static checking has not (yet) been introduced, so there is indeed a potential for an error at run time which is not flagged at compile-time, as shown in @hugocbpassos' example here.

It wouldn't be difficult at all to flag this error at compile-time, but it is a breaking change, which is the main reason why it hasn't been done.

It would actually be a very good candidate for a lint, cf. dart-lang/sdk#58179.

@Levi-Lesches
Copy link

@lrhn pointed out in dart-lang/sdk#57805:

class Foo {  // using today's syntax, so no "open" yet
  final String _foo;
  Foo(String foo) : _foo = foo;
  int get hashCode => _foo.hashCode;
  bool operator==(Object other) => other is Foo && _foo == other._foo;
}
class MyFoo implements Foo { }

void main() {
  Foo foo = Foo("h");
  Foo foo2 = Foo("h");
  MyFoo myFoo1 = MyFoo();
  MyFoo myFoo2 = MyFoo();

  print(foo1 == foo2);  // true
  print(myFoo1 == myFoo2);  // false
  print(myFoo1 == foo1);  // false
  /// Unhandled Exception. NoSuchMethodError: `MyFoo` has no instance getter `_foo`
  print(foo1 == myFoo1); 
}

I'm curious how has this not been a bigger issue yet? I'd definitely be supportive of at least a lint to catch this.

@lrhn
Copy link
Member

lrhn commented Jul 5, 2021

The behavior has been consistent since the dawn of Dart (at last since the removal of interface declarations), and I am genuinely impressed that it hasn't been a real issue in practice.

What likely happens is that classes are either intended to be implemented or not. If they are, the author won't write the unsafe == operator above. If they are not, if anyone tries anyway, and things fail, they'll just silently stop trying. The users will learn that "FooBar must be extended, otherwise it doesn't work".
I know we have a few such classes in the dart:io library, likely around sockets, and I probably some in the main platform libraries too, like Symbol and Type. You can implement the interface, it just won't work with any of the code expecting platform sockets, symbols or types. So people learn that and just don't implement them.

@Levi-Lesches
Copy link

Levi-Lesches commented Mar 15, 2023

This issue can also be fixed by disallowing private members to be accessed outside of the class that declares them (#2918), but this looks like a much less breaking change.

@lrhn
Copy link
Member

lrhn commented Mar 15, 2023

That won't fix the problem for:

class Optimist {
  final int _value;
  Optimist(this.value);
  Optimist operator+(Optimist other) => Optimist(_value, other._value);
}

The other._value is the risky one, but it occurs smack in the middle of the declaring class.

The issue is not the location, the issue is that Optimist is used as an interface in other._value, but as a self-access in _value (aka. this._value)

@Levi-Lesches
Copy link

Levi-Lesches commented Mar 15, 2023

Wow, that was fast. Guess this issue is still needed then since it would force any implementer of Pessimist to override _value.

What if that proposal did restrict access to anything other than a self-access (this._private)? EDIT: I added that into the proposal.

@Levi-Lesches
Copy link

Levi-Lesches commented Mar 16, 2023

The core of the issue here is that in the following code, B is an acceptable replacement for A, but C is not.

class A { }
// ------ Different libraries -------
class B extends A { }
class C implements A { }

The reason for this is that A and B might have some private members that C does not, meaning they don't truly share the same interface and are thus not truly interchangeable. No one is debating that the author of this file doesn't make a direct choice when choosing to extend or implement, but I wouldn't agree that they're choosing to make C a "lesser" version of A. In other words, I don't think the author of B and C is deliberately choosing this behavior:

void test(A a) { /* Use some private members of A */ }
void main() {
  test(A());  // OK
  test(B());  // OK
  test(C());  // Compiles, but can lead to runtime errors
}

If either this proposal or dart-lang/linter#2918 is accepted, then there will no longer be any difference. It will break some (inherently unsafe) code, but will ultimately lead to safer usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

6 participants