fix(set): clamp hour() to 23 instead of the invalid 24
hour(n) with n >= 24 clamped n to 24, which walkTo() rejects as an invalid hour (valid range 0-23), nulling the epoch and invalidating the object. Clamp to 23 to match the other overflow-clamping setters (date, month).
This commit is contained in:
1 parent
cdeb1866ab
commit
d48b259893
3 files changed
+96
-1
No files matched your search
@@ -0,0 +1,79 @@
|
|||||||
|
# Fix hour() setter clamping to an invalid hour (24)
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
Calling `.hour(n)` with `n >= 24` produces an invalid spacetime whose epoch is
|
||||||
|
`null`, instead of clamping to a valid hour:
|
||||||
|
|
||||||
|
```js
|
||||||
|
const s = spacetime('2021-03-14 10:00:00', 'UTC')
|
||||||
|
|
||||||
|
s.hour(24).isValid() // false
|
||||||
|
s.hour(24).epoch // null
|
||||||
|
s.hour(25).format('iso') // '' (empty - object is invalid)
|
||||||
|
```
|
||||||
|
|
||||||
|
This is inconsistent with every other overflow-clamping setter:
|
||||||
|
|
||||||
|
- `.date(40)` clamps to the last day of the month,
|
||||||
|
- `.month(13)` clamps to December (11),
|
||||||
|
- `.hour(-1)` clamps to 0.
|
||||||
|
|
||||||
|
Only the upper bound of `.hour()` is broken.
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
|
||||||
|
`src/methods/set/set.js`, in `hours()`:
|
||||||
|
|
||||||
|
```js
|
||||||
|
if (n >= 24) {
|
||||||
|
n = 24
|
||||||
|
} else if (n < 0) {
|
||||||
|
n = 0
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Valid hours are `0-23`. Clamping to `24` yields a value that
|
||||||
|
`src/methods/set/walk.js` rejects (`hour.valid: (n) => n >= 0 && n < 24`), and
|
||||||
|
on an invalid unit `walkTo()` sets `s.epoch = null`, invalidating the whole
|
||||||
|
object. The negative branch is already correct (clamps to the valid `0`); the
|
||||||
|
`>= 24` branch is off by one and should clamp to `23`.
|
||||||
|
|
||||||
|
## Fix
|
||||||
|
|
||||||
|
```js
|
||||||
|
if (n >= 24) {
|
||||||
|
n = 23
|
||||||
|
} else if (n < 0) {
|
||||||
|
n = 0
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
`hour(24)`, `hour(25)`, `hour(100)` now all clamp to `23:00` on the same day,
|
||||||
|
matching the behavior of the other setters.
|
||||||
|
|
||||||
|
## Test
|
||||||
|
|
||||||
|
Added a test to `test/set.test.js` (next to the existing month/date clamp test)
|
||||||
|
asserting that out-of-range hours clamp to `23`, keep the object valid, keep the
|
||||||
|
same day, and that negatives still clamp to `0`.
|
||||||
|
|
||||||
|
Counterfactual (revert `n = 23` back to `n = 24`):
|
||||||
|
|
||||||
|
```
|
||||||
|
not ok - hour(24) clamps to 23
|
||||||
|
operator: equal
|
||||||
|
expected: 23
|
||||||
|
actual: 0
|
||||||
|
```
|
||||||
|
|
||||||
|
(the invalid object reports hour `0`).
|
||||||
|
|
||||||
|
## Gate
|
||||||
|
|
||||||
|
- `npm test` (tape) run against `src` (the default dev target of
|
||||||
|
`test/lib/index.js`): the new test passes; the full suite shows only 18
|
||||||
|
pre-existing DST/timezone failures (`europe/london`, `atlantic/madeira`,
|
||||||
|
`atlantic/azores`, some date-dependent "dst-now") that are present verbatim
|
||||||
|
without this change and are unrelated to it.
|
||||||
|
- `npm run lint` (eslint ./src): exit 0.
|
||||||
@@ -67,8 +67,10 @@ const minutes = function (s, n, goFwd) {
|
|||||||
|
|
||||||
const hours = function (s, n, goFwd) {
|
const hours = function (s, n, goFwd) {
|
||||||
n = validate(n)
|
n = validate(n)
|
||||||
|
// clamp to a valid hour (0-23). Clamping to 24 produces an hour that
|
||||||
|
// walkTo() rejects, which nulls the epoch and invalidates the object.
|
||||||
if (n >= 24) {
|
if (n >= 24) {
|
||||||
n = 24
|
n = 23
|
||||||
} else if (n < 0) {
|
} else if (n < 0) {
|
||||||
n = 0
|
n = 0
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -238,3 +238,17 @@ test('month()/date() setters clamp the day to the month length', (t) => {
|
|||||||
t.equal(iso(spacetime('2021-02-10').date(31)), '2021-02-28', 'date(31) in a common february clamps to the 28th')
|
t.equal(iso(spacetime('2021-02-10').date(31)), '2021-02-28', 'date(31) in a common february clamps to the 28th')
|
||||||
t.end()
|
t.end()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('hour() setter clamps out-of-range hours to a valid hour', (t) => {
|
||||||
|
const s = spacetime('2021-03-14 10:00:00', 'UTC')
|
||||||
|
// an out-of-range hour must clamp to the last valid hour (23), not produce
|
||||||
|
// an invalid spacetime with a null epoch
|
||||||
|
t.equal(s.hour(24).hour(), 23, 'hour(24) clamps to 23')
|
||||||
|
t.equal(s.hour(25).hour(), 23, 'hour(25) clamps to 23')
|
||||||
|
t.equal(s.hour(100).hour(), 23, 'hour(100) clamps to 23')
|
||||||
|
t.equal(s.hour(24).isValid(), true, 'hour(24) stays valid')
|
||||||
|
t.equal(s.hour(24).format('iso-short'), '2021-03-14', 'hour(24) keeps the same day')
|
||||||
|
// negative hours still clamp to 0
|
||||||
|
t.equal(s.hour(-1).hour(), 0, 'hour(-1) clamps to 0')
|
||||||
|
t.end()
|
||||||
|
})
|
||||||
Reference in new issue
Block a user