Skip to content

Commit

Permalink
Revert to distinct classes for Equals, etc.
Browse files Browse the repository at this point in the history 
To avoid attaching a compare function as a class member, per https://github.com/mapbox/mapbox-gl-js/pull/5949/files
  • Loading branch information
Anand Thakker committed Jul 13, 2018
1 parent d07a818 commit cfbc92f
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 107 deletions.
181 changes: 81 additions & 100 deletions src/style-spec/expression/definitions/comparison.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,6 @@ function gtCollate(ctx, a, b, c) { return c.evaluate(ctx).compare(a.evaluate(ctx
function lteqCollate(ctx, a, b, c) { return c.evaluate(ctx).compare(a.evaluate(ctx), b.evaluate(ctx)) <= 0; }
function gteqCollate(ctx, a, b, c) { return c.evaluate(ctx).compare(a.evaluate(ctx), b.evaluate(ctx)) >= 0; }

const compare = {
basic: {
'==': eq,
'!=': neq,
'<': lt,
'>': gt,
'<=': lteq,
'>=': gteq
},
collator: {
'==': eqCollate,
'!=': neqCollate,
'<': ltCollate,
'>': gtCollate,
'<=': lteqCollate,
'>=': gteqCollate
}
};


/**
* Special form for comparison operators, implementing the signatures:
* - (T, T, ?Collator) => boolean
Expand All @@ -78,99 +58,100 @@ const compare = {
*
* @private
*/
class Comparison implements Expression {
type: Type;
op: ComparisonOperator;
lhs: Expression;
rhs: Expression;
collator: ?Expression;
compare: (EvaluationContext, Expression, Expression, ?Expression) => any;

constructor(op: *, lhs: Expression, rhs: Expression, collator: ?Expression, compare: *) {
this.type = BooleanType;
this.op = op;
this.lhs = lhs;
this.rhs = rhs;
this.collator = collator;
this.compare = compare;
}
function makeComparison(op: ComparisonOperator, compareBasic, compareWithCollator) {
return class Comparison implements Expression {
type: Type;
lhs: Expression;
rhs: Expression;
collator: ?Expression;

constructor(lhs: Expression, rhs: Expression, collator: ?Expression) {
this.type = BooleanType;
this.lhs = lhs;
this.rhs = rhs;
this.collator = collator;
}

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

const op: ComparisonOperator = (args[0]: any);
const op: ComparisonOperator = (args[0]: any);

let lhs = context.parse(args[1], 1, ValueType);
if (!lhs) return null;
if (!isComparableType(op, lhs.type)) {
return context.concat(1).error(`"${op}" comparisons are not supported for type '${toString(lhs.type)}'.`);
}
let rhs = context.parse(args[2], 2, ValueType);
if (!rhs) return null;
if (!isComparableType(op, rhs.type)) {
return context.concat(2).error(`"${op}" comparisons are not supported for type '${toString(rhs.type)}'.`);
}
let lhs = context.parse(args[1], 1, ValueType);
if (!lhs) return null;
if (!isComparableType(op, lhs.type)) {
return context.concat(1).error(`"${op}" comparisons are not supported for type '${toString(lhs.type)}'.`);
}
let rhs = context.parse(args[2], 2, ValueType);
if (!rhs) return null;
if (!isComparableType(op, rhs.type)) {
return context.concat(2).error(`"${op}" comparisons are not supported for type '${toString(rhs.type)}'.`);
}

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

if (op !== '==' && op !== '!=') {
// typing rules specific to less/greater than operators
if (lhs.type.kind === 'value' && rhs.type.kind !== 'value') {
// (value, T)
lhs = new Assertion(rhs.type, [lhs]);
} else if (lhs.type.kind !== 'value' && rhs.type.kind === 'value') {
// (T, value)
rhs = new Assertion(lhs.type, [rhs]);
} else if (lhs.type.kind === 'value' && rhs.type.kind === 'value') {
// (value, value)
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 (op !== '==' && op !== '!=') {
// typing rules specific to less/greater than operators
if (lhs.type.kind === 'value' && rhs.type.kind !== 'value') {
// (value, T)
lhs = new Assertion(rhs.type, [lhs]);
} else if (lhs.type.kind !== 'value' && rhs.type.kind === 'value') {
// (T, value)
rhs = new Assertion(lhs.type, [rhs]);
} else if (lhs.type.kind === 'value' && rhs.type.kind === 'value') {
// (value, value)
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.`);
}
}
}

let collator = null;
let comparisonFn;
if (args.length === 4) {
if (lhs.type.kind !== 'string' && rhs.type.kind !== 'string') {
return context.error(`Cannot use collator to compare non-string types.`);
let collator = null;
if (args.length === 4) {
if (lhs.type.kind !== 'string' && rhs.type.kind !== 'string') {
return context.error(`Cannot use collator to compare non-string types.`);
}
collator = context.parse(args[3], 3, CollatorType);
if (!collator) return null;
}
collator = context.parse(args[3], 3, CollatorType);
if (!collator) return null;
comparisonFn = compare.collator[op];
} else {
comparisonFn = compare.basic[op];
}

return new Comparison(op, lhs, rhs, collator, comparisonFn);
}
return new Comparison(lhs, rhs, collator);
}

evaluate(ctx: EvaluationContext) {
return this.compare(ctx, this.lhs, this.rhs, this.collator);
}
evaluate(ctx: EvaluationContext) {
return this.collator ?
compareWithCollator(ctx, this.lhs, this.rhs, this.collator) :
compareBasic(ctx, this.lhs, this.rhs);
}

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

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

serialize() {
const serialized = [this.op];
this.eachChild(child => { serialized.push(child.serialize()); });
return serialized;
}
serialize() {
const serialized = [op];
this.eachChild(child => { serialized.push(child.serialize()); });
return serialized;
}
};
}

export default Comparison;
export const Equals = makeComparison('==', eq, eqCollate);
export const NotEquals = makeComparison('!=', neq, neqCollate);
export const LessThan = makeComparison('<', lt, ltCollate);
export const GreaterThan = makeComparison('>', gt, gtCollate);
export const LessThanOrEqual = makeComparison('<=', lteq, lteqCollate);
export const GreaterThanOrEqual = makeComparison('>=', gteq, gteqCollate);
21 changes: 14 additions & 7 deletions src/style-spec/expression/definitions/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,14 @@ import Case from './case';
import Step from './step';
import Interpolate from './interpolate';
import Coalesce from './coalesce';
import Comparison from './comparison';
import {
Equals,
NotEquals,
LessThan,
GreaterThan,
LessThanOrEqual,
GreaterThanOrEqual
} from './comparison';
import { CollatorExpression } from './collator';
import Length from './length';

Expand All @@ -27,12 +34,12 @@ import type { ExpressionRegistry } from '../expression';

const expressions: ExpressionRegistry = {
// special forms
'==': Comparison,
'!=': Comparison,
'>': Comparison,
'<': Comparison,
'>=': Comparison,
'<=': Comparison,
'==': Equals,
'!=': NotEquals,
'>': GreaterThan,
'<': LessThan,
'>=': GreaterThanOrEqual,
'<=': LessThanOrEqual,
'array': ArrayAssertion,
'at': At,
'boolean': Assertion,
Expand Down

0 comments on commit cfbc92f

Please sign in to comment.