Copilot commented on code in PR #3716:
URL: https://github.com/apache/thrift/pull/3716#discussion_r3787461319
##########
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!"))
Review Comment:
`Thrift::Struct#initialize` is defined as `initialize(d = {})` (positional
Hash). Calling `SpecNamespace::Hello.new(greeting: ...)` uses keyword args in
Ruby 3+, which will raise `ArgumentError: unknown keyword: greeting`. Wrap the
options in `{ ... }` so it is passed as a Hash while still using modern label
syntax.
##########
lib/rb/spec/client_spec.rb:
##########
@@ -91,18 +91,18 @@ def close
expect(trans).to receive(:flush)
end
end
- klass = double("TestMessage_args", :new => mock_args)
- @client.send_message("testMessage", klass, :foo => "foo", :bar => 42)
+ klass = double("TestMessage_args", new: mock_args)
+ @client.send_message("testMessage", klass, foo: "foo", bar: 42)
end
Review Comment:
`Thrift::Client#send_message` expects the third argument to be a positional
Hash (`args = {}`), but `foo: "foo", bar: 42` is a keyword-args call in Ruby
3+. Pass an explicit Hash literal so this remains compatible across Ruby
versions.
##########
compiler/cpp/src/thrift/generate/t_rb_generator.cc:
##########
@@ -961,7 +961,7 @@ void t_rb_generator::generate_service_client(t_service*
tservice) {
f_service_.indent() << messageSendProc << "(\"" << funname << "\", " <<
argsname;
for (fld_iter = fields.begin(); fld_iter != fields.end(); ++fld_iter) {
- f_service_ << ", :" << (*fld_iter)->get_name() << " => " <<
(*fld_iter)->get_name();
+ f_service_ << ", " << (*fld_iter)->get_name() << ": " <<
(*fld_iter)->get_name();
}
Review Comment:
In Ruby 3+, `foo: foo` in a method call is a keyword argument, not a
positional Hash. `Thrift::Client#send_message`/`send_oneway_message` take a
positional `args = {}` Hash, so generated clients will raise `ArgumentError:
unknown keyword` for every arg. Emit an explicit Hash literal (with braces) so
this stays a Hash argument while still using label syntax.
##########
lib/rb/spec/thin_http_server_spec.rb:
##########
@@ -82,36 +82,36 @@
path = "/thin"
expect(Thin::Server).to receive(:new).with(ip, port,
an_instance_of(Rack::Builder))
Thrift::ThinHTTPServer.new(processor,
- :ip => ip,
- :port => port,
- :path => path)
+ ip: ip,
+ port: port,
+ path: path)
end
Review Comment:
`Thrift::ThinHTTPServer#initialize(processor, options = {})` takes a
positional Hash. The current call site uses keyword args (`ip:`, `port:`,
`path:`), which will error on Ruby 3+. Pass an explicit Hash literal instead.
##########
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 (`d = {}`);
`TestNamespace::BoolPasser.new(value: false)` becomes a keyword-arg call in
Ruby 3+ and will raise `ArgumentError: unknown keyword: value` unless the
generated struct initializer accepts keywords. Pass an explicit Hash literal.
##########
lib/rb/spec/http_client_spec.rb:
##########
@@ -224,7 +224,7 @@
it "should set SSL verify mode when specified" do
client =
Thrift::HTTPClientTransport.new("#{@server_uri}#{@service_path}",
- :ssl_verify_mode => OpenSSL::SSL::VERIFY_NONE)
+ ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE)
Review Comment:
`Thrift::HTTPClientTransport#initialize(url, opts = {})` takes a positional
Hash. `ssl_verify_mode:` here is a keyword arg in Ruby 3+, so this will raise
`ArgumentError: unknown keyword: ssl_verify_mode`. Pass an explicit Hash
literal (and apply the same pattern to the `ssl_ca_file:` call below).
--
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]
