dart-lang / language

Design of the Dart language
Other
2.65k stars 201 forks source link

Cannot hide individual extension methods. #1627

Open lrhn opened 3 years ago

lrhn commented 3 years ago

See https://github.com/dart-lang/collection/issues/183 for context.

Imagine we have two separate extension libraries on Iterable which both introduce multiple useful members, and both introduce something with the same name, say sortedBy. Someone wants to use both libraries for the methods where there is no conflict, without needing a prefixed explicit extension invocation, but cannot do so and also use sortedBy without a prefix and explicit extension invocation.

If we allowed hiding individual extension methods, say hide IterableExtension.sortedBy, then that would be easier.

The alternative, which is happening is to declare each extension method in its own extension. That's strictly less usable than hiding the individual methods because you have to hide a name which is unrelated to the problem (you have a conflict on sortedBy, so you to find the name and hide IterableSortedByExtension, that's not great usability).

If generally having one-method extensions is the end evolution of the extension method feature in practice, we should consider changing the syntax to something shorter.

eernstg commented 3 years ago

Hiding individual extension methods (or method matchers like hide IterableExtension.[sortedBy, sample]) seems to be much more manageable than splitting a large extension declaration into many single-method extension declarations: The large extension can have a non-trivial on-type which would otherwise have to be repeated many times, the large extension could be hidden in one step and composed using hide and show (e.g., hide IterableExtension show IterableExtension.sample), and so on.

Another matter is that we may actually want something slightly different from hiding and showing: When several extension methods clash we may want to use a specific one by default, and still have access to it by explicit invocation. But we could handle that by making hide and show on individual extension methods an operation which does just that: Resolve any conflicts in favor of "shown" rather than "hidden" extension methods, and preserve the ability to invoke said methods explicitly.

passsy commented 3 years ago

Multiple extensions for same type

Coming from Kotlin, I'm very used to keep extensions separate from each other. Especially because most extensions are unrelated and share nothing but the type they operate on.

Let's consider two examples

Because List<T>.sortedBy and List<T>.mapIndexed operate on the same type they are usually placed in the same extension. Although they are not as close related as List<T>.mapIndexed and Iterable<T>.mapIndexed, they are not only placed in different extensions, but also in different files (iterable_extensions.dart and list_extensions.dart).

In production code, I often find util/xyz_extensions.dart files containing tons of extensions of completely unrelated functionality and layers of an application. Honestly, thousands of lines long. It can become a total mess.

Although this is an unnecessary religious discussion of "folder by type" or "folder by feature" I have to note that grouping extensions by type is the preferred way in dart. Grouping by functionality wasn't much considered during the design and thus is very verbose.

Comparison to Kotlin

As comparison, here are Kotlins extensions that can be imported by name. They are not grouped and can be written in a single line (dart requires 3).

package org.example.declarations

fun List<String>.getLongestString() { /*...*/}
package org.example.usage

import org.example.declarations.getLongestString

fun main() {
    val list = listOf("red", "green", "blue")
    list.getLongestString()
}

Notable is that extensions are actually grouped by feature rather than type (see single extensions) in the stdlib. This also helps with discoverability. Users of dartx (currently grouped by type) have problems finding related methods because they have been split up across files.

Hiding extensions

Extension methods can currently only be referenced by their extension name. Right now it's all or nothing. A single method name conflict with another extension prevents users from using one of the conflicting libraries.

Yes, with as extensions could still be referenced, but the syntax then becomes horrible.

import 'package:dartx/dartx.dart';

final sorted = dogs
    .sortedBy((dog) => dog.name)
    .filter((dog) => dog.name.startsWith('a'));

Becomes

import 'package:dartx/dartx.dart' as dartx;

final sorted = dartx.IterableFilter(
    dartx.IterableSortedBy(dogs).sortedBy((dog) => dog.name))
    .filter((dog) => dog.name.startsWith('a'));
kasperpeulen commented 3 years ago

If generally having one-method extensions is the end evolution of the extension method feature in practice, we should consider changing the syntax to something shorter.

I agree with this. I think one-method extensions are the only reasonable thing. Why shouldn't the consumer be able to only use one of the extensions? Or all except one that clashes which his own extension?

Making a name for an extension also feels off in general. Naming is hard, while you also feel that it doesn't matter that much in this case. I now started using the following convention. Always calling it X[Type]:

extension XTodo on Todo {
  bool dueToday(DateTime now) => dueDate?.isAtSameDayAs(now) ?? false;

  TodoForm asForm() => TodoForm(description: description, dueDate: dueDate, submitting: false);

  Todo updateFromForm(TodoForm form) => copyWith(description: form.description, dueDate: form.dueDate);
}

But then you come into this problem that you want hide a specific one, and making a naming convention for that feels even more off:

extension XTodoDueDate on Todo {
  bool dueToday(DateTime now) => dueDate?.isAtSameDayAs(now) ?? false;
}
extension XTodoAsForm on Todo {
   TodoForm asForm() => TodoForm(description: description, dueDate: dueDate, submitting: false);
}
extension XTodoUpdateFromForm( on Todo {
  Todo updateFromForm(TodoForm form) => copyWith(description: form.description, dueDate: form.dueDate);
}

What about this syntax?

bool Todo.dueToday(DateTime now) => dueDate?.isAtSameDayAs(now) ?? false;
TodoForm Todo.asForm() => TodoForm(description: description, dueDate: dueDate, submitting: false);
Todo Todo.updateFromForm(TodoForm form) => copyWith(description: form.description, dueDate: form.dueDate);

And then hide/show it like this:

import 'todo.dart' show Todo.dueToday;

This is also similar as Kotlin.

eernstg commented 3 years ago

@passsy wrote:

Right now it's all or nothing. A single method name conflict with another extension prevents users from using one of the conflicting libraries.

That is not true: Dart extensions can be imported and used even in the case where they have conflicts:

Assume that extension E1 has a member m which is applicable to a receiver of type T, and extension E2 has a member m which is also applicable to a receiver of type T, then we could get a conflict for call sites like e.m() where e has type T (if the two extensions are equally specific then there is a conflict, otherwise there's none).

But there is certainly no conflict when we're invoking a member m2 where we don't have a member with that basename in both E1 and E2.

So the conflict doesn't exist between extensions, it exists between individual members with the same name.

Yes, with as extensions could still be referenced, but the syntax then becomes horrible.

There is no need to import the extensions with a prefix unless the extensions have the same name (that's the name of the extension, not the name of any member).

By the way, there is no conflict in the given example:

final sorted = dogs
    .sortedBy((dog) => dog.name)
    .filter((dog) => dog.name.startsWith('a'));

If dogs has type Iterable<Dog> then IterableExtension is applicable and ListExtension is not, so there is no need to disambiguate by specifying which extension we want to get the sortedBy and the filter methods from. If dogs has type List<Dog> then ListExtension prevails because it is more specific.

However, even though there's no conflict, we might still want to use the IterableExtension methods in the case where dogs has static type List<Dog>. That could be achieved as follows:

final sorted = IterableExtension(
    IterableExtension(dogs).sortedBy((dog) => dog.name))
    .filter((dog) => dog.name.startsWith('a'));

// OR

final sorted2 = (dogs as Iterable<Dog>)
    .sortedBy((dog) => dog.name)
    .filter((dog) => dog.name.startsWith('a'));

So you're definitely right that the explicit extension method invocation can be verbose and inconvenient.

But the fact that extensions only clash on shared names makes conflicts a lot less common than they would have been if the conflict were on the extensions as a whole. And I believe that the specificity based disambiguation will allow us to get the desired interpretation of extension method invocations in most cases.

For the remaining ones, we could allow local namespace operations to resolve the conflict for any given scope:

extension E1<X> on X { X foo() => this; }
extension E2<X> on X { X foo() => this; }

void main() {
  {
    1.foo(); // Compile-time error, conflict on `E1.foo` and `E2.foo`.
  }
  {
    extension hide E1.foo; // Disable implicit invocations.
    1.foo().foo().foo(); // `E2(E2(E2(1).foo()).foo()).foo();`
    E1(1).foo(); // Still possible.
  }
}
munificent commented 3 years ago

The alternative, which is happening is to declare each extension method in its own extension.

I really don't want users to have to do this. The whole reason we made extensions use a class-like syntax instead of Kotlin's approach is for the convenience of not having to repeat the receiver type for each of a batch of extensions. If users aren't getting that benefit because they need the ability to individually hide extensions, we should fix that glitch. Otherwise, we're giving them a syntax that is strictly worse than Kotlin's.

hatemragab commented 3 years ago

is there updates ? to hide extension