Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Normative: Allow users to specify rounding based on cash denominations in common use #839

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Normative: Allow users to specify rounding based on cash denominations in common use #839

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c2abdfc
Normative: Add support for cashDigits attribute from CLDR, which allo…
ben-allen Oct 15, 2023
1428546
Normative: Add support for cashRounding attribute from CLDR, allowing…
ben-allen Oct 17, 2023
ba8b719
fixup! Normative: Add support for cashRounding attribute from CLDR, a…
ben-allen Oct 23, 2023
7d125bb
fixup! fixup! Normative: Add support for cashRounding attribute from …
ben-allen Oct 28, 2023
90db107
Make new CLDR requirement a recommendation instead
ben-allen Oct 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions spec/normative-references.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ <h1>Normative References</h1>
<li>
<a href="https://www.iana.org/time-zones/">IANA Time Zone Database</a>
</li>
<li>
<a href="https://cldr.unicode.org/">The Unicode Common Locale Data Repository</a>
</li>
<li>
<a href="https://unicode.org/versions/latest">The Unicode Standard</a>
</li>
Expand Down
43 changes: 38 additions & 5 deletions spec/numberformat.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ <h1>Intl.NumberFormat ( [ _locales_ [ , _options_ ] ] )</h1>

<emu-alg>
1. If NewTarget is *undefined*, let _newTarget_ be the active function object, else let _newTarget_ be NewTarget.
1. Let _numberFormat_ be ? OrdinaryCreateFromConstructor(_newTarget_, *"%NumberFormat.prototype%"*, &laquo; [[InitializedNumberFormat]], [[Locale]], [[DataLocale]], [[NumberingSystem]], [[Style]], [[Unit]], [[UnitDisplay]], [[Currency]], [[CurrencyDisplay]], [[CurrencySign]], [[MinimumIntegerDigits]], [[MinimumFractionDigits]], [[MaximumFractionDigits]], [[MinimumSignificantDigits]], [[MaximumSignificantDigits]], [[RoundingType]], [[Notation]], [[CompactDisplay]], [[UseGrouping]], [[SignDisplay]], [[RoundingIncrement]], [[RoundingMode]], [[ComputedRoundingPriority]], [[TrailingZeroDisplay]], [[BoundFormat]] &raquo;).
1. Let _numberFormat_ be ? OrdinaryCreateFromConstructor(_newTarget_, *"%NumberFormat.prototype%"*, &laquo; [[InitializedNumberFormat]], [[Locale]], [[DataLocale]], [[NumberingSystem]], [[Style]], [[Unit]], [[UnitDisplay]], [[Currency]], [[CurrencyDisplay]], [[CurrencyPrecision]], [[CurrencySign]], [[MinimumIntegerDigits]], [[MinimumFractionDigits]], [[MaximumFractionDigits]], [[MinimumSignificantDigits]], [[MaximumSignificantDigits]], [[RoundingType]], [[Notation]], [[CompactDisplay]], [[UseGrouping]], [[SignDisplay]], [[RoundingIncrement]], [[RoundingMode]], [[ComputedRoundingPriority]], [[TrailingZeroDisplay]], [[BoundFormat]] &raquo;).
1. Perform ? InitializeNumberFormat(_numberFormat_, _locales_, _options_).
1. If the implementation supports the normative optional constructor mode of <emu-xref href="#legacy-constructor"></emu-xref>, then
1. Let _this_ be the *this* value.
Expand Down Expand Up @@ -77,18 +77,21 @@ <h1>
1. Let _style_ be _numberFormat_.[[Style]].
1. If _style_ is *"currency"*, then
1. Let _currency_ be _numberFormat_.[[Currency]].
1. Let _cDigits_ be CurrencyDigits(_currency_).
1. Let _currencyPrecision_ be _numberFormat_.[[CurrencyPrecision]].
1. Let _cDigits_ be CurrencyDigits(_currency_, _currencyPrecision_).
1. Let _defaultRoundingIncrement_ be CurrencyRoundingIncrement(_currency_, _currencyPrecision_).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an interaction between {minimum,maximum}FractionDigits and roundingIncrement that I think this is failing to account for by setting defaultRoundingIncrement in ignorance of the former:

const input = 55.555;
for (const maximumFractionDigits of [0, 1, 2]) {
  for (const roundingIncrement of [1, 50]) {
    const nfOptions = {
      style: "currency",
      currency: "USD",
      maximumFractionDigits,
      roundingIncrement,
    };
    let pattern = String(10 ** -maximumFractionDigits).replace(/[0-9]/g, "#");
    const output = new Intl.NumberFormat("en", nfOptions).format(input);
    console.log(`${input} as ${pattern} @ increment ${roundingIncrement}: ${output}`);
  }
}
/* =>
55.555 as # @ increment 1: $56
55.555 as # @ increment 50: $50
55.555 as #.# @ increment 1: $55.6
55.555 as #.# @ increment 50: $55.0
55.555 as #.## @ increment 1: $55.56
55.555 as #.## @ increment 50: $55.50
*/

Given the above, what would we expect from currencyPrecision: "cash", maximumFractionDigits: 0 vs. currencyPrecision: "cash", maximumFractionDigits: 1 vs. currencyPrecision: "cash", maximumFractionDigits: 2?

Speaking personally, I would expect currencyPrecision to always apply at the same absolute position regardless of output precision, which suggests to me that the algorithm needs to account for digits options (and for that matter, notation as well—we arguably have a preexisting bug where currency-derived fractional digit count defaults are applied even when the decimal point is moved away from its absolute position, as in new Intl.NumberFormat("en", { style: "currency", currency, currencyDisplay: "code", notation: "engineering" }).format(12345).replace(/^.*\s/, "") resulting in "12E3" for JPY but "12.35E3" for USD).

Put another way, I think I expect currency-derived precision and corresponding rounding to apply only when notation is "standard" (i.e., not even when the decimal point happens to be in the right place with scientific/engineering/compact notation), and only in that context does it make sense for a new option to further refine the output.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's a good point. Great catch. I should have noticed that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, very good catch!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of thoughts:

  1. It seems reasonable to throw if currencyPrecision: "cash" is set and notation isn't "standard", since cash rounding only really makes sense in standard notation.
  2. If cash rounding only makes sense if notation is "standard", this suggests that the non-cash option for currencyPrecision should be something other than "financial" — maybe just "standard"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an issue not only when notation isn't "standard" but also when a different value was set for minimumFractionDigits or maximumFractionDigits. So the condition when this is used should be exactly the condition when _cDigits_ is used.

I'm kind-of okay with any of the following outcomes:

  1. Do a complicated check that the setting isn't present with conflicting options
  2. Ignore the setting if conflicting options are present (this is how we behave when e.g. currencySign is present but style is not currency)
  3. Define the algorithm similar to how you have it defined here and let people shoot themselves in the foot

Copy link
Collaborator Author

@ben-allen ben-allen Oct 24, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gut feelings when I see those potential outcomes worded that way:

3 seems bad (needlessly confusing for users)
1 seems less bad but not in keeping with the rest of the spec
2 seems least bad

on edit: Making sure I understand 2: if minimumFractionDigits or maximumFractionDigits is set, currencyPrecision is ignored, correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concretely the change would be the following in SetNumberFormatDigitOptions:

Undo your change on this line:

        1. Let _roundingIncrement_ be ? GetNumberOption(_options_, *"roundingIncrement"*, 1, 5000, 1).

And add the following line:

             1. Set _intlObj_.[[MinimumFractionDigits]] to _mnfdDefault_.
             1. Set _intlObj_.[[MaximumFractionDigits]] to _mxfdDefault_.
+            1. Set _intlObj_.[[RoundingIncrement]] to _defaultRoundingIncrement_.

I think that gets the job done?

1. Let _mnfdDefault_ be _cDigits_.
1. Let _mxfdDefault_ be _cDigits_.
1. Else,
1. Let _defaultRoundingIncrement_ be 1.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observation: not sure how the spec scopes variables but this Let is not in the same lexical scope as the code below that reads from it. If it is a problem, you could Let it be 1 further up and then Set it to something else when needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine; aliases are implicitly scoped to the containing algorithm: https://tc39.es/ecma262/multipage/notational-conventions.html#sec-algorithm-conventions (emphasis mine)

Once declared, an alias may be referenced in any subsequent steps and must not be referenced from steps prior to the alias's declaration.

1. Let _mnfdDefault_ be 0.
1. If _style_ is *"percent"*, then
1. Let _mxfdDefault_ be 0.
1. Else,
1. Let _mxfdDefault_ be 3.
1. Let _notation_ be ? GetOption(_options_, *"notation"*, ~string~, &laquo; *"standard"*, *"scientific"*, *"engineering"*, *"compact"* &raquo;, *"standard"*).
1. Set _numberFormat_.[[Notation]] to _notation_.
1. Perform ? SetNumberFormatDigitOptions(_numberFormat_, _options_, _mnfdDefault_, _mxfdDefault_, _notation_).
1. Perform ? SetNumberFormatDigitOptions(_numberFormat_, _options_, _mnfdDefault_, _mxfdDefault_, _notation_, _defaultRoundingIncrement_).
1. Let _compactDisplay_ be ? GetOption(_options_, *"compactDisplay"*, ~string~, &laquo; *"short"*, *"long"* &raquo;, *"short"*).
1. Let _defaultUseGrouping_ be *"auto"*.
1. If _notation_ is *"compact"*, then
Expand All @@ -113,6 +116,7 @@ <h1>
_mnfdDefault_: an integer,
_mxfdDefault_: an integer,
_notation_: a String,
_defaultRoundingIncrement_: an integer,
): either a normal completion containing ~unused~ or a throw completion
</h1>
<dl class="header">
Expand All @@ -126,7 +130,7 @@ <h1>
1. Let _mnsd_ be ? Get(_options_, *"minimumSignificantDigits"*).
1. Let _mxsd_ be ? Get(_options_, *"maximumSignificantDigits"*).
1. Set _intlObj_.[[MinimumIntegerDigits]] to _mnid_.
1. Let _roundingIncrement_ be ? GetNumberOption(_options_, *"roundingIncrement"*, 1, 5000, 1).
1. Let _roundingIncrement_ be ? GetNumberOption(_options_, *"roundingIncrement"*, 1, 5000, _defaultRoundingIncrement_).
1. If _roundingIncrement_ is not in « 1, 2, 5, 10, 20, 25, 50, 100, 200, 250, 500, 1000, 2000, 2500, 5000 », throw a *RangeError* exception.
1. Let _roundingMode_ be ? GetOption(_options_, *"roundingMode"*, ~string~, &laquo; *"ceil"*, *"floor"*, *"expand"*, *"trunc"*, *"halfCeil"*, *"halfFloor"*, *"halfExpand"*, *"halfTrunc"*, *"halfEven"* &raquo;, *"halfExpand"*).
1. Let _roundingPriority_ be ? GetOption(_options_, *"roundingPriority"*, ~string~, &laquo; *"auto"*, *"morePrecision"*, *"lessPrecision"* &raquo;, *"auto"*).
Expand Down Expand Up @@ -215,6 +219,7 @@ <h1>
1. Else,
1. If IsWellFormedCurrencyCode(_currency_) is *false*, throw a *RangeError* exception.
1. Let _currencyDisplay_ be ? GetOption(_options_, *"currencyDisplay"*, ~string~, &laquo; *"code"*, *"symbol"*, *"narrowSymbol"*, *"name"* &raquo;, *"symbol"*).
1. Let _currencyPrecision_ be ? GetOption(_options_, *"currencyPrecision"*, ~string~, &laquo; *"cash"*, *"financial"* &raquo;, *"financial"*).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a reminder to bikeshed all parts of this: the option name and both string options.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering about the difference between "financial" here vs. "accounting" for currencySign (i.e., should both options instead use the same "accounting" value?).

Copy link
Collaborator Author

@ben-allen ben-allen Oct 24, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My sense is no, since there's situations where you'd want to use one and not the other. I can easily imagine someone using (for example) a personal finance application wanting debts represented by wrapping them in parentheses, but also not caring about anything smaller than a cent.

Avoiding confusion between the options is one reason why I think "financial" would be a better name than "accounting" for this option.

1. Let _currencySign_ be ? GetOption(_options_, *"currencySign"*, ~string~, &laquo; *"standard"*, *"accounting"* &raquo;, *"standard"*).
1. Let _unit_ be ? GetOption(_options_, *"unit"*, ~string~, ~empty~, *undefined*).
1. If _unit_ is *undefined*, then
Expand All @@ -225,6 +230,7 @@ <h1>
1. If _style_ is *"currency"*, then
1. Set _intlObj_.[[Currency]] to the ASCII-uppercase of _currency_.
1. Set _intlObj_.[[CurrencyDisplay]] to _currencyDisplay_.
1. Set _intlObj_.[[CurrencyPrecision]] to _currencyPrecision_.
1. Set _intlObj_.[[CurrencySign]] to _currencySign_.
1. If _style_ is *"unit"*, then
1. Set _intlObj_.[[Unit]] to _unit_.
Expand Down Expand Up @@ -470,6 +476,11 @@ <h1>Intl.NumberFormat.prototype.resolvedOptions ( )</h1>
<td>*"currencyDisplay"*</td>
<td></td>
</tr>
<tr>
<td>[[CurrencyPrecision]]</td>
<td>*"currencyPrecision"*</td>
<td></td>
</tr>
<tr>
<td>[[CurrencySign]]</td>
<td>*"currencySign"*</td>
Expand Down Expand Up @@ -577,6 +588,7 @@ <h1>Properties of Intl.NumberFormat Instances</h1>
<li>[[Style]] is one of the String values *"decimal"*, *"currency"*, *"percent"*, or *"unit"*, identifying the type of quantity being measured.</li>
<li>[[Currency]] is a String value with the currency code identifying the currency to be used if formatting with the *"currency"* unit type. It is only used when [[Style]] has the value *"currency"*.</li>
<li>[[CurrencyDisplay]] is one of the String values *"code"*, *"symbol"*, *"narrowSymbol"*, or *"name"*, specifying whether to display the currency as an ISO 4217 alphabetic currency code, a localized currency symbol, or a localized currency name if formatting with the *"currency"* style. It is only used when [[Style]] has the value *"currency"*.</li>
<li>[[CurrencyPrecision]] is one of the String values *"cash"* or *"financial"*, specifying whether to round currency values to the smallest denomination of the currency in common usage, or instead to round to the number of fractional digits used for the currency in accounting and by financial institutions. It is only used when [[Style]] has the value *"currency"*.</li>
<li>[[CurrencySign]] is one of the String values *"standard"* or *"accounting"*, specifying whether to render negative numbers in accounting format, often signified by parenthesis. It is only used when [[Style]] has the value *"currency"* and when [[SignDisplay]] is not *"never"*.</li>
<li>[[Unit]] is a core unit identifier. It is only used when [[Style]] has the value *"unit"*.</li>
<li>[[UnitDisplay]] is one of the String values *"short"*, *"narrow"*, or *"long"*, specifying whether to display the unit as a symbol, narrow symbol, or localized long name if formatting with the *"unit"* style. It is only used when [[Style]] has the value *"unit"*.</li>
Expand Down Expand Up @@ -713,14 +725,35 @@ <h1>Abstract Operations for NumberFormat Objects</h1>
<h1>
CurrencyDigits (
_currency_: a String,
_currencyPrecision_: a String,
): a non-negative integer
</h1>
<dl class="header">
</dl>
<emu-alg>
1. Assert: IsWellFormedCurrencyCode(_currency_) is *true*.
1. Assert: _currency_ is equal to the ASCII-uppercase of _currency_.
1. If the Common Locale Data Repository supplemental data pertaining to fractional currency values contains an element with _currency_ as the value of its iso4217 attribute, then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm comfortable with generalizing beyond ISO 4217, but this seems like the kind of data for which an implementation could conceivably have reasons to diverge from CLDR (i.e., superior local knowledge). So I'd rather have the reference be a recommendation rather than a requirement (aligning with other sections of the spec).

1. If _currencyPrecision_ is *"cash"* and the element with _currency_ as the value of its iso4217 attribute has a cashDigits attribute, return the value of that attribute; otherwise, return the value of that element's digits attribute.
1. Return 2.
</emu-alg>
</emu-clause>

<emu-clause id="sec-currencyrounding" type="abstract operation">
<h1>
CurrencyRoundingIncrement (
_currency_: a String,
_currencyPrecision_: a String,
): a non-negative integer
</h1>
<dl class="header">
</dl>
<emu-alg>
1. Assert: IsWellFormedCurrencyCode(_currency_) is *true*.
1. Assert: _currency_ is equal to the ASCII-uppercase of _currency_.
1. If the ISO 4217 currency and funds code list contains _currency_ as an alphabetic code, return the minor unit value corresponding to the _currency_ from the list; otherwise, return 2.
1. If the Common Locale Repository supplemental data pertaining to fractional currency values contains an element with _currency_ as the value of its iso4217 attribute, then
1. If _currencyPrecision_ is *"cash"* and the element with _currency_ as the value of its iso4217 attribute has a cashRounding attribute, return the value of that attribute.
1. Return 1.
</emu-alg>
</emu-clause>

Expand Down
2 changes: 1 addition & 1 deletion spec/pluralrules.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ <h1>
1. Set _opt_.[[localeMatcher]] to _matcher_.
1. Let _t_ be ? GetOption(_options_, *"type"*, ~string~, &laquo; *"cardinal"*, *"ordinal"* &raquo;, *"cardinal"*).
1. Set _pluralRules_.[[Type]] to _t_.
1. Perform ? SetNumberFormatDigitOptions(_pluralRules_, _options_, 0, 3, *"standard"*).
1. Perform ? SetNumberFormatDigitOptions(_pluralRules_, _options_, 0, 3, *"standard"*, 1).
1. Let _localeData_ be %PluralRules%.[[LocaleData]].
1. Let _r_ be ResolveLocale(%PluralRules%.[[AvailableLocales]], _requestedLocales_, _opt_, %PluralRules%.[[RelevantExtensionKeys]], _localeData_).
1. Set _pluralRules_.[[Locale]] to _r_.[[locale]].
Expand Down