Repository: calmm-js/partial.lenses
Pull request #23 (2016-05-12 17:26:29)

Consistent semantics for invalid get and structure changing set operations


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 67d9251..0f5917a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,20 @@
 # Changelog
 
+## 3.4.1
+
+Fixed bugs when removing a non-existing property from an object or a
+non-existent index from an array.  Previously `L.remove("x", {})` returned `{}`.
+Now it returns `undefined` as it was previously documented.  Similarly
+`L.remove(index, [])` now returns `undefined` as was documented.
+
+Tightened the semantics of combinators, including `L.index`, `L.filter`,
+`L.prop` and `L.augment` (and other combinators whose semantics are defined in
+terms of those), that specifically work on objects or arrays.  Previously such
+combinators worked asymmetrically when operating on values not in their domain.
+Now they consistently treat values that are not in their domain as `undefined`.
+For example, `L.get("x", null)` now returns `undefined` (previously `null`) and,
+consistently, `L.set("x", 1, null)` now returns `{x: 1}` (previously error).
+
 ## 3.4.0
 
 Added minimalistic *experimental* traversal support in the form of the
diff --git a/README.md b/README.md
index b64a6a4..db0bb92 100644
--- a/README.md
+++ b/README.md
@@ -424,10 +424,9 @@ In alphabetical order.
 
 #### <a name="append"></a>[`L.append`](#append "L.append :: PLens [a] a")
 
-`L.append` is a special semi-degenerate lens that operates on arrays.  The view
-of `L.append` is always undefined.  Setting `L.append` to undefined has no
-effect by itself.  Setting `L.append` to a defined value appends the value to
-the end of the focused array.
+`L.append` is a special semi-degenerate lens that operates on arrays and is
+equivalent to `L.index(i)` with the index `i` set to the length of the focused
+array or 0 in case the focus is not a defined array.
 
 For example:
 
@@ -439,10 +438,10 @@ L.set(L.append, "x", undefined)
 #### <a name="augment"></a>[`L.augment({prop: obj => val, ...props})`](#augment "L.augment :: {p1 :: o -> a1, ...ps} -> PLens {...o} {...o, p1 :: a1, ...ps}")
 
 `L.augment({prop: obj => val, ...props})` is given a template of functions to
-compute new properties.  When viewing or setting undefined, the result is
-undefined.  When viewing a defined object, the object is extended with the
-computed properties.  When set with a defined object, the extended properties
-are removed.
+compute new properties.  When not viewing or setting a defined object, the
+result is undefined.  When viewing a defined object, the object is extended with
+the computed properties.  When set with a defined object, the extended
+properties are removed.
 
 For example:
 
@@ -544,11 +543,11 @@ required value for an element.
 
 #### <a name="filter"></a>[`L.filter(predicate)`](#filter "L.filter :: (a -> Boolean) -> PLens [a] [a]")
 
-`L.filter(predicate)` operates on arrays.  When viewed, only elements matching
-the given predicate will be returned.  When set, the resulting array will be
-formed by concatenating the set array and the complement of the filtered
-context.  If the resulting array would be empty, the whole result will be
-undefined.
+`L.filter(predicate)` operates on arrays.  When not viewing an array, the result
+is undefined.  When viewing an array, only elements matching the given predicate
+will be returned.  When set, the resulting array will be formed by concatenating
+the set array and the complement of the filtered context.  If the resulting
+array would be empty, the whole result will be undefined.
 
 For example:
 
@@ -607,8 +606,7 @@ L.modify(L.identity, f, x) = f(x)
 
 `L.index(integer)` or `integer` focuses on the specified array index.
 
-* When viewing an undefined array index or an undefined array, the result is
-  undefined.
+* When not viewing a defined array index, the result is undefined.
 * When setting to undefined, the element is removed from the resulting array,
   shifting all higher indices down by one.  If the result would be an array
   without indices (ignoring length), the whole result will be undefined.
@@ -775,8 +773,7 @@ be an object.
 
 `L.prop(string)` or `string` focuses on the specified object property.
 
-* When viewing an undefined property or an undefined object, the result is
-  undefined.
+* When not viewing a defined object property, the result is undefined.
 * When setting property to undefined, the property is removed from the result.
   If the result would be an empty object, the whole result will be undefined.
 
diff --git a/src/partial.lenses.js b/src/partial.lenses.js
index 8e2f2e3..23a2742 100644
--- a/src/partial.lenses.js
+++ b/src/partial.lenses.js
@@ -28,6 +28,14 @@ const warn = message => {
 
 //
 
+const isArray  = x => x && x.constructor === Array
+const isObject = x => x && x.constructor === Object
+
+const unArray  = x =>  isArray(x) ? x : undefined
+const unObject = x => isObject(x) ? x : undefined
+
+//
+
 const id = x => x
 const snd = (_, c) => c
 
@@ -46,9 +54,7 @@ const assert = process.env.NODE_ENV === "production" ? () => id : check
 
 const empty = {}
 
-const deleteKey = (k, o) => {
-  if (o === undefined || !(k in o))
-    return o
+const deleteKey = (k, o = {}) => {
   let r
   for (const p in o) {
     if (p !== k) {
@@ -60,9 +66,7 @@ const deleteKey = (k, o) => {
   return r
 }
 
-const setKey = (k, v, o) => {
-  if (o === undefined)
-    return {[k]: v}
+const setKey = (k, v, o = {}) => {
   if (k in o && R.equals(v, o[k]))
     return o
   const r = {[k]: v}
@@ -158,15 +162,18 @@ const isProp = x => typeof x === "string"
 
 export const prop = assert("a string", isProp)
 
-const toRamdaProp = k =>
-  lensI(o => o && o[k],
-        (v, o) => v === undefined ? deleteKey(k, o) : setKey(k, v, o))
+const toRamdaProp = k => lensI(o => unObject(o) && o[k], (v, oIn) => {
+  const o = unObject(oIn)
+  return v === undefined ? deleteKey(k, o) : setKey(k, v, o)
+})
 
 export const find = predicate => choose(xs => {
-  if (xs === undefined)
+  if (isArray(xs)) {
+    const i = xs.findIndex(predicate)
+    return i < 0 ? append : i
+  } else {
     return append
-  const i = xs.findIndex(predicate)
-  return i < 0 ? append : i
+  }
 })
 
 export const findWith = (...ls) => {
@@ -178,15 +185,15 @@ const isIndex = x => Number.isInteger(x) && 0 <= x
 
 export const index = assert("a non-negative integer", isIndex)
 
-const toRamdaIndex = i => lensI(xs => xs && xs[i], (x, xs) => {
+const toRamdaIndex = i => lensI(xs => unArray(xs) && xs[i], (x, xs) => {
   if (x === undefined) {
-    if (xs === undefined)
+    if (!isArray(xs))
       return undefined
     if (i < xs.length)
       return dropped(xs.slice(0, i).concat(xs.slice(i+1)))
-    return xs
+    return dropped(xs)
   } else {
-    if (xs === undefined)
+    if (!isArray(xs))
       return Array(i).concat([x])
     if (xs.length <= i)
       return xs.concat(Array(i - xs.length), [x])
@@ -197,35 +204,42 @@ const toRamdaIndex = i => lensI(xs => xs && xs[i], (x, xs) => {
 })
 
 export const append = lensI(snd, (x, xs) =>
-  x === undefined ? xs : xs === undefined ? [x] : xs.concat([x]))
+  x === undefined ? unArray(xs) : isArray(xs) ? xs.concat([x]) : [x])
 
-export const filter = p => lensI(xs => xs && xs.filter(p), (ys, xs) =>
-  conserve(dropped(R.concat(ys || [], (xs || []).filter(R.complement(p)))), xs))
+export const filter = p => lensI(xs => unArray(xs) && xs.filter(p), (ys, xs) =>
+  conserve(dropped(R.concat(ys || [], (unArray(xs) || []).filter(R.complement(p)))), xs))
 
 export const augment = template => lensI(
-  toPartial(x => {
-    const z = {...x}
-    for (const k in template)
-      z[k] = template[k](x)
-    return z
-  }),
-  toConserve((y, c) => {
-    if (y === undefined)
+  x => {
+    if (isObject(x)) {
+      const z = {...x}
+      for (const k in template)
+        z[k] = template[k](x)
+      return z
+    } else {
       return undefined
-    let z
-    const set = (k, v) => {
-      if (undefined === z)
-        z = {}
-      z[k] = v
     }
-    for (const k in y) {
-      if (!(k in template))
-        set(k, y[k])
-      else
-        if (k in c)
-          set(k, c[k])
+  },
+  toConserve((y, cIn) => {
+    if (isObject(y)) {
+      const c = unObject(cIn) || {}
+      let z
+      const set = (k, v) => {
+        if (undefined === z)
+          z = {}
+        z[k] = v
+      }
+      for (const k in y) {
+        if (!(k in template))
+          set(k, y[k])
+        else
+          if (k in c)
+            set(k, c[k])
+      }
+      return z
+    } else {
+      return undefined
     }
-    return z
   }))
 
 export const pick = template => lensI(


Test patch:
diff --git a/test/tests.js b/test/tests.js
index 5d81246..5de1b8a 100644
--- a/test/tests.js
+++ b/test/tests.js
@@ -34,6 +34,8 @@ const testThrows = expr => it(`${expr} => throws`, () => {
     throw new Error(`Expected ${expr} to throw, returned ${show(result)}`)
 })
 
+const empties = [undefined, null, false, true, "a", 0, 0.0/0.0, {}, []]
+
 describe("compose", () => {
   testEq("P === L.compose", true)
   testEq('P() === L.identity', true)
@@ -107,6 +109,11 @@ describe('L.index', () => {
   testEq('L.get(5, undefined)', undefined)
   testEq('L.get(P(5), [1, 2, 3])', undefined)
   testEq('L.set(1, "2", ["1", "2", "3"])', ["1", "2", "3"])
+  empties.forEach(invalid => testEq(`L.get(0, ${show(invalid)})`, undefined))
+  empties.forEach(invalid => testEq(`L.set(0, "f", ${show(invalid)})`, ["f"]))
+  testEq('L.set(L.index(0), "Hello", "x, world!")', ["Hello"])
+  testEq('L.remove(0, [])', undefined)
+  testEq('L.remove(1, [])', undefined)
 })
 
 describe('L.prop', () => {
@@ -124,6 +131,9 @@ describe('L.prop', () => {
   testEq('L.set(P("z"), 3, undefined)', {z: 3})
   testEq('L.get("z", undefined)', undefined)
   testEq('L.get(P("z"), {x: 1})', undefined)
+  empties.forEach(invalid => testEq(`L.get("x", ${show(invalid)})`, undefined))
+  empties.forEach(invalid => testEq(`L.set("ex", true, ${show(invalid)})`, {ex: true}))
+  testEq('L.remove("x", {})', undefined)
 })
 
 describe("L.replace", () => {
@@ -199,12 +209,19 @@ describe("L.filter", () => {
   testEq('L.set(L.filter(R.lt(0)), [], [3,1,4,1,5,9,2])', undefined)
   testEq('L.remove(L.filter(R.lt(0)), [3,1,4,1,5,9,2])', undefined)
   testEq('L.remove(L.filter(R.lt(2)), [3,1,4,1,5,9,2])', [1,1,2])
+  empties.filter(x => !(x instanceof Array)).forEach(invalid => testEq(`L.get(L.filter(R.always(true)), ${show(invalid)})`, undefined))
+  empties.filter(x => !(x instanceof Array)).forEach(invalid => testEq(`L.set(L.filter(R.always(true)), [1,"2",3], ${show(invalid)})`, [1,"2",3]))
 })
 
 describe("L.removeAll", () => {
   testEq('L.removeAll(L.find(x => x < 2), [3,1,4,1,5,9,2])', [3,4,5,9,2])
 })
 
+describe("L.append", () => {
+  testEq('L.remove(L.append, "anything")', undefined)
+  empties.forEach(invalid => testEq(`L.set(L.append, "a", ${show(invalid)})`, ["a"]))
+})
+
 describe("L.augment", () => {
   testEq('L.get(L.augment({y: c => c.x+1, z: c => c.x-1}), {x: 0})', {x: 0, y: 1, z: -1})
   testEq('L.get(L.augment({y: c => c.x+1}), {x: 2, y: -1})', {x: 2, y: 3})
@@ -212,6 +229,8 @@ describe("L.augment", () => {
   testEq('L.set(L.augment({y: c => c.x+1}), {x: 2, y: 1}, {x: 0, y: -1})', {x: 2, y: -1})
   testEq('L.remove(P(L.augment({y: () => 1}), "x"), {x:0})', undefined)
   testEq('L.remove(L.augment({z: c => c.x + c.y}), {x: 1, y: 2})', undefined)
+  empties.filter(x => !R.equals(x, {})).forEach(invalid => testEq(`L.get(L.augment({x: () => 1}), ${show(invalid)})`, undefined))
+  empties.forEach(invalid => testEq(`L.set(L.augment({x: () => 1}), {y: 2}, ${show(invalid)})`, {y: 2}))
 })
 
 describe("L.sequence", () => {
@@ -229,6 +248,7 @@ describe("L.pick", () => {
   testEq('L.remove(P(L.pick({x: "b", y: "a"}), "y"), {a: [2], b: 1})', {b: 1})
   testEq('L.remove(P(L.pick({x: "b"}), "x"), {a: [2], b: 1})', {a: [2]})
   testEq('L.removeAll(P(L.pick({x: "b", y: "a"}), L.choice("y", "x")), {a: [2], b: 1})', undefined)
+  testEq('L.get(L.pick({x: 0, y: 1}), ["a", "b"])', {x: "a", y: "b"})
 })
 
 describe("L.props", () => {