[bugfix] isBetween should return false for invalid dates (#4417)

* [bugfix]: isBetween should return false for invalid dates

* Tweak to use void not null for TS 1.x

* Add tests requested in code review

diff --git a/moment.d.ts b/moment.d.ts
index 55a9d1f98bf2827f00211c4d03ee2236d1e42627..dfc3d5422baa7549609f718c75966b08050e923f 100644 (file)
--- a/moment.d.ts
+++ b/moment.d.ts
@@ -302,7 +302,7 @@ declare namespace moment {
 
     type DurationAs = Base;
 
-    type StartOf = Base | _quarter | _isoWeek | _date;
+    type StartOf = Base | _quarter | _isoWeek | _date | void; // null
 
     type Diff = Base | _quarter;
 

diff --git a/src/lib/moment/compare.js b/src/lib/moment/compare.js
index b26bac633c4c697aa9e556603fbdac4c00be6e36..8b23dba0267d9b2b59fa09398d44d25daeb125c8 100644 (file)
--- a/src/lib/moment/compare.js
+++ b/src/lib/moment/compare.js
@@ -1,14 +1,13 @@
 import { isMoment } from './constructor';
 import { normalizeUnits } from '../units/aliases';
 import { createLocal } from '../create/local';
-import isUndefined from '../utils/is-undefined';
 
 export function isAfter (input, units) {
     var localInput = isMoment(input) ? input : createLocal(input);
     if (!(this.isValid() && localInput.isValid())) {
         return false;
     }
-    units = normalizeUnits(!isUndefined(units) ? units : 'millisecond');
+    units = normalizeUnits(units) || 'millisecond';
     if (units === 'millisecond') {
         return this.valueOf() > localInput.valueOf();
     } else {
@@ -21,7 +20,7 @@ export function isBefore (input, units) {
     if (!(this.isValid() && localInput.isValid())) {
         return false;
     }
-    units = normalizeUnits(!isUndefined(units) ? units : 'millisecond');
+    units = normalizeUnits(units) || 'millisecond';
     if (units === 'millisecond') {
         return this.valueOf() < localInput.valueOf();
     } else {
@@ -30,9 +29,14 @@ export function isBefore (input, units) {
 }
 
 export function isBetween (from, to, units, inclusivity) {
+    var localFrom = isMoment(from) ? from : createLocal(from),
+        localTo = isMoment(to) ? to : createLocal(to);
+    if (!(this.isValid() && localFrom.isValid() && localTo.isValid())) {
+        return false;
+    }
     inclusivity = inclusivity || '()';
-    return (inclusivity[0] === '(' ? this.isAfter(from, units) : !this.isBefore(from, units)) &&
-        (inclusivity[1] === ')' ? this.isBefore(to, units) : !this.isAfter(to, units));
+    return (inclusivity[0] === '(' ? this.isAfter(localFrom, units) : !this.isBefore(localFrom, units)) &&
+        (inclusivity[1] === ')' ? this.isBefore(localTo, units) : !this.isAfter(localTo, units));
 }
 
 export function isSame (input, units) {
@@ -41,7 +45,7 @@ export function isSame (input, units) {
     if (!(this.isValid() && localInput.isValid())) {
         return false;
     }
-    units = normalizeUnits(units || 'millisecond');
+    units = normalizeUnits(units) || 'millisecond';
     if (units === 'millisecond') {
         return this.valueOf() === localInput.valueOf();
     } else {
@@ -51,9 +55,9 @@ export function isSame (input, units) {
 }
 
 export function isSameOrAfter (input, units) {
-    return this.isSame(input, units) || this.isAfter(input,units);
+    return this.isSame(input, units) || this.isAfter(input, units);
 }
 
 export function isSameOrBefore (input, units) {
-    return this.isSame(input, units) || this.isBefore(input,units);
+    return this.isSame(input, units) || this.isBefore(input, units);
 }

diff --git a/src/test/moment/is_between.js b/src/test/moment/is_between.js
index b58f4ec93b66373390b36ee48f5c0b2c1f18b6d4..656112cc2d918927737a6eb7d0352e74353af487 100644 (file)
--- a/src/test/moment/is_between.js
+++ b/src/test/moment/is_between.js
@@ -226,7 +226,7 @@ test('is between year', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2010, 5, 6, 7, 8, 9, 10)),
                 moment(new Date(2011, 5, 6, 7, 8, 9, 10)), 'year'), false, 'year is later');
-    assert.equal(m.isBetween(m, 'year'), false, 'same moments are not between the same year');
+    assert.equal(m.isBetween(m, m, 'year'), false, 'same moments are not between the same year');
     assert.equal(+m, +mCopy, 'isBetween year should not change moment');
 });
 
@@ -247,7 +247,7 @@ test('is between month', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 11, 6, 7, 8, 9, 10)),
                 moment(new Date(2011, 1, 6, 7, 8, 9, 10)), 'month'), false, 'month is later');
-    assert.equal(m.isBetween(m, 'month'), false, 'same moments are not between the same month');
+    assert.equal(m.isBetween(m, m, 'month'), false, 'same moments are not between the same month');
     assert.equal(+m, +mCopy, 'isBetween month should not change moment');
 });
 
@@ -268,7 +268,7 @@ test('is between day', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 1, 1, 7, 8, 9, 10)),
                 moment(new Date(2011, 1, 2, 7, 8, 9, 10)), 'day'), false, 'day is later');
-    assert.equal(m.isBetween(m, 'day'), false, 'same moments are not between the same day');
+    assert.equal(m.isBetween(m, m, 'day'), false, 'same moments are not between the same day');
     assert.equal(+m, +mCopy, 'isBetween day should not change moment');
 });
 
@@ -289,7 +289,7 @@ test('is between hour', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 1, 2, 7, 8, 9, 10)),
                 moment(new Date(2011, 1, 2, 7, 8, 9, 10)), 'hour'), false, 'hour is later');
-    assert.equal(m.isBetween(m, 'hour'), false, 'same moments are not between the same hour');
+    assert.equal(m.isBetween(m, m, 'hour'), false, 'same moments are not between the same hour');
     assert.equal(+m, +mCopy, 'isBetween hour should not change moment');
 });
 
@@ -310,7 +310,7 @@ test('is between minute', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 1, 2, 3, 2, 9, 10)),
                 moment(new Date(2011, 1, 2, 3, 3, 59, 999)), 'minute'), false, 'minute is later');
-    assert.equal(m.isBetween(m, 'minute'), false, 'same moments are not between the same minute');
+    assert.equal(m.isBetween(m, m, 'minute'), false, 'same moments are not between the same minute');
     assert.equal(+m, +mCopy, 'isBetween minute should not change moment');
 });
 
@@ -331,7 +331,7 @@ test('is between second', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 1, 2, 3, 4, 3, 10)),
                 moment(new Date(2011, 1, 2, 3, 4, 4, 999)), 'second'), false, 'second is later');
-    assert.equal(m.isBetween(m, 'second'), false, 'same moments are not between the same second');
+    assert.equal(m.isBetween(m, m, 'second'), false, 'same moments are not between the same second');
     assert.equal(+m, +mCopy, 'isBetween second should not change moment');
 });
 
@@ -352,6 +352,30 @@ test('is between millisecond', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 1, 2, 3, 4, 5, 4)),
                 moment(new Date(2011, 1, 2, 3, 4, 5, 6)), 'millisecond'), false, 'millisecond is later');
-    assert.equal(m.isBetween(m, 'millisecond'), false, 'same moments are not between the same millisecond');
+    assert.equal(m.isBetween(m, m, 'millisecond'), false, 'same moments are not between the same millisecond');
     assert.equal(+m, +mCopy, 'isBetween millisecond should not change moment');
 });
+
+test('is between invalid', function (assert) {
+    var invalid = moment(NaN),
+        valid = moment(new Date(2011, 1, 2, 3, 4, 5, 6)),
+        validFrom = moment(new Date(2010, 1, 2, 3, 4, 5, 6)),
+        validTo = moment(new Date(2012, 1, 2, 3, 4, 5, 6));
+    assert.equal(invalid.isBetween(validFrom, validTo), false, 'this instance invalid');
+    assert.equal(invalid.isBetween(validFrom, validTo, '[]'), false, 'this instance invalid []');
+    assert.equal(invalid.isBetween(validFrom, validTo, '[)'), false, 'this instance invalid [)');
+    assert.equal(invalid.isBetween(validFrom, validTo, '(]'), false, 'this instance invalid (]');
+    assert.equal(invalid.isBetween(validFrom, validTo, '()'), false, 'this instance invalid ()');
+
+    assert.equal(valid.isBetween(invalid, validTo), false, 'from invalid moment');
+    assert.equal(valid.isBetween(invalid, validTo, '[]'), false, 'from invalid moment []');
+    assert.equal(valid.isBetween(invalid, validTo, '[)'), false, 'from invalid moment [)');
+    assert.equal(valid.isBetween(invalid, validTo, '(]'), false, 'from invalid moment (]');
+    assert.equal(valid.isBetween(invalid, validTo, '()'), false, 'from invalid moment ()');
+
+    assert.equal(valid.isBetween(validFrom, invalid), false, 'to invalid moment');
+    assert.equal(valid.isBetween(validFrom, invalid, '[]'), false, 'to invalid moment []');
+    assert.equal(valid.isBetween(validFrom, invalid, '[)'), false, 'to invalid moment [)');
+    assert.equal(valid.isBetween(validFrom, invalid, '(]'), false, 'to invalid moment (]');
+    assert.equal(valid.isBetween(validFrom, invalid, '()'), false, 'to invalid moment ()');
+});

diff --git a/typing-tests/moment-tests.ts b/typing-tests/moment-tests.ts
index 2e74f819ac9621f73cae87fe289ed77de0516625..6d4d1635f55be833140738b3862aefb993d7ba6f 100644 (file)
--- a/typing-tests/moment-tests.ts
+++ b/typing-tests/moment-tests.ts
@@ -229,6 +229,7 @@ moment.isDuration(moment.duration());
 moment().isBetween(moment(), moment());
 moment().isBetween(new Date(), new Date());
 moment().isBetween([1,1,2000], [1,1,2001], "year");
+moment().isBetween([1,1,2000], [1,1,2001], null, "()");
 
 moment.localeData('fr');
 moment(1316116057189).fromNow();