Skip to content

Commit

Permalink
Align match behavior with case/==
Browse files Browse the repository at this point in the history
Makes `["match", ["get", k], label, match, otherwise]` equivalent to `["case", ["==", ["get", k], label], match, otherwise]`. This changes the behavior of match expressions where the runtime type of the input does not match the type of the labels: previously such expressions produced a runtime type error and then fell back to the property default value; now they produce the fallback value from the match expression.
  • Loading branch information
jfirebaugh committed May 16, 2018
1 parent 4fd646c commit fcdff77
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 18 deletions.
14 changes: 10 additions & 4 deletions src/style-spec/expression/definitions/match.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
import assert from 'assert';

import { typeOf } from '../values';
import { ValueType, type Type } from '../types';

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

// Map input label values to output expression index
type Cases = {[number | string]: number};
Expand Down Expand Up @@ -84,19 +84,25 @@ class Match implements Expression {
outputs.push(result);
}

const input = context.parse(args[1], 1, inputType);
const input = context.parse(args[1], 1, ValueType);
if (!input) return null;

const otherwise = context.parse(args[args.length - 1], args.length - 1, outputType);
if (!otherwise) return null;

assert(inputType && outputType);

if (input.type.kind !== 'value' && context.concat(1).checkSubtype(inputType, input.type)) {
return null;
}

return new Match((inputType: any), (outputType: any), input, cases, outputs, otherwise);
}

evaluate(ctx: EvaluationContext) {
const input = (this.input.evaluate(ctx): any);
return (this.outputs[this.cases[input]] || this.otherwise).evaluate(ctx);
const output = (typeOf(input) === this.inputType && this.outputs[this.cases[input]]) || this.otherwise;
return output.evaluate(ctx);
}

eachChild(fn: (Expression) => void) {
Expand Down Expand Up @@ -134,7 +140,7 @@ class Match implements Expression {
}
}

const coerceLabel = (label) => this.input.type.kind === 'number' ? Number(label) : label;
const coerceLabel = (label) => this.inputType.kind === 'number' ? Number(label) : label;

for (const [outputIndex, labels] of groupedByOutput) {
if (labels.length === 1) {
Expand Down
10 changes: 5 additions & 5 deletions test/integration/expression-tests/match/basic/test.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
[{}, {"properties": {"x": "a"}}],
[{}, {"properties": {"x": "b"}}],
[{}, {"properties": {"x": "c"}}],
[{}, {"properties": {"x": 0}}]
[{}, {"properties": {"x": 0}}],
[{}, {"properties": {}}]
],
"expected": {
"compiled": {
Expand All @@ -17,13 +18,12 @@
"Apple",
"Banana",
"Kumquat",
{
"error": "Expected value to be of type string, but found number instead."
}
"Kumquat",
"Kumquat"
],
"serialized": [
"match",
["string", ["get", "x"]],
["get", "x"],
"a",
"Apple",
"b",
Expand Down
14 changes: 5 additions & 9 deletions test/integration/expression-tests/match/label-number/test.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,11 @@
"match",
"otherwise",
"otherwise",
{
"error": "Expected value to be of type number, but found string instead."
},
{
"error": "Expected value to be of type number, but found boolean instead."
},
{"error": "Expected value to be of type number, but found null instead."},
{"error": "Expected value to be of type number, but found null instead."}
"otherwise",
"otherwise",
"otherwise",
"otherwise"
],
"serialized": ["match", ["number", ["get", "x"]], 0, "match", "otherwise"]
"serialized": ["match", ["get", "x"], 0, "match", "otherwise"]
}
}

0 comments on commit fcdff77

Please sign in to comment.