Operator precedence not being respected

[lauracallahan-wk]

General info

• Dart 2.19.6 (stable) (Tue Mar 28 13:41:04 2023 +0000) on "macos_arm64"
• on macos / Version 15.6.1 (Build 24G90)
• locale is en-US
• Chrome

val: autoDismiss ?? shouldDoSomething ? state.autoDismiss : false

According to the operators documentation, an if-null should be considered higher priority than a ternary operation. However, in our app we were seeing that if autoDismiss was set to true, we were still setting val to false. shouldDoSomething is false in this scenario.

I would have expected this to be read as if autoDismiss is null, then evaluate the ternary, but it seems to be evaluating instead as autoDismiss(true) AND shouldDoSomething(false), then evaluate the ternary or just ignoring the if-null portion.

Adding parentheses resolves the issue and evaluates the way that I would expect.

val: autoDismiss ?? (shouldDoSomething ? state.autoDismiss : false)

Is this expected behavior?

[FMorschel]

If I understand that table correctly, yes, this is expected.

If you read that table, you have in this order:

• &&
• ||
• ??
• expr1 ? expr2 : expr3

And the same way you don't have to wrap an && inside your condition, you should not have to wrap your ?? for it to be part of your condition.

So it would actually read like:

val: (autoDismiss ?? shouldDoSomething) ? state.autoDismiss : false

---

Side note, there is an enhancement I did in two different lints that will be on Dart 3.10 that, with the fix, should help you see this (#60614 - no_literal_bool_comparisons and avoid_bool_literals_in_conditional_expressions). Since you have a false on your else, it should update your expression to (which would need the parentheses):

val: (autoDismiss ?? shouldDoSomething) && state.autoDismiss

[FMorschel]

@bwilkerson, just a note that this is not the first time confusion with the combination of these operators happens (see my PR to land the fix I said above flutter/packages#9409). Just so you can keep in mind, maybe we could have a way to identify this (precedence) better.

[lauracallahan-wk]

Interesting, thank you for the clarification @FMorschel!

[bwilkerson]

@DanTup Is this something we could reasonably help with by using inlay hints to inject parentheses into an expression in places where the precedence might not be clear? It doesn't really match the existing kinds, so probably not, but I wanted to ask anyway in case I'm wrong.

[DanTup]

It doesn't really match the existing kinds, so probably not

Yeah, this is the main problem. VS Code only recognises specific kinds, so all the settings and visuals are tied to those. I opened a VS Code issue about having custom kinds at microsoft/vscode#151920.

This is also what prevents us moving Closing Labels to them.

Of course, one option is to do what we do with Closing Labels - that is, use a custom API to get them to Dart-Code and then use the Decorations API in VS Code to inject them in the editor. The decorations API isn't available as native LSP but is visually more flexible (we control the CSS - within some parameters that are exposed - rather than just providing text/ranges to VS Code).

[DanTup]

Although, with decorations I'm not sure we could do "show them only while some keys are held down" like VS Code does with inlay hints.

[bwilkerson]

I don't think we always want them to be displayed, but it's interesting to know about Decorations as that might be useful for other cases.

So it sounds like there isn't a great path forward here, at least for now.

[FMorschel]

Something I'd like to note for anyone stumbling into this looking for a temporary solution (at least in VS Code), though I'm not 100% sure how this works, it seems to be consistent with precedence because it is probably using the target and parameters for each operator appearance. Hovering the operator seems to faintly highlight what it is using:

Repro:

var a = 1 + 2 + 3 * 3;
var b = foo() ?? 1 + 2;

int? foo() => null;

I've tested on IntelliJ, and it doesn't work there, unfortunately.

[bwilkerson]

What doesn't work?

[FMorschel]

Hovering the operator to faintly highlight the expression it provides. Or at least I couldn't find a way to do it.

[bwilkerson]

Ok. The legacy protocol doesn't have an equivalent range, so no, we can't do the same in IntelliJ today. I don't know whether IntelliJ supports an equivalent feature, but if so it would take some work in the IntelliJ plugin in order to be able to support it.

As a side-note, that highlight is way too subtle. If you hadn't pointed it out I would never have realized it was there.

[DanTup]

(at least in VS Code), though I'm not 100% sure how this works

In an LSP hover request, the response has a range in it. I think the main reason for it is so the editor knows a) that it doesn't need to send more hover request as your mouse moves around within that range and b) where the mouse needs to move to for it to close the hover. Hovers that flicker if you move the house around still on the same symbol look a bit naff :-)

I agree with Brian that the visual is too subtle (at least in this theme) and is not helped as-is. It can be customised like this:

"workbench.colorCustomizations": {
	"editor.hoverHighlightBackground": "#264f78fe", // default for Dark+ theme is #264f7840 so I increased the alpha to fe
}

There's also an "Expand Selection" command that you can use to increase the selection (Shift+Ctrl+Right) based on the AST (it just moves up the nodes), but it's a also a bit fiddly as a way to try and see this.

[bwilkerson]

a) that it doesn't need to send more hover request as your mouse moves around within that range

That's the same as for the legacy protocol for IntelliJ.

It also means that we don't want this range to include the operands because moving to hover over the operands should change the hover content to show information about the operands.

There's also an "Expand Selection" command ...

The same exists in IntelliJ, though the behavior is probably inconsistent in a few places because they are implemented differently.