Merge pull request #39 from alexjfisher/rubocop_auto_fixes

Rubocop auto fixes

lib/puppet/provider/posix_acl/genericacl.rb

@@ -1,3 +1,2 @@
-Puppet::Type.type(:posix_acl).provide(:genericacl, :parent => Puppet::Provider) do
-
+Puppet::Type.type(:posix_acl).provide(:genericacl, parent: Puppet::Provider) do
 end

lib/puppet/provider/posix_acl/posixacl.rb

@@ -1,19 +1,19 @@
-Puppet::Type.type(:posix_acl).provide(:posixacl, :parent => Puppet::Provider) do
-  desc "Provide posix 1e acl functions using posix getfacl/setfacl commands"
+Puppet::Type.type(:posix_acl).provide(:posixacl, parent: Puppet::Provider) do
+  desc 'Provide posix 1e acl functions using posix getfacl/setfacl commands'
 
-  commands :setfacl => '/usr/bin/setfacl'
-  commands :getfacl => '/usr/bin/getfacl'
+  commands setfacl: '/usr/bin/setfacl'
+  commands getfacl: '/usr/bin/getfacl'
 
-  confine :feature => :posix
-  defaultfor :operatingsystem => [:debian, :ubuntu, :redhat, :centos, :fedora, :sles]
+  confine feature: :posix
+  defaultfor operatingsystem: [:debian, :ubuntu, :redhat, :centos, :fedora, :sles]
 
   def exists?
     permission
   end
 
   def unset_perm(perm, path)
     # Don't try to unset mode bits, it don't make sense!
-    if !(perm =~ /^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):/)
+    unless perm =~ %r{^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):}
       perm = perm.split(':')[0..-2].join(':')
       if check_recursive
         setfacl('-R', '-n', '-x', perm, path)
@@ -48,19 +48,17 @@ def purge
   def permission
     return [] unless File.exist?(@resource.value(:path))
     value = []
-    #String#lines would be nice, but we need to support Ruby 1.8.5
+    # String#lines would be nice, but we need to support Ruby 1.8.5
     getfacl('--absolute-names', '--no-effective', @resource.value(:path)).split("\n").each do |line|
       # Strip comments and blank lines
-      if !(line =~ /^#/) and !(line == "")
-        value << line.gsub('\040', ' ')
-      end
+      value << line.gsub('\040', ' ') if line !~ %r{^#} && line != ''
     end
     value.sort
   end
 
   def check_recursive
     # Changed functionality to return boolean true or false
-    @resource.value(:recursive) == :true and resource.value(:recursemode) == :lazy
+    @resource.value(:recursive) == :true && resource.value(:recursemode) == :lazy
   end
 
   def check_exact
@@ -79,7 +77,7 @@ def check_set
     @resource.value(:action) == :set
   end
 
-  def permission=(value)
+  def permission=(_value) # TODO: Investigate why we're not using this parameter
     Puppet.debug @resource.value(:action)
     case @resource.value(:action)
     when :unset
@@ -90,15 +88,13 @@ def permission=(value)
       cur_perm = permission
       perm_to_set = @resource.value(:permission) - cur_perm
       perm_to_unset = cur_perm - @resource.value(:permission)
-      if (perm_to_set.length == 0 && perm_to_unset.length == 0)
-        return false
-      end
+      return false if perm_to_set.empty? && perm_to_unset.empty?
       # Take supplied perms literally, unset any existing perms which
       # are absent from ACLs given
       if check_exact
         perm_to_unset.each do |perm|
           # Skip base perms in unset step
-          if perm =~ /^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):/
+          if perm =~ %r{^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):}
             Puppet.debug "skipping unset of base perm: #{perm}"
           else
             unset_perm(perm, @resource.value(:path))

lib/puppet/type/posix_acl.rb

@@ -41,13 +41,13 @@
     EOT
 
   newparam(:action) do
-    desc "What do we do with this list of ACLs? Options are set, unset, exact, and purge"
+    desc 'What do we do with this list of ACLs? Options are set, unset, exact, and purge'
     newvalues(:set, :unset, :exact, :purge)
     defaultto :set
   end
 
   newparam(:path) do
-    desc "The file or directory to which the ACL applies."
+    desc 'The file or directory to which the ACL applies.'
     isnamevar
     validate do |value|
       path = Pathname.new(value)
@@ -74,25 +74,25 @@ def self.is_descendant?(a, b)
     a_list = File.expand_path(a).split('/')
     b_list = File.expand_path(b).split('/')
 
-    b_list[0..a_list.size-1] == a_list and b_list != a_list
+    b_list[0..a_list.size - 1] == a_list && b_list != a_list
   end
 
   # Snippet based on upstream Puppet (ASL 2.0)
-  [:posix_acl, :file].each do | autorequire_type |
+  [:posix_acl, :file].each do |autorequire_type|
     autorequire(autorequire_type) do
       req = []
       path = Pathname.new(self[:path])
       if autorequire_type != :posix_acl
         if self[:recursive] == :true
-          catalog.resources.find_all { |r|
-            r.is_a?(Puppet::Type.type(autorequire_type)) and self.class.is_descendant?(self[:path], r[:path])
-          }.each do | found |
+          catalog.resources.select do |r|
+            r.is_a?(Puppet::Type.type(autorequire_type)) && self.class.is_descendant?(self[:path], r[:path])
+          end.each do |found|
             req << found[:path]
           end
         end
         req << self[:path]
       end
-      if !path.root?
+      unless path.root?
         # Start at our parent, to avoid autorequiring ourself
         parents = path.parent.enum_for(:ascend)
         if found = parents.find { |p| catalog.resource(autorequire_type, p.to_s) }
@@ -108,19 +108,19 @@ def self.is_descendant?(a, b)
     ['acl']
   end
 
-  newproperty(:permission, :array_matching => :all) do
-    desc "ACL permission(s)."
+  newproperty(:permission, array_matching: :all) do
+    desc 'ACL permission(s).'
 
     def is_to_s(value)
-      if value == :absent or value.include?(:absent)
+      if value == :absent || value.include?(:absent)
         super
       else
         value.sort.inspect
       end
     end
 
     def should_to_s(value)
-      if value == :absent or value.include?(:absent)
+      if value == :absent || value.include?(:absent)
         super
       else
         value.sort.inspect
@@ -136,11 +136,11 @@ def strip_perms(pl)
               user:root:rwx
                 becomes
               user:root:"
-      Puppet.debug "permission.strip_perms"
+      Puppet.debug 'permission.strip_perms'
       value = []
       pl.each do |perm|
-        unless perm =~ /^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):/
-          perm = perm.split(':',-1)[0..-2].join(':')
+        unless perm =~ %r{^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):}
+          perm = perm.split(':', -1)[0..-2].join(':')
           value << perm
         end
       end
@@ -154,37 +154,31 @@ def strip_perms(pl)
     def unset_insync(cur_perm)
       # Puppet.debug "permission.unset_insync"
       test_should = []
-      @should.each { |x| test_should << x.downcase() }
+      @should.each { |x| test_should << x.downcase }
       cp = strip_perms(cur_perm)
       sp = strip_perms(test_should)
       (sp - cp).sort == sp
     end
 
     def set_insync(cur_perm)
       should = @should.uniq.sort
-      (cur_perm.sort == should) or (provider.check_set and ((should - cur_perm).length == 0))
+      (cur_perm.sort == should) || (provider.check_set && (should - cur_perm).empty?)
     end
 
     def purge_insync(cur_perm)
       # Puppet.debug "permission.purge_insync"
       cur_perm.each do |perm|
         # If anything other than the mode bits are set, we're not in sync
-        if !(perm =~ /^(((u(ser)?)|(g(roup)?)|(o(ther)?)):):/)
-          return false
-        end
+        return false unless perm =~ %r{^(((u(ser)?)|(g(roup)?)|(o(ther)?)):):}
       end
-      return true
+      true
     end
 
     def insync?(is)
       Puppet.debug "permission.insync? is: #{is.inspect} @should: #{@should.inspect}"
-      if provider.check_purge
-        return purge_insync(is)
-      end
-      if provider.check_unset
-        return unset_insync(is)
-      end
-      return set_insync(is)
+      return purge_insync(is) if provider.check_purge
+      return unset_insync(is) if provider.check_unset
+      set_insync(is)
     end
 
     # Munge into normalised form
@@ -206,20 +200,20 @@ def insync?(is)
       end
       t = a.shift # Copy the type.
       r << case t
-      when 'u', 'user'
-        'user:'
-      when 'g', 'group'
-        'group:'
-      when 'o', 'other'
-        'other:'
-      when 'm', 'mask'
-        'mask:'
-      else
-        raise ArgumentError, %(Unknown type "#{t}", expected "user", "group", "other" or "mask".)
+           when 'u', 'user'
+             'user:'
+           when 'g', 'group'
+             'group:'
+           when 'o', 'other'
+             'other:'
+           when 'm', 'mask'
+             'mask:'
+           else
+             raise ArgumentError, %(Unknown type "#{t}", expected "user", "group", "other" or "mask".)
       end
       r << "#{a.shift}:" # Copy the "who".
       p = a.shift
-      if p =~ /[0-7]/
+      if p =~ %r{[0-7]}
         p = p.oct
         r << (p | 4 ? 'r' : '-')
         r << (p | 2 ? 'w' : '-')
@@ -230,27 +224,25 @@ def insync?(is)
         r << (s.sub!('r', '') ? 'r' : '-')
         r << (s.sub!('w', '') ? 'w' : '-')
         r << (s.sub!('x', '') ? 'x' : '-')
-        if !s.empty?
-          raise ArgumentError, %(Invalid permission set "#{p}".)
-        end
+        raise ArgumentError, %(Invalid permission set "#{p}".) unless s.empty?
       end
       r
     end
   end
 
   newparam(:recursive) do
-    desc "Apply ACLs recursively."
+    desc 'Apply ACLs recursively.'
     newvalues(:true, :false)
     defaultto :false
   end
 
   def self.pick_default_perms(acl)
-    return acl.reject { |a| a.split(':', -1).length == 4 }
+    acl.reject { |a| a.split(':', -1).length == 4 }
   end
 
   def newchild(path)
-    options = @original_parameters.merge(:name => path).reject { |param, value| value.nil? }
-    unless File.directory?(options[:name]) then
+    options = @original_parameters.merge(name: path).reject { |_param, value| value.nil? }
+    unless File.directory?(options[:name])
       options[:permission] = self.class.pick_default_perms(options[:permission]) if options.include?(:permission)
     end
     [:recursive, :recursemode, :path].each do |param|
@@ -260,9 +252,9 @@ def newchild(path)
   end
 
   def generate
-    return [] unless self[:recursive] == :true and self[:recursemode] == :deep
+    return [] unless self[:recursive] == :true && self[:recursemode] == :deep
     results = []
-    paths = Set.new()
+    paths = Set.new
     if File.directory?(self[:path])
       Dir.chdir(self[:path]) do
         Dir['**/*'].each do |path|
@@ -273,21 +265,20 @@ def generate
     # At the time we generate extra resources, all the files might now be present yet.
     # In prediction to that we also create ACL resources for child file resources that
     # might not have been applied yet.
-    catalog.resources.find_all { |r|
-      r.is_a?(Puppet::Type.type(:file)) and self.class.is_descendant?(self[:path], r[:path])
-    }.each do | found |
+    catalog.resources.select do |r|
+      r.is_a?(Puppet::Type.type(:file)) && self.class.is_descendant?(self[:path], r[:path])
+    end.each do |found|
       paths << found[:path]
     end
-    paths.each { | path |
+    paths.each do |path|
       results << newchild(path)
-    }
+    end
     results
   end
 
   validate do
     unless self[:permission]
-      raise(Puppet::Error, "permission is a required property.")
+      raise(Puppet::Error, 'permission is a required property.')
     end
   end
-
 end

spec/unit/puppet/provider/posixacl_spec.rb

@@ -4,24 +4,23 @@
 provider_class = Puppet::Type.type(:posix_acl).provider(:posixacl)
 
 describe provider_class do
-
   it 'declares a getfacl command' do
-    expect{
+    expect do
       provider_class.command :getfacl
-    }.not_to raise_error
+    end.not_to raise_error
   end
   it 'declares a setfacl command' do
-    expect{
+    expect do
       provider_class.command :setfacl
-    }.not_to raise_error
+    end.not_to raise_error
   end
   it 'encodes spaces in group names' do
     RSpec::Mocks.with_temporary_scope do
       Puppet::Type.stubs(:getfacl).returns("group:test group:rwx\n")
       File.stubs(:exist?).returns(true)
-      expect{
+      expect do
         provider_class.command :permission
-      } == ['group:test\040group:rwx']
+      end == ['group:test\040group:rwx']
     end
   end
 end