Skip to content

Commit

Permalink
Merge pull request #2069 from opal/elia/fix-delete-if-keep-if
Browse files Browse the repository at this point in the history 
Fix Array keep_if, delete_if, reject!, select!
  • Loading branch information
@elia
elia committed Jan 29, 2020
2 parents 310339b + 1d6d262 commit 0e18ef7
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 63 deletions.
4 changes: 4 additions & 0 deletions UNRELEASED.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ Whitespace conventions:

### Fixed

- Array#delete_if (#2069)
- Array#keep_if (#2069)
- Array#reject! (#2069)
- Array#select! (#2069)
- Struct#dup (#1995)
- Integer#gcdlcm (#1972)
- Enumerable#to_h (#1979)
Expand Down
122 changes: 63 additions & 59 deletions opal/corelib/array.rb
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# helpers: truthy, falsy, hash_ids, yield1, hash_get, hash_put, hash_delete

require 'corelib/enumerable'
require 'corelib/numeric'

Expand All @@ -15,6 +17,34 @@ class Array < `Array`
return klass.$allocate().$replace(#{`obj`.to_a});
}
}
// A helper for keep_if and delete_if, filter is either Opal.truthy
// or Opal.falsy.
function filterIf(self, filter, block) {
var value, raised = null, updated = new Array(self.length);
for (var i = 0, i2 = 0, length = self.length; i < length; i++) {
if (!raised) {
try {
value = $yield1(block, self[i])
} catch(error) {
raised = error;
}
}
if (raised || filter(value)) {
updated[i2] = self[i]
i2 += 1;
}
}
if (i2 !== i) {
self.splice.apply(self, [0, updated.length].concat(updated));
self.splice(i2, updated.length);
}
if (raised) throw raised;
}
}

def self.[](*objects)
Expand Down Expand Up @@ -90,12 +120,12 @@ def &(other)
var result = [], hash = #{{}}, i, length, item;
for (i = 0, length = other.length; i < length; i++) {
Opal.hash_put(hash, other[i], true);
$hash_put(hash, other[i], true);
}
for (i = 0, length = self.length; i < length; i++) {
item = self[i];
if (Opal.hash_delete(hash, item) !== undefined) {
if ($hash_delete(hash, item) !== undefined) {
result.push(item);
}
}
Expand All @@ -115,11 +145,11 @@ def |(other)
var hash = #{{}}, i, length, item;
for (i = 0, length = self.length; i < length; i++) {
Opal.hash_put(hash, self[i], true);
$hash_put(hash, self[i], true);
}
for (i = 0, length = other.length; i < length; i++) {
Opal.hash_put(hash, other[i], true);
$hash_put(hash, other[i], true);
}
return hash.$keys();
Expand Down Expand Up @@ -171,12 +201,12 @@ def -(other)
var result = [], hash = #{{}}, i, length, item;
for (i = 0, length = other.length; i < length; i++) {
Opal.hash_put(hash, other[i], true);
$hash_put(hash, other[i], true);
}
for (i = 0, length = self.length; i < length; i++) {
item = self[i];
if (Opal.hash_get(hash, item) === undefined) {
if ($hash_get(hash, item) === undefined) {
result.push(item);
}
}
Expand Down Expand Up @@ -507,7 +537,7 @@ def bsearch_index(&block)
while (min < max) {
mid = min + Math.floor((max - min) / 2);
val = self[mid];
ret = Opal.yield1(block, val);
ret = $yield1(block, val);
if (ret === true) {
satisfied = mid;
Expand Down Expand Up @@ -565,7 +595,7 @@ def cycle(n = nil, &block)
if (n === nil) {
while (true) {
for (i = 0, length = self.length; i < length; i++) {
value = Opal.yield1(block, self[i]);
value = $yield1(block, self[i]);
}
}
}
Expand All @@ -577,7 +607,7 @@ def cycle(n = nil, &block)
while (n > 0) {
for (i = 0, length = self.length; i < length; i++) {
value = Opal.yield1(block, self[i]);
value = $yield1(block, self[i]);
}
n--;
Expand Down Expand Up @@ -613,7 +643,7 @@ def collect(&block)
var result = [];
for (var i = 0, length = self.length; i < length; i++) {
var value = Opal.yield1(block, self[i]);
var value = $yield1(block, self[i]);
result.push(value);
}
Expand All @@ -626,7 +656,7 @@ def collect!(&block)

%x{
for (var i = 0, length = self.length; i < length; i++) {
var value = Opal.yield1(block, self[i]);
var value = $yield1(block, self[i]);
self[i] = value;
}
}
Expand Down Expand Up @@ -826,20 +856,7 @@ def delete_at(index)

def delete_if(&block)
return enum_for(:delete_if) { size } unless block_given?

%x{
for (var i = 0, length = self.length, value; i < length; i++) {
value = block(self[i]);
if (value !== false && value !== nil) {
self.splice(i, 1);
length--;
i--;
}
}
}

%x{filterIf(self, $falsy, block)}
self
end

Expand Down Expand Up @@ -887,7 +904,7 @@ def each(&block)

%x{
for (var i = 0, length = self.length; i < length; i++) {
var value = Opal.yield1(block, self[i]);
var value = $yield1(block, self[i]);
}
}

Expand All @@ -899,7 +916,7 @@ def each_index(&block)

%x{
for (var i = 0, length = self.length; i < length; i++) {
var value = Opal.yield1(block, i);
var value = $yield1(block, i);
}
}

Expand Down Expand Up @@ -1166,29 +1183,29 @@ def flatten!(level = undefined)

def hash
%x{
var top = (Opal.hash_ids === undefined),
var top = ($hash_ids === undefined),
result = ['A'],
hash_id = self.$object_id(),
item, i, key;
try {
if (top) {
Opal.hash_ids = Object.create(null);
$hash_ids = Object.create(null);
}
// return early for recursive structures
if (Opal.hash_ids[hash_id]) {
if ($hash_ids[hash_id]) {
return 'self';
}
for (key in Opal.hash_ids) {
item = Opal.hash_ids[key];
for (key in $hash_ids) {
item = $hash_ids[key];
if (#{eql?(`item`)}) {
return 'self';
}
}
Opal.hash_ids[hash_id] = self;
$hash_ids[hash_id] = self;
for (i = 0; i < self.length; i++) {
item = self[i];
Expand All @@ -1198,7 +1215,7 @@ def hash
return result.join(',');
} finally {
if (top) {
Opal.hash_ids = undefined;
$hash_ids = undefined;
}
}
}
Expand Down Expand Up @@ -1355,20 +1372,7 @@ def join(sep = nil)

def keep_if(&block)
return enum_for(:keep_if) { size } unless block_given?

%x{
for (var i = 0, length = self.length, value; i < length; i++) {
value = block(self[i]);
if (value === false || value === nil) {
self.splice(i, 1);
length--;
i--;
}
}
}

%x{filterIf(self, $truthy, block)}
self
end

Expand Down Expand Up @@ -1471,7 +1475,7 @@ def permutation(num = undefined, &block)
for (var j = 0; j < perm.length; j++) {
output.push(self[perm[j]]);
}
Opal.yield1(blk, output);
$yield1(blk, output);
}
}
}
Expand Down Expand Up @@ -1877,7 +1881,7 @@ def select(&block)
for (var i = 0, length = self.length, item, value; i < length; i++) {
item = self[i];
value = Opal.yield1(block, item);
value = $yield1(block, item);
if (Opal.truthy(value)) {
result.push(item);
Expand Down Expand Up @@ -2147,7 +2151,7 @@ def to_h
}
key = ary[0];
val = ary[1];
Opal.hash_put(hash, key, val);
$hash_put(hash, key, val);
}
return hash;
Expand Down Expand Up @@ -2191,17 +2195,17 @@ def uniq(&block)
if (block === nil) {
for (i = 0, length = self.length; i < length; i++) {
item = self[i];
if (Opal.hash_get(hash, item) === undefined) {
Opal.hash_put(hash, item, item);
if ($hash_get(hash, item) === undefined) {
$hash_put(hash, item, item);
}
}
}
else {
for (i = 0, length = self.length; i < length; i++) {
item = self[i];
key = Opal.yield1(block, item);
if (Opal.hash_get(hash, key) === undefined) {
Opal.hash_put(hash, key, item);
key = $yield1(block, item);
if ($hash_get(hash, key) === undefined) {
$hash_put(hash, key, item);
}
}
}
Expand All @@ -2216,10 +2220,10 @@ def uniq!(&block)
for (i = 0, length = original_length; i < length; i++) {
item = self[i];
key = (block === nil ? item : Opal.yield1(block, item));
key = (block === nil ? item : $yield1(block, item));
if (Opal.hash_get(hash, key) === undefined) {
Opal.hash_put(hash, key, item);
if ($hash_get(hash, key) === undefined) {
$hash_put(hash, key, item);
continue;
}
Expand Down
4 changes: 0 additions & 4 deletions spec/filters/bugs/array.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,17 @@
opal_filter "Array" do
fails "Array#== compares with an equivalent Array-like object using #to_ary" # Expected false to be true
fails "Array#== returns true for [NaN] == [NaN] because Array#== first checks with #equal? and NaN.equal?(NaN) is true" # Expected [NaN] to equal [NaN]
fails "Array#delete_if updates the receiver after all blocks"
fails "Array#each does not yield elements deleted from the end of the array" # Expected [2, 3, nil] to equal [2, 3]
fails "Array#each yields elements added to the end of the array by the block" # Expected [2] to equal [2, 0, 0]
fails "Array#flatten does not call #to_ary on elements beyond the given level"
fails "Array#flatten performs respond_to? and method_missing-aware checks when coercing elements to array"
fails "Array#flatten with a non-Array object in the Array calls #method_missing if defined"
fails "Array#inspect does not call #to_str on the object returned from #to_s when it is not a String" # Exception: Cannot convert object to primitive value
fails "Array#join raises a NoMethodError if an element does not respond to #to_str, #to_ary, or #to_s"
fails "Array#keep_if updates the receiver after all blocks"
fails "Array#partition returns in the left array values for which the block evaluates to true"
fails "Array#rassoc calls elem == obj on the second element of each contained array"
fails "Array#rassoc does not check the last element in each contained but specifically the second" # Expected [1, "foobar", #<MockObject:0x4ef6e>] to equal [2, #<MockObject:0x4ef6e>, 1]
fails "Array#reject! updates the receiver after all blocks"
fails "Array#select returns a new array of elements for which block is true"
fails "Array#select! updates the receiver after all blocks"
fails "Array#to_s does not call #to_str on the object returned from #to_s when it is not a String" # Exception: Cannot convert object to primitive value
fails "Array#uniq! properly handles recursive arrays"
fails "Array#zip fills nil when the given enumerator is shorter than self" # LocalJumpError: no block given
Expand Down

0 comments on commit 0e18ef7

Please sign in to comment.