Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SemanticBlocks collides with Style/RedundantBegin #94

Closed
wadetandy opened this issue Mar 4, 2019 · 4 comments · Fixed by #263
Closed

SemanticBlocks collides with Style/RedundantBegin #94

wadetandy opened this issue Mar 4, 2019 · 4 comments · Fixed by #263
Labels
standard bug 🐞 A bug in Standard

Comments

@wadetandy
Copy link

wadetandy commented Mar 4, 2019

Whenever the Standard/SemanticBlocks rule autofixes a block which also matches rubocop's built-in Style/RedundantBegin rule, the autofix results in invalid ruby:

def do_something(array)
  value = array.map do |item|
    begin
      maybe_fail(item)
    rescue
      handle_error
    end
  end
end

running bundle exec standardrb --fix on the above results in

def do_something(array)
  value = array.map { |item|
    
      maybe_fail(item)
    rescue
      handle_error
    
  }
end

Have created a reproduction repo here.

@searls
Copy link
Contributor

searls commented Mar 4, 2019

tbqh the fact that Style/RedundantBegin behaves differently post-2.3 had me almost wanting to eliminate it yesterday so I'm open to waffle on it. I don't know how strongly folks feel about it

@searls searls added the standard bug 🐞 A bug in Standard label Mar 4, 2019
@wadetandy
Copy link
Author

My 2 cents: it definitely doesn’t match our org’s typical style (use braces for one liners and do/end for multi-line), but it wasn’t a big enough deal to not try out standard. But if it went away I’d be be in favor.

@wadetandy
Copy link
Author

Further feedback: ran into a much more complicated example in the codebase I'm converting where a SemanticBlocks error was thrown, though this time the autofixer was not able to handle it for me. To satisfy it and ruby's parser, I had to turn this:

value = object.some_method :arg1, arg2: :val do |something|
  transform_item(something)
end

into this:

value = object.some_method :arg1, arg2: :val, &->(something) {
  transform_item(something)
}

I think this is far harder to read, and there are some on my team who wouldn't quite know how to make this conversion. If SemanticBlocks does remain in standard, I'd suggest only matching cases where the autofixer can handle the change.

@CodingAnarchy
Copy link

CodingAnarchy commented Jul 31, 2020

Braces for one-liners and do/end for multi-line blocks has also been a much more common standard than semantic blocks in my experience, for whatever that's worth. I understand the argument for semantic blocks, but actually using it just makes my head hurt a bit as I try to process it. Not all that readable from my perspective as being used to the other standard. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard bug 🐞 A bug in Standard
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants