Skip to content

custom data readers loader should not recurse further than first subdirectories of sys.path #1139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 22, 2024
Merged

custom data readers loader should not recurse further than first subdirectories of sys.path #1139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0f8d53
Data readers loader should only look at dir and immediate subdirs
ikappaki Nov 22, 2024
01e62a4
update doc
ikappaki Nov 22, 2024
03c688a
Address review comments (mispelling, use of scandir)
ikappaki Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
* Added support for a subset of qualified method syntax introduced in Clojure 1.12 (#1109)

### Changed
* The Custom Data Readers Loader will only now examine the top directory and up to its immediate subdirectories of each `sys.path` entry, instead of recursive descending into every subdirectory, improving start up performance (#1135)

### Fixed
* Fix a bug where tags in data readers were resolved as Vars within syntax quotes, rather than using standard data readers rules (#1129)
* Fix a bug where `keyword` and `symbol` functions did not treat string arguments as potentially namespaced (#1131)
Expand Down
2 changes: 1 addition & 1 deletion docs/reader.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ Custom Data Readers

When Basilisp starts it can load data readers from multiple sources.

It will search in :external:py:data:`sys.path` for files named ``data_readers.lpy`` or else ``data_readers.cljc``; each which must contain a mapping of qualified symbol tags to qualified symbols of function vars.
It will search in the top level directory and up to its immediate subdirectories (which typically representing installed modules) of the :external:py:data:`sys.path` entries for files named ``data_readers.lpy`` or else ``data_readers.cljc``; each which must contain a mapping of qualified symbol tags to qualified symbols of function vars.

.. code-block:: clojure

Expand Down
194 changes: 105 additions & 89 deletions src/basilisp/core.lpy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7214,95 +7214,6 @@
(.write writer content)
nil))

;;;;;;;;;;;;;;;;;;;;;;;;;
;; Custom Data Readers ;;
;;;;;;;;;;;;;;;;;;;;;;;;;

(defmulti ^:private make-custom-data-readers
(fn [obj ^:no-warn-when-unused metadata]
(type obj)))

(defmethod make-custom-data-readers :default
[obj metadata]
(throw (ex-info "Not a valid data-reader map" (assoc metadata :object obj))))

(defmethod make-custom-data-readers basilisp.lang.interfaces/IPersistentMap
[mappings metadata]
(reduce (fn [m [k v]]
(let [v' (if (qualified-symbol? v)
(intern (create-ns (symbol (namespace v)))
(symbol (name v)))
v)]
(cond
(not (qualified-symbol? k))
(throw
(ex-info "Invalid tag in data-readers. Expected qualified symbol."
(merge metadata {:form k})))

(not (ifn? v'))
(throw (ex-info "Invalid reader function in data-readers"
(merge metadata {:form v})))

:else
(assoc m (with-meta k metadata) v'))))
mappings
mappings))

(defmethod make-custom-data-readers importlib.metadata/EntryPoint
[entry-point metadata]
(make-custom-data-readers (.load entry-point)
(assoc metadata
:basilisp.entry-point/name (.-name entry-point)
:basilisp.entry-point/group (.-group entry-point))))

(defmethod make-custom-data-readers pathlib/Path
[file metadata]
(make-custom-data-readers
(with-open [rdr (basilisp.io/reader file)]
(read (if (.endswith (name file) "cljc")
{:eof nil :read-cond :allow}
{:eof nil})
rdr))
(assoc metadata :file (str file))))

(defn- data-readers-entry-points []
(when (#{"true" "t" "1" "yes" "y"} (.lower
(os/getenv
"BASILISP_USE_DATA_READERS_ENTRY_POINT"
"true")))
(#?@(:lpy39- [get (.entry_points importlib/metadata)]
:lpy310+ [.entry_points importlib/metadata ** :group])
"basilisp_data_readers")))

(defn- data-readers-files []
(->> sys/path
(mapcat file-seq)
(filter (comp #{"data_readers.lpy" "data_readers.cljc"} name))
(group-by #(.-parent %))
vals
;; Only load one data readers file per directory and prefer
;; `data_readers.lpy` to `data_readers.cljc`
(map #(first (sort-by name > %)))))

(defn- load-data-readers []
(alter-var-root
#'*data-readers*
(fn [mappings additional-mappings]
(reduce (fn [m [k v]]
(if (not= (get m k v) v)
(throw (ex-info "Conflicting data-reader mapping"
(merge (meta k) {:conflict k, :mappings m})))
(assoc m k v)))
mappings
additional-mappings))
;; Can't use `read` when altering `*data-readers*` so do reads ahead of time
(->> (concat (data-readers-files)
(data-readers-entry-points))
(mapcat #(make-custom-data-readers % nil))
doall)))

(load-data-readers)

;;;;;;;;;;;;;;;;;
;; Transducers ;;
;;;;;;;;;;;;;;;;;
Expand Down Expand Up @@ -7496,6 +7407,111 @@
([result input]
(reduce rf result input))))

;;;;;;;;;;;;;;;;;;;;;;;;;
;; Custom Data Readers ;;
;;;;;;;;;;;;;;;;;;;;;;;;;

(defmulti ^:private make-custom-data-readers
(fn [obj ^:no-warn-when-unused metadata]
(type obj)))

(defmethod make-custom-data-readers :default
[obj metadata]
(throw (ex-info "Not a valid data-reader map" (assoc metadata :object obj))))

(defmethod make-custom-data-readers basilisp.lang.interfaces/IPersistentMap
[mappings metadata]
(reduce (fn [m [k v]]
(let [v' (if (qualified-symbol? v)
(intern (create-ns (symbol (namespace v)))
(symbol (name v)))
v)]
(cond
(not (qualified-symbol? k))
(throw
(ex-info "Invalid tag in data-readers. Expected qualified symbol."
(merge metadata {:form k})))

(not (ifn? v'))
(throw (ex-info "Invalid reader function in data-readers"
(merge metadata {:form v})))

:else
(assoc m (with-meta k metadata) v'))))
mappings
mappings))

(defmethod make-custom-data-readers importlib.metadata/EntryPoint
[entry-point metadata]
(make-custom-data-readers (.load entry-point)
(assoc metadata
:basilisp.entry-point/name (.-name entry-point)
:basilisp.entry-point/group (.-group entry-point))))

(defmethod make-custom-data-readers pathlib/Path
[file metadata]
(make-custom-data-readers
(with-open [rdr (basilisp.io/reader file)]
(read (if (.endswith (name file) "cljc")
{:eof nil :read-cond :allow}
{:eof nil})
rdr))
(assoc metadata :file (str file))))

(defn- data-readers-entry-points []
(when (#{"true" "t" "1" "yes" "y"} (.lower
(os/getenv
"BASILISP_USE_DATA_READERS_ENTRY_POINT"
"true")))
(#?@(:lpy39- [get (.entry_points importlib/metadata)]
:lpy310+ [.entry_points importlib/metadata ** :group])
"basilisp_data_readers")))

(defn- data-readers-files
"Return a list of :external:py:class:`pathlib/Path`s pointing to
`data_readers.lpy` and `data_readers.cljc` found in each top
directory and up to its immediate subdirectories of
the :external:py:data:`sys.path` entries. The list is ordered such
that entries with the filename `data_readers.lpy` appear first."
[]
(->> sys/path
(mapcat (fn [dir] (when (os.path/isdir dir)
(-> (comp
(mapcat (fn [^os/DirEntry entry]
(if (.is-dir entry)
;; immediate subdirectory
(os/scandir (.-path entry))
;; top level file
[entry])))
(filter #(.is-file %))
(map #(pathlib/Path (.-path %)))
(filter (comp #{"data_readers.lpy" "data_readers.cljc"} name)))
(eduction (os/scandir dir))))))
(group-by #(.-parent %))
vals
;; Only load one data readers file per directory and prefer
;; `data_readers.lpy` to `data_readers.cljc`
(map #(first (sort-by name > %)))))

(defn- load-data-readers []
(alter-var-root
#'*data-readers*
(fn [mappings additional-mappings]
(reduce (fn [m [k v]]
(if (not= (get m k v) v)
(throw (ex-info "Conflicting data-reader mapping"
(merge (meta k) {:conflict k, :mappings m})))
(assoc m k v)))
mappings
additional-mappings))
;; Can't use `read` when altering `*data-readers*` so do reads ahead of time
(->> (concat (data-readers-files)
(data-readers-entry-points))
(mapcat #(make-custom-data-readers % nil))
doall)))

(load-data-readers)

;;;;;;;;;;;;;;;;;;;;;;;;
;; Stateful Iteration ;;
;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down
7 changes: 3 additions & 4 deletions tests/basilisp/test_data_readers.lpy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,11 @@
(is (= #'custom-data-reader (get *data-readers* 'test/test2)))
(is (= '(x) (read-string "#test/test2 x"))))

(testing "from submodule"
(testing "from submodule are not considered"
(make-path-file ["my_module" "submodule" "data_readers.lpy"]
(pr-str {'test/test3 `custom-data-reader}))
(#'basilisp.core/load-data-readers)
(is (= #'custom-data-reader (get *data-readers* 'test/test3)))
(is (= '(x) (read-string "#test/test3 x"))))
(is (= nil (get *data-readers* 'test/test3))))

(testing "from cljc file"
(make-path-file ["from_cljc" "data_readers.cljc"]
Expand All @@ -97,7 +96,7 @@
(make-path-file ["prefer_lpy_to_cljc" "data_readers.cljc"]
(pr-str {'test/test5 (constantly :fail)}))
(#'basilisp.core/load-data-readers)
(is (= #'custom-data-reader (get *data-readers* 'test/test3)))
(is (= #'custom-data-reader (get *data-readers* 'test/test5)))
(is (= '(x) (read-string "#test/test5 x"))))

(testing "does not load clj file"
Expand Down
Loading