dart-lang / language

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

Enum shorthand design parameters #4149

Open lrhn opened 2 weeks ago

lrhn commented 2 weeks ago

This is an attempt to summarize the options currently on the table for enum shorthand functionality.

The core functionality is writing .id to refer to an enum value (or enum-like value) as on an implicit class derived from the context type.

Members that can be accessed

The feature can be extended to allow access to more than just static constant variables.

Different options include:

The current consensus seem to be any static getter, constructor or function invocation.

Constructor invocation syntax

Allows consistent use of unnamed constructors, with the syntax we have for treating it as a named constructor.

To invoke a constructor as const in a non-const context, prefix const:

(Mainly to lower the usability cliff and make being const orthogonal to this feature. Changing from .id(args) to const LongNameOfClass.id(args) would feel annoying.)

Extent

Other languages allow .id to be used in places where it doesn't have a context type, but the expression it starts does.

Namespace

The static namespace that id is looked up in is derived from the context type scheme. Not all schemes denote a static namespace, and it's an error if it doesn't.

It may be possible for a context to denote multiple namespaces, if so there needs to be a disambiguation logic. If it's only one namespace, no such logic is needed. A disambiguation conflict is a compile-time error.

Possibilities in order of increasing

Plus optionally:

Instantiated scope for constructors

If the context type is an instantiated type, C<T1, T2>, the a constructor invocation .id(args) is equivalent to either:

Static extension members

Should definitely apply.

If multiple namespaces are available for resolving the shorthand, extension methods should:

Equality (== and !=)

An expression of the form e1 == .id gives .foo a context type scheme of _ today. (It should have given it the parameter type of e1's static type's == operator, made nullable.)


@dart-lang/analyzer-team Did I forget anything?

My personal current recommendation is (with the goal of .id always being equivalent to Foo.id for some easily deducible Foo):

So grammar:

<postfixExpression> ::= ... | <staticShorthandExpression>

<staticShorthandExpression> ::= 
      '.' <identifier>                              -- for getters only
    | 'const'? '.' 'new' <arguments>                -- for unnamed constructor only
    | 'const' '.' <identifier><arguments>           -- for named constructor only
    | '.' <identifier> <arguments>                  -- for named constructors or functions
    | '.' <identifier> <typeArguments><arguments>   -- for functions only
bwilkerson commented 2 weeks ago

This looks good to me.

I'm especially happy to see that there's no specification of an extra namespace in this version of the feature. It should be backward compatible to add that later if there's a strong enough need for it, but we can ship the most critical part of the feature much sooner without it.

... with the goal of .id always being equivalent to Foo.id for some easily deducible Foo ...

That's a fairly critical requirement. The analysis server needs to be able to quickly compute the context type in order to keep code completion fast enough to be usable.

The code completion support already has a notion of context type (used for ranking). We'll need to make sure that it's consistent with the definition in the spec (the two might have diverged) and that the definition in the spec can be computed efficiently enough for completion to work well.

Allow static getters, functions, and constructors to be accessed.

I'm guessing that "functions" here means "static methods".

Static extensions apply if ...

What do you mean by "static extension"? Is this a reference to the possible future language feature that I've heard referred to as "static extension methods", or do you mean something that's in the language today that I might know by a different name?

lrhn commented 2 weeks ago

I'm guessing that "functions" here means "static methods".

Correct. I usually only use "methods" about instance functions. Static/global functions are just functions.

What do you mean by "static extension"?

The hypothetical future "static extension methods" feature, which is an imprecise name if it also allows static extension variables, functions and constructors.

scheglov commented 2 weeks ago

Looks as a fascinating feature for Flutter development :-)

The proposed subset looks reasonable to implement to me from analysis point of view.

I liked reading about sealed subtypes or even just any imported subtypes. Could be useful to refer static getters, but for constructors (usually unnamed) not so much, the new in .new() would not disambiguate. For code completion we already iterate over all possible classes, so can filter based on the context type.

tatumizer commented 2 weeks ago

but for constructors (usually unnamed) not so much, the new in .new() would not disambiguate

That's why you need a "pronoun" (like _) for this namespace. ClassName.id -> _.id (and only then) -> .id, so the default constructor can be referred to as _(...)

Doing it in a single step throws the baby out with the bathwater IMO. :smile: Anyway, this, too, can be dealt with later on.

(As a bonus, you get a place to attach type parameters: Map map = _<String, int>();)

stereotype441 commented 2 weeks ago

@lrhn what's your personal current recommendation for extent? It looks like you're missing a bullet point.

stereotype441 commented 2 weeks ago

Nit: every mention of == above should probably be changed to == or != . We want to preserve the fact that != and == behave the same way (just with inverted outcomes).

stereotype441 commented 2 weeks ago

My personal current recommendation (differences from @lrhn's recommendation noted in bold):

Side note on that last bullet point: last week I was leaning toward "We recognize e1 == <shorthandExpression> or e1 != <shorthandExpression>, and use the namespace denoted by the static type of e1 as target namespace for the shorthand expression." My rationale was that I didn't want to regress the behavior that if the LHS's operator == has a (covariant) parameter type, then that type is used as the context type for the RHS. But then I did some more experiments and remembered that we don't have that behavior anyway! (Today, the RHS of == always has a context of Object?). And besides, as @lrhn points out, no one marks the parameter of operator == as covariant anyway.

lrhn commented 2 weeks ago

what's your personal current recommendation for extent?

That was included in the "Syntax": One getter, one metod or one constructor invocation only. (I also like the "full selector chain", but to keep it simple, we can start with just one member access.) For a selector chain, I'd have made the grammar <postfixExpression> ::= ... | '.' <identifier> <selector>*. (For assignments, I'm not sure if it needs to touch other productions too.)

Not allowing a tear-off is not a necessity, but without a following selector chain, a function type will never be valid for a context type that has a static namespace. So they'll never work (unless the context type is only a suggestion, not a requirement). If we allow a full selector chain, there is no need to make restrictions, we'll just require the result to type-check in the end.

munificent commented 2 weeks ago

Thanks for writing this up, it's really helpful. My perhaps somewhat controversial weakly-held opinion is to keep it as simple as possible and only support static variable/getter access. So just .id, no .id(args).

I like the idea of supporting functions and constructors, but I'm really worried it will lead to code like:

Foo f = .new(.new(.new(), .new(1)), .new(.new(2, .new())));

Of course, we can simply say "Don't do that, it's bad style." But I'm not confident that will be enough.

mateusfccp commented 2 weeks ago

Thanks for writing this up, it's really helpful. My perhaps somewhat controversial weakly-held opinion is to keep it as simple as possible and only support static variable/getter access. So just .id, no .id(args).

I like the idea of supporting functions and constructors, but I'm really worried it will lead to code like:

Foo f = .new(.new(.new(), .new(1)), .new(.new(2, .new())));

Of course, we can simply say "Don't do that, it's bad style." But I'm not confident that will be enough.

I wouldn't worry much about it. I would rather simply suggest it, and people do as they want. Just as today, one can name their variables with single letters, let they do what they want.

Practically speaking, at least in the context of Flutter (and this is the way I program anyway, even when using pure Dart), named parameters are much more used than unnamed (unless they are like 1-parameter functions), and the parameter name usually gives a lot of context when reading the code.

rrousselGit commented 2 weeks ago

IMO being able to write .foo.bar is quite important.

For example, folks would likely want Material shades accessible in Color, to write:

backgroundColor: .amber.shade100;

I'd also have use cases for this in my packages, as a mean to improve the namespace.

Currently in Riverpod, given:

@riverpod
Future<Model> example(Ref) => ...;

Folks can write:

ref.watch(exampleProvider.future)

But we could use this feature to instead write:

ref.watch(.example.future)

Riverpod folks regularly ask for a way to know what all the "providers" in the app are ; or complain about the verbosity of the trailing ...Provider.
This could be an elegant solution to both.

tatumizer commented 2 weeks ago

My perhaps somewhat controversial weakly-held opinion is to keep it as simple as possible and only support static variable/getter access. So just .id, no .id(args).

Here's an extra argument in support of this idea. In Flutter, unnamed constructors are used most often. E.g. whenever we have "child" or "children", each of them involves a (usually unnamed) constructor. The word ".new" suggested as an unnamed constructor shortcut contains zero semantic information - it's pure noise and an eyesore. Supporting constants (and chains anchored at them) is not controversial IMO. Other stuff can be added later, but if the style guide discourages it, why bother?

rrousselGit commented 2 weeks ago

I don't mind removing support for .new(). But IMO .namedConstructor() is critical.

In Flutter, unnamed constructors are used most often. whenever we have "child" or "children", each of them involves a (usually unnamed) constructor.

This feature targets values, not Widgets.
IMO a good indicator of whether shorthands should be used or not is whether there's a correspondance in CSS. So things like:

child/children would typically be HTML, not CSS.

But .id(args) is necessary to support things like padding: 10px vs padding: 10px 5px.

tatumizer commented 2 weeks ago

For this, as a minimum, dart has to introduce another form of factory that can return a subclass

static extension on EdgeInsetGeometry {
   const factory EdgeInset all = EdgeInset.all; 
}

(Not sure about the syntax).

rrousselGit commented 2 weeks ago

^ There are some separate issues for those:

I don't think they are mandatory to implement shorthands though ; as it's just boilerplate reduction. But they are good to have :)

Off-topic, but we can use macros to help defining static extensions here. You previously proposed show/hide keywords. We could have:

@Show(EdgeInset) // Injects all EdgeInsets static member into EdgeInsetGeometry
static extension on EdgeInsetGeometry {}

With macros, boilerplate can be reduced without modifying the language directly.

tatumizer commented 2 weeks ago

For this, as a minimum, dart has to introduce another form of factory that can return a subclass

WOW, that was quick: :smile:

Could someone open an issue to discuss the "injection" mechanism? Is it show/hide, macro or not, what to do about duplicate method names, etc.

kallentu commented 1 week ago

The language team has come to consensus on the following parts from meetings in the past two weeks:

Members that can be accessed:

Constructor invocation syntax:

Extent:

Namespace

The points below have not been contested, so I’ll assume we’ll move forward with these. If not, please bring up concerns and further discussion.

Static extensions

And lastly, our open issues are:

Syntax:

Equality:

@dart-lang/language-team again to discuss the opens above.

kallentu commented 1 week ago

If I'm missing or misunderstanding anything, feel free to call it out. Thanks.

lrhn commented 1 week ago

Syntax:

  • Do we allow calling getters, no tearing off constructors or functions? (And for my understanding, please explain why or why not?)

Since we allow any selector chains, we do not restrict to "one static access", so there are no requirements on the accesses other than that the result is eventually type-valid.

This question only matters for limited syntax, and it's about whether the limit is grammatic or semantic: Is .id allowed because the grammar allows it, even if it denotes a function or constructor, or only if it denotes a getter, and is .id() allowed even if it denotes a getter, or only if it denotes a function or constructor? Is the limit "this syntax" or is it "one static access"?

Allowing full selector chains makes the distinction irrelevant. The limit is the grammar, and that the result ends up valid (which is always a requirement).

(With "only" selectors, not assignment, one cannot do Foo x = .fooDefault ??= Foo(0);, and has to write the Foo in front. There has to be a limit, and this is it for the chosen syntax.)

That leaves equality.

If we're comfortable using the static type of the LHS, made nullable, as context type for the RHS (and in a way that does not cause dynamic downcasts, but which does cause downcasts to the parameter type of the == operator of LHS type of the RHS has type dynamic), then that is the most general approach, which doesn't depend on the syntax of the RHS being recognizable as a static shorthand. It will allow e == (t ? .foo : .bar), where I would be worried about specifying "RHS needs an implicit static context" separately.

It does mean:

class Weird {
  static const dynamic yes = 42;
  static const dynamic bad = "not";
  bool operator ==(covariant int x) => x.isEven;
}
void main () {
  print(Weird() == .yes); // prints true
  print(Weird() == .bad); // throws type error
}

The expression after == has context type Weird?, but must be assignable to int? and is then coerced to int if it's not int?.

That feels weird. "Then don't do that."

Allowing shorthand == expr also requires recognizing the shorthand syntactically. I prefer to avoid that.

tatumizer commented 1 week ago

Consider a valid program:

extension double on num {
  static final x = 1.0;
}
void main() {
  var a = 1.5;
  a = double.x;  // using extension name as a namespace
  print(a); // works!
}

Q: Can we use the shortcut .x to shorten a = double.x; into a = .x ?

lrhn commented 1 week ago

A: No. The context type introduced by a = .x; would be double (the one from dart:core). The static extension on num is not a static extension on double, statics are not inherited, and double (from dart:core) has no static member named x and no static extension adding an x to that namespace.

The accidental clash of names has no effect on anything other than making it harder to directly refer to dart:core's double.