Make Style/RedundantBegin aware of do-end block in Ruby 2.5

[pocke]

We can omit begin in do-end block since Ruby 2.5.
https://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/57376
For example, this code is correct in Ruby 2.5.

do_something do
  something
rescue => ex
  anything
end

This change will make Style/RedundantBegin aware of do-end block in Ruby 2.5.

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Used the same coding conventions as the rest of the project.
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• All tests(rake spec) are passing.
• The new code doesn't generate RuboCop offenses that are checked by rake internal_investigation.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.
• Updated cop documentation with rake generate_cops_documentation (required only when you've added a new cop or changed the configuration/documentation of an existing cop).

[Drenmi]

I think the cop is not accounting for the case where there are additional expressions before the begin. In those cases the body is wrapped in a begin node. Consider:

foo do
  bar

  begin
    baz
  rescue
    qux
  end
end

Which will result in the following AST:

(block
  (send nil :foo)
  (args)
  (begin
    (send nil :bar)
    (kwbegin
      (rescue
        (send nil :baz)
        (resbody nil nil
          (send nil :qux)) nil))))

[bbatsov]

There's a difference between begin and kwbegin. The code seems correct to me, although adding an extra test case probably won't hurt.

[Drenmi]

The problem is in how parser handles most bodies. If the body a single expression, it omits wrapping it in a begin. So if we want to check for offenses in both single- and multiple expression blocks, we need to do something like:

if body.offending_type? || body.begin_type? && body.children.any?(&:offending_type?)

The code above won't register an offense currently, even though it can be changed to:

foo do
  bar
  baz
rescue
  qux
end

in Ruby 2.5.

[pocke]

The code above won't register an offense currently, even though it can be changed to:

I think the changing is not safe. The code has different behaviour when bar raises an error.
In some cases the changing is safe, but in other cases the changing is not safe.
For example:

# In this case, omitting `begin` and `end` is safe.
# Because `Repository.find` (`ActiveRecord method`) never raise `Octokit` error.
foo do
   repository = Repository.find(params[:id])
  begin
    Octokit::Client.new.repository(repository.full_name)
  rescue Octokit::NotFound
    handle_error
  end
end

# In this case, omitting `begin` and `end` is not safe.
# Because `Repository.find` may raise `ActiveRecord::RecordNotFound` error.
foo do
   repository = Repository.find(params[:id])
  begin
    user = User.find(params[:user_id])
  rescue ActiveRecord::RecordNotFound
    handle_error
  end
end

It depends on an exception and methods, so I think this cop should not add any offense for the code.

[Drenmi]

Nice enhancement in Ruby, and nice implementation by you! 🚀

[koic]

Maybe '>= ruby 2.5'?

[pocke]

True. I'll fix it soon. Thanks!

[pocke]

I added test cases that are mentioned in the comment ( #5175 (comment) ), and rebased.