Upgrade rubocop

[aurelien-reeves]

Description

Rubocop is fixed to a pretty old version.
The purpose of this PR is to try upgrading it step by step.

Type of change

Please delete options that are not relevant.

• Refactoring (improvements to code design or tooling that don't change behaviour)

Note to other contributors

One of the first offense to be fixed with a new version is done in lib/cucumber/glue/dsl.rb. Before we just had the extend(Cucumber::Glue::Dsl) instruction to extend the global Object class. This triggered an offense. I've fixed it by explicitly replacing it with the following:

class Object
  # TODO: can we avoid adding methods to the global namespace (Kernel)
  prepend(Cucumber::Glue::Dsl)
end

That way the extension of the global Object class is made explicit. Tests seems to be passing. Does that looks good to advanced ruby developers?

Checklist:

Your PR is ready for review once the following checklist is
complete. You can also add some checks if you want to.

• New and existing tests are passing locally and on CI
• bundle exec rubocop reports no offenses

[aurelien-reeves]

Rubocop has been bumped to the latest version, and offenses have been fixed.

To review this PR, I think it would be easier to look at the commits rather than the diff.

@luke-hill What do you think? I removed all inline magic comment I had added before except for one that I have documented.

[luke-hill]

So yeh reviewing this was hell! Please don't give me a PR with 158 things again to review 😓

We should use the todo file to partially fix things up and then just click them off in the todo file.

[aurelien-reeves]

So yeh reviewing this was hell! Please don't give me a PR with 158 things again to review 😓

We should use the todo file to partially fix things up and then just click them off in the todo file.

Yeah, sorry for that 😓
I hate big PR myself.

I'll summarize all your comments and plan some specific actions.

[aurelien-reeves]

So, to summarize:

What we could do as part of dedicated PR (I would open dedicated issues to make sure we do not forget those):

• removing the unused _=nil from cli::main#initialize parameter list in v8 (Remove former 'stdin' parameter from Cucumber::Cli::Main#initialize #1581)
• allowing inner slashes in RegexpLiteral rubocop configuration (Allow inner slashes in RegexpLiteral rubocop configuration #1582)
• formatter/ansicolor.rb need to be refactored (Refactor formatter/ansicolor.rb #1583)
• removing DataTable#map_column! in v8 which was planned to be removed in v2 (Remove deprecated DataTable#map_column! #1584)
• refactor fake modules and classes in configuration_spec.rb, http_io_spec.rb, junit_spec.rb and fake_objects.rb to prevent defining those fakes globally (Refactor fakes and mocks used for some tests #1585)

To merge the current PR:

• make sure that calling super in the events initialize methods is safe as part of the actual PR (lib/cucumber/events/step_activated.rb and lib/cucumber/events/step_definition_registered.rb)
• can we safely delete some code from proto_world.rb? I am still confused with this. It seems to be useless. What would make it useful? Cucumber::Glue#steps and Cucumber::Glue#inspect are just calling super without any other code.

Did I missed something? Does that look like a plan?

[luke-hill]

Yep sounds great. I've attached a couple of additional comments where needed.

[aurelien-reeves]

make sure that calling super in the events initialize methods is safe as part of the actual PR (lib/cucumber/events/step_activated.rb and lib/cucumber/events/step_definition_registered.rb)

Actually, Cucumber::Core::Event generates sub-classes with initializers dedicated to generate attribute readers.

So in derived classes as StepDefinitionRegistered or StepActivated, writing an initializer to initialize the attribute readers is doing what already does the base class.

Calling superis doing it twice.

Removing the initializer is removing duplicated code.

And this is actually covered with tests: when not properly initializing attribute readers for those two classes, some tests broke.

[aurelien-reeves]

can we safely delete some code from proto_world.rb? I am still confused with this. It seems to be useless. What would make it useful? Cucumber::Glue#steps and Cucumber::Glue#inspect are just calling super without any other code.

I have restored the deleted methods. There purpose is actually documentation.

[aurelien-reeves]

@luke-hill I guess I answer all your questions. Can we merge?