Merge pull request #11332 from fatkodima/class_structure-private-cons…

…tants Ignore private constants in `Layout/ClassStructure` cop
rubocop · Dec 29, 2022 · aaa4166 · aaa4166
2 parents fa401dc + bf6c018
commit aaa4166
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 14 deletions.
diff --git a/changelog/change_class_structure_to_ignore_private_constants.md b/changelog/change_class_structure_to_ignore_private_constants.md
@@ -0,0 +1 @@
+* [#8366](https://github.com/rubocop/rubocop/issues/8366): Ignore private constants in `Layout/ClassStructure` cop. ([@fatkodima][])
diff --git a/lib/rubocop/cop/base.rb b/lib/rubocop/cop/base.rb
@@ -329,11 +329,8 @@ def begin_investigation(processed_source)
         @current_corrector = nil
       end
 
-      # rubocop:disable Layout/ClassStructure
       EMPTY_OFFENSES = [].freeze
       private_constant :EMPTY_OFFENSES
-      # rubocop:enable Layout/ClassStructure
-
       # Called to complete an investigation
       def complete_investigation
         InvestigationReport.new(

diff --git a/lib/rubocop/cop/layout/class_structure.rb b/lib/rubocop/cop/layout/class_structure.rb
@@ -216,7 +216,7 @@ def find_category(node)
         def walk_over_nested_class_definition(class_node)
           class_elements(class_node).each do |node|
             classification = classify(node)
-            next if ignore?(classification)
+            next if ignore?(node, classification)
 
             yield node, classification
           end
@@ -234,17 +234,20 @@ def class_elements(class_node)
           end
         end
 
-        def ignore?(classification)
+        def ignore?(node, classification)
           classification.nil? ||
             classification.to_s.end_with?('=') ||
-            expected_order.index(classification).nil?
+            expected_order.index(classification).nil? ||
+            private_constant?(node)
         end
 
         def ignore_for_autocorrect?(node, sibling)
           classification = classify(node)
           sibling_class = classify(sibling)
 
-          ignore?(sibling_class) || classification == sibling_class || dynamic_constant?(node)
+          ignore?(sibling, sibling_class) ||
+            classification == sibling_class ||
+            dynamic_constant?(node)
         end
 
         def humanize_node(node)
@@ -264,6 +267,22 @@ def dynamic_constant?(node)
             !(expression.method?(:freeze) && expression.receiver&.recursive_basic_literal?)
         end
 
+        def private_constant?(node)
+          return false unless node.casgn_type? && node.namespace.nil?
+          return false unless (parent = node.parent)
+
+          parent.each_child_node(:send) do |child_node|
+            return true if marked_as_private_constant?(child_node, node.name)
+          end
+          false
+        end
+
+        def marked_as_private_constant?(node, name)
+          return false unless node.method?(:private_constant)
+
+          node.arguments.any? { |arg| (arg.sym_type? || arg.str_type?) && arg.value == name }
+        end
+
         def source_range_with_comment(node)
           begin_pos, end_pos =
             if (node.def_type? && !node.method?(:initialize)) ||

diff --git a/lib/rubocop/cop/mixin/annotation_comment.rb b/lib/rubocop/cop/mixin/annotation_comment.rb
@@ -47,7 +47,7 @@ def split_comment(comment)
         match.captures
       end
 
-      KEYWORDS_REGEX_CACHE = {} # rubocop:disable Layout/ClassStructure, Style/MutableConstant
+      KEYWORDS_REGEX_CACHE = {} # rubocop:disable Style/MutableConstant
       private_constant :KEYWORDS_REGEX_CACHE
 
       def regex

diff --git a/lib/rubocop/cop/variable_force.rb b/lib/rubocop/cop/variable_force.rb
@@ -108,7 +108,6 @@ def skip_children!
         :skip_children
       end
 
-      # rubocop:disable Layout/ClassStructure
       NODE_HANDLER_METHOD_NAMES = [
         [VARIABLE_ASSIGNMENT_TYPE, :process_variable_assignment],
         [REGEXP_NAMED_CAPTURE_TYPE, :process_regexp_named_captures],
@@ -123,8 +122,6 @@ def skip_children!
         *SCOPE_TYPES.product([:process_scope])
       ].to_h.freeze
       private_constant :NODE_HANDLER_METHOD_NAMES
-      # rubocop:enable Layout/ClassStructure
-
       def node_handler_method_name(node)
         NODE_HANDLER_METHOD_NAMES[node.type]
       end

diff --git a/lib/rubocop/target_ruby.rb b/lib/rubocop/target_ruby.rb
@@ -234,7 +234,6 @@ def self.supported_versions
       KNOWN_RUBIES
     end
 
-    # rubocop:disable Layout/ClassStructure
     SOURCES = [
       RuboCopConfig,
       RubyVersionFile,
@@ -245,8 +244,6 @@ def self.supported_versions
     ].freeze
 
     private_constant :SOURCES
-    # rubocop:enable Layout/ClassStructure
-
     def initialize(config)
       @config = config
     end

diff --git a/spec/rubocop/cop/layout/class_structure_spec.rb b/spec/rubocop/cop/layout/class_structure_spec.rb
@@ -287,6 +287,31 @@ def name; end
     RUBY
   end
 
+  it 'ignores misplaced private constants' do
+    expect_offense(<<~RUBY)
+      class Foo
+        def name; end
+
+        PRIVATE_CONST1 = 1
+        PRIVATE_CONST2 = 2
+        private_constant :PRIVATE_CONST1, :PRIVATE_CONST2
+        PUBLIC_CONST = 'public'
+        ^^^^^^^^^^^^^^^^^^^^^^^ `constants` is supposed to appear before `public_methods`.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class Foo
+        PUBLIC_CONST = 'public'
+        def name; end
+
+        PRIVATE_CONST1 = 1
+        PRIVATE_CONST2 = 2
+        private_constant :PRIVATE_CONST1, :PRIVATE_CONST2
+      end
+    RUBY
+  end
+
   it 'registers an offense and corrects when str heredoc constant is defined after public method' do
     expect_offense(<<~RUBY)
       class Foo