dart-lang / language

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

Consider allowing "name-last" syntax for primary constructors #3299

Open leafpetersen opened 1 year ago

leafpetersen commented 1 year ago

One of the concerns that some people have raised around primary constructors is that with longer parameter lists, it moves the extends and implements clauses a significant distance from the start of the class. For example:

class const PasswordField({
  super.key,
  final String? restorationId,
  final Key? fieldKey,
  final String? hintText,
  final String? labelText,
  final String? helperText,
  final FormFieldSetter<String>? onSaved,
  final FormFieldValidator<String>? validator,
  final ValueChanged<String>? onFieldSubmitted,
  final FocusNode? focusNode,
  final TextInputAction? textInputAction,
}) extends StatefulWidget {
  @override
  State<PasswordField> createState() => _PasswordFieldState();
}

Here, the extends StatefulWidget clause is pushed to the bottom where it is (at least in some people's eyes) hard to find.

One thing we could consider is allowing the primary constructor to be moved towards the end notably including the class name. The above would then become:

class extends StatefulWidget
  const PasswordField({
    super.key,
    final String? restorationId,
    final Key? fieldKey,
    final String? hintText,
    final String? labelText,
    final String? helperText,
    final FormFieldSetter<String>? onSaved,
    final FormFieldValidator<String>? validator,
    final ValueChanged<String>? onFieldSubmitted,
    final FocusNode? focusNode,
    final TextInputAction? textInputAction,
  })  {
  @override
  State<PasswordField> createState() => _PasswordFieldState();
}

Possibly we require the constructor to be prefixed with either new or const to make parser recovery better.

For shorter classes with no members besides the fields (POD classes) we allow the class body to be elided as before.

class extends Point new Cartesian({int x, int y});

For generic classes, we allow the generic parameters to be placed on the class keyword in the same way that we allow generic to be attached to the extension keyword:

class<S, T> extends Pair<S, T> new ColorPair({S fst, T snd, Color color});

cc @dart-lang/language-team

eernstg commented 1 year ago

This gets pretty close to your own idea about allowing the primary constructor to be specified in the body. That is, a class with no primary constructor in the header can have zero or one constructors with a special modifier, using primary as a strawman below. The primary body constructor is a primary constructor in the sense that it can introduce instance variable declarations in the same way as a primary constructor in the class header.

// Proposed in this issue
class extends StatefulWidget
  const PasswordField({
    super.key,
    final String? restorationId,
    final Key? fieldKey,
    final String? hintText,
    final String? labelText,
    final String? helperText,
    final FormFieldSetter<String>? onSaved,
    final FormFieldValidator<String>? validator,
    final ValueChanged<String>? onFieldSubmitted,
    final FocusNode? focusNode,
    final TextInputAction? textInputAction,
  })  {
  @override
  State<PasswordField> createState() => _PasswordFieldState();
}

// Using a `primary` constructor.
class PasswordField extends StatefulWidget {
  primary const PasswordField({
    super.key,
    final String? restorationId,
    final Key? fieldKey,
    final String? hintText,
    final String? labelText,
    final String? helperText,
    final FormFieldSetter<String>? onSaved,
    final FormFieldValidator<String>? validator,
    final ValueChanged<String>? onFieldSubmitted,
    final FocusNode? focusNode,
    final TextInputAction? textInputAction,
  });

  @override
  State<PasswordField> createState() => _PasswordFieldState();
}
lrhn commented 1 year ago

The difference between a primary constructor and all other constructors, is that the primary constructor also declares fields.

We could let every construtor be able to declare fields, but that gets very messy and unreadable.

Or we can allow any single in-body constructor to declare fields, and not need the "outside-of-body constructor syntax". You have to designate it somehow, so, strawman, a primary modifier. (Which doesn't fit as well to that design.)

And we can allow you to write new or const instead of the class name, or const+class name, when declaring constructors in general. That should reduce verbosity and will likely be very popular. (Even if you can write new.new for the unnamed constructor, we would just recommend that you don't.)

Then the in-body primary constructor is just:

class PasswordField extends StatefulWidget {
  primary const({
    super.key,
    final String? restorationId,
    ...
    final FocusNode? focusNode,
    final TextInputAction? textInputAction,
  });
}

Then we allow the single primary constructor to be written ouside of the body, before the {. If you do that, it will automatically be a primary constructor, and it cannot have a body.

class PasswordField extends StatefulWidget const({
    super.key,
    final String? restorationId,
    ...
    final FocusNode? focusNode,
    final TextInputAction? textInputAction,
  }) {
  // class body
}

That's basically Bob's original trailing syntax. I prefer it over moving the class name all the way to the end, because the class name is also important for reading.

We can even allow an initializer list in the out-of-body primary constructor, just no constructor body:

class PasswordField extends StatefulWidget const({
    super.key,
    final String? restorationId,
    ...
    final FocusNode? focusNode,
    final TextInputAction? textInputAction,
  }) : focusDescription = focusNode == null ? "No focus" : "Has focus",
       super.otherConstructor("Fixed argument") {
  // class body
}

And we'll allow an empty class body to be written as ;, with no difference in meaning from {}.

It makes a simple data class go from:

class Name(final String firstName, final String lastName);

to

class Name new(final String firstName, final String lastName);

And we could possibly allow omitting the new if there is no extends, with or implements clauses. If there are such clauses, then the new/const is needed to mark where they end. That would bring us back to the one-liner class Name(...). (We could allow both syntaxes, class const Foo.name(args) extends Bar {} and class Foo extends Bar const.name(args) {}, and let users choose which one they prefer for which classes. That's probably a bad idea, taking up too much syntactic space for too little benefit, and not being opinionated. In the end, style guides will emerge which says to always use one or the other anyway, and everybody will use only half the syntax.)

About using new/const as the name for a constructor, it could mean changing the order of const factory Foo(...) to factory const(...). Or we can use factory as the marker for factory constructors. That's probably better. So:

class Foo {
  final int x;

  new(this.x);                   // Non-const, generative, unnamed
  new.new(this.x);               // Non-const, generative, unnamed
  new.id(this.x);                // Non-const, generative, named
  const(this.x);                 // Const    , generative, unnamed
  const.new(this.x);             // Const    , generative, unnamed
  const.id(this.x);              // Const    , generative, named
  factory(int x) = Foo;          // Non-const, factory   , unnamed
  factory(int x) = Foo;          // Non-const, factory   , unnamed
  factory.id(int x) = Foo;       // Non-const, factory   , named
  const factory(int x) = Foo;    // Const    , factory   , unnamed
  const factory(int x) = Foo;    // Const    , factory   , unnamed
  const factory.id(int x) = Foo; // Const    , factory   , named
}

That basically means removing the class name, except for non-const generative constructors, where that would be removing everything, so we add back new in that case.

It's only for declaration, you still have to write Foo(..) to create an instance, and Foo.new to tear it off. (Except, possibly, inside the same class, where you could write just new(..).)

eernstg commented 1 year ago

Nice ideas galore!

I've adjusted the PR in https://github.com/dart-lang/language/pull/3023 to support primary constructors in the body of the class / extension type / enum declaration, using the modifier primary when it occurs in the body. Also, it includes support for omitting required on named parameters when the type is potentially non-nullable and there is no default value (cf. https://github.com/dart-lang/language/issues/3206, https://github.com/dart-lang/language/issues/156, https://github.com/dart-lang/language/issues/878, https://github.com/dart-lang/language/issues/2050, and probably more).

class PasswordField extends StatefulWidget {
  primary const PasswordField({
    super.key,
    String? restorationId,
    Key? fieldKey,
    String? hintText,
    String? labelText,
    String? helperText,
    FormFieldSetter<String>? onSaved,
    FormFieldValidator<String>? validator,
    ValueChanged<String>? onFieldSubmitted,
    FocusNode? focusNode,
    TextInputAction? textInputAction,
  });

  @override
  State<PasswordField> createState() => _PasswordFieldState();
}
Levi-Lesches commented 1 year ago

My 2 cents: As much as I like the idea of declaring a field and its constructor parameter in the same step, I never liked its placement in the class "signature". IMO, with mixin/base/sealed/interface/final/class/extends/with/on/implements, that line (but really, declaration) has gotten way too cluttered already. Fields should stay in the body of the class so one can read the declaration of a class without getting bogged down by every public, private, late, and nullable field in the process.

So a modifier like primary or some other way to keep that constructor in the body would fit much nicer visually. Maybe it would also be valuable to disallow the inline form, at least for classes that use extends/implements/on or any other post-classname modifier. This would also solve the problem of how to call super-constructors -- just use super.field or : super(field) like a normal body constructor.