-
-
Notifications
You must be signed in to change notification settings - Fork 269
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
34376e0
commit 6958079
Showing
5 changed files
with
241 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,103 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Rails | ||
# Looks for `delegate` in private section without `private: true` option. | ||
# | ||
# @example | ||
# # bad | ||
# private | ||
# delegate :baz, to: :bar | ||
# | ||
# # bad | ||
# delegate :baz, to: :bar, private: true | ||
# | ||
# # good | ||
# private | ||
# delegate :baz, to: :bar, private: true | ||
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 | ||
|
||
def on_class(node) | ||
cut_from_private_range(node.location.first_line..node.location.last_line) | ||
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) | ||
return if node.receiver || !node.arguments.empty? | ||
|
||
@private_ranges ||= [] | ||
|
||
scope_range = node.parent.location.first_line..node.parent.location.last_line | ||
if node.method?(:private) | ||
add_to_private_range(scope_range) | ||
elsif node.method?(:public) | ||
cut_from_private_range(scope_range) | ||
end | ||
end | ||
|
||
def delegate_node?(node) | ||
return false if node.receiver | ||
|
||
node.method?(: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_to_private_range(scope_range) | ||
@private_ranges ||= [] | ||
@private_ranges += [scope_range] | ||
end | ||
|
||
def cut_from_private_range(scope_range) | ||
@private_ranges ||= [] | ||
|
||
@private_ranges = @private_ranges.each.with_object([]) do |private_range, new_ranges| | ||
next if scope_range.cover?(private_range) | ||
|
||
if private_range.cover?(scope_range) | ||
new_ranges << (private_range.begin...scope_range.begin) | ||
new_ranges << ((scope_range.end + 1)..private_range.end) | ||
else | ||
new_ranges << private_range | ||
end | ||
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,131 @@ | ||
# 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 inner class is put in private scope and has delegate in public scope without "private: true"' do | ||
it 'does not register an offense' do | ||
expect_no_offenses(<<~RUBY) | ||
class User | ||
private | ||
class InnerUser | ||
delegate :name, to: :user | ||
end | ||
delegate :foo, to: :bar, private: true | ||
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 |