-
Notifications
You must be signed in to change notification settings - Fork 167
Normative: Validate unit-valued options after all options are read #3130
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -548,51 +548,58 @@ <h1> | |||||||||||||||||||||||||||
GetTemporalUnitValuedOption ( | ||||||||||||||||||||||||||||
_options_: an Object, | ||||||||||||||||||||||||||||
_key_: a property key, | ||||||||||||||||||||||||||||
_unitGroup_: ~date~, ~time~, or ~datetime~, | ||||||||||||||||||||||||||||
_default_: ~required~, ~unset~, ~auto~, or a Temporal unit, | ||||||||||||||||||||||||||||
optional _extraValues_: a List of either Temporal units or ~auto~, | ||||||||||||||||||||||||||||
_default_: ~required~ or ~unset~, | ||||||||||||||||||||||||||||
): either a normal completion containing either a Temporal unit, ~unset~, or ~auto~, or a throw completion | ||||||||||||||||||||||||||||
</h1> | ||||||||||||||||||||||||||||
<dl class="header"> | ||||||||||||||||||||||||||||
<dt>description</dt> | ||||||||||||||||||||||||||||
<dd>It attempts to read from the specified property of _options_ a Temporal unit that is <emu-not-ref>covered</emu-not-ref> by the union of _unitGroup_ and _extraValues_, substituting _default_ if the property value is *undefined*.</dd> | ||||||||||||||||||||||||||||
<dd>It attempts to read a Temporal unit from the specified property of _options_.</dd> | ||||||||||||||||||||||||||||
</dl> | ||||||||||||||||||||||||||||
<p>Both singular and plural unit names are accepted, but only the singular form is used internally.</p> | ||||||||||||||||||||||||||||
<emu-alg> | ||||||||||||||||||||||||||||
1. Let _allowedValues_ be a new empty List. | ||||||||||||||||||||||||||||
1. Let _allowedStrings_ be a new empty List. | ||||||||||||||||||||||||||||
1. Append *"auto"* to _allowedStrings_. | ||||||||||||||||||||||||||||
1. For each row of <emu-xref href="#table-temporal-units"></emu-xref>, except the header row, in table order, do | ||||||||||||||||||||||||||||
1. Let _unit_ be the value in the "Value" column of the row. | ||||||||||||||||||||||||||||
1. If the "Category" column of the row is ~date~ and _unitGroup_ is ~date~ or ~datetime~, append _unit_ to _allowedValues_. | ||||||||||||||||||||||||||||
1. Else if the "Category" column of the row is ~time~ and _unitGroup_ is ~time~ or ~datetime~, append _unit_ to _allowedValues_. | ||||||||||||||||||||||||||||
1. If _extraValues_ is present, then | ||||||||||||||||||||||||||||
1. Set _allowedValues_ to the list-concatenation of _allowedValues_ and _extraValues_. | ||||||||||||||||||||||||||||
1. Let _singularName_ be the value in the "Singular property name" column of the row. | ||||||||||||||||||||||||||||
1. Append _singularName_ to _allowedStrings_. | ||||||||||||||||||||||||||||
1. Let _pluralName_ be the value in the "Plural property name" column of the row. | ||||||||||||||||||||||||||||
1. Append _pluralName_ to _allowedStrings_. | ||||||||||||||||||||||||||||
1. NOTE: For each singular Temporal unit name that is contained within _allowedStrings_, the corresponding plural name is also contained within it. | ||||||||||||||||||||||||||||
1. If _default_ is ~unset~, then | ||||||||||||||||||||||||||||
1. Let _defaultValue_ be *undefined*. | ||||||||||||||||||||||||||||
1. Else if _default_ is ~required~, then | ||||||||||||||||||||||||||||
1. Let _defaultValue_ be ~required~. | ||||||||||||||||||||||||||||
1. Else if _default_ is ~auto~, then | ||||||||||||||||||||||||||||
1. Append _default_ to _allowedValues_. | ||||||||||||||||||||||||||||
1. Let _defaultValue_ be *"auto"*. | ||||||||||||||||||||||||||||
1. Else, | ||||||||||||||||||||||||||||
1. Assert: _allowedValues_ contains _default_. | ||||||||||||||||||||||||||||
1. Let _defaultValue_ be the value in the "Singular property name" column of <emu-xref href="#table-temporal-units"></emu-xref> corresponding to the row with _default_ in the "Value" column. | ||||||||||||||||||||||||||||
1. Let _allowedStrings_ be a new empty List. | ||||||||||||||||||||||||||||
1. For each element _value_ of _allowedValues_, do | ||||||||||||||||||||||||||||
1. If _value_ is ~auto~, then | ||||||||||||||||||||||||||||
1. Append *"auto"* to _allowedStrings_. | ||||||||||||||||||||||||||||
1. Else, | ||||||||||||||||||||||||||||
1. Let _singularName_ be the value in the "Singular property name" column of <emu-xref href="#table-temporal-units"></emu-xref> corresponding to the row with _value_ in the "Value" column. | ||||||||||||||||||||||||||||
1. Append _singularName_ to _allowedStrings_. | ||||||||||||||||||||||||||||
1. Let _pluralName_ be the value in the "Plural property name" column of the corresponding row. | ||||||||||||||||||||||||||||
1. Append _pluralName_ to _allowedStrings_. | ||||||||||||||||||||||||||||
1. NOTE: For each singular Temporal unit name that is contained within _allowedStrings_, the corresponding plural name is also contained within it. | ||||||||||||||||||||||||||||
1. Let _defaultValue_ be _default_. | ||||||||||||||||||||||||||||
1. Let _value_ be ? GetOption(_options_, _key_, ~string~, _allowedStrings_, _defaultValue_). | ||||||||||||||||||||||||||||
1. If _value_ is *undefined*, return ~unset~. | ||||||||||||||||||||||||||||
1. If _value_ is *"auto"*, return ~auto~. | ||||||||||||||||||||||||||||
1. Return the value in the "Value" column of <emu-xref href="#table-temporal-units"></emu-xref> corresponding to the row with _value_ in its "Singular property name" or "Plural property name" column. | ||||||||||||||||||||||||||||
</emu-alg> | ||||||||||||||||||||||||||||
</emu-clause> | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
<emu-clause id="sec-temporal-validatetemporalunitvaluedoption" type="abstract operation"> | ||||||||||||||||||||||||||||
<h1> | ||||||||||||||||||||||||||||
ValidateTemporalUnitValue ( | ||||||||||||||||||||||||||||
_value_: a Temporal unit, ~unset~, or ~auto~, | ||||||||||||||||||||||||||||
_unitGroup_: ~date~, ~time~, or ~datetime~, | ||||||||||||||||||||||||||||
optional _extraValues_: a List of either Temporal units or ~auto~, | ||||||||||||||||||||||||||||
): either a normal completion containing ~unused~ or a throw completion | ||||||||||||||||||||||||||||
</h1> | ||||||||||||||||||||||||||||
<dl class="header"> | ||||||||||||||||||||||||||||
<dt>description</dt> | ||||||||||||||||||||||||||||
<dd>It validates that the result of GetTemporalUnitValuedOption is <emu-not-ref>covered</emu-not-ref> by the union of _unitGroup_ and _extraValues_.</dd> | ||||||||||||||||||||||||||||
</dl> | ||||||||||||||||||||||||||||
<emu-alg> | ||||||||||||||||||||||||||||
1. Let _allowedValues_ be a new empty List. | ||||||||||||||||||||||||||||
1. For each row of <emu-xref href="#table-temporal-units"></emu-xref>, except the header row, in table order, do | ||||||||||||||||||||||||||||
1. Let _unit_ be the value in the "Value" column of the row. | ||||||||||||||||||||||||||||
1. If the "Category" column of the row is ~date~ and _unitGroup_ is ~date~ or ~datetime~, append _unit_ to _allowedValues_. | ||||||||||||||||||||||||||||
1. Else if the "Category" column of the row is ~time~ and _unitGroup_ is ~time~ or ~datetime~, append _unit_ to _allowedValues_. | ||||||||||||||||||||||||||||
1. If _extraValues_ is present, then | ||||||||||||||||||||||||||||
1. Set _allowedValues_ to the list-concatenation of _allowedValues_ and _extraValues_. | ||||||||||||||||||||||||||||
1. If _allowedValues_ does not contain _value_, throw a RangeError exception. | ||||||||||||||||||||||||||||
Comment on lines
+592
to
+599
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if it makes sense to keep the allowedValues list at all.
Suggested change
|
||||||||||||||||||||||||||||
</emu-alg> | ||||||||||||||||||||||||||||
</emu-clause> | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
<emu-clause id="sec-temporal-gettemporalrelativetooption" type="abstract operation"> | ||||||||||||||||||||||||||||
<h1> | ||||||||||||||||||||||||||||
GetTemporalRelativeToOption ( | ||||||||||||||||||||||||||||
|
@@ -1864,13 +1871,19 @@ <h1> | |||||||||||||||||||||||||||
</dl> | ||||||||||||||||||||||||||||
<emu-alg> | ||||||||||||||||||||||||||||
1. NOTE: The following steps read options and perform independent validation in alphabetical order. | ||||||||||||||||||||||||||||
1. Let _largestUnit_ be ? GetTemporalUnitValuedOption(_options_, *"largestUnit"*, _unitGroup_, ~auto~). | ||||||||||||||||||||||||||||
1. If _disallowedUnits_ contains _largestUnit_, throw a *RangeError* exception. | ||||||||||||||||||||||||||||
1. Let _largestUnit_ be ? GetTemporalUnitValuedOption(_options_, *"largestUnit"*, ~unset~). | ||||||||||||||||||||||||||||
1. Let _roundingIncrement_ be ? GetRoundingIncrementOption(_options_). | ||||||||||||||||||||||||||||
1. Let _roundingMode_ be ? GetRoundingModeOption(_options_, ~trunc~). | ||||||||||||||||||||||||||||
1. Let _smallestUnit_ be ? GetTemporalUnitValuedOption(_options_, *"smallestUnit"*, ~unset~). | ||||||||||||||||||||||||||||
1. Perform ? ValidateTemporalUnitValue(_largestUnit_, _unitGroup_). | ||||||||||||||||||||||||||||
1. If _largestUnit_ is ~unset~, then | ||||||||||||||||||||||||||||
1. Set _largestUnit_ to ~auto~. | ||||||||||||||||||||||||||||
Comment on lines
+1878
to
+1880
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't look right to me... if largestUnit is ~unset~, wouldn't ValidateTemporalUnitValue(largestUnit, unitGroup) have already rejected it?
Suggested change
And likewise for the other call sites. |
||||||||||||||||||||||||||||
1. If _disallowedUnits_ contains _largestUnit_, throw a *RangeError* exception. | ||||||||||||||||||||||||||||
1. If _operation_ is ~since~, then | ||||||||||||||||||||||||||||
1. Set _roundingMode_ to NegateRoundingMode(_roundingMode_). | ||||||||||||||||||||||||||||
1. Let _smallestUnit_ be ? GetTemporalUnitValuedOption(_options_, *"smallestUnit"*, _unitGroup_, _fallbackSmallestUnit_). | ||||||||||||||||||||||||||||
1. Perform ? ValidateTemporalUnitValue(_smallestUnit_, _unitGroup_). | ||||||||||||||||||||||||||||
1. If _smallestUnit_ is ~unset~, then | ||||||||||||||||||||||||||||
1. Set _smallestUnit_ to _fallbackSmallestUnit_. | ||||||||||||||||||||||||||||
1. If _disallowedUnits_ contains _smallestUnit_, throw a *RangeError* exception. | ||||||||||||||||||||||||||||
1. Let _defaultLargestUnit_ be LargerOfTwoTemporalUnits(_smallestLargestDefaultUnit_, _smallestUnit_). | ||||||||||||||||||||||||||||
1. If _largestUnit_ is ~auto~, set _largestUnit_ to _defaultLargestUnit_. | ||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -402,13 +402,15 @@ <h1>Temporal.Duration.prototype.round ( _roundTo_ )</h1> | |
1. Let _smallestUnitPresent_ be *true*. | ||
1. Let _largestUnitPresent_ be *true*. | ||
1. NOTE: The following steps read options and perform independent validation in alphabetical order (GetTemporalRelativeToOption reads *"relativeTo"*, GetRoundingIncrementOption reads *"roundingIncrement"* and GetRoundingModeOption reads *"roundingMode"*). | ||
1. Let _largestUnit_ be ? GetTemporalUnitValuedOption(_roundTo_, *"largestUnit"*, ~datetime~, ~unset~, « ~auto~ »). | ||
1. Let _largestUnit_ be ? GetTemporalUnitValuedOption(_roundTo_, *"largestUnit"*, ~unset~). | ||
1. Let _relativeToRecord_ be ? GetTemporalRelativeToOption(_roundTo_). | ||
1. Let _zonedRelativeTo_ be _relativeToRecord_.[[ZonedRelativeTo]]. | ||
1. Let _plainRelativeTo_ be _relativeToRecord_.[[PlainRelativeTo]]. | ||
1. Let _roundingIncrement_ be ? GetRoundingIncrementOption(_roundTo_). | ||
1. Let _roundingMode_ be ? GetRoundingModeOption(_roundTo_, ~half-expand~). | ||
1. Let _smallestUnit_ be ? GetTemporalUnitValuedOption(_roundTo_, *"smallestUnit"*, ~datetime~, ~unset~). | ||
1. Let _smallestUnit_ be ? GetTemporalUnitValuedOption(_roundTo_, *"smallestUnit"*, ~unset~). | ||
1. Perform ? ValidateTemporalUnitValue(_largestUnit_, ~datetime~, « ~auto~ »). | ||
1. Perform ? ValidateTemporalUnitValue(_smallestUnit_, ~datetime~). | ||
Comment on lines
+412
to
+413
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As noted above, it seems like these calls will reject ~unset~ values. |
||
1. If _smallestUnit_ is ~unset~, then | ||
1. Set _smallestUnitPresent_ to *false*. | ||
1. Set _smallestUnit_ to ~nanosecond~. | ||
|
@@ -476,7 +478,8 @@ <h1>Temporal.Duration.prototype.total ( _totalOf_ )</h1> | |
1. Let _relativeToRecord_ be ? GetTemporalRelativeToOption(_totalOf_). | ||
1. Let _zonedRelativeTo_ be _relativeToRecord_.[[ZonedRelativeTo]]. | ||
1. Let _plainRelativeTo_ be _relativeToRecord_.[[PlainRelativeTo]]. | ||
1. Let _unit_ be ? GetTemporalUnitValuedOption(_totalOf_, *"unit"*, ~datetime~, ~required~). | ||
1. Let _unit_ be ? GetTemporalUnitValuedOption(_totalOf_, *"unit"*, ~required~). | ||
1. Perform ? ValidateTemporalUnitValue(_unit_, ~datetime~). | ||
1. If _zonedRelativeTo_ is not *undefined*, then | ||
1. Let _internalDuration_ be ToInternalDurationRecord(_duration_). | ||
1. Let _timeZone_ be _zonedRelativeTo_.[[TimeZone]]. | ||
|
@@ -512,7 +515,8 @@ <h1>Temporal.Duration.prototype.toString ( [ _options_ ] )</h1> | |
1. NOTE: The following steps read options and perform independent validation in alphabetical order (GetTemporalFractionalSecondDigitsOption reads *"fractionalSecondDigits"* and GetRoundingModeOption reads *"roundingMode"*). | ||
1. Let _digits_ be ? GetTemporalFractionalSecondDigitsOption(_resolvedOptions_). | ||
1. Let _roundingMode_ be ? GetRoundingModeOption(_resolvedOptions_, ~trunc~). | ||
1. Let _smallestUnit_ be ? GetTemporalUnitValuedOption(_resolvedOptions_, *"smallestUnit"*, ~time~, ~unset~). | ||
1. Let _smallestUnit_ be ? GetTemporalUnitValuedOption(_resolvedOptions_, *"smallestUnit"*, ~unset~). | ||
1. Perform ? ValidateTemporalUnitValue(_smallestUnit_, ~time~). | ||
1. If _smallestUnit_ is ~hour~ or ~minute~, throw a *RangeError* exception. | ||
1. Let _precision_ be ToSecondsStringPrecisionRecord(_smallestUnit_, _digits_). | ||
1. If _precision_.[[Unit]] is ~nanosecond~ and _precision_.[[Increment]] = 1, then | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested simplification: