From 02e570abeedce0c9ef6f2d893ff5b4d500944b19 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Tue, 15 Oct 2019 15:34:54 -0700 Subject: [PATCH 1/3] Implement "in" expression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ádám Barancsuk Co-authored-by: Ryan Hamley --- src/style-spec/expression/definitions/in.js | 55 +++++++++++++++++++ .../expression/definitions/index.js | 2 + src/style-spec/feature_filter/index.js | 1 + src/style-spec/reference/v8.json | 9 +++ .../expression-tests/in/basic/test.json | 30 ++++++++++ 5 files changed, 97 insertions(+) create mode 100644 src/style-spec/expression/definitions/in.js create mode 100644 test/integration/expression-tests/in/basic/test.json diff --git a/src/style-spec/expression/definitions/in.js b/src/style-spec/expression/definitions/in.js new file mode 100644 index 00000000000..8ce35c6a78c --- /dev/null +++ b/src/style-spec/expression/definitions/in.js @@ -0,0 +1,55 @@ +// @flow + +import {array, ValueType, BooleanType} from '../types'; + +import type {Expression} from '../expression'; +import type ParsingContext from '../parsing_context'; +import type EvaluationContext from '../evaluation_context'; +import type {Type} from '../types'; +import type {Value} from '../values'; + +class In implements Expression { + type: Type; + needle: Expression; + haystack: Expression; + + constructor(needle: Expression, haystack: Expression) { + this.type = BooleanType; + this.needle = needle; + this.haystack = haystack; + } + + static parse(args: $ReadOnlyArray, context: ParsingContext) { + if (args.length !== 3) + return context.error(`Expected 2 arguments, but found ${args.length - 1} instead.`); + + const needle = context.parse(args[1], 1, ValueType); + const haystack = context.parse(args[2], 2, array(ValueType)); + + if (!needle || !haystack) return null; + + return new In(needle, haystack); + } + + evaluate(ctx: EvaluationContext) { + const needle = (this.needle.evaluate(ctx): any); + const haystack = ((this.haystack.evaluate(ctx): any): Array); + + return haystack.indexOf(needle) >= 0; + } + + eachChild(fn: (Expression) => void) { + fn(this.needle); + fn(this.haystack); + } + + possibleOutputs() { + return [true, false]; + } + + serialize() { + return ["in", this.needle.serialize(), this.haystack.serialize()]; + } +} + +export default In; diff --git a/src/style-spec/expression/definitions/index.js b/src/style-spec/expression/definitions/index.js index 1eded2452fa..7ec0107a549 100644 --- a/src/style-spec/expression/definitions/index.js +++ b/src/style-spec/expression/definitions/index.js @@ -23,6 +23,7 @@ import Literal from './literal'; import Assertion from './assertion'; import Coercion from './coercion'; import At from './at'; +import In from './in'; import Match from './match'; import Case from './case'; import Step from './step'; @@ -61,6 +62,7 @@ const expressions: ExpressionRegistry = { 'collator': CollatorExpression, 'format': FormatExpression, 'image': ImageExpression, + 'in': In, 'interpolate': Interpolate, 'interpolate-hcl': Interpolate, 'interpolate-lab': Interpolate, diff --git a/src/style-spec/feature_filter/index.js b/src/style-spec/feature_filter/index.js index c3f45a2dfb1..d5bdb16fa02 100644 --- a/src/style-spec/feature_filter/index.js +++ b/src/style-spec/feature_filter/index.js @@ -21,6 +21,7 @@ function isExpressionFilter(filter: any) { return filter.length >= 2 && filter[1] !== '$id' && filter[1] !== '$type'; case 'in': + return filter.length >= 3 && Array.isArray(filter[2]); case '!in': case '!has': case 'none': diff --git a/src/style-spec/reference/v8.json b/src/style-spec/reference/v8.json index aae6bc2c5d2..cd25f596411 100644 --- a/src/style-spec/reference/v8.json +++ b/src/style-spec/reference/v8.json @@ -2606,6 +2606,15 @@ } } }, + "in": { + "doc": "Determines whether an item exists in an array.", + "group": "Lookup", + "sdk-support": { + "basic functionality": { + "js": "1.6.0" + } + } + }, "case": { "doc": "Selects the first output whose corresponding test condition evaluates to true, or the fallback value otherwise.", "group": "Decision", diff --git a/test/integration/expression-tests/in/basic/test.json b/test/integration/expression-tests/in/basic/test.json new file mode 100644 index 00000000000..e3da9d024f0 --- /dev/null +++ b/test/integration/expression-tests/in/basic/test.json @@ -0,0 +1,30 @@ +{ + "expression": [ + "boolean", + ["in", ["get", "i"], ["array", ["get", "arr"]]] + ], + "inputs": [ + [{}, {"properties": {"i": null, "arr": [9, 8, 7]}}], + [{}, {"properties": {"i": 1, "arr": [9, 8, 7]}}], + [{}, {"properties": {"i": 9, "arr": [9, 8, 7]}}], + [{}, {"properties": {"i": 1, "arr": null}}] + ], + "expected": { + "compiled": { + "result": "success", + "isFeatureConstant": false, + "isZoomConstant": true, + "type": "boolean" + }, + "outputs": [ + false, + false, + true, + {"error":"Expected value to be of type array, but found null instead."} + ], + "serialized": [ + "boolean", + ["in", ["get", "i"], ["array", ["get", "arr"]]] + ] + } +} From 2060260850bf70236008e43a6dcc8cc4c546883a Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 18 Oct 2019 16:55:58 -0700 Subject: [PATCH 2/3] Extend in operator to handle substrings --- src/style-spec/expression/definitions/in.js | 46 +++++++++++++++++++-- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/src/style-spec/expression/definitions/in.js b/src/style-spec/expression/definitions/in.js index 8ce35c6a78c..8dfad83e95c 100644 --- a/src/style-spec/expression/definitions/in.js +++ b/src/style-spec/expression/definitions/in.js @@ -1,6 +1,8 @@ // @flow -import {array, ValueType, BooleanType} from '../types'; +import {ValueType, BooleanType, toString} from '../types'; +import RuntimeError from '../runtime_error'; +import {typeOf} from '../values'; import type {Expression} from '../expression'; import type ParsingContext from '../parsing_context'; @@ -8,6 +10,26 @@ import type EvaluationContext from '../evaluation_context'; import type {Type} from '../types'; import type {Value} from '../values'; +// haystack should only be able to be a string or an array +function isComparableType(type: Type) { + return type.kind === 'boolean' || + type.kind === 'string' || + type.kind === 'number' || + type.kind === 'null' || + type.kind === 'value'; +} + +function isComparableRuntimeValue(needle: boolean | string | number | null) { + return typeof needle === 'boolean' || + typeof needle === 'string' || + typeof needle === 'number'; +} + +function isSearchableRuntimeValue(haystack: Array | string) { + return Array.isArray(haystack) || + typeof haystack === 'string'; +} + class In implements Expression { type: Type; needle: Expression; @@ -20,20 +42,36 @@ class In implements Expression { } static parse(args: $ReadOnlyArray, context: ParsingContext) { - if (args.length !== 3) + if (args.length !== 3) { return context.error(`Expected 2 arguments, but found ${args.length - 1} instead.`); + } const needle = context.parse(args[1], 1, ValueType); - const haystack = context.parse(args[2], 2, array(ValueType)); + + const haystack = context.parse(args[2], 2, ValueType); if (!needle || !haystack) return null; + if (!isComparableType(needle.type)) { + return context.error(`Expected first argument to be of type boolean, string, number or null, but found ${toString(needle.type)}`); + } + return new In(needle, haystack); } evaluate(ctx: EvaluationContext) { const needle = (this.needle.evaluate(ctx): any); - const haystack = ((this.haystack.evaluate(ctx): any): Array); + const haystack = (this.haystack.evaluate(ctx): any); + + if (!needle || !haystack) return false; + + if (!isComparableRuntimeValue(needle)) { + throw new RuntimeError(`Expected first argument to be of type boolean, string or number, but found ${toString(typeOf(needle))} instead.`); + } + + if (!isSearchableRuntimeValue(haystack)) { + throw new RuntimeError(`Expected second argument to be of type array or string, but found ${toString(typeOf(haystack))} instead.`); + } return haystack.indexOf(needle) >= 0; } From 1eb592ab1300631e83bf1eecb8490cbc8f922f6b Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 25 Oct 2019 19:02:13 -0700 Subject: [PATCH 3/3] Add tests --- src/style-spec/expression/definitions/in.js | 3 +- src/style-spec/reference/v8.json | 2 +- .../in/{basic => assert-array}/test.json | 0 .../in/assert-string/test.json | 30 ++++++++++++++++ .../expression-tests/in/basic-array/test.json | 34 ++++++++++++++++++ .../in/basic-string/test.json | 36 +++++++++++++++++++ .../in/invalid-haystack/test.json | 28 +++++++++++++++ .../in/invalid-needle/test.json | 26 ++++++++++++++ 8 files changed, 156 insertions(+), 3 deletions(-) rename test/integration/expression-tests/in/{basic => assert-array}/test.json (100%) create mode 100644 test/integration/expression-tests/in/assert-string/test.json create mode 100644 test/integration/expression-tests/in/basic-array/test.json create mode 100644 test/integration/expression-tests/in/basic-string/test.json create mode 100644 test/integration/expression-tests/in/invalid-haystack/test.json create mode 100644 test/integration/expression-tests/in/invalid-needle/test.json diff --git a/src/style-spec/expression/definitions/in.js b/src/style-spec/expression/definitions/in.js index 8dfad83e95c..4ab63beb81f 100644 --- a/src/style-spec/expression/definitions/in.js +++ b/src/style-spec/expression/definitions/in.js @@ -10,7 +10,6 @@ import type EvaluationContext from '../evaluation_context'; import type {Type} from '../types'; import type {Value} from '../values'; -// haystack should only be able to be a string or an array function isComparableType(type: Type) { return type.kind === 'boolean' || type.kind === 'string' || @@ -53,7 +52,7 @@ class In implements Expression { if (!needle || !haystack) return null; if (!isComparableType(needle.type)) { - return context.error(`Expected first argument to be of type boolean, string, number or null, but found ${toString(needle.type)}`); + return context.error(`Expected first argument to be of type boolean, string, number or null, but found ${toString(needle.type)} instead`); } return new In(needle, haystack); diff --git a/src/style-spec/reference/v8.json b/src/style-spec/reference/v8.json index cd25f596411..9a6ef879025 100644 --- a/src/style-spec/reference/v8.json +++ b/src/style-spec/reference/v8.json @@ -2607,7 +2607,7 @@ } }, "in": { - "doc": "Determines whether an item exists in an array.", + "doc": "Determines whether an item exists in an array or a substring exists in a string.", "group": "Lookup", "sdk-support": { "basic functionality": { diff --git a/test/integration/expression-tests/in/basic/test.json b/test/integration/expression-tests/in/assert-array/test.json similarity index 100% rename from test/integration/expression-tests/in/basic/test.json rename to test/integration/expression-tests/in/assert-array/test.json diff --git a/test/integration/expression-tests/in/assert-string/test.json b/test/integration/expression-tests/in/assert-string/test.json new file mode 100644 index 00000000000..ce59df2ab02 --- /dev/null +++ b/test/integration/expression-tests/in/assert-string/test.json @@ -0,0 +1,30 @@ +{ + "expression": [ + "boolean", + ["in", ["get", "substr"], ["string", ["get", "str"]]] + ], + "inputs": [ + [{}, {"properties": {"substr": null, "str": "helloworld"}}], + [{}, {"properties": {"substr": "foo", "str": "helloworld"}}], + [{}, {"properties": {"substr": "low", "str": "helloworld"}}], + [{}, {"properties": {"substr": "low", "str": null}}] + ], + "expected": { + "compiled": { + "result": "success", + "isFeatureConstant": false, + "isZoomConstant": true, + "type": "boolean" + }, + "outputs": [ + false, + false, + true, + {"error":"Expected value to be of type string, but found null instead."} + ], + "serialized": [ + "boolean", + ["in", ["get", "substr"], ["string", ["get", "str"]]] + ] + } +} diff --git a/test/integration/expression-tests/in/basic-array/test.json b/test/integration/expression-tests/in/basic-array/test.json new file mode 100644 index 00000000000..91b2270d2c5 --- /dev/null +++ b/test/integration/expression-tests/in/basic-array/test.json @@ -0,0 +1,34 @@ +{ + "expression": [ + "boolean", + ["in", ["get", "i"], ["get", "arr"]] + ], + "inputs": [ + [{}, {"properties": {"i": null, "arr": [9, 8, 7]}}], + [{}, {"properties": {"i": 1, "arr": [9, 8, 7]}}], + [{}, {"properties": {"i": 9, "arr": [9, 8, 7]}}], + [{}, {"properties": {"i": "foo", "arr": ["baz", "bar", "hello", "foo", "world"]}}], + [{}, {"properties": {"i": true, "arr": ["foo", 123, null, 456, false, {}, true]}}], + [{}, {"properties": {"i": 1, "arr": null}}] + ], + "expected": { + "compiled": { + "result": "success", + "isFeatureConstant": false, + "isZoomConstant": true, + "type": "boolean" + }, + "outputs": [ + false, + false, + true, + true, + true, + false + ], + "serialized": [ + "boolean", + ["in", ["get", "i"], ["get", "arr"]] + ] + } +} diff --git a/test/integration/expression-tests/in/basic-string/test.json b/test/integration/expression-tests/in/basic-string/test.json new file mode 100644 index 00000000000..f985e9ca98b --- /dev/null +++ b/test/integration/expression-tests/in/basic-string/test.json @@ -0,0 +1,36 @@ +{ + "expression": [ + "boolean", + ["in", ["get", "substr"], ["get", "str"]] + ], + "inputs": [ + [{}, {"properties": {"substr": null, "str": "helloworld"}}], + [{}, {"properties": {"substr": "foo", "str": "helloworld"}}], + [{}, {"properties": {"substr": "low", "str": "helloworld"}}], + [{}, {"properties": {"substr": true, "str": "falsetrue"}}], + [{}, {"properties": {"substr": false, "str": "falsetrue"}}], + [{}, {"properties": {"substr": 123, "str": "hello123world"}}], + [{}, {"properties": {"substr": "low", "str": null}}] + ], + "expected": { + "compiled": { + "result": "success", + "isFeatureConstant": false, + "isZoomConstant": true, + "type": "boolean" + }, + "outputs": [ + false, + false, + true, + true, + false, + true, + false + ], + "serialized": [ + "boolean", + ["in", ["get", "substr"], ["get", "str"]] + ] + } +} diff --git a/test/integration/expression-tests/in/invalid-haystack/test.json b/test/integration/expression-tests/in/invalid-haystack/test.json new file mode 100644 index 00000000000..f855578eb8d --- /dev/null +++ b/test/integration/expression-tests/in/invalid-haystack/test.json @@ -0,0 +1,28 @@ +{ + "expression": [ + "boolean", + ["in", ["get", "needle"], ["get", "haystack"]] + ], + "inputs": [ + [{}, {"properties": {"needle": 1, "haystack": 123}}], + [{}, {"properties": {"needle": "foo", "haystack": {}}}], + [{}, {"properties": {"needle": "foo", "haystack": null}}] + ], + "expected": { + "compiled": { + "result": "success", + "isFeatureConstant": false, + "isZoomConstant": true, + "type": "boolean" + }, + "outputs": [ + {"error":"Expected second argument to be of type array or string, but found number instead."}, + {"error":"Expected second argument to be of type array or string, but found object instead."}, + false + ], + "serialized": [ + "boolean", + ["in", ["get", "needle"], ["get", "haystack"]] + ] + } +} diff --git a/test/integration/expression-tests/in/invalid-needle/test.json b/test/integration/expression-tests/in/invalid-needle/test.json new file mode 100644 index 00000000000..0ff5cae1b2e --- /dev/null +++ b/test/integration/expression-tests/in/invalid-needle/test.json @@ -0,0 +1,26 @@ +{ + "expression": [ + "boolean", + ["in", ["get", "needle"], ["get", "haystack"]] + ], + "inputs": [ + [{}, {"properties": {"needle": {}, "haystack": [9, 8, 7]}}], + [{}, {"properties": {"needle": {}, "haystack": "helloworld"}}] + ], + "expected": { + "compiled": { + "result": "success", + "isFeatureConstant": false, + "isZoomConstant": true, + "type": "boolean" + }, + "outputs": [ + {"error":"Expected first argument to be of type boolean, string or number, but found object instead."}, + {"error":"Expected first argument to be of type boolean, string or number, but found object instead."} + ], + "serialized": [ + "boolean", + ["in", ["get", "needle"], ["get", "haystack"]] + ] + } +}