-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#494] Create (or use) a Rubocop for the inverse keyword needed in the associations #495
Merged
malparty
merged 10 commits into
develop
from
feature/gh494-custom-rubocop-inverse-of-association
Jan 11, 2024
Merged
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
9294058
[#494] Add RequiredInverseOfRelations custom cop
Goose97 8f79086
[#494] Add more test cases
Goose97 bba04d5
[#494] Fix linter
Goose97 0c69a8d
[#494] Only run on model files
Goose97 f9dc30a
[#494] Fix failing CI
Goose97 a384ec3
[#494] Move rubocop cops install
Goose97 4631bb2
[#494] Better function names
Goose97 fba261b
[#494] Rename cops folder
Goose97 fbdd5c6
[#494] More readable control flow
Goose97 fd294bb
[#494] Rename rubocop/cops to rubocop/custom_cops
Goose97 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'fileutils' | ||
|
||
copy_file 'custom_cops/required_inverse_of_relations.rb', mode: :preserve |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
# frozen_string_literal: true | ||
|
||
source 'https://rubygems.org' | ||
|
||
gem 'rspec' | ||
gem 'rubocop' |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
GEM | ||
remote: https://rubygems.org/ | ||
specs: | ||
ast (2.4.2) | ||
diff-lcs (1.5.0) | ||
json (2.7.1) | ||
language_server-protocol (3.17.0.3) | ||
parallel (1.24.0) | ||
parser (3.2.2.4) | ||
ast (~> 2.4.1) | ||
racc | ||
racc (1.7.3) | ||
rainbow (3.1.1) | ||
regexp_parser (2.8.3) | ||
rexml (3.2.6) | ||
rspec (3.12.0) | ||
rspec-core (~> 3.12.0) | ||
rspec-expectations (~> 3.12.0) | ||
rspec-mocks (~> 3.12.0) | ||
rspec-core (3.12.2) | ||
rspec-support (~> 3.12.0) | ||
rspec-expectations (3.12.3) | ||
diff-lcs (>= 1.2.0, < 2.0) | ||
rspec-support (~> 3.12.0) | ||
rspec-mocks (3.12.6) | ||
diff-lcs (>= 1.2.0, < 2.0) | ||
rspec-support (~> 3.12.0) | ||
rspec-support (3.12.1) | ||
rubocop (1.59.0) | ||
json (~> 2.3) | ||
language_server-protocol (>= 3.17.0) | ||
parallel (~> 1.10) | ||
parser (>= 3.2.2.4) | ||
rainbow (>= 2.2.2, < 4.0) | ||
regexp_parser (>= 1.8, < 3.0) | ||
rexml (>= 3.2.5, < 4.0) | ||
rubocop-ast (>= 1.30.0, < 2.0) | ||
ruby-progressbar (~> 1.7) | ||
unicode-display_width (>= 2.4.0, < 3.0) | ||
rubocop-ast (1.30.0) | ||
parser (>= 3.2.1.0) | ||
ruby-progressbar (1.13.0) | ||
unicode-display_width (2.5.0) | ||
|
||
PLATFORMS | ||
arm64-darwin-22 | ||
|
||
DEPENDENCIES | ||
rspec | ||
rubocop | ||
|
||
BUNDLED WITH | ||
2.4.17 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
# frozen_string_literal: true | ||
|
||
module CustomCops | ||
class RequiredInverseOfRelations < RuboCop::Cop::Base | ||
MSG = 'Use inverse_of option when defining associations to prevent avoidable SQL queries and keep models in sync.' | ||
|
||
# Optimization: don't call `on_send` unless the method name is in this list | ||
RESTRICT_ON_SEND = %i[has_many belongs_to].freeze | ||
ASSOCIATION_METHODS = RESTRICT_ON_SEND.to_set | ||
|
||
def_node_matcher :association_expression_no_arguments?, <<~PATTERN | ||
(send nil? ASSOCIATION_METHODS (sym _)) | ||
PATTERN | ||
|
||
def_node_matcher :association_expression_with_arguments?, <<~PATTERN | ||
(send nil? ASSOCIATION_METHODS (sym _) (hash $...)) | ||
PATTERN | ||
|
||
def on_send(node) | ||
return add_offense(node) if association_expression_no_arguments?(node) | ||
|
||
return unless (hash_pairs = association_expression_with_arguments?(node)) | ||
|
||
add_offense(node) unless contain_inverse_of?(hash_pairs) | ||
end | ||
|
||
private | ||
|
||
def contain_inverse_of?(nodes) | ||
pattern = RuboCop::NodePattern.new('(pair (sym :inverse_of) _)') | ||
|
||
nodes.any? { pattern.match(_1) } | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'rubocop' | ||
require 'rubocop/rspec/support' | ||
require_relative '../required_inverse_of_relations' | ||
|
||
RSpec.configure do |config| | ||
config.include RuboCop::RSpec::ExpectOffense | ||
end | ||
|
||
describe CustomCops::RequiredInverseOfRelations, :config do | ||
context 'given an association expression without :inverse_of option' do | ||
it 'registers an offense' do | ||
expect_offense(<<~RUBY) | ||
class Test | ||
has_many :books | ||
^^^^^^^^^^^^^^^ Use inverse_of option when defining associations to prevent avoidable SQL queries and keep models in sync. | ||
end | ||
RUBY | ||
|
||
expect_offense(<<~RUBY) | ||
class Test | ||
belongs_to :books | ||
^^^^^^^^^^^^^^^^^ Use inverse_of option when defining associations to prevent avoidable SQL queries and keep models in sync. | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'given an association expression with :inverse_of option' do | ||
it 'registers NO offenses' do | ||
expect_no_offenses(<<~RUBY) | ||
class Test | ||
has_many :books, inverse_of: :author | ||
end | ||
RUBY | ||
|
||
expect_no_offenses(<<~RUBY) | ||
class Test | ||
belongs_to :author, inverse_of: :books | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'given a method which have the same name with one of Rails association helpers' do | ||
context 'given the method invocation has explicit module/class' do | ||
it 'registers NO offenses' do | ||
expect_no_offenses(<<~RUBY) | ||
class Test | ||
def test | ||
Module.has_many :books | ||
end | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'given the method invocation has explicit receiver' do | ||
it 'registers NO offenses' do | ||
expect_no_offenses(<<~RUBY) | ||
class Test | ||
def test | ||
self.has_many :books | ||
end | ||
end | ||
RUBY | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if that works, but can we try to rename the folder:
./rubocop/cops/
. At first glance,custom_cops
might be hard to recognize for people new in the world of Rails 🛤️There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Fixed in fba261b