diff --git a/CHANGELOG.md b/CHANGELOG.md index 12e44c484..2df159475 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Security in case of vulnerabilities. ## [Unreleased] +- Fix `Worldwide::Calendar::Gregorian.weekday_names` and `.month_names` returning partial results for 104 locales, including `en-CA`, `en-GB`, `en-IN` and every other `en-001` descendant. CLDR inheritance is item by item, but `I18n` returns the first node it finds in the fallback chain, so a locale that overrides a single entry (`en-CA` overrides only `Sept`) hid the rest of the entries it inherits: `month_names(locale: "en-CA", width: :abbreviated)` returned `["Sept"]` instead of all twelve names. Both methods now resolve names with CLDR's inheritance rules and raise `MissingCalendarDataError` instead of silently returning a degraded key. [#582](https://github.com/Shopify/worldwide/pull/582) --- diff --git a/lib/worldwide/calendar/gregorian.rb b/lib/worldwide/calendar/gregorian.rb index 8e326b1d5..b6f155af2 100644 --- a/lib/worldwide/calendar/gregorian.rb +++ b/lib/worldwide/calendar/gregorian.rb @@ -3,27 +3,51 @@ module Worldwide module Calendar class Gregorian + class MissingCalendarDataError < StandardError; end + class << self # We intentionally don't support `short` weekday names, as we haven't found anyone asking for them. # People usually want the full weekday name, or the abbreviated version. VALID_WEEKDAY_WIDTHS = [:abbreviated, :narrow, :wide].freeze + WEEKDAYS = [:sun, :mon, :tue, :wed, :thu, :fri, :sat].freeze def weekday_names(width: :wide, locale: I18n.locale) raise ArgumentError, "Invalid width: #{width}" unless VALID_WEEKDAY_WIDTHS.include?(width) - Worldwide::Cldr.t("calendars.gregorian.days.stand_alone.#{width}", locale: locale) + complete_names("calendars.gregorian.days.stand_alone.#{width}", WEEKDAYS, locale) end VALID_MONTH_WIDTHS = [:abbreviated, :narrow, :wide].freeze + MONTHS = (1..12).to_a.freeze def month_names(width: :wide, locale: I18n.locale) raise ArgumentError, "Invalid width: #{width}" unless VALID_MONTH_WIDTHS.include?(width) - Worldwide::Cldr.t("calendars.gregorian.months.stand_alone.#{width}", locale: locale).values + complete_names("calendars.gregorian.months.stand_alone.#{width}", MONTHS, locale).values end def quarter(date, locale: I18n.locale) format_string = Worldwide::Cldr.t("calendars.gregorian.additional_formats.yQQQ", locale: locale) Worldwide::Cldr::DateFormatPattern.format(date, format_string, locale: locale) end + + private + + # `stand_alone` is an alias to `format` in CLDR for most locales, and many locales + # override only a handful of entries, so these names have to be assembled with + # CLDR's inheritance rules rather than read out of a single locale's data. + # + # Anything still missing afterwards is a data bug. Raise rather than let it through: + # `Worldwide::Cldr.t` degrades a missing key into a humanized version of its last + # segment, so the alternative is silently handing back the string "wide". + def complete_names(key, expected_keys, locale) + names = Worldwide::Cldr.resolved_hash(key, locale: locale) + missing = expected_keys - names.keys + + unless missing.empty? + raise MissingCalendarDataError, "Missing CLDR data for '#{key}' in locale '#{locale}': #{missing.join(", ")}" + end + + expected_keys.to_h { |name_key| [name_key, names.fetch(name_key)] } + end end end end diff --git a/lib/worldwide/cldr.rb b/lib/worldwide/cldr.rb index b53f0c597..a5e0a46b6 100644 --- a/lib/worldwide/cldr.rb +++ b/lib/worldwide/cldr.rb @@ -68,8 +68,64 @@ def config CONFIG end + # Look up a structural (hash-valued) CLDR key with CLDR's own inheritance rules. + # + # CLDR resolves locale data item by item: the effective data for a locale is the + # union of its own data with each of its ancestors', the more specific locale + # winning per item, and `` elements resolved against the locale originally + # requested (UTS #35 §4.1 "Multiple Inheritance" and §4.4 "Alias Elements"). + # + # `I18n`'s fallback backend instead returns the first non-nil node it finds in the + # chain, which is correct for leaves but wrong for whole nodes: a locale that + # overrides a single entry hides every sibling entry it should inherit. `en-CA` + # only overrides September ("Sept" rather than "Sep"), so a plain `t` of the + # abbreviated month names returns a one-entry hash instead of all twelve. + # + # Leaf lookups should keep using `t`; only whole-node lookups need this. + def resolved_hash(key, locale: I18n.locale) + with_cldr do + merge_ancestors(key.to_s, locale.to_sym, Set.new) + end + end + private + # Walks the CLDR fallback chain from the least specific ancestor to the most + # specific, so that descendants overwrite what they inherit. + def merge_ancestors(key, locale, seen) + # Guards against a cycle between two aliases pointing at each other. + return {} unless seen.add?([key, locale]) + + fallbacks[locale].reverse_each.with_object({}) do |ancestor, merged| + case (node = unresolved_node(key, ancestor)) + when ::Symbol # A CLDR ``, resolved against the requested locale rather than the ancestor. + deep_merge!(merged, merge_ancestors(node.to_s, locale, seen)) + when ::Hash + deep_merge!(merged, node) + end + end + end + + # A single locale's own contribution: no fallbacks, no alias resolution, and no + # exception handler (`default: nil` makes a miss return nil instead of degrading). + def unresolved_node(key, locale) + I18n.t(key, locale: locale, default: nil, fallback: false, resolve: false) + rescue ::I18n::InvalidLocale + nil + end + + def deep_merge!(target, source) + source.each do |key, value| + target[key] = if target[key].is_a?(::Hash) && value.is_a?(::Hash) + deep_merge!(target[key], value) + else + value + end + end + + target + end + def respond_to_missing?(method_name, include_private = false) I18n.respond_to?(method_name, include_private) end diff --git a/test/worldwide/calendar/gregorian_test.rb b/test/worldwide/calendar/gregorian_test.rb index 642b60924..b8c7e272b 100644 --- a/test/worldwide/calendar/gregorian_test.rb +++ b/test/worldwide/calendar/gregorian_test.rb @@ -7,6 +7,8 @@ module Calendar class GregorianTest < ActiveSupport::TestCase include PluralizationHelper + WIDTHS = [:abbreviated, :narrow, :wide].freeze + setup do @calendar = Worldwide::Calendar::Gregorian end @@ -64,10 +66,65 @@ class GregorianTest < ActiveSupport::TestCase assert_equal "Q4 2016", Worldwide::Calendar::Gregorian.quarter(Date.new(2016, 12, 1)) end + # `stand_alone` is an alias to `format` in CLDR for these locales, so the names only + # resolve if the alias in `root` is followed and then read back in the requested locale. + test "#weekday_names resolves the CLDR stand-alone alias" do + expected = { sun: "Sunday", mon: "Monday", tue: "Tuesday", wed: "Wednesday", thu: "Thursday", fri: "Friday", sat: "Saturday" } + + assert_equal expected, @calendar.weekday_names(locale: :en) + + expected = { sun: "søndag", mon: "mandag", tue: "tirsdag", wed: "onsdag", thu: "torsdag", fri: "fredag", sat: "lørdag" } + + # `nb` holds no calendar data of its own; it inherits everything from `no`. + assert_equal expected, @calendar.weekday_names(locale: :nb) + end + + test "#month_names resolves the CLDR stand-alone alias" do + assert_equal ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"], @calendar.month_names(locale: :en) + assert_equal ["januar", "februar", "mars", "april", "mai", "juni", "juli", "august", "september", "oktober", "november", "desember"], @calendar.month_names(locale: :nb) + end + + # CLDR inheritance is item by item, so a locale that overrides one entry still + # inherits its siblings. `en-CA` only overrides September. + test "#month_names inherits the entries a locale does not override" do + expected = ["Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sept", "Oct", "Nov", "Dec"] + + assert_equal expected, @calendar.month_names(locale: :"en-CA", width: :abbreviated) + assert_equal expected, @calendar.month_names(locale: :"en-GB", width: :abbreviated) + + # `ar-MA` overrides 7 of the 12 wide month names. + expected = ["يناير", "فبراير", "مارس", "أبريل", "ماي", "يونيو", "يوليوز", "غشت", "شتنبر", "أكتوبر", "نونبر", "دجنبر"] + + assert_equal expected, @calendar.month_names(locale: :"ar-MA") + end + + test "#weekday_names inherits the entries a locale does not override" do + # `se-FI` overrides 4 of the 7 wide weekday names. + expected = { sun: "sotnabeaivi", mon: "mánnodat", tue: "disdat", wed: "gaskavahkku", thu: "duorastat", fri: "bearjadat", sat: "lávvordat" } + + assert_equal expected, @calendar.weekday_names(locale: :"se-FI") + end + Worldwide::Locales.each do |locale| test "#quarter formatting doesn't fail (i.e., rely on date fields that have not been implemented) in #{locale}" do Worldwide::Calendar::Gregorian.quarter(Date.new(2016, 1, 1), locale: locale) end + + # Guards the whole class of bug: a missing key degrades into a humanized copy of its + # last segment, so an unresolved lookup hands back the string "wide" rather than raising. + test "#weekday_names and #month_names return complete data in #{locale}" do + WIDTHS.each do |width| + weekdays = Worldwide::Calendar::Gregorian.weekday_names(width: width, locale: locale) + + assert_equal [:sun, :mon, :tue, :wed, :thu, :fri, :sat], weekdays.keys, "#{locale} #{width} weekday names" + assert_empty weekdays.values.select { |name| name.nil? || name.empty? }, "#{locale} #{width} weekday names" + + months = Worldwide::Calendar::Gregorian.month_names(width: width, locale: locale) + + assert_equal 12, months.size, "#{locale} #{width} month names" + assert_empty months.select { |name| name.nil? || name.empty? }, "#{locale} #{width} month names" + end + end end end end diff --git a/test/worldwide/cldr_test.rb b/test/worldwide/cldr_test.rb index 8ed009b23..2d394d14d 100644 --- a/test/worldwide/cldr_test.rb +++ b/test/worldwide/cldr_test.rb @@ -44,5 +44,33 @@ class CldrTest < ActiveSupport::TestCase ensure I18n.fallbacks = old_fallbacks end + + test "#resolved_hash follows a CLDR alias and reads it back in the requested locale" do + # `en` has no `days.stand_alone.wide` of its own: `root` holds an alias to + # `days.format.wide`, which has to be resolved against `en`, not against `root`. + assert_nil Worldwide::Cldr.t("calendars.gregorian.days.stand_alone.wide", locale: :en, default: nil, fallback: false) + assert_equal "Sun", Worldwide::Cldr.t("calendars.gregorian.days.format.wide", locale: :root)[:sun] + + assert_equal "Sunday", Worldwide::Cldr.resolved_hash("calendars.gregorian.days.stand_alone.wide", locale: :en)[:sun] + end + + test "#resolved_hash merges every ancestor rather than stopping at the first match" do + # `en-CA` overrides only September, so `t` returns a single entry. + assert_equal({ 9 => "Sept" }, Worldwide::Cldr.t("calendars.gregorian.months.stand_alone.abbreviated", locale: :"en-CA")) + + resolved = Worldwide::Cldr.resolved_hash("calendars.gregorian.months.stand_alone.abbreviated", locale: :"en-CA") + + assert_equal (1..12).to_a, resolved.keys.sort + assert_equal "Sept", resolved[9] + assert_equal "Jan", resolved[1] + end + + test "#resolved_hash returns an empty hash for a missing key instead of degrading it" do + # `t` humanizes the last segment of a key it cannot find, which is how a missing + # weekday lookup used to come back as the string "wide". + assert_equal "wide", Worldwide::Cldr.t("calendars.gregorian.__missing__.wide", locale: :en) + + assert_empty Worldwide::Cldr.resolved_hash("calendars.gregorian.__missing__.wide", locale: :en) + end end end