Add auto-correction to RedundantBegin cop

[tamird]

This correction leaves some whitespace around, but other cops should handle that, right?

[bbatsov]

👍 for having auto-correct in this cop. Not sure about the implementation, though. Ideally the auto-correct should leave the code in a clean state, as people might not be using whitespace-related cops. @jonas054 Thoughts?

[jonas054]

Yes. Much better if indentation is in a good state after the change. @tamird So please make the example code in the spec a little longer (and do the necessary adjustments in RedundantBegin#autocorrect) so it's clear that indentation is taken care of.

[tamird]

@jonas054 is there an existing facility with this kind of block unwrapping?

[tamird]

cc @yujinakayama

[yujinakayama]

@tamird I think you can consult some code in my Transpec:

• https://github.com/yujinakayama/transpec/blob/38b3fcc/spec/transpec/syntax/pending_spec.rb#L139-L380
• https://github.com/yujinakayama/transpec/blob/38b3fcc/lib/transpec/syntax/pending.rb#L35-L96
• https://github.com/yujinakayama/transpec/blob/38b3fcc/lib/transpec/util.rb#L123-L168

[tamird]

one question: how can you know what the correct indentation here will be? presumably you guys wanted me to assume that the user has indented inside the begin bit, but we really don't know that. I think this cop should probably not correct the whitespace at all, and leave that to another cop (which doesn't exist at the moment, i believe). WDYT?

[bbatsov]

Doesn't really matter how the begin was indented as we're removing it anyways. There's only only valid indentation point for the rescue in the final code and should not care if the core code in the begin and rescue's body was properly indented.

[tamird]

Alright, this code is much uglier now, but it does the whitespace stuff. PTAL

[bbatsov]

The code looks good to me, but I also see two unrelated commits in the PR.

[tamird]

They both fix the same issue, just two different cops. Would you like this split into two PRs?

[tamird]

oh, sorry, mistook this for the other PR. i'll leave those changes up to you to do elsewhere

[bbatsov]

I think you're looking at the wrong PR. :-)

[tamird]

Fixed!

[bbatsov]

Btw, something did came to mind. You might add an example showing what will happen to a single-line begin block. Something like:

begin x; y rescue z end

[tamird]

yeah, this code doesn't fix that case very well at all. I'll add a test case to the cop as well and take another stab

[tamird]

@bbatsov added

[tamird]

the code actually handles it fine, it just takes some special syntax to get the same AST

[bbatsov]

👍 Looks good! Keeps those improvements coming! :-)