From 4b3c23d7181d99f08664addbf37df1da7df1f22f Mon Sep 17 00:00:00 2001 From: Chris Stringer Date: Fri, 6 Dec 2013 15:05:37 -0800 Subject: [PATCH 1/6] Error on exists? with nil key --- lib/i18n.rb | 6 +++--- test/i18n_test.rb | 14 +++++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/i18n.rb b/lib/i18n.rb index ca477ea1..ad702466 100755 --- a/lib/i18n.rb +++ b/lib/i18n.rb @@ -148,7 +148,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.is_a?(String) && key.empty? result = catch(:exception) do if key.is_a?(Array) @@ -170,7 +170,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 @@ -359,7 +359,7 @@ def default_exception_handler(exception, locale, key, options) def handle_enforce_available_locales_deprecation if config.enforce_available_locales.nil? && !@unenforced_available_locales_deprecation - $stderr.puts "[deprecated] I18n.enforce_available_locales will default to true in the future. If you really want to skip validation of your locale you can set I18n.enforce_available_locales = false to avoid this message." + $stderr.puts "[deprecated] I18n.enforce_available_locales will default to true in the future. If you really want to skip validation of your locale you can set I18n.enforce_available_locales = false to avoid this message." @unenforced_available_locales_deprecation = true end end diff --git a/test/i18n_test.rb b/test/i18n_test.rb index 040d4cb9..cffa5df9 100644 --- a/test/i18n_test.rb +++ b/test/i18n_test.rb @@ -56,7 +56,7 @@ def setup assert_equal :de, Thread.current[:i18n_config].locale I18n.locale = :en end - + test "raises an I18n::InvalidLocale exception when setting an unavailable locale" do begin I18n.config.enforce_available_locales = true @@ -206,6 +206,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 @@ -223,6 +227,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 From 4f2830823b951637ff8ac2f5679e0fa82f41bbdb Mon Sep 17 00:00:00 2001 From: Chris Stringer Date: Fri, 6 Dec 2013 15:06:16 -0800 Subject: [PATCH 2/6] Whoops remove comment --- lib/i18n.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/i18n.rb b/lib/i18n.rb index ad702466..dd940201 100755 --- a/lib/i18n.rb +++ b/lib/i18n.rb @@ -148,7 +148,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.is_a?(String) && key.empty? result = catch(:exception) do if key.is_a?(Array) From ca21a110bf6532fabfa668038ddab45f9e9928ad Mon Sep 17 00:00:00 2001 From: Chris Stringer Date: Wed, 18 Dec 2013 12:08:11 -0800 Subject: [PATCH 3/6] Don't allow nil keys in translate --- lib/i18n.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/i18n.rb b/lib/i18n.rb index dd940201..a97f7b85 100755 --- a/lib/i18n.rb +++ b/lib/i18n.rb @@ -148,7 +148,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) From 2e64e3e8609929fedd89052d70db6a2297af913c Mon Sep 17 00:00:00 2001 From: Chris Stringer Date: Wed, 18 Dec 2013 12:09:09 -0800 Subject: [PATCH 4/6] Pass in original key instead of nil when calling translate --- lib/i18n/backend/fallbacks.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/i18n/backend/fallbacks.rb b/lib/i18n/backend/fallbacks.rb index 7252bb00..6b41a752 100644 --- a/lib/i18n/backend/fallbacks.rb +++ b/lib/i18n/backend/fallbacks.rb @@ -35,6 +35,7 @@ module Fallbacks # it's a Symbol. When the default contains a String, Proc or Hash # it is evaluated last after all the fallback locales have been tried. def translate(locale, key, options = {}) + return super if options[:fallback] default = extract_non_symbol_default!(options) if options[:default] @@ -46,8 +47,7 @@ def translate(locale, key, options = {}) end end options.delete(:fallback) - - return super(locale, nil, options.merge(:default => default)) if default + return super(locale, key, options.merge(:default => default)) throw(:exception, I18n::MissingTranslation.new(locale, key, options)) end From 8ccf9a4fbf7183b1069dbe501898459922d669e4 Mon Sep 17 00:00:00 2001 From: Chris Stringer Date: Wed, 18 Dec 2013 12:10:15 -0800 Subject: [PATCH 5/6] Update tests to not send nil values as keys --- lib/i18n/tests/defaults.rb | 6 +++--- lib/i18n/tests/pluralization.rb | 14 +++++++------- lib/i18n/tests/procs.rb | 20 ++++++++++---------- test/backend/cascade_test.rb | 2 +- test/backend/chain_test.rb | 8 ++++---- test/backend/pluralization_test.rb | 10 +++++----- test/backend/simple_test.rb | 4 ++-- 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/lib/i18n/tests/defaults.rb b/lib/i18n/tests/defaults.rb index 081dcbd1..c89200b9 100644 --- a/lib/i18n/tests/defaults.rb +++ b/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 @@ -33,7 +33,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 diff --git a/lib/i18n/tests/pluralization.rb b/lib/i18n/tests/pluralization.rb index d3319dcd..4055fe93 100644 --- a/lib/i18n/tests/pluralization.rb +++ b/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 diff --git a/lib/i18n/tests/procs.rb b/lib/i18n/tests/procs.rb index 55ff9529..a1f17ab4 100644 --- a/lib/i18n/tests/procs.rb +++ b/lib/i18n/tests/procs.rb @@ -10,30 +10,30 @@ module Procs test "defaults: given a default is a Proc it calls it with the key and interpolation values" do proc = lambda { |*args| 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 I18n.backend.store_translations(:en, :a_lambda => lambda { |*args| filter_args(*args) }) - 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| filter_args(*args) } - assert_match %r(\[\{:foo=>#\}\]), I18n.t(nil, :default => '%{foo}', :foo => proc) + assert_match %r(\[\{:foo=>#\}\]), 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 + test "interpolation: given a key resolves to a lambda that returns a string then interpolation still works" do proc = lambda { |*args| "%{foo}: " + 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 @@ -42,7 +42,7 @@ module Procs 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| filter_args(*args) }, :resolve => false).class + assert_equal Proc, I18n.t(:does_not_exist, :default => lambda { |*args| filter_args(*args) }, :resolve => false).class end protected diff --git a/test/backend/cascade_test.rb b/test/backend/cascade_test.rb index c23f1674..71fae50c 100644 --- a/test/backend/cascade_test.rb +++ b/test/backend/cascade_test.rb @@ -38,7 +38,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 diff --git a/test/backend/chain_test.rb b/test/backend/chain_test.rb index a3bb44b4..67147be5 100644 --- a/test/backend/chain_test.rb +++ b/test/backend/chain_test.rb @@ -28,10 +28,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 diff --git a/test/backend/pluralization_test.rb b/test/backend/pluralization_test.rb index 54fbba92..9d883db7 100644 --- a/test/backend/pluralization_test.rb +++ b/test/backend/pluralization_test.rb @@ -18,24 +18,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 diff --git a/test/backend/simple_test.rb b/test/backend/simple_test.rb index 3ebc8ee4..515a1b98 100644 --- a/test/backend/simple_test.rb +++ b/test/backend/simple_test.rb @@ -7,8 +7,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 From 229ee94f63f2068cc26fad50d0b60358c4845d55 Mon Sep 17 00:00:00 2001 From: Chris Stringer Date: Wed, 18 Dec 2013 14:07:39 -0800 Subject: [PATCH 6/6] Standardize on Proc (vs proc or lambda) --- lib/i18n/tests/procs.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/i18n/tests/procs.rb b/lib/i18n/tests/procs.rb index a1f17ab4..345d8995 100644 --- a/lib/i18n/tests/procs.rb +++ b/lib/i18n/tests/procs.rb @@ -3,7 +3,7 @@ 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| filter_args(*args) }) assert_equal '[:a_lambda, {:foo=>"foo"}]', I18n.t(:a_lambda, :foo => 'foo') end @@ -19,12 +19,12 @@ module Procs 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 + test "interpolation: given an interpolation value is a Proc it calls it with key and values before interpolating it" do proc = lambda { |*args| filter_args(*args) } assert_match %r(\[\{:foo=>#\}\]), I18n.t(:does_not_exist, :default => '%{foo}', :foo => proc) end - test "interpolation: given a key resolves to a lambda that returns a string then interpolation still works" do + test "interpolation: given a key resolves to a Proc that returns a string then interpolation still works" do proc = lambda { |*args| "%{foo}: " + filter_args(*args) } assert_equal "foo: [:does_not_exist, {:foo=>\"foo\"}]", I18n.t(:does_not_exist, :default => proc, :foo => 'foo') end @@ -36,12 +36,12 @@ module Procs 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| 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 + 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| filter_args(*args) }, :resolve => false).class end