Merge pull request #236 from jcstringer/nil_errors

Don't allow nil to be submitted as a key to i18n.translate()

lib/i18n.rb

@@ -149,7 +149,7 @@ def translate(*args)
       handling = options.delete(:throw) && :throw || options.delete(:raise) && :raise # TODO deprecate :raise
 
       enforce_available_locales!(locale)
-      raise I18n::ArgumentError if key.is_a?(String) && key.empty?
+      raise I18n::ArgumentError if key.nil? || (key.is_a?(String) && key.empty?)
 
       result = catch(:exception) do
         if key.is_a?(Array)
@@ -171,7 +171,7 @@ def translate!(key, options={})
 
     # Returns true if a translation exists for a given key, otherwise returns false.
     def exists?(key, locale = config.locale)
-      raise I18n::ArgumentError if key.is_a?(String) && key.empty?
+      raise I18n::ArgumentError if key.nil? || (key.is_a?(String) && key.empty?)
       config.backend.exists?(locale, key)
     end
 

lib/i18n/backend/fallbacks.rb

@@ -55,7 +55,7 @@ def translate(locale, key, options = {})
           @fallback_locked = false
         end
 
-        return super(locale, nil, options.merge(:default => default)) if default
+        return super(locale, key, options.merge(:default => default)) if default
         throw(:exception, I18n::MissingTranslation.new(locale, key, options))
       end
 

lib/i18n/tests/defaults.rb

@@ -9,11 +9,11 @@ def setup
       end
 
       test "defaults: given nil as a key it returns the given default" do
-        assert_equal 'default', I18n.t(nil, :default => 'default')
+        assert_equal 'default', I18n.t(:does_not_exist, :default => 'default')
       end
 
       test "defaults: given a symbol as a default it translates the symbol" do
-        assert_equal 'bar', I18n.t(nil, :default => :'foo.bar')
+        assert_equal 'bar', I18n.t(:does_not_exist, :default => :'foo.bar')
       end
 
       test "defaults: given a symbol as a default and a scope it stays inside the scope when looking up the symbol" do
@@ -41,7 +41,7 @@ def setup
       test "defaults: using a custom scope separator" do
         # data must have been stored using the custom separator when using the ActiveRecord backend
         I18n.backend.store_translations(:en, { :foo => { :bar => 'bar' } }, { :separator => '|' })
-        assert_equal 'bar', I18n.t(nil, :default => :'foo|bar', :separator => '|')
+        assert_equal 'bar', I18n.t(:does_not_exist, :default => :'foo|bar', :separator => '|')
       end
     end
   end

lib/i18n/tests/pluralization.rb

@@ -4,31 +4,31 @@ module I18n
   module Tests
     module Pluralization
       test "pluralization: given 0 it returns the :zero translation if it is defined" do
-        assert_equal 'zero', I18n.t(:default => { :zero => 'zero' }, :count => 0)
+        assert_equal 'zero', I18n.t(:does_not_exist, :default => { :zero => 'zero' }, :count => 0)
       end
 
       test "pluralization: given 0 it returns the :other translation if :zero is not defined" do
-        assert_equal 'bars', I18n.t(:default => { :other => 'bars' }, :count => 0)
+        assert_equal 'bars', I18n.t(:does_not_exist, :default => { :other => 'bars' }, :count => 0)
       end
 
       test "pluralization: given 1 it returns the singular translation" do
-        assert_equal 'bar', I18n.t(:default => { :one => 'bar' }, :count => 1)
+        assert_equal 'bar', I18n.t(:does_not_exist, :default => { :one => 'bar' }, :count => 1)
       end
 
       test "pluralization: given 2 it returns the :other translation" do
-        assert_equal 'bars', I18n.t(:default => { :other => 'bars' }, :count => 2)
+        assert_equal 'bars', I18n.t(:does_not_exist, :default => { :other => 'bars' }, :count => 2)
       end
 
       test "pluralization: given 3 it returns the :other translation" do
-        assert_equal 'bars', I18n.t(:default => { :other => 'bars' }, :count => 3)
+        assert_equal 'bars', I18n.t(:does_not_exist, :default => { :other => 'bars' }, :count => 3)
       end
 
       test "pluralization: given nil it returns the whole entry" do
-        assert_equal({ :one => 'bar' }, I18n.t(:default => { :one => 'bar' }, :count => nil))
+        assert_equal({ :one => 'bar' }, I18n.t(:does_not_exist, :default => { :one => 'bar' }, :count => nil))
       end
 
       test "pluralization: given incomplete pluralization data it raises I18n::InvalidPluralizationData" do
-        assert_raise(I18n::InvalidPluralizationData) { I18n.t(:default => { :one => 'bar' }, :count => 2) }
+        assert_raise(I18n::InvalidPluralizationData) { I18n.t(:does_not_exist, :default => { :one => 'bar' }, :count => 2) }
       end
     end
   end

lib/i18n/tests/procs.rb

@@ -3,47 +3,47 @@
 module I18n
   module Tests
     module Procs
-      test "lookup: given a translation is a proc it calls the proc with the key and interpolation values" do
+      test "lookup: given a translation is a Proc it calls the Proc with the key and interpolation values" do
         I18n.backend.store_translations(:en, :a_lambda => lambda { |*args| I18n::Tests::Procs.filter_args(*args) })
         assert_equal '[:a_lambda, {:foo=>"foo"}]', I18n.t(:a_lambda, :foo => 'foo')
       end
 
       test "defaults: given a default is a Proc it calls it with the key and interpolation values" do
         proc = lambda { |*args| I18n::Tests::Procs.filter_args(*args) }
-        assert_equal '[nil, {:foo=>"foo"}]', I18n.t(nil, :default => proc, :foo => 'foo')
+        assert_equal '[:does_not_exist, {:foo=>"foo"}]', I18n.t(:does_not_exist, :default => proc, :foo => 'foo')
       end
 
       test "defaults: given a default is a key that resolves to a Proc it calls it with the key and interpolation values" do
         the_lambda = lambda { |*args| I18n::Tests::Procs.filter_args(*args) }
         I18n.backend.store_translations(:en, :a_lambda => the_lambda)
-        assert_equal '[:a_lambda, {:foo=>"foo"}]', I18n.t(nil, :default => :a_lambda, :foo => 'foo')
-        assert_equal '[:a_lambda, {:foo=>"foo"}]', I18n.t(nil, :default => [nil, :a_lambda], :foo => 'foo')
+        assert_equal '[:a_lambda, {:foo=>"foo"}]', I18n.t(:does_not_exist, :default => :a_lambda, :foo => 'foo')
+        assert_equal '[:a_lambda, {:foo=>"foo"}]', I18n.t(:does_not_exist, :default => [nil, :a_lambda], :foo => 'foo')
       end
 
       test "interpolation: given an interpolation value is a lambda it calls it with key and values before interpolating it" do
         proc = lambda { |*args| I18n::Tests::Procs.filter_args(*args) }
-        assert_match %r(\[\{:foo=>#<Proc.*>\}\]), I18n.t(nil, :default => '%{foo}', :foo => proc)
+        assert_match %r(\[\{:foo=>#<Proc.*>\}\]), I18n.t(:does_not_exist, :default => '%{foo}', :foo => proc)
       end
 
       test "interpolation: given a key resolves to a Proc that returns a string then interpolation still works" do
         proc = lambda { |*args| "%{foo}: " + I18n::Tests::Procs.filter_args(*args) }
-        assert_equal 'foo: [nil, {:foo=>"foo"}]', I18n.t(nil, :default => proc, :foo => 'foo')
+        assert_equal 'foo: [:does_not_exist, {:foo=>"foo"}]', I18n.t(:does_not_exist, :default => proc, :foo => 'foo')
       end
 
       test "pluralization: given a key resolves to a Proc that returns valid data then pluralization still works" do
         proc = lambda { |*args| { :zero => 'zero', :one => 'one', :other => 'other' } }
-        assert_equal 'zero',  I18n.t(:default => proc, :count => 0)
-        assert_equal 'one',   I18n.t(:default => proc, :count => 1)
-        assert_equal 'other', I18n.t(:default => proc, :count => 2)
+        assert_equal 'zero',  I18n.t(:does_not_exist, :default => proc, :count => 0)
+        assert_equal 'one',   I18n.t(:does_not_exist, :default => proc, :count => 1)
+        assert_equal 'other', I18n.t(:does_not_exist, :default => proc, :count => 2)
       end
 
-      test "lookup: given the option :resolve => false was passed it does not resolve proc translations" do
+      test "lookup: given the option :resolve => false was passed it does not resolve Proc translations" do
         I18n.backend.store_translations(:en, :a_lambda => lambda { |*args| I18n::Tests::Procs.filter_args(*args) })
         assert_equal Proc, I18n.t(:a_lambda, :resolve => false).class
       end
 
-      test "lookup: given the option :resolve => false was passed it does not resolve proc default" do
-        assert_equal Proc, I18n.t(nil, :default => lambda { |*args| I18n::Tests::Procs.filter_args(*args) }, :resolve => false).class
+      test "lookup: given the option :resolve => false was passed it does not resolve Proc default" do
+        assert_equal Proc, I18n.t(:does_not_exist, :default => lambda { |*args| I18n::Tests::Procs.filter_args(*args) }, :resolve => false).class
       end
 
 

test/backend/cascade_test.rb

@@ -39,7 +39,7 @@ def lookup(key, options = {})
   end
 
   test "cascades defaults, too" do
-    assert_equal 'foo', lookup(nil, :default => [:'missing.missing', :'missing.foo'])
+    assert_equal 'foo', lookup(:does_not_exist, :default => [:'missing.missing', :'missing.foo'])
   end
 
   test "works with :offset => 2 and a single key" do

test/backend/chain_test.rb

@@ -39,10 +39,10 @@ def setup
   end
 
   test "default" do
-    assert_equal 'Fuh',  I18n.t(:default => 'Fuh')
-    assert_equal 'Zero', I18n.t(:default => { :zero => 'Zero' }, :count => 0)
-    assert_equal({ :zero => 'Zero' }, I18n.t(:default => { :zero => 'Zero' }))
-    assert_equal 'Foo', I18n.t(:default => :foo)
+    assert_equal 'Fuh',  I18n.t(:does_not_exist, :default => 'Fuh')
+    assert_equal 'Zero', I18n.t(:does_not_exist, :default => { :zero => 'Zero' }, :count => 0)
+    assert_equal({ :zero => 'Zero' }, I18n.t(:does_not_exist, :default => { :zero => 'Zero' }))
+    assert_equal 'Foo', I18n.t(:does_not_exist, :default => :foo)
   end
 
   test 'default is returned if translation is missing' do

test/backend/pluralization_test.rb

@@ -19,24 +19,24 @@ def setup
   end
 
   test "pluralization picks :one for 1" do
-    assert_equal 'one', I18n.t(:count => 1, :default => @entry, :locale => :xx)
+    assert_equal 'one', I18n.t(:does_not_exist, :count => 1, :default => @entry, :locale => :xx)
   end
 
   test "pluralization picks :few for 2" do
-    assert_equal 'few', I18n.t(:count => 2, :default => @entry, :locale => :xx)
+    assert_equal 'few', I18n.t(:does_not_exist, :count => 2, :default => @entry, :locale => :xx)
   end
 
   test "pluralization picks :many for 11" do
-    assert_equal 'many', I18n.t(:count => 11, :default => @entry, :locale => :xx)
+    assert_equal 'many', I18n.t(:does_not_exist, :count => 11, :default => @entry, :locale => :xx)
   end
 
   test "pluralization picks zero for 0 if the key is contained in the data" do
-    assert_equal 'zero', I18n.t(:count => 0, :default => @entry, :locale => :xx)
+    assert_equal 'zero', I18n.t(:does_not_exist, :count => 0, :default => @entry, :locale => :xx)
   end
 
   test "pluralization picks few for 0 if the key is not contained in the data" do
     @entry.delete(:zero)
-    assert_equal 'few', I18n.t(:count => 0, :default => @entry, :locale => :xx)
+    assert_equal 'few', I18n.t(:does_not_exist, :count => 0, :default => @entry, :locale => :xx)
   end
 
   test "Fallbacks can pick up rules from fallback locales, too" do

test/backend/simple_test.rb

@@ -8,8 +8,8 @@ def setup
   end
 
   # useful because this way we can use the backend with no key for interpolation/pluralization
-  test "simple backend translate: given nil as a key it still interpolations the default value" do
-    assert_equal "Hi David", I18n.t(nil, :default => "Hi %{name}", :name => "David")
+  test "simple backend translate: given an invalid key it still interpolates the default value" do
+    assert_equal "Hi David", I18n.t(:does_not_exist, :default => "Hi %{name}", :name => "David")
   end
 
   # loading translations

test/i18n_test.rb

@@ -215,6 +215,10 @@ def setup
     assert_raise(I18n::ArgumentError) { I18n.t("") }
   end
 
+  test "translate given nil as a key raises an I18n::ArgumentError" do
+    assert_raise(I18n::ArgumentError) { I18n.t(nil) }
+  end
+
   test "translate given an unavailable locale rases an I18n::InvalidLocale" do
     begin
       I18n.config.enforce_available_locales = true
@@ -249,6 +253,14 @@ def setup
     assert_equal false, I18n.exists?(:bogus)
   end
 
+  test "exists? given an empty string will raise an error" do
+    assert_raise(I18n::ArgumentError) { I18n.exists?("") }
+  end
+
+  test "exists? given nil will raise an error" do
+    assert_raise(I18n::ArgumentError) { I18n.exists?(nil) }
+  end
+
   test "exists? given an existing dot-separated key will return true" do
     assert_equal true, I18n.exists?('currency.format.delimiter')
   end
@@ -396,17 +408,17 @@ def call(exception, locale, key, options); key; end
       I18n.config.enforce_available_locales = false
     end
   end
-  
+
   test 'I18n.reload! reloads the set of locales that are enforced' do
     begin
       # Clear the backend that affects the available locales and somehow can remain
       # set from the last running test.
       # For instance, it contains enough translations to cause a false positive with
       # this test when ran with --seed=50992
       I18n.backend = I18n::Backend::Simple.new
-      
+
       assert !I18n.available_locales.include?(:de), "Available locales should not include :de at this point"
-      
+
       I18n.enforce_available_locales = true
 
       assert_raise(I18n::InvalidLocale) { I18n.default_locale = :de }
@@ -422,11 +434,11 @@ def call(exception, locale, key, options); key; end
       store_translations(:en, :foo => 'Foo in :en')
       store_translations(:de, :foo => 'Foo in :de')
       store_translations(:pl, :foo => 'Foo in :pl')
-      
+
       assert I18n.available_locales.include?(:de), ":de should now be allowed"
       assert I18n.available_locales.include?(:en), ":en should now be allowed"
       assert I18n.available_locales.include?(:pl), ":pl should now be allowed"
-      
+
       assert_nothing_raised { I18n.default_locale = I18n.locale = :en }
       assert_nothing_raised { I18n.default_locale = I18n.locale = :de }
       assert_nothing_raised { I18n.default_locale = I18n.locale = :pl }