Style/BlockDelimiters breaks natural language of rspec matchers

[aspiers]

RSpec Expectations allow syntax like:

expect { ... some code ... }.to raise_error(MyErrorClass, my_error_message)

However if the code under test is not trivially short, it will need to be split onto multiple lines, e.g. here's some real-world code:

it "should barf if the definition's type is not right" do
  expect {
    fixture.definition = "sometype #{fixture.name} blah blah"
  }.to \
    raise_error(
      Pacemaker::CIBObject::TypeMismatch,
      "Expected #{object_type} type but loaded definition was type sometype"
    )
end

but this causes

Style/BlockDelimiters: Avoid using {...} for multi-line blocks.
https://github.com/bbatsov/ruby-style-guide#single-line-blocks

complaints from rubocop.

The problem is that changing it to

expect do
  ... some code ...
end.to raise_error(MyErrorClass, my_error_message)

completely ruins the previously natural language flow of the "expect ... to" statement. So what's the solution?

A related problem is that I'm also struggling to find a combination of line breaks/continuations and indentation which looks good here. Suggestions very welcome.

[onebree]

This is not just a problem for rspec, but any time you want to chain off a method block -- such as mapping out a new array and calling #uniq on it

[fuadsaud]

Possibly related: #162

[JelF]

what about

expect { fixture.definition = "sometype #{fixture.name} blah blah" }
  .to ...

???

[ghost]

It's easy to suppress the complaints through .rubocop.yml

Style/BlockDelimiters:
  Exclude:
    - 'spec/**/*'

[aspiers]

@monkseal Thanks, I'll give that a try when I get a chance! Does it make sense to change the rubocop defaults based on this new information?

[zzeni]

There's a built-in configuration that allows usage of the curly braces when chaining methods:

Style/BlockDelimiters:
  EnforcedStyle: braces_for_chaining

https://github.com/bbatsov/rubocop/pull/2329

[monkseal]

@zzeni Thanks! Removed my comment incase anybody is doing it the wrong way.

[aspiers]

Great to see some progress here, but I think this is still an issue until the default behaviour of this cop doesn't complain about the natural language of rspec matchers.

[aspiers]

Turns out that @zzeni's approach doesn't just allow usage of curly braces, it insists on them. I'm looking for a solution which normally insists on do ... end but makes an exception when it's an expect block. In the mean time it seems @ghost's solution is the best workaround.

[pirj]

RuboCop discussions aside, I believe the Guide can be improved:

1. ~multi-line chaining is always ugly~

# not so ugly
expect {
  filter_applies?(:foo, lambda { |a,b,c| true }, example_metadata)
}.to raise_error(ArgumentError)

# a little uglier
expect { filter_applies?(:foo, lambda { |a,b,c| true }, example_metadata) }
  .to raise_error(ArgumentError)

# bad
names.each do |name|
  puts name
end

# not so bad already?
names.each do |name|
  SomeExtravagantServiceName.process_incoming_names_and_welcome_new_user(full_name: name)
end

# this is arguably uglier than the one above
names.each { |name| SomeExtravagantServiceName.process_incoming_names_and_welcome_new_user(full_name: name) }

Some will argue that multi-line chaining would look OK with the use of {…​}, but they should ask themselves - is this code really readable and can the blocks' contents be extracted into nifty methods?

I ask this myself quite often, but most of my colleagues get surprised that expect(...) can be extracted to expect_filter_for(:foo) and contain_exactly(...) to return_a_full_list. So, in general, the answer is "yes, it's readable" and "no, extracting to methods gives my colleagues a hard time".