#4555 active_support/core_ext/array/random_access.rb conflicts with standard Ruby library - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4555 ✓resolved

active_support/core_ext/array/random_access.rb conflicts with standard Ruby library

Reported by djbob | May 8th, 2010 @ 08:49 AM | in 3.0.2

Hello,

The rand() method is overriden for Array unsafely in active_support. rand() is originally a private method taking a single parameter, but active_support adds a public rand() with no parameters. This causes some trouble with extensions of Array that want to use this method, such as the rand gem. A revised random_access.rb with a safe rand() is included.

Thanks,
djbob

random_access.rb 382 Bytes

Comments and changes to this ticket

You flagged this item as spam.
djbob May 8th, 2010 @ 08:50 AM
Ermm, probably change that ">" into a ">=" in the included file.
Dan Pickett May 9th, 2010 @ 06:44 PM
- Tag changed from “active_support core_ext” to “active_support core_ext, bugmash”
can someone work to create a patch in accordance with http://rails.lighthouseapp.com/projects/8994/sending-patches? Other bugmashers can then verify and pass the issue over for core review.
Santiago Pastorino May 10th, 2010 @ 05:30 PM
- Milestone cleared.
- Tag changed from “active_support core_ext, bugmash” to “active_support core_ext, bugmash, patch”
- State changed from “new” to “verified”
- Assigned user set to “Santiago Pastorino”
Patches for master and 2-3-stable attached
- array-master.patch 1.4 KB
- array-2-3-stable.patch 1.6 KB
Xavier Noria May 11th, 2010 @ 09:27 PM
The method Kernel#rand can be called in a procedural style. It is technically a private method of any class because it is defined that way in Kernel as a trick. The same happens with Kernel#gsub and others, you can call [].gsub, and you'll get an error message about a private method. A hack is leaking.

I think a 3rd party library should be able to extend Array and expect a call to rand with no receiver to be calling Kernel#rand.

Since Array#rand has nothing to do with Kernel#rand, I think that it is not a good solution to change the signature and choose the implementation based on the argument. Additionally, Kernel#rand can be called with no argument, but this is secondary, my main point is the previous one.

I believe we should rename Array#rand instead.
Rizwan Reza May 16th, 2010 @ 03:04 AM
Anything on this one, considering Xavier's comment?
Rizwan Reza May 16th, 2010 @ 03:06 AM
- State changed from “verified” to “open”
You flagged this item as spam.
Eric Hutzelman May 16th, 2010 @ 06:16 AM
Anyone in favor of Array#random or Array#random_element? Is it worth the deprecation to make this kind of change?
Xavier Noria May 16th, 2010 @ 09:49 AM
I prefer random_element, since random (and rand) are a bit ambiguous in my view and could also mean shuffle.

This method has been there for a long time, I think it is more likely that Rails developers and plugins use it than 3rd party libraries are extending Array and using rand.

Thus, I think it has to be renamed but I also think it is worth a deprecation cycle to ease migration to Rails 3. We can wait until 3.1 or something. If we just change what #rand returns that's going to be a subtle bug for people.
You flagged this item as spam.
Wijnand Wiersma May 16th, 2010 @ 10:28 AM
+1 for Xaviers comment. Rename it but use a deprecation cycle.
José Valim May 16th, 2010 @ 10:30 AM
- Tag changed from “active_support core_ext, bugmash, patch” to “active_support core_ext, patch”
Santiago Pastorino May 16th, 2010 @ 08:23 PM
- Assigned user changed from “Santiago Pastorino” to “Xavier Noria”
patch for both master and 2-3-stable
- rand-master.patch 4.3 KB
- rand-2-3-stable.patch 4.7 KB
Repository May 16th, 2010 @ 09:24 PM
- State changed from “open” to “committed”
(from [821e15e5f2d9ef2aa43918a16cbd00f40c221e95]) Change on Array extension from rand => random_element [#4555 state:committed]

Signed-off-by: Xavier Noria fxn@hashref.com
http://github.com/rails/rails/commit/821e15e5f2d9ef2aa43918a16cbd00...
Jeremy Kemper May 16th, 2010 @ 09:47 PM
- State changed from “committed” to “open”
The master patch deprecates but does not fix. Since 3.0 is unreleased yet, we can remove rand there and deprecate it in 2-3-stable.
Rizwan Reza May 16th, 2010 @ 10:11 PM
- Tag changed from “active_support core_ext, patch” to “active_support core_ext, bugmash-review, patch”
- State changed from “open” to “verified”
The patches accommodate JK's suggestions.
- 0001-remove-rand-since-it-s-deprecated-in-2-3-stable-now.patch 1.2 KB
- 0001-deprecate-array-rand-in-favor-of-array-random_elemen.patch 1.4 KB
Rizwan Reza May 16th, 2010 @ 10:26 PM
Removed relevant tests added them in 2-3-stable.
- 0001-deprecate-array-rand-in-favor-of-array-random_elemen.patch 2.3 KB
- 0001-removed-array-rand.patch 1.8 KB
Rizwan Reza May 16th, 2010 @ 10:41 PM
Please apply these patches on top of the ones above.
- for-master.patch 1.3 KB
Rizwan Reza May 16th, 2010 @ 10:48 PM
- no changes were found...
- for-2-3-stable.patch 2.4 KB
Santiago Pastorino May 17th, 2010 @ 02:43 PM
Didn't understand yet why this https://rails.lighthouseapp.com/projects/8994/tickets/4555/a/522934... (The latest i did for 2-3-stable) was re done.
For me seems ok.
Xavier Noria May 17th, 2010 @ 09:45 PM
Santiago, the one for 2.3 was not applied because of the assert_deprecated block. I had it in my TODO but then the thread moved on.
Rizwan Reza May 17th, 2010 @ 10:58 PM
(from [32b0b5f7b25a05179981d97d6b47aa4a1c683f2f]) Deprecate Array#rand in favor of Array#random_element [#4555 stated:committed]

Signed-off-by: Xavier Noria fxn@hashref.com
http://github.com/rails/rails/commit/32b0b5f7b25a05179981d97d6b47aa...
Santiago Pastorino May 18th, 2010 @ 08:08 PM
This issue could be closed now, right?
Santiago Pastorino May 18th, 2010 @ 08:11 PM
- State changed from “verified” to “resolved”
You flagged this item as spam.
Marc-André Lafortune May 26th, 2010 @ 06:49 AM
I'm surprised nobody pointed out that Array#sample is what it should be called, since that's what exists natively in Ruby 1.9. Ideally it should also support an optional parameter.

My implementation passes RubySpec, so activesupport could adapt it: http://github.com/marcandre/backports/blob/master/lib/backports/1.8...

Let me know if I should open a different ticket.
Xavier Noria June 5th, 2010 @ 12:22 AM
Thank you very much Marc-André, I removed #random_element altogether and backported Array#sample using your implementation: http://github.com/rails/rails/commit/67a43554f153a3ddb97039b5fac305...
Rizwan Reza June 6th, 2010 @ 09:29 AM
- Tag changed from “active_support core_ext, bugmash-review, patch” to “active_support core_ext, patch”
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
- Importance changed from “” to “Low”
[bulk edit]
You flagged this item as spam.
teiddy November 30th, 2010 @ 05:53 AM
Instructions: download and untar to /sites/all/modules

also - Download and install 'Rules' module to /sites/all/modules

Run /update.php

Goto Admin>Build>Modules, enable "OA Single Group Login Redirect" module and allow Rules module to be activated when prompted.

Log-out as admin and log-in as user with 1 group - page redirects as expected.

Greyside Thank-you!

Thank you for this information,I like it very much,Would you lik a pair of
Pachuco Suits
pack linen clothes
pack suit into suit
pant length
pant length for men

paul smith mens suits
Welcome to our store,We have the best service team!