Avoid warnings about redefined methods in EnforceSuperclass module

[davidrunger]

Fix #1465

This change avoids the printing of warnings about some methods (included, on_class, and on_send) of the RuboCop::Cop::EnforceSuperclass module being redefined. These warnings are printed because, in addition to the definition of that module here in rubocop-rails, that module is first also defined in rubocop. (The module has been extracted to rubocop-rails, but is also left in rubocop, for backwards compatibility.)

This change avoids the warnings by first undefining the RuboCop::Cop::EnforceSuperclass module (the one from rubocop) before then redefining that module here in rubocop-rails.

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests. (I am not sure we can easily test this?)
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.
• If this is a new cop, consider making a corresponding update to the Rails Style Guide. (Not applicable.)

[davidrunger]

I'm not very sure that this is actually a good way to fix this issue, though it might be. But it feels sort of ugly/hacky to me.

It's extremely possible that there is some more elegant way to fix the issue. I don't even really know specifically what caused this issue to emerge in version 2.30.0, which definitely limits my ability to conceive of a more elegant fix.

If nothing else, I want to just submit this PR as a proof of concept / to illustrate the issue and a possible solution.

But I'd be very comfortable with this PR being closed, if there's some better way to fix the issue.

[davidrunger]

I didn't add any tests, because I'm not really sure that we can easily test this? This repo already has RSpec configured with config.warnings = true, and yet no warnings are printed when running bundle exec rspec. I guess that the warnings appear only when rubocop-rails is required into another project?

[davidrunger]

(This require statement is slightly misleading. It refers to active_support/core_ext/object/blank, but it not only patches Object, but also several other classes, including String.)

[davidrunger]

There are three warnings printed about methods of EnforceSuperclass being redefined. Rather than undefining each of those three methods individually, here I just undefine the whole EnforceSuperclass module (if it is already defined). We will then (re-)define it just below.

[davidrunger]

Because this RuboCop::Cop module is no longer purely just a wrapper/namespace, since below we are adding remove_const(:EnforceSuperclass) if defined?(EnforceSuperclass) to it, RuboCop now expects a documentation comment for it. But I don't think that we really want to document this module here, so I am disabling the cop here.

[Earlopain]

These warnings definitly have been a thing for a while already, I'm not sure why you only got them now. Tests here don't catch it because rubocop is already loaded before rspec can do its thing with warnings = true.

I don't think your solution is the right thing to do. It removes the warnings but you might as well temporarily set $VERBOSE to achieve the same thing.

I haven't looked into the EnforceSuperclass one, but the rails core extension can just be dropped in my opinion. There's no real point in using it, and rubocop does already define blank? anyways (hence the warning). I openend #1467 as an alternative for that.

[Earlopain]

How you handle EnforceSuperclass seems ok to me. RuboCop used to own this module but at some point it was moved to rubocop-rails and kept in rubocop itself for backwards compatibility.

Seems to be that that module here should have been scoped to a different namespace (but it wasn't, oh well). Can you add a comment that this module is also defined in RuboCop for backwards compatibility?

[davidrunger]

I openend #1467 as an alternative for that.

@Earlopain Sounds good. I have removed from this PR the removal of String#blank?, leaving that to be fixed by #1467.

Can you add a comment that this module is also defined in RuboCop for backwards compatibility?

Yes, good suggestion, thank you! I added this comment:

rubocop-rails/lib/rubocop/cop/mixin/enforce_superclass.rb

Lines 5 to 8 in f5c8ee8

	# The EnforceSuperclass module is also defined in `rubocop` (for backwards
	# compatibility), so here we remove it before (re)defining it, to avoid
	# warnings about methods in the module being redefined.
	remove_const(:EnforceSuperclass) if defined?(EnforceSuperclass)

[koic]

Thanks!