Rubocop Implementation (#838)

* Rubocop Implemented

* Update spec_helper.rb

* Update spec_helper.rb

Author: david22swan

.rubocop.yml

@@ -18,3 +18,5 @@ spec/spec_helper.rb:
     - rvm: 2.1.9
       env: PUPPET_GEM_VERSION="~> 4.7.0"
       bundler_args: --without system_tests
+  - rvm: 2.1.9
+    script: bundle exec rake rubocop

.rubocop_todo.yml

@@ -28,5 +28,7 @@ matrix:
   - rvm: 2.1.9
     bundler_args: --without system_tests
     env: PUPPET_GEM_VERSION="~> 4.0"
+  - rvm: 2.1.9
+    script: bundle exec rake rubocop
 notifications:
   email: false

.sync.yml

@@ -11,19 +11,18 @@
 # The cache is stored in $libdir/facts_dot_d.cache as a mode
 # 600 file and will have the end result of not calling your
 # fact scripts more often than is needed
-
 class Facter::Util::DotD
   require 'yaml'
 
-  def initialize(dir="/etc/facts.d", cache_file=File.join(Puppet[:libdir], "facts_dot_d.cache"))
+  def initialize(dir = '/etc/facts.d', cache_file = File.join(Puppet[:libdir], 'facts_dot_d.cache'))
     @dir = dir
     @cache_file = cache_file
     @cache = nil
-    @types = {".txt" => :txt, ".json" => :json, ".yaml" => :yaml}
+    @types = { '.txt' => :txt, '.json' => :json, '.yaml' => :yaml }
   end
 
   def entries
-    Dir.entries(@dir).reject { |f| f =~ /^\.|\.ttl$/ }.sort.map { |f| File.join(@dir, f) }
+    Dir.entries(@dir).reject { |f| f =~ %r{^\.|\.ttl$} }.sort.map { |f| File.join(@dir, f) }
   rescue
     []
   end
@@ -35,17 +34,17 @@ def fact_type(file)
 
     type = :script if type == :unknown && File.executable?(file)
 
-    return type
+    type
   end
 
   def txt_parser(file)
     File.readlines(file).each do |line|
-      if line =~ /^([^=]+)=(.+)$/
-        var = $1; val = $2
+      next unless line =~ %r{^([^=]+)=(.+)$}
+      var = Regexp.last_match(1)
+      val = Regexp.last_match(2)
 
-        Facter.add(var) do
-          setcode { val }
-        end
+      Facter.add(var) do
+        setcode { val }
       end
     end
   rescue StandardError => e
@@ -60,7 +59,7 @@ def json_parser(file)
       raise
     end
 
-    JSON.load(File.read(file)).each_pair do |f, v|
+    JSON.parse(File.read(file)).each_pair do |f, v|
       Facter.add(f) do
         setcode { v }
       end
@@ -85,25 +84,25 @@ def script_parser(file)
     result = cache_lookup(file)
     ttl = cache_time(file)
 
-    unless result
+    if result
+      Facter.debug("Using cached data for #{file}")
+    else
       result = Facter::Util::Resolution.exec(file)
 
       if ttl > 0
         Facter.debug("Updating cache for #{file}")
         cache_store(file, result)
         cache_save!
       end
-    else
-      Facter.debug("Using cached data for #{file}")
     end
 
     result.split("\n").each do |line|
-      if line =~ /^(.+)=(.+)$/
-        var = $1; val = $2
+      next unless line =~ %r{^(.+)=(.+)$}
+      var = Regexp.last_match(1)
+      val = Regexp.last_match(2)
 
-        Facter.add(var) do
-          setcode { val }
-        end
+      Facter.add(var) do
+        setcode { val }
       end
     end
   rescue StandardError => e
@@ -113,15 +112,15 @@ def script_parser(file)
 
   def cache_save!
     cache = load_cache
-    File.open(@cache_file, "w", 0600) { |f| f.write(YAML.dump(cache)) }
-  rescue
+    File.open(@cache_file, 'w', 0o600) { |f| f.write(YAML.dump(cache)) }
+  rescue # rubocop:disable Lint/HandleExceptions - Is meant to be suppressed
   end
 
   def cache_store(file, data)
     load_cache
 
-    @cache[file] = {:data => data, :stored => Time.now.to_i}
-  rescue
+    @cache[file] = { data: data, stored: Time.now.to_i }
+  rescue # rubocop:disable Lint/HandleExceptions - Is meant to be suppressed
   end
 
   def cache_lookup(file)
@@ -131,21 +130,18 @@ def cache_lookup(file)
 
     ttl = cache_time(file)
 
-    if cache[file]
-      now = Time.now.to_i
+    return nil unless cache[file]
+    now = Time.now.to_i
 
-      return cache[file][:data] if ttl == -1
-      return cache[file][:data] if (now - cache[file][:stored]) <= ttl
-      return nil
-    else
-      return nil
-    end
+    return cache[file][:data] if ttl == -1
+    return cache[file][:data] if (now - cache[file][:stored]) <= ttl
+    return nil
   rescue
     return nil
   end
 
   def cache_time(file)
-    meta = file + ".ttl"
+    meta = file + '.ttl'
 
     return File.read(meta).chomp.to_i
   rescue
@@ -154,11 +150,11 @@ def cache_time(file)
 
   def load_cache
     unless @cache
-      if File.exist?(@cache_file)
-        @cache = YAML.load_file(@cache_file)
-      else
-        @cache = {}
-      end
+      @cache = if File.exist?(@cache_file)
+                 YAML.load_file(@cache_file)
+               else
+                 {}
+               end
     end
 
     return @cache
@@ -172,28 +168,26 @@ def create
       type = fact_type(fact)
       parser = "#{type}_parser"
 
-      if respond_to?("#{type}_parser")
-        Facter.debug("Parsing #{fact} using #{parser}")
+      next unless respond_to?("#{type}_parser")
+      Facter.debug("Parsing #{fact} using #{parser}")
 
-        send(parser, fact)
-      end
+      send(parser, fact)
     end
   end
 end
 
-
-mdata = Facter.version.match(/(\d+)\.(\d+)\.(\d+)/)
+mdata = Facter.version.match(%r{(\d+)\.(\d+)\.(\d+)})
 if mdata
-  (major, minor, patch) = mdata.captures.map { |v| v.to_i }
+  (major, minor, _patch) = mdata.captures.map { |v| v.to_i }
   if major < 2
     # Facter 1.7 introduced external facts support directly
-    unless major == 1 and minor > 6
-      Facter::Util::DotD.new("/etc/facter/facts.d").create
-      Facter::Util::DotD.new("/etc/puppetlabs/facter/facts.d").create
+    unless major == 1 && minor > 6
+      Facter::Util::DotD.new('/etc/facter/facts.d').create
+      Facter::Util::DotD.new('/etc/puppetlabs/facter/facts.d').create
 
       # Windows has a different configuration directory that defaults to a vendor
       # specific sub directory of the %COMMON_APPDATA% directory.
-      if Dir.const_defined? 'COMMON_APPDATA' then
+      if Dir.const_defined? 'COMMON_APPDATA' # rubocop:disable Metrics/BlockNesting : Any attempt to alter this breaks it
         windows_facts_dot_d = File.join(Dir::COMMON_APPDATA, 'PuppetLabs', 'facter', 'facts.d')
         Facter::Util::DotD.new(windows_facts_dot_d).create
       end

.travis.yml

@@ -12,10 +12,10 @@
 
 Facter.add(:package_provider) do
   setcode do
-    if defined? Gem and Gem::Version.new(Facter.value(:puppetversion).split(' ')[0]) >= Gem::Version.new('3.6')
-      Puppet::Type.type(:package).newpackage(:name => 'dummy', :allow_virtual => 'true')[:provider].to_s
+    if defined? Gem && Gem::Version.new(Facter.value(:puppetversion).split(' ')[0]) >= Gem::Version.new('3.6')
+      Puppet::Type.type(:package).newpackage(name: 'dummy', allow_virtual: 'true')[:provider].to_s
     else
-      Puppet::Type.type(:package).newpackage(:name => 'dummy')[:provider].to_s
+      Puppet::Type.type(:package).newpackage(name: 'dummy')[:provider].to_s
     end
   end
 end

lib/facter/facter_dot_d.rb

@@ -8,50 +8,53 @@
 #
 # Caveats:
 #
-Facter.add("pe_version") do
+Facter.add('pe_version') do
   setcode do
-    puppet_ver = Facter.value("puppetversion")
-    if puppet_ver != nil
-      pe_ver = puppet_ver.match(/Puppet Enterprise (\d+\.\d+\.\d+)/)
+    puppet_ver = Facter.value('puppetversion')
+    if !puppet_ver.nil?
+      pe_ver = puppet_ver.match(%r{Puppet Enterprise (\d+\.\d+\.\d+)})
       pe_ver[1] if pe_ver
     else
       nil
     end
   end
 end
 
-Facter.add("is_pe") do
+Facter.add('is_pe') do
   setcode do
-    if Facter.value(:pe_version).to_s.empty? then
+    if Facter.value(:pe_version).to_s.empty?
       false
     else
       true
     end
   end
 end
 
-Facter.add("pe_major_version") do
-  confine :is_pe => true
+Facter.add('pe_major_version') do
+  confine is_pe: true
   setcode do
-    if pe_version = Facter.value(:pe_version)
+    pe_version = Facter.value(:pe_version)
+    if pe_version
       pe_version.to_s.split('.')[0]
     end
   end
 end
 
-Facter.add("pe_minor_version") do
-  confine :is_pe => true
+Facter.add('pe_minor_version') do
+  confine is_pe: true
   setcode do
-    if pe_version = Facter.value(:pe_version)
+    pe_version = Facter.value(:pe_version)
+    if pe_version
       pe_version.to_s.split('.')[1]
     end
   end
 end
 
-Facter.add("pe_patch_version") do
-  confine :is_pe => true
+Facter.add('pe_patch_version') do
+  confine is_pe: true
   setcode do
-    if pe_version = Facter.value(:pe_version)
+    pe_version = Facter.value(:pe_version)
+    if pe_version
       pe_version.to_s.split('.')[2]
     end
   end

lib/facter/package_provider.rb

@@ -13,10 +13,9 @@
   # #4248). It should (in the future) but for the time being we need to be
   # defensive which is what this rescue block is doing.
   rb_file = File.join(File.dirname(__FILE__), 'util', 'puppet_settings.rb')
-  load rb_file if File.exists?(rb_file) or raise e
+  load rb_file if File.exist?(rb_file) || raise(e)
 end
 
-
 # These will be nil if Puppet is not available.
 Facter.add(:puppet_vardir) do
   setcode do

lib/facter/pe_version.rb

@@ -1,44 +1,43 @@
 # A facter fact to determine the root home directory.
 # This varies on PE supported platforms and may be
 # reconfigured by the end user.
-
 module Facter::Util::RootHome
   class << self
-  def get_root_home
-    root_ent = Facter::Util::Resolution.exec("getent passwd root")
+  def returnt_root_home
+    root_ent = Facter::Util::Resolution.exec('getent passwd root')
     # The home directory is the sixth element in the passwd entry
     # If the platform doesn't have getent, root_ent will be nil and we should
     # return it straight away.
-    root_ent && root_ent.split(":")[5]
+    root_ent && root_ent.split(':')[5]
   end
   end
 end
 
 Facter.add(:root_home) do
-  setcode { Facter::Util::RootHome.get_root_home }
+  setcode { Facter::Util::RootHome.returnt_root_home }
 end
 
 Facter.add(:root_home) do
-  confine :kernel => :darwin
+  confine kernel: :darwin
   setcode do
-    str = Facter::Util::Resolution.exec("dscacheutil -q user -a name root")
+    str = Facter::Util::Resolution.exec('dscacheutil -q user -a name root')
     hash = {}
     str.split("\n").each do |pair|
-      key,value = pair.split(/:/)
+      key, value = pair.split(%r{:})
       hash[key] = value
     end
     hash['dir'].strip
   end
 end
 
 Facter.add(:root_home) do
-  confine :kernel => :aix
+  confine kernel: :aix
   root_home = nil
   setcode do
-    str = Facter::Util::Resolution.exec("lsuser -c -a home root")
+    str = Facter::Util::Resolution.exec('lsuser -c -a home root')
     str && str.split("\n").each do |line|
-      next if line =~ /^#/
-      root_home = line.split(/:/)[1]
+      next if line =~ %r{^#}
+      root_home = line.split(%r{:})[1]
     end
     root_home
   end

lib/facter/puppet_settings.rb

@@ -12,6 +12,6 @@
 
 Facter.add(:service_provider) do
   setcode do
-    Puppet::Type.type(:service).newservice(:name => 'dummy')[:provider].to_s
+    Puppet::Type.type(:service).newservice(name: 'dummy')[:provider].to_s
   end
 end