Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

require 'active_support/duration' loads a broken inflector #6884

Closed
jeremyevans opened this Issue · 7 comments

2 participants

Jeremy Evans Xavier Noria
Jeremy Evans

This is certainly not the only case I've seen where you can end up with a broken inflector, but this is the first case that has affected me personally. By "broken", I mean a situation where the string inflection methods are defined, but the inflection rules are not, so that singularize and pluralize just return the string as is.

Here's are some examples. First, the default case, where nothing has been loaded:

$ ruby -e "p 'foo'.pluralize"
-e:1:in `<main>': undefined method `pluralize' for "foo":String (NoMethodError)

Next, the case where the inflector is loaded, where it pluralizes correctly:

$ ruby -ractive_support/inflector -e "p 'foo'.pluralize"
"foos"

Finally, the case where the duration class is loaded, which apparently loads the inflection methods without the default inflections:

$ ruby -ractive_support/duration -e "p 'foo'.pluralize"
"foo"

There are two possibilities to fix this:

1) Don't load the inflector when the duration class is loaded. I think that Duration#inspect uses the inflections, so that would have to be modified if you go this route.

2) Make it impossible to load the string inflection methods without the default inflections. This should probably done anyway, as I've seen numerous people bitten by this. This can be fixed by changing lib/active_support/core_ext/string/inflections.rb to require 'active_support/inflector' instead of 'active_support/inflector/methods' and 'active_support/inflector/transliterate'.

Note that I've not tested master, only 3.2.6. However, looking at the source code, the problem appears to exist in master as well. After master is fixed, the fix should be backported to 3.2-stable.

Xavier Noria
Owner

Since 3.0, Active Support has been totally split so that you can cherry-pick stuff instead of loading everything as was the case before.

You can load extensions at various levels, they are documented in the section "How to Load Core Extensions" of the AS guide. See http://guides.rubyonrails.org/active_support_core_extensions.html#how-to-load-core-extensions.

In particular, Active Support makes no promises about the side-effects of loading active_support/duration. Active Support only tells you it will load as less as possible from itself to make durations available.

If your code needs the inflections, it has to load them.

Xavier Noria fxn closed this
Jeremy Evans

This isn't about active_support/duration, that's only a simple example that showcases the issue. The underlying issue is that you can end up with a broken inflector. Is the following behavior really desired?:

$ ruby -ractive_support/core_ext/string/inflections -e "p 'foo'.pluralize"
"foo"

If that's the expected/desired behavior, fine. Seems broken to me, though.

Jeremy Evans

Also, I should point out that Duration#inspect is broken by default because of the broken inflector. For example, the correct behavior:

$ ruby -ractive_support/all -e "p ActiveSupport::Duration.new(86400, [[:days, 1]]).inspect"      
"1 day"

versus the behavior you get when you just require 'active_support/duration':

$ ruby -ractive_support/duration -e "p ActiveSupport::Duration.new(86400, [[:days, 1]]).inspect"
"1 days"
Xavier Noria
Owner

The example with active_support/duration is not relevant. As I said, the exact code you get as a side-effect of loading that file is unspecified.

Now, pluralize should work after loading active_support/core_ext/string/inflections.rb. And the inspect method of duration should also load whatever it needs, which it does not.

I do think those two are issues.

Xavier Noria fxn reopened this
Xavier Noria fxn was assigned
Jeremy Evans

At the risk of beating a dead horse, in the How To Load Core Extensions guide that you link to, it shows all of the inflection methods (e.g. pluralize) as "Defined in active_support/core_ext/string/inflections.rb". But as I've shown, if you just require that file, you end up with a broken pluralize.

Xavier Noria
Owner

Not sure I follow.

The original description of this ticket showed nothing to fix. I explained why it was the case, and closed the issue.

Then, at the light of the followups, I reopened and assigned the issue to me myself, because I agree the new remarks show bugs.

Jeremy Evans

Fair enough. I thought my original description showed an obvious bug, but there are obviously more obvious examples of the problem that I didn't post until after the issue was closed. I'm sorry that I didn't post those examples first, but I didn't realize the apparent non-obviousness of the initial examples until after the issue was closed. :)

In terms of fixing, it's probably a -2 line, +1 line fix to the top of lib/active_support/core_ext/string/inflections.rb as I explained in my initial report.

Xavier Noria fxn closed this issue from a commit
Xavier Noria fxn make sure the inflection rules are loaded when cherry-picking active_…
…support/core_ext/string/inflections.rb [fixes #6884]
58c83d4
Xavier Noria fxn closed this in 58c83d4
Maurício Linhares mauricio referenced this issue from a commit
Xavier Noria fxn make sure the inflection rules are loaded when cherry-picking active_…
…support/core_ext/string/inflections.rb [fixes #6884]
9e0b3fc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.