Skip to content

Commit

Permalink
Support collators in feature filter expressions.
Browse files Browse the repository at this point in the history
Fixes issue #6920: feature filters using collator expressions would incorrectly be treated as legacy filters.
  • Loading branch information
ChrisLoer authored and jfirebaugh committed Jul 9, 2018
1 parent 2a66237 commit f73f7e2
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
3 changes: 1 addition & 2 deletions src/style-spec/feature_filter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function isExpressionFilter(filter: any) {
case '>=':
case '<':
case '<=':
return filter.length === 3 && (Array.isArray(filter[1]) || Array.isArray(filter[2]));
return filter.length !== 3 || (Array.isArray(filter[1]) || Array.isArray(filter[2]));

case 'any':
case 'all':
Expand Down Expand Up @@ -153,4 +153,3 @@ function convertHasOp(property: string) {
function convertNegation(filter: mixed) {
return ['!', filter];
}

14 changes: 13 additions & 1 deletion test/unit/style-spec/feature_filter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ test('filter', (t) => {
t.end();
});

t.test('expression, collator comparison', (t) => {
const caseSensitive = filter(['==', ['string', ['get', 'x']], ['string', ['get', 'y']], ['collator', { 'case-sensitive': true }]]);
t.equal(caseSensitive({zoom: 0}, {properties: {x: 'a', y: 'b'}}), false);
t.equal(caseSensitive({zoom: 0}, {properties: {x: 'a', y: 'A'}}), false);
t.equal(caseSensitive({zoom: 0}, {properties: {x: 'a', y: 'a'}}), true);

const caseInsensitive = filter(['==', ['string', ['get', 'x']], ['string', ['get', 'y']], ['collator', { 'case-sensitive': false }]]);
t.equal(caseInsensitive({zoom: 0}, {properties: {x: 'a', y: 'b'}}), false);
t.equal(caseInsensitive({zoom: 0}, {properties: {x: 'a', y: 'A'}}), true);
t.equal(caseInsensitive({zoom: 0}, {properties: {x: 'a', y: 'a'}}), true);
t.end();
});

t.test('expression, any/all', (t) => {
t.equal(filter(['all'])(), true);
t.equal(filter(['all', true])(), true);
Expand Down Expand Up @@ -52,7 +65,6 @@ test('filter', (t) => {
t.end();
});


t.test('degenerate', (t) => {
t.equal(filter()(), true);
t.equal(filter(undefined)(), true);
Expand Down
6 changes: 3 additions & 3 deletions test/unit/style-spec/fixture/filters.output.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
"line": 29
},
{
"message": "layers[3].filter: filter array for operator \"==\" must have 3 elements",
"message": "layers[3].filter: Expected two or three arguments.",
"line": 40
},
{
"message": "layers[4].filter: filter array for operator \"==\" must have 3 elements",
"message": "layers[4].filter: Expected two or three arguments." ,
"line": 47
},
{
Expand Down Expand Up @@ -55,4 +55,4 @@
"message": "layers[14].filter[2][1]: string expected, array found",
"line": 152
}
]
]

0 comments on commit f73f7e2

Please sign in to comment.