Enforce Style/RedundantBegin for new modules

[adfoster-r7]

This has come up a few times during PR review, probably best to just automate it.

# Use implicit begin blocks where possible.

# bad
def foo
  begin
    # main logic goes here
  rescue
    # failure handling goes here
  end
end

# good
def foo
  # main logic goes here
rescue
  # failure handling goes here
end

https://github.com/rubocop/ruby-style-guide#implicit-begin

Verification

Best viewed with whitespace disabled ?=w1

[adfoster-r7]

Without this, the following code:

def cleanup
  begin
    disconnect
  rescue StandardError
    nil
  end
end

Is automatically fixed to:

def cleanup

  disconnect
rescue StandardError
  nil

end

It's also not a bad rule to enable either

[gwillcox-r7]

Will pick this up tomorrow however if anyone wants to go ahead and review/land this before me, feel free to unassign me and land this so long if that would be faster.

[bwatters-r7]

I'm not willing to die on this hill, but I've never been a fan of explicit begins, as it creates a dual-use code block that obfuscates the error handling at the beginning and sort of encourages an error handler to cover the entirety of a method rather than specific lines that need to be protected. I totally get the limited use case here, but in the event the code is altered, the maintainer may not see that the code is contained in this dual-use code block unless they scan down and see the handler code.
It is a minor thing, but it still makes me shake my fist.

[jmartin-tech]

I will add my assent to @bwatters-r7 view in that explicit well scoped rescue blocks should be the norm. Review of this PR and the name for the cop suggests the rule is specific enough to recognize a limited scope rescue and only transform blocks that truly intend to recuse an entire scope already.

[gwillcox-r7]

After reviewing this I also have to agree with @bwatters-r7 and @jmartin-r7's points. Whilst there were some cases where there is definitely an advantage for code cleanliness, there were one or two cases here where the lack of explicit begin and end cases meant that users have to rely mostly on indentation to determine where things being and end. Ruby isn't Python, and therefore indentation shouldn't matter this heavily for readily determining what the code is doing in my opinion.

That being said I do think the spacing changes are good and I have no issue with those, and the majority of the begin and end changes are still good, I just think there is still some very valid points r.e this test reducing code comprehension in some cases that should be taken into account.

[adfoster-r7]

I'm in favor of the change as it's idiomatic Ruby, and with the autofix support it won't be an inconvenience to new contributors who aren't aware of this Ruby feature. Accidental overzealous try/catches of course aren't good either, but overall I don't mind if this gets merged in

[wvu]

I prefer to narrow my rescue scope as much as possible, since it makes for more precise error handling. If that isn't possible, and the rescue would otherwise cover the entire method, rescue-on-method is stylistically superior to explicit begin. JMHO.

[wvu]

However, I will also state that explicit begin, just like explicit return, provides more clarity at the expense of so-called idiomatic Ruby. This is why I like Python sometimes.

[wvu]

tl;dr I could go either way. Previous discussion.