Copilot commented on code in PR #3716:
URL: https://github.com/apache/thrift/pull/3716#discussion_r3787962967
##########
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` takes a positional hash (`initialize(d = {})`).
Using keyword-argument style here (`value: false`) will raise `ArgumentError:
unknown keyword` on Ruby 3+. Wrap the hash in `{}` so it’s passed positionally.
##########
lib/rb/spec/serializer_spec.rb:
##########
@@ -138,7 +138,7 @@ def get_protocol(transport)
describe Thrift::Serializer do
it "should serialize structs to binary by default" do
serializer =
Thrift::Serializer.new(Thrift::BinaryProtocolAcceleratedFactory.new)
- data = serializer.serialize(SpecNamespace::Hello.new(:greeting => "'Ello
guv'nor!"))
+ data = serializer.serialize(SpecNamespace::Hello.new(greeting: "'Ello
guv'nor!"))
expect(data).to eq("\x0B\x00\x01\x00\x00\x00\x0E'Ello guv'nor!\x00")
Review Comment:
`SpecNamespace::Hello` is a `Thrift::Struct` (`initialize(d = {})`).
`Hello.new(greeting: ...)` passes keyword args and will raise on Ruby 3+. Pass
a positional hash instead.
This issue also appears in the following locations of the same file:
- line 145
- line 162
- line 185
- line 201
- line 214
- ...and 2 more
##########
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
end
Review Comment:
`SpecNamespace::Hello` is a `Thrift::Struct` (`initialize(d = {})`).
`Hello.new(greeting: ...)` uses keyword args and will raise on Ruby 3+. Pass an
explicit hash.
This issue also appears in the following locations of the same file:
- line 181
- line 223
##########
lib/rb/spec/union_spec.rb:
##########
@@ -188,7 +188,7 @@
end
it "should support old style constructor" do
- union = SpecNamespace::My_union.new(:integer32 => 26)
+ union = SpecNamespace::My_union.new(integer32: 26)
expect(union.get_set_field).to eq(:integer32)
expect(union.get_value).to eq(26)
end
Review Comment:
`Thrift::Union#initialize` accepts a positional hash (it checks
`name.is_a?(Hash)`). `My_union.new(integer32: 26)` uses keyword args and will
raise on Ruby 3+. Pass an explicit hash.
This issue also appears in the following locations of the same file:
- line 200
- line 206
- line 213
##########
lib/rb/spec/processor_spec.rb:
##########
@@ -110,7 +110,7 @@ def output_protocol
handler = double("Handler")
expect(handler).to receive(:unblock).with(9)
processor = SpecNamespace::NonblockingService::Processor.new(handler)
- args = SpecNamespace::NonblockingService::Unblock_args.new(:n => 9)
+ args = SpecNamespace::NonblockingService::Unblock_args.new(n: 9)
input = input_protocol("unblock", Thrift::MessageTypes::CALL, 13, args)
output_transport, output = output_protocol
Review Comment:
`Unblock_args` is a generated `Thrift::Struct` (initializer `initialize(d =
{})`). `new(n: 9)` passes keyword args and will raise on Ruby 3+. Pass a
positional hash instead.
This issue also appears on line 121 of the same file.
##########
lib/rb/spec/struct_spec.rb:
##########
@@ -178,7 +178,7 @@ def validate_default_arguments(object)
expect(struct.simple).to eq(42)
expect(struct.complex).to eq({1 => {"pi" => Math::PI, "e" => Math::E},
14 => {"feigenbaum" => 4.669201609}})
- expect(struct.hello).to eq(SpecNamespace::Hello.new(:greeting => "what's
up?"))
+ expect(struct.hello).to eq(SpecNamespace::Hello.new(greeting: "what's
up?"))
expect(struct.words).to eq("apple banana")
expect(struct.ints).to eq([4, 23, 4, 29])
Review Comment:
`SpecNamespace::Hello` is a `Thrift::Struct` and expects a positional hash.
`Hello.new(greeting: ...)` passes keyword args and will raise on Ruby 3+.
##########
lib/rb/spec/compact_protocol_spec.rb:
##########
@@ -465,11 +465,11 @@
it "should deal with fields following fields that have non-delta ids" do
brcp = Thrift::Test::BreaksRubyCompactProtocol.new(
- :field1 => "blah",
- :field2 => Thrift::Test::BigFieldIdStruct.new(
- :field1 => "string1",
- :field2 => "string2"),
- :field3 => 3)
+ field1: "blah",
+ field2: Thrift::Test::BigFieldIdStruct.new(
+ field1: "string1",
+ field2: "string2"),
Review Comment:
`Thrift::Test::BreaksRubyCompactProtocol` and
`Thrift::Test::BigFieldIdStruct` are generated `Thrift::Struct`s (`initialize(d
= {})`). This constructor form passes keyword args and will raise on Ruby 3+.
Wrap the field hashes in `{}` so they’re passed positionally.
This issue also appears on line 482 of the same file.
--
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]
