-
Notifications
You must be signed in to change notification settings - Fork 38
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
Implement the Starlark index expression assignment operation #497
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Iapetus999
reviewed
May 6, 2024
larky/src/main/java/com/verygood/security/larky/modules/types/LarkyMapping.java
Show resolved
Hide resolved
Iapetus999
reviewed
May 6, 2024
larky/src/main/java/com/verygood/security/larky/modules/types/LarkyMapping.java
Show resolved
Hide resolved
Iapetus999
reviewed
May 6, 2024
larky/src/main/java/com/verygood/security/larky/modules/types/LarkyMapping.java
Show resolved
Hide resolved
…throwing an exception. The error message is: Error: can only assign an element in a dictionary or a list, not in a 'MutableStruct'. Let's make the tests pass!
StarlarkIndexable. We follow the exact same interface and modifications for StarlarkIndexable and update the error message to signify that we now support setIndex on dictionaries, lists, and any implementation of StarlarkSetIndexable. We also support the equivalent StarlarkIndexable.Threaded interface for StarlarkSetIndexable, similary called StarlarkSetIndexable.Threaded.
…arlarkSetIndexable(.Threaded) interface
…ll the exact same interface with LarkyMapping. Delete StarlarkMapping and move the functionality to LarkyMapping
…dict) for cardinality checks
…lasses to primitives in larky (as we would do in python). as a result, we have to return a raw list on the view instead of an actual view
mahmoudimus
force-pushed
the
allow-set-indexable
branch
from
May 6, 2024 23:02
409c1ee
to
7ddfac1
Compare
sharifelgamal
approved these changes
May 7, 2024
mahmoudimus
added a commit
that referenced
this pull request
May 9, 2024
…ze() and updateIteratorCount.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes in release / Impact of release:
In a nutshell, this PR allows one to set an object's value using
object[key] = value
.Starlark's specification actually supports this, but the Java implementation does not, even though the Go and Rust implementation do.
This is an initial implementation attempt to do so. In addition, the failing tests are now passing to ensure that this behavior is additive and enhancing while maintaining backward compatibility. A nice side effect of this PR is that it also addresses #292.
Documentation
Read the
StarlarkSetIndexable
class and observe the implementation inLarkyIndexable
Risks of this release
Is this a breaking change?
Is there a way to disable the change?