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

Enum shorthand design parameters #4149

Open
lrhn opened this issue Nov 3, 2024 · 21 comments
Open

Enum shorthand design parameters #4149

lrhn opened this issue Nov 3, 2024 · 21 comments
Labels
enum-shorthands Issues related to the enum shorthands feature. feature Proposed language feature that solves one or more problems

Comments

@lrhn
Copy link
Member

lrhn commented Nov 3, 2024

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:

  • Any static getter (not just constant getters): .id
  • Constant static getters and constant constructor invocations (must be constant): .id respectively .id(args).
  • Any static getter or constructor invocation. Same syntax
  • Any static getter, constructor or function invocation. .id, .id(args) and .id<typeArgs?>(args) respectively.

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

Constructor invocation syntax

  • Only .id(args), cannot call unnamed constructor.
  • Also .new(args) for the unnamed constructor.
  • Alternatively .(args) for the unnamed constructor.

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:

  • Explicit const .id(args)/const .new(args)/const .(args)

(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.

  • Only .id/.id(args)-or-.new(args)/.id<typeArgs?>(args) allowed of getter, constructor and function respectively.
  • Only .id/.id(args)-or-.new(args)/.id<typeArgs?>(args)? allowed, including a .id<typeArgs?>(args) invoking a function typed getter.
  • .('new'|id)(<selector>*) allowed as shorthand expression, context type used is that of entire selector chain expression. Result must then end up having the correct type
  • .('new'|id) (<selector>*) (assignableSelector op?= expression)? allowed, matching the extent of null shortening/cascades.
  • <shorthandExpression> <op> <expression> also allowed, context type used for implicit access of receiver is that of outer expression. (Generally the outermost expression with a leading .id uses the context type of that.)

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

  • A context type of the form C<T1,...,Tn> (or C if n is zero) where C is a type introduced by a class, enum, mixin or extension type declaration denotes the static namespace of C.
  • Also, a context type of S? denotes the same namespace as S.
    • Optionally: It also denotes the namespace of the class Null. Conflict if the name is in more than one namespace.
  • Also, a context type of FutureOr<S> denotes the same namespace as S.
    • Optionally: It also denotes the namespace of the class Future. Conflict if the name is in more than one namespace.
  • If C is introduced by a sealed class also denotes the static namespaces denoted by all immediate subtype declarations (transitively if those are also sealed). Disambiguation prefers declarations in a superclass to that of a subclass, conflict if there is no most general declaration.)
  • The context type also denotes the static namespaces of every declaration of a subtype of C that is imported into the current library. (Disambiguation prefers superclasses to subclasses, conflict if no most general declaration.)

Plus optionally:

  • Extra namepace provided on the side: foo(Color x from Colors) ... introduces a context where a shorthand .red is looked up in both Color and Colors.
    • Or only in Colors.
  • No reason to stop at one" foo(Color x from Colors | MaterialColors | BananaColors) (strawman syntax!)

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:

  • The raw invocation C.id(args) (which will hopefully have context type C<T1, T2> and will infer <T1, T2>).
  • The instantiated invocation C<T1, T2>.id(args).
    • This does not play well with looking for constructors in subclasses. If NumList<E extends num> extends List<num> and
      has a fromNums constructor, List<String> x = .fromNums(1, 2, 3); won't be valid.
    • Should coordinate with static extensions about how to go from C<T1, T2> to NumList<E>?

Static extension members

Should definitely apply.

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

  • Not apply if any namespace has a non-static-extension member with the same name.
  • Apply on equal footing, as long as the namespace of the static extension doesn't have a member with that name, and that namespace is in play. (Disambiguation is based on the subtype relationships of the type, not the member.)

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.)

  • With no special casing, it's just not allowed, _ denotes no static namespace.
  • We recognize e1 == <shorthandExpression> (and != everywhere we says ==) and use the namespace denoted by the static type of e1 as target namespace for the shorthand expression. (Where e1 cannot be a shorthand expression, not unless we include .id op e above).
    • Will not recognize e1 == condition ? .foo : .bar.
  • We recognize e1 == <shorthandExpression> and use the static type of e1, made nullable, as context type for the RHS.
  • We also recognize <shorthandExpression> == e2 where e2 is not a shorthand expression, and symmetrically infer e2
    first and use its static type, made nullable, as target namespace for the LHS.
    • Or as context type for the LHS.

@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):

  • Allow static getters, functions, and constructors to be accessed.
  • Syntax: Only allow .id for getters, optional const plus .id(args)/.new(args) for constructors, and .id(args) or .id<typeArgs>(args) for functions. No calling getters, no tearing off constructors or functions, whether it manages to type-check or not. (If we get implicit constructors, or other coercions, more things might start type-checking!)
  • S? and FutureOr<S> denotes the same namespace as S (but not Null/Future), and just the one namespace denoted by a class/enum/mixin/extension type context type. No subtypes, no scope-on-the-side. So .id is equivalent to some Foo.id.
  • Non-instantiated/inferred scope for constructors. Foo<int> x = .id(42) is equivalent to Foo<int> x = Foo.id(42); which then infers as Foo<int>.id(42) from context. (It makes a difference if the context type is super-bounded. It may make a difference for static extension constructors with different bounds, if such can exist.)
  • Static extensions apply if the single namespace has no member with the same base name, and there is a single available and applicable static extension which does. Just as if it was written as Foo.id.
  • Equality recognizes RHS shorthand expression, uses the static type of the LHS, made nullable, as context type for the RHS.
    Context type because it matters for constructors.
    • Alternatively: Always use the static type of the LHS, made nullable, as context type of the RHS of ==,
      whether the RHS is a shorthand expression or not. (Not as a required context type, just as a suggestion.)
    • Or only if the LHS's == doesn't have a (covariant) parameter type that's a proper subtype of Object. Which never happens anyway.

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
@lrhn lrhn added the feature Proposed language feature that solves one or more problems label Nov 3, 2024
@bwilkerson
Copy link
Member

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
Copy link
Member Author

lrhn commented Nov 3, 2024

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
Copy link
Contributor

scheglov commented Nov 3, 2024

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
Copy link

tatumizer commented Nov 3, 2024

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. 😄
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
Copy link
Member

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

@stereotype441
Copy link
Member

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
Copy link
Member

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

  • Allow static getters, functions, and constructors to be accessed.
  • Syntax: Only allow .id for getters, optional const plus .id(args)/.new(args) for constructors, and .id(args) or .id<typeArgs>(args) for functions. Calling getters and tearing off constructors or functions is ok, whether it manages to type-check or not. (My rationale: we allow calling a getter via Foo.someGetter(args) or tearing off a constructor/function when writing Foo.id. Not supporting these forms via .someGetter(args) or .id feels like an arbitrary restriction.)
  • Extent: .('new'|id) (<selector>*) (assignableSelector op?= expression)? allowed. <shorthandExpression> <op> <expression> also allowed. (My rationale: although I don't expect this to be used frequently, supporting anything less feels like an arbitrary restriction to me. Also, I did some prototyping and I don't believe it will be significantly more difficult to support than most of the other possibilities.)
  • S? and FutureOr<S> denotes the same namespace as S (but not Null/Future), and just the one namespace denoted by a class/enum/mixin/extension type context type. No subtypes, no scope-on-the-side. So .id is equivalent to some Foo.id.
  • Non-instantiated/inferred scope for constructors. Foo<int> x = .id(42) is equivalent to Foo<int> x = Foo.id(42); which then infers as Foo<int>.id(42) from context. (It makes a difference if the context type is super-bounded. It may make a difference for static extension constructors with different bounds, if such can exist.)
  • Static extensions apply if the single namespace has no member with the same base name, and there is a single available and applicable static extension which does. Just as if it was written as Foo.id.
  • Always use the static type of the LHS, made nullable, as context type of the RHS of == or !=, whether the RHS is a shorthand expression or not. (Not as a required context type, just as a suggestion.) (Note that this is one of @lrhn's alternative suggestions. My rationale: this change will improve the user experience independent of enum shorthands, and it will make the implementation of enum shorthands easier. It's a win all around.)

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
Copy link
Member Author

lrhn commented Nov 4, 2024

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
Copy link
Member

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
Copy link
Contributor

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
Copy link

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
Copy link

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
Copy link

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:

  •  padding: 10px;
    Padding(padding: .all(10))
  •  backGround-color: red
    Container(backgroundColor: .red)
  •  padding: var(--small)
    Padding(padding: .small)

child/children would typically be HTML, not CSS.

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

@tatumizer
Copy link

tatumizer commented Nov 4, 2024

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
Copy link

^ 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.

@kallentu kallentu added the enum-shorthands Issues related to the enum shorthands feature. label Nov 4, 2024
@tatumizer
Copy link

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

WOW, that was quick: 😄

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
Copy link
Member

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

Members that can be accessed:

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

Constructor invocation syntax:

  • .id for getters
  • .id(args) for named constructors + optional const
  • .new(args) for unnamed constructors + optional const

Extent:

  • Up to and including selector chains .('new'|id)(<selector>*)

Namespace

  • No subtyping allowed.

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.

  • S? and FutureOr<S> denotes the same namespace as S (but not Null/Future), and just the one namespace denoted by a class/enum/mixin/extension type context type. No subtypes, no scope-on-the-side. So .id is equivalent to some Foo.id.
  • Non-instantiated/inferred scope for constructors. Foo<int> x = .id(42) is equivalent to Foo<int> x = Foo.id(42); which then infers as Foo<int>.id(42) from context.

Static extensions

  • Static extensions apply if the single namespace has no member with the same base name, and there is a single available and applicable static extension which does. Just as if it was written as Foo.id.

And lastly, our open issues are:

Syntax:

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

Equality:

  • Should we use the static type of e1 as the target namespace or as the context type for the RHS?
  • Should we allow <shorthandExpression> == e2? And if so, the same question as above.

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

@kallentu
Copy link
Member

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

@lrhn
Copy link
Member Author

lrhn commented Nov 14, 2024

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
Copy link

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
Copy link
Member Author

lrhn commented Nov 14, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enum-shorthands Issues related to the enum shorthands feature. feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

9 participants