-
Notifications
You must be signed in to change notification settings - Fork 205
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
Super parameters #1855
Comments
This breaks encapsulation. A superclass maintains an abstraction that is only accessible through its interface and the generative constructor you use. That ensures that a class like class C {
final int x;
C(this.x);
} can safely be refactored to class C {
final int _x;
C(int x) : _x = x;
int get x {
_log("x access!!");
return _x;
}
} without any subclass ever noticing. If you can initialize super-class fields from subclasses then the superclass is locked into having that field. Abstraction and encapsulation is important. It's inconvenient at times, but I firmly believe that in the long run, the language is better off by being strict about who can see through which abstractions. Now, if That might be possible. Not sure it's worth the effort by itself, though. Maybe we want other kinds of parameter forwarding as well, and it could be included in a larger feature. |
This is something that's been asked for in several issues. Someone also suggested the ability to specify a certain constructor, if the superclass has multiple constructors, instead of having the compiler try to be smart.
@eernstg suggested in one of the threads that Hopefully these examples help show the logic behind it. class User {
final String username;
final String? email;
User({required this.username, this.email}); // all named parameters
User.positional(this.username, this.email); // has positional parameters
User.genEmail({required this.username}) : email = username + "@gmail.com";
}
class Person extends User {
final int age;
Person(this.age, super.username, super.email); // error, super parameters must be named
Person(this.age, {super.usermame, super.email});
// translates to
Person(this.age, {username, email}) : super(username: username, email: email); // error, username is required
Person(this.age, {required super.username, super.email, super.address});
// translates to
Person(this.age, {required username, email, address) :
super(username: username, email: email, address: address) // error, 'address' isn't defined
Person(this.age, {required super.username, super.email});
// translates to
Person(this.age, {required username, email}) : super(username: username, email: email); // ok
Person(this.age, {super.email, required super.username});
// translates to
Person(this.age, {email, required username}) : super(email: email, username: username); // ok, order doesn't matter
Person(this.age, {required super.username});
// translates to
Person(this.age, {required username}) : super(username: username); // ok, email is optional in User.new()
Person(this.age, {required super.username}) : super.genEmail; // you can specify a specific constructor
// translates to
Person(this.age, {required username}) : super.genEmail(username: username);
Person(this.age, {required super.username, super.email}) : super.positional; // error, super parameters must be named
} |
I worry that this feature is a little too magical and special... but I have to admit that forwarding parameters to superclass constructors is a really annoying chore in Dart. I often wish superclass constructors were straight up inherited to avoid it, and I have sometimes designed classes to have two-phase initialization (i.e. a separate initializing method in the superclass to pass in its state) just to avoid having to forward all the parameters through the subclass. Here is one example. The suggested
It is a compile-error if a constructor parameter list containing a The real question is how useful this sugar would be. My hunch is pretty useful. Actually, I went ahead and wrote a little script to scrape a corpus and try to answer it empirically. It looks for
If all of those are true, then the
So of the non-empty If you're curious, the longest // syncfusion_flutter_core-18.4.44/lib/src/theme/range_selector_theme.dart:271
/// Create a [SfRangeSelectorThemeData] given a set of exact values.
/// All the values must be specified.
///
/// This will rarely be used directly. It is used by [lerp] to
/// create intermediate themes based on two themes created with the
/// [SfRangeSelectorThemeData] constructor.
const SfRangeSelectorThemeData.raw({
@required Brightness brightness,
@required double activeTrackHeight,
@required double inactiveTrackHeight,
@required Size tickSize,
@required Size minorTickSize,
@required Offset tickOffset,
@required Offset labelOffset,
@required TextStyle inactiveLabelStyle,
@required TextStyle activeLabelStyle,
@required TextStyle tooltipTextStyle,
@required Color inactiveTrackColor,
@required Color activeTrackColor,
@required Color thumbColor,
@required Color thumbStrokeColor,
@required Color overlappingThumbStrokeColor,
@required Color activeDivisorStrokeColor,
@required Color inactiveDivisorStrokeColor,
@required Color activeTickColor,
@required Color inactiveTickColor,
@required Color disabledActiveTickColor,
@required Color disabledInactiveTickColor,
@required Color activeMinorTickColor,
@required Color inactiveMinorTickColor,
@required Color disabledActiveMinorTickColor,
@required Color disabledInactiveMinorTickColor,
@required Color overlayColor,
@required Color inactiveDivisorColor,
@required Color activeDivisorColor,
@required Color disabledActiveTrackColor,
@required Color disabledInactiveTrackColor,
@required Color disabledActiveDivisorColor,
@required Color disabledInactiveDivisorColor,
@required Color disabledThumbColor,
@required this.activeRegionColor,
@required this.inactiveRegionColor,
@required Color tooltipBackgroundColor,
@required Color overlappingTooltipStrokeColor,
@required double trackCornerRadius,
@required double overlayRadius,
@required double thumbRadius,
@required double activeDivisorRadius,
@required double inactiveDivisorRadius,
@required double thumbStrokeWidth,
@required double activeDivisorStrokeWidth,
@required double inactiveDivisorStrokeWidth,
}) : super.raw(
brightness: brightness,
activeTrackHeight: activeTrackHeight,
inactiveTrackHeight: inactiveTrackHeight,
tickSize: tickSize,
minorTickSize: minorTickSize,
tickOffset: tickOffset,
labelOffset: labelOffset,
inactiveLabelStyle: inactiveLabelStyle,
activeLabelStyle: activeLabelStyle,
tooltipTextStyle: tooltipTextStyle,
inactiveTrackColor: inactiveTrackColor,
activeTrackColor: activeTrackColor,
inactiveDivisorColor: inactiveDivisorColor,
activeDivisorColor: activeDivisorColor,
thumbColor: thumbColor,
thumbStrokeColor: thumbStrokeColor,
overlappingThumbStrokeColor: overlappingThumbStrokeColor,
activeDivisorStrokeColor: activeDivisorStrokeColor,
inactiveDivisorStrokeColor: inactiveDivisorStrokeColor,
overlayColor: overlayColor,
activeTickColor: activeTickColor,
inactiveTickColor: inactiveTickColor,
disabledActiveTickColor: disabledActiveTickColor,
disabledInactiveTickColor: disabledInactiveTickColor,
activeMinorTickColor: activeMinorTickColor,
inactiveMinorTickColor: inactiveMinorTickColor,
disabledActiveMinorTickColor: disabledActiveMinorTickColor,
disabledInactiveMinorTickColor: disabledInactiveMinorTickColor,
disabledActiveTrackColor: disabledActiveTrackColor,
disabledInactiveTrackColor: disabledInactiveTrackColor,
disabledActiveDivisorColor: disabledActiveDivisorColor,
disabledInactiveDivisorColor: disabledInactiveDivisorColor,
disabledThumbColor: disabledThumbColor,
tooltipBackgroundColor: tooltipBackgroundColor,
overlappingTooltipStrokeColor: overlappingTooltipStrokeColor,
overlayRadius: overlayRadius,
thumbRadius: thumbRadius,
activeDivisorRadius: activeDivisorRadius,
inactiveDivisorRadius: inactiveDivisorRadius,
thumbStrokeWidth: thumbStrokeWidth,
activeDivisorStrokeWidth: activeDivisorStrokeWidth,
inactiveDivisorStrokeWidth: inactiveDivisorStrokeWidth,
trackCornerRadius: trackCornerRadius); In the Flutter repo itself:
Flutter's best/worst example is: // packages/flutter/lib/src/material/raised_button.dart:32
/// Create a filled button.
///
/// The [autofocus] and [clipBehavior] arguments must not be null.
/// Additionally, [elevation], [hoverElevation], [focusElevation],
/// [highlightElevation], and [disabledElevation] must be non-negative, if
/// specified.
@Deprecated(
'Use ElevatedButton instead. See the migration guide in flutter.dev/go/material-button-migration-guide). '
'This feature was deprecated after v1.26.0-18.0.pre.',
)
const RaisedButton({
Key? key,
required VoidCallback? onPressed,
VoidCallback? onLongPress,
ValueChanged<bool>? onHighlightChanged,
MouseCursor? mouseCursor,
ButtonTextTheme? textTheme,
Color? textColor,
Color? disabledTextColor,
Color? color,
Color? disabledColor,
Color? focusColor,
Color? hoverColor,
Color? highlightColor,
Color? splashColor,
Brightness? colorBrightness,
double? elevation,
double? focusElevation,
double? hoverElevation,
double? highlightElevation,
double? disabledElevation,
EdgeInsetsGeometry? padding,
VisualDensity? visualDensity,
ShapeBorder? shape,
Clip clipBehavior = Clip.none,
FocusNode? focusNode,
bool autofocus = false,
MaterialTapTargetSize? materialTapTargetSize,
Duration? animationDuration,
Widget? child,
}) : assert(autofocus != null),
assert(elevation == null || elevation >= 0.0),
assert(focusElevation == null || focusElevation >= 0.0),
assert(hoverElevation == null || hoverElevation >= 0.0),
assert(highlightElevation == null || highlightElevation >= 0.0),
assert(disabledElevation == null || disabledElevation >= 0.0),
assert(clipBehavior != null),
super(
key: key,
onPressed: onPressed,
onLongPress: onLongPress,
onHighlightChanged: onHighlightChanged,
mouseCursor: mouseCursor,
textTheme: textTheme,
textColor: textColor,
disabledTextColor: disabledTextColor,
color: color,
disabledColor: disabledColor,
focusColor: focusColor,
hoverColor: hoverColor,
highlightColor: highlightColor,
splashColor: splashColor,
colorBrightness: colorBrightness,
elevation: elevation,
focusElevation: focusElevation,
hoverElevation: hoverElevation,
highlightElevation: highlightElevation,
disabledElevation: disabledElevation,
padding: padding,
visualDensity: visualDensity,
shape: shape,
clipBehavior: clipBehavior,
focusNode: focusNode,
autofocus: autofocus,
materialTapTargetSize: materialTapTargetSize,
animationDuration: animationDuration,
child: child,
); I think we should do it. |
Those stats are quite supportive! One thing I noticed is that while the big one (70%) can be refactored to use
Does this have to be the restriction, or can it just be the default? Some have commented that it would be helpful to be able to manually specify the super constructor after the class User {
final String username;
final String? email;
User({required this.username, this.email});
User.genEmail({required this.username}) : email = username + "@gmail.com";
}
class Person extends User {
final int age;
/// Here, we specify a super constructor to use while keeping this the default constructor for [Person].
Person(this.age, {required super.username}) : super.genEmail;
// translates to
Person(this.age, {required username}) : super.genEmail(username: username);
}
Can you elaborate on this? I'm assuming you mean something like this: class Person {
final int age;
Person({required this.age});
}
class User extends Person {
final String username;
User({required this.username, required super.age});
// translates to
User({required this.username, required age}) : super(age: age);
}
class Admin extends User {
final String token;
Admin({required this.token, required super.username, required super.age}); // error
// but why can't this simply translate to:
Admin({required this.token, required username, required age}) :
super(username: username, age: age);
} Since each constructor needs to forward its arguments to its super-constructor, multi-level inheritance shouldn't be an issue; the constructor always forwards all |
Couldn't you eliminate boiler plate further than having It might feel magical (it really does not), but if you have 20 parameters that are just passed to super those links are really an inconvenience in readability simply because they don't bring meaningful information. |
Yes! It was a much higher percentage than I expected.
Yeah, you can always add more and more sophisticated syntactic sugar to cover more edge cases, but you reach the point of diminishing returns. Sugar adds to the cognitive load of the language and we always have to be sensitive to minimizing the amount that users need to know to understand a page of Dart code. I think the fairly simple rules I suggested are probably pretty close to a sweet spot where they cover a lot of cases but aren't that magical.
Sure, we could do that. I don't know if it buys us enough to justify, though.
I mean in the same constructor: class Person {
final int age;
Person({required this.age});
}
class User extends Person {
final String username;
User({
required this.username,
required super.age, // <-- "super." param
}) // <--
: super(age: 23); // <-- and super in initializer list.
} This would be prohibited because now you're trying to specify the same super clause two different ways, explicitly and implicitly in terms of |
Do we worry about a usability cliff when you need to pass one non-forwarded parameter to a superclass? class C extends B {
C({int foo, int bar, int baz}) : super("FromC", foo: foo, bar: bar, baz: baz);
} Here we cannot use the It still breaks forwarding if the extra argument goes after the ones you want to forward, so it isn't entirely general. |
Yeah, that's a reasonable extension. I considered that but tried to keep things as simple as possible so left it out of my strawman. I think it's pretty interesting that even without that, the simple proposal still covers most superclass constructor invocations. |
We also tend to avoid the constructor and split the state management. |
We spent some time discussion various strategies for merging positional |
I hope the |
DartDoc is not structured in a way that guarantees that you can inherit parameter documentation. /// The [nameOfParameter] yadda, yadda, cahoots. we might be able to extract that paragraph, but it might be referring to other parts of the super-constructor as well, and copying that into the subclass constructor documentation won't necessarily work. |
@cedvdb, the way I look at it, your docs aren't about parameters, you should be documenting the functions, methods, and constructors to which the parameters belong. Put this way, you want to focus your docs on why the parameters are in this constructor (they inherit from a super constructor) and how they mesh with the other parameters in your subclass's constructor: class Person {
final String name;
final int age;
/// Creates a normal person with [name] and [age].
const Person(this.name, this.age);
}
class User extends Person {
final String username;
final bool isAdmin;
/// Creates a user object.
///
/// User is just a subclass of [Person], so be sure to pass in [name] and [age] first. User-specific fields are
/// appended to the end.
///
/// Notice how this doc explains how to use the parameters, which is specific to [User], and not just
/// what they mean, which could be inherited from [Person]. I would use [Person.field] for explaining
/// the purpose of a field, not the parameter name itself, since that _is_ inherited.
const User(super.name, super.age, this.username, {required this.isAdmin}); |
I know that but it's not convenient. When you have 20 parameters, which is not uncommon when making a flutter package ( people need customization) then you have to find the correct param in a wall of text. It would be simpler to just hover the parameter in question and have its associated documentation popup instead of the whole class documentation. This already happens with instance fields that are documented I don't see why this restriction has to be made for non instance fields. |
Instance fields have their own documentation, and you can point to that when hoovering over an initializing formal for that field. If we can deduce which part of the super-constructor documentation is documenting the corresponding super-constructor parameter, then we can show it. If we can't, there isn't a lot we can do instead. |
Just for the record, here is an earlier comment (from @apps-transround) where the use of |
Thank you very much, Erik @eernstg ! |
"Deducing" is ambiguous here. Does it mean deducing which part of the super class documentation is related to the param or deducing from the super field name ? The act of deducing the part of the documentation can be deduced by the parameter name in the same vein the class A {
/// documentation for p
int p;
A({this.p = 1});
}
class B extends A {
/// documentation for q
int q;
B({super.p, this.q = 2});
} Where when hovering each of those param I can have the documentation related to the param: B(
p: 2, //hovering here gives me the documentation of p
q: 1, //hovering here gives me the documentation of q
) |
If the associated super-constructor parameter is an initializing formal, then we could "inherit" the documentation of that field. So, for the example you show here, I see no problem with giving the documentation you ask for on hovering. Inheriting documentation in the case where the super-parameter doesn't target an initializing formal (directly or transitiviely) means that there is no field, so the only way to "inherit" documentation is to extract it from the super-class constructor's documentation (which is indeed what I meant by "deduce"): class A {
/// Some docs
int p;
/// Creates an A.
///
/// The [p] is one less than the [A.p] of the created `A`!
A({int p = 1) : this.p = p + 1;
}
class B extends A {
B({super.p});
}
... B(p: 2) ... /// Can we show documentation here? Maybe!. Here we could possibly deduce that |
What's the priority of this ? Is there a way to see what is planned and what is not ? I honestly don't know where to look in this repository beside the language funnel. In the accepted folder it only goes back only to |
@cedvdb we are trying 2.16 and you can already experiment it: class Bar {
final List items;
Bar(this.items);
}
class Foo extends Bar {
Foo(super.items): super(items);
}
but I guess it is not finished as I think it should be possible to omit |
Wait, so now you can write |
The upcoming super-parameters feature will allow |
Will this have support for default values ? This would be super (hehe) useful for testing because the IDE can generate the code for super constructors (well hopefully IDE will be able to) class TestHouse extends House {
TestHouse({ super.windows = 4, super.doors = 3 });
}
// in the original House, both windows and doors are required.
expect(TestHouse(windows: 2).windows, equals(2));
expect(TestHouse().doors, equals(3)); The IDE can generate the constructor then you add default here and there for your unit tests. Creating a function that returns an instance of the original class with defaults is verbose to setup. |
Yes, they can be specified, and if they are available from the given superclass constructor then they can be provided implicitly as well. |
I figure it would require non constant defaults #140 to be useful for flutter widget testing, where you often have callbacks or maybe a way to have constant functions. Currently this require going into the source of parent class, copy pasting the constructor and replace I hope dart --fix will be able to clean the code that can be cleaned. |
A constant expression that evaluates to a function object is not hard to obtain, e.g., // Wanted.
class A {
void Function(int) f;
A([this.f = const (int _) {}]); // Error, we don't (yet) have constant function literals.
}
// Workaround.
class A {
void Function(int) f;
A([this.f = _fDefault]); // OK.
static void _fDefault(int _) {}
}
// Works with super parameters.
class B extends A {
B([super.f]); // Means `B([void Function(int) f = _fDefault]): super(f);
} |
how can we try this in the 2.16 beta? it says
but my sdk constraint is |
This feature is still under development behind a flag. You can turn it on with |
@leafpetersen yeah, the risks are ok, there is this project that will take months to go to production and would certainly take good use of this feature because the project uses AngularDart heavily. Neither of these commands work:
Error:
|
Note that AngularDart itself definitely doesn't have any support for this yet (it might work, but if so, just by coincidence).
I don't know if build_runner supports experiments or not. @jakemac53 might know. For analysis, you can add the option in analysis_options.yaml, but I don't think the compiler looks at that. |
Do we have a published analyzer that supports the experiment? If so then the build_runner command should work (or something very similar, can't remember the exact details but can verify tomorrow). |
(Side note, this side conversation is going way into the weeds, but I'll keep it going :) ) analyzer 3.1.0 was published recently, after Dart 2.17 dev.1.0, which should understand the current language as 2.17.0. That should also understand the experiment flag. |
Can you try |
same result @mit-mit |
worked by cleaning .dart_tool and executing after that webdev also started working through no idea why, thought. (AngularDart really seem not work with this feature yet. It appears to not find the correct type to inject. We already tweaked Angular to work with Analyzer 3.1.0 but we will wait for this feature to be stable before updating Angulars compiler) |
Looks like You also will need to be on the 2.17.0 dev sdk, have |
This feature has been accepted into the language, and the implementation is handled from here: dart-lang/sdk#48055. 🎉 |
This feature is now turned on by default at HEAD, and will be available without a flag in the upcoming beta. Work on downstream tooling support is still in progress. |
Admin comment by @mit-mit:
This proposal now has a feature specification here: super parameters.
This feature enables not having to repeat all parameters from a super class twice. Code previously written like:
can now be written as:
This is likely especially useful is code like Flutter widgets, see for example
RaisedButton
.Original comment by @roy-sianez:
Currently, in Dart, there is syntactic sugar to initialize the property of a class from a constructor parameter:
However, if you want to initialize the property of a superclass, you have to write boilerplate code:
I propose adding syntactic sugar where
super
could be used instead ofthis
in the context of a constructor parameter to initialize the property of a superclass, like so:Example 3 would be equivalent to example 2.
To prevent the bypassing of important logic in superclass constructors, it could be a requirement that to use this syntactic sugar, the superclass must provide a constructor that uses all
this.
orsuper.
parameters; a class that usessuper.
syntactic sugar would delegate to this constructor in its superclass.This syntactic sugar would be most useful for simple classes where fields are initialized without much preprocessing or logic.
The text was updated successfully, but these errors were encountered: