Skip to content
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
merged 10 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ require:
- rubocop-rails
- rubocop-rspec
- rubocop-performance
- ./custom_cops/required_inverse_of_relations.rb
Copy link
Member

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 🛤️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. Fixed in fba261b


AllCops:
Exclude:
Expand Down Expand Up @@ -50,3 +51,9 @@ RSpec/NestedGroups:

RSpec/MultipleExpectations:
Max: 10

CustomCops/RequiredInverseOfRelations:
Enabled: true
Include:
# Only Rails model files
- !ruby/regexp /models\//
5 changes: 5 additions & 0 deletions .template/addons/custom_cops/template.rb
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
6 changes: 6 additions & 0 deletions custom_cops/Gemfile
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'
53 changes: 53 additions & 0 deletions custom_cops/Gemfile.lock
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
35 changes: 35 additions & 0 deletions custom_cops/required_inverse_of_relations.rb
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
71 changes: 71 additions & 0 deletions custom_cops/spec/required_inverse_of_relations_spec.rb
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
3 changes: 3 additions & 0 deletions template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ def apply_template!(template_root)
apply '.template/variants/api/template.rb' if API_VARIANT
apply '.template/variants/web/template.rb' if WEB_VARIANT

# Custom Rubocop cops
apply '.template/addons/custom_cops/template.rb'

# A list necessary jobs that run before complete, ex: Fixing rubocop on Ruby files that generated by Rails
apply '.template/hooks/before_complete/fix_rubocop.rb'
apply '.template/hooks/before_complete/report.rb'
Expand Down