Add option to allow positional arguments

[Arcovion]

I like making wrappers like PrimaryKey[1]/UserId[5] just for type safety: the classic example being extracting from two database columns and wanting to avoid mixing up an integer from one column with another.
In these cases it's unnecessary to write out the field name, as that information is included in the type name.

This makes the interface even more similar to Struct.
While I was at it, I found it's also possible to support both positional and keyword arguments at once. I don't have a use case for that, but I included it anyway ¯\_(ツ)_/¯

I borrowed the keyword_init option name from the Struct option of the same name - here it works in reverse - the default is true but you can manually set it to false to enable positional arguments.

[Arcovion]

You could also allow both positional and named arguments by default, then you wouldn't need to add this additional keyword_init option. Or maybe have the option be a symbol and let the user choose between :named/:positional/:both.

[johansenja]

This is great, thanks for your contribution! It's nice to have this more inline with Struct - though it's almost a shame that keyword_init would now work the other way 🤔

Would you mind adding some examples of this to the README.md too?

And afterwards I think I'll be able to approve and release 👌🏻

[johansenja]

In the spirit of being less verbose, I wonder if it would be helpful to have a new class whose default is keyword_init: false (in the same way that Struct normally is too). On the other hand, perhaps it wouldn't bring that much, because you only have to write your struct definition once.

[johansenja]

Thanks for this tidying up, this is great!

[johansenja]

Would you mind adding another spec without the keyword_init, to confirm that it is keyword_init by default?

[Arcovion]

Thanks for reviewing! So I thought about it some more:
Somewhere between Ruby 2.x to 3.x kwargs were fully separated from args, which might be why I found this so easy to implement. I realised that meant it might be broken or inconsistent on older rubies though, and while investigating the ruby version differences I found this changelog entry:

A Struct class can also be initialized with keyword arguments without keyword_init: true on Struct.new [Feature #16806]

Which says that in the upcoming Ruby 3.2.0 release the default struct behaviour will be changed to allow keyword arguments. On the feature page I see they've done something similar to my idea of using a symbol, where they now permit three options instead of two:

When keyword_init is used

• nil: default behavior. Positional arguments given use positional init. Keyword arguments without positional arguments treated as positional in 3.0 with warning, and treated as keyword init in Ruby 3.1.
• true: Require keyword init, disallow positional init.
• false: Treat keywords as positional hash.

By default it looks like you'll then be able to use either keyword arguments or positional arguments - not mixed like how I did it in this PR (which I never had a use for anyway).

So as far as I'm concerned the ruby team has already done all the work for us here in terms of how the interface should look and ensuring backwards compatibility.
Armed with this knowledge, and with a view that we want to delegate as much as possible to native ruby struct (including same errors where possible, same interface etc) and have this gem be a relatively thin wrapper, my plan of action is to upgrade to Ruby 3.2.0-preview2 and push some changes tomorrow or over the weekend once I get everything working like a Struct.
Hard part is then gonna be making sure it works with older rubies/gives correct warnings. Once that's done I'll update the readme and force push this branch with everything rebased.

[johansenja]

Those are good considerations, thanks! I'm glad that this will be more in keepiing with Struct's future behaviours.

IIRC it was 2.7.1 which made the changes to kwargs? With #5 I just added CI tests for this, with ruby versions 2.6 - 3.1, so perhaps you could rebase onto master and try it out there?

I also dropped ruby 2.4 from the minimum ruby version in the gemspec (which I believe was the default on creation), because rbs doesn't support <2.6 anyway - so that's something at least

[Arcovion]

Ok so what I've done, is copy the ruby test file for Struct and modify it to work with TypedStruct. It was mostly just a matter of adding : Rbs("untyped") everywhere. There are tests I haven't done, and have no real interest in doing, to which I've added FIXME and commented out bits that don't pass.
Thanks to those specs I realised I forgot to add index setters struct[1] = x. Before, you had to specify by name only struct[:name] = x.
I also added support for the class_name argument from Struct::new, which is used all over the place in the ruby test file.
That and the keyword_init argument are now passed straight through to the super call to Struct::new.
I also made sure all the error messages are identical to Struct.
Here you can see the diff without whitespace: https://github.com/johansenja/typed_struct/pull/4/files?&w=1#diff-08034257b1323fbd1d612125938e7cae1938357be964deae99d9d83651d79aea
This probably doesn't need a readme entry anymore seeing as it's just replicating Ruby 3.2.0 structs. I've yet to test it on older rubies, lets see what CI says...

[johansenja]

Thanks for the update! I have to say though, I don't love having these copied over from another code base, especially with everything else from the test helpers - though I understand how it could be helpful, so I'm not sure what the best solution is here.

Looking at the tests themselves, a lot of them seem to be testing some of Struct's basic methods that this gem doesn't [deliberately] touch.

Just thinking out loud here, but perhaps something like this could be a simple option of comparing functionality?

RSpec.describe TypedStruct do
  # maybe it could be helpful to more complexity here, but not sure
  let!(:struct) { Struct.new(:a, :b, :c) }
  let!(:typed_struct) { TypedStruct.new(a: Rbs("untyped"), b: Rbs("untyped"), c: Rbs("untyped")) }

  it "has the same singleton methods as a normal struct" do
    struct.methods.each do |method|
      expect(struct.send(method)).to eq typed_struct.send(method)
    end
  end

  it "has the same behaviour as a normal struct" do
    str = struct.new(1, 2, 3)
    ts = typed_struct.new(a: 1, b: 2, c: 3)

    str.methods.each do |method|
      expect(str.send(method)).to eq ts.send(method)
    end
  end
end

Though obviously not everything would have zero arity and there may be some methods which we clearly be different eg. class 🤷🏻 (struct.methods - Object.methods).select { |m| struct.method(m).arity.zero? }

[johansenja]

But in addition, I wouldn't mind so much if compatibility is completely exact just at the moment (as long as it is always trending upwards over time), given the relatively early stage of this gem (pre 1.0)

[Arcovion]

Yea it's probably impossible to get full compatibility too, since much of the struct interface is implemented in C.
Indeed the vendored files are just basic tests, the full spec suite has loads more.
They were useful for finding the index setter bug, but incomplete as there was another bug in index setters: using negative numbers like struct[-1] = x to modify the last member didn't work.
I've fixed that bug, removed those files, and added a bunch more tests.
With your struct.methods - Object.methods idea, it turns out if you ignore Enumerable too there are only a handful of methods left:

$ irb
3.2.0-preview2 :001 > Struct.new(:test).methods(false)
 => [:new, :[], :members, :inspect, :keyword_init?]
3.2.0-preview2 :002 > Struct.instance_methods(false)
 => [:hash, :deconstruct_keys, :==, :members, :to_a, :to_s, :[], :[]=, :values_at, :to_h, :eql?, :select, :filter, :pretty_print_cycle, :deconstruct, :inspect, :each_pair, :dig, :pretty_print, :values, :length, :size, :each]

I added two describe blocks to test all those except deconstruct/deconstruct_keys/pretty_print/pretty_print_cycle. I used your idea to do the .arity.zero? methods length/size/members/to_a/to_h/to_s/inspect/values all at once.
Many of those tests aren't really needed either of course, but some might be, like the []= test which ensures negative indexes work.
I've finally tested on older rubies, though Ruby 2.7.2 was the oldest I tried. The test suite passes fine! What needs to happen is keyword_init must be set to false when using a Ruby version less than Ruby 3.2. In the interest of not breaking backwards compatibility, I used a trick to change the default only during testing. This means current users can still use keyword arguments before Ruby 3.2 without having to specify keyword_init: true. So this PR just means you can now use positional arguments if you choose to... The only potential problem is that hashes can be mixed up with keyword arguments accidentally, but since this gem is all about typed structs, you'll most likely get a TypeError if you pass a Hash without meaning to.

[Arcovion]

So as to avoid mixing test stuff with code, I added a TypedStruct.default_keyword_init option instead.
This is then a breaking change, as it requires current users to set TypedStruct.default_keyword_init = true to allow passing keywords by default (unless they're using Ruby 3.2+).
This means it now has the same interface as Struct on all Ruby versions. Those wanting to keep the current behaviour need only set this variable once.

[Arcovion]

Ruby 3.2.0 released! Seems to work fine with this PR.