forked from rubocop/rubocop-rails
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Fix rubocop#368] Add DelegatePrivate cop
- Loading branch information
1 parent
34376e0
commit 9fc7bd6
Showing
5 changed files
with
228 additions
and
0 deletions.
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* [#368](https://github.com/rubocop/rubocop-rails/issues/368): Add DelegatePrivate cop. ([@povilasjurcys][]) |
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,106 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Rails | ||
# @example DelegatePrivate | ||
# # bad | ||
# class Foo | ||
# def bar=Bar.new | ||
# | ||
# private | ||
# | ||
# delegate :baz, to: :bar | ||
# end | ||
# | ||
# # bad | ||
# class Foo | ||
# delegate :baz, to: :bar, private: true | ||
# | ||
# def bar=Bar.new | ||
# end | ||
# | ||
# # good | ||
# class Foo | ||
# def bar=Bar.new | ||
# | ||
# private | ||
# | ||
# delegate :baz, to: :bar, private: true | ||
# end | ||
class DelegatePrivate < Base | ||
extend TargetRailsVersion | ||
|
||
MSG_MISSING_PRIVATE = '`delegate` in private section should have `private: true` option' | ||
MSG_WRONG_PRIVATE = 'private `delegate` should be put in private section' | ||
|
||
minimum_target_rails_version 6.0 | ||
|
||
def on_send(node) | ||
mark_scope(node) | ||
return unless delegate_node?(node) | ||
|
||
if private_scope?(node) && !private_delegate?(node) | ||
add_offense(node, message: MSG_MISSING_PRIVATE) | ||
elsif public_scope?(node) && private_delegate?(node) | ||
add_offense(node, message: MSG_WRONG_PRIVATE) | ||
end | ||
end | ||
|
||
private | ||
|
||
def private_delegate?(node) | ||
node.arguments.select(&:hash_type?).each do |hash_node| | ||
hash_node.each_pair do |key_node, value_node| | ||
return true if key_node.value == :private && value_node.true_type? | ||
end | ||
end | ||
|
||
false | ||
end | ||
|
||
def mark_scope(node) | ||
receiver_node, method_name, arg_node = *node | ||
return if receiver_node || arg_node | ||
|
||
@private_ranges ||= [] | ||
|
||
if method_name == :private | ||
add_private_range(node) | ||
elsif method_name == :public | ||
cut_private_range_from(node.location.first_line) | ||
end | ||
end | ||
|
||
def delegate_node?(node) | ||
return false if node.receiver | ||
|
||
node.method_name == :delegate | ||
end | ||
|
||
def private_scope?(node) | ||
@private_ranges&.any? { |range| range.include?(node.location.first_line) } | ||
end | ||
|
||
def public_scope?(node) | ||
!private_scope?(node) | ||
end | ||
|
||
def add_private_range(node) | ||
@private_ranges ||= [] | ||
@private_ranges += [node.location.first_line..node.parent.last_line] | ||
end | ||
|
||
def cut_private_range_from(from_line) | ||
@private_ranges ||= [] | ||
@private_ranges = @private_ranges.each.with_object([]) do |range, new_ranges| | ||
next if range.begin > from_line | ||
|
||
new_range = range.include?(from_line) ? (range.begin...from_line) : range | ||
new_ranges << new_range | ||
end | ||
end | ||
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
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,115 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::Rails::DelegatePrivate, :config, :rails60 do | ||
context 'when no delegate is provided' do | ||
it 'registers no offense' do | ||
expect_no_offenses(<<~RUBY) | ||
class User | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when delegate is provided in public scope without "private: true"' do | ||
it 'registers no offense' do | ||
expect_no_offenses(<<~RUBY) | ||
class User | ||
delegate :name, to: :user | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when delegate is provided in public scope with "private: true"' do | ||
it 'registers an offense' do | ||
expect_offense(<<~RUBY) | ||
class User | ||
delegate :name, to: :user, private: true | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ private `delegate` should be put in private section | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when delegate is provided in private scope without "private: true"' do | ||
it 'registers an offense' do | ||
expect_offense(<<~RUBY) | ||
class User | ||
private | ||
delegate :name, to: :user | ||
^^^^^^^^^^^^^^^^^^^^^^^^^ `delegate` in private section should have `private: true` option | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when delegate is provided in private scope with "private: true"' do | ||
it 'registers no offense' do | ||
expect_no_offenses(<<~RUBY) | ||
class User | ||
private | ||
delegate :name, to: :user, private: true | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when delegate is provided in public scope without "private: true" in outer class' do | ||
it 'registers no offense' do | ||
expect_no_offenses(<<~RUBY) | ||
class User | ||
class InnerUser | ||
private | ||
def foo; end | ||
end | ||
delegate :name, to: :user | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when `private: true` is in explicit public scope' do | ||
it 'registers no offense' do | ||
expect_no_offenses(<<~RUBY) | ||
class User | ||
private | ||
def foo | ||
end | ||
public | ||
delegate :name, to: :user | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when private scope is set on method, but `private: true` is used in public scope' do | ||
it 'registers no offense' do | ||
expect_no_offenses(<<~RUBY) | ||
class User | ||
private def foo | ||
end | ||
delegate :name, to: :user | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'with rails < 6.0', :rails52 do | ||
it 'registers no offense' do | ||
expect_no_offenses(<<~RUBY) | ||
class User | ||
private | ||
delegate :name, to: :user | ||
end | ||
RUBY | ||
end | ||
end | ||
end |