(PDK-1523) Use PDK::Util::Filesystem.read_file instead of File.read

.rubocop.yml

@@ -196,3 +196,10 @@ PDK/FileUtilsRm:
   Exclude:
     - lib/pdk/util/filesystem.rb
     - spec/acceptance/**/*.rb
+
+PDK/FileRead:
+  Exclude:
+    - lib/pdk/util/filesystem.rb
+    - spec/acceptance/**/*.rb
+    - Gemfile
+    - spec/unit/pdk/util/filesystem_spec.rb

ext/rubocop.rb

@@ -301,6 +301,44 @@ def autocorrect(node)
           end
         end
       end
+
+      class FileRead < Cop
+        MSG = 'Use PDK::Util::Filesystem.read_file instead of File.read'.freeze
+
+        def_node_matcher :file_read?,
+                         '(send (const nil? :File) :read ...)'
+
+        def_node_matcher :allow_file?, <<-MATCHER
+          (send
+            (send nil? {:allow :expect} (const nil? :File))
+            {:to :not_to}
+            ...)
+        MATCHER
+
+        def_node_search :receive_read?, '(send nil? :receive (sym :read))'
+
+        def on_send(node)
+          return unless file_read?(node) || (allow_file?(node) && receive_read?(node))
+
+          add_offense(node)
+        end
+
+        def autocorrect(node)
+          ->(corrector) do
+            if file_read?(node)
+              const = node.children[0].loc.expression
+              method = node.loc.selector
+              new_method = 'read_file'
+            else
+              const = node.children[0].children[2].loc.expression
+              method = node.children[2].receiver.node_parts[0].first_argument.loc.expression
+              new_method = ':read_file'
+            end
+            corrector.replace(const, 'PDK::Util::Filesystem')
+            corrector.replace(method, new_method)
+          end
+        end
+      end
     end
   end
 end

lib/pdk/answer_file.rb

@@ -72,7 +72,7 @@ def read_from_disk
         }
       end
 
-      answers = JSON.parse(File.read(answer_file_path))
+      answers = JSON.parse(PDK::Util::Filesystem.read_file(answer_file_path))
       if answers.is_a?(Hash)
         answers
       else

lib/pdk/module/templatedir.rb

@@ -315,7 +315,7 @@ def read_config(loc)
           require 'yaml'
 
           begin
-            YAML.safe_load(File.read(loc), [], [], true)
+            YAML.safe_load(PDK::Util::Filesystem.read_file(loc), [], [], true)
           rescue Psych::SyntaxError => e
             PDK.logger.warn _("'%{file}' is not a valid YAML file: %{problem} %{context} at line %{line} column %{column}") % {
               file:    loc,

lib/pdk/report/event.rb

@@ -352,7 +352,7 @@ def context_lines(max_num_lines = 5)
 
         return if file_path.nil?
 
-        file_content = File.read(file_path).split("\n")
+        file_content = PDK::Util::Filesystem.read_file(file_path).split("\n")
         delta = (max_num_lines - 1) / 2
         min = [0, (line - 1) - delta].max
         max = [(line - 1) + delta, file_content.length].min

lib/pdk/template_file.rb

@@ -63,7 +63,7 @@ def config_for(path)
     # @api private
     def template_content
       if PDK::Util::Filesystem.file?(@template_file) && PDK::Util::Filesystem.readable?(@template_file)
-        return File.read(@template_file)
+        return PDK::Util::Filesystem.read_file(@template_file)
       end
 
       raise ArgumentError, _("'%{template}' is not a readable file") % { template: @template_file }

lib/pdk/util/version.rb

@@ -16,7 +16,7 @@ def self.pdk_ref
 
       def self.pkg_sha
         if version_file && PDK::Util::Filesystem.exist?(version_file)
-          ver = File.read(version_file)
+          ver = PDK::Util::Filesystem.read_file(version_file)
           sha = ver.strip.split('.')[5] unless ver.nil?
         end
 

lib/pdk/validate/metadata/metadata_syntax.rb

@@ -75,7 +75,7 @@ def self.invoke(report, options = {})
           end
 
           begin
-            JSON.parse(File.read(target))
+            JSON.parse(PDK::Util::Filesystem.read_file(target))
 
             report.add_event(
               file:     target,

lib/pdk/validate/tasks/metadata_lint.rb

@@ -91,7 +91,7 @@ def self.invoke(report, options = {})
               JSON.generator = JSON::Ext::Generator
 
               begin
-                errors = JSON::Validator.fully_validate(schema_file, File.read(target)) || []
+                errors = JSON::Validator.fully_validate(schema_file, PDK::Util::Filesystem.read_file(target)) || []
               rescue JSON::Schema::SchemaError => e
                 raise PDK::CLI::FatalError, _('Unable to validate Task Metadata. %{error}.') % { error: e.message }
               end

lib/pdk/validate/yaml/syntax.rb

@@ -80,7 +80,7 @@ def self.invoke(report, options = {})
             end
 
             begin
-              ::YAML.safe_load(File.read(target), YAML_WHITELISTED_CLASSES, [], true)
+              ::YAML.safe_load(PDK::Util::Filesystem.read_file(target), YAML_WHITELISTED_CLASSES, [], true)
 
               report.add_event(
                 file:     target,

spec/unit/pdk/answer_file_spec.rb

@@ -10,7 +10,7 @@
     allow(PDK::Util::Filesystem).to receive(:file?).with(default_path).and_return(true)
     allow(File).to receive(:zero?).with(default_path).and_return(false)
     allow(PDK::Util::Filesystem).to receive(:readable?).with(default_path).and_return(true)
-    allow(File).to receive(:read).with(default_path).and_return('{"question": "answer"}')
+    allow(PDK::Util::Filesystem).to receive(:read_file).with(default_path).and_return('{"question": "answer"}')
   end
 end
 
@@ -78,7 +78,7 @@
         before(:each) do
           allow(File).to receive(:zero?).with(default_path).and_return(false)
           allow(PDK::Util::Filesystem).to receive(:readable?).with(default_path).and_return(true)
-          allow(File).to receive(:read).with(default_path).and_return(file_contents)
+          allow(PDK::Util::Filesystem).to receive(:read_file).with(default_path).and_return(file_contents)
         end
 
         context 'but contains invalid JSON' do

spec/unit/pdk/generate/puppet_object_spec.rb

@@ -8,7 +8,7 @@
   before(:each) do
     allow(PDK::Util::Filesystem).to receive(:file?).with(metadata_path).and_return(true)
     allow(PDK::Util::Filesystem).to receive(:readable?).with(metadata_path).and_return(true)
-    allow(File).to receive(:read).with(metadata_path).and_return(module_metadata)
+    allow(PDK::Util::Filesystem).to receive(:read_file).with(metadata_path).and_return(module_metadata)
   end
 end
 

spec/unit/pdk/module/template_dir_spec.rb

@@ -215,7 +215,7 @@
       allow(PDK::Util).to receive(:make_tmpdir_name).with('pdk-templates').and_return(tmp_path)
       allow(PDK::CLI::Exec).to receive(:git).with('clone', path_or_url, tmp_path).and_return(exit_code: 0)
       allow(PDK::Util::Filesystem).to receive(:file?).with(anything).and_return(File.join(path_or_url, 'config_defaults.yml')).and_return(true)
-      allow(File).to receive(:read).with(File.join(path_or_url, 'config_defaults.yml')).and_return(config_defaults)
+      allow(PDK::Util::Filesystem).to receive(:read_file).with(File.join(path_or_url, 'config_defaults.yml')).and_return(config_defaults)
       allow(Dir).to receive(:rmdir).with(tmp_path).and_return(0)
 
       allow(described_class).to receive(:new).with(uri, module_metadata).and_yield(template_dir)
@@ -379,7 +379,7 @@
       allow(PDK::Util).to receive(:make_tmpdir_name).with('pdk-templates').and_return(tmp_path)
       allow(PDK::CLI::Exec).to receive(:git).with('clone', path_or_url, tmp_path).and_return(exit_code: 0)
       allow(PDK::Util::Filesystem).to receive(:file?).with(anything).and_return(File.join(path_or_url, 'config_defaults.yml')).and_return(true)
-      allow(File).to receive(:read).with(File.join(path_or_url, 'config_defaults.yml')).and_return(config_defaults)
+      allow(PDK::Util::Filesystem).to receive(:read_file).with(File.join(path_or_url, 'config_defaults.yml')).and_return(config_defaults)
       allow(PDK::Util::Filesystem).to receive(:readable?).with(File.join(path_or_url, 'config_defaults.yml')).and_return(true)
       allow(YAML).to receive(:safe_load).with(config_defaults, [], [], true).and_return config_hash
     end
@@ -412,7 +412,7 @@
       before(:each) do
         allow(PDK::Util::Filesystem).to receive(:file?).with('/path/to/module/.sync.yml').and_return(true)
         allow(PDK::Util::Filesystem).to receive(:readable?).with('/path/to/module/.sync.yml').and_return(true)
-        allow(File).to receive(:read).with('/path/to/module/.sync.yml').and_return(yaml_text)
+        allow(PDK::Util::Filesystem).to receive(:read_file).with('/path/to/module/.sync.yml').and_return(yaml_text)
         allow(YAML).to receive(:safe_load).with(yaml_text, [], [], true).and_return(yaml_hash)
         allow(PDK::Util).to receive(:module_root).and_return('/path/to/module')
       end
@@ -492,7 +492,7 @@
       before(:each) do
         allow(PDK::Util::Filesystem).to receive(:file?).with('/path/to/module/.sync.yml').and_return true
         allow(PDK::Util::Filesystem).to receive(:readable?).with('/path/to/module/.sync.yml').and_return true
-        allow(File).to receive(:read).with('/path/to/module/.sync.yml').and_return yaml_text
+        allow(PDK::Util::Filesystem).to receive(:read_file).with('/path/to/module/.sync.yml').and_return yaml_text
         allow(YAML).to receive(:safe_load).with(yaml_text, [], [], true).and_call_original
         allow(PDK::Util).to receive(:module_root).and_return('/path/to/module')
       end

spec/unit/pdk/module/update_manager_spec.rb

@@ -151,7 +151,7 @@
 
     before(:each) do
       allow(PDK::Util::Filesystem).to receive(:readable?).with(dummy_file).and_return(true)
-      allow(File).to receive(:read).with(dummy_file).and_return(original_content)
+      allow(PDK::Util::Filesystem).to receive(:read_file).with(dummy_file).and_return(original_content)
       allow(File).to receive(:stat).with(dummy_file).and_return(instance_double(File::Stat, mtime: Time.now - 60))
     end
 

spec/unit/pdk/report/event_spec.rb

@@ -424,7 +424,7 @@
         end
 
         before(:each) do
-          allow(File).to receive(:read).with('testfile.rb').and_return(file_content)
+          allow(PDK::Util::Filesystem).to receive(:read_file).with('testfile.rb').and_return(file_content)
           allow(PDK::Util::Filesystem).to receive(:file?).with('testfile.rb').and_return(true)
           allow(PDK::Util).to receive(:module_root).and_return(File.join('my', 'module_root'))
         end

spec/unit/pdk/template_file_spec.rb

@@ -45,20 +45,20 @@
         let(:template_path) { '/path/to/some/file.erb' }
 
         it 'renders the contents of the file as an ERB template' do
-          expect(File).to receive(:read).with(template_path).and_return('<%= some %>')
+          expect(PDK::Util::Filesystem).to receive(:read_file).with(template_path).and_return('<%= some %>')
           expect(template_file.render).to eq(data[:some])
         end
 
         # modulesync compatibility
         it 'exposes any data provided as :configs to the template as an instance variable' do
-          expect(File).to receive(:read).with(template_path).and_return("<%= @configs['test'] %>")
+          expect(PDK::Util::Filesystem).to receive(:read_file).with(template_path).and_return("<%= @configs['test'] %>")
           expect(template_file.render).to eq(data[:configs]['test'])
         end
       end
 
       context 'and does not have an .erb extension' do
         it 'renders the contents of the file as a plain file' do
-          expect(File).to receive(:read).with(template_path).and_return('some content')
+          expect(PDK::Util::Filesystem).to receive(:read_file).with(template_path).and_return('some content')
           expect(template_file.render).to eq('some content')
         end
       end

spec/unit/pdk/util/ruby_version_spec.rb

@@ -179,9 +179,11 @@
 
       gem_path_results.merge(gem_home_results).each do |spec_path, spec_content|
         # rubocop:disable PDK/FileFilePredicate Gem internal method
+        # rubocop:disable PDK/FileRead Gem internal method
         allow(File).to receive(:file?).with(spec_path).and_return(true)
         allow(File).to receive(:read).with(spec_path, mode: 'r:UTF-8:-').and_return(spec_content)
         # rubocop:enable PDK/FileFilePredicate
+        # rubocop:enable PDK/FileRead
       end
     end
 

spec/unit/pdk/util/version_spec.rb

@@ -34,7 +34,7 @@
     before(:each) do
       allow(PDK::Util).to receive(:find_upwards).and_return('/tmp/package/PDK_VERSION')
       allow(PDK::Util::Filesystem).to receive(:exist?).with('/tmp/package/PDK_VERSION').and_return(true)
-      allow(File).to receive(:read).with('/tmp/package/PDK_VERSION').and_return('0.1.2.3.4.pkg_hash')
+      allow(PDK::Util::Filesystem).to receive(:read_file).with('/tmp/package/PDK_VERSION').and_return('0.1.2.3.4.pkg_hash')
       allow(PDK::Util::Filesystem).to receive(:directory?).with(%r{.git\Z}).and_return(false)
       allow(PDK::CLI::Exec).to receive(:git).never
     end

spec/unit/pdk/validate/metadata/metadata_syntax_spec.rb

@@ -22,7 +22,7 @@
         allow(PDK::Util::Filesystem).to receive(:directory?).with(target[:name]).and_return(target.fetch(:directory, false))
         allow(PDK::Util::Filesystem).to receive(:file?).with(target[:name]).and_return(target.fetch(:file, true))
         allow(PDK::Util::Filesystem).to receive(:readable?).with(target[:name]).and_return(target.fetch(:readable, true))
-        allow(File).to receive(:read).with(target[:name]).and_return(target.fetch(:content, ''))
+        allow(PDK::Util::Filesystem).to receive(:read_file).with(target[:name]).and_return(target.fetch(:content, ''))
       end
     end
 

spec/unit/pdk/validate/tasks/metadata_lint_spec.rb

@@ -69,12 +69,11 @@
 
     before(:each) do
       allow(described_class).to receive(:schema_file).and_return(schema)
-      allow(File).to receive(:read).and_call_original
       targets.each do |target|
         allow(PDK::Util::Filesystem).to receive(:directory?).with(target[:name]).and_return(target.fetch(:directory, false))
         allow(PDK::Util::Filesystem).to receive(:file?).with(target[:name]).and_return(target.fetch(:file, true))
         allow(PDK::Util::Filesystem).to receive(:readable?).with(target[:name]).and_return(target.fetch(:readable, true))
-        allow(File).to receive(:read).with(target[:name]).and_return(target.fetch(:content, ''))
+        allow(PDK::Util::Filesystem).to receive(:read_file).with(target[:name]).and_return(target.fetch(:content, ''))
       end
     end
 

spec/unit/pdk/validate/yaml/syntax_spec.rb

@@ -26,7 +26,7 @@
         allow(PDK::Util::Filesystem).to receive(:directory?).with(target[:name]).and_return(target.fetch(:directory, false))
         allow(PDK::Util::Filesystem).to receive(:file?).with(target[:name]).and_return(target.fetch(:file, true))
         allow(PDK::Util::Filesystem).to receive(:readable?).with(target[:name]).and_return(target.fetch(:readable, true))
-        allow(File).to receive(:read).with(target[:name]).and_return(target.fetch(:content, ''))
+        allow(PDK::Util::Filesystem).to receive(:read_file).with(target[:name]).and_return(target.fetch(:content, ''))
       end
     end