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

Safe navigation operator #89

Merged
merged 3 commits into from
Nov 15, 2021
Merged

Safe navigation operator #89

merged 3 commits into from
Nov 15, 2021

Conversation

SomeRanDev
Copy link
Contributor

Add safe access operators for nullable values.

@:nullSafety(Strict)
function add12(myArray: Null<Array<Int>>): Null<Int> {
    myArray?.push(12);
    return myArray?.length;
}

Rendered version

@skial skial mentioned this pull request May 17, 2021
1 task
@Aurel300
Copy link
Member

I do think this would make for a good addition, also combined with null coalescing (PR #85). For the index and call variants, ?.[ (resp. ?.() might be easier to parse.

@Simn
Copy link
Member

Simn commented May 17, 2021

The detailed design fails to design how exactly this would be implemented.

Also, there's still the open problem that cond?.1:.2 is valid syntax already.

@Aurel300
Copy link
Member

Aurel300 commented May 17, 2021

Yes, the detailed design should mention the AST changes at least. I guess the least intrusive change would be an optional flag nullSafe on EField, ECall, and EArray (false by default)?

Also is the cond?.1:.2 really a problem? ?. followed by a letter or underscore begins a null-safe field access, whereas ?. followed by a number is the start of a ternary and a shorthand float literal.

@Simn
Copy link
Member

Simn commented May 17, 2021

Also is the cond?.1:.2 really a problem? ?. followed by a letter or underscore begins a null-safe field access, wherease ?. followed by a number is the start of a ternary and a shorthand float literal.

Perhaps, but this is still something that has to be addressed in the proposal.

Anyway, my tentative opinion here is that ?. and ?[ are useful, but ?( is a step too far. I'll elaborate a bit later if necessary.

@ibilon
Copy link
Member

ibilon commented May 17, 2021

but ?( is a step too far.

I find it useful for optional callbacks.

@kaikoga
Copy link

kaikoga commented May 17, 2021

(nullableDynamicArrayContainsString?[0]:String) is valid Haxe syntax, given var String = [1]; and var nullableDynamicArrayContainsString = false;.

https://try.haxe.org/index.php#f251cdC9

@RblSb
Copy link
Member

RblSb commented May 17, 2021

Maybe instead of ?[ we can add get/set methods to Array after, like in Map, and use arr?.get(0) for such cases?

@Gama11
Copy link
Member

Gama11 commented May 17, 2021

@RblSb Then we'll suddenly have code style debates about which variant should be used for array access (like we already have for maps). :/

@SomeRanDev
Copy link
Contributor Author

(nullableDynamicArrayContainsString?[0]:String) is valid Haxe syntax, given var String = [1]; and var nullableDynamicArrayContainsString = false;.

Wow, in retrospect I'm very embarrassed I overlooked this. The same can also be applied to the safe function call. It now makes a lot more sense to me why JS uses jsFunction?.() or jsArray?.[0]. With Haxe having very similar syntax, I think it makes sense to shoot for that (assuming safe array-access/function call are even included)? I'll update the proposal.

Yes, the detailed design should mention the AST changes at least. I guess the least intrusive change would be an optional flag nullSafe on EField, ECall, and EArray (false by default)?

I'll go ahead and move it from "Impact on Existing Code" to the "Detailed Design" with a bit more specificity. Wasn't sure if optional flags were desired when it came to AST? Figured new EMaybeField/EMaybeCall/etc would be used, or an ESafeNav(e:Expr) that wraps around EField, ECall, or EArray, but I have no experience with designing Haxe AST, so I can't say myself.

Also, there's still the open problem that cond?.1:.2 is valid syntax already.

Will acknowledge in proposal as well.

Changed the proposed syntax of safe array-access and function call to match better with JavaScript's to help avoid conflict with array literals within ternary conditionals.

Also made additional changes to help discuss the modification of `ExprDef` and other clarifications.
@grepsuzette
Copy link

It is adding syntax for something which, as a whole, is like quicksands.

In the end it may encourage some people using null over things like haxe.ds.Option.
While I occasionnally like to use nullability I am in general not for creating those syntaxes.

@back2dos
Copy link
Member

  1. Option generally yields poorer performance (more work for the GC, slower access).
  2. Code is not written in a vacuum. When you load 3rd party data, there can be whole object trees that are nullable. Time is better spent on helping people with common situations beyond their control, than trying to police them into using one thing vs. another, in the hopes of preventing them from doing stupid things (which is a lost cause).
  3. When using null safety, there's nothing wrong with using Null. One could define map / flatMap / or in userland (as is common practice for Option). This is no different, except that it's less noisy, and it is built right into the language core - as is null itself.

@grepsuzette
Copy link

But that's what I dislike, it's like defining a Maybe monad implemented over null. Why the privilege?
Can we imagine they are available as well for something like haxe.ds.Option?

@Aurel300
Copy link
Member

It's an operator that will be given a default meaning, just like + has a default meaning for numeric types. If you want safe navigation operators for Option too, you can create OptionTools and (probably macro) methods to handle ?. there. Whether you like that Null is given "privilege" is irrelevant given how common it is in Haxe and even more so external libraries.

@grepsuzette
Copy link

Ok I see.

@sebthom
Copy link

sebthom commented Aug 19, 2021

For me the biggest benefit of a safe navigation operator would be to reduce the boilerplate when using @:nullSafety(StrictThreaded) in conjunction with class/instance variables. So we can avoid having to use temporary variables to satisfy null checks.

In the following case:

class Test {
  static var myArray:Null<Array<Int>>;
  
  static function main() {
    // without @:nullSafety
    myArray.push(1);

    // with @:nullSafety(Strict)
    if(myArray != null) myArray.push(1);

    // with @:nullSafety(StrictThreaded)
    final myArrayRef = myArray;
    if(myArrayRef != null) {
       myArrayRef.push(1);
    }
  }
}

So classVariable?.member() should be expanded to something like:

final classVariableTmp = classVariable;
if(classVariableTmp != null) {
   classVariableTmp.member();
}

when @:nullSafety(StrictThreaded) is enabled.

@Simn Simn added this to the 2021-12 milestone Nov 8, 2021
@Simn
Copy link
Member

Simn commented Nov 15, 2021

This proposal has been accepted, see https://haxe.org/blog/evolution-meeting-2021/ for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants