From 72401e5b31074164e623182a3f8c9ea01e99f8b5 Mon Sep 17 00:00:00 2001 From: Povilas Jurcys Date: Wed, 2 Aug 2023 09:40:22 +0300 Subject: [PATCH] [Fix #368] Add DelegatePrivate cop --- changelog/new_delegate_private_cop.md | 1 + config/default.yml | 5 + lib/rubocop/cop/rails/delegate_private.rb | 93 ++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../cop/rails/delegate_private_spec.rb | 115 ++++++++++++++++++ 5 files changed, 215 insertions(+) create mode 100644 changelog/new_delegate_private_cop.md create mode 100644 lib/rubocop/cop/rails/delegate_private.rb create mode 100644 spec/rubocop/cop/rails/delegate_private_spec.rb diff --git a/changelog/new_delegate_private_cop.md b/changelog/new_delegate_private_cop.md new file mode 100644 index 0000000000..c4f869a1a2 --- /dev/null +++ b/changelog/new_delegate_private_cop.md @@ -0,0 +1 @@ +* [#368](https://github.com/rubocop/rubocop-rails/issues/368): Add DelegatePrivate cop. ([@povilasjurcys][]) diff --git a/config/default.yml b/config/default.yml index 704ba72ca2..a68f844343 100644 --- a/config/default.yml +++ b/config/default.yml @@ -336,6 +336,11 @@ Rails/DelegateAllowBlank: Enabled: true VersionAdded: '0.44' +Rails/DelegatePrivate: + Description: 'Use delegate with `private: true` option in private scope.' + Enabled: true + VersionAdded: '2.20' + Rails/DeprecatedActiveModelErrorsMethods: Description: 'Avoid manipulating ActiveModel errors hash directly.' Enabled: pending diff --git a/lib/rubocop/cop/rails/delegate_private.rb b/lib/rubocop/cop/rails/delegate_private.rb new file mode 100644 index 0000000000..3db3916c1d --- /dev/null +++ b/lib/rubocop/cop/rails/delegate_private.rb @@ -0,0 +1,93 @@ +# 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 + + 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 ||= [] + + if node.method?(:private) + add_private_range(node) + elsif node.method?(:public) + cut_private_range_from(node.location.first_line) + 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_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 diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 5e0db983d6..3577cbf3a3 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -36,6 +36,7 @@ require_relative 'rails/default_scope' require_relative 'rails/delegate' require_relative 'rails/delegate_allow_blank' +require_relative 'rails/delegate_private' require_relative 'rails/deprecated_active_model_errors_methods' require_relative 'rails/dot_separated_keys' require_relative 'rails/duplicate_association' diff --git a/spec/rubocop/cop/rails/delegate_private_spec.rb b/spec/rubocop/cop/rails/delegate_private_spec.rb new file mode 100644 index 0000000000..022faf5643 --- /dev/null +++ b/spec/rubocop/cop/rails/delegate_private_spec.rb @@ -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