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

Using constructor and function parameter lists as implicit parameter groups #2234

Open
apps-transround opened this issue May 12, 2022 · 13 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@apps-transround
Copy link

apps-transround commented May 12, 2022

Creating constructors may require a lot of repeated code when using super, named and redirecting constructors as described here.
Although super parameters made a big impact, there is still room for improvement.

The proposed change is

  • syntactic sugar to allow referring other constructors in a constructor parameter list
  • the parameters of the referred constructors are automatically expanded for the callers
  • IDE wizards and autofill should support the parameter expansion
  • working for this and super constructors
  • named constructors too
  • a more generic version of @Hixie ’s …super approach
  • a potential answer for @yjbanov 's request on factory constructors

Example:
Current code:

class OutlinedButton extends ButtonStyleButton {
  const OutlinedButton({
    Key? key,
    required VoidCallback? onPressed,
    VoidCallback? onLongPress,
    ValueChanged<bool>? onHover,
    ValueChanged<bool>? onFocusChange,
    ButtonStyle? style,
    FocusNode? focusNode,
    bool autofocus = false,
    Clip clipBehavior = Clip.none,
    required Widget child,
  }) : super(
    key: key,
    onPressed: onPressed,
    onLongPress: onLongPress,
    onHover: onHover,
    onFocusChange: onFocusChange,
    style: style,
    focusNode: focusNode,
    autofocus: autofocus,
    clipBehavior: clipBehavior,
    child: child,
  );
}`

Code with the upcoming super parameters:

class OutlinedButton extends ButtonStyleButton {
  const OutlinedButton({
    super.key,
    required super.onPressed,
    super.onLongPress,
    super.onHover,
    super.onFocusChange,
    super.style,
    super.focusNode,
    super.autofocus = false,
    super.clipBehavior = Clip.none,
    required Widget super.child,
  });
}

Code with the proposed feature:

class OutlinedButtonProposed extends ButtonStyleButton {
  const OutlinedButton({
    ...super.ButtonStyleButton,
  });
}

Code with the proposed feature on this constructor:

  OutlinedButton.red({
    ...this.OutlinedButton,
    super.style = ButtonStyle(backgroundColor: Colors.red)
  });

Advantages

  • Shorter code.
  • No change on the caller side
  • Only the specific parts are needed in the constructors.
  • Easy to see and understand overrides/differences.
  • Transparent changes, no need to modify all passing constructors

Explicit and implicit parameter conflicts are resolved with the following priority policy:

  1. explicitly defined
  2. implicit this
  3. implicit super

Notation: adding a keyword or a separator may improve clarity. Samples:

  • ... this/super . constructor/namedConstructor
  • this/super . constructor/namedConstructor . params
  • { this/super . constructor/namedConstructor, }

I agree with @eernstg that positional arguments can be very tricky, further evaluation is needed whether to support them.

A similar feature is to use the parameter list of a function as a parameter group with the new notation. For example:

String foo({String left, String right, int count}){ … }
String foo10({...foo.params, count = 10}){ … }

Invocation is not changing:

String a = foo10(left:ABCD’, right:EFGH’)
@apps-transround apps-transround added the feature Proposed language feature that solves one or more problems label May 12, 2022
@eernstg
Copy link
Member

eernstg commented May 16, 2022

It looks like the proposal is to make a formal parameter super.C (in general: 'super' '.' <typeIdentifier>) an abbreviation for "every possible super-parameter for the denoted super-constructor C, where C denotes the superclass—except that it does not include a super-parameter if it would conflict with the explicitly declared formal parameters, or if an explicit actual argument is provided for the associated super-constructor parameter in an explicitly specified superinitializer.

In other words, we'd be able to ask for "the rest" of the possible super-parameters using a single term, and if we want to customize something (say, the default value of a specific super-parameter) then we'd just go ahead and write it. That might certainly be a useful abbreviation feature on top of the new super-parameters feature!

A couple of things should be considered: I'm guessing (based on the examples) that we'd only have super.C in the class C, not super.C.named, and that the source of the "rest of the super-parameters" would be the superconstructor which is actually invoked by the current constructor (because otherwise we can't expect the "rest of" to make sense, and we can't expect them to be correct for that actual superconstructor). So why wouldn't we just return to a simpler (and arguably more readable/visible) syntax like a plain ...super? It looks like that C just makes the construct more verbose, and harder to spot at a glance.

Another thing to think about is maintainability: Suppose we'd have the "rest of supers" feature as an IDE feature rather than a language construct. We'd ask for "the rest of the super-parameters" in the IDE (using something like a quick fix), and then the appropriate source code would be generated. So we'd have all the super-parameters written out, same as today.

Is it more maintainable as a language construct (where we'd have something like ...super in the constructor declaration, and the breaking changes are simply propagating invisibly into the constructor that uses ...super, and potentially into a bunch of subclasses)? Or is it more maintainable to generate the super-parameters, and have them written out explicitly?

The underlying conflict is between two principles: (1) "Declare your interfaces explicitly", and (2) "Use abstraction to obtain more concise and consistent code". In the particular case where we're using abstraction to specify the interface, we may be hit by problems if said interface is too unstable, because it propagates changes made by others, elsewhere. Of course, super-parameters do that already, but this would mean that we do even more of it.

@apps-transround
Copy link
Author

The examples do not contain named constructors but the proposal strongly supports them. See Notation

... this/super . constructor/namedConstructor

I think explicitly pointing to the constructor helps to avoid confusion.
This proposal relates to the generic Parameter Group implementation proposal and that may extend the scope far beyond super and this.

@apps-transround
Copy link
Author

This proposal is rather about the language construct than the IDE feature.

Named constructor support is available if super parameters implement :super.named or :super.named() as agreed here earlier.

Regarding stability and change propagation: while creating this proposal

  • I had OOP in mind
  • not for classes but for parameter lists
  • with a simplified syntax
  • using an existing parameter list
  • extend it with new parameters
  • overwrite already defined parameters by specifying new values.

It’s something new but – I hope – it is similar enough to a proven and widely used methodology. I also hope that it really improves developer experience.

@lrhn
Copy link
Member

lrhn commented May 19, 2022

This is something we did think about designing super-parameters, but decided to at least punt on.
I believe the proposed syntax at the time was just super,
so:

class Bar {
 Bar.bar(T1 bar1, T2 bar2, {T3 barNamed});
}

class Foo extends Bar {
 Foo.bar(int x, super) : super.bar();
}

would be equivalent to

 Foo.bar(int x, super.bar1, super.bar2, {super.barNamed}) : super.bar();

aka

 Foo.bar(int x, T1 bar1, T2 bar2, {T3 barNamed}) : super.bar(bar1, bar2, barNamed: barNamed);

One of the problems with that design is that you insert "all of the remaining" of the positional parameters at the super position.

What if you have;

class A {
  A(int a, int b, int c);
}
class B extends A {
  B(super, super.a);
}

Will that recognize the later a and only expand to super.b, super.c?

What if some of the super-parameters are optional positional, is that inherited? What if it can't be?
Like

 Baz(int x, super, int y);

If the super constructor has optional positional parameters, they must become required in the subclass. That's fair, but potentially surprising, and something you may want to write explicitly.

Even if y had been optional, [int? y], in the above, if the super constructor adds one more optional parameter (which used to be non-breaking), it now pushes the position of a the y parameter in the subclass by one position. That is breaking.

So, if anything, the "rest" super parameter should only be allowed to go "last" in the positional parameters.
And only if the subclass does not have optional positional parameters.
And it's a compile-time error to add named parameters if you inherit optional positional parameters that way.

All in all, inheriting optional positional parameters implicitly is a big mess.
By not having a shortcut, you have to write out each one explicitly, chose its position and you can make it required if you want to.

@eernstg
Copy link
Member

eernstg commented May 19, 2022

As usual ;-), we could consider a less powerful (and arguably less confusing) mechanism where super will only introduce named parameters, and only the ones that are accepted by the given superconstructor and aren't already passed as actual arguments to the superinitializer, including the ones that are passed implicitly because of a super-parameter.

class Bar {
 Bar(T1 bar1, {T2 bar2 = cBar2, T3? bar3, required T4 bar4});
}

class Foo extends Bar {
  Foo(super.bar1, super); // Like Foo(super.bar1, {super.bar2, super.bar3, required super.bar4}).
  Foo.name1(super.bar1, {required super.bar4, super}): super(bar3: null); // `super` --> `super.bar2`.
}

This means that super can appear as the last positional parameter if there are no named parameters (syntactically, the associated superconstructor does have some, or super would be a compile-time error), and if there are any named parameters then super must be the last element in {}.

@apps-transround
Copy link
Author

In my original proposal for super parameters, I wanted to have positional arguments but @eernstg 's comment convinced to eliminate them. The same happens now, I agree to have named argument only.

Regarding the notation: …super is more self-explanatory for me than super

Although the biggest win is simplifying super calls, please note that the current proposal also includes this.constructors and there is a related but more generic proposal here.

@lrhn
Copy link
Member

lrhn commented May 19, 2022

Ignoring positional arguments is always a hard sell to me, because I very rarely use named parameters. They're just too cumbersome to be worth it in almost all cases. I don't think I've ever written a required named parameter outside of language tests.
So, a feature which only works for named parameters is just completely useless to me. It feels like half a feature.
To sell it, I'd need to see numbers that indicate that a large number of our users will benefit, and won't also be annoyed that it only works for named parameters.

@jodinathan
Copy link

wouldn't it work if used as last argument and when there isn't an optional positional param?

We hardly use/see constructors with optional params.

In fact, we don't use optional positional params at all.

@Hixie
Copy link

Hixie commented May 20, 2022

I'd need to see numbers that indicate that a large number of our users will benefit, and won't also be annoyed that it only works for named parameters.

It would be good to collect data on this (@munificent has done this kind of thing before). Anecdotally, widgets in Flutter make heavy heavy use of named parameters, FWIW.

@apps-transround
Copy link
Author

It seems like there are two options:

  1. Limit the feature scope to named parameters only: clean and easy to understand concept but may lose developer support.
  2. Keep the feature wide but try to avoid the mess.

Keeping positional parameters may be done by understanding the differences between named and positional parameters and apply the learning on this feature.
In the wide (named + positional) approach, named parameters are easy:

  • They "live a different life" from positional parameters without real interference. They do not influence positional parameters neither in the expanded nor in the original parameter list now that they can be everywhere.
  • Repeating items can be found by name and conflict resolution will work

So let's focus on positional parameters only. How this feature should work on them:

  • The super parameter list is expanded exactly where the …super is placed
  • Then the super parameter feature is applied on the whole list

It's obvious that positional arguments cannot be identified by name, only by position: a parameter in another position is another parameter. So @lrhn example should result in a 'too many positional arguments' compile time error

class A {
  A(int a, int b, int c);
}
class B extends A {
  B(super, super.a);
}

For optional positional parameters it is said they can only be declared after any required parameters. So if super had any optional positional this should also result in error:

 Baz(int x, super, int y);

Lint rules for these cases may really improve developer experience.

Summary: it seems like the existing language syntax is enough to guide us in this case, too. Although careful usage is required for positional arguments we don't have to exclude them by default.

@eernstg
Copy link
Member

eernstg commented May 23, 2022

Perhaps we could specify that ...super (I still prefer that syntax over a plain super) can only be placed as the last positional parameter, or as a named parameter. As a named parameter it would give rise to generation of "the rest" of the named super-parameters as previously proposed. As the last positional parameter it would give rise to generation of positional super-parameters, based on the positional parameters (required or optional) of the targeted superconstructor from the position of ...super in the current parameter declaration list. If you want both, you would have to write it in both locations.

A variant of this proposal would be to allow one or the other placement (not both), but a ...super that occurs as the last positional parameter would specify that both positional and named super-parameters should be generated. This implies that we can't use the abbreviation if we don't want the named super-parameters, but it's a bit more concise when we do want them.

@apps-transround
Copy link
Author

apps-transround commented May 27, 2022

The theoretic question is that how the record spread operator should work inside another record. But Dart does not have Records at the moment and record spread is not even mentioned in the design docs.

By checking other languages such as JavaScript and TypeScript, spread on objects is similar to spread on named arguments and spread on arrays is similar to spread on positional parameters and the behaviors (add, modify items) are in line with my previous comment.

The problem is that although JS and TS support spread on both objects and arrays; but distinctly, not in the same operation. If we plan to support positional and named parameters at the same time, we have to define the behavior. What I proposed and still find valid:

  • …super is a positional parameter
  • Spread is executed where the …super is positioned but the positional and named items are separated and the operation is done on both according to the JS/TS way.
  • The resulting two lists are joined together: positional arguments first, named arguments second.
  • If the result is not meeting the rules, then compile time errors and lint warnings are shown. For example, too many positional arguments, optional positional is not the last positional, unknown named super argument.
  • Otherwise, the Super Parameter feature is applied.

@Laszlo11-dev
Copy link

Laszlo11-dev commented Jun 11, 2024

Dart macros may offer a straightforward solution for this proposal without further language support.
I have created a non-functional preview package. I copy here the package readme and looking forward to learn others' opinion.

https://pub.dev/packages/parameters

parameters macro

Experimental implementation of the Dart Parameter Group proposal Using constructor and function parameter lists as implicit parameter groups #2234 (https://github.com/dart-lang/language/issues/2234/) via Dart static meta programming (macro).

Please note:

  • This is an experimental implementation of an experimental language feature using an experimental implementation of an experimental language feature. What could go wrong...

  • This package is not working and only intended for the verification of the parameter group proposal.

  • Due to its limited and temporary state, the license is also limited. Later this may change.

Goal

In Object Oriented languages, classes and functional parts have good tooling (extends, implements, override, super, mustCallSuper, to name a few).
But constructor and method parameter lists had nothing. The result was long, and copy-pasted code parts.

This macro changes the way we handle parameters: All constructor, method and function definitions are implicit parameter groups.
Any of them within the scope can be added to an other constructor, method and function definition via macro annotation.
The macro

  • adds the source parameters to the destination parameter list.
  • clears conflicting parameter items by repeatedly overriding items with the same name parameters to the right.
  • handles this and super
  • and default values

Example 1: the Flutter FloatingActionButton

In the floating_action_button.dart file the actual class has total 558 lines, including 201 comment line, so 357 real lines.
The default constructor parameter list is 23 lines + it has 3 additional constructors:

  • FloatingActionButton.small: 21 lines

  • FloatingActionButton.large 21

  • FloatingActionButton.extended 26

Hard to follow, easy to miss.

const FloatingActionButton({
    super.key,
    this.child,
    this.tooltip,
    this.foregroundColor,
    this.backgroundColor,
    this.focusColor,
    this.hoverColor,
    this.splashColor,
    this.heroTag = const _DefaultHeroTag(),
    this.elevation,
    this.focusElevation,
    this.hoverElevation,
    this.highlightElevation,
    this.disabledElevation,
    required this.onPressed,
    this.mouseCursor,
    this.mini = false,
    this.shape,
    this.clipBehavior = Clip.none,
    this.focusNode,
    this.autofocus = false,
    this.materialTapTargetSize,
    this.isExtended = false,
    this.enableFeedback,
}) : //asserts removed for clarity. 
    _floatingActionButtonType = mini ? _FloatingActionButtonType.small : _FloatingActionButtonType.regular,
    _extendedLabel = null,
    extendedIconLabelSpacing = null,
    extendedPadding = null,
    extendedTextStyle = null;

const FloatingActionButton.small({
        super.key,
        this.child,
        this.tooltip,
        this.foregroundColor,
        this.backgroundColor,
        this.focusColor,
        this.hoverColor,
        this.splashColor,
        this.heroTag = const _DefaultHeroTag(),
        this.elevation,
        this.focusElevation,
        this.hoverElevation,
        this.highlightElevation,
        this.disabledElevation,
        required this.onPressed,
        this.mouseCursor,
        this.shape,
        this.clipBehavior = Clip.none,
        this.focusNode,
        this.autofocus = false,
        this.materialTapTargetSize,
        this.enableFeedback,
}) : // asserts removed
    _floatingActionButtonType = _FloatingActionButtonType.small,
    mini = true,
    isExtended = false,
    _extendedLabel = null,
    extendedIconLabelSpacing = null,
    extendedPadding = null,
    extendedTextStyle = null;
 
// two more similar...

With this macro package we can rewrite the constructors: instead of repeating the whole parameter list, we ask the macro to add all parameters of the FloatingActionButton constructor.

  @ParamFrom('FloatingActionButton.')
  OptimizedFloatingActionButton.small({
        this.autofocus = false,
        this.clipBehavior = Clip.none,
  
  }) 

The @ParamFrom('FloatingActionButton.') copies all the parameters from the default constructor and overrides the two parameters with the default values*.

We haven't lost anything. The augmented code has all the parameters:

// Generated code
augment class OptimizedFloatingActionButton {
  augment  OptimizedFloatingActionButton.small({
    prefix0.Key? key,
    prefix1.Widget? child,
    prefix2.String? tooltip,
    prefix3.Color? foregroundColor,
    prefix3.Color? backgroundColor,
    prefix3.Color? focusColor,
    prefix3.Color? hoverColor,
    prefix3.Color? splashColor,
    prefix2.Object? heroTag,
    prefix2.double? elevation,
    prefix2.double? focusElevation,
    prefix2.double? hoverElevation,
    prefix2.double? highlightElevation,
    prefix2.double? disabledElevation,
    required void Function()? onPressed,
    prefix4.MouseCursor? mouseCursor,
    prefix2.bool mini,
    prefix5.ShapeBorder? shape,
    prefix3.Clip clipBehavior,
    prefix6.FocusNode? focusNode,
    prefix2.bool autofocus,
    prefix7.MaterialTapTargetSize? materialTapTargetSize,
    prefix2.bool isExtended,
    prefix2.bool? enableFeedback,});
}

With the parameters macro we can spare 3*21 = 63 lines => 17% of the code.
If go further, the default constructor of the FloatingActionButton has 17 command fields with the RawMaterialButton constructor.
That way 80 repeated lines can be avoided, that's 22% save.

The resulting code is not only smaller but easier to understand because it clearly indicates what is the same and where are the differences.
This a Dart feature but Flutter will benefit a lot from it.

Example 2: Flutter TextFormField

It unites two worlds: FormField and TextField and you can see this on its constructor

 TextFormField({
    super.key,
    this.controller,
    String? initialValue,
    FocusNode? focusNode,
    InputDecoration? decoration = const InputDecoration(),
    TextInputType? keyboardType,
    TextCapitalization textCapitalization = TextCapitalization.none,
    TextInputAction? textInputAction,
    TextStyle? style,
    StrutStyle? strutStyle,
    TextDirection? textDirection,
    TextAlign textAlign = TextAlign.start,
    TextAlignVertical? textAlignVertical,
    bool autofocus = false,
    bool readOnly = false,
    @Deprecated(
      'Use `contextMenuBuilder` instead. '
      'This feature was deprecated after v3.3.0-0.5.pre.',
    )
    ToolbarOptions? toolbarOptions,
    bool? showCursor,
    String obscuringCharacter = '•',
    bool obscureText = false,
    bool autocorrect = true,
    SmartDashesType? smartDashesType,
    SmartQuotesType? smartQuotesType,
    bool enableSuggestions = true,
    MaxLengthEnforcement? maxLengthEnforcement,
    int? maxLines = 1,
    int? minLines,
    bool expands = false,
    int? maxLength,
    this.onChanged,
    GestureTapCallback? onTap,
    bool onTapAlwaysCalled = false,
    TapRegionCallback? onTapOutside,
    VoidCallback? onEditingComplete,
    ValueChanged<String>? onFieldSubmitted,
    super.onSaved,
    super.validator,
    List<TextInputFormatter>? inputFormatters,
    bool? enabled,
    bool? ignorePointers,
    double cursorWidth = 2.0,
    double? cursorHeight,
    Radius? cursorRadius,
    Color? cursorColor,
    Color? cursorErrorColor,
    Brightness? keyboardAppearance,
    EdgeInsets scrollPadding = const EdgeInsets.all(20.0),
    bool? enableInteractiveSelection,
    TextSelectionControls? selectionControls,
    InputCounterWidgetBuilder? buildCounter,
    ScrollPhysics? scrollPhysics,
    Iterable<String>? autofillHints,
    AutovalidateMode? autovalidateMode,
    ScrollController? scrollController,
    super.restorationId,
    bool enableIMEPersonalizedLearning = true,
    MouseCursor? mouseCursor,
    EditableTextContextMenuBuilder? contextMenuBuilder = _defaultContextMenuBuilder,
    SpellCheckConfiguration? spellCheckConfiguration,
    TextMagnifierConfiguration? magnifierConfiguration,
    UndoHistoryController? undoController,
    AppPrivateCommandCallback? onAppPrivateCommand,
    bool? cursorOpacityAnimates,
    ui.BoxHeightStyle selectionHeightStyle = ui.BoxHeightStyle.tight,
    ui.BoxWidthStyle selectionWidthStyle = ui.BoxWidthStyle.tight,
    DragStartBehavior dragStartBehavior = DragStartBehavior.start,
    ContentInsertionConfiguration? contentInsertionConfiguration,
    MaterialStatesController? statesController,
    Clip clipBehavior = Clip.hardEdge,
    bool scribbleEnabled = true,
    bool canRequestFocus = true,
  }) 

This macro can simplify the code to something like this:

@ParamFrom('TextField.', library: ''text_field.dart'')
@ParamFrom('FormField.', library: 'package:flutter/widgets.dart')
 TextFormField()

The macro will put a doc comment link into the destination comments, pointing to the source parameter list doc comments, if any.

Motivation

The motivation with this package is to

  • explore the Macro solution for the Parameter Group proposal
  • decide weather to deploy it as a standalone package or as a part of the core Dart tools.

Issues

  • Default values are not handled by the current version of Macros. So, I had to omit default values at the moment. There is an open issue on Default values, hope it will go through soon.
  • 'this.' and 'super.' are not used, all parameters are defined with type. I think it can be done, just didn't have the time.
  • The same applies to constructor and method bodies.
  • Since resolveIdentifier is depreceated, the library look-up doesn't work.
  • I couldn't find a way to use ParamD macros, because parameters do not expose the definingType. Therefore, instead of the parameter list, I had to put the annotation before the method/contructor.

Due to these issues, the code is less attractive and not usable, but still very promising.

How to proceed

If the Macros will be a part of Dart, and the missing features will be implemented, than this macro will really make an impact on how we work in Dart/Flutter.

A decision is needed whether this remains a third-party package or will be somehow integrated into the core set.

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

No branches or pull requests

6 participants