zed icon indicating copy to clipboard operation
zed copied to clipboard

Improve parser errors during op/func mix-up

Open philrz opened this issue 7 months ago • 0 comments

tl;dr

A user attempted to call operators from within a user-defined func, but the parse error produced was generic and did not point at the operator.

Details

Repro is with Zed commit 6b4afe5

In a community Slack thread a user reported successfully writing a user-defined op after having tried and failed to write it as a func.

The working op defined in switch-example.zed:

op lookup_key(): (
    switch (
      case parent.key=="foo" => yield "we got foo"
      case parent.key=="bar" => yield "we got bar"
      default => yield "¯\\_(ツ)_/¯" // escape char bug?
    )
)

over this | lookup_key()

Working as intended:

$ zq -version
Version: v1.16.0-12-g6b4afe50

$ echo '[{parent:{key:"foo"}},{parent:{key:"bar"}},{parent:{key:"baz"}}]' | zq -I switch-example.zed -
"¯\\_(ツ)_/¯"
"we got foo"
"we got bar"

If you replace op with func and repeat, the error that's produced:

$ echo '[{parent:{key:"foo"}},{parent:{key:"bar"}},{parent:{key:"baz"}}]' | zq -I switch-example-as-func.zed -
zq: error parsing Zed in switch-example-as-func.zed at line 3, column 12:
      case parent.key=="foo" => yield "we got foo"
       === ^ ===

In the words of the user:

ideally it’d tell me “Hey, you can’t call an operator inside a func” and point to switch

Indeed, only a handful of users have attempted to build user-defined operators and functions, but within that small population this is not the first time we've heard confusion over which language elements can be used within op and which within func. There's surely multiple ways we could go about improving here. A few that come to mind:

  1. Continue improving the docs. I do plan to chip away at this as time allows.

  2. There's been talk of working on "unifying dataflow and expressions". Depending on the result of that effort when it happens, maybe that would remove the confusion by combining user-defined operators and functions into one concept or having them remain as separate concepts but provide ways to invoke more language elements in each.

  3. Assuming the tech remains as it is now for some time, it would be great if the parser could better inform the user of why it failed. As the user suggested above, having it point at the operator (switch in this case) would have been good. If it could have said something specific about operators not being permitted in expression context, even better. If we willing to show a URL to a docs page, that might help too. Since the prior two bullets are being pursued elsewhere, this point is the essence of this issue.

This user happens to come from a heavy jq background, and explained folks like him might need this careful guidance. I said to him at one point in the chat:

We might need more coverage to contrast "dataflow context" (i.e., places where pipe-separated operators can appear) vs. "expression context"

And his reply:

I'd agree with this. Esp. coming from jq where the pipe operator there is pretty ubiquitous, and on first blush, it's pretty common in zq too ... so IMO that'll be a big callout to jq converts that you can't just pipe everything.

philrz avatar Jul 10 '24 22:07 philrz