Skip to content
This repository has been archived by the owner on Jan 5, 2024. It is now read-only.

Suppress deprecation warning for Regexp.new with 3rd argument #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

koic
Copy link

@koic koic commented Feb 18, 2023

Follow up ruby/ruby#6976.

This PR suppresses the following deprecation warning.

% ruby -v
ruby 3.2.1 (2023-02-08 revision 31819e82c8) [x86_64-darwin19]
% bundle exec rake
(snip)

/Users/koic/src/github.com/dwaite/cookiejar/lib/cookiejar/cookie_validation.rb:48: warning:
3rd argument to Regexp.new is deprecated and will be removed in Ruby 3.3; use 2nd argument instead

Follow up ruby/ruby#6976.

This PR suppresses the following deprecation warning.

```console
% ruby -v
ruby 3.2.1 (2023-02-08 revision 31819e82c8) [x86_64-darwin19]
% bundle exec rake
(snip)

/Users/koic/src/github.com/dwaite/cookiejar/lib/cookiejar/cookie_validation.rb:48: warning:
3rd argument to Regexp.new is deprecated and will be removed in Ruby 3.3; use 2nd argument instead
```
@@ -45,7 +45,7 @@ module PATTERN
HDN = /\A#{PATTERN::HOSTNAME}\Z/
TOKEN = /\A#{PATTERN::TOKEN}\Z/
PARAM1 = /\A(#{PATTERN::TOKEN})(?:=#{PATTERN::VALUE1})?\Z/
PARAM2 = Regexp.new "(#{PATTERN::TOKEN})(?:=(#{PATTERN::VALUE2}))?(?:\\Z|;)", '', 'n'
PARAM2 = Regexp.new "(#{PATTERN::TOKEN})(?:=(#{PATTERN::VALUE2}))?(?:\\Z|;)", Regexp::NOENCODING
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [100/80]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already over 80 line length in this lib/cookiejar/cookie_validation.rb. Also, RuboCop's default is not 80.
rubocop/rubocop#7952

This line length can be changed to 80 or less, but it probably won't improve readability, so I'm leaving it there.

@yahonda
Copy link

yahonda commented Mar 16, 2023

I want this pull request merged because this behavior gets error at Ruby 3.3.0dev since ruby/ruby#7039 .

Rails 7.0.z needs blade gem and the CI is failing.
https://buildkite.com/rails/rails/builds/94669#0186dc1f-05f9-4363-b83c-95d392a7c5d9/3376-3379

Rails 7.0.z depends on blade gem. Then blade gem indirectly depends on cookiejar via these gems like that: blade -> faye -> em-http-request -> cookiejar

@koic
Copy link
Author

koic commented Mar 16, 2023

Yeah, that's exactly it!
@dwaite Could you review this PR and cut a new release if it is fine with you?

yahonda added a commit to yahonda/rails that referenced this pull request Mar 28, 2023
This commit addresses the CI failure at 7-0-stable branch since ruby/ruby#7039
https://buildkite.com/rails/rails/builds/94361#0186a504-e06f-4f75-bcb6-df93822f78ed/3283-3286

- Without this commit

```ruby
$ ruby -v
ruby 3.3.0dev (2023-03-28T07:26:46Z master d766d5346b) [x86_64-linux]
$ bundle exec blade build
bundler: failed to load command: blade (/home/yahonda/.rbenv/versions/3.3.0-dev/bin/blade)
/home/yahonda/.rbenv/versions/3.3.0-dev/lib/ruby/gems/3.3.0+0/gems/cookiejar-0.3.3/lib/cookiejar/cookie_validation.rb:48:in `initialize': wrong number of arguments (given 3, expected 1..2) (ArgumentError)

    PARAM2 = Regexp.new "(#{PATTERN::TOKEN})(?:=(#{PATTERN::VALUE2}))?(?:\\Z|;)", '', 'n'
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	from /home/yahonda/.rbenv/versions/3.3.0-dev/lib/ruby/gems/3.3.0+0/gems/cookiejar-0.3.3/lib/cookiejar/cookie_validation.rb:48:in `new'
```

- With this commit

```ruby
$ ruby -v
ruby 3.3.0dev (2023-03-28T07:26:46Z master d766d5346b) [x86_64-linux]
$ bundle exec blade build
Building assets…
$
```

This commit has been made on top of `7-0-stable` not `main` branch
because `blade` has been removed from main branch via rails#46206

`blade` gem depends on `cookiejar` indirectly as follows.
`blade` depending on `faye` depending on `em-http-request` depending on `cookiejar`

To use `cookiejar` with Ruby 3.3 it needs dwaite/cookiejar#52 merged.
Instead of using `https://github.com/koic/cookiejar/tree/suppress_deprecation_warning_for_regexp_n_flag` branch cherry-pick the commit a73e526 because the this pull request may be changed like addressing RuboCop offenses.

blade is only used for testing this change should be enough.

Co-authored-by: Koichi ITO <koic.ito@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants