Fix a bug with using fields with special characters in records (#1031)

Fixes #1029

Author: chrisrink10

.github/workflows/run-tests.yml

@@ -77,6 +77,8 @@ jobs:
         with:
           name: code-coverage.py${{ matrix.version }}
           path: coverage/.coverage.py*
+          include-hidden-files: true
+          if-no-files-found: error
 
   run-pypy-tests:
     runs-on: ${{matrix.os}}
@@ -126,6 +128,8 @@ jobs:
         with:
           name: code-coverage.pypy${{ matrix.version }}
           path: coverage/.coverage.py*
+          include-hidden-files: true
+          if-no-files-found: error
 
   min-deps-test:
     # Attempt to run the TOX-ENV tests for the given OS and python

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * Fix a bug where the compiler would always generate inline function definitions even if the `inline-functions` compiler option is disabled (#1023)
  * Fix a bug where `defrecord`/`deftype` constructors could not be used in the type's methods. (#1025)
  * Fix a bug where `keys` and `vals` would fail for records (#1030)
+ * Fix a bug where operations on records created by `defrecord` failed for fields whose Python-safe names were mangled by the Python compiler (#1029)
 
 ## [v0.2.1]
 ### Changed

pyproject.toml

@@ -31,7 +31,7 @@ include = ["README.md", "LICENSE"]
 
 [tool.poetry.dependencies]
 python = "^3.8"
-attrs = ">=20.2.0"
+attrs = ">=22.2.0"
 immutables = ">=0.20,<1.0.0"
 prompt-toolkit = ">=3.0.0,<4.0.0"
 pyrsistent = ">=0.18.0,<1.0.0"

src/basilisp/core.lpy

@@ -6675,6 +6675,16 @@
 
 (prefer-method evolve basilisp.lang.interfaces/IRecord basilisp.lang.interfaces/IPersistentMap)
 
+(defn -mangle-py-field
+  "Implementation detail of :lpy:fn:`defrecord` to support Python's managled field
+  names."
+  [type-sym field-name]
+  (if (and (.startswith field-name "__")
+           (not (.endswith field-name "__")))
+    (let [munged-type-name (.lstrip (munge type-sym) "_")]
+      (str "_" munged-type-name field-name))
+    field-name))
+
 (defn ^:private validate-record-fields
   "Validate that record fields do not contain any reserved entries, are not declared as
   mutable, and do not declare any default values."
@@ -6779,12 +6789,10 @@
                                                                :map))))
 
                 new-recmap# (when (seq map#)
-                              ;; Python attrs does not allow the underscore prefixed
-                              ;; field name in the constructor
-                              ["recmap" (->> (mapcat identity map#)
-                                             (apply assoc ~'_recmap))])]
+                              ["_recmap" (->> (mapcat identity map#)
+                                              (apply assoc ~'_recmap))])]
             (->> fields#
-                 (map (fn [[k# v#]] [(name k#) v#]))
+                 (map (fn [[k# v#]] [(munge k#) v#]))
                  (mapcat identity)
                  (concat new-recmap#)
                  (apply evolve ~this-gs))))
@@ -6846,25 +6854,21 @@
               (contains? (.- new-rec#  ~'_recmap) f#)
               (recur r#
                      (->> (dissoc (.- new-rec# ~'_recmap) f#)
-                          (evolve new-rec# "recmap")))
+                          (evolve new-rec# "_recmap")))
 
               :else
               (recur r# new-rec#))))
          (~'contains [~this-gs ~key-gs]
           (or (~field-kw-set ~key-gs)
               (contains? ~'_recmap ~key-gs)))
          (~'entry [~this-gs ~key-gs]
-          (cond
-            (contains? ~field-kw-set ~key-gs)
-            (map-entry ~key-gs (python/getattr ~this-gs (munge ~key-gs)))
-
-            (contains? ~'_recmap ~key-gs)
-            (map-entry ~key-gs (get ~'_recmap ~key-gs))))
+          (when-let [val# (.val-at ~this-gs ~key-gs)]
+            (map-entry ~key-gs val#)))
          (~'val-at [~this-gs ~key-gs ~'& args#]
           (let [[default#] args#]
             (cond
               (contains? ~field-kw-set ~key-gs)
-              (python/getattr ~this-gs (munge ~key-gs))
+              (python/getattr ~this-gs (-mangle-py-field '~type-name (munge ~key-gs)))
 
               (contains? ~'_recmap ~key-gs)
               (get ~'_recmap ~key-gs default#))))
@@ -6891,12 +6895,16 @@
          (~'_record_lrepr [~this-gs py-kwargs#]
           (let [{print-meta :print_meta} (py->lisp py-kwargs#)
 
-                ns-name   (name *ns*)
-                qual-name (.- ~type-name ~'__qualname__)]
+                ;; Accessing this type name directly as `(.-__qualname__ this)`
+                ;; can fail in cases where the type name is munged and starts
+                ;; with a leading double underscore. Python apparently mangles
+                ;; the name as it would for any object field prefixed with a
+                ;; dunder.
+                qual-name (.- (python/type ~this-gs) ~'__qualname__)]
             (cond->> (->> (mapcat identity ~this-gs)
                           (apply hash-map)
                           (repr)
-                          (str "#" ns-name "." qual-name))
+                          (str "#" ~(name *ns*) "." qual-name))
               print-meta (str "^" (repr (meta ~this-gs)) " "))))
 
          ;; object

src/basilisp/lang/compiler/generator.py

@@ -1468,7 +1468,12 @@ def _deftype_to_py_ast(  # pylint: disable=too-many-locals
                 _tagged_assign(
                     target=ast.Name(id=safe_field, ctx=ast.Store()),
                     value=ast.Call(
-                        func=_ATTRIB_FIELD_FN_NAME, args=[], keywords=attr_default_kws
+                        func=_ATTRIB_FIELD_FN_NAME,
+                        args=[],
+                        keywords=[
+                            *attr_default_kws,
+                            ast.keyword(arg="alias", value=ast.Constant(safe_field)),
+                        ],
                     ),
                     annotation=tag,
                 )

tests/basilisp/test_defrecord.lpy

@@ -259,6 +259,20 @@
                        "#tests.basilisp.test-defrecord.Circle{:name \"Kurt\" :radius 1}"}
                      (str c1))))))
 
+(defrecord ?Name [? x? ?x ?? ?x? ??x ?x- x?x])
+
+(deftest defrecord-with-munged-fields
+  (let [fields [:? :x? :?x :?? :?x? :??x :?x- :x?x]
+        o      (->?Name 1 2 3 4 5 6 7 8)]
+    (is (= [1 2 3 4 5 6 7 8]
+           (map (partial get o) fields)))
+    (is (= [2 3 4 5 6 7 8 9]
+           (map #(get (assoc o % (inc (get o %))) %) fields)))
+    (is (= o (dissoc (assoc o :other-key 1000) :other-key)))
+    (is (= (dissoc (assoc (zipmap fields (range 1 100)) :other-key 1000) :?x)
+           (dissoc (assoc o :other-key 1000) :?x)))
+    (is (= o (map->?Name (zipmap fields (range 1 100)))))))
+
 (deftest defrecord-reader-form
   (testing "record without methods"
     (let [p  (->Point 1 2 3)
@@ -287,6 +301,9 @@
                  (read-string "#tests.basilisp.test-defrecord.Point(1 2 3)")))))
 
 (deftest defrecord-field-restrictions
+  (testing "attrs does not strip prefixes"
+    (is (eval '(defrecord NonDuplicateFieldName [x -x]))))
+
   (testing "reserved fields"
     (is (thrown? basilisp.lang.compiler/CompilerException
                  (eval '(defrecord NewType [a b meta]))))