diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 71c267f..31d01ee 100644 --- 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 new file mode 100644 index 0000000..d05ad7a --- /dev/null +++ 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 index 263767c..4c9545f 100644 --- 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 =~ /Facter.add\(["':](?[^"'\)]+)["']?\)/ + fact_name = Regexp.last_match(:fact) - 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 index 5e43ae7..ddc0777 100644 --- 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 index 31853f0..33ba5ca 100644 --- 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 index 8377453..71f6441 100644 --- 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, { diff --git a/spec/fixtures/modules/foo/lib/facter/asym.rb b/spec/fixtures/modules/foo/lib/facter/asym.rb new file mode 100644 index 0000000..a5e0bc4 --- /dev/null +++ b/spec/fixtures/modules/foo/lib/facter/asym.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +# Fact is a symbol +Facter.add(:asym) do + setcode do + 'asym' + end +end diff --git a/spec/puppet-lint/plugins/ghostbuster_facts_spec.rb b/spec/puppet-lint/plugins/ghostbuster_facts_spec.rb index bcb6307..1f554c0 100644 --- a/spec/puppet-lint/plugins/ghostbuster_facts_spec.rb +++ b/spec/puppet-lint/plugins/ghostbuster_facts_spec.rb @@ -53,5 +53,17 @@ expect(problems.size).to eq(0) end end + + context 'when added fact is a symbol and unused' do + let(:path) { './spec/fixtures/modules/foo/lib/facter/asym.rb' } + + it 'detects one problem' do + expect(problems.size).to eq(1) + end + + it 'creates a warning' do + expect(problems).to contain_warning('Fact asym seems unused') + end + end end end