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

Allow for shorter dot syntax to access enum values #357

Open
rami-a opened this issue May 16, 2019 · 331 comments
Open

Allow for shorter dot syntax to access enum values #357

rami-a opened this issue May 16, 2019 · 331 comments
Labels
brevity A feature whose purpose is to enable concise syntax, typically expressible already in a longer form enum-shorthands Issues related to the enum shorthands feature. enums feature Proposed language feature that solves one or more problems

Comments

@rami-a
Copy link

rami-a commented May 16, 2019

When using enums in Dart, it can become tedious to have to specify the full enum name every time. Since Dart has the ability to infer the type, it would be nice to allow the use of shorter dot syntax in a similar manner to Swift

The current way to use enums:

enum CompassPoint {
  north,
  south,
  east,
  west,
}

if (myValue == CompassPoint.north) {
  // do something
}

The proposed alternative:

enum CompassPoint {
  north,
  south,
  east,
  west,
}

if (myValue == .north) {
  // do something
}
@johnsonmh
Copy link

This would be especially nice in collections:

const supportedDirections = <CompassPoint>{.north, .east, .west};
bool isSupported = supportedDirections.containsAll({.north, .east});

It's worth noting too that we would only allow it in places where we can infer the enum type.

So

final north = .north; // Invalid.
final CompassPoint north = .north; // Valid.
final north = CompassPoint.north; // Valid.

@kasperpeulen
Copy link

kasperpeulen commented May 18, 2019

In Swift this feature works not only for enums but also for static properties of classes. See also:

munificent/ui-as-code#7

class Fruit {
    static var apple = Fruit(name: "apple");
    static var banana = Fruit(name: "banana");
    
    var name: String;
    
    init(name: String) {
        self.name = name;
    }
}

func printFruit(fruit: Fruit) {
    print(fruit.name);
}

// .banana is here inferred as Fruit.banana
printFruit(fruit: .banana);

@lrhn
Copy link
Member

lrhn commented May 20, 2019

How would the resolution work?

If I write .north, then the compiler has to look for all enums that are available (say, any where the name of the enum resolves to the enum class), and if it finds exactly one such which has a north element, use that.
If there is more than one enum class in scope with a north element, it's a compile-time error. If there is zero, it is a compile-time error.

If we have a context type, we can use that as a conflict resolution: <CompassPoint>[.north, .south] would prefer CompassPoint.north, CompassPoint,south over any other enum with a north or south element.
We won't always have a context type, the example if (myValue == .north) { does not.

Alternatively, we could only allow the short syntax when there is a useful context type.
For the equality, you will have to write CompassPoint.north (unless we introduce something like "context type hints" because we know that if one operand of an == operator is a CompassPoint enum type, and enums don't override Object.==, then the other is probably also a CompassPoint, but that's a different can of worms).
Then we could extend the behavior to any static constant value of the type it's embedded in.
That is, if you have Foo x = .bar; then we check whether Foo has a static constant variable named bar of type Foo, and if so, we use it. That way, a user-written enum class gets the same affordances as a language enum.

I guess we can do that for the non-context type version too, effectively treating any self-typed static constant variable as a potential target for .id.

(Even more alternatively, we can omit the . and just write north. If that name is not in scope, and it's not defined on the interface of this. then we do "magical constant lookup" for enum or enum-like constant declarations in scope.
That's a little more dangerous because it might happen by accident.

@eernstg
Copy link
Member

eernstg commented May 20, 2019

One approach that could be used to avoid writing CompassPoint several times is a local import (#267).

@kasperpeulen
Copy link

How would the resolution work?

@lrhn You may want to study how it works in Swift. I think their implementation is fine.

@johnsonmh
Copy link

@lrhn

Alternatively, we could only allow the short syntax when there is a useful context type.

If we're taking votes, I vote this ☝️

Regarding the case with if (myValue == .north) {, if myValue is dynamic, then I agree, this should not compile. However; myValue would often already be typed, if it is typed, it should work fine. For example:

void _handleCompassPoint(CompassPoint myValue) {
  if (myValue == .north) {
    // do something
  }   
}

For the equality, you will have to write CompassPoint.north

I don't know enough about this, but I don't see why this would need to be the case if we're going with the "useful context type" only route?

Right now we can do:

final direction = CompassPoint.north;
print(direction == CompassPoint.south); // False.
print(direction == CompassPoint.north); // True.
print("foo" == CompassPoint.north); // False.

If we know that direction is CompassPoint, can we not translate direction == .south to direction == CompassPoint.south? Or is that not how this works?

Even more alternatively, we can omit the . and just write north

I don't personally prefer this approach because we risk collisions with existing in scope variable names. If someone has var foo = 5; and enum Bar { foo, }, and they already have a line foo == 5, we won't know if they mean Bar.foo == 5 or 5 == 5.

@lrhn
Copy link
Member

lrhn commented May 22, 2019

The problem with context types is that operator== has an argument type of Object. That gives no useful context type.

We'd have to special case equality with an enum type, so if one operand has an enum type and the other is a shorthand, the shorthand is for an enum value of the other operand's type. That's quite possible, it just doesn't follow from using context types. We have to do something extra for that.

@lrhn
Copy link
Member

lrhn commented Jun 24, 2019

We can generalize the concept of "enum value" to any value or factory.

If you use .foo with a context type of T, then check whether the class/mixin declaration of T declares a static foo getter with a type that is a subtype of T. If so, use that as the value.
If you do an invocation on .foo, that is .foo<...>(...), then check if the declaration of T declares a constructor or static function with a return type which is a subtype of T. If so, invoke that. For constructors, the context type may even apply type arguments.

It still only works when there is a context type. Otherwise, you have to write the name to give context.

@ReinBentdal
Copy link

ReinBentdal commented Jun 24, 2019

To omit the . would make sense for widgets with constructors.

From

Text(
  'some text',
  style: FontStyle(
    fontWeight: FontWeight.bold
  ),
),

To

Text(
  'some text',
  style: ( // [FontStyle] omitted
    fontWeight: .bold // [FontWeight] omitted
  ),
),

For enums and widgets without a constructor the . makes sense to keep, but for widgets where the . never existed, it makes sense to not add it.

FontWeight.bold -> .bold // class without a constructor
Overflow.visible -> .visible // enum
color: Color(0xFF000000) -> color: (0xFF000000) // class with constructor

From issue #417

_Some pints may have been presented already

Not include subclasses of type

Invalid
padding: .all(10)

This wont work because the type EdgeInsetsGeometry is expected, but the type EdgeInsets which is a subclass is given.

Valid
textAlign: .cener

This will work because TextAlign is expected and TextAlign is given.
The solution for the invalid version would be for flutter to adapt to this constraint.

The ?. issue

Alot of people have pointed out this issue on reddit. The problem is as follows:

bool boldText = true;

textAlign = boldText ? .bold : .normal;

The compiler could interpret this as boldText?.bold.
But as mentioned on reddit: https://www.reddit.com/r/FlutterDev/comments/c3prpu/an_option_to_not_write_expected_code_fontweight/ert1nj1?utm_source=share&utm_medium=web2x
This will probably not be a problem because the compiler cares about spaces.

Other usecases

void weight(FontWeight fontWeight) {
  // do something
}
weight(.bold);

@andrewackerman
Copy link

@ReinBentdal

Omitting the period for constructors would lead to a whole slew of ambiguous situations simply because parentheses by themselves are meant to signify a grouping of expressions. Ignoring that, though, I think removing the period will make the intent of the code far less clear. (I'm not even sure I'd agree that this concise syntax should be available for default constructors, only for named constructors and factories.)

And about the ?. issue, like I said in both the reddit post and issue #417, the larger issue is not whether the compiler can use whitespace to tell the difference between ?. and ? .. It's what the compiler should do when there isn't any whitespace at all between the two symbols. Take this for example:

int value = isTrue?1:2;

Notice how there is no space between the ? and the 1. It's ugly, but it's valid Dart code. That means the following also needs to be valid code under the new feature:

textAlign = useBold?.bold:.normal;

And now that there's no space between the ? and the ., how should the compiler interpret the ?.? Is it a null-aware accessor? Is it part of the ternary followed by a type-implicit static accessor? This is an ambiguous situation, so a clear behavior needs to be established.

@ReinBentdal
Copy link

A solution could be to introduce a identifyer.

*.bold // example symbol

But then again, that might just bloat the code/ language.

@lukepighetti
Copy link

lukepighetti commented Feb 27, 2020

I'd like to see something along these lines

final example = MyButton("Press Me!", onTap: () => print("foo"));

final example2 = MyButton("Press Me!",
    size: .small, theme: .subtle(), onTap: () => print("foo"));

class MyButton {
  MyButton(
    this.text, {
    @required this.onTap,
    this.icon,
    this.size = .medium,
    this.theme = .standard(),
  });

  final VoidCallback onTap;
  final String text;
  final MyButtonSize size;
  final MyButtonTheme theme;
  final IconData icon;
}

enum MyButtonSize { small, medium, large }

class MyButtonTheme {
  MyButtonTheme.primary()
      : borderColor = Colors.transparent,
        fillColor = Colors.purple,
        textColor = Colors.white,
        iconColor = Colors.white;

  MyButtonTheme.standard()
      : borderColor = Colors.transparent,
        fillColor = Colors.grey,
        textColor = Colors.white,
        iconColor = Colors.white;

  MyButtonTheme.subtle()
      : borderColor = Colors.purple,
        fillColor = Colors.transparent,
        textColor = Colors.purple,
        iconColor = Colors.purple;

  final Color borderColor;
  final Color fillColor;
  final Color textColor;
  final Color iconColor;
}

@MarcelGarus
Copy link
Contributor

MarcelGarus commented Jul 3, 2020

Exhaustive variants and default values are both concepts applicable in a lot of scenarios, and this feature would help in all of them to make the code more readable. I'd love to be able to use this in Flutter!

return Column(
  mainAxisSize: .max,
  mainAxisAlignment: .end,
  crossAxisAlignment: .start,
  children: <Widget>[
    Text('Hello', textAlign: .justify),
    Row(
      crossAxisAlignment: .baseline,
      textBaseline: .alphabetic,
      children: <Widget>[
        Container(color: Colors.red),
        Align(
          alignment: .bottomCenter,
          child: Container(color: Colors.green),
        ),
      ],
    ),
  ],
);

@lrhn lrhn added the small-feature A small feature which is relatively cheap to implement. label Jul 8, 2020
@lrhn lrhn removed the small-feature A small feature which is relatively cheap to implement. label Sep 8, 2020
@munificent
Copy link
Member

munificent commented Sep 10, 2020

Replying to @mraleph's comment #1077 (comment) on this issue since this is the canonical one for enum shorthands:

I think this is extremely simple feature to implement - yet it has a very delightful effect, code becomes less repetetive and easier to read (in certain cases).

I agree that it's delightful when it works. Unfortunately, I don't think it's entirely simple to implement. At least two challenges are I know are:

How does it interact with generics and type inference?

You need a top-down inference context to know what .foo means, but we often use bottom-up inference based on argument types. So in something like:

f<T>(T t) {}

f(.foo)

We don't know what .foo means. This probably tractable by saying, "Sure, if there's no concrete inference context type, you can't use the shorthand", but I worry there are other complications related to this that we haven't realized yet. My experience is that basically anything touching name resolution gets complex.

What does it mean for enum-like classes?

In large part because enums are underpowered in Dart, it's pretty common to turn an enum into an enum-like class so that you can add other members. If this shorthand only works with actual enums, that breaks any existing code that was using the shorthand syntax to access an enum member. I think that would be really painful.

We could try to extend the shorthand to work with enum-like members, but that could get weird. Do we allow it at access any static member defined on the context type? Only static getters whose return type is the surrounding class's type? What if the return type is a subtype?

Or we could make enum types more full-featured so that this transformation isn't needed as often. That's great, but it means the shorthand is tied to a larger feature.

How does it interact with subtyping?

If we extend the shorthand to work with enum-like classes, or make enums more powerful, there's a very good chance you'll have enum or enum-like types that have interesting super- and subtypes. How does the shorthand play with those?

Currently, if I have a function:

foo(int n) {}

I can change the parameter type to accept a wider type:

foo(num n) {}

That's usually not a breaking change, and is a pretty minor, safe thing to do. But if that original parameter was an enum type and people were calling foo with the shorthand syntax, then widening the parameter type might break the context needed to resolve those shorthands. Ouch.

All of this does not mean that I think a shorthand is intractable or a bad idea. Just that it's more complex than it seems and we'll have to put some real thought into doing it right.

@Abion47
Copy link

Abion47 commented Sep 10, 2020

@munificent

If changing the interface breaks the context to the point that name inference breaks, then that is probably a good thing in the same way that making a breaking change in a package should be statically caught by the compiler. It means that the developer needs to update their code to address the breaking change.

To your last example in particular

foo(int n) {}
// to
foo(num n) {}

if that original parameter was an enum type

Enums don't have a superclass type, so I don't really see how an inheritance issue could arise when dealing with enums. With enum-like classes, maybe, but if you have a function that takes an enum-like value of a specific type, changing the type to a wider superclass type seems like it would be an anti-pattern anyway, and regardless would also fall into what I said earlier about implementing breaking changes resulting in errors in the static analysis of your code being a good thing.

@mraleph
Copy link
Member

mraleph commented Sep 10, 2020

Unfortunately, I don't think it's entirely simple to implement. At least two challenges are I know are:

FWIW you list design challenges, not implementation challenges. The feature as I have described it (treat .m as E.m if .m occurs in place where E is statically expected) is in fact extremely simple to implement. You just treat all occurrences of .m as a dynamic, run the whole inference and then at the very end return to .m shorthands - for each of those look at the context type E and check if E.m is assignable to E (this condition might be tightened to require E.m to be specifically static final|const E m). If it is - great, if it is not issue an error. Done. As described it's a feature on the level of complexity of double literals change that we did few years back (double x = 1 is equivalent to double x = 1.0).

I concede that there might be some design challenges here, but I don't think resolving them should be a blocker for releasing "MVP" version of this feature.

Obviously things like grammar ambiguities would need to be ironed out first: but I am not very ambitions here either, I would be totally fine shipping something that only works in parameter positions, lists and on the right hand side of comparisons - which just side steps known ambiguities.

Just that it's more complex than it seems and we'll have to put some real thought into doing it right.

Sometimes putting too much thought into things does not pay off because you are entering the area of diminishing returns (e.g. your design challenges are the great example of things which I think is not worth even thinking about in the context of this language feature) or worse you are entering analysis paralysis which prevents you from moving ahead and actually making the language more delightful to use with simple changes to it.

That's usually not a breaking change, and is a pretty minor, safe thing to do.

You break anybody doing this:

var x = foo;
x = (int n) { /* ... */ }

Does it mean we should maybe unship static tear-offs? Probably not. Same applies to the shorthand syntax being discussed here.

@lukepighetti
Copy link

lukepighetti commented Sep 10, 2020

I'm not a computer scientist but aren't the majority of these issues solved by making it only work with constructors / static fields that share return a type that matches the host class & enum values? That's my only expectation for it anyway, and none of those come through generic types to begin with. If the type is explicit, it seems like the dart tooling would be able to to know what type you're referring to.

I don't think the value of this sugar can be understated. In the context of Flutter it would offer a ton of positive developer experience.

enum FooEnum {
  foo,
  bar,
  baz
}

f(FooEnum t) {}

f(.foo) // tooling sees f(FooEnum .foo)
f(.bar) // tooling sees f(FooEnum .bar)
f(.baz) // tooling sees f(FooEnum .baz)

In the context of Flutter the missing piece that I find first is how to handle foo(Color c) and trying to do foo(.red) for Colors.red. That seems like it would be a nice feature but I'm not sure how you'd handle that quickly and cleanly. I don't think it's necessary to be honest, though.

@munificent
Copy link
Member

munificent commented Sep 10, 2020

FWIW you list design challenges, not implementation challenges.

Yes, good point. I mispoke there. :)

As described it's a feature on the level of complexity of double literals change that we did few years back

That feature has caused some problems around inference, too, though, for many of the same reasons. Any time you use the surrounding context to know what an expression means while also using the expression to infer the surrounding context, you risk circularity and ambiguity problems. If we ever try to add overloading, this will be painful.

I concede that there might be some design challenges here, but I don't think resolving them should be a blocker for releasing "MVP" version of this feature.

We have been intensely burned on Dart repeatedly by shipping minimum viable features:

  • The cascade syntax is a readability nightmare when used in nested contexts. The language team at the time dismissed this as, "Well, users shouldn't nest it." But they do, all the time, and the code is hard to read because of it. No one correctly understands the precedence and god help you if you try to combine it with a conditional operator.

  • We shipped minimal null-aware operators that were described as a "slam dunk" because of how simple and easy it was. If I recall right, the initial release completely forgot to specify what short-circuiting ??= does. The ?. specified no short-circuiting at all which made it painful and confusing to use in method chains. We are laboriously fixing that now with NNBD and we had to bundle that change into NNBD because it's breaking and needs an explicit migration.

  • The generalized tear-off syntax was basically dead-on-arrival and ended up getting removed.

  • Likewise, the "minimal" type promotion rules initially added to the language didn't cover many common patterns and we are again fixing that with NNBD (even though most of it is not actually related to NNBD) because doing otherwise is a breaking change.

  • The crude syntax-driven exhaustiveness checking for switch statements was maybe sufficient when we were happy with any function possibly silently returning null if it ran past the end without a user realizing but had to be fixed for NNBD.

  • The somewhat-arbitrary set of expressions that are allowed in const is a constant friction point and every couple of releases we end up adding a few more cherry-picked operations to be used there because there is no coherent principle controlling what is and is not allowed in a const expression.

  • The completely arbitrary restriction preventing a method from having both optional positional and optional named parameters causes real pain to users trying to evolve APIs in non-breaking ways.

  • The deliberate simplifications to the original optional type system—mainly covariant everything, no generic methods, and implicit downcasts—were the wrong choice (though made for arguably good reasons at the time) and had to be fixed with an agonizing migration in Dart 2.

I get what you're saying. I'm not arguing that the language team needs to go meditate on a mountain for ten years before we add a single production to the grammar. But I'm pretty certain we have historically been calibrated to underthink language designs to our detriment.

I'm not proposing that we ship a complex feature, I'm suggesting that we think deeply so that we can ship a good simple feature. There are good complex features (null safety) and bad simple ones (non-shorting ?.). Thinking less may by necessity give you a simple feature, but there's no guarantee it will give you a good one.

It's entirely OK if we think through something and decide "We're OK with the feature simply not supporting this case." That's fine. What I want to avoid is shipping it and then realizing "Oh shit, we didn't think about that interaction at all." which has historically happened more than I would like.

You break anybody doing this:

var x = foo;
x = (int n) { /* ... */ }

Does it mean we should maybe unship static tear-offs?

That's why I said "usually". :) I don't think we should unship that, no. But it does factor into the trade-offs of static tear-offs and it is something API maintainers have to think about. The only reason we have been able to change the signature of constructors in the core libraries, which we have done, is because constructors currently can't be torn off.

@lrhn
Copy link
Member

lrhn commented Oct 22, 2024

About constructors, the design goal is to allow you to call a constructor to produce something of the context type.
Allowing chains after that is bonus, but if it doesn't work because you need to start with something of a subtype, write that subtype.

I see no problem with someone adding a num.fromEnvironment that forwards to int.fromEnvironment, if they want that.
It's not an argument that if num.fromEnvironment had existed from the start, then it would be able to handle doubles.
It didn't exist from the start.
Maybe that's because it doesn't really fit on num, and we should question whoever wants to add the forwarder.
If it's just make it easier to write const num x = .fromEnvironment('...');, I'd probablyt question whether it's worth it (int is short!), but it's not my job to question how people use language features. Every feature can be used for good, or in a way that makes things harder to understand. If the good uses are an improvement, the feature works.
If it encourages bad uses, because that makes something possible that you can't otherwise do, even if it's convoluted and hard to read, maybe we need to add a simpler way to do the same thing.

If a context expects a Foo, then adding an extension factory constructor on Foo which returns a value of type Foo is a reasonable design. If it returns a member of a subclass, then it still returns a Foo. It shouldn't matter which subclass it is, because the context type is Foo, so that's all that matters.

The shorthand is just a shorthand. It doesn't allow you to do anything you couldn't do without it. You can always write out the full name of the context.
That means that the point here is usability, which means that it should make the common cases easy, and if some other cases become easy too, that's great. It shouldn't try to optimize for something that you would never write to begin with, if you were designing for a language with that feature already in it.
If AlignmentGeometry does not have any factory constructors, then it's because there is no default implementation of AlignmentGeometry. You need to choose, which means you need to write the name of the subclass that you want to construct.
Or, if AlignmentGeometry does have a default implementation class, one that you should always use unless you have compelling reasons for something else, there should be no issue adding factory constructors for that default class to AlignmentGeometry.
(For AlignmentGeometry.center, which should behave exactly the same no matter whether it's LTR or RTL, I don't see why AlignmentGeometry doesn't have the constructor. Other than AlignmentGeometry being a long word, so everybody uses Alignment.center anyway.)

A superclass doesn't have to use the unnamed constructor for subclasses to extend it. It can have a

/// Used by subclasses extending this class.
const SuperClass.extend();

constructor that the subclasses can use.
The unnamed constructor is prime real-estate. You have to design around it. If you want your abstract class to be easily constructable by users, by having an unnamed factory constructor, you may have to use a different constructor as superclass constructor in subclasses. You can do that. It's a trade-off of who you want to make things easier for.

That may be a problem for APIs that already exist today, but I don't think we should let that guide the design.
Let the design be guided by which code would be written against it in the future.
And I don't see future APIs, in a language with static shorthands and static extensions (and a class being able to have a static and instance member with the same base name), needing a second namespace to look up the static shorthands.
If the constructor should be on the supertype, put it there. If not, do not.
If anything, it feels like an overcomplication.

static extension on AlignmentGeometry default $=Alignment{}

Early Dart actually had something like that:

class C default D {}

which would allow D to declare constructors for C. I don't remember if Dart still had interface declarations back then, so it was actually interface C default D {}.
In any case, that design was scrapped and replaced by redirecting factory constructors, so it becomes

class C {
  factory C() = D;
}

That allows the dependency to go from the type you actually write, C(), to the constructor it invokes. It makes it possible to forward to constructors of different classes. It's generally just more flexible and easier to reason about.

That's why I think adding (extension) factory constructors to the base class is better than magically looking up constructors in some sub-class.

None of these problems exist with two namespaces.

Of course they do ... when you want to add the third namespace on top. I do not believe that going from one to two solves a problem in generality. It's zero, one or an infinite amount. Anything else is ad-hoc 😁.

@tatumizer
Copy link

tatumizer commented Oct 22, 2024

I do not believe that going from one to two solves a problem in generality. It's zero, one or an infinite amount. Anything else is ad-hoc

It depends on how statistically significant your hoc is :-)
Syntax optimizations for simple cases (related to 0 or 1 of something) are very common. Some examples:

  • In dart, single-expression function body can be written with =>, whereas in general, you need {..} plus refactoring (adding return statements).

  • Switch expression is optimized for a single-expression body to an extent that anything else requires either IIFE or another type of switch or some other inconvenience

  • Same is true in collection literals.

  • In kotlin, 1-parameter lambdas can be written with an implicit parameter it.

I'm sure you can find many more examples. (My suggestion of $.id corresponds to n=1 - see below)

Speaking of generality, the design that injects everything from subclasses to a base class is "general" only on the surface.
If someone creates a third subclass (in some other library), does this library need to provide yet another extension on the base class? If yes, it creates a Tower of Babel in the namespace. If no, it leads to inconsistency.

And what happens if B extends A, C extends B? Are you supposed to create an extension that injects the stuff from B and C into A, and another - from C into B?

At some point, you will have to invoke statistical considerations anyway. Either this, or you need to develop a theory explaining what has to be injected to what, and what has to be omitted. The approach you are advocating for doesn't (yet?) provide this theory. In static extension, methods get renamed based on unspecified naming conventions or on arbitrary (= really ad-hoc) decisions. Basically, creating the extensions as currently proposed involves taking tweezers, holding each method up to the light, and deciding what to do with it. 😄 / 😱

The choice of a single default namespace is deliberate. It has the same justification as the above examples of "optimizations for n=1". The word "default" captures this meaning (there can be at most one default).

@tatumizer
Copy link

tatumizer commented Oct 23, 2024

General theory question:
suppose we had the constants defined:
in class int: static const zero = 0;
in class double: static const zero = 0.0;
in class num: no zero
Consider a program

void f(int x)=>print(x);
f(.zero);

Now let's loosen the type of parameter: void f(num x);
Q: Is this change supposed to be backward-compatible?

@lrhn
Copy link
Member

lrhn commented Oct 24, 2024

The feature will not be compatible with changes to expected types.
That is a little worrisome, but fundamentally unavoidable.
When you omit information and rely on context to reintroduce it, you become dependent on that context not changing.

Your example shows how it's not something that can be easily fixed.
Even if the feature checks subtypes for candidates, it would find two equally valid ones here, with no non-arbitrary way to choose (and therefore any choice would have a 50% chance of silently changing the program, which is arguably worse than not compiling).

You can break code today by changing a static num foo() to static int foo(), in case someone does var x = foo(); … ; x = 3.14;.

Inference based on someone else's API is inherently optimistic. And the vast majority of time, it pays off.

(Makes me wonder if there could be a lint to check whether your code depends on inferred types in a way that will break for supposedly non-breaking API changes.)

@tatumizer
Copy link

tatumizer commented Oct 24, 2024

(Unrelated to the previous question).
Some classes in Flutter have a large number of subclasses. E.g. StatelessWidget has almost 150 subclasses in Flutter proper, and unknown number in the wild. Each subclass comes with its own constructors, factories and what-not.
Herding all this variety into the namespace of StetelessWidget is out of the question.
The "single namesapce" theory (let's call it "Herding theory", or for brevity just "H-theory") doesn't provide an answer as to why in some cases, herding is applied, and in others it is not.
In constrast, in $-theory we have a simple explanation for the absence of $-shortcuts: "there's no sensible default".

@lrhn
Copy link
Member

lrhn commented Oct 28, 2024

My take is that if an abstract superclass has a default subclass, then it makes sense for the superclass to have redirecting factory constructors to that subclass, like Map forwards to LinkedHashMap and Queue to ListQueue (used to be to LinkedListQueue before ListQueue was added, and it made sense to make the change).

That's a design principle that applies with or without the shorthand feature.

If a class does not have a default subclass, you just have to say which subclass you want.
With or without the shorthand feature.

I don't expect StatelessWidget (or Widget) to have a default implementation.

I don't see a need for a $ shorthand that can provide a particular subclass or namespace for another class. If it's the default for the superclass, put the features on the superclass. If it's not, don't.
And if we have static extensions, anyone can decide their own defaults for superclasses that have none.

@tatumizer
Copy link

tatumizer commented Oct 28, 2024

The interface of LinkedHashMap is identical to that of Map, which, in theory, would make the "abstract factory" pattern applicable. In theory. But in practice, LinkedHashMap has 7 factory constructors, with names clashing with those from Map, and it's difficult to imagine anyone being excited about copying those, one by one, with their names mangled, into the namespace of Map,

A more realistic idea would be to define an alias for the entire class name LinkedHashMap, e.g.

static extension on Map {
   alias Linked = LinkedHashMap;
}
Map map = .Linked.of(...);

This won't be especially helpful in Flutter because it's difficult to come up with a short alias for *Geometry classes, hence the idea of $. If aliases are supported as a general feature, the definition may look like

static extension on AlignmentGeometry {
  alias A = Alignment; // can't find a good and short name, sorry
}
AlignmentGeometry a = .A(...); // constructor call, returns Alignment object - no upcasting!
a = .A.topRight; // this kinda sucks - compare with $.topRight, which sucks less IMO

WDYT?

@TekExplorer
Copy link

TekExplorer commented Oct 28, 2024

We already "have" that - it's called typedefs.

That said, static typedefs would be very nice, and would allow for "nested" (namespaces) classes and would indeed synergize with this.

That example I think is better suited to instead specifying each option directly. .topRight is strictly better.

If that's tedious, make/use a macro.

@tatumizer
Copy link

Typedefs looks like a viable option, but by itself, it won't help.
Consider

static extension on AlignmentGeometry {
  typedef Absolute = Alignment;
  typedef Directional = AlignmentDirectional;
}
AlignmentGeometry a = .Absolute.topRight; 

How is .Absolute.topRight better than Alignment.topRight ?

.topRight is strictly better.

What do you mean by "better"? It's shorter, but to be able to say it's better, you have to come up with a theory that explains how this constant makes its way into AlignmentGeometry namespace.
We can try

static extension on AlignmentGeometry {
  typedef Absolute = Alignment default;
  typedef Directional = AlignmentDirectional;
}
AlignmentGeometry a = $.topRight; 

where default enables a shorter syntax, but what syntax? That's what led me to $.topRight, but this is not the only option. We can experiment with different symbols: \ # % ^ & * + = : | < > /, with or without the dot. like

AlignmentGeometry a = #.topRight; 
AlignmentGeometry a = `.topRight; 
AlignmentGeometry a = `topRight; // no dot
//etc. 

@lrhn
Copy link
Member

lrhn commented Oct 28, 2024

I'm not advocating that every static declaration on a subclass should necessarily be available on the superclass.
Only those that make sense.

you have to come up with a theory that explains how this constant makes its way into AlignmentGeometry namespace.

Someone put it there. That's the answer every time.
If you don't want to put every constructor of Alignment into AlignmentGeometry, then don't. If it makes sense to have it there, then do.

@tatumizer
Copy link

tatumizer commented Oct 28, 2024

I don't agree with your characterization of the proposal of copying/pasting/renaming stuff in bulk as a "theory". At least not a "good theory". I tried to list some arguments explaining why it was not a good theory every time it resurfaced, but the only important factor is whether Flutter people will like it or not, and I can bet a proverbial farm on them not liking it passionately enough. :-)

@TekExplorer
Copy link

Keep in mind, this feature making it in will have package maintainers move to improve their apis with this in mind.
Plus, static extensions will help as well, as accepted patterns will likely end up with static extensions on the context type eventually.

for AlignmentGeomentry I expect for all Alignment options to be basically grafted onto AlignmentGeomentry for the convenience. Future useful constructors will make their way into that static interface, if it makes sense to.

@tatumizer
Copy link

Keep in mind, this feature making it in will have package maintainers move to improve their apis with this in mind.

You have to convince the maintainers to do the following:

  1. Mangle the names - because of name collisions, or because the original name does not make sense in another namespace. There's always at least one collision (default constructor). What to do about it? (In fact, the number of name collisions in general is unknown, and there's no theory saying how to rename them)

  2. Fake the return type. Now not only some grafted constructors will have different names - they will also have different return types.

  3. The proposal promises to support chaining. How can you explain to users why chaining is not supported for "grafted" methods? There's nothing in the dot syntax to suggest the method was grafted. This will look like a bug (because it is!)

  4. Clutter the code with massive copy-pasted material

  5. Realize that after all manipulations with names/types, your entire API changes - so you have to explain how to convert all your tutorials to the language of shortcuts. (This is not a mechanical procedure).

  6. And for what? To enable the code like this?

Look inside
Container(
      decoration: const .box(
        border: .border(
          top: .new(color: .new(0xFFFFFFFF)),
          left: .new(color: .new(0xFFFFFFFF)),
          right: .new(),
          bottom: .new(),
        ),
      ),
      child: Container(
        padding: const EdgeInsets.symmetric(horizontal: 20.0, vertical: 2.0),
        decoration: const .box(
          border: .border(
            top: .new(color: .new(0xFFDFDFDF)),
            left: .new(color: .new(0xFFDFDFDF)),
            right: .new(color: .new(0xFF7F7F7F)),
            bottom: .new(color: .new(0xFF7F7F7F)),
          ),
          color: .new(0xFFBFBFBF),
        ),
        child: const Text('OK',
            textAlign: .center,
            style: .new(color: .new(0xFF000000))),
      ),
    );
  }

Please be honest: do you like this code?

@TekExplorer
Copy link

TekExplorer commented Nov 2, 2024

If mangling the name results in a bad API, then it's probably not a good default and you shouldn't use it that way anyway. Generally a "default constructor" conflict doesn't exist as the type names are different and can provide a reasonable name.

There is also nothing stopping you from namespacing things with records or middleman objects, riverpod style.

There is no problem with having the type of a getter or static method as a subtype, and I don't know what you're talking about regarding chaining and grafted members.

If you're copy pasting a lot, you're doing something wrong. It's just a single getter, static field, or factory/static method for most cases

A change to the API would be purely additive. And of course you need to take that into account for docs, it's an improvement on how defaults are accesses.

That code is mediocre because it abuses the feature. There is nothing stating we couldn't have an annotation + lint that says "don't dot-notate this member", or a lint that's specifically about .new

@tatumizer
Copy link

I don't know what you're talking about regarding chaining and grafted members.

Suppose you have grafted EdgeInset constructor all into EdgeInsetGeometry. Suppose also in EdgeInset, you have a method copyWith(...}.
Then

EdgeInsetGeometry g = EdgeInset.all(10).copyWith(right: 15); // works with full name
EdgeInsetGeometry g1 = .all(10).copyWith(right: 15); // error! 

The reason for an error is that the "grafted" constructor .all returns an object of type EdgeInsetGeometry, for which copyWith is not defined.

@Levi-Lesches
Copy link

Levi-Lesches commented Nov 3, 2024

I may have said it before, but with all the long conversations in this thread, I believe there is no one clear rule that is obvious and intuitive for everyone. Maybe this would be better suited as an IDE auto-complete suggestion that transforms eg .center --> Alignment.center by offering suggestions filtered by the context type and not an actual language feature that affects how code is read and written. It would ultimately end up with the developer experience most people want -- quick to write and simple to understand -- but I'm not sure some of the proposals here would accomplish both (again, as evidenced by many disagreements over what's natural or obvious)

@TekExplorer
Copy link

The reason for an error is that the "grafted" constructor .all returns an object of type EdgeInsetGeometry, for which copyWith is not defined.

Not necessarily. The static member can be a subtype, which we can act on, so long as the type we ended up with is still valid. How we get there doesn't matter.

After all, .all is not a constructor, it's a static getter/member.

@tatumizer
Copy link

.all is a constructor in EdgeInset. You have two choices while grafting it into EdgeInsetGeometry namespace:

  • translate it to a factory constructor. Then the return type will be EdgeInsetGeometry, and you lose the ability to call copyWith
  • translate it to a static method. Then you lose constness.

@jodinathan
Copy link

I may have said it before, but with all the long conversations in this thread, I believe there is no one clear rule that is obvious and intuitive for everyone. Maybe this would be better suited as an IDE auto-complete suggestion that transforms eg .center --> Alignment.center by offering suggestions filtered by the context type and not an actual language feature that affects how code is read and written. It would ultimately end up with the developer experience most people want -- quick to write and simple to understand -- but I'm not sure some of the proposals here would accomplish both (again, as evidenced by many disagreements over what's natural or obvious)

I disagree because this solves a lot of problems with source generated stuff.

@TekExplorer
Copy link

.all is a constructor in EdgeInset. You have two choices while grafting it into EdgeInsetGeometry namespace:

  • translate it to a factory constructor. Then the return type will be EdgeInsetGeometry, and you lose the ability to call copyWith
  • translate it to a static method. Then you lose constness.

Const is not exactly going to help with your copyWith.

Pick which is a better default. A factory for const, a static method for subtype context, or, if you don't need arguments, let it be a constant static variable.

This issue does not replace the normal way of doing things, it's only a useful shorthand for most usecases. If you need more, go back to the specific one.

@eernstg
Copy link
Member

eernstg commented Nov 5, 2024

@tatumizer wrote:

.all is a constructor in EdgeInset. You have two choices while grafting it into EdgeInsetGeometry namespace:

  • translate it to a factory constructor. Then the return type will be EdgeInsetGeometry, and you lose the ability to call copyWith
  • translate it to a static method. Then you lose constness.

It wouldn't be difficult to preserve both: #4153.

@eernstg eernstg added the brevity A feature whose purpose is to enable concise syntax, typically expressible already in a longer form label Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brevity A feature whose purpose is to enable concise syntax, typically expressible already in a longer form enum-shorthands Issues related to the enum shorthands feature. enums feature Proposed language feature that solves one or more problems
Projects
Status: Being spec'ed
Development

No branches or pull requests