From 4b9e02d0e4384fd313281792823dfdd02222c0c4 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 27 Mar 2015 12:44:56 +0200 Subject: [PATCH 1/3] style(filterFilter): fix indentation and remove newline for consistency --- test/ng/filter/filterSpec.js | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/test/ng/filter/filterSpec.js b/test/ng/filter/filterSpec.js index 217ad25c5f7f..8537bc72b3a1 100644 --- a/test/ng/filter/filterSpec.js +++ b/test/ng/filter/filterSpec.js @@ -438,31 +438,31 @@ describe('Filter: filter', function() { it('should not throw an error if property is null when comparing object', function() { - var items = [ - { office:1, people: {name:'john'}}, - { office:2, people: {name:'jane'}}, - { office:3, people: null} - ]; - var f = { }; - expect(filter(items, f).length).toBe(3); - - f = { people:null }; - expect(filter(items, f).length).toBe(1); + var items = [ + { office:1, people: {name:'john'}}, + { office:2, people: {name:'jane'}}, + { office:3, people: null} + ]; + var f = { }; + expect(filter(items, f).length).toBe(3); - f = { people: {}}; - expect(filter(items, f).length).toBe(2); + f = { people:null }; + expect(filter(items, f).length).toBe(1); - f = { people:{ name: '' }}; - expect(filter(items, f).length).toBe(2); + f = { people: {}}; + expect(filter(items, f).length).toBe(2); - f = { people:{ name:'john' }}; - expect(filter(items, f).length).toBe(1); + f = { people:{ name: '' }}; + expect(filter(items, f).length).toBe(2); - f = { people:{ name:'j' }}; - expect(filter(items, f).length).toBe(2); + f = { people:{ name:'john' }}; + expect(filter(items, f).length).toBe(1); + f = { people:{ name:'j' }}; + expect(filter(items, f).length).toBe(2); }); + describe('should support comparator', function() { it('not consider objects without a custom `toString` in non-strict comparison', function() { From bd25dd7f135bd5892b397395f17b91064585f547 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 27 Mar 2015 16:48:06 +0200 Subject: [PATCH 2/3] fix(filterFilter): fix matching against `null`/`undefined` Included fixes: * Do not convert `null`/`undefined` to strings for substring matching in non-strict comparison mode. Prevents `null`/`undefined` from being matched against e.g. 'u'. * Let `null` (as a top-level filter expression) match "deeply" (as do booleans, numbers and strings). E.g. let `filterFilter(arr, null)` match an item like `{someProp: null}`. Closes #11432 --- src/ng/filter/filter.js | 12 ++++++++- test/ng/filter/filterSpec.js | 51 ++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/ng/filter/filter.js b/src/ng/filter/filter.js index 3b6b444d3375..d658de900e65 100644 --- a/src/ng/filter/filter.js +++ b/src/ng/filter/filter.js @@ -135,14 +135,16 @@ function filterFilter() { } } + var expressionType = (expression !== null) ? typeof expression : 'null'; var predicateFn; var matchAgainstAnyProp; - switch (typeof expression) { + switch (expressionType) { case 'function': predicateFn = expression; break; case 'boolean': + case 'null': case 'number': case 'string': matchAgainstAnyProp = true; @@ -172,6 +174,14 @@ function createPredicateFn(expression, comparator, matchAgainstAnyProp) { comparator = equals; } else if (!isFunction(comparator)) { comparator = function(actual, expected) { + if (isUndefined(actual)) { + // No substring matching against `undefined` + return false; + } + if ((actual === null) || (expected === null)) { + // No substring matching against `null`; only match against `null` + return actual === expected; + } if (isObject(expected) || (isObject(actual) && !hasCustomToString(actual))) { // Should not compare primitives against objects, unless they have custom `toString` method return false; diff --git a/test/ng/filter/filterSpec.js b/test/ng/filter/filterSpec.js index 8537bc72b3a1..fb71ef054ab3 100644 --- a/test/ng/filter/filterSpec.js +++ b/test/ng/filter/filterSpec.js @@ -463,8 +463,59 @@ describe('Filter: filter', function() { }); + it('should match `null` against `null` only', function() { + var items = [ + {value: null}, + {value: undefined}, + {value: true}, + {value: false}, + {value: NaN}, + {value: 42}, + {value: 'null'}, + {value: 'test'}, + {value: {}}, + {value: new Date()} + ]; + var flt; + + flt = null; + expect(filter(items, flt).length).toBe(1); + expect(filter(items, flt)[0]).toBe(items[0]); + + flt = {value: null}; + expect(filter(items, flt).length).toBe(1); + expect(filter(items, flt)[0]).toBe(items[0]); + + flt = {value: undefined}; + expect(filter(items, flt).length).toBe(items.length); + + flt = {value: NaN}; + expect(includes(filter(items, flt), items[0])).toBeFalsy(); + + flt = {value: false}; + expect(includes(filter(items, flt), items[0])).toBeFalsy(); + + flt = ''; + expect(includes(filter(items, flt), items[0])).toBeFalsy(); + + flt = {value: 'null'}; + expect(includes(filter(items, flt), items[0])).toBeFalsy(); + }); + + describe('should support comparator', function() { + it('not convert `null` or `undefined` to string in non-strict comparison', function() { + var items = [ + {value: null}, + {value: undefined} + ]; + var flt = {value: 'u'}; + + expect(filter(items, flt).length).toBe(0); + }); + + it('not consider objects without a custom `toString` in non-strict comparison', function() { var items = [{test: {}}]; var expr = '[object'; From a543eda280317141eeaf68f103fac760248b2f1e Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 27 Mar 2015 16:59:22 +0200 Subject: [PATCH 3/3] refactor(filterFilter): introduce helper function for "DRYness" --- src/ng/filter/filter.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/ng/filter/filter.js b/src/ng/filter/filter.js index d658de900e65..6e29e2cf5a87 100644 --- a/src/ng/filter/filter.js +++ b/src/ng/filter/filter.js @@ -135,7 +135,7 @@ function filterFilter() { } } - var expressionType = (expression !== null) ? typeof expression : 'null'; + var expressionType = getTypeForFilter(expression); var predicateFn; var matchAgainstAnyProp; @@ -204,8 +204,8 @@ function createPredicateFn(expression, comparator, matchAgainstAnyProp) { } function deepCompare(actual, expected, comparator, matchAgainstAnyProp, dontMatchWholeObject) { - var actualType = (actual !== null) ? typeof actual : 'null'; - var expectedType = (expected !== null) ? typeof expected : 'null'; + var actualType = getTypeForFilter(actual); + var expectedType = getTypeForFilter(expected); if ((expectedType === 'string') && (expected.charAt(0) === '!')) { return !deepCompare(actual, expected.substring(1), comparator, matchAgainstAnyProp); @@ -251,3 +251,8 @@ function deepCompare(actual, expected, comparator, matchAgainstAnyProp, dontMatc return comparator(actual, expected); } } + +// Used for easily differentiating between `null` and actual `object` +function getTypeForFilter(val) { + return (val === null) ? 'null' : typeof val; +}