Merge pull request rails#55446 from skipkayhil/hm-lsmkvnurqqruoswp

[rail_inspector] Fix recognizing `self.` config

Author: rafaelfranca

tools/rail_inspector/lib/rail_inspector/configuring.rb

@@ -6,21 +6,42 @@
 
 module RailInspector
   class Configuring
-    class CachedParser
-      def initialize
-        @cache = {}
+    class Files
+      class Proxy
+        def initialize(pathname)
+          @pathname = pathname
+        end
+
+        def parse
+          @parse ||= Prism.parse_file(@pathname.to_s).value
+        end
+
+        def read
+          @pathname.read
+        end
+
+        def write(string)
+          @pathname.write(string)
+        end
+
+        def to_s
+          @pathname.to_s
+        end
       end
 
-      def call(path)
-        @cache[path] ||= Prism.parse_file(path.to_s).value
+      def initialize(root)
+        @root = Pathname.new(root)
+        @files = {}
       end
-    end
 
-    DOC_PATH = "guides/source/configuring.md"
-    APPLICATION_CONFIGURATION_PATH =
-      "railties/lib/rails/application/configuration.rb"
-    NEW_FRAMEWORK_DEFAULTS_PATH =
-      "railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_%{version}.rb.tt"
+      def []=(name, path)
+        @files[name] = Proxy.new(@root.join(path))
+      end
+
+      def method_missing(name, ...)
+        @files[name] || super
+      end
+    end
 
     class Doc
       attr_accessor :general_config, :versioned_defaults
@@ -45,12 +66,19 @@ def to_s
       end
     end
 
-    attr_reader :errors, :parser
+    attr_reader :errors, :files
 
     def initialize(rails_path)
       @errors = []
-      @parser = CachedParser.new
-      @rails_path = Pathname.new(rails_path)
+      @files = Files.new(rails_path)
+
+      @files[:application_configuration] = "railties/lib/rails/application/configuration.rb"
+      @files[:doc_path] = "guides/source/configuring.md"
+      @files[:rails_version] = "RAILS_VERSION"
+
+      @files[:new_framework_defaults] = "railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_%{version}.rb.tt" % {
+        version: rails_version.tr(".", "_")
+      }
     end
 
     def check
@@ -60,27 +88,15 @@ def check
     end
 
     def doc
-      @doc ||=
-        begin
-          content = File.read(doc_path)
-          Configuring::Doc.new(content)
-        end
-    end
-
-    def parse(relative_path)
-      parser.call(@rails_path.join(relative_path))
-    end
-
-    def read(relative_path)
-      File.read(@rails_path.join(relative_path))
+      @doc ||= Configuring::Doc.new(files.doc_path.read)
     end
 
     def rails_version
-      @rails_version ||= File.read(@rails_path.join("RAILS_VERSION")).to_f.to_s
+      @rails_version ||= files.rails_version.read.to_f.to_s
     end
 
     def write!
-      File.write(doc_path, doc.to_s)
+      files.doc_path.write(doc.to_s)
     end
 
     def error_message
@@ -90,10 +106,5 @@ def error_message
         "Make sure new configurations are added to configuring.md#rails-general-configuration in alphabetical order.\n" +
         "Errors may be autocorrectable with the --autocorrect flag"
     end
-
-    private
-      def doc_path
-        @rails_path.join(DOC_PATH)
-      end
   end
 end

tools/rail_inspector/lib/rail_inspector/configuring/check/framework_defaults.rb

@@ -10,40 +10,33 @@ class FrameworkDefaults
         class NewFrameworkDefaultsFile
           attr_reader :checker, :visitor
 
-          def initialize(checker, visitor)
+          # Defaults are strings like:
+          #   self.yjit
+          #   action_controller.escape_json_responses
+          def initialize(checker, defaults, file_content)
             @checker = checker
-            @visitor = visitor
+            @defaults = defaults
+            @file_content = file_content
           end
 
           def check
-            visitor.config_map[checker.rails_version].each_key do |config|
-              app_config = config.gsub(/^self/, "config")
+            @defaults.each do |config|
+              if config.start_with? "self"
+                next if @file_content.include? config.gsub(/^self/, "config")
+                next if @file_content.include? config.gsub(/^self/, "configuration")
+              end
 
-              next if defaults_file_content.include? app_config
+              next if @file_content.include? config
 
               next if config == "self.yjit"
 
-              add_error(config)
-            end
-          end
-
-          private
-            def add_error(config)
               checker.errors << <<~MESSAGE
-              #{new_framework_defaults_path}
-              Missing: #{config}
+                #{checker.files.new_framework_defaults}: Missing new default
+                #{config}
 
               MESSAGE
             end
-
-            def defaults_file_content
-              @defaults_file_content ||= checker.read(new_framework_defaults_path)
-            end
-
-            def new_framework_defaults_path
-              NEW_FRAMEWORK_DEFAULTS_PATH %
-                { version: checker.rails_version.tr(".", "_") }
-            end
+          end
         end
 
         attr_reader :checker
@@ -55,7 +48,11 @@ def initialize(checker)
         def check
           header, *defaults_by_version = documented_defaults
 
-          NewFrameworkDefaultsFile.new(checker, visitor).check
+          NewFrameworkDefaultsFile.new(
+            checker,
+            visitor.config_map[checker.rails_version].keys,
+            checker.files.new_framework_defaults.read
+          ).check
 
           checker.doc.versioned_defaults =
             header +
@@ -66,7 +63,7 @@ def check
 
         private
           def app_config_tree
-            checker.parse(APPLICATION_CONFIGURATION_PATH)
+            checker.files.application_configuration.parse
           end
 
           def check_defaults(defaults)
@@ -106,7 +103,7 @@ def check_defaults(defaults)
               end
 
             checker.errors << <<~MESSAGE unless config_diff.empty?
-              #{APPLICATION_CONFIGURATION_PATH}: Incorrect load_defaults docs
+              #{checker.files.application_configuration}: Incorrect load_defaults docs
               --- Expected
               +++ Actual
               #{config_diff.split("\n")[5..].join("\n")}

tools/rail_inspector/lib/rail_inspector/configuring/check/general_configuration.rb

@@ -21,7 +21,7 @@ def call
             APP_CONFIG_CONST = "Rails::Application::Configuration"
 
             def app_config_tree
-              @checker.parse(APPLICATION_CONFIGURATION_PATH)
+              @checker.files.application_configuration.parse
             end
         end
 

tools/rail_inspector/test/rail_inspector/configuring/check/framework_defaults_test.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "rail_inspector/configuring"
+
+class TestFrameworkDefaults < ActiveSupport::TestCase
+  def test_identifies_self_when_file_uses_config
+    defaults = ["self.log_file_size"]
+
+    check(defaults, <<~FILE).check
+    ###
+    # You must apply this in config/application.rb
+    # config.log_file_size = 100 * 1024 * 1024
+    FILE
+
+    assert_empty checker.errors
+  end
+
+  def test_identifies_self_when_file_uses_configuration
+    defaults = ["self.log_file_size"]
+
+    check(defaults, <<~FILE).check
+    ###
+    # You must apply this in config/application.rb
+    # Rails.configuration.log_file_size = 100 * 1024 * 1024
+    FILE
+
+    assert_empty checker.errors
+  end
+
+  private
+    def check(defaults, file_content)
+      @check ||= RailInspector::Configuring::Check::FrameworkDefaults::NewFrameworkDefaultsFile.new(checker, defaults, file_content)
+    end
+
+    def checker
+      @checker ||= RailInspector::Configuring.new("../..")
+    end
+end