From c2289d7723a57a62e1c5f96861f5a08d80174b45 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 9 Aug 2019 12:00:28 -0700 Subject: [PATCH 1/6] Number validation should fail on NaN --- src/style-spec/util/get_type.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/style-spec/util/get_type.js b/src/style-spec/util/get_type.js index c31925f35a3..2d1ff998082 100644 --- a/src/style-spec/util/get_type.js +++ b/src/style-spec/util/get_type.js @@ -1,6 +1,8 @@ export default function getType(val) { - if (val instanceof Number) { + if (Number.isNaN(val)) { + return 'NaN'; + } else if (val instanceof Number) { return 'number'; } else if (val instanceof String) { return 'string'; From 74c85f3f01fdf2b438a17e1785296515c67bac58 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 9 Aug 2019 13:06:15 -0700 Subject: [PATCH 2/6] Avoid Number.isNaN --- src/style-spec/util/get_type.js | 4 +- src/style-spec/validate/validate_number.js | 7 +- .../style-spec/fixture/numbers.input.json | 94 +++++++++++++++++++ .../style-spec/fixture/numbers.output.json | 21 +++++ 4 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 test/unit/style-spec/fixture/numbers.input.json create mode 100644 test/unit/style-spec/fixture/numbers.output.json diff --git a/src/style-spec/util/get_type.js b/src/style-spec/util/get_type.js index 2d1ff998082..c31925f35a3 100644 --- a/src/style-spec/util/get_type.js +++ b/src/style-spec/util/get_type.js @@ -1,8 +1,6 @@ export default function getType(val) { - if (Number.isNaN(val)) { - return 'NaN'; - } else if (val instanceof Number) { + if (val instanceof Number) { return 'number'; } else if (val instanceof String) { return 'string'; diff --git a/src/style-spec/validate/validate_number.js b/src/style-spec/validate/validate_number.js index adf7f3e5ef3..1db12f0364d 100644 --- a/src/style-spec/validate/validate_number.js +++ b/src/style-spec/validate/validate_number.js @@ -6,7 +6,12 @@ export default function validateNumber(options) { const key = options.key; const value = options.value; const valueSpec = options.valueSpec; - const type = getType(value); + let type = getType(value); + + // eslint-disable-next-line no-self-compare + if (type === 'number' && value !== value) { + type = 'NaN'; + } if (type !== 'number') { return [new ValidationError(key, value, `number expected, ${type} found`)]; diff --git a/test/unit/style-spec/fixture/numbers.input.json b/test/unit/style-spec/fixture/numbers.input.json new file mode 100644 index 00000000000..b3146196d77 --- /dev/null +++ b/test/unit/style-spec/fixture/numbers.input.json @@ -0,0 +1,94 @@ +{ + "version": 8, + "sources": { + "point": { + "type": "geojson", + "data": { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [ 0, 0 ] + } + } + ] + } + } + }, + "layers": [ + { + "id": "valid", + "type": "circle", + "source": "point", + "paint": { + "circle-radius": 5 + } + }, + { + "id": "zero", + "type": "circle", + "source": "point", + "paint": { + "circle-radius": 0 + } + }, + { + "id": "less-than-zero", + "type": "circle", + "source": "point", + "paint": { + "circle-radius": -1 + } + }, + { + "id": "null-not-number", + "type": "circle", + "source": "point", + "paint": { + "circle-radius": null + } + }, + { + "id": "object-not-number", + "type": "circle", + "source": "point", + "paint": { + "circle-radius": {} + } + }, + { + "id": "array-not-number", + "type": "circle", + "source": "point", + "paint": { + "circle-radius": [] + } + }, + { + "id": "boolean-not-number", + "type": "circle", + "source": "point", + "paint": { + "circle-radius": true + } + }, + { + "id": "expression", + "type": "circle", + "source": "point", + "paint": { + "circle-radius": ["sqrt", 16] + } + }, + { + "id": "expression-invalid", + "type": "circle", + "source": "point", + "paint": { + "circle-radius": ["/", 0, 0] + } + } + ] +} diff --git a/test/unit/style-spec/fixture/numbers.output.json b/test/unit/style-spec/fixture/numbers.output.json new file mode 100644 index 00000000000..8fadd34d7ee --- /dev/null +++ b/test/unit/style-spec/fixture/numbers.output.json @@ -0,0 +1,21 @@ +[ + { + "message": "layers[2].paint.circle-radius: -1 is less than the minimum value 0", + "line": 42 + }, + { + "message": "layers[3].paint.circle-radius: number expected, null found" + }, + { + "message": "layers[4].paint.circle-radius: missing required property \"stops\"", + "line": 58 + }, + { + "message": "layers[5].paint.circle-radius: number expected, array found", + "line": 66 + }, + { + "message": "layers[6].paint.circle-radius: number expected, boolean found", + "line": 74 + } +] From 23981b7f0ea5e939819f5b6e8b0e4fc89ec9205a Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 20 Nov 2019 12:47:33 -0800 Subject: [PATCH 3/6] Add runtime checks --- src/style-spec/expression/index.js | 3 ++- .../fixture/numbers.output-api-supported.json | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 test/unit/style-spec/fixture/numbers.output-api-supported.json diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js index 02c2163c9a7..c100497a702 100644 --- a/src/style-spec/expression/index.js +++ b/src/style-spec/expression/index.js @@ -78,7 +78,8 @@ export class StyleExpression { try { const val = this.expression.evaluate(this._evaluator); - if (val === null || val === undefined) { + // eslint-disable-next-line no-self-compare + if (val === null || val === undefined || (typeof val === 'number' && val !== val)) { return this._defaultValue; } if (this._enumValues && !(val in this._enumValues)) { diff --git a/test/unit/style-spec/fixture/numbers.output-api-supported.json b/test/unit/style-spec/fixture/numbers.output-api-supported.json new file mode 100644 index 00000000000..d264215475a --- /dev/null +++ b/test/unit/style-spec/fixture/numbers.output-api-supported.json @@ -0,0 +1,25 @@ +[ + { + "line": 42, + "message": "layers[2].paint.circle-radius: -1 is less than the minimum value 0" + }, + { + "message": "layers[3].paint.circle-radius: number expected, null found" + }, + { + "line": 58, + "message": "layers[4].paint.circle-radius: missing required property \"stops\"" + }, + { + "line": 66, + "message": "layers[5].paint.circle-radius: number expected, array found" + }, + { + "line": 74, + "message": "layers[6].paint.circle-radius: number expected, boolean found" + }, + { + "line": 6, + "message": "source.data: Unsupported property \"data\"" + } + ] From b3ff4099090c230d22695c28167f7717a6bd2bd9 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Thu, 21 Nov 2019 13:44:49 -0800 Subject: [PATCH 4/6] Add render and unit test --- .../render-tests/text-size/nan/expected.png | Bin 0 -> 681 bytes .../render-tests/text-size/nan/style.json | 37 ++++++++++++++++++ .../style-spec/fixture/numbers.input.json | 9 +++++ 3 files changed, 46 insertions(+) create mode 100644 test/integration/render-tests/text-size/nan/expected.png create mode 100644 test/integration/render-tests/text-size/nan/style.json diff --git a/test/integration/render-tests/text-size/nan/expected.png b/test/integration/render-tests/text-size/nan/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..d27ef2f5c9784ba76540af6fd9dc49e473d8c48f GIT binary patch literal 681 zcmeAS@N?(olHy`uVBq!ia0vp^4j|0I1|(Ny7TyC=Or9=|Ar*{or0OmKIY}Vi+k^fY zS6O8kKAeBQUHuEc@Dud6hYL*8+fJz|iXY~7LmQS#m)+nt7g)+nq@ z+S{=F1M|MaDr^%XwOpG-vM+Q?#yE4XPh?zP;6FK~%e2EX=79VU*54QUwX>oMIMy%X zeQnVG{eyPZg=V8;CV{Igv_EpKOLlsvu5-Mhic{{O)*1))pf#@QUK$IVcF)oXo-=); z9QO~2?iX&_-yTb(J$d)o!cW1GzweQ+&m-+S2M@nEv_;KRRQaGn(4S7BTY(2n%#yax zay_%Ssj}yB^c{EmWrl6;8$*RtyEZcFCyM+wP@evvbj}g!AIF-vK9Zbb(9JsMs2jS1e22WQ% Jmvv4FO#pJQD7*jw literal 0 HcmV?d00001 diff --git a/test/integration/render-tests/text-size/nan/style.json b/test/integration/render-tests/text-size/nan/style.json new file mode 100644 index 00000000000..ca4ffb6c311 --- /dev/null +++ b/test/integration/render-tests/text-size/nan/style.json @@ -0,0 +1,37 @@ +{ + "version": 8, + "metadata": { + "test": { + "width": 64, + "height": 64 + } + }, + "sources": { + "geojson": { + "type": "geojson", + "data": { + "type": "Point", + "coordinates": [ + 0, + 0 + ] + } + } + }, + "glyphs": "local://glyphs/{fontstack}/{range}.pbf", + "layers": [ + { + "id": "symbol", + "type": "symbol", + "source": "geojson", + "layout": { + "text-size": ["sqrt", -1], + "text-field": "ABC", + "text-font": [ + "Open Sans Semibold", + "Arial Unicode MS Bold" + ] + } + } + ] +} diff --git a/test/unit/style-spec/fixture/numbers.input.json b/test/unit/style-spec/fixture/numbers.input.json index b3146196d77..1e56fb73bce 100644 --- a/test/unit/style-spec/fixture/numbers.input.json +++ b/test/unit/style-spec/fixture/numbers.input.json @@ -82,6 +82,15 @@ "circle-radius": ["sqrt", 16] } }, + { + "id": "expression-nan", + "type": "symbol", + "source": "point", + "paint": {}, + "layout": { + "text-size": ["sqrt", -1] + } + }, { "id": "expression-invalid", "type": "circle", From 042888ac390f6537b9f846396863c83e54fafdd9 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Thu, 21 Nov 2019 13:53:21 -0800 Subject: [PATCH 5/6] Remove not useful unit test --- debug/index.html | 61 ++++++++++++++++++- .../style-spec/fixture/numbers.input.json | 9 --- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/debug/index.html b/debug/index.html index 04198d03ec9..b136795590b 100644 --- a/debug/index.html +++ b/debug/index.html @@ -18,14 +18,73 @@ diff --git a/test/unit/style-spec/fixture/numbers.input.json b/test/unit/style-spec/fixture/numbers.input.json index 1e56fb73bce..b3146196d77 100644 --- a/test/unit/style-spec/fixture/numbers.input.json +++ b/test/unit/style-spec/fixture/numbers.input.json @@ -82,15 +82,6 @@ "circle-radius": ["sqrt", 16] } }, - { - "id": "expression-nan", - "type": "symbol", - "source": "point", - "paint": {}, - "layout": { - "text-size": ["sqrt", -1] - } - }, { "id": "expression-invalid", "type": "circle", From ed5911089991f47c8d8e41487f5139f0d20dac87 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Thu, 21 Nov 2019 13:54:40 -0800 Subject: [PATCH 6/6] Revert debug page change --- debug/index.html | 61 +----------------------------------------------- 1 file changed, 1 insertion(+), 60 deletions(-) diff --git a/debug/index.html b/debug/index.html index b136795590b..04198d03ec9 100644 --- a/debug/index.html +++ b/debug/index.html @@ -18,73 +18,14 @@