Remove some annoying cops

[cilim]

I would add these as well:

Layout/LineLength:
  IgnoreCopDirectives: true
  IgnoredPatterns: ['\A#']

RSpec/MultipleExpectations:
  Enabled: false

RSpec/MultipleMemoizedHelpers:
  Enabled: false

Style/HashAsLastArrayItem:
  Enabled: false

Metrics/BlockLength:
  Exclude:
    - '**/*.rake'
    - 'spec/**/*.rb'

AllCops:
  Exclude:
    - 'node_modules/**/*'

• IgnoreCopDirectives - if we comment something and the comment passes 100 line length, it won't flag as an offence
• IgnoredPatterns: ['\A#'] - if our model annotations pass more than 100 lines, it won't flag as an offence
• MultipleExpectations - we don't write spec examples with only 1 expect (at least not in request specs, so maybe we can exclude it there)
• MultipleMemoizedHelpers - after giving this new cop a glance, I don't think it's useful to us
• BlockLength rule excluded from rake tasks and all spec files. Sometimes it doesn't make sense to go into a new row just because of a character or 2 that passes the 100 mark. It's more difficult to read, and doesn't give any benefits.
• exclude node_modules completely, since a lot of us went to webpacker and yarn, which creates the folder and a lot of 3rd party dependent offences

I would also remove these lines:

Style/HashEachMethods:
  Enabled: true

Style/HashTransformKeys:
  Enabled: true

Style/HashTransformValues:
  Enabled: true

Style/FrozenStringLiteralComment:
  Enabled: false

since we have NewCops: enable. As for the FrozenStringLiteralComment, I'd keep that enabled too.

[JureCindro]

@cilim It's quite important for the RSpec/MultipleExpectations to remain on. Reasoning being that unless you have the :aggregate_failures tag on the example, the first error or failed expect statement would return from the example block, which is usually not what you want, unless the latter expectation is somehow dependant on the former. Frequent example from our request specs would be, having a post method that fails to create a resource because of a missing attribute, and specs failing in the first expectation which checks the status code. The next expectation that usually checks the json body, would not be run, making the debugging process harder. Having :aggregate_failures would make this cop happy and provide you with the actual error response in the spec failure message, notifying you about the details of the problem without you needing to manually set breakpoints, etc.

Adding this tag to the whole requests folder does not work with rubocop unfortunately, so I usually exclude the requests folder manually if the meta tag is added to specs in that folder.

Proposed solution:

# .rubocop.yml
RSpec/MultipleExpectations:
  Exclude:
    - 'spec/requests/**/*_spec.rb'

# spec/rails_helper.rb
RSpec.configure do |config|
  # ...
  config.define_derived_metadata(type: :request) { |metadata| metadata[:aggregate_failures] = true }
  # ..
end

[JureCindro]

While RSpec/MultipleMemoizedHelpers is really annoying on older project, keeping it on new ones makes you double think the spec setup before disabling it manually, helping you to keep you specs as slim as possible and thus faster. Maybe exclude just the request folder, as they are our system tests which usually have more elaborate setup.

[JureCindro]

I think that Style/HashAsLastArrayItem goes well with the ruby 3.0 change of separation of positional and keyword arguments. I have it enabled on one of the projects and the only real change was how the policy permitted_attributes are written, the syntax changing slightly:

[:first_name, :last_name, tags: []]
# to
[:first_name, :last_name, { tags: [] }]

[cilim]

@JureCindro good forward thinking 👍 I agree on all points.

[d4be4st]

@JureCindro Ruby 3.0 will need to use this syntax? [:first_name, :last_name, { tags: [] }]

[JureCindro]

As far as I know it won't, it accepts the arguments as just as *args, and not *args, **kwargs.

But the usage without the braces feel off to the separation of arguments feel, where you will have to be explicit whether you're passing a hash or keyword arguments. It makes sense to me that we would extend that rule to the literal array constructor, even if it is not deprecated.

Ruby 3.0.0-dev:

Having an explicit hash as a last array item makes the input of an array constructor the same as the output, it looks like it will be more future proof, and the usage convention would be the same as with other method calls. As of ruby 3.0.0-dev it still works both ways though.