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.

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

Truncate all number arguments (closes #746); review usage of === undefined (closes #767); review handling of arguments of wrong type #765

Merged
merged 9 commits into from Jun 7, 2018
29 changes: 24 additions & 5 deletions CHANGELOG.md
Expand Up @@ -577,11 +577,29 @@ for the list of changes made since `v2.0.0-alpha.1`.
This change is introduced for consistency with ECMAScript standard library which does the same.
See [docs/Options.js](https://github.com/date-fns/date-fns/blob/master/docs/Options.js)

- **BREAKING**: all functions now handle arguments by following rules:

- **BREAKING**: all functions now implicitly convert arguments by following rules:

| | date | number | string | boolean |
|-----------|---------------|--------|-------------|---------|
| 0 | new Date(0) | 0 | '0' | false |
| '0' | Invalid Date | 0 | '0' | false |
| 1 | new Date(1) | 1 | '1' | true |
| '1' | Invalid Date | 1 | '1' | true |
| true | Invalid Date | NaN | 'true' | true |
| false | Invalid Date | NaN | 'false' | false |
| null | Invalid Date | NaN | 'null' | false |
| undefined | Invalid Date | NaN | 'undefined' | false |
| NaN | Invalid Date | NaN | 'NaN' | false |

Notes:
- as before, arguments expected to be `Date` are converted to `Date` using *date-fns'* `toDate` function;
- arguments expected to be numbers are converted to numbers using JavaScript's `Number` function;
- arguments expected to be strings arguments are converted to strings using JavaScript's `String` function.
- arguments expected to be numbers are converted to integer numbers using our custom `toInteger` implementation
(see [#765](https://github.com/date-fns/date-fns/pull/765));
- arguments expected to be strings arguments are converted to strings using JavaScript's `String` function;
- arguments expected to be booleans are converted to strings using JavaScript's `Boolean` function.

`null` and `undefined` passed to optional arguments (i.e. properties of `options` argument)
are ignored as if no argument was passed.

If any of resulting arguments is invalid (i.e. `NaN` for numbers and `Invalid Date` for dates),
an invalid value will be returned:
Expand All @@ -591,7 +609,8 @@ for the list of changes made since `v2.0.0-alpha.1`.
- `NaN` for functions that return numbers;
- and `String('Invalid Date')` for functions that return strings.

See tests and PR [#460](https://github.com/date-fns/date-fns/pull/460) for exact behavior.
See tests and PRs [#460](https://github.com/date-fns/date-fns/pull/460) and
[#765](https://github.com/date-fns/date-fns/pull/765) for exact behavior.

- **BREAKING**: all functions now check if the passed number of arguments is less
than the number of required arguments and throw `TypeError` exception if so.
Expand Down
3 changes: 2 additions & 1 deletion src/_lib/addUTCMinutes/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'

// This function will be a part of public API when UTC function will be implemented.
Expand All @@ -8,7 +9,7 @@ export default function addUTCMinutes (dirtyDate, dirtyAmount, dirtyOptions) {
}

var date = toDate(dirtyDate, dirtyOptions)
var amount = Number(dirtyAmount)
var amount = toInteger(dirtyAmount)
date.setUTCMinutes(date.getUTCMinutes() + amount)
return date
}
5 changes: 5 additions & 0 deletions src/_lib/addUTCMinutes/test.js
Expand Up @@ -24,6 +24,11 @@ describe('addUTCMinutes', function () {
assert.deepEqual(result, new Date(Date.UTC(2014, 6 /* Jul */, 10, 12, 20)))
})

it('converts a fractional number to an integer', function () {
var result = addUTCMinutes(new Date(Date.UTC(2014, 6 /* Jul */, 10, 12, 0)), 30.5)
assert.deepEqual(result, new Date(Date.UTC(2014, 6 /* Jul */, 10, 12, 30)))
})

it('implicitly converts number arguments', function () {
var result = addUTCMinutes(new Date(Date.UTC(2014, 6 /* Jul */, 10, 12, 0)), '30')
assert.deepEqual(result, new Date(Date.UTC(2014, 6 /* Jul */, 10, 12, 30)))
Expand Down
9 changes: 5 additions & 4 deletions src/_lib/getUTCWeekYear/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'
import startOfUTCWeek from '../startOfUTCWeek/index.js'

Expand All @@ -17,13 +18,13 @@ export default function getUTCWeekYear (dirtyDate, dirtyOptions) {
locale.options &&
locale.options.firstWeekContainsDate
var defaultFirstWeekContainsDate =
localeFirstWeekContainsDate === undefined
localeFirstWeekContainsDate == null
? 1
: Number(localeFirstWeekContainsDate)
: toInteger(localeFirstWeekContainsDate)
var firstWeekContainsDate =
options.firstWeekContainsDate === undefined
options.firstWeekContainsDate == null
? defaultFirstWeekContainsDate
: Number(options.firstWeekContainsDate)
: toInteger(options.firstWeekContainsDate)

// Test if weekStartsOn is between 1 and 7 _and_ is not NaN
if (!(firstWeekContainsDate >= 1 && firstWeekContainsDate <= 7)) {
Expand Down
7 changes: 4 additions & 3 deletions src/_lib/setUTCDay/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'

// This function will be a part of public API when UTC function will be implemented.
Expand All @@ -10,16 +11,16 @@ export default function setUTCDay (dirtyDate, dirtyDay, dirtyOptions) {
var options = dirtyOptions || {}
var locale = options.locale
var localeWeekStartsOn = locale && locale.options && locale.options.weekStartsOn
var defaultWeekStartsOn = localeWeekStartsOn === undefined ? 0 : Number(localeWeekStartsOn)
var weekStartsOn = options.weekStartsOn === undefined ? defaultWeekStartsOn : Number(options.weekStartsOn)
var defaultWeekStartsOn = localeWeekStartsOn == null ? 0 : toInteger(localeWeekStartsOn)
var weekStartsOn = options.weekStartsOn == null ? defaultWeekStartsOn : toInteger(options.weekStartsOn)

// Test if weekStartsOn is between 0 and 6 _and_ is not NaN
if (!(weekStartsOn >= 0 && weekStartsOn <= 6)) {
throw new RangeError('weekStartsOn must be between 0 and 6 inclusively')
}

var date = toDate(dirtyDate, dirtyOptions)
var day = Number(dirtyDay)
var day = toInteger(dirtyDay)

var currentDay = date.getUTCDay()

Expand Down
5 changes: 5 additions & 0 deletions src/_lib/setUTCDay/test.js
Expand Up @@ -86,6 +86,11 @@ describe('setUTCDay', function () {
assert.deepEqual(result, new Date(Date.UTC(2014, 8 /* Sep */, 3)))
})

it('converts a fractional number to an integer', function () {
var result = setUTCDay(new Date(Date.UTC(2014, 8 /* Sep */, 1)), 0.9)
assert.deepEqual(result, new Date(Date.UTC(2014, 7 /* Aug */, 31)))
})

it('implicitly converts number arguments', function () {
var result = setUTCDay(new Date(Date.UTC(2014, 8 /* Sep */, 1)), '0')
assert.deepEqual(result, new Date(Date.UTC(2014, 7 /* Aug */, 31)))
Expand Down
3 changes: 2 additions & 1 deletion src/_lib/setUTCISODay/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'

// This function will be a part of public API when UTC function will be implemented.
Expand All @@ -7,7 +8,7 @@ export default function setUTCISODay (dirtyDate, dirtyDay, dirtyOptions) {
throw new TypeError('2 arguments required, but only ' + arguments.length + ' present')
}

var day = Number(dirtyDay)
var day = toInteger(dirtyDay)

if (day % 7 === 0) {
day = day - 7
Expand Down
5 changes: 5 additions & 0 deletions src/_lib/setUTCISODay/test.js
Expand Up @@ -49,6 +49,11 @@ describe('setUTCISODay', function () {
assert.deepEqual(result, new Date(Date.UTC(2014, 8 /* Sep */, 3)))
})

it('converts a fractional number to an integer', function () {
var result = setUTCISODay(new Date(Date.UTC(2014, 8 /* Sep */, 1)), 3.33)
assert.deepEqual(result, new Date(Date.UTC(2014, 8 /* Sep */, 3)))
})

it('implicitly converts number arguments', function () {
var result = setUTCISODay(new Date(Date.UTC(2014, 8 /* Sep */, 1)), '3')
assert.deepEqual(result, new Date(Date.UTC(2014, 8 /* Sep */, 3)))
Expand Down
3 changes: 2 additions & 1 deletion src/_lib/setUTCISOWeek/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'
import getUTCISOWeek from '../getUTCISOWeek/index.js'

Expand All @@ -9,7 +10,7 @@ export default function setUTCISOWeek (dirtyDate, dirtyISOWeek, dirtyOptions) {
}

var date = toDate(dirtyDate, dirtyOptions)
var isoWeek = Number(dirtyISOWeek)
var isoWeek = toInteger(dirtyISOWeek)
var diff = getUTCISOWeek(date, dirtyOptions) - isoWeek
date.setUTCDate(date.getUTCDate() - diff * 7)
return date
Expand Down
5 changes: 5 additions & 0 deletions src/_lib/setUTCISOWeek/test.js
Expand Up @@ -20,6 +20,11 @@ describe('setUTCISOWeek', function () {
assert.deepEqual(result, new Date(Date.UTC(2008, 11 /* Dec */, 31)))
})

it('converts a fractional number to an integer', function () {
var result = setUTCISOWeek(new Date(Date.UTC(2004, 7 /* Aug */, 7)), 53.53)
assert.deepEqual(result, new Date(Date.UTC(2005, 0 /* Jan */, 1)))
})

it('implicitly converts number arguments', function () {
var result = setUTCISOWeek(new Date(Date.UTC(2004, 7 /* Aug */, 7)), '53')
assert.deepEqual(result, new Date(Date.UTC(2005, 0 /* Jan */, 1)))
Expand Down
3 changes: 2 additions & 1 deletion src/_lib/setUTCWeek/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'
import getUTCWeek from '../getUTCWeek/index.js'

Expand All @@ -9,7 +10,7 @@ export default function setUTCWeek (dirtyDate, dirtyWeek, dirtyOptions) {
}

var date = toDate(dirtyDate, dirtyOptions)
var week = Number(dirtyWeek)
var week = toInteger(dirtyWeek)
var diff = getUTCWeek(date, dirtyOptions) - week
date.setUTCDate(date.getUTCDate() - diff * 7)
return date
Expand Down
5 changes: 5 additions & 0 deletions src/_lib/setUTCWeek/test.js
Expand Up @@ -20,6 +20,11 @@ describe('setUTCWeek', function () {
assert.deepEqual(result, new Date(Date.UTC(2008, 11 /* Dec */, 31)))
})

it('converts a fractional number to an integer', function () {
var result = setUTCWeek(new Date(Date.UTC(2005, 0 /* Jan */, 2)), 1.1)
assert.deepEqual(result, new Date(Date.UTC(2004, 11 /* Dec */, 26)))
})

it('implicitly converts number arguments', function () {
var result = setUTCWeek(new Date(Date.UTC(2004, 7 /* Aug */, 7)), '53')
assert.deepEqual(result, new Date(Date.UTC(2005, 0 /* Jan */, 1)))
Expand Down
5 changes: 3 additions & 2 deletions src/_lib/startOfUTCWeek/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import toDate from '../../toDate/index.js'

// This function will be a part of public API when UTC function will be implemented.
Expand All @@ -10,8 +11,8 @@ export default function startOfUTCWeek (dirtyDate, dirtyOptions) {
var options = dirtyOptions || {}
var locale = options.locale
var localeWeekStartsOn = locale && locale.options && locale.options.weekStartsOn
var defaultWeekStartsOn = localeWeekStartsOn === undefined ? 0 : Number(localeWeekStartsOn)
var weekStartsOn = options.weekStartsOn === undefined ? defaultWeekStartsOn : Number(options.weekStartsOn)
var defaultWeekStartsOn = localeWeekStartsOn == null ? 0 : toInteger(localeWeekStartsOn)
var weekStartsOn = options.weekStartsOn == null ? defaultWeekStartsOn : toInteger(options.weekStartsOn)

// Test if weekStartsOn is between 0 and 6 _and_ is not NaN
if (!(weekStartsOn >= 0 && weekStartsOn <= 6)) {
Expand Down
9 changes: 5 additions & 4 deletions src/_lib/startOfUTCWeekYear/index.js
@@ -1,3 +1,4 @@
import toInteger from '../toInteger/index.js'
import getUTCWeekYear from '../getUTCWeekYear/index.js'
import startOfUTCWeek from '../startOfUTCWeek/index.js'

Expand All @@ -14,13 +15,13 @@ export default function startOfUTCWeekYear (dirtyDate, dirtyOptions) {
locale.options &&
locale.options.firstWeekContainsDate
var defaultFirstWeekContainsDate =
localeFirstWeekContainsDate === undefined
localeFirstWeekContainsDate == null
? 1
: Number(localeFirstWeekContainsDate)
: toInteger(localeFirstWeekContainsDate)
var firstWeekContainsDate =
options.firstWeekContainsDate === undefined
options.firstWeekContainsDate == null
? defaultFirstWeekContainsDate
: Number(options.firstWeekContainsDate)
: toInteger(options.firstWeekContainsDate)

var year = getUTCWeekYear(dirtyDate, dirtyOptions)
var firstWeek = new Date(0)
Expand Down
13 changes: 13 additions & 0 deletions src/_lib/toInteger/index.js
@@ -0,0 +1,13 @@
export default function toInteger (dirtyNumber) {
if (dirtyNumber === null || dirtyNumber === true || dirtyNumber === false) {
return NaN
}

var number = Number(dirtyNumber)

if (isNaN(number)) {
return number
}

return number < 0 ? Math.ceil(number) : Math.floor(number)
}
64 changes: 64 additions & 0 deletions src/_lib/toInteger/test.js
@@ -0,0 +1,64 @@
// @flow
/* eslint-env mocha */

import assert from 'power-assert'
import toInteger from '.'

describe('toInteger', function () {
it('truncates positive numbers', function () {
var result = toInteger(10.99)
assert(result === 10)
})

it('truncates negative numbers', function () {
var result = toInteger(-5.5)
assert(result === -5)
})

it('converts convertable strings', function () {
var result = toInteger('-10.75')
assert(result === -10)
})

it('returns NaN for non-convertable strings', function () {
var result = toInteger('Foobar')
assert(typeof result === 'number' && isNaN(result))
})

it('returns NaN for false', function () {
var result = toInteger(false)
assert(typeof result === 'number' && isNaN(result))
})

it('returns NaN for true', function () {
var result = toInteger(true)
assert(typeof result === 'number' && isNaN(result))
})

it('returns NaN for null', function () {
var result = toInteger(null)
assert(typeof result === 'number' && isNaN(result))
})

it('returns NaN for undefined', function () {
var result = toInteger(undefined)
assert(typeof result === 'number' && isNaN(result))
})

it('returns NaN for NaN', function () {
var result = toInteger(NaN)
assert(typeof result === 'number' && isNaN(result))
})

it('converts convertable objects', function () {
// eslint-disable-next-line no-new-wrappers
var result = toInteger(new Number(123))
assert(result === 123)
})

it('returns NaN for non-convertable objects', function () {
// eslint-disable-next-line no-new-wrappers
var result = toInteger({})
assert(typeof result === 'number' && isNaN(result))
})
})
3 changes: 2 additions & 1 deletion src/addDays/index.js
@@ -1,3 +1,4 @@
import toInteger from '../_lib/toInteger/index.js'
import toDate from '../toDate/index.js'

/**
Expand Down Expand Up @@ -27,7 +28,7 @@ export default function addDays (dirtyDate, dirtyAmount, dirtyOptions) {
}

var date = toDate(dirtyDate, dirtyOptions)
var amount = Number(dirtyAmount)
var amount = toInteger(dirtyAmount)
date.setDate(date.getDate() + amount)
return date
}
5 changes: 5 additions & 0 deletions src/addDays/test.js
Expand Up @@ -20,6 +20,11 @@ describe('addDays', function () {
assert.deepEqual(result, new Date(2014, 8 /* Sep */, 11))
})

it('converts a fractional number to an integer', function () {
var result = addDays(new Date(2014, 8 /* Sep */, 1), 10.5)
assert.deepEqual(result, new Date(2014, 8 /* Sep */, 11))
})

it('implicitly converts number arguments', function () {
// $ExpectedMistake
var result = addDays(new Date(2014, 8 /* Sep */, 1), '10')
Expand Down
3 changes: 2 additions & 1 deletion src/addHours/index.js
@@ -1,3 +1,4 @@
import toInteger from '../_lib/toInteger/index.js'
import addMilliseconds from '../addMilliseconds/index.js'

var MILLISECONDS_IN_HOUR = 3600000
Expand Down Expand Up @@ -28,6 +29,6 @@ export default function addHours (dirtyDate, dirtyAmount, dirtyOptions) {
throw new TypeError('2 arguments required, but only ' + arguments.length + ' present')
}

var amount = Number(dirtyAmount)
var amount = toInteger(dirtyAmount)
return addMilliseconds(dirtyDate, amount * MILLISECONDS_IN_HOUR, dirtyOptions)
}
5 changes: 5 additions & 0 deletions src/addHours/test.js
Expand Up @@ -24,6 +24,11 @@ describe('addHours', function () {
assert.deepEqual(result, new Date(2014, 6 /* Jul */, 12, 1, 0))
})

it('converts a fractional number to an integer', function () {
var result = addHours(new Date(2014, 6 /* Jul */, 10, 23, 0), 2.5)
assert.deepEqual(result, new Date(2014, 6 /* Jul */, 11, 1, 0))
})

it('implicitly converts number arguments', function () {
// $ExpectedMistake
var result = addHours(new Date(2014, 6 /* Jul */, 10, 23, 0), '2')
Expand Down
3 changes: 2 additions & 1 deletion src/addISOWeekYears/index.js
@@ -1,3 +1,4 @@
import toInteger from '../_lib/toInteger/index.js'
import getISOWeekYear from '../getISOWeekYear/index.js'
import setISOWeekYear from '../setISOWeekYear/index.js'

Expand Down Expand Up @@ -29,6 +30,6 @@ export default function addISOWeekYears (dirtyDate, dirtyAmount, dirtyOptions) {
throw new TypeError('2 arguments required, but only ' + arguments.length + ' present')
}

var amount = Number(dirtyAmount)
var amount = toInteger(dirtyAmount)
return setISOWeekYear(dirtyDate, getISOWeekYear(dirtyDate, dirtyOptions) + amount, dirtyOptions)
}