tc39 / proposal-nullish-coalescing

Nullish coalescing proposal x ?? y
https://tc39.github.io/proposal-nullish-coalescing/
1.23k stars 23 forks source link

Add grammar flags to disallow mixing ?? with || and &&. #39

Closed DanielRosenwasser closed 5 years ago

DanielRosenwasser commented 5 years ago

Alternative to #38. Instead of using early errors, this change disallows combining the nullish coalescing operator (??) with both the logical AND (&&) and logical OR (||) operators directly through the grammar.

DanielRosenwasser commented 5 years ago

@codehag @rkirsling @jridgewell

DanielRosenwasser commented 5 years ago

The current changes were adapted from https://gist.github.com/bakkot/332ddeb26978c41613abc092120448b9, but I think there might have been a mistake in that grammar. Should it be the following?

 NullishExpression[Nullish]:
   LogicalORExpression[?Nullish]
-  LogicalORExpression[+Nullish] ?? NullishExpression[+Nullish]
+  NullishExpression[+Nullish] ?? LogicalORExpression[+Nullish]
rkirsling commented 5 years ago

(Note: This is spill-over from #38. Sorry in advance for the full-throttle teacher mode. 😅)


I believe this change is motivated by a view of the differing precedence of || and && as arbitrary, but I want to encourage us to remember why this isn't so (at least, not any more than PEMDAS itself is :smile:).

|| is an additive operation while && is a multiplicative operation—they act on true and false in the same way as + and * act on 0 and 1 (as long as we clamp anything greater than 1 to be just 1).

So we've got stuff like:

But more importantly, we have short-circuiting:

And thankfully, that fact doesn't become less useful when we're dealing with truthiness—that's why folks have been rightfully able to write stuff like the following up 'til now:

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

I understand that people make mistakes and that bugs can be costly. It's important that we have lint rules and even compiler warnings for such things. But I don't think a && b || c is more deserving to be a syntax error than a * b + c, and I think this same rationale applies to a && b ?? c.

ljharb commented 5 years ago

There is a significant difference in that nearly everyone is taught some form of PEMDAS in school, but many people have never seen && or || prior to programming - it’s why the airbnb style guide requires parens when mixing && and ||, but not when mixing * + / -.

claudepache commented 5 years ago

Note that, if we do want to disallow mixing ?? and other operators without the use of parentheses, we could very well include everything between (in term of precedence level) comparison operators (<=, ==, etc.) and logical operators (&&, ||) , since some languages has found more convenient to give ?? a precedence level higher than comparisons (per https://github.com/tc39/proposal-nullish-coalescing/issues/26#issuecomment-504699520).

Anyway, introducing such rules at the language level is not common. (Not that I am against that.)

PR #40 (giving ?? a precedence level just below ||) solves the problem raised in #26 in a more traditional way, while respecting what I believe to be the original intent of giving ?? approximately the same precedence level as ||, namely: Code patterns using a || b, where a is guaranteed to never be falsy-but-not-nullish, could be safely upgraded to a ?? b, that is to say, without being trapped by the differing precedence level of other surrounding operators.

bakkot commented 5 years ago

@rkirsling beyond what @ljharb says about || not being common in elementary school, it's also the case that

|| is an additive operation while && is a multiplicative operation

is a fact not frequently not taught to or understood by even to people who have university degrees in computer science (who themselves not that large a fraction of JS programmers).

rkirsling commented 5 years ago

@bakkot That is fair, and I'm not in the least suggesting that people "should know better" or something (though I do get excited to share it with others 'cause I think it's really nifty :joy:); I only mean to argue that syntax errors are not something to impose lightly.

(Now, if we wanted to introduce syntax warnings into the spec... :wink:)

@ljharb I actually don't take your comment as a point of disagreement—I've been an enthusiastic advocate of the Airbnb style guide at my workplace for years. :smile:

DanielRosenwasser commented 5 years ago

I've included updates to uses or LogicalORExpression and ConditionalExpression, and included NullishExpression in the appropriate places. The spec diff isn't as tiny as it was before, but by-and-large it's not too bad when rendered.

DanielRosenwasser commented 5 years ago

Hey all, I've removed the redundant clauses now that spec.html reflects additions/removals from the actual spec text. Please give it a final look, as I'll be merging the changes in later tonight.

hax commented 5 years ago

About associativity: https://gist.github.com/bakkot/332ddeb26978c41613abc092120448b9#gistcomment-3071704