Skip to content
This repository has been archived by the owner on Jan 28, 2023. It is now read-only.

?? has poor associativity/precedence, leading to counterintuitive behavior #26

Closed
waldemarhorwat opened this issue Mar 14, 2018 · 22 comments

Comments

@waldemarhorwat
Copy link

When faced with a long short-circuiting expression such as

a || b || c || d || e

I usually think of it as selecting the first one of a, b, c, d, e that's not falsy and not evaluating the rest. In effect, I'm viewing the expression as though it were right-associative:

a || (b || (c || (d || e)))

That works fine. The spec grammar happens to actually specify it as left-associative:

(((a || b) || c) || d) || e

However, that associativity is merely a spec-writing artifact, invisible to users. The spec could have associated the other way without any visible changes to existing user code behavior.

Unfortunately, introducing ?? at the same precedence level as || makes the spec associativity visible in the language and leads to problematic consequences:

0 ?? null || 7

Intuitively, if ?? and || have the same precedence level, I'd expect this to short-circuit starting at the 0 and produce the result 0. However, as specified, this will produce 7 instead.

There are three possible solutions:

  • Switch to using right-associativity for the logical operators.
  • Give ?? its own precedence level
  • Forbid mixing ?? with || without parenthesizing one
@ljharb
Copy link
Member

ljharb commented Mar 14, 2018

0 ?? null returns 0, and i'd expect 0 ?? null || 7 to return the same as 0 || 7, which is 7. In other words, I think I'd intuitively expect it to be left-associative (if I'm reading your parens example right).

In other words, in a chain like that, with no parens, I'd assume ?? and || and && would all be left-to-right, and evaluated at the same level.

@littledan
Copy link
Member

@waldemarhorwat The precedence of ?? was designed specifically to be the same as || in order to keep behavior the same when any upgrade from || to ?? happens. However, you make a decent case for giving ?? a precedence lower than || (I'm not sure why we'd go with the other options).

@ljharb What gives you that intuition?

@ljharb
Copy link
Member

ljharb commented Mar 14, 2018

I see all three of these operators as value-selection operators, and see the short-circuiting behavior as just something that falls out of that.?? is identical to || for me, conceptually, except for the criteria that applies: a || b is a ? a : b and a ?? b is a == null ? a : b.

@littledan
Copy link
Member

@ljharb Are you aware that && associates more tightly than ||?

@ljharb
Copy link
Member

ljharb commented Mar 14, 2018

Somewhere in the back of my mind :-) but I use linting rules to force parens when && and || are mixed.

So perhaps it'd be clearer to not talk about && at all - I'm still not clear on the reason why || and ?? can't have identical precedence?

@waldemarhorwat
Copy link
Author

Yeah, giving ?? a lower precedence than || would solve this problem and avoid the sticky associativity questions.

@ljharb
Copy link
Member

ljharb commented Mar 15, 2018

@waldemarhorwat would that mean that a ?? b || c would be like a ?? (b || c) or (a ?? b) || c? If the latter, then it sounds great to me!

@littledan
Copy link
Member

littledan commented Mar 15, 2018

If ?? has lower precedence than ||, then the semantics of a ?? b || c would be a ?? (b || c)

(edited, as the last patch had some big typo in it)

@jridgewell
Copy link
Member

This was raised in #15, which some good points in it.

@zenparsing
Copy link
Member

zenparsing commented Dec 21, 2018

Thanks @waldemarhorwat. We should follow C# here and give ?? a lower precedence.

https://github.com/dotnet/csharplang/blob/master/spec/expressions.md#the-null-coalescing-operator

@dantman
Copy link

dantman commented Dec 21, 2018

If I remember correctly, C#'s || always returns a boolean. It's not like JS' ||. So I would caution against using C#'s behaviour itself as a rationale.

There are some other practical concerns I think are worth considering.

I was going to suggest that the foo ?? 'value' || 'not-value' format is not very likely in code. Because bar = (foo ?? null) || 'value', bar = (foo ?? 0) || 'value', and bar = foo || 'value' all do the exact same thing (parenthesis included only for understandability of this comment). And bar = (foo ?? 'a') || 'b' is a very cryptic and nonsensical "if foo is null/undefined then bar is 'a', if foo is false/0/'' then bar is 'b', if foo is truthy then bar is foo".

And I was going to suggest that it's more important we make sure that a || b ?? c makes sense. Because it's likely to drop a opts.someoption ?? 'defaultvalue' into a || like somethingLocalAndVerySpecific || opts.somebool ?? true (read as "somethingLocalAndVerySpecific OR some boolean option which is true when not defined in the options).

However, I am wrong about the likelyhood of a ?? b || c, I can see it being used shorthand in libraries that don't like merging options objects like:

if (opts.somebool ?? true || someSituationThatForcesThisPartOfTheLibraryToRunUnconditionally ) {
  // Do something that can be disabled sometimes by passing {somebool: false} to the options
}

I also want to double check that the result of a ?? b ?? c makes sense. Because I do think that it may be used in a similar way to || chains.

i.e. I can see libraries writing code like this:

const someoption = opts.someoption ?? defaults.someoption ?? 0;`

Read as "someoption is equal to someoption from the options passed in to the function, but if not defined falls back to the library wide default values the user specified, but if that library wide default was not supplied by the user is 0".

@hax
Copy link
Member

hax commented Dec 24, 2018

I still feel a || b ?? c, a ?? b || c etc. are confusing anyway and harmful for code review, in real engineering, parens should be enforced. The only problem is whether we follow -1 ** x to enforce it in language, or leave it to linters. I would vote for the former.

@rkirsling
Copy link
Member

rkirsling commented Jun 22, 2019

Adding as much cross-language data here as I can find:

Precedence Languages
Lower than ||, higher than ternary C# (??), Dart (??), PHP (??), CoffeeScript (?)
Same as ternary (i.e. consequent is optional) GNU C (?:), Groovy (?:)
Higher than not only && but also comparisons Swift (??), Kotlin (?:)
Same as || Perl (//)

@hax
Copy link
Member

hax commented Jul 10, 2019

@rkirsling What #38 mean for this issue? Will we change the precedence?

@rkirsling
Copy link
Member

rkirsling commented Jul 10, 2019

@hax I'm not the PR author, but hopefully @DanielRosenwasser's latest reply over there helps. 😄

From my personal perspective, I really believe that C# et al. are doing the right thing.
Concretely, I consider it perfectly reasonable to expect code like this to work...

const isValid = options.isValid ?? hasValidFoo() && hasValidBar();

...and I would hope that it would work equally well if the condition were based on || instead of &&.

@DanielRosenwasser
Copy link
Member

I agree with @rkirsling in that if there are no restrictions, ?? should have lower precedence as in C#, Dart, and PHP. #38 allows us to change the precedence at a later time if we choose to.

I will bring up lowering the precedence at the next meeting to gauge whether there is strong opposition to doing so, but based on feedback from the last meeting, and in the interest of progress on the feature, adding an early error appears to be the best way forward.

@hax
Copy link
Member

hax commented Jul 15, 2019

Thank you @DanielRosenwasser and @rkirsling for explaining! And I really appreciate the #39 instead of #38 which also rule out &&.


About the code example

const isValid = options.alreadyValidated ?? hasValidFoo() && hasValidBar();

The name alreadyValidated implies boolean and I would guess ?? here is a possible wrong usage if I'm doing code review. I always suspect whether there is good use case of combination of ?? and &&. Maybe @rkirsling could find a better example.

@hax
Copy link
Member

hax commented Jul 15, 2019

@rkirsling It seems coffeescript is same as C#. (?? is corresponding BIN? in coffee grammar)

@hax
Copy link
Member

hax commented Jul 15, 2019

I will bring up lowering the precedence at the next meeting to gauge whether there is strong opposition to doing so,

Personally I kind of like Swift way, or strictly speaking, even more aggressive than Swift, I expect the precedence of ?? should higher than **.

Let me explain the idea:

All binary expressions from x ** y to x | y never output nullish value (arith ops output number/NaN, bitwise ops output int32, relationship ops output boolean/TypeError), for any op have higher precedence, the common form x [op] y ?? z is meaningless or error as missing parens (x [op] (y ?? z)).

So in real engineering, we need to introduce a linter rule to disallow x [op] y ?? z, (or even make all x [op] y ?? z SyntaxError though it sounds a little crazy,) and hope programmers won't make mistake anymore. But ?? is not symmetrical, x ?? y [op] z is totally valid and programmers need extra effort to remember which side is allowed and still the precedence.

To summary the idea of ?? higher than **:

Pros:

  • x [op] y ?? z work as it could be, no extra parens, no extra linter rule (nor crazy SyntaxError) needed
  • More consistent between x [op] y ?? z with x ?? y [op] z
  • Easy to remember precedence (the highest among all normal binary operators)

Cons:

  • Inconsistent with other programming language (but they already diverge)

@rkirsling
Copy link
Member

rkirsling commented Jul 15, 2019

About the code example

const isValid = options.alreadyValidated ?? hasValidFoo() && hasValidBar();

The name alreadyValidated implies boolean and I would guess ?? here is a possible wrong usage if I'm doing code review. I always suspect whether there is good use case of combination of ?? and &&. Maybe @rkirsling could find a better example.

Derp, guess I tried to get too clever with the name and made it imply the wrong thing. What's there is correct if options.alreadyValidated is only ever true or nullish, but what I really wanted is something that distinguishes false from nullish; i.e., it should just be options.isValid. Fixed now, thanks.

It's difficult to think of a real-world example for x && y ?? z (or x || y ?? z), but I think this is an excellent one for x ?? y && z (or x ?? y || z).

@rkirsling It seems coffeescript is same as C#. (?? is corresponding BIN? in coffee grammar)

Whoa, you're right! I was looking at the unary postfix ? operator. Thanks for pointing that out—I've updated the table accordingly.

@kaizhu256
Copy link

i'm currently against ?? being lower-precedence than ||. javascript is not c++, and determining whether a value is null or undefined is more "atomic" than determining whether its null, undefined, false, 0, 0n, ""

imagine reviewing/debugging integration-code, and you stumble across the logic aa ?? bb || cc to handle malformed data. which scenario do you think is more likely:

  1. first check aa is really "null" and then proceed with logical operator ||
  2. first perform logical operator || and then check to see if result is really "null"

the code's intent is likely option 1. option 2 is pathological/bad-programming, in terms of data-handling.

@hax
Copy link
Member

hax commented Jan 22, 2020

Already stage 4, this issue could be closed.

@ljharb ljharb closed this as completed Jan 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants