dart-lang / language

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

In a function, non-nullable optional parameters with default value should allow to be called with nullable values #1639

Open ramsestom opened 3 years ago

ramsestom commented 3 years ago

I am trying to migrate a (large) flutter app to null sound safety and I encountered an issue on some functions where the dart control flow analyzer is complaining if I try to assign a nullable value to a non nullable optional parameter with a default value

For exemple, I have a function defined like this: static Vector pixelsToWorld(Vector xyz, Matrix4 pixelUnprojectionMatrix, [double targetZ=0]) So the targetZ parametter is optional and default to 0 The problem is I have some objects that have a targetZ nullable property (double?) and that have to keep this as it is (because I want to be able to differentiate objects that have some information on this targetZ property (targetZ can have a 0 value or something else in this case) from those that don't (in that case the targetZ property would be null)). When I try to call my pixelsToWorld function from one of these objects, the flow analyzer is complaining that The argument type 'double?' can't be assigned to the parameter type 'double' So, if I want to get rid of this error, I have to test if my targetZ value is null or not before calling the pixelsToWorld function without or with this optional parameter, which is not really convenient...

Wouldn't it make more sense to automatically attribute an optional parametter with a default value its default value if the function is called with a null value for it? (alter all that is what default parameter values are meant for, assign a default value in case of a missing information on the value of this parameter, be it because of an implicit or explicit null)

eernstg commented 3 years ago

Why not just declare the type of the optional parameter as double?? You can still have a default value, and it can still be zero.

I don't think it's a great idea to allow passing null as the value of an optional parameter even in the case where that parameter has a non-nullable type: This just means that we give up null checks on that parameter, and that means that we can have a number of bugs where the parameter gets the default value because of an unintended null, which is basically just another kind of null error.

Conversely, if the parameter has a nullable type then it would surely be confusing if passing null to that parameter wouldn't cause that parameter to have the value null.

void f([int? i = 42]) => print(i);

void main() => f(null); // Prints '42'!
ramsestom commented 3 years ago

Why not just declare the type of the optional parameter as double?? You can still have a default value, and it can still be zero.

Because I know that this parameter should never be null in my pixelsToWorld function (where I am performing some maths with it) so I don't want to have to test if null or not later on in my function And if the type of the optional parameter with a default value is double?, how would the compiler react if I pass a null value to the function? would this parameter be attributed the default value (this is what I want), or would it keep its null value?

I don't think it's a great idea to allow passing null as the value of an optional parameter even in the case where that parameter has a non-nullable type: This just means that we give up null checks on that parameter, and that means that we can have a number of bugs where the parameter gets the default value because of an unintended null, which is basically just another kind of null error.

the parameter would get the default value in the case of an implicit null parameter already (ie: this parameter is omited in the function call). So I don't see why the behaviour wouldn't be the same in the case of an explicit null (if you declare a default value for a parametter, this is clearly that you want this paramater to take this default value if you have no information on its value. So if attributing this default in case of a null value result in a 'bug' in your application, this means that the variable you passed to the function should not be declared as nullable and the unintended null value must be checked at this level, not when entering the function)

ramsestom commented 3 years ago

So, if I want to get rid of this error, I have to test if my targetZ value is null or not before calling the pixelsToWorld function without or with this optional parameter, which is not really convenient...

You don't have to test it if you assign a default value in the body of a function.

foo([double? x]) {
  x ??= 42;
  print(x+1); // compiler doesn't complain
}

Yes, that is another solution, but not really satisfying either as it obfuscate the fact that x would actually receive a default value (when calling the function from another part of your code, you will have to actually look at the content of the function to see that x is assigned a default value, the description of the function will no longer make a reference to the fact that x has a default value. It therefore amounts to no longer being able to benefit from the advantage for which the parameter declaration with a default value was created in the language: knowing at a glance which parameter of a function will be assigned a default value in case of lack of information on its value.) Anyway considering that transmetting a null value is not the same as transmetting nothing is completely illogical . Giving a function "nothing" (aka null) should always be the same as not giving the function anything. When an optional function parameter is nullable and has no default, if I omit this parameter in a function call, it ends up to receive the null value. So it is the proof that not giving any value to a paramater and giving it a null value in a function call should be considered the same.

eernstg commented 3 years ago

@ramsestom wrote:

the parameter would get the default value in the case of an implicit null parameter already (ie: this parameter is omited in the function call).

I don't see how it could be justified to say that null is passed implicitly in the situation where a given optional parameter isn't passed. It's not there, and there is no particular value that you can claim for it. In that situation the default value will be passed, and that doesn't have to be null (and with a non-nullable type it actually can't be null).

void print2([String s = 'No argument provided!']) => print(s);

// Works like a set of functions, one for each actual argument list shape:

void print2$0() => print2$1('No argument provided!'); // Pass default values for each omitted argument.
void print2$1(String s) => print(s);

if attributing this default in case of a null value result in a 'bug' in your application, this means that the variable you passed to the function should not be declared as nullable

I mentioned two cases:

  1. The parameter type is non-nullable. We're hypothetically still allowed to pass null because the parameter is optional; if we do pass null then the parameter will, hypothetically, get the default value. I said that this approach is error prone, because it means that call sites can pass null by accident rather than passing null in order to get the default value. You're essentially turning off null safety for that parameter.

  2. The parameter type is nullable, because null is a meaningful value for that parameter. I said that in this case it would be really surprising if null is a meaningful parameter value, but if you're passing null then it will be replaced by the default value (which is some non-null value).

eernstg commented 3 years ago

@ramsestom wrote:

Giving a function "nothing" (aka null) should always be the same as not giving the function anything.

I just don't buy this. f() is not the same thing as f(null) in general, not even in the case where the first positional parameter of f is optional, it's only the same thing in the case where that optional parameter has the default value null.

ramsestom commented 3 years ago

@ramsestom wrote:

Giving a function "nothing" (aka null) should always be the same as not giving the function anything.

I just don't buy this. f() is not the same thing as f(null) in general, not even in the case where the first positional parameter of f is optional, it's only the same thing in the case where that optional parameter has the default value null.

foo([double? x]) {
  if (x==null){print("x is null");}
}

call foo(); output: x is null

So yes f() and f(null) is exactly the same thing. An optional parameter is automatically considered as null if it has no default...

eernstg commented 3 years ago

With [double? x], the default value is null. That's a property of nullable types. For a non-nullable parameter type you don't get any default default value, you have to write one explicitly, or the parameter can't be optional.

ramsestom commented 3 years ago

But the automatic conversion of null into non-null is logically inconsistent.

Can't disagree more. In the case where the parameter has a default value, any null value should be converted into the non-null default value. If I don't pass any value, the parameter would receive the default value. So if I pass a null value to a variable that can not be null and has a default, it should also get this default value. If I define a default value, this is exactly for the case where I have no information on that parameter (the distinction you make on the fact that I pass the function no information (null) or don't pass any information (parameter omited) is completely irrelevant). If a function has an optional parameter with a default value, it means that it is meant to use this default value in case of a call with no information on the value of this parameter. And if I define this parameter to be non-nullable and try to give it a 'null' value that means that I don't know the value of this parameter so it should default to its default value. If I have an object with a nullable property int? x and I try to call a function f([int y=0]) there is only two logical scenarios if x is null in my object:

lrhn commented 3 years ago

I also want to treat passing null the same as not passing anything, in every possible way. It has some pros and some cons, and I tend to favor the pros over the cons. (And I also want to completely conflate nullability and optionality of parameters at the type level). We have been over the arguments for and against a number of times, and we didn't go for that - more radical - approach with null safety. It was a breaking change for some existing functions which treated null different from not passing a value.

It's not what we did, and changing it now is a breaking change.

eernstg commented 3 years ago

passing null the same as not passing anything, in every possible way

Including this one?:

void main() {
  print('Hello, world!', null); // OK!
}
lrhn commented 3 years ago

I'd put it the opposite way: Passing nothing for a parameter is the same as passing null, and passing null triggers replacing with the default value if there is one.

Let's define fully what I'd want (and will likely never get):

I'm fairly sure I am forgetting at least one thing which really annoyingly doesn't work well. Maybe because I want to combine it with non-constant default values. Also, it's quite breaking. But it would be nice. 😄

ramsestom commented 3 years ago

@ramsestom: see this example. Generally, the whole NNBD endeavor in dart is about specifically prohibiting the interpretation you are advocating for.

I saw you exemple but in my point of view the goal of the NNBD approach is not to detect any kind of typo in a code. If the developper did another type of "typo" and called the computeCreditScore function by using a wrong existing int variable in his code like:

var score = computeCreditScore(
   income: myFinancialSituation["income"],
   debt: myFinancialSituation["income"] //error, the developper made a copy-paste and forgot to change the key
);  

or simply forgot to call the computeCreditScore with a second argument

var score = computeCreditScore(
   income: myFinancialSituation["income"],
   //debt: myFinancialSituation["debt"] //commented line that shouldn't be
);  

the result would be the same and the compiler won't complain, with or without the NNBD. Can the developper blame the compiler in any case? no, it's the responsbility of the développer to ensure that he transmitted the correct information to a function he called. If I made a typo in a math sum and wrote x = 1+2 rather than x = 1+1, I won't blame the compiler because it did not gave me the correct result. The NNBD should not be thought as a way to detect any type of typo but only to detect data flow errors and ensure that, at any point of my code, If I expect an information for a variable (so it can't be null), there is no way this information would be missing (so I can't get or generate data that would lack this information = this variable can't be null). That's all. If I have a function with a parameter with a default value, it necessarily mean that I accept to have a missing information on this parameter (else I won't define a default value). In that case it means that if I am not able to provide a value for this parameter (either by passing null or omiting the parameter), I want the function to use the default value...

ramsestom commented 3 years ago

It's not about the typo. You can use the correct spelling and still get null accidentally - because the corresponding entry in the map was not assigned, but you are sure it was. You are proposing to ignore this effect. What else would you like to ignore? Passing a parameter is not different from assigning a value to a variable. Then the next logical step is to ignore the potential error also here:

int x=0;
...
x=map["x"]; // you are SURE you assigned map["x"]! But you didn't.
...
x=x+1; 

I am not proposing to ignore a null value due to a non or miss-assigment in absolute. What I say is that kind of error must be ignored when calling a function with a default parameter. There is a difference! NNBD is meant to detect null errors where unintended. If you define a default parameter in a function, that means that you expect this parameter to be possibly absent when entering this function. So checking if this parameter is null at this point is irrelevant. Yes it can possibly be due to a typo or a missassignment, but it can also be (and most probably is) because you don't have any information on this parameter. So you intended this (the lack of information on the value of a variable, at this exact point) to be possible (by defining an optional parameter). NNBD should be used to detect null only where completely unintended. As Dart do not make a difference between null and undefined, if it occurs when trying to cast a null value to an optional default parameter, it shouldn't be considered as unintended. If your typo or missassignment has other repercussions in your code where you really declared a null value to be completely uninended (for exemple you have int y = x; somewhere in your code while x is null or x=map["x"]; where map["x"] is undefined like in your example) then, with NNBD, you expect the compiler to complain. But only at this point.

For the record, I have never been a fan of NNBD. IMO, by switching to NNBD, dart lost more than it gained. The kind of applications normally written in dart is not mission-critical stuff; focusing on nulls takes away your attention from other things (the capacity of the brain is limited), and in the end, the total number of bugs may not decrease (I saw some articles about rust claiming exactly that). But once the decision has been made, the implementation has better be at least logically consistent.

I'm not a fan of NNBD either ;) Having to convert a really large flutter app, with many complex objects with legitimate nullable properties, to null-safety, I can say this turned Dart to be really verbose (even more than Java). As properties can't be promoted, I now have to bang cast my nullable properties everywhere... Having non null objects by default really complicated my app code rather than it bringed advantages and the possibility to explicitly define objects or variable as non-nullable, rather than making it the default, would have been much more smarter and easier to use for the developpers in my point of view.

munificent commented 3 years ago

@ramsestom, what would you have the following program print:

foo([String? optional = "default") {
  print(optional);
}

main() {
  foo(null));
}

In other words, in cases where null is an allowed value of the parameter's type, how does the language know when an explicit null means "no parameter, use the default" versus "I explicitly want this null instead of the default"?

Whichever answer you choose, how can you be certain that that is the behavior that most Dart users will intuitively expect?

ramsestom commented 3 years ago

@munificent I am only talking about the case where the optional parameter is non-nullable and you call the function with a null value for this parameter. So I only consider this case:

foo([String optional = "default") {
  print(optional);
}

main() {
  foo(null));
}

So you know that you would never want a null value instead of the default at that function level. Currently this is resulting into an error in the dart code analyser because the optional parameter is non-nullable and we are trying to assign it a null value. What I say is that it shouldn't be the case. Dart, unlike javascript, do not (or at least is not supposed to) make a distinction between a value being null or unasigned. So, trying to call a function with a non-nullable optional parameter with a null value should be the same as calling this function with this parameter omited, and that parameter should gracefully fallback to its default value in both cases. Some people here argue that it is the goal of the NNBD to detect this kind of events, to possibly reveal some typos. But it is not. Writting String optional = "default" in a function parameters declaration is exactly the same as writting String optional = passedvalue ?? "default" (where passedvalue can be null or undefined, which is the same thing in Dart). If you write this code:

String? nativehi = "bonjour";
String hi = nativhi ?? "hello"; //<- notice the typo on nativehi here
print(hi);

The compiler won't crash on the nativehi typo.

So if you have


printHi([String hi = "hello"]) {
     print(hi);
}

String? nativehi = "bonjour";
printHi(nativhi); 

it should be exactly the same.

As for the possibility to have a nullable optional parameter with a default like:

foo([String? optional = "default") {
  print(optional);
} 

and what to do if we pass this function a null value (does the optional var take the default value or remains null?) this is something that sould be correctly adressed too. If we want to stay consistent with how Dart is currently handeling undefined variables (wich are considered to be exactly the same as a null var), optional should take the "default" value when a null value is passed to the function. But in that case, this type of declaration (having a nullable optional parameter with a default) should be simply prohibited in the langage (It shouldn't be allowed to have a nullable optional parametter with a default value) as it wouldn't make any sense (the nullable optinal parameter has no way to remain null). If this type of declaration is accepted by the langage and result in the optional parameter to remain null when called with null and to take the default only when omited, then this is an exeption to the Dart current contract on null and undefined values to be the same thing...

lrhn commented 3 years ago

@munificent I'd very happily make it print "default". I'm also quite willing to break any code which currently relies passing null meaning something else than passing nothing. I never considered that a good design, or something we should encourage.

ramsestom commented 3 years ago

Prior to NNBD, Dart was automatically giving a null optional parameter its default value. So

foo([String optional = "default") {
  print(optional);
}

main() {
  foo(null));
}

would print "default" prior to NNBD. So this is actually considering that passing null and passing nothing are two different things now that is a breaking change and a break in the Dart postulate that null and undefined are the same thing (unlike in javascript).

Even if the ability to distinguish null from undefined might be usefull (and used) in some cases in langages that make the distinction between the two, this is not suposed to be the case in Dart, like in most other langages (for good reasons too. Considering that null != undefined generally introduce more issues and inconstitencies than the few flexibility it may allow)

lrhn commented 3 years ago

@ramsestom Actually not. Prior to null safety, Dart would happily pass null as a value just like any other, and only use the default value if you didn't pass any value at all. That has not changed (some may think that it should have, but it didn't).

@tatumizer I am certain that there were Flutter APIs which had a non-null default value and treated null differently (something with borders, I believe). I can't find it now, so maybe it has changed. If so, yey! If not, my search-fu just sucks.

A quick code search finds a few cases of a nullable parameter with a (non-null) default value. It's mainly booleans, and therefore probably either bugs or (less likely) backwards compatibility features.

I'd say that it's almost invariably a bug to have a nullable parameter with a non-null default value. If you do parameter ??= defaultValue; inside the body, then it's safe, but the default value is unnecessary. If you don't, you need to handle the null case (at least null safety forces you to do that, but I've seen a few cases of just adding ! already, because "it's not going to be null in practice", but in that case, why is the parameter nullable to begin with?!?). If you actually handle null and treat it differently from the default value, some users will probably get confused (me included).

Levi-Lesches commented 3 years ago

I think this issue just highlights the importance of having some way to programmatically not pass an argument, preferably relying on null. I know there is discussion of this on #219, and I particularly support the use of the default keyword (which already exists!).

Combining @munificent's example here with one of @tatumizer's proposals on @219:

printNullableWithDefault([String? optional = "This is the default option"]) {
  print(optional);
}

String? getValue() => null;  // pretend this does actual work

main() {
  String? value = getValue();  // turns out to be null
  printNullableWithDefault(value);  // prints null
  printNullableWithDefault(value ?? default);  // prints the default value
}

Since default is already a keyword, that's backward-compatible. And the behavior of passing null to print null (and not the default) is already how Dart works currently, so that's backward-compatible as well. It's also very readable. Thoughts?

lucaesposto commented 1 year ago

I think this issue just highlights the importance of having some way to programmatically not pass an argument, preferably relying on null. I know there is discussion of this on #219, and I particularly support the use of the default keyword (which already exists!).

Combining @munificent's example here with one of @tatumizer's proposals on @219:

printNullableWithDefault([String? optional = "This is the default option"]) {
  print(optional);
}

String? getValue() => null;  // pretend this does actual work

main() {
  String? value = getValue();  // turns out to be null
  printNullableWithDefault(value);  // prints null
  printNullableWithDefault(value ?? default);  // prints the default value
}

Since default is already a keyword, that's backward-compatible. And the behavior of passing null to print null (and not the default) is already how Dart works currently, so that's backward-compatible as well. It's also very readable. Thoughts?

I would really like to have a similar solution to avoid redundant code and improve readability and maintainability. An alternative, in order to avoid using "default" keyword (that currently has a slightly different meaning) could be to introduce a new ad hoc keyword, specific to parameters in function calls. For example: "??ignore".

Examples (from Levi-Lesches' quoted code): printNullableWithDefault(value ??ignore);

Above code prints the default value because it's equivalent to

value != null
? printNullableWithDefault(value)
: printNullableWithDefault();

This would allow to code new functions/classes/FlutterWidgets reusing/calling/returning existing functions/classes/FlutterWidgets having optional parameters with defined defaults, without the need to redefine the default value.

lucaesposto commented 9 months ago

Expanding on previous reasoning, i've focused on two main reason to consider this feature, as i mentioned in #2269

1. Default values duplication

Extending a class which constructor parameters are non-nullable and have default values:

class A {
  const A({this.prop1 = 'v1'}); 
  final String prop1;
}

class B extends A {
  const B({this.prop1 = 'v1'});  // we desire parent default as default, but we have to redefine it
  final String prop1;
}

We are forced to duplicate default values definition. This adds boilerplate and/or sources of bugs if a default value is modified. In a chain of extensions the problem increases even more.

What is desired is something like:

[ class A ... ] 

class B extends A {
  const B({this.prop1 = super.prop1.default}); // or any other syntax that defines the concept of "use parent default"
  final String prop1;
}

2. Ignore passed argument value if null

Another related issue, that needs a different solution, arises when you have to pass a nullable value to a non-nullable parameter with defined default and you want to use default value if null:

class CustomButton extends StatelessWidget {
  final void Function()? onPressed;
  final Widget? child;
  final Clip? clipBehavior;

  const CustomButton({
    required this.onPressed,
    required this.child,
    this.clipBehavior,
  });

  @override
  Widget build(BuildContext context) {
    return ElevatedButton(
      onPressed: onPressed,
      child: child,
      clipBehavior: clipBehavior ?? Clip.none, // clipBehavior is non-nullable, Clip.none is its default value
    );
  }
}

In this case, desired solution is something like:

class CustomButton extends StatelessWidget {
  final void Function()? onPressed;
  final Widget? child;
  final Clip? clipBehavior;

  const CustomButton({
    required this.onPressed,
    required this.child,
    this.clipBehavior,
  });

  @override
  Widget build(BuildContext context) {
    return ElevatedButton(
      onPressed: onPressed,
      child: child,
      clipBehavior?: clipBehavior, // or clipBehavior: clipBehavior ??ignore as mentioned before, 
        // or any other syntax that defines the concept of "this should behave like if clipBehavior was not provided in the function call (in case it's null)"
    );
  }
}