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

Swift-like guard statement #1548

Open
erf opened this issue Mar 26, 2021 · 29 comments
Open

Swift-like guard statement #1548

erf opened this issue Mar 26, 2021 · 29 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@erf
Copy link

erf commented Mar 26, 2021

I would like a better way to handle optional/null values. I think Swift does this well using the guard statement.

I especially like this feature in order to unwrap optional values, and return early if null.

E.g.

String? name;
guard final nonNullName = name else {
    print("name is null. Cannot process");
    return;
}
print('name is $nonNullName');

Here are some more examples of how this works in Swift.

@erf erf added the feature Proposed language feature that solves one or more problems label Mar 26, 2021
@weenzeel
Copy link

I'm no expert and there may be edge cases, but do we need this in Dart? I think the way that the Dart compiler is able to analyze the code paths and promote nullable types to proved non-nullable types is quite elegant.

void doStuff([String? someString]) {

  if (someString == null) {
    return;
  }
  
  // OK to use nullable someString here.
  // Compiler knows we won't get here unless someString isn't null.
  
  print(someString.substring(0,10));
  // ...
  
}

void main() {

  doStuff('12345678901234567890');
  
}

@erf
Copy link
Author

erf commented Mar 26, 2021

My experience is that the analyzer can't detect when you have a class or chains with nullable types, like this:

class MyClass {
  String? someString;
}

void testNull(MyClass myObj) {
  if (myObj.someString == null) {
    return;
  }
  // Error: Analyzer don't know that someString is not null
  print(myObj.someString.substring(0, 10));
}

If we had guard we could do:

void testNull(MyClass myObj) {
  guard final someString = myObj.someString else {
    return;
  }
  print(someString.substring(0, 10));
}

Or check for more complex conditions / chains like this:

void testNull(MyClass? myObj) {
  guard final someString = myObj?.someVariable?.someString else {
    return;
  }
  print(someString.substring(0, 10));
}

@weenzeel
Copy link

You are correct that nullable fields aren't promoted. The reason given in the documentation is that the compiler can't prove that the field won't change its value between the time we check it and the time we use it.

The way to handle this today would be:

class MyClass {
  String? someString;
}

void testNull(MyClass myObj) {
  var myGuardedValue = myObj.someString;
  if (myGuardedValue == null) {
    return;
  }
  // Analyzer do know that my guarded value can't be null.
  print(myGuardedValue.substring(0, 10));
}

void main() {
  testNull(MyClass());
}

This is just a different syntax compared to Swift isn't it? I think the functionality is the same. I tried a small sample in Swift and I'm not allowed to do stuff with the real field without added ceremony there either. All I'm allowed to touch is a promoted local variable, just like in Dart.

@erf
Copy link
Author

erf commented Mar 26, 2021

That works, but i rather not have to declare a new variable, on a new line to have the analyzer conclude it is not null. With a guard statement you could do it in a one-liner.

Not sure what you mean with the Swift example (please provide an example), but with the guard statement you would check in run-time and not only analyze the code before hand.

@weenzeel
Copy link

As a layman it would be interesting to know why the compiler can't prove that the field won't be null though.

@mateusfccp
Copy link
Contributor

mateusfccp commented Mar 26, 2021

That works, but i rather not have to declare a new variable, on a new line to have the analyzer conclude it is not null. With a guard statement you could do it in a one-liner.

As far as I could understand your proposal, what you are proposing is similar to #1201 and wouldn't bring any benefit in relation to it.

As a layman it would be interesting to know why the compiler can't prove that the field won't be null though.

Consider the following:

class A {
  final num? amount = 5;
}

class B implements A {
  var _returnInt = false;

  @override
  num? get amount {
     // Evil, but this is a simplistic example.
    _returnInt = !_returnInt;
    return _returnInt ? 5 : null;
  }
}

void main() {
  A a = B();
  if (a.amount == null) {
    print(a.amount.runtimeType);
  }
}

If the compiler deems a.amount as non-nullable in if (a.amount == null) it will be clearly incorrect. This doesn't happen only with null-safety but any kind of promotion. You may want to refer to field promotion label.

@erf erf mentioned this issue Mar 26, 2021
@erf
Copy link
Author

erf commented Mar 26, 2021

@mateusfccp I was not aware of that proposal, and it looks like it could solve the simplest cases, but not sure if you be able to unwrap a chain of nullable types like this? Also it seem you must reuse the same variable name, so you can't asign a member of another type to that local variable.

  guard final myNonNullStr = myObj?.someVar?.someString else {
    // was not able to declare myNonNullStr, something was null
    return;
  }
  print(myNonNullStr);

@erf
Copy link
Author

erf commented Mar 26, 2021

Maybe this guard solution could be related to destructuring or pattern matching.

@lsegal
Copy link

lsegal commented Apr 10, 2021

As a layman it would be interesting to know why the compiler can't prove that the field won't be null though.

If the compiler deems a.amount as non-nullable in if (a.amount == null) it will be clearly incorrect.

@mateusfccp this seems just a little orthogonal to the original question which was specifically about fields. The example above is demonstrating the compiler's inability to determine nullness of a function. Perhaps my nomenclature is a bit off, but based on my understanding, "getter" functions sit atop the actual fields themselves, which means getters are not considered fields-- and presumably the compiler knows this?

If so, surely the compiler can detect when a function is accessed vs. a bare field, at which point presumably we should be able to promote fields without introducing any new syntaxes? Am I wrong about the distinction between fields/accessors?

It just seems pretty inconsistent to me for promotion to only work on variables in a local scope. Even global variables (which are by no means considered fields) do not get promoted?

image

This behavior breaks some fairly intuitive expectations around what is a variable and what is not. If we were legitimately dealing with method calls, sure, but we "know" (both intuitively and ideally provably so in Dart's AST) that x y and z above all return values in the exact same way.

@lrhn
Copy link
Member

lrhn commented Apr 11, 2021

Dart getters are "functions" in the sense that they can do and return anything. Non-local variable declarations introduce getters which just return the content of the field.

The compiler might be able to detect that some getters won't actually change value between calls, and that it's therefore sound to promote a later access based on an earlier check. However, that is breaking the abstraction. It means that if you ever change any of the implementation details that the compiler used to derive this, it will stop promoting. Any such change becomes a breaking change.

Since you should always be able to change a non-local variable to a getter (and possibly setter) and vice versa, the only safe approach is to not promote non-local variables. It's not sound towards future supposedly non-breaking changes.

@eernstg
Copy link
Member

eernstg commented Apr 12, 2021

@lrhn wrote:

Since you should always be able to change a non-local variable to a getter (and
possibly setter) and vice versa, the only safe approach is to not promote ...

I agree that we should support the encapsulation of properties (such that a non-local variable can be changed to a getter-&-maybe-a-setter). This ensures that the implementer of the property has a certain amount of freedom.

However, I don't see a problem in supporting a different contract with a different trade-off as well: The designer of the property could decide that the property is stable (#1518), which means that the associated getter must return the same value each time it is invoked. The implementer now has less freedom, but clients have more guarantees (in particular, such getters can be promoted). The loss of flexibility only affects variables/getters declared as stable, so it's fully controlled by the designer of each declaration, and there is no penalty for non-stable variables/getters.

// Assuming #1518.

import 'dart:math';

stable late int? x;

class Foo {
  stable late int? y = null;
  void run() {
    int? z = b ? 5 : null;
    if (x == null || y == null || z == null) return;
    print('${x + 1}, ${y + 1}, ${z + 1}'); // OK.
  }
}

bool get b => Random().nextBool();

void main() {
  x = b ? 2 : null;
  var foo = Foo();
  foo.y = b ? 3 : null;
  foo.run();
}

@yuukiw00w
Copy link

I would like to emphasize not only the use of guard as a syntax for unwrapping null values but also the readability aspect of the code, where it is guaranteed that the scope will be exited if the condition is false, simply by seeing guard.
For instance, in Flutter, a common condition check is context.mounted.
If we could write it as guard context.mounted else {}, it would make it easier to recognize that context.mounted is true below the guard statement, which is a significant readability advantage.

// This is the current way of writing it
if !context.mounted {
  return;
}

// This way is more readable
guard context.mounted else {
  return;
}

@tatumizer
Copy link

I would just say if (!context.mounted) return;. Looks quite readable to me.

To be honest, I don't understand the meaning of the verb "guard" here:

guard context.mounted else {
  return;
}

According to Webster, "to guard" means "to protect against damage or harm". What kind of harm? Who is protected by whom? Against what threat? And what "else" means in this context? 😄

@Reprevise
Copy link

I think that guard is pretty readable, and of course that if statement is also readable!

void hello() {
  guard (context.mounted) else {
    return;
  }

  // This is being "guarded"
  // ...
}

else must exit the block.

I'd argue guard provides the most value with patterns though.

final map = <String, Object?>{
  'name': 'John',
};
guard (map case {'name': final String name}) else {
  return;
}

// 'name' is usable here...
print(name); // 'John'

@tatumizer
Copy link

There's only one use case for guard-like construct that makes sense to me: when the condition contains declarations of variables (case clause). I think if! can work: when you use it, you promise to exit the scope in the "then" branch.
It's not the same as if (!cond) {...}, where you promise nothing, but the difference meaningfully manifests itself just in case of "case" expression in the condition. However, in the case-expression, negation is not supported anyway: we cannot say if (!(obj case A(:int x))), so there's no potential for confusion.
For other conditions. use if! to signal the intention to exit in then-branch (early return), or use negation !cond in the condition itself - there's no potential for any issues related to shadowed names (because there are none).

If if! statement has an else block, the variables declared in the case condition are available there (and only there). If there's no else block - they are available in the rest of the containing block.

Examples:

foo() {
  if! (obj case A(:int x)) {
    print("no match");
    return;
  } 
  print ("match");
  use(x); // available here
}
bar() {
  if! (obj case A(:int x)) {
    print("no match");
    return;
  } else {
    print ("match");
    use(x); // available here
  }
  use(x); // error, x is not defined here
}

@Mike278
Copy link

Mike278 commented May 23, 2024

According to Webster, "to guard" means "to protect against damage or harm". What kind of harm? Who is protected by whom? Against what threat? And what "else" means in this context? 😄

And a class is "a body of students meeting regularly to study the same subject" :P

While I agree guard isn't necessarily what I'd use as the keyword (Ruby's unless is better imo), it is as close as you can get to being the "official" term to describe the concept - https://en.wikipedia.org/wiki/Guard_(computer_science)

@tatumizer
Copy link

tatumizer commented May 23, 2024

I looked into the wikipedia article. The term "guard" is defined loosely as a condition that "guards" the entrance to some execution branch.
E.g. in OCAML:

Guards

Cases of a pattern matching can include guard expressions, which are arbitrary boolean expressions that must evaluate to true >for the match case to be selected. Guards occur just before the -> token and are introduced by the when keyword:

match expr with
     pattern1 [when cond1] -> expr1
   | ...
   | patternN [when condN] -> exprN

So it's the same type of "guard" that dart uses for similar purposes - see https://dart.dev/language/branches#guard-clause
I couldn't find the swift's kind of "guard" in other languages listed in Wikipedia article (no guard keyword, no guard-else or guard-let-else). I still find this construct exotic.

unless means the same as if!, but it's longer and requires a new keyword.
In dart if! rhymes with is!

@mateusfccp
Copy link
Contributor

I would love to have unless. It would be specially useful with patterns, as we can't do something like if (a case! pattern) or if (a case !pattern).

@yuukiw00w
Copy link

I don't mind if the language feature with the same meaning as guard is if! or unless.
What I care about is having a syntax that ensures the condition is met after that statement, meaning it guarantees a return within that statement.
This allows for writing more readable code.
While I agree that it is more useful during pattern matching, I don't think it needs to be limited to just pattern matching.

@tatumizer
Copy link

There's another good keyword: on.
on (cond) may signal the intent to bail out if the condition is true. The form on !(cond) means "bail out if the condition is false".

on (x == null) return;
on !(obj case A(:int x)) { 
    print("no match");
    return;
} 

The problem is that "on" is not quite a keyword (It can be used as an identifier).
The expression on (x==null) can be interpreted as an invocation of user-defined function with boolean parameter. To parse the on-statement, the compiler has to look beyond the closing ).

@lucavenir
Copy link

lucavenir commented Jul 18, 2024

I came from #3865 and, if you ask me, it feels like this issue should be rewritten as "let if case be negatable".

AFAIK what OP's asking for is almost obtainable via pattern match, but he's asking for some syntactic sugar, whereas in #3865 we'd like to obtain a negatable match.
Issue #3865 is now closed because the two ideas might be "mergeable".

Say we have

abstract class X {
  int get a;
  int get b;
  int get c;
}

And then, maybe, we want a particular condition to return prematurely as OP asked, while also capturing a "valid value".
If you're allowed to negate a match, you'd simply write:

X? something = ...;

if (something case! !=null && X(a: > 9, b: < 10, c: 2)) {
  throw AssertionError('this should *not* happen');
}

// use `something` which is now non-nullable and it's also been "validated"

This would be even more flexible and readable if case ... would simply return a bool.

X? something = ...;

final isValid = something case !=null && X(a: > 9, b: < 10, c: 2);

if (!isValid)  throw AssertionError('this should *not* happen');

So OP's problem is solved: something is promoted to non null (also the function stops if something == null).
And we get to write custom validation logic that, if not matched, we return prematurely.

Of course I have no clue if this is even possible, e.g. would this usage of ! clash with something else?

@lucavenir
Copy link

lucavenir commented Jul 24, 2024

I just stumbled on another usecase for negating a pattern.

Say we want to validate a json response, but we want to prematurely throw an exception if a "fundamental" field is missing or, if it's not missing, ensure it's not null.
Finally, we want to throw a CheckedFromJsonException if the contents of the actual response don't match what we expect.

The three errors are semantically different and should be handled accordingly.
Without patterns we could do:

void parse(Map<String, Object?> jsonResponse) {
  final token = jsonResponse['token'];
  if (token == null) throw Exception("invalid token, either not sent or null");

  final result = switch(jsonResponse) {
    {'some': final shape} => shape,
    _ => throw CheckedFromJsonException(...),
  };
  
  return result;
}

Turns out (AFAIK!) there's no "easy" way to do this with patterns.

switch (jsonResponse) {
  case {'token': != null}: // happy path
    print("token field has been sent and it's not null");
    print("in here I can further match on jsonResponse's shape");
  case {'token': null}:
    print('token field has been sent, but has been explicitly set to null');
    throw Exception("invalid token, it's null");
  case final map when map['token'] == null:
    print('token field has *not* been sent at all');
    throw Exception("invalid token, not sent");
  default:
    throw CheckedFromJsonException(...)
}

With a negatable pattern I could just write:

if (jsonResponse case! {'token': != null}) {
  throw Exception('invalid token');
}

And I'd be good to go towards parsing.

@munificent
Copy link
Member

I'll have to think more about negatable patterns, but as far as your example goes, I think you could write it like:

Object? parse(Map<String, Object?> jsonResponse) {
  return switch (jsonResponse) {
    _ when !jsonResponse.containsKey('token') =>
        throw Exception("invalid token, not sent"),
    {'token': null} =>
        throw Exception("invalid token, it's null"),
    {'some': final shape} => shape,
    _ => throw CheckedFromJsonException(...),
  }
}

@Albert221
Copy link

I wanted to +1 the part regarding the early-return, else promote variable type. I'm not a fan of the entirety of the guard clause. I just want concise early returns so that I don't have to wrap whole body in if-case body because of the type promotion, and the alternative is to introduce a variable before the if and use is and optionally normal conditions instead, more details in this duplicate issue: #4273

@lrhn
Copy link
Member

lrhn commented Feb 24, 2025

See also #3059. If (e1 case p when e2) was an expression, with its variables available in the same scope as its promotions, then a lot of things would become possible. Including

if (!(expr case pattern)) return;
// Variables of `pattern` available here.

Might need to end the variable scope at the next block end, but the condition part of the if can be considered as belonging to the surrounding block.

That's the very general feature I'd go for first, not something "guard against null" specific. Pattern matches anywhere!
You cannot negate a pattern, but you can negate a pattern test, because that's just a boolean expression with down-stream effects, just like a promoting test. Negating that just swaps the effects of the branches.

@tatumizer
Copy link

if (!(expr case pattern)) return;
// Variables of `pattern` available here.

What do you do about "lexical scope wins" rule? If the pattern introduces a variable a, and the variable by the same name is defined in a lexical scope, who wins? Is it an error?

@lrhn
Copy link
Member

lrhn commented Feb 24, 2025

The pattern would contain a declaration that introduces a name in the scope, and it would be closer than a declaration further out, so it wins if it is in scope.

The question is where that declaration is in scope.

Since I want the most flexibility with this feature, I'd go with:

  • The variable is in scope from the position of the pattern until the end of the nearest surrounding scope block.
  • It's a compile-time error to reference the variable in code where it's not definitely initialized by the pattern.

That would allow:

(expr case var v?) || (throw "Not null!");
use(v);
if (!(expr case var v?)) throw "Not null!";
use(v);

and

if (expr case var v?) {
  valid = true;
} else {
  throw "Not null!";
}
use(v);

If the last one is too confusion, one option is to restrict if specifically, so that a pattern variable of the condition is only
in scope in code of the branch where it matched if there is such a branch. The second example here had no branch in the if for the pattern matching, so the scope isn't limited to that branch.

Generalized, a (expr case var v?) : e2 : e3 would then restrict the scope to those branches, because there is always a branch corresponding to the match.

And a:

while (expr case var v?) {
  use(v);
}

would similarly restrict thescope to the body, whereas

while (!(expr case var v?)) {
  print("has no v");
  // do something.
}
use(v);

would be counted as not having a branch for the pattern matching, so the scope contines after the while.
(Generally, and if and a while should work the same way wrt. scope for their conditions.)

There is also #1420, because if you can introduce a local variable by doing ((expr case var v) ? v : throw 0), we might as well allow just writing (var v = expr) as an expression.
The scopes should agree with that as well.

@Reprevise
Copy link

Reprevise commented Feb 24, 2025

What about if (expr !case var v) instead of wrapping the whole thing in parentheses? (or case! to be more inline with is!)

@tatumizer
Copy link

tatumizer commented Feb 24, 2025

The question of whether it's in scope or not might be obvious to the compiler (due to specific scoping rules you introduced), but not to a human user. And all because of shadowing. I think dart has made a mistake already by allowing this:
Consider

main() {
  var v = 0;
  var w = 1;
  if (w case var v) {
    print(v);
  } else {
    print(v); // v, but a different v
    // different v can come from the global context or super (in the body of the method)
  }
}

It compiles and runs OK. The situation will become worse with negative cases. The negative "if-case" statement creates an optical illusion: visually, the variable is not in scope, but... it is. Might be a source of great confusion, thus increasing the "number of Bugs and Issues reaching the Repository". :-)

But now it's too late anyway. :-(

(Maybe there's no good solution here at all: if shadowing is not allowed, then adding the field in the superclass may break the sublcasses. But at least it would be a static error - easily fixable. The alternative (user's confusion about the meaning of v) is worse)

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