Add a Strict mode to Style/MutableConstants

CHANGELOG.md

@@ -18,6 +18,7 @@
 
 * [#6704](https://github.com/rubocop-hq/rubocop/pull/6704): Add new `Rails/ReflectionClassName` cop. ([@Bhacaz][])
 * [#6643](https://github.com/rubocop-hq/rubocop/pull/6643): Support `AllowParenthesesInCamelCaseMethod` option on `Style/MethodCallWithArgsParentheses` `omit_parentheses`. ([@dazuma][])
+* Add an experimental strict mode to `Style/MutableConstant` that will freeze all constants, rather than just literals. ([@rrosenblum][])
 
 ### Bug fixes
 

config/default.yml

@@ -3549,6 +3549,17 @@ Style/MutableConstant:
   Description: 'Do not assign mutable objects to constants.'
   Enabled: true
   VersionAdded: '0.34'
+  VersionChanged: '0.65'
+  EnforcedStyle: literals
+  SupportedStyles:
+    # literals: freeze literals assigned to constants
+    # strict: freeze all constants
+    # Strict mode is considered an experimental feature. It has not been updated
+    # with an exhaustive list of all methods that will produce frozen objects so
+    # there is a decent chance of getting some false positives. Luckily, there is
+    # no harm in freezing an already frozen object.
+    - literals
+    - strict
 
 Style/NegatedIf:
   Description: >-

lib/rubocop/cop/style/mutable_constant.rb

@@ -6,7 +6,15 @@ module Style
       # This cop checks whether some constant value isn't a
       # mutable literal (e.g. array or hash).
       #
-      # @example
+      # Strict mode can be used to freeze all constants, rather than
+      # just literals.
+      # Strict mode is considered an experimental feature. It has not been
+      # updated with an exhaustive list of all methods that will produce
+      # frozen objects so there is a decent chance of getting some false
+      # positives. Luckily, there is no harm in freezing an already
+      # frozen object.
+      #
+      # @example EnforcedStyle: literals (default)
       #   # bad
       #   CONST = [1, 2, 3]
       #
@@ -15,10 +23,36 @@ module Style
       #
       #   # good
       #   CONST = <<~TESTING.freeze
-      #   This is a heredoc
+      #     This is a heredoc
       #   TESTING
+      #
+      #   # good
+      #   CONST = Something.new
+      #
+      #
+      # @example EnforcedStyle: strict
+      #   # bad
+      #   CONST = Something.new
+      #
+      #   # bad
+      #   CONST = Struct.new do
+      #     def foo
+      #       puts 1
+      #     end
+      #   end
+      #
+      #   # good
+      #   CONST = Something.new.freeze
+      #
+      #   # good
+      #   CONST = Struct.new do
+      #     def foo
+      #       puts 1
+      #     end
+      #   end.freeze
       class MutableConstant < Cop
         include FrozenStringLiteral
+        include ConfigurableEnforcedStyle
 
         MSG = 'Freeze mutable objects assigned to constants.'.freeze
 
@@ -39,24 +73,41 @@ def autocorrect(node)
           expr = node.source_range
 
           lambda do |corrector|
-            if node.array_type? && !node.bracketed?
+            splat_value = splat_value(node)
+            if splat_value
+              correct_splat_expansion(corrector, expr, splat_value)
+            elsif node.array_type? && !node.bracketed?
               corrector.insert_before(expr, '[')
-              corrector.insert_after(expr, '].freeze')
-            elsif node.irange_type? || node.erange_type?
+              corrector.insert_after(expr, ']')
+            elsif requires_parentheses?(node)
               corrector.insert_before(expr, '(')
-              corrector.insert_after(expr, ').freeze')
-            else
-              corrector.insert_after(expr, '.freeze')
+              corrector.insert_after(expr, ')')
             end
+
+            corrector.insert_after(expr, '.freeze')
           end
         end
 
         private
 
         def on_assignment(value)
-          range_enclosed_in_parentheses = range_enclosed_in_parentheses?(value)
+          if style == :strict
+            strict_check(value)
+          else
+            check(value)
+          end
+        end
 
-          value = splat_value(value) if splat_value(value)
+        def strict_check(value)
+          return if immutable_literal?(value)
+          return if operation_produces_immutable_object?(value)
+          return if frozen_string_literal?(value)
+
+          add_offense(value)
+        end
+
+        def check(value)
+          range_enclosed_in_parentheses = range_enclosed_in_parentheses?(value)
 
           return unless mutable_literal?(value) ||
                         range_enclosed_in_parentheses
@@ -70,10 +121,51 @@ def mutable_literal?(value)
           value && value.mutable_literal?
         end
 
+        def immutable_literal?(node)
+          node.nil? || node.immutable_literal?
+        end
+
+        def frozen_string_literal?(node)
+          FROZEN_STRING_LITERAL_TYPES.include?(node.type) &&
+            frozen_string_literals_enabled?
+        end
+
+        def requires_parentheses?(node)
+          node.irange_type? ||
+            node.erange_type? ||
+            (node.send_type? && node.loc.dot.nil?)
+        end
+
+        def correct_splat_expansion(corrector, expr, splat_value)
+          if range_enclosed_in_parentheses?(splat_value)
+            corrector.replace(expr, "#{splat_value.source}.to_a")
+          else
+            corrector.replace(expr, "(#{splat_value.source}).to_a")
+          end
+        end
+
         def_node_matcher :splat_value, <<-PATTERN
           (array (splat $_))
         PATTERN
 
+        # Some of these patterns may not actually return an immutable object,
+        # but we want to consider them immutable for this cop.
+        def_node_matcher :operation_produces_immutable_object?, <<-PATTERN
+          {
+            (const _ _)
+            (send (const nil? :Struct) :new ...)
+            (block (send (const nil? :Struct) :new ...) ...)
+            (send _ :freeze)
+            (send {float int} {:+ :- :* :** :/ :% :<<} _)
+            (send _ {:+ :- :* :** :/ :%} {float int})
+            (send _ {:== :=== :!= :<= :>= :< :>} _)
+            (send (const nil? :ENV) :[] _)
+            (or (send (const nil? :ENV) :[] _) _)
+            (send _ {:count :length :size} ...)
+            (block (send _ {:count :length :size} ...) ...)
+          }
+        PATTERN
+
         def_node_matcher :range_enclosed_in_parentheses?, <<-PATTERN
           (begin ({irange erange} _ _))
         PATTERN

manual/cops_style.md

@@ -3630,13 +3630,23 @@ foo if ['a', 'b', 'c'].include?(a)
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Enabled | Yes | Yes  | 0.34 | -
+Enabled | Yes | Yes  | 0.34 | 0.65
 
 This cop checks whether some constant value isn't a
 mutable literal (e.g. array or hash).
 
+Strict mode can be used to freeze all constants, rather than
+just literals.
+Strict mode is considered an experimental feature. It has not been
+updated with an exhaustive list of all methods that will produce
+frozen objects so there is a decent chance of getting some false
+positives. Luckily, there is no harm in freezing an already
+frozen object.
+
 ### Examples
 
+#### EnforcedStyle: literals (default)
+
 ```ruby
 # bad
 CONST = [1, 2, 3]
@@ -3646,9 +3656,41 @@ CONST = [1, 2, 3].freeze
 
 # good
 CONST = <<~TESTING.freeze
-This is a heredoc
+  This is a heredoc
 TESTING
+
+# good
+CONST = Something.new
 ```
+#### EnforcedStyle: strict
+
+```ruby
+# bad
+CONST = Something.new
+
+# bad
+CONST = Struct.new do
+  def foo
+    puts 1
+  end
+end
+
+# good
+CONST = Something.new.freeze
+
+# good
+CONST = Struct.new do
+  def foo
+    puts 1
+  end
+end.freeze
+```
+
+### Configurable attributes
+
+Name | Default value | Configurable values
+--- | --- | ---
+EnforcedStyle | `literals` | `literals`, `strict`
 
 ## Style/NegatedIf