-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Add support for keyword arguments for lanes in Ruby 3 #21587
Conversation
While that previously worked implicitly in Ruby 2.x (with our internal implementation always passing a `Hash` in `lane.call(…)` but Ruby converting it as keywords dynamically if the block expected keywords), this has to be explicit in Ruby 3.x now. See https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/
|
||
context 'when a lane expects its parameters as keywords' do | ||
def keywords_list(list) | ||
# The way keyword lists appear in error messages is different in Windows & Linux vs macOS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🫨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor comment to improve spec wording but overall this discovery is AMAZING, I love it! I think one of the benefits that I don't think you listed of using the keyword arguments is that the values are automatically "unwrapped", meaning you don't need to do e.g.
version = options[:version]
You simply just access version
😃
One thing that I missed in the specs was testing what happens if you pass nil
to a keyword arguments, like:
it 'allows a required parameter to receive nil value' do
result = @ff.runner.execute(:lane_kw_params, :ios, { version: "12.3", interactive: true, name: nil })
expect(result).to eq('name: nil; version: "12.3"; interactive: true')
end
I haven't tested this myself but I believe it allows passing nil as a valid value, right? Not ideal IMO but I guess there's no much we can do about it with the current infra haha. Also not sure if it's possible to pass nil
from fastlane's CLI 🤷♂️
Anyway, great contribution!! I'll for sure start using this the moment it gets shipped 😁
Co-authored-by: Roger Oba <[email protected]>
Addressed in 944909f 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯 💯 💯
Can't wait to use this! 🤩
Checklist
bundle exec rspec
from the root directory to see all new and existing tests passbundle exec rubocop -a
to ensure the code style is validci/circleci
builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)Motivation and Context
In Ruby 2, when a method or a block expects keyword parameters, calling that method with a
Hash
(whose keys matches those keyword parameters) was valid and resulted in Ruby 2.x making an implicit conversion. In Ruby 3, that conversion has to be explicit (see this official article).As a result, while writing lanes using keyword parameter syntax like below works in Ruby 2, it stopped working when running Fastlane in Ruby 3:
Description
This PR allows back the usage of keyword syntax in lanes block' parameters in Ruby 3, by detecting if the block provided to the lane expects at least one keyword (as opposed to just expecting a single positional parameter expected to be a Hash), and if so use the double-splat operator
**options
to make an explicit conversion of theoptions
Hash
(that our internal implementation passes tolane.call
) into a keyword list before calling the lane's provided block.Note: because the code actively checks if the block expects keywords or not, this change is not breaking, but just additive (i.e. the
lane :foo do |options|
syntax expecting aHash
will still work too)Benefits of using the (sadly lesser-known) keyword syntax in your lanes
Note that this feature of using keyword parameters in lane blocks has always been working in Ruby 2, even though not many fastlane users might have known about it. As such, this PR simply makes it so this already-existing support for it in Ruby 2 doesn't break in Ruby 3.
Some benefits of using the keyword syntax for blocks in general (and for fastlane lane parameters in particular):
kwparams(name: 'oops', version_name: 'bad')
will make Ruby complain aboutversion_name
being an unexpected parameter for thatkwparams
lane (indeed, it should beversion:
, notversion_name:
).interactive: true
in example above)While on the contrary, if you use Hash parameters (e.g.
do |options| …
), you'll get none of those benefits, and as a result:options[:interactive] || true
)options[:artifacts]
silently returningnil
without any warning or error if you accidentally usedartefacts
instead ofartifacts
at call site)Testing Steps
Check that CI passes on
bundle exec rspec
in both Ruby 2 and Ruby 3.Alternatively, if you want to test this locally:
rbenv local 2.7.4
).bundle exec rspec fastlane/spec/runner_spec.rb
, and check there is no error.rbenv local 3.2.2
).bundle exec rspec fastlane/spec/runner_spec.rb
, and observe that the tests which are running thelane_kw_params
lane of our test Fastfile are crashing withArgumentError: missing keywords: :name, :version
— because Ruby 3 doesn't do the implicit conversion of theHash
to keyword parameters so the method is called with one positional argument (theHash
) but no keyword argument.bundle exec rspec fastlane/spec/runner_spec.rb
again, and observe how the tests are now passing again.