Rubocop Updates

[bobbrodie]

Overview

This PR updates Rubocop to pass, and addresses a number of temporarily disabled rules.

What's Changed

• Remove files from CodeQL that no longer exist
• Remove files from Rubocop that no longer exist
• Switch rubocop-rspec to plugin instead of require
• Fixed and enabled the following cops that were temporarily disabled
  • Layout/HashAlignment and
  • Lint/RedundantCopDisableDirective
  • Lint/UselessAssignment
  • Naming/BlockForwarding
  • RSpec/AnyInstance
  • RSpec/BeEq
  • RSpec/MessageChain
  • RSpec/NoExpectationExample
  • RSpec/PredicateMatcher
  • RSpec/RepeatedSubjectCall
  • Style/CaseEquality (there was only one and we should keep it)
  • Style/ClassAndModuleChildren
• Enabled the following cops that were temporarily disabled (there were no offenses)
  • RSpec/ContextWording
  • RSpec/VerifiedDoubleReference
  • Style/ArgumentsForwarding
• Bump version to 3.0.0.beta2

[bobbrodie]

@marlinpierce since you're actively involved with the gem, I'm curious what your thoughts are on predicate method naming. We currently have this default cop disabled:

Naming/PredicateName:
  Enabled: false

If we enable these, we would need to change our method names, for example

Old Name	New Name
has_one	one?
has_many	many?
has_errors?	errors?

Since we're bumping a major version (3.0.0) and expect some backwards incompatibility, do you have an opinion on this? I don't want to make headaches for folks, but we could add the new methods and mark the old ones deprecated for later removal.

[marlinpierce]

@bobbrodie I have never seen methods one? and many?. Further, searching on the web I see documentation of has_one and has_many but not those. Also, the method name ending in a question mark is strange here, as it is not a test, (as for if it has related objects).

I see the issue here, that rubocop is saying methods, has_one, has_many, and has_errors are reserved as they are common in ActiveRecord, and so they are misleading.

Documentation for Active Model show a comment that the attr_accessor emulates the has_many, there is an example of good practice to not use the reserved method names even when emulating them.

Since you are using these self defined methods to emulate the Active Record counterparts, I think this is appropriate and is another place where rubocop may be wrong by restricting a reasonable practice. I say, go ahead and use the method names you have. It makes the code clear and readable, which is the goal. When rubocop defeats that goal instead of supporting it, it is reasonable to make an exception to robocop.

If you do rename the methods, I suggest, has_one_resource, and has_many_resources. (I don't know what to suggest for has_errors.)