Rubocop: Old specs need love

Created on 9 Jun 2020  路  16Comments  路  Source: rubocop-hq/rubocop

Many cop specs still use inspect_source or autocorrect_source. We'll like to deprecated this (see #8003), so they should be refactored to use expect_offense + expect_correction instead (I'd love to have an even nicer API for this).

good first issue help wanted refactoring

Most helpful comment

Hi all!
I'm planning to refactor all the Layout namespace specs if it's OK :)

All 16 comments

Some of these are much easier to convert than others. Most of the tests are as easy as merging an inspect test with and autocorrect test and rewriting a couple of lines.

The tests that make use of shared_examples will likely need to be rewritten to not use example groups. Since the code is dynamic, you likely won't be able to get the highlighting to work in a clean way.

The tests that make use of shared_examples will likely need to be rewritten to not use example groups. Since the code is dynamic, you likely won't be able to get the highlighting to work in a clean way.

Like which one?

We introduced ^{label} and _{label} to ease variable length matchers

Interesting, I didn't know ^{label} and _{label} had been added. It looks like those would solve the issue of having different length highlights!

@marcandre I am willing to take some of this refactoring work. What would you prefer of the following?

  1. Separate PRs each with 2-3 cops' specs refactored - easy to review - don't worry about git history.

  2. Club all Style cops into one PR, Lint in another, so on.. - pain to review - will keep git history clean.

Or anything else?

Either work, I'd say group them as you like.

I'm not sure if it could be a hint, the following is an examples that PRs were split into A-Z and opened in the past.

5134, #5132, #5131, #5130, and #5129.

Some new syntax of use here:

  • ^{} to mark an offense on an empty line (added in #8255)
  • [...] to abbreviate offense messages (added in #8297)

I'm planning to pick up some more of these this weekend, to pick up the ones I missed in the Lint namespace and make a start on the Style namespace (probably A-M split over 2 or 3 PRs).

Hi all!
I'm planning to refactor all the Layout namespace specs if it's OK :)

I'm picking up the rest of the Style namespace cops this week as I have a bit of free time.

I ran into a problem while I was working on the task. I can't understand how to implement expect_offense for missing final newline.

To throw an error I've written a wrong spec

expect_no_offenses(<<~RUBY.chomp)
  puts 'testing rubocop when final new line is missing
                            after block comments'

  =begin
  first line
  second line
  third line
  =end
RUBY

The error was

-=end
+=end    ^{} Final newline missing.

So if I'll write =end ^{} Final newline missing. in spec it won't caught. If I move error message to next line error will disappeared because we will have the newline.

Have anybody already met the problem? Is there a solution or do we have to enhance parser?

@AndreiEres Good point. You are right, I don't think there's a way around this one. We could add some functionality to expect_offense but since this case is very rare, probably best to leave it as is.

I have a solution for it, but life happened and didn't create a PR with it yet.
I created a separate matcher expect_newline_offense that works like this:

expect_newline_offense("#{str}=end", <<~RUBY)
  #{str}=end
      ^{} Final newline missing.
RUBY

But @marcandre is right in that it's a very rare case - so I was hesitant wether this matcher is worth adding or not.

If you have something that works @mechos3d, we can definitely include it.
Another "sneaky way" might be to check if the string we're being passed is missing a newline at the end, and if so reproduce that when we extract the failure messages. That might even be simpler.

So I've used error raising in https://github.com/rubocop-hq/rubocop/pull/9263 because that is built-in and needn't additional helpers. Thus we've completed with using inspect_source (if you accept PR of course).

Could you tell me, please? What can we do with old syntax in that files?
spec/support/alignment_examples.rb
spec/support/multiline_literal_brace_layout_trailing_comma_examples.rb

Was this page helpful?
0 / 5 - 0 ratings

Related issues

lepieru picture lepieru  路  3Comments

AndreiMotinga picture AndreiMotinga  路  3Comments

tedPen picture tedPen  路  3Comments

printercu picture printercu  路  3Comments

Aqualon picture Aqualon  路  3Comments