Skip to content

Commit

Permalink
Improve typing for ==, != expressions (#5840)
Browse files Browse the repository at this point in the history
* Improve typing for ==, != expressions

Closes #5761
Closes #5835

The overloads of == and != are now effectively:
 - `(T1: Comparable, T2: Comparable) => boolean { T1 == T2 }`
 - `(Comparable, value) => boolean`
 - `(value, Comparable) => boolean`

Where `Comparable = string | number | boolean | null`.

* Simplify comparability check

* Add internal docs, fix possibleValues
  • Loading branch information
anandthakker committed Dec 12, 2017
1 parent c1cf5f0 commit 537e4dd
Show file tree
Hide file tree
Showing 18 changed files with 223 additions and 88 deletions.
83 changes: 83 additions & 0 deletions src/style-spec/expression/definitions/equals.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// @flow

const {
ValueType,
BooleanType,
} = require('../types');
const {toString} = require('../types');

import type { Expression } from '../expression';
import type EvaluationContext from '../evaluation_context';
import type ParsingContext from '../parsing_context';
import type { Type } from '../types';

function eq(ctx) { return this.lhs.evaluate(ctx) === this.rhs.evaluate(ctx); }
function ne(ctx) { return this.lhs.evaluate(ctx) !== this.rhs.evaluate(ctx); }

function isComparableType(type: Type) {
return type.kind === 'string' ||
type.kind === 'number' ||
type.kind === 'boolean' ||
type.kind === 'null';
}

/**
* Special form for ==, !=, implementing the following signatures:
* - (T1: Comparable, T2: Comparable) => boolean { T1 == T2 }
* - (Comparable, value) => boolean
* - (value, Comparable) => boolean
*
* Where Comparable = string | number | boolean | null.
*
* Evaluation semantics for the value cases are equivalent to Javascript's
* strict equality (===/!==) -- i.e., when the value argument's type doesn't
* match that of the Comparable argument, == evaluates to false, != to true.
*
* @private
*/
class Equals implements Expression {
type: Type;
lhs: Expression;
rhs: Expression;
evaluate: (EvaluationContext) => any;

constructor(op: '==' | '!=', lhs: Expression, rhs: Expression) {
this.type = BooleanType;
this.lhs = lhs;
this.rhs = rhs;
this.evaluate = op === '==' ? eq : ne;
}

static parse(args: Array<mixed>, context: ParsingContext): ?Expression {
if (args.length !== 3)
return context.error(`Expected two arguments.`);

const op: '==' | '!=' = (args[0]: any);

const lhs = context.parse(args[1], 1, ValueType);
if (!lhs) return null;
const rhs = context.parse(args[2], 2, ValueType);
if (!rhs) return null;

if (!isComparableType(lhs.type) && !isComparableType(rhs.type)) {
return context.error(`Expected at least one argument to be a string, number, boolean, or null, but found (${toString(lhs.type)}, ${toString(rhs.type)}) instead.`);
}

if (lhs.type.kind !== rhs.type.kind && lhs.type.kind !== 'value' && rhs.type.kind !== 'value') {
return context.error(`Cannot compare ${toString(lhs.type)} and ${toString(rhs.type)}.`);
}

return new Equals(op, lhs, rhs);
}

eachChild(fn: (Expression) => void) {
fn(this.lhs);
fn(this.rhs);
}

possibleOutputs() {
return [true, false];
}
}

module.exports = Equals;
46 changes: 14 additions & 32 deletions src/style-spec/expression/definitions/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// @flow

const {
NullType,
NumberType,
StringType,
BooleanType,
Expand All @@ -28,27 +27,30 @@ const Case = require('./case');
const Step = require('./step');
const Interpolate = require('./interpolate');
const Coalesce = require('./coalesce');
const Equals = require('./equals');

import type { Expression } from '../expression';

const expressions: { [string]: Class<Expression> } = {
// special forms
'let': Let,
'var': Var,
'literal': Literal,
'string': Assertion,
'number': Assertion,
'boolean': Assertion,
'object': Assertion,
'!=': Equals,
'==': Equals,
'array': ArrayAssertion,
'to-number': Coercion,
'to-color': Coercion,
'at': At,
'boolean': Assertion,
'case': Case,
'match': Match,
'coalesce': Coalesce,
'interpolate': Interpolate,
'let': Let,
'literal': Literal,
'match': Match,
'number': Assertion,
'object': Assertion,
'step': Step,
'interpolate': Interpolate
'string': Assertion,
'to-color': Coercion,
'to-number': Coercion,
'var': Var
};

function rgba(ctx, [r, g, b, a]) {
Expand All @@ -74,8 +76,6 @@ function length(ctx, [v]) {
return v.evaluate(ctx).length;
}

function eq(ctx, [a, b]) { return a.evaluate(ctx) === b.evaluate(ctx); }
function ne(ctx, [a, b]) { return a.evaluate(ctx) !== b.evaluate(ctx); }
function lt(ctx, [a, b]) { return a.evaluate(ctx) < b.evaluate(ctx); }
function gt(ctx, [a, b]) { return a.evaluate(ctx) > b.evaluate(ctx); }
function lteq(ctx, [a, b]) { return a.evaluate(ctx) <= b.evaluate(ctx); }
Expand Down Expand Up @@ -315,24 +315,6 @@ CompoundExpression.register(expressions, {
varargs(NumberType),
(ctx, args) => Math.max(...args.map(arg => arg.evaluate(ctx)))
],
'==': {
type: BooleanType,
overloads: [
[[NumberType, NumberType], eq],
[[StringType, StringType], eq],
[[BooleanType, BooleanType], eq],
[[NullType, NullType], eq]
]
},
'!=': {
type: BooleanType,
overloads: [
[[NumberType, NumberType], ne],
[[StringType, StringType], ne],
[[BooleanType, BooleanType], ne],
[[NullType, NullType], ne]
]
},
'>': {
type: BooleanType,
overloads: [
Expand Down
2 changes: 1 addition & 1 deletion test/expression.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ expressionSuite.run('js', { ignores, tests }, (fixture) => {
}
};

for (const input of fixture.inputs) {
for (const input of fixture.inputs || []) {
try {
const feature = { properties: input[1].properties || {} };
if ('id' in input[1]) {
Expand Down
14 changes: 14 additions & 0 deletions test/integration/expression-tests/equal/array/test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"expression": ["==", ["get", "x"], ["literal", [1]]],
"expected": {
"compiled": {
"result": "error",
"errors": [
{
"key": "",
"error": "Expected at least one argument to be a string, number, boolean, or null, but found (value, array<number, 1>) instead."
}
]
}
}
}
14 changes: 14 additions & 0 deletions test/integration/expression-tests/equal/color/test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"expression": ["==", ["get", "x"], ["to-color", "red"]],
"expected": {
"compiled": {
"result": "error",
"errors": [
{
"key": "",
"error": "Expected at least one argument to be a string, number, boolean, or null, but found (value, color) instead."
}
]
}
}
}
7 changes: 1 addition & 6 deletions test/integration/expression-tests/equal/mismatch/test.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,7 @@
"expected": {
"compiled": {
"result": "error",
"errors": [
{
"key": "",
"error": "Expected arguments of type (number, number) | (string, string) | (boolean, boolean) | (null, null), but found (string, number) instead."
}
]
"errors": [{"key": "", "error": "Cannot compare string and number."}]
}
}
}
13 changes: 13 additions & 0 deletions test/integration/expression-tests/equal/null-lhs/test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"expression": ["==", null, ["get", "x"]],
"inputs": [[{}, {"properties": {}}], [{}, {"properties": {"x": 1}}]],
"expected": {
"outputs": [true, false],
"compiled": {
"result": "success",
"isZoomConstant": true,
"isFeatureConstant": false,
"type": "boolean"
}
}
}
13 changes: 13 additions & 0 deletions test/integration/expression-tests/equal/null-rhs/test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"expression": ["==", null, ["get", "x"]],
"inputs": [[{}, {"properties": {}}], [{}, {"properties": {"x": 1}}]],
"expected": {
"outputs": [true, false],
"compiled": {
"result": "success",
"isZoomConstant": true,
"isFeatureConstant": false,
"type": "boolean"
}
}
}
7 changes: 4 additions & 3 deletions test/integration/expression-tests/equal/number/test.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
{
"expression": ["==", ["number", ["get", "x"]], ["number", ["get", "y"]]],
"expression": ["==", ["number", ["get", "x"]], ["get", "y"]],
"inputs": [
[{}, {"properties": {"x": 1, "y": 1}}],
[{}, {"properties": {"x": 1, "y": 2}}]
[{}, {"properties": {"x": 1, "y": 2}}],
[{}, {"properties": {"x": 1, "y": "1"}}]
],
"expected": {
"outputs": [true, false],
"outputs": [true, false, false],
"compiled": {
"result": "success",
"isZoomConstant": true,
Expand Down
14 changes: 14 additions & 0 deletions test/integration/expression-tests/equal/object/test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"expression": ["==", ["get", "x"], ["literal", {}]],
"expected": {
"compiled": {
"result": "error",
"errors": [
{
"key": "",
"error": "Expected at least one argument to be a string, number, boolean, or null, but found (value, object) instead."
}
]
}
}
}
13 changes: 5 additions & 8 deletions test/integration/expression-tests/equal/string/test.json
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
{
"expression": [
"==",
["to-string", ["get", "x"]],
["to-string", ["get", "y"]]
],
"expression": ["==", ["string", ["get", "x"]], ["get", "y"]],
"inputs": [
[{}, {"properties": {"x": 1, "y": 1}}],
[{}, {"properties": {"x": 1, "y": 2}}]
[{}, {"properties": {"x": "1", "y": "1"}}],
[{}, {"properties": {"x": "1", "y": 2}}],
[{}, {"properties": {"x": "1", "y": 1}}]
],
"expected": {
"outputs": [true, false],
"outputs": [true, false, false],
"compiled": {
"result": "success",
"isZoomConstant": true,
Expand Down
14 changes: 0 additions & 14 deletions test/integration/expression-tests/equal/untagged/test.json

This file was deleted.

20 changes: 20 additions & 0 deletions test/integration/expression-tests/equal/value/test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"expression": ["==", ["get", "x"], ["get", "y"]],
"inputs": [
[{}, {"properties": {"x": 0, "y": 0}}],
[{}, {"properties": {"x": "0", "y": "0"}}],
[{}, {"properties": {"x": 0, "y": false}}],
[{}, {"properties": {"x": 0, "y": "0"}}]
],
"expected": {
"compiled": {
"result": "error",
"errors": [
{
"key": "",
"error": "Expected at least one argument to be a string, number, boolean, or null, but found (value, value) instead."
}
]
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,7 @@
"expected": {
"compiled": {
"result": "error",
"errors": [
{
"key": "",
"error": "Expected arguments of type (number, number) | (string, string) | (boolean, boolean) | (null, null), but found (string, number) instead."
}
]
"errors": [{"key": "", "error": "Cannot compare string and number."}]
}
}
}
5 changes: 3 additions & 2 deletions test/integration/expression-tests/not_equal/number/test.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
{
"expression": ["!=", ["number", ["get", "x"]], ["number", ["get", "y"]]],
"expression": ["!=", ["number", ["get", "x"]], ["get", "y"]],
"inputs": [
[{}, {"properties": {"x": 1, "y": 1}}],
[{}, {"properties": {"x": 1, "y": "1"}}],
[{}, {"properties": {"x": 1, "y": 2}}]
],
"expected": {
"outputs": [false, true],
"outputs": [false, true, true],
"compiled": {
"result": "success",
"isZoomConstant": true,
Expand Down
5 changes: 3 additions & 2 deletions test/integration/expression-tests/not_equal/string/test.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
{
"expression": ["!=", ["string", ["get", "x"]], ["string", ["get", "y"]]],
"expression": ["!=", ["string", ["get", "x"]], ["get", "y"]],
"inputs": [
[{}, {"properties": {"x": "1", "y": "1"}}],
[{}, {"properties": {"x": "1", "y": 1}}],
[{}, {"properties": {"x": "1", "y": "2"}}]
],
"expected": {
"outputs": [false, true],
"outputs": [false, true, true],
"compiled": {
"result": "success",
"isZoomConstant": true,
Expand Down
14 changes: 0 additions & 14 deletions test/integration/expression-tests/not_equal/untagged/test.json

This file was deleted.

Loading

0 comments on commit 537e4dd

Please sign in to comment.