optimize search file content

* read file by line until match * only read files once per check * prefer include over match replacing readlines.grep happens to fix #63
voxpupuli · Jan 29, 2024 · de8d879 · de8d879
1 parent ad5edd4
commit de8d879
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 29 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -90,6 +90,7 @@ Style/ClassAndModuleChildren:
 Style/ClassVars:
   Exclude:
     - 'lib/puppet-ghostbuster/puppetdb.rb'
+    - 'lib/puppet-ghostbuster/util.rb'
 
 # Offense count: 9
 # Configuration parameters: AllowedConstants.
@@ -98,6 +99,7 @@ Style/Documentation:
     - 'spec/**/*'
     - 'test/**/*'
     - 'lib/puppet-ghostbuster/puppetdb.rb'
+    - 'lib/puppet-ghostbuster/util.rb'
     - 'lib/puppet-lint/plugins/check_ghostbuster_classes.rb'
     - 'lib/puppet-lint/plugins/check_ghostbuster_defines.rb'
     - 'lib/puppet-lint/plugins/check_ghostbuster_facts.rb'
@@ -114,6 +116,7 @@ Style/Documentation:
 Style/FrozenStringLiteralComment:
   Exclude:
     - 'lib/puppet-ghostbuster/puppetdb.rb'
+    - 'lib/puppet-ghostbuster/util.rb'
     - 'lib/puppet-ghostbuster/version.rb'
     - 'lib/puppet-lint/plugins/check_ghostbuster_classes.rb'
     - 'lib/puppet-lint/plugins/check_ghostbuster_defines.rb'
@@ -167,4 +170,4 @@ Style/ZeroLengthPredicate:
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
 # URISchemes: http, https
 Layout/LineLength:
-  Max: 189
+  Max: 195
diff --git a/lib/puppet-ghostbuster/util.rb b/lib/puppet-ghostbuster/util.rb
@@ -0,0 +1,19 @@
+class PuppetGhostbuster
+  class Util
+    class << self
+      def search_file(name, search)
+        return search_file_regexp(name, search) if search.is_a?(Regexp)
+
+        File.foreach(name) do |line|
+          return true if line.include?(search)
+        end
+      end
+
+      def search_file_regexp(name, search)
+        File.foreach(name) do |line|
+          return true if line.match?(search)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/puppet-lint/plugins/check_ghostbuster_facts.rb b/lib/puppet-lint/plugins/check_ghostbuster_facts.rb
@@ -1,3 +1,5 @@
+require 'puppet-ghostbuster/util'
+
 class PuppetLint::Checks
   def load_data(path, content)
     lexer = PuppetLint::Lexer.new
@@ -25,29 +27,30 @@ def check
     m = path.match(%r{.*/([^/]+)/lib/facter/(.+)$})
     return if m.nil?
 
-    File.readlines(path).grep(/Facter.add\(["']([^"']+)["']\)/).each do |line|
-      fact_name = line.match(/Facter.add\(["']([^"']+)["']\)/).captures[0]
+    File.foreach(path) do |line|
+      if line.match?(/Facter.add\(["']([^"']+)["']\)/)
+        fact_name = line.match(/Facter.add\(["']([^"']+)["']\)/).captures[0]
 
-      found = false
+        found = false
 
-      manifests.each do |manifest|
-        found = true if File.readlines(manifest).grep(/\$\{?::#{fact_name}\}?/).size > 0
-        found = true if File.readlines(manifest).grep(/@#{fact_name}/).size > 0
-        break if found
-      end
+        manifests.each do |manifest|
+          found = true unless PuppetGhostbuster::Util.search_file(manifest, /(\$\{?::#{fact_name}\}?|@#{fact_name})/).nil?
+          break if found
+        end
 
-      templates.each do |template|
-        found = true if File.readlines(template).grep(/@#{fact_name}/).size > 0
-        break if found
-      end
+        templates.each do |template|
+          found = true unless PuppetGhostbuster::Util.search_file(template, /@#{fact_name}/).nil?
+          break if found
+        end
 
-      next if found
+        next if found
 
-      notify :warning, {
-        message: "Fact #{fact_name} seems unused",
-        line: 1,
-        column: 1,
-      }
+        notify :warning, {
+          message: "Fact #{fact_name} seems unused",
+          line: 1,
+          column: 1,
+        }
+      end
     end
   end
 end
diff --git a/lib/puppet-lint/plugins/check_ghostbuster_files.rb b/lib/puppet-lint/plugins/check_ghostbuster_files.rb
@@ -1,4 +1,5 @@
 require 'puppet-ghostbuster/puppetdb'
+require 'puppet-ghostbuster/util'
 
 class PuppetLint::Checks
   def load_data(path, content)
@@ -42,9 +43,9 @@ def check
     end
 
     manifests.each do |manifest|
-      return if File.readlines(manifest).grep(%r{["']#{module_name}/#{file_name}["']}).size > 0
+      return if PuppetGhostbuster::Util.search_file(manifest, %r{["']#{module_name}/#{file_name}["']})
 
-      if (match = manifest.match(%r{.*/([^/]+)/manifests/.+$})) && (match.captures[0] == module_name) && (File.readlines(manifest).grep(%r{["']\$\{module_name\}/#{file_name}["']}).size > 0)
+      if (match = manifest.match(%r{.*/([^/]+)/manifests/.+$})) && (match.captures[0] == module_name) && PuppetGhostbuster::Util.search_file(manifest, %r{["']\$\{module_name\}/#{file_name}["']})
         return
       end
     end

diff --git a/lib/puppet-lint/plugins/check_ghostbuster_functions.rb b/lib/puppet-lint/plugins/check_ghostbuster_functions.rb
@@ -1,3 +1,5 @@
+require 'puppet-ghostbuster/util'
+
 class PuppetLint::Checks
   def load_data(path, content)
     lexer = PuppetLint::Lexer.new
@@ -28,12 +30,11 @@ def check
     function_name = m.captures[0]
 
     manifests.each do |manifest|
-      return if File.readlines(manifest).grep(/#{function_name}\(/).size > 0
+      return if PuppetGhostbuster::Util.search_file(manifest, "#{function_name}(")
     end
 
     templates.each do |template|
-      return if File.readlines(template).grep(/scope.function_#{function_name}\(/).size > 0
-      return if File.readlines(template).grep(/Puppet::Parser::Functions.function\(:#{function_name}/).size > 0
+      return if PuppetGhostbuster::Util.search_file(template, /(Puppet::Parser::Functions\.function\(:|scope\.function_)#{function_name}/)
     end
 
     notify :warning, {

diff --git a/lib/puppet-lint/plugins/check_ghostbuster_templates.rb b/lib/puppet-lint/plugins/check_ghostbuster_templates.rb
@@ -1,3 +1,5 @@
+require 'puppet-ghostbuster/util'
+
 class PuppetLint::Checks
   def load_data(path, content)
     lexer = PuppetLint::Lexer.new
@@ -28,19 +30,17 @@ def check
     module_name, template_name = m.captures
 
     manifests.each do |manifest|
-      return if File.readlines(manifest).grep(%r{["']#{module_name}/#{template_name}["']}).size > 0
+      return if PuppetGhostbuster::Util.search_file(manifest, %r{["']#{module_name}/#{template_name}["']})
 
       next unless match = manifest.match(%r{.*/([^/]+)/manifests/.+$})
 
-      if match.captures[0] == module_name && (File.readlines(manifest).grep(%r{["']\$\{module_name\}/#{template_name}["']}).size > 0)
+      if match.captures[0] == module_name && PuppetGhostbuster::Util.search_file(manifest, %r{["']\$\{module_name\}/#{template_name}["']})
         return
       end
     end
 
     templates.each do |template|
-      if File.readlines(template).grep(%r{scope.function_template\(\['#{module_name}/#{template_name}'\]\)}).size > 0
-        return
-      end
+      return if PuppetGhostbuster::Util.search_file(template, "scope.function_template(['#{module_name}/#{template_name}'])")
     end
 
     notify :warning, {