Copilot commented on code in PR #3716:
URL: https://github.com/apache/thrift/pull/3716#discussion_r3787854560
##########
lib/rb/spec/struct_spec.rb:
##########
@@ -68,7 +68,7 @@ def validate; end
def validate_default_arguments(object)
expect(object.simple).to eq(53)
expect(object.words).to eq("words")
- expect(object.hello).to eq(SpecNamespace::Hello.new(:greeting => "hello,
world!"))
+ expect(object.hello).to eq(SpecNamespace::Hello.new(greeting: "hello,
world!"))
expect(object.ints).to eq([1, 2, 2, 3])
Review Comment:
`Thrift::Struct#initialize` only accepts a positional hash (`def
initialize(d = {})`), so `SpecNamespace::Hello.new(greeting: ...)` will be
treated as keyword args on Ruby 3 and raise. Wrap the hash in `{}` to force a
positional Hash.
This issue also appears in the following locations of the same file:
- line 179
- line 265
- line 326
##########
lib/rb/spec/union_spec.rb:
##########
@@ -171,15 +171,15 @@
it "should properly serialize and match structs with a union" do
union = SpecNamespace::My_union.new(:integer32, 26)
- swu = SpecNamespace::Struct_with_union.new(:fun_union => union)
+ swu = SpecNamespace::Struct_with_union.new(fun_union: union)
trans = Thrift::MemoryBufferTransport.new
Review Comment:
`Struct_with_union` includes `Thrift::Struct` (positional-hash initializer).
`Struct_with_union.new(fun_union: union)` will be parsed as keyword args on
Ruby 3 and raise. Pass an explicit hash instead.
This issue also appears on line 190 of the same file.
##########
lib/rb/spec/nonblocking_server_spec.rb:
##########
@@ -33,7 +33,7 @@ def greeting(english)
if english
SpecNamespace::Hello.new
else
- SpecNamespace::Hello.new(:greeting => "Aloha!")
+ SpecNamespace::Hello.new(greeting: "Aloha!")
end
Review Comment:
`SpecNamespace::Hello` is a `Thrift::Struct` with `initialize(d = {})`, so
`Hello.new(greeting: "Aloha!")` will be treated as keyword args on Ruby 3 and
raise. Pass an explicit hash argument.
##########
lib/rb/spec/serializer_spec.rb:
##########
@@ -299,7 +299,7 @@ def get_protocol(transport)
end
it "does not retain previous struct state when reading fails" do
- target = SpecNamespace::Foo.new(:simple => 99, :opt_string => "old")
+ target = SpecNamespace::Foo.new(simple: 99, opt_string: "old")
payload = binary_payload(finish: false) do |protocol, transport|
Review Comment:
`Foo` is a `Thrift::Struct` (positional-hash initializer). `Foo.new(simple:
99, opt_string: "old")` uses keyword args and will raise on Ruby 3 unless
wrapped in an explicit hash.
##########
lib/rb/spec/struct_spec.rb:
##########
@@ -310,8 +310,8 @@ def validate_default_arguments(object)
it "should serialize subclasses of Set like Set" do
set_subclass = Class.new(Set)
- regular = SpecNamespace::Foo.new(:shorts => Set.new([5, 17, 239]))
- subclassed = SpecNamespace::Foo.new(:shorts => set_subclass.new([5, 17,
239]))
+ regular = SpecNamespace::Foo.new(shorts: Set.new([5, 17, 239]))
+ subclassed = SpecNamespace::Foo.new(shorts: set_subclass.new([5, 17,
239]))
serializer = Thrift::Serializer.new(Thrift::BinaryProtocolFactory.new)
Review Comment:
`Foo.new(shorts: ...)` passes Ruby keyword args; `Thrift::Struct#initialize`
expects a positional hash. This will raise on Ruby 3 unless wrapped in `{}`.
##########
lib/rb/spec/struct_spec.rb:
##########
@@ -86,14 +86,14 @@ def validate_default_arguments(object)
end
it "should properly initialize boolean values" do
- struct = SpecNamespace::BoolStruct.new(:yesno => false)
+ struct = SpecNamespace::BoolStruct.new(yesno: false)
expect(struct.yesno).to be_falsey
end
Review Comment:
Several `Thrift::Struct` instances here are being constructed with Ruby
keyword arguments (e.g., `BoolStruct.new(yesno: false)`, `Foo.new(simple: 52)`,
and the local `StructEqualityFixtures` structs). `Thrift::Struct#initialize`
only accepts a positional hash, so these will fail on Ruby 3 unless you pass an
explicit Hash argument.
This issue also appears on line 235 of the same file.
##########
test/rb/generation/test_struct.rb:
##########
@@ -38,7 +38,7 @@ def test_default_values
assert_kind_of(Hash, hello.complex)
assert_equal(hello.complex, { 6243 => 632, 2355 => 532, 23 => 532})
- bool_passer = TestNamespace::BoolPasser.new(:value => false)
+ bool_passer = TestNamespace::BoolPasser.new(value: false)
Review Comment:
`Thrift::Struct#initialize` only accepts a positional hash (`def
initialize(d = {})`), so `TestNamespace::BoolPasser.new(value: false)` will be
treated as keyword arguments on Ruby 3 and raise `unknown keyword: :value`.
Wrap the hash in `{}` to force a positional Hash argument.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
