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

Alternative for "required" keyword #2574

Open
IvanDembicki opened this issue Oct 16, 2022 · 12 comments
Open

Alternative for "required" keyword #2574

IvanDembicki opened this issue Oct 16, 2022 · 12 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@IvanDembicki
Copy link

bool longSyntax({required String named}) => ...
bool shortSyntax({String! named}) => ...

@IvanDembicki IvanDembicki added the feature Proposed language feature that solves one or more problems label Oct 16, 2022
@lrhn
Copy link
Member

lrhn commented Oct 17, 2022

While short, I think it puts the emphasis in the wrong place. It's not the type which is required, but the parameter.
It's possible (but probably not recommended) to have a required nullable parameter, so you'd write String?! name. Really?! 😉

(Personal favorite: Make parameters required unless they have a default value. Pro: Same as other languages. Con: Requires = null on all the optional nullable parameters.
Slightly more breaking alternative: Make parmeters required unless they are nullable or have a default value. Con(?): Cannot have required nullable parameters. Not a big loss! Potentially nullable parameters will just be required, which is fine, since you can't give them a default value anyway. Con: Have to provide a default value in function types too, to mark parameter optional.
Or just use [...] around the optional parameters: foo({int x, int y, [Color c = Color.red]}). Con: Adds extra characters for optional parameters (which already use more characters, and are still the majority of named parameters.)

@mateusfccp
Copy link
Contributor

IMO, #878 should have been the way to go. The issue has been opened some time before the feature release, but the team wanted to rush and simply discarded the idea. Now, it's harder to make this viable...

@lrhn
Copy link
Member

lrhn commented Oct 17, 2022

Trust me, it wasn't just "rushing". There were long and arduous discussions about all the variants of parameter syntax. They all had trade-offs. You might not agree with the weighting assigned to each trade-off (that's why there were discussions), but that doesn't mean it wasn't a very deliberate choice in the end.

@mateusfccp
Copy link
Contributor

mateusfccp commented Oct 17, 2022

@lrhn Sorry, I remember seeing a comment that gave this idea to me in this issue, although I couldn't find it. I know there were internal discussions in the team, but apparently the discussions took place before this issue has been opened, and in this issue many points were reconsidered to the point that it was concluded by some that it would be possible. Also, user-base wise, the great majority were in favor of the proposal.

Would something like this be reconsidered for Dart 3?

@jakemac53
Copy link
Contributor

jakemac53 commented Oct 17, 2022

It's possible (but probably not recommended) to have a required nullable parameter

Yeah while somewhat rare this does happen in practice. Sometimes you want the user to have to be explicit about making something null. That way when you add a new parameter, it is easy to see all the places that need to be updated to pass that parameter (even if they just pass null, it forces the user to consider what the appropriate value is, which can be useful).

@lrhn
Copy link
Member

lrhn commented Oct 18, 2022

A large change like this is very unlikely to make it into Dart 3. The necessary time just isn't there. (Dart 3.0 is visible on the horizon, and plans are being made. Adding a new big feature to those plans is unlikely, unless the benefit of getting it sooner, rather than later, is massive.)

Even with language versioning, it's getting harder to change syntax, because we still have to migrate everybody to the new syntax, or it will block them from using other new features. If the migration is completely automatable, then it might be possible. If it's entirely automatable, then it's probably not a big change.

If there is also a change of behavior, it gets even harder. We need to play the long migration game, with deprecations and lints to move people in a direction where they don't use the part of the feature which changes behavior (when we're lucky, it's lints we already have), then when we've gotten everybody to stop using it, we can do the change, and people can start using the new behavior. Which means there need to be an existing alternative to the thing we change.

@mateusfccp
Copy link
Contributor

@lrhn Is this change really large? I mean, IIRC one of the approaches suggested in the aforementioned issue was that the required keyword keeps existing. The difference is that non-nullable parameters without default value would be required by default. This is already the case in some sense, as if you try making a function with named parameter with no default value, the linter forces you to put required anyway.

Thus, it seems the migration would be simply to remove this required from the non-nullable parameters that has no default values. Having the required keyword would be no more than a warning, so it wouldn't break compilation.

Maybe I oversimplified something, but it doesn't seem that hard to me. WDYT?

@lrhn
Copy link
Member

lrhn commented Oct 18, 2022

If all we do is to make something which is currently an error into a non-error, by making a non-nullable named parameter required, then it's not breaking.

There are still details to consider.

It doesn't affect types.

typedef Foo = int Function({int x});

which today defines an optional parameter, and still does. The reader might assume it's required because it's non-nullable, but that only applies where you can add a default value (and where it's currently an error, which this isn't).

Same for abstract methods:

abstract class C {
  int foo({int x});
}

This x parameter is optional. It's valid today, so we don't change it.
Again, the author might think it's required.

So, the change only affects non-abstract function argument lists.

The problem with silently changing something is that it can mask an error.
If you intended to make the argument nullable, but forgot the ?, then you get no warning. It's just required, which might take a while to figure out. (Eventually your testing should get to calling without the argument. Because you do thorough testing.)

Similarly, if you intend a parameter to be required, but make a mistake in the type (say ending up with FutureOr<T?> as the type), then the parameter is just-as-implicitly optional. (Making the choice implicit means that either direction is implicit.)
If you make a mistake, and that gets released, then it's a breaking change to make the parameter required.

This sounds contrieved, but consider:

class Foo<T> {
  void foo(int x, {T value}) { ... }
}
class FooBar extends Foo<dynamic> {
  void foo(int x, {dynamic value}) { ... } // value optional.
}
class FooBaz extends Foo<Object?> {
  void foo(int x, {Object? value}) { ... } // value optional.
}

In the FooBar/FooBaz classes you just blindly inserted the type for T in the parameters, and accidentally made the parameter optional. You then have to go back and add required. Which you will definitely forget, because your tests will not check that the supposedly required parameter can be omitted.

And then there is trying to explain to someone why a nullable parameter is required:

Foo<int?> f = Foo<int?>();
f.foo(42); // Why is `value` argument required, when its type is `int?`?

But it is, and has to be, because in the interface declaration, the type T was potentially non-nullable, and that means that parameter is required. It depends on the type at the declaration, not the actual type of the parameter.

Things get confusing. Even with a simple model that just inserts required for you, the loss of explicitness makes things harder on both the reader and the author. ("Favor explicit over implicit.")

@cedvdb
Copy link

cedvdb commented Oct 20, 2022

Yeah while somewhat rare this does happen in practice. Sometimes you want the user to have to be explicit about making something null.

Those time a nullable parameter is required, it's often important. It is mostly relevant for nullable things, else there is no point in having that feature to begin with ?

The philosophy to have errors being caught at compilation time is something I wish will stay. Making null optional would be quite a big step backward as I understand it. Most of the annoyance comes from the fact that required on non nullable seems redundant.

@Wdestroier
Copy link

Other ideas:

  • Allow the C and TypeScript syntax to define positional and named parameters, respectively
  • Allow the ? mark on variables and on types to define optional parameters and nullable types, respectively

@Levi-Lesches
Copy link

Ugh.. I'm surprised it compiles as it "looks like" the type is widening. Why not mark it as nullable on the abstract class to begin with ? This looks pretty implicit to me.

To be fair, it's quite explicit: you're specifically allowing nullable types in D. Think of it as analogous to the following:

abstract class A {
  void test(int value);
}

abstract class B extends A { 
  @override
  void test(num value);
}

You're good if you explicitly say you accept any number and handle it within B. The same goes for nullability: if an interface demands non-nullable, you can make an exception for your class so long as you handle it within the function.

@IvanDembicki
Copy link
Author

Please nothing special. Just short alternative for

class SomeClass {
SomeClass({
required this.value,
....

class SomeClass {
SomeClass({
this.value!,
....

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

7 participants