Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Struct #1621

Merged
merged 3 commits into from
Nov 22, 2023
Merged

Update Struct #1621

merged 3 commits into from
Nov 22, 2023

Conversation

sampersand
Copy link
Contributor

@sampersand sampersand commented Nov 16, 2023

This PR updates Struct's definitions, as well as adds in the Hash::_Key interface (needed for Struct#deconstruct_keys), which will be used when I work on Hash.

There's one small caveat: There's no real way to annotate <subclass of Struct>.new. After talking it over with @soutaro, we've decided it's fine and users can just add their own news type definitions if needed. e.g.:

Point = Struct.new(:x, :y) do def to_s = "(#{x}, #{y})" end

would become

class Point < Struct[Numeric]
  attr_reader x: Numeric

  attr_reader y: Numeric
  
  def initialize: (Numeric x, Numeric y) -> void
  
  def to_s: () ->  String
end

@sampersand sampersand marked this pull request as ready for review November 16, 2023 22:08
core/struct.rbs Outdated
@@ -243,7 +246,208 @@ class Struct[Elem] < Object
# Any.new(1, 2)
# # => #<struct Any foo=1, bar=2>
#
def initialize: (attribute_name, *attribute_name, ?keyword_init: boolish) ?{ () -> void } -> void
def self.new: [K < _StructClass[Elem]] (string? classname, *interned fields, ?keyword_init: boolish?) ?{ (K) -> void } -> K
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about the type definitions and typical Ruby programs using Struct, having a generic parameter here seems unnecessary to me.

  def self.new: (string? classname, *interned fields, ?keyword_init: boolish?) ?{ (singleton(Struct)) [self: singleton(Struct)] -> void } -> untyped
              | (Symbol field1, *interned fields, ?keyword_init: boolish?) ?{ (singleton(Struct)) [self: singleton(Struct)] -> void } -> untyped

Probably, having singleton(Struct) as generic type upper bound would improve, but it looks like using untyped makes sense here. (And we no longer need _StructClass with the type definition.)

@soutaro
Copy link
Member

soutaro commented Nov 21, 2023

It looks like the hash.rbs update contains unexpected indentation?

@soutaro soutaro added this to the RBS 3.4 milestone Nov 21, 2023
Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

I will try pushing commits with docs generated by bin/generate_docs and see what happens.

@soutaro soutaro added this pull request to the merge queue Nov 22, 2023
Merged via the queue into ruby:master with commit 73a6a02 Nov 22, 2023
23 checks passed
soutaro added a commit that referenced this pull request Dec 20, 2023
…Struct"

This reverts commit 73a6a02, reversing
changes made to 0344191.
@soutaro soutaro added the Released PRs already included in the released version label Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Released PRs already included in the released version
Development

Successfully merging this pull request may close these issues.

2 participants