From f0f65de1df62c8fee0599b582ba728103d640202 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Sun, 15 Jan 2017 12:20:24 -0800 Subject: [PATCH 1/6] url: add additional tests for URLSearchParams#forEach --- test/parallel/test-whatwg-url-searchparams.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/parallel/test-whatwg-url-searchparams.js b/test/parallel/test-whatwg-url-searchparams.js index ca8f4ff070bfeb..ccc849993e58ce 100644 --- a/test/parallel/test-whatwg-url-searchparams.js +++ b/test/parallel/test-whatwg-url-searchparams.js @@ -44,6 +44,16 @@ n = 0; for (val of sp.values()) { assert.strictEqual(val, String(values[n++])); } +n = 0; +sp.forEach(function(val, key, obj) { + assert.strictEqual(this, undefined); + assert.strictEqual(key, 'a'); + assert.strictEqual(val, String(values[n++])); + assert.strictEqual(obj, sp); +}); +sp.forEach(function() { + assert.strictEqual(this, m); +}, m); m.search = '?a=a&b=b'; assert.strictEqual(sp.toString(), 'a=a&b=b'); From 44acced7958aa2aeca7aebc7d9886239dd624888 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Sun, 15 Jan 2017 12:20:43 -0800 Subject: [PATCH 2/6] url: check forEach callback is a function Per spec. --- lib/internal/url.js | 3 +++ test/parallel/test-whatwg-url-searchparams.js | 2 ++ 2 files changed, 5 insertions(+) diff --git a/lib/internal/url.js b/lib/internal/url.js index b61b699ecdfca7..4113a3bfe887df 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -817,6 +817,9 @@ class URLSearchParams { if (arguments.length < 1) { throw new TypeError('The `callback` argument needs to be specified'); } + if (typeof callback !== 'function') { + throw new TypeError('The `callback` argument must be a function'); + } let list = this[searchParams]; diff --git a/test/parallel/test-whatwg-url-searchparams.js b/test/parallel/test-whatwg-url-searchparams.js index ccc849993e58ce..e6bba03bb7768d 100644 --- a/test/parallel/test-whatwg-url-searchparams.js +++ b/test/parallel/test-whatwg-url-searchparams.js @@ -54,6 +54,8 @@ sp.forEach(function(val, key, obj) { sp.forEach(function() { assert.strictEqual(this, m); }, m); +assert.throws(() => sp.forEach(), TypeError); +assert.throws(() => sp.forEach(1), TypeError); m.search = '?a=a&b=b'; assert.strictEqual(sp.toString(), 'a=a&b=b'); From 182ce97f3bfaf351d2c64a5f65415b2b68443b75 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Thu, 19 Jan 2017 18:02:35 -0800 Subject: [PATCH 3/6] More specific error message tests --- test/parallel/test-whatwg-url-searchparams.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-whatwg-url-searchparams.js b/test/parallel/test-whatwg-url-searchparams.js index e6bba03bb7768d..a5d8b2f595a54b 100644 --- a/test/parallel/test-whatwg-url-searchparams.js +++ b/test/parallel/test-whatwg-url-searchparams.js @@ -54,8 +54,10 @@ sp.forEach(function(val, key, obj) { sp.forEach(function() { assert.strictEqual(this, m); }, m); -assert.throws(() => sp.forEach(), TypeError); -assert.throws(() => sp.forEach(1), TypeError); +assert.throws(() => sp.forEach(), + /^TypeError: The `callback` argument needs to be specified$/); +assert.throws(() => sp.forEach(1), + /^TypeError: The `callback` argument must be a function$/); m.search = '?a=a&b=b'; assert.strictEqual(sp.toString(), 'a=a&b=b'); From c6f590055d2c2448f422acc486d7ee3a9e9dbdcc Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Fri, 20 Jan 2017 11:28:18 -0800 Subject: [PATCH 4/6] Remove obsolete arguments.length check in forEach() --- lib/internal/url.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index 4113a3bfe887df..9c37f78c6def66 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -814,9 +814,6 @@ class URLSearchParams { if (!this || !(this instanceof URLSearchParams)) { throw new TypeError('Value of `this` is not a URLSearchParams'); } - if (arguments.length < 1) { - throw new TypeError('The `callback` argument needs to be specified'); - } if (typeof callback !== 'function') { throw new TypeError('The `callback` argument must be a function'); } From 6775629d71ea619038dcb59a8980caa39bd3a567 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Fri, 20 Jan 2017 11:34:17 -0800 Subject: [PATCH 5/6] Make argument check error messages consistent with rest of code base --- lib/internal/url.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index 9c37f78c6def66..75a72da936646f 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -672,8 +672,7 @@ class URLSearchParams { throw new TypeError('Value of `this` is not a URLSearchParams'); } if (arguments.length < 2) { - throw new TypeError( - 'Both `name` and `value` arguments need to be specified'); + throw new TypeError('"name" and "value" arguments must be specified'); } name = String(name); @@ -687,7 +686,7 @@ class URLSearchParams { throw new TypeError('Value of `this` is not a URLSearchParams'); } if (arguments.length < 1) { - throw new TypeError('The `name` argument needs to be specified'); + throw new TypeError('"name" argument must be specified'); } const list = this[searchParams]; @@ -708,8 +707,7 @@ class URLSearchParams { throw new TypeError('Value of `this` is not a URLSearchParams'); } if (arguments.length < 2) { - throw new TypeError( - 'Both `name` and `value` arguments need to be specified'); + throw new TypeError('"name" and "value" arguments must be specified'); } const list = this[searchParams]; @@ -749,7 +747,7 @@ class URLSearchParams { throw new TypeError('Value of `this` is not a URLSearchParams'); } if (arguments.length < 1) { - throw new TypeError('The `name` argument needs to be specified'); + throw new TypeError('"name" argument must be specified'); } const list = this[searchParams]; @@ -767,7 +765,7 @@ class URLSearchParams { throw new TypeError('Value of `this` is not a URLSearchParams'); } if (arguments.length < 1) { - throw new TypeError('The `name` argument needs to be specified'); + throw new TypeError('"name" argument must be specified'); } const list = this[searchParams]; @@ -786,7 +784,7 @@ class URLSearchParams { throw new TypeError('Value of `this` is not a URLSearchParams'); } if (arguments.length < 1) { - throw new TypeError('The `name` argument needs to be specified'); + throw new TypeError('"name" argument must be specified'); } const list = this[searchParams]; @@ -815,7 +813,7 @@ class URLSearchParams { throw new TypeError('Value of `this` is not a URLSearchParams'); } if (typeof callback !== 'function') { - throw new TypeError('The `callback` argument must be a function'); + throw new TypeError('"callback" argument must be a function'); } let list = this[searchParams]; From 593a0d937aa2de647634d8458d4b1d2505f0430e Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Fri, 20 Jan 2017 20:41:01 -0800 Subject: [PATCH 6/6] Update error messages in test --- test/parallel/test-whatwg-url-searchparams.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-whatwg-url-searchparams.js b/test/parallel/test-whatwg-url-searchparams.js index a5d8b2f595a54b..8b9e65123d9506 100644 --- a/test/parallel/test-whatwg-url-searchparams.js +++ b/test/parallel/test-whatwg-url-searchparams.js @@ -55,9 +55,9 @@ sp.forEach(function() { assert.strictEqual(this, m); }, m); assert.throws(() => sp.forEach(), - /^TypeError: The `callback` argument needs to be specified$/); + /^TypeError: "callback" argument must be a function$/); assert.throws(() => sp.forEach(1), - /^TypeError: The `callback` argument must be a function$/); + /^TypeError: "callback" argument must be a function$/); m.search = '?a=a&b=b'; assert.strictEqual(sp.toString(), 'a=a&b=b');