This project is archived and is in readonly mode.
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
Comments and changes to this ticket
-
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
-
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:06 AM
- State changed from verified to open
-
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.
-
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
-
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.
-
Rizwan Reza May 16th, 2010 @ 10:48 PM
- no changes were found...
-
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:11 PM
- State changed from verified to resolved
-
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
-
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 menpaul smith mens suits
Welcome to our store,We have the best service team!
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 4555 active_support/core_ext/array/random_access.rb conflicts with standard Ruby library (from [821e15e5f2d9ef2aa43918a16cbd00f40c221e95]) Change ...
- 4555 active_support/core_ext/array/random_access.rb conflicts with standard Ruby library Didn't understand yet why this https://rails.lighthousea...
- 4555 active_support/core_ext/array/random_access.rb conflicts with standard Ruby library (from [32b0b5f7b25a05179981d97d6b47aa4a1c683f2f]) Depreca...