JSON::Serializable causes compilation error when combined with an initialize method with exactly 2 params

[drum445]

When you have a class that includes JSON::Serializable and also has an initialize method that takes in 2 params a compilation error is raised:

instance variable '@foo' of Bar must be String, not JSON::PullParser

See playgrounds below for examples
https://play.crystal-lang.org/#/r/4jka - works fine (notice 3 vars in init call)
https://play.crystal-lang.org/#/r/4jkb - compilation error (notices 2 vars in init call)
https://play.crystal-lang.org/#/r/4jkc - also works fine (notice 1 var in init call)

Seems to only be occurring when init takes 2 params, 1 and >2 work as expected.
Cheers

[asterite]

When you include that module, you must always use type restrictions in initializers from there on. It's something I dislike, for me JSON serialization shouldn't involve constructors, but the community seem to dislike my suggestions regarding this.

[drum445]

@asterite how do you mean sorry, I don't quite understand why the behaviour would be different depending on how many params your init method requires?

You are correct though, adding a type restriction to init param removes the error. Although it only seems you have to add it to the first param in the method sig.

[drum445]

For what it's worth, I agree that JSON serialization shouldn't worry about constructors

[asterite]

Json setializable defines constructors for you. They conflict with your constructors. To avoid the conflict you must use type restrictions, like @x : String in the argument

[drum445]

Makes sense, but only required when your init method accepts 2 params how odd

[asterite]

Overloads in action

[drum445]

Ah, ok so when using two params just make sure you declare the type of your first one.
Sorts the issue at least

Cheers sir

[asterite]

Json Serializable declares a constructor with two parameters. It conflicts with yours, only if it has two parameters.

[luislavena]

@asterite, what about changing JSON::Serializable initialize signature to use named arguments?

diff --git a/src/json/serialization.cr b/src/json/serialization.cr
index 46f0d31..2b1050a
--- a/src/json/serialization.cr
+++ b/src/json/serialization.cr
@@ -117,7 +117,7 @@ module JSON
 
       def self.new(pull : ::JSON::PullParser)
         instance = allocate
-        instance.initialize(pull, nil)
+        instance.initialize(pull: pull)
         GC.add_finalizer(instance) if instance.responds_to?(:finalize)
         instance
       end
@@ -132,7 +132,7 @@ module JSON
       end
     end
 
-    def initialize(pull : ::JSON::PullParser, dummy : Nil)
+    def initialize(*, pull : ::JSON::PullParser)
       {% begin %}
         {% properties = {} of Nil => Nil %}
         {% for ivar in @type.instance_vars %}

require "json"

class User
  include JSON::Serializable

  property id : String
  property username : String

  def initialize(@id, @username)
  end
end

user = User.from_json(%({"id":"10","username":"luis"}))
p user # => #<User:0x17edec0 @id="10", @username="luis">

[asterite]

That can work :-)

Anyone feel free to send a PR with that, or discuss it first.

[asterite]

(though to me it's just hiding a bigger problem)