[Merged by Bors] - Safe wrapper for JsSet
#2162
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2162 +/- ##
==========================================
- Coverage 42.32% 42.14% -0.19%
==========================================
Files 228 230 +2
Lines 21047 21232 +185
==========================================
+ Hits 8909 8949 +40
- Misses 12138 12283 +145
Continue to review full report at Codecov.
|
jedel1043
added
enhancement
|
Ok, with #2115 we now have a simple API to handle JS iterators. Maybe it would be good if you checked out the PR and the corresponding implementation of |
boa_engine/src/builtins/set/mod.rs
Outdated
| @@ -164,7 +164,8 @@ impl Set { | |||
| Ok(set.into()) | |||
| } | |||
|
|
|||
| /// Utility for constructing `Set` Objects. | |||
| /// Utility for constructing `Set` objects. | |||
| #[allow(clippy::unnecessary_wraps)] | |||
There was a problem hiding this comment.
What is the reason for allowing this lint?
I think it would be better to return only a JsObject as the lint recommends
There was a problem hiding this comment.
Because JsArray does it too (internal_methods::ordinary_define_own_property), I did the same to maintain some sort of consistency.
There was a problem hiding this comment.
It's good to maintain consistency when it doesn't affect the usability of our program. However, this change impacts the usability, because now you need to handle an error on a function that cannot error.
There was a problem hiding this comment.
Alright, I understand.
|
bors r+ |
This PR adds a safe wrapper around JavaScript `JsSet` from `builtins::set`, and is being tracked at #2098. Implements following methods - [x] `Set.prototype.size` - [x] `Set.prototype.add(value)` - [x] `Set.prototype.clear()` - [x] `Set.prototype.delete(value)` - [x] `Set.prototype.has(value)` - [x] `Set.prototype.forEach(callbackFn[, thisArg])` Implement wrapper for `builtins::set_iterator`, to be used by following. - [x] `Set.prototype.values()` - [x] `Set.prototype.keys()` - [x] `Set.prototype.entries()` *Note: Are there any other functions that should be added? Also adds `set_create()` and made `get_size()` public in `builtins::set`.
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
JsSetJsSet
This PR adds a safe wrapper around JavaScript
JsSetfrombuiltins::set, and is being tracked at #2098.Implements following methods
Set.prototype.sizeSet.prototype.add(value)Set.prototype.clear()Set.prototype.delete(value)Set.prototype.has(value)Set.prototype.forEach(callbackFn[, thisArg])Implement wrapper for
builtins::set_iterator, to be used by following.Set.prototype.values()Set.prototype.keys()Set.prototype.entries()*Note: Are there any other functions that should be added?
Also adds
set_create()and madeget_size()public inbuiltins::set.