Skip to content

Commit

Permalink
[Fix #9061] Add new Lint/IncompatibleIoSelectWithFiberScheduler cop
Browse files Browse the repository at this point in the history
Fixes #9061.

This PR adds new `Lint/IncompatibleIoSelectWithFiberScheduler` cop.
It checks for `IO.select` that is incompatible with Fiber Scheduler since Ruby 3.0.

```ruby
# bad
IO.select([io], [], [], timeout)

# good
io.wait_readable(timeout)

# bad
IO.select([], [io], [], timeout)

# good
io.wait_writable(timeout)
```

Ref: `Fiber Scheduler` section of https://www.ruby-lang.org/en/news/2020/12/25/ruby-3-0-0-released/

This PR will make it possible to detect proven cases with redis/redis-rb#960.
  • Loading branch information
koic authored and bbatsov committed Sep 12, 2021
1 parent d199e94 commit f7c21e1
Show file tree
Hide file tree
Showing 5 changed files with 206 additions and 1 deletion.
@@ -0,0 +1 @@
* [#9061](https://github.com/rubocop/rubocop/issues/9061): Add new `Lint/IncompatibleIoSelectWithFiberScheduler` cop. ([@koic][])
5 changes: 5 additions & 0 deletions config/default.yml
Expand Up @@ -1777,6 +1777,11 @@ Lint/ImplicitStringConcatenation:
Enabled: true
VersionAdded: '0.36'

Lint/IncompatibleIoSelectWithFiberScheduler:
Description: 'Checks for `IO.select` that is incompatible with Fiber Scheduler.'
Enabled: pending
VersionAdded: '<<next>>'

Lint/IneffectiveAccessModifier:
Description: >-
Checks for attempts to use `private` or `protected` to set
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop.rb
Expand Up @@ -307,8 +307,9 @@
require_relative 'rubocop/cop/lint/heredoc_method_call_position'
require_relative 'rubocop/cop/lint/identity_comparison'
require_relative 'rubocop/cop/lint/implicit_string_concatenation'
require_relative 'rubocop/cop/lint/inherit_exception'
require_relative 'rubocop/cop/lint/incompatible_io_select_with_fiber_scheduler'
require_relative 'rubocop/cop/lint/ineffective_access_modifier'
require_relative 'rubocop/cop/lint/inherit_exception'
require_relative 'rubocop/cop/lint/interpolation_check'
require_relative 'rubocop/cop/lint/lambda_without_literal_block'
require_relative 'rubocop/cop/lint/literal_as_condition'
Expand Down
@@ -0,0 +1,67 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
#
# This cop checks for `IO.select` that is incompatible with Fiber Scheduler since Ruby 3.0.
#
# @example
#
# # bad
# IO.select([io], [], [], timeout)
#
# # good
# io.wait_readable(timeout)
#
# # bad
# IO.select([], [io], [], timeout)
#
# # good
# io.wait_writable(timeout)
#
class IncompatibleIoSelectWithFiberScheduler < Base
extend AutoCorrector

MSG = 'Use `%<preferred>s` instead of `%<current>s`.'
RESTRICT_ON_SEND = %i[select].freeze

# @!method io_select(node)
def_node_matcher :io_select, <<~PATTERN
(send
(const {nil? cbase} :IO) :select $_ $_ {(array) nil} $...)
PATTERN

def on_send(node)
return unless (read, write, timeout = io_select(node))
return unless scheduler_compatible?(read, write) || scheduler_compatible?(write, read)

preferred = preferred_method(read, write, timeout)
message = format(MSG, preferred: preferred, current: node.source)

add_offense(node, message: message) do |corrector|
corrector.replace(node, preferred)
end
end

private

def scheduler_compatible?(io1, io2)
return false unless io1.array_type? && io1.values.size == 1

io2.array_type? ? io2.values.empty? : io2.nil_type?
end

def preferred_method(read, write, timeout)
timeout_argument = timeout.empty? ? '' : "(#{timeout[0].source})"

if read.array_type? && read.values[0]
"#{read.values[0].source}.wait_readable#{timeout_argument}"
else
"#{write.values[0].source}.wait_writable#{timeout_argument}"
end
end
end
end
end
end
@@ -0,0 +1,131 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::IncompatibleIoSelectWithFiberScheduler, :config do
it 'registers and corrects an offense when using `IO.select` with single read argument' do
expect_offense(<<~RUBY)
IO.select([io], [], [])
^^^^^^^^^^^^^^^^^^^^^^^ Use `io.wait_readable` instead of `IO.select([io], [], [])`.
RUBY

expect_correction(<<~RUBY)
io.wait_readable
RUBY
end

it 'registers and corrects an offense when using `IO.select` with single read and timeout arguments' do
expect_offense(<<~RUBY)
IO.select([io], [], [], timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `io.wait_readable(timeout)` instead of `IO.select([io], [], [], timeout)`.
RUBY

expect_correction(<<~RUBY)
io.wait_readable(timeout)
RUBY
end

it 'registers and corrects an offense when using `::IO.select` with single read argument' do
expect_offense(<<~RUBY)
::IO.select([io], [], [])
^^^^^^^^^^^^^^^^^^^^^^^^^ Use `io.wait_readable` instead of `::IO.select([io], [], [])`.
RUBY

expect_correction(<<~RUBY)
io.wait_readable
RUBY
end

it 'registers and corrects an offense when using `::IO.select` with single read and timeout arguments' do
expect_offense(<<~RUBY)
::IO.select([io], [], [], timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `io.wait_readable(timeout)` instead of `::IO.select([io], [], [], timeout)`.
RUBY

expect_correction(<<~RUBY)
io.wait_readable(timeout)
RUBY
end

it 'registers and corrects an offense when using `IO.select` with single read, `nil`, and timeout arguments' do
expect_offense(<<~RUBY)
IO.select([io], nil, nil, timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `io.wait_readable(timeout)` instead of `IO.select([io], nil, nil, timeout)`.
RUBY

expect_correction(<<~RUBY)
io.wait_readable(timeout)
RUBY
end

it 'registers and corrects an offense when using `IO.select` with single write, `nil`, and timeout arguments' do
expect_offense(<<~RUBY)
IO.select(nil, [io], nil, timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `io.wait_writable(timeout)` instead of `IO.select(nil, [io], nil, timeout)`.
RUBY

expect_correction(<<~RUBY)
io.wait_writable(timeout)
RUBY
end

it 'registers and corrects an offense when using `IO.select` with single write argument' do
expect_offense(<<~RUBY)
IO.select([], [io], [])
^^^^^^^^^^^^^^^^^^^^^^^ Use `io.wait_writable` instead of `IO.select([], [io], [])`.
RUBY

expect_correction(<<~RUBY)
io.wait_writable
RUBY
end

it 'registers and corrects an offense when using `IO.select` with single write and timeout arguments' do
expect_offense(<<~RUBY)
IO.select([], [io], [], timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `io.wait_writable(timeout)` instead of `IO.select([], [io], [], timeout)`.
RUBY

expect_correction(<<~RUBY)
io.wait_writable(timeout)
RUBY
end

it 'registers and corrects an offense when using `IO.select` with single read as `self` and timeout arguments' do
expect_offense(<<~RUBY)
IO.select([self], nil, nil, timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `self.wait_readable(timeout)` instead of `IO.select([self], nil, nil, timeout)`.
RUBY

expect_correction(<<~RUBY)
self.wait_readable(timeout)
RUBY
end

it 'registers and corrects an offense when using `IO.select` with single write as `self` and timeout arguments' do
expect_offense(<<~RUBY)
IO.select(nil, [self], nil, timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `self.wait_writable(timeout)` instead of `IO.select(nil, [self], nil, timeout)`.
RUBY

expect_correction(<<~RUBY)
self.wait_writable(timeout)
RUBY
end

it 'does not register an offense when using `IO.select` with multiple read arguments' do
expect_no_offenses(<<~RUBY)
IO.select([foo, bar], [], [])
RUBY
end

it 'does not register an offense when using `IO.select` with multiple write arguments' do
expect_no_offenses(<<~RUBY)
IO.select([], [foo, bar], [])
RUBY
end

it 'does not register an offense when using `IO.select` with read and write arguments' do
expect_no_offenses(<<~RUBY)
IO.select([rp], [wp], [])
RUBY
end
end

0 comments on commit f7c21e1

Please sign in to comment.