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

Cover function expressions as function parameters #53

Merged

Conversation

jg-rp
Copy link
Contributor

@jg-rp jg-rp commented Dec 20, 2023

These additional test cases cover using function expressions as function parameters, plus an additional length() case mentioned in the spec.

Note that it's not currently possible to test count() and value() with function expressions as parameters using standard function extensions alone. None of the standard function extensions return a NodesType.

This PR partially replaces #51. See also the conversation on PR #51.

Copy link
Collaborator

@f3ath f3ath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this addition. @gregsdennis care to merge?

Copy link
Collaborator

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look good, but I'd like to see some examples where value() returns Nothing (the nodelist is either empty or contains multiple elements).

For example, with this data

{
  "c": "cd",
  "values": [
    {
      "a": "ab"
    },
    {
      "c": "d"
    },
    {
      "a": null
    }
  ]
}

value(@.a) returns Nothing for {"c": "d"} and null for {"a": null"}. length() should then return Nothing for both of these values.

value($..c) returns Nothing because there are multiple matching locations.

Finally, Nothing == Nothing returns true, meaning that the last two elements are returned in the final result:

[
  {
    "c": "d"
  },
  {
    "a": null
  }
]

@gregsdennis
Copy link
Collaborator

@glyn can you verify my logic ☝️ please? This is what my lib currently does.

@glyn
Copy link
Contributor

glyn commented Dec 27, 2023

@glyn can you verify my logic ☝️ please? This is what my lib currently does.

LGTM

@jg-rp
Copy link
Contributor Author

jg-rp commented Dec 27, 2023

These look good, but I'd like to see some examples where value() returns Nothing (the nodelist is either empty or contains multiple elements).

Hi @gregsdennis, I've added a couple more cases which I believe cover the behaviour demonstrated by your example.

cts.json Outdated Show resolved Hide resolved
@jg-rp jg-rp force-pushed the functions-as-function-params branch from f0aee1d to d554778 Compare December 27, 2023 20:22
Copy link
Collaborator

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These work for me.

@gregsdennis gregsdennis merged commit 446336c into jsonpath-standard:main Dec 27, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants