Skip to content

Commit

Permalink
checking writable flag on property descriptor would be too easy
Browse files Browse the repository at this point in the history 
don't be naive when trying to understand Ecmascript
  • Loading branch information
@darwin
darwin committed Apr 9, 2018
1 parent ce8dda8 commit 8064575
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 21 deletions.
10 changes: 6 additions & 4 deletions src/lib/oops/codegen.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,14 +306,16 @@
(defn gen-check-key-write-access [obj-sym mode-sym key]
(debug-assert (symbol? obj-sym))
(debug-assert (symbol? mode-sym))
(let [descriptor-sym (gensym "descriptor")]
(let [descriptor-sym (gensym "descriptor")
reason-sym (gensym "reason")]
`(if-some [~descriptor-sym (oops.helpers/get-property-descriptor ~obj-sym ~key)]
(if (oops.helpers/is-property-writable? ~descriptor-sym)
true
(if-some [~reason-sym (oops.helpers/determine-property-non-writable-reason ~descriptor-sym)]
~(gen-report-if-needed :object-key-not-writable `{:obj (oops.state/get-target-object)
:key ~key
:reason ~reason-sym
:frozen? (oops.helpers/is-object-frozen? ~obj-sym)
:path (oops.state/get-key-path-str)}))
:path (oops.state/get-key-path-str)})
true)
(cond
; note that frozen object imply sealed
(oops.helpers/is-object-frozen? ~obj-sym)
Expand Down
18 changes: 16 additions & 2 deletions src/lib/oops/helpers.cljs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,22 @@
descriptor
(recur (.getPrototypeOf js/Object o))))))

(defn is-property-writable? [property-descriptor]
(.-writable property-descriptor))
(defn determine-property-non-writable-reason [property-descriptor]
; this gets a bit more tricky...
;
; there are two kinds of property descriptors
; 1) data property descriptors
; 2) accessor property descriptors
; only data descriptors have writable flag present
; see https://abdulapopoola.com/2016/11/21/deep-dive-into-javascript-property-descriptors
;
; we first check for "writable" property presence and test it only if it exists
; otherwise we assume accessor property is writable if it has some setter method
(if (.hasOwnProperty property-descriptor "writable")
(if (false? (.-writable property-descriptor))
"data property descriptor has writable=false")
(if (nil? (.-set property-descriptor))
"accessor property descriptor has neither writable flag nor a setter function")))

(defn is-object-sealed? [obj]
(.isSealed js/Object obj))
Expand Down
36 changes: 21 additions & 15 deletions test/src/tests/oops/main.cljs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,16 +300,19 @@
:object-is-sealed :warn}
(let [recorder (atom [])]
(with-console-recording recorder
(let [frozen-o (js-obj "k2" (js-obj))
_ (.freeze js/Object frozen-o)
o-frozen-child (js-obj "frozen" frozen-o)
sealed-o (js-obj "k2" (js-obj))
_ (.seal js/Object sealed-o)
o-sealed-child (js-obj "sealed" sealed-o)
o-non-writable-k2 (js-obj)
_ (.defineProperty js/Object o-non-writable-k2 "nowrite" #js {:value 42 :writable false})
o-non-writable-child (js-obj "k" o-non-writable-k2)
o-path (js-obj "c" o-frozen-child)]
(let [o-frozen (js-obj "k2" (js-obj))
_ (.freeze js/Object o-frozen)
o-frozen-child (js-obj "frozen" o-frozen)
o-sealed (js-obj "k2" (js-obj))
_ (.seal js/Object o-sealed)
o-sealed-child (js-obj "sealed" o-sealed)
o-nowrite (js-obj)
_ (.defineProperty js/Object o-nowrite "nowrite" #js {:value 42 :writable false})
o-non-writable-child (js-obj "k" o-nowrite)
o-path (js-obj "c" o-frozen-child)
o-accessor-prop (js-obj)
_ (.defineProperty js/Object o-accessor-prop "accessor1" #js {:get (fn [] 42) :set (fn [v] (js/console.log "SET", v))})
_ (.defineProperty js/Object o-accessor-prop "accessor2" #js {:get (fn [] 4242)})]
(oset! o-frozen-child "frozen.!k2" "val")
(oset! o-frozen-child "frozen.k2" "val")
(oset! o-frozen-child "frozen.!k3" "val")
Expand All @@ -319,14 +322,17 @@
(oset! o-non-writable-child "k.nowrite" "val") ; this should not work because nowrite is read-only
(oset! o-non-writable-child "k.!nowrite" "val") ; this should not work because nowrite is read-only
(oset! o-path "c.frozen.!k5.!k6.!k7" "val") ; test punching failure mid-path
(oset! o-accessor-prop "accessor1" "val") ; this should work because set fn exists
(oset! o-accessor-prop "accessor2" "val") ; this should not work because set fn does not exist
))
(is (= @recorder ["WARN: (\"Oops, Object key 'k2' is not writable on key path 'frozen.k2' because the object is frozen\" {:path \"frozen.k2\", :key \"k2\", :frozen? true, :obj #js {:frozen #js {:k2 #js {}}}})"
"WARN: (\"Oops, Object key 'k2' is not writable on key path 'frozen.k2' because the object is frozen\" {:path \"frozen.k2\", :key \"k2\", :frozen? true, :obj #js {:frozen #js {:k2 #js {}}}})"
(is (= @recorder ["WARN: (\"Oops, Object key 'k2' is not writable on key path 'frozen.k2' because the object is frozen\" {:path \"frozen.k2\", :key \"k2\", :frozen? true, :reason \"data property descriptor has writable=false\", :obj #js {:frozen #js {:k2 #js {}}}})"
"WARN: (\"Oops, Object key 'k2' is not writable on key path 'frozen.k2' because the object is frozen\" {:path \"frozen.k2\", :key \"k2\", :frozen? true, :reason \"data property descriptor has writable=false\", :obj #js {:frozen #js {:k2 #js {}}}})"
"WARN: (\"Oops, Cannot create object key 'k3' on key path 'frozen.k3' because the object is frozen\" {:path \"frozen.k3\", :key \"k3\", :obj #js {:frozen #js {:k2 #js {}}}})"
"WARN: (\"Oops, Cannot create object key 'k3' on key path 'sealed.k3' because the object is sealed\" {:path \"sealed.k3\", :key \"k3\", :obj #js {:sealed #js {:k2 \"val\"}}})"
"WARN: (\"Oops, Object key 'nowrite' is not writable on key path 'k.nowrite'\" {:path \"k.nowrite\", :key \"nowrite\", :frozen? false, :obj #js {:k #js {}}})"
"WARN: (\"Oops, Object key 'nowrite' is not writable on key path 'k.nowrite'\" {:path \"k.nowrite\", :key \"nowrite\", :frozen? false, :obj #js {:k #js {}}})"
"WARN: (\"Oops, Cannot create object key 'k5' on key path 'c.frozen.k5' because the object is frozen\" {:path \"c.frozen.k5\", :key \"k5\", :obj #js {:c #js {:frozen #js {:k2 #js {}}}}})"])))))))
"WARN: (\"Oops, Object key 'nowrite' is not writable on key path 'k.nowrite'\" {:path \"k.nowrite\", :key \"nowrite\", :frozen? false, :reason \"data property descriptor has writable=false\", :obj #js {:k #js {}}})"
"WARN: (\"Oops, Object key 'nowrite' is not writable on key path 'k.nowrite'\" {:path \"k.nowrite\", :key \"nowrite\", :frozen? false, :reason \"data property descriptor has writable=false\", :obj #js {:k #js {}}})"
"WARN: (\"Oops, Cannot create object key 'k5' on key path 'c.frozen.k5' because the object is frozen\" {:path \"c.frozen.k5\", :key \"k5\", :obj #js {:c #js {:frozen #js {:k2 #js {}}}}})" "LOG: (\"SET\" \"val\")"
"WARN: (\"Oops, Object key 'accessor2' is not writable\" {:path \"accessor2\", :key \"accessor2\", :frozen? false, :reason \"accessor property descriptor has neither writable flag nor a setter function\", :obj #js {}})"])))))))

(deftest test-ocall
(testing "simple invocation via call"
Expand Down

0 comments on commit 8064575

Please sign in to comment.