From 16ba039f51b0c8ab143b6e2b04f95f959bb7e14f Mon Sep 17 00:00:00 2001
From: Alexander Plavin <alexander@plav.in>
Date: Wed, 22 Jun 2022 17:29:30 +0300
Subject: [PATCH] use properties, not fields

Co-authored-by: Jan Weidner <jw3126@gmail.com>
---
 Project.toml            |  2 +-
 src/ConstructionBase.jl | 33 +++++++++++++++++++++++++--------
 test/runtests.jl        | 24 ++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/Project.toml b/Project.toml
index 9375eb9..65cdbad 100644
--- a/Project.toml
+++ b/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"
diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl
index 0302fc0..2e67f8e 100644
--- a/src/ConstructionBase.jl
+++ b/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)
diff --git a/test/runtests.jl b/test/runtests.jl
index fc18070..da513d0 100644
--- a/test/runtests.jl
+++ b/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,