Add new Style/MixinUsage cop

[koic]

Feature

Checks that include, extend and prepend exists at the top level.

Using these at the top level affects the behavior of Object. Under the app directory of the Rails application, the convention that a file name and a class name are mapped is applied.

% cat app/models/example.rb
# frozen_string_literal: true

include M

class Example < ApplicationRecord
end

% bundle exec rubocop app/models/example.rb
Inspecting 1 file
C

Offenses:

app/models/example.rb:3:1: C: Rails/TopLevelInclude: include is used at the top level. Use inside class or module.
include M
^^^^^^^^^

1 file inspected, 1 offense detected

Since it is difficult to automatically determine the position of include for class including multiple modules, autocorrect is not provided.

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Used the same coding conventions as the rest of the project.
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• All tests(rake spec) are passing.
• The new code doesn't generate RuboCop offenses that are checked by rake internal_investigation.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.
• Updated cop documentation with rake generate_cops_documentation (required only when you've added a new cop or changed the configuration/documentation of an existing cop).

[Drenmi]

Hm. Looks like you managed to trick Lint/AssignmentInCondition by using parentheses. 😅 Will open an issue for that.

[Drenmi]

Never mind. This is the intended behaviour. Can be disabled by setting AllowSafeAssignment: false. 🙂

[Drenmi]

There's a Node#sibling_index method you can use. 🙂

[koic]

Thanks for the review. I fixed it with f9a06a2 together with the following review comment.
#4840 (comment)

[Drenmi]

Please use the annotated form for the format string, e.g.:

`%<keyword>s`

(I think someone was working on a cop for this. 🤔)

[rrosenblum]

Isn't there a cop that checks for this?

[bbatsov]

Yeah, there's such a cop indeed. Not sure what we set it do, though.

[bbatsov]

Btw, since when this is bad just for Rails apps?

[koic]

Yes. Outside of Rails, programmers may want to use include at the top level. For example, a script programmed at the top level without class definition, a library that intentionally want to change at the top level, etc are conceivable. For that reason I restricted the scope to app directory of Rails application. That case surely comes off the rail and breaks the responsibility of object design.

[bbatsov]

My point was that no application developer will want to do something like this most of the time, and people do write applications without Rails.

[koic]

I think that's right. A use case I encountered this time was a Rails application, but I think there is a possibility that it can be applied to applications without Rails.
However, I'm wondering if this cop can be generalized for applications without file splitting rules 😟
In this PR, I started with applying it to the smallest and practical use case first.

[pocke]

I think it's a style cop. The cop has three styles, implicit, explicit and open_class. (but I think the all options are not necessary in the first pull-request.)

# implicit
include M

# explicit
Obuject.include M

# open_class
class Object
  include M
end

[bbatsov]

That sounds reasonable to me.

[koic]

I think it's a style cop. The cop has three styles

That makes sense. This looks good to me.
First of all, I'd like to make the following changes in this PR.

• Move the department of TopLevelInclude cop from Rails to Style.
• Remove stories related to Rails from document.

@bbatsov First This PR will target open_class style only. explicit style and inplicit style will be implemented on another occasion. WDYT?

[bbatsov]

@bbatsov First This PR will target open_class style only. explicit style and inplicit style will be implemented on another occasion. WDYT?

Fine by me.

[koic]

I updated and rebased. The following is the commit that changed the department.
9de3fc9

Also I updat PR's title and commit message to Style/TopLevelInclude cop.

[bbatsov]

I'm just not sure about the name of the cop as it deals with more than just include.

[koic]

Indeed. I considered the name Style/Mixin cop or Style/TopLevelMixin cop. Since the name Style/TopLevelMixin cop is not suitable when supporting explicit style and inplicit style in the future, I would like to make it Style/Mixin cop. However, Style/Mixin cop is a prime name so I'm worried about using it. WDYT?

[bbatsov]

Maybe Style/MixinUsage?

[koic]

It looks like a very concise and clear name🌟 I changed this cop name to Style/MixinUsage cop. Thank you!

[bbatsov]

👍

I forgot to reword the commit message (which mentions the old name), but we can live with this.