diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a6877dc6..0192d2729 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed * Fix an issue with `basilisp test` standard streams output that can lead to failures on MS-Windows (#1080) + * Fix an issue where destructuring a vector would throw an exception rather than returning `nil` for invalid key types (#1090) + * Fix an issue where destructuring default values would take precedence over falsey values in the source data structure (#1078) ### Removed * Removed support for Python 3.8 (#1083) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 885934ca2..db5b9e5ef 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -5806,38 +5806,34 @@ (:children ddef)) kw-binding (fn [arg] - (let [kw-ns (namespace arg) - kw-name (name arg) - sym (symbol kw-name) - kw (if kw-ns - (keyword kw-ns kw-name) - (keyword kw-name)) - or-binding (get ors sym)] - (if or-binding - [sym `(or (get ~fn-arg ~kw) ~or-binding)] + (let [kw-ns (namespace arg) + kw-name (name arg) + sym (symbol kw-name) + kw (if kw-ns + (keyword kw-ns kw-name) + (keyword kw-name))] + (if (contains? ors sym) + [sym `(get ~fn-arg ~kw ~(get ors sym))] [sym `(get ~fn-arg ~kw)]))) map-binding (fn [f arg] - (let [k (f arg) - or-binding (get ors arg)] - (if or-binding - [arg `(or (get ~fn-arg ~k) ~or-binding)] + (let [k (f arg)] + (if (contains? ors arg) + [arg `(get ~fn-arg ~k ~(get ors arg))] [arg `(get ~fn-arg ~k)]))) sym-binding (fn [arg] - (let [sym-name (name arg) - sym (symbol sym-name) - or-binding (get ors sym)] - (if or-binding - [sym `(or (get ~fn-arg (quote ~arg)) ~or-binding)] + (let [sym-name (name arg) + sym (symbol sym-name)] + (if (contains? ors sym) + [sym `(get ~fn-arg (quote ~arg) ~(get ors sym))] [sym `(get ~fn-arg (quote ~arg))]))) named-binding (fn [arg] - (let [binding (get-in arg [:binding :name]) - key (:key arg) - or-binding (get ors binding)] - (if or-binding - [binding `(or (get ~fn-arg (quote ~key)) ~or-binding)] + (let [binding (get-in arg [:binding :name]) + key (:key arg)] + (if (contains? ors binding) + [binding `(get ~fn-arg (quote ~key) ~(get ors binding))] [binding `(get ~fn-arg ~key)]))) child-binding (fn [child] @@ -5845,6 +5841,12 @@ k (:key child)] [arg `(get ~fn-arg ~k)]))] (concat + [fn-arg + `(if (seq? ~fn-arg) + (if (next ~fn-arg) + (apply hash-map ~fn-arg) + (first ~fn-arg)) + ~fn-arg)] (mapcat kw-binding (:keys ddef)) (mapcat (partial map-binding name) (:strs ddef)) (mapcat sym-binding (:syms ddef)) diff --git a/src/basilisp/lang/runtime.py b/src/basilisp/lang/runtime.py index 65aa8d99d..d2e132835 100644 --- a/src/basilisp/lang/runtime.py +++ b/src/basilisp/lang/runtime.py @@ -1405,7 +1405,7 @@ def get(m, k, default=None): # pylint: disable=unused-argument def _get_others(m, k, default=None): try: return m[k] - except (KeyError, IndexError): + except (KeyError, IndexError, TypeError): return default diff --git a/src/basilisp/lang/vector.py b/src/basilisp/lang/vector.py index 46cfcef64..569c13c84 100644 --- a/src/basilisp/lang/vector.py +++ b/src/basilisp/lang/vector.py @@ -191,7 +191,7 @@ def entry(self, k: int) -> Optional[IMapEntry[int, T]]: def val_at(self, k: int, default: Optional[T] = None) -> Optional[T]: try: return self._inner[k] - except IndexError: + except (IndexError, TypeError): return default @staticmethod diff --git a/tests/basilisp/runtime_test.py b/tests/basilisp/runtime_test.py index 4fbb0d849..1a8464c52 100644 --- a/tests/basilisp/runtime_test.py +++ b/tests/basilisp/runtime_test.py @@ -392,7 +392,9 @@ def test_get(): assert "nada" == runtime.get(vec.v(1, 2, 3).to_transient(), 3, "nada") assert "l" == runtime.get("hello world", 2) assert None is runtime.get("hello world", 50) + assert None is runtime.get("hello world", "key") assert "nada" == runtime.get("hello world", 50, "nada") + assert "nada" == runtime.get("hello world", "key", "nada") assert "l" == runtime.get(["h", "e", "l", "l", "o"], 2) assert None is runtime.get(["h", "e", "l", "l", "o"], 50) assert "nada" == runtime.get(["h", "e", "l", "l", "o"], 50, "nada") diff --git a/tests/basilisp/test_core_macros.lpy b/tests/basilisp/test_core_macros.lpy index 8174262d9..c820a8c85 100644 --- a/tests/basilisp/test_core_macros.lpy +++ b/tests/basilisp/test_core_macros.lpy @@ -2,7 +2,7 @@ (:import contextlib inspect os socket tempfile) (:require [basilisp.string :as str] - [basilisp.test :refer [deftest is testing]])) + [basilisp.test :refer [deftest is are testing]])) (deftest defn-test (testing "single arity defn" @@ -739,6 +739,37 @@ (g v)))))) (deftest fn-associative-destructuring + (testing "ignore non-associative types" + (let [f (fn [{:keys [a]}] + a)] + (are [v] (nil? (f v)) + nil + [] + [:a 5] + "" + "abc" + 5 + 5.5)) + + (testing "with defaults" + (let [f (fn [{:keys [a] :or {a :not-defined}}] + a)] + (are [v] (= :not-defined (f v)) + nil + [] + [:a 5] + "" + "abc" + 5 + 5.5)))) + + (testing "falsey values not overridden by default values" + (let [f (fn [{:keys [a] :or {a :not-defined}}] + a)] + (are [v] (= v (f {:a v})) + nil + false))) + (testing "destructuring simple keys" (let [m {:a "a" :b "b"} @@ -754,11 +785,15 @@ (let [m {:a "a"} m1 {:a "a" :c "d"} + m2 '(:a "a" :c "d") + m3 (list m1) f (fn [{:keys [a c] :as m :or {c "c"}}] [a c m])] (is (= ["a" "c" m] (f m))) - (is (= ["a" "d" m1] (f m1))))) + (is (= ["a" "d" m1] (f m1))) + (is (= ["a" "d" (apply hash-map m2)] (f m2))) + (is (= ["a" "d" m1] (f m3))))) (testing "destructuring simple string keys" (let [m {:a 1 :b "b" "a" :a "b" :b} @@ -1019,6 +1054,32 @@ (is (= [[:a :b] [:c]] all))))) (deftest let-associative-destructuring + (testing "ignore non-associative types" + (are [v] (nil? (let [{:keys [a]} v] a)) + nil + [] + [:a 5] + "" + "abc" + 5 + 5.5) + + (testing "with defaults" + (are [v] (= :not-defined (let [{:keys [a] :or {a :not-defined}} v] a)) + nil + [] + [:a 5] + "" + "abc" + 5 + 5.5))) + + (testing "falsey values not overridden by default values" + (are [v] (= v (let [{:keys [a] :or {a :not-defined}} {:a v}] + a)) + nil + false)) + (testing "destructuring simple keys" (let [{:keys [a b]} {:a "a" :b "b"}] (is (= "a" a)) @@ -1032,7 +1093,17 @@ (let [{:keys [a b] :as m :or {b "c"}} {:a "a"}] (is (= "a" a)) (is (= "c" b)) - (is (= {:a "a"} m)))) + (is (= {:a "a"} m))) + + (let [{:keys [a b] :as m :or {b "c"}} '(:a "a")] + (is (= "a" a)) + (is (= "c" b)) + (is (= {:a "a"} m))) + + (let [{:keys [a b] :as m :or {b "c"}} '({:a "a" :b "b"})] + (is (= "a" a)) + (is (= "b" b)) + (is (= {:a "a" :b "b"} m)))) (testing "destructuring simple string keys" (let [{:strs [a b]} {:a 1 :b 2 "a" :a "b" :b}] @@ -1253,6 +1324,37 @@ (g v))))))) (deftest letfn-associative-destructuring + (testing "ignore non-associative types" + (letfn [(f [{:keys [a]}] + a)] + (are [v] (nil? (f v)) + nil + [] + [:a 5] + "" + "abc" + 5 + 5.5)) + + (testing "with defaults" + (letfn [(f [{:keys [a] :or {a :not-defined}}] + a)] + (are [v] (= :not-defined (f v)) + nil + [] + [:a 5] + "" + "abc" + 5 + 5.5)))) + + (testing "falsey values not overridden by default values" + (letfn [(f [{:keys [a] :or {a :not-defined}}] + a)] + (are [v] (= v (f {:a v})) + nil + false))) + (testing "destructuring simple keys" (let [m {:a "a" :b "b"}] (letfn [(f [{:keys [a]}] @@ -1266,11 +1368,15 @@ (is (= ["a" "b" m] (h m))))) (let [m {:a "a"} - m1 {:a "a" :c "d"}] + m1 {:a "a" :c "d"} + m2 '(:a "a" :c "d") + m3 (list m1)] (letfn [(f [{:keys [a c] :as m :or {c "c"}}] [a c m])] (is (= ["a" "c" m] (f m))) - (is (= ["a" "d" m1] (f m1)))))) + (is (= ["a" "d" m1] (f m1))) + (is (= ["a" "d" (apply hash-map m2)] (f m2))) + (is (= ["a" "d" m1] (f m3)))))) (testing "destructuring simple string keys" (let [m {:a 1 :b "b" "a" :a "b" :b}] @@ -1498,9 +1604,42 @@ (recur r orig-all (conj pairs pair) (+ accum v1 v2)))))))) (deftest loop-associative-destructuring + (testing "ignore non-associative types" + (is (= [] + (loop [items [nil [] [:a 5] "" "abc" 5 5.5] + {:keys [a]} (first items) + accum []] + (if (seq items) + (recur (rest items) + (first (rest items)) + (cond-> accum a (conj a))) + accum)))) + + (testing "with defaults" + (is (= (vec (repeat 7 :not-defined)) + (loop [items [nil [] [:a 5] "" "abc" 5 5.5] + {:keys [a] :or {a :not-defined}} (first items) + accum []] + (if (seq items) + (recur (rest items) + (first (rest items)) + (cond-> accum a (conj a))) + accum)))))) + + (testing "falsey values not overridden by default values" + (is (= [nil false] + (loop [items (map #(hash-map :a %) [nil false]) + {:keys [a] :as m :or {a :not-defined}} (first items) + accum []] + (if (seq items) + (recur (rest items) + (first (rest items)) + (conj accum a)) + accum))))) + (testing "destructuring simple keys" - (is (= [[{:a "a" :b "b"} {:a "c" :b "d"} {:a "e"}] "abcdee"] - (loop [items [{:a "a" :b "b"} {:a "c" :b "d"} {:a "e"}] + (is (= [[{:a "a" :b "b"} {:a "c" :b "d"} {:a "e"} {:a "f"} {:a "g" :b "h"}] "abcdeefegh"] + (loop [items [{:a "a" :b "b"} {:a "c" :b "d"} {:a "e"} '(:a "f") '({:a "g" :b "h"})] {:keys [a b] :as m :or {b "e"}} (first items) coll [] accum []] diff --git a/tests/basilisp/vector_test.py b/tests/basilisp/vector_test.py index 1af9d27e7..89c817f99 100644 --- a/tests/basilisp/vector_test.py +++ b/tests/basilisp/vector_test.py @@ -124,6 +124,10 @@ def test_val_at(): assert None is vec.PersistentVector.empty().val_at(0) assert None is vec.PersistentVector.empty().val_at(1) assert None is vec.PersistentVector.empty().val_at(-1) + assert None is vec.PersistentVector.empty().val_at(keyword("blah")) + assert "default" == vec.PersistentVector.empty().val_at(keyword("blah"), "default") + assert None is vec.PersistentVector.empty().val_at("key") + assert "default" == vec.PersistentVector.empty().val_at("key", "default") def test_peek():