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

Fix Ruby 2.7 warnings in Action Pack 6.0 #38051

Merged

Conversation

casperisfine
Copy link
Contributor

There are a few left that I couldn't figure out:

rails/actionpack/lib/action_dispatch/middleware/cookies.rb:574: warning: The last argument is used as the keyword parameter
rails/actionpack/lib/action_dispatch/middleware/cookies.rb:574: warning: for method defined here; maybe ** should be added to the call?
rails/actionpack/lib/action_dispatch/middleware/cookies.rb:574: warning: The last argument is used as the keyword parameter
rails/actionpack/lib/action_dispatch/middleware/cookies.rb:574: warning: for method defined here; maybe ** should be added to the call?
rails/actionview/lib/action_view/renderer/abstract_renderer.rb:20: warning: The last argument is used as the keyword parameter
rails/actionview/lib/action_view/lookup_context.rb:142: warning: for `template_exists?' defined here; maybe ** should be added to the call?
rails/actionview/lib/action_view/renderer/abstract_renderer.rb:20: warning: The last argument is used as the keyword parameter
rails/actionview/lib/action_view/lookup_context.rb:142: warning: for `template_exists?' defined here; maybe ** should be added to the call?

cc @rafaelfranca @Edouard-chin @kamipo

@rails-bot rails-bot bot added the actionpack label Dec 20, 2019
@kamipo
Copy link
Member

kamipo commented Dec 20, 2019

rails/actionpack/lib/action_dispatch/middleware/cookies.rb:574: warning: The last argument is used as the keyword parameter
rails/actionpack/lib/action_dispatch/middleware/cookies.rb:574: warning: for method defined here; maybe ** should be added to the call?

In Ruby 2.7, each like method cannot pass args to block as kwargs.

https://bugs.ruby-lang.org/issues/14183#note-101

In that case it is required to split kwargs manually.

https://github.com/rails/rails/blob/a86dd0ce686ee2ba6bbfb07526e4911b9a271928/actionpack/lib/action_dispatch/middleware/cookies.rb#L574-L576

@kamipo
Copy link
Member

kamipo commented Dec 20, 2019

Okey I've fixed with the diff:

diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb
index 1554be9ef1..5c38787b55 100644
--- a/actionpack/lib/action_dispatch/middleware/cookies.rb
+++ b/actionpack/lib/action_dispatch/middleware/cookies.rb
@@ -571,7 +571,8 @@ def initialize(parent_jar)
         secret = request.key_generator.generate_key(request.signed_cookie_salt)
         @verifier = ActiveSupport::MessageVerifier.new(secret, digest: signed_cookie_digest, serializer: SERIALIZER)
 
-        request.cookies_rotations.signed.each do |*secrets, **options|
+        request.cookies_rotations.signed.each do |(*secrets)|
+          options = secrets.extract_options!
           @verifier.rotate(*secrets, serializer: SERIALIZER, **options)
         end
       end
diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb
index 08019cff1d..51431fe045 100644
--- a/actionview/lib/action_view/renderer/template_renderer.rb
+++ b/actionview/lib/action_view/renderer/template_renderer.rb
@@ -95,7 +95,7 @@ def resolve_layout(layout, keys, formats)
             end
           rescue ActionView::MissingTemplate
             all_details = @details.merge(formats: @lookup_context.default_formats)
-            raise unless template_exists?(layout, nil, false, [], all_details)
+            raise unless template_exists?(layout, nil, false, [], **all_details)
           end
         when Proc
           resolve_layout(layout.call(@lookup_context, formats), keys, formats)

@casperisfine casperisfine force-pushed the actionpack-6-0-stable-ruby-2.7-warnings branch from a86dd0c to 5b84be7 Compare December 20, 2019 13:33
@rails-bot rails-bot bot added the actionview label Dec 20, 2019
@casperisfine
Copy link
Contributor Author

Thanks. I applied it.

@kamipo kamipo merged commit ea5749c into rails:6-0-stable Dec 20, 2019
kamipo added a commit that referenced this pull request Dec 20, 2019
…-warnings

Fix Ruby 2.7 warnings in Action Pack 6.0
@kamipo
Copy link
Member

kamipo commented Dec 20, 2019

Forwardported to master in abf0465.

@nezirz
Copy link

nezirz commented Dec 26, 2019

Hmm I am using rbenv ruby 2.7.0 and rails 6.0.2.1 and by running rails s or rails s -u puma getting this console warning:

:~/rcode/rb27$ rails s
=> Booting Puma
=> Rails 6.0.2.1 application starting in development 
=> Run `rails server --help` for more startup options
/home/zire/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/stack.rb:37: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/home/zire/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/static.rb:110: warning: The called method `initialize' is defined here
Puma starting in single mode...
* Version 4.3.1 (ruby 2.7.0-p0), codename: Mysterious Traveller
* Min threads: 5, max threads: 5
* Environment: development
* Listening on tcp://127.0.0.1:3000
* Listening on tcp://[::1]:3000

@connorshea
Copy link
Contributor

@nezirz try using the 6-0-stable branch from git, that should fix any deprecation warnings:

# Gemfile
gem 'rails', git: 'https://github.com/rails/rails', branch: '6-0-stable'

@nezirz
Copy link

nezirz commented Dec 27, 2019

@connorshea I have option also to hide warnings within instructions from: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

with:

If you want to disable the deprecation warnings, please use a command-line argument -W:no-deprecated or add Warning[:deprecated] = false to your code.

But, I was more thinking of updating the Actionpack 6.0.2.1 for handling this warnings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants