Merge pull request #60 from aplavin/master

use properties, not fields

Author: jw3126

Project.toml

@@ -1,7 +1,7 @@
 name = "ConstructionBase"
 uuid = "187b0558-2788-49d3-abe0-74a17ed4e7c9"
 authors = ["Takafumi Arakaki", "Rafael Schouten", "Jan Weidner"]
-version = "1.3.0"
+version = "1.3.1"
 
 [deps]
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"

src/ConstructionBase.jl

@@ -43,13 +43,31 @@ end
 
 getproperties(o::NamedTuple) = o
 getproperties(o::Tuple) = o
-@generated function getproperties(obj)
-    fnames = fieldnames(obj)
-    fvals = map(fnames) do fname
-        Expr(:call, :getproperty, :obj, QuoteNode(fname))
+if VERSION >= v"1.7"
+    function getproperties(obj)
+        fnames = propertynames(obj)
+        NamedTuple{fnames}(getproperty.(Ref(obj), fnames))
+    end
+else
+    @generated function getproperties(obj)
+        if which(propertynames, Tuple{obj}).sig != Tuple{typeof(propertynames), Any}
+            # custom propertynames defined for this type
+            return quote
+                msg = """
+                Different fieldnames and propertynames are only supported on Julia v1.7+.
+                For older julia versions, consider overloading
+                `ConstructionBase.getproperties(obj::$(typeof(obj))`.
+                See also https://github.com/JuliaObjects/ConstructionBase.jl/pull/60.
+                """
+                error(msg)
+            end
+        end
+        fnames = fieldnames(obj)
+        fvals = map(fnames) do fname
+            :(obj.$fname)
+        end
+        :(NamedTuple{$fnames}(($(fvals...),)))
     end
-    fvals = Expr(:tuple, fvals...)
-    :(NamedTuple{$fnames}($fvals))
 end
 
 ################################################################################
@@ -86,9 +104,8 @@ function validate_setproperties_result(
 end
 @noinline function validate_setproperties_result(nt_new, nt_old, obj, patch)
     O = typeof(obj)
-    P = typeof(patch)
     msg = """
-    Failed to assign properties $(fieldnames(P)) to object with fields $(fieldnames(O)).
+    Failed to assign properties $(propertynames(patch)) to object with properties $(propertynames(obj)).
     You may want to overload
     ConstructionBase.setproperties(obj::$O, patch::NamedTuple)
     ConstructionBase.getproperties(obj::$O)

test/runtests.jl

@@ -238,6 +238,30 @@ end
     end
 end
 
+# example of a struct with different fields and properties
+struct FieldProps{NT <: NamedTuple{(:a, :b)}}
+    components::NT
+end
+
+Base.propertynames(obj::FieldProps) = (:a, :b)
+Base.getproperty(obj::FieldProps, name::Symbol) = getproperty(getfield(obj, :components), name)
+ConstructionBase.constructorof(::Type{<:FieldProps}) = (a, b) -> FieldProps((a=a, b=b))
+
+@testset "use properties, not fields" begin
+    x = FieldProps((a=1, b=:b))
+    if VERSION >= v"1.7"
+        @test getproperties(x) == (a=1, b=:b)
+        @test setproperties(x, a="aaa") == FieldProps((a="aaa", b=:b))
+        VERSION >= v"1.8-dev" ?
+            (@test_throws "Failed to assign properties (:c,) to object with properties (:a, :b)" setproperties(x, c=0)) :
+            (@test_throws ArgumentError setproperties(x, c=0))
+    else
+        @test_throws ErrorException getproperties(x)
+        @test_throws ErrorException setproperties(x, a="aaa")
+        @test_throws ErrorException setproperties(x, c=0)
+    end
+end
+
 function funny_numbers(::Type{Tuple}, n)::Tuple
     types = [
         Int128, Int16, Int32, Int64, Int8,