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

Fix MemberExpression and add MethodCalls #16

Merged
merged 3 commits into from
Oct 8, 2024
Merged

Fix MemberExpression and add MethodCalls #16

merged 3 commits into from
Oct 8, 2024

Conversation

jonsmock
Copy link
Collaborator

@jonsmock jonsmock commented Sep 24, 2024

Builds on #15, only last 3 commits are new.

Fixes a bug in previous member access to allow things like fromJSON(...).foo. Previously only foo.a.b.c would work, but you could not access members on values returned by functions.

Adds MethodCall to the ast to support methods like x.toString() and x.count().

Adds some additional functions for strings and conversions.

@jonsmock jonsmock requested a review from kanaka September 25, 2024 14:31
@jonsmock jonsmock mentioned this pull request Oct 7, 2024
@jonsmock jonsmock requested a review from gdw2 October 7, 2024 19:49
Copy link
Collaborator

@kanaka kanaka left a comment

Choose a reason for hiding this comment

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

I think the functions fromJSON, toJSON, contains, etc, are used before the commit where they are defined.

Otherwise, looks good.

Previously MemberExpression prohibited something like `foo(env).b.c`,
because `foo(env)` is a FunctionCall, not a ContextName.

We now require that MemberExpression contains at least one Property
access. In order to match `foo(env).b.c` correctly, we must move
MemberExpression to the first matching position of UnaryExpression;
otherwise, FunctionCall will match `foo(env)`, followed by an end of
input error when attempting to match `.b.c`.

Choosing for now to allow member access on Value, but that is up for
debate (ex: `{"a": 3}.a`).
Initial method call support with the first supported methods including
`toString` and `count`.

MemberExpression supports both MethodCall and Property, so that we can
greedily parse expressions like `x.foo().bar.baz().z`. If we attempt to
separate methods and properties into separate expression types, the
MemberExpression will attempt to capture `foo` and `baz` as member
accesses, and we run into end of input errors for the method calls.

MemberExpression evaluation needs to also evaluate its MethodCall
children, so we can pass the referenced object of the MethodCall. This
works but has the drawback of needing to check for uncaught errors again
inside the MemberExpression evaluation.
Upgrade cljs-bean when using shadow-cljs to get :keywordize-keys
functionality.
@jonsmock
Copy link
Collaborator Author

jonsmock commented Oct 8, 2024

Gah, thanks! I've moved the offending examples/tests to the last commit, where those functions are introduced (and fixed some typos in the commit messages).

@jonsmock jonsmock merged commit 11e551d into main Oct 8, 2024
4 checks passed
@jonsmock jonsmock deleted the more-functions branch October 8, 2024 14:18
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.

2 participants