Code smell: Line modifiers are preferred to one-line blocks.

* Replaced 6 occurances of (while .*?) *do$ with

  The do is unneeded in the block header form and causes problems
  with the block-to-one-line transformation.

  3 Examples:

      The code:
          while line = f.gets do
      becomes:
          while line = f.gets
      The code:
          while line = shadow.gets do
      becomes:
          while line = shadow.gets
      The code:
          while wrapper = zeros.pop do
      becomes:
          while wrapper = zeros.pop

* Replaced 19 occurances of ((if|unless) .*?) *then$ with

  The then is unneeded in the block header form and causes problems
  with the block-to-one-line transformation.

  3 Examples:

      The code:
          if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } then
      becomes:
          if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) }
      The code:
          unless defined?(@spec_command) then
      becomes:
          unless defined?(@spec_command)
      The code:
          if c == ?\n then
      becomes:
          if c == ?\n

* Replaced 758 occurances of

      ((?:if|unless|while|until) .*)
          (.*)
      end

  with

  The one-line form is preferable provided:

      * The condition is not used to assign a variable
      * The body line is not already modified
      * The resulting line is not too long

  3 Examples:

      The code:
          if Puppet.features.libshadow?
              has_feature :manages_passwords
          end
      becomes:
          has_feature :manages_passwords if Puppet.features.libshadow?
      The code:
          unless (defined?(@current_pool) and @current_pool)
              @current_pool = process_zpool_data(get_pool_data)
          end
      becomes:
          @current_pool = process_zpool_data(get_pool_data) unless (defined?(@current_pool) and @current_pool)
      The code:
          if Puppet[:trace]
              puts detail.backtrace
          end
      becomes:
          puts detail.backtrace if Puppet[:trace]

19 files changed, 39 insertions, 117 deletions

diff --git a/lib/puppet/provider/package/aix.rb b/lib/puppet/provider/package/aix.rb
index 7523b776d..fd0cfdc7e 100644
--- a/lib/puppet/provider/package/aix.rb
+++ b/lib/puppet/provider/package/aix.rb
@@ -22,9 +22,7 @@ Puppet::Type.type(:package).provide :aix, :parent => Puppet::Provider::Package d
     end
 
     def self.prefetch(packages)
-        if Process.euid != 0
-            raise Puppet::Error, "The aix provider can only be used by root"
-        end
+        raise Puppet::Error, "The aix provider can only be used by root" if Process.euid != 0
 
         return unless packages.detect { |name, package| package.should(:ensure) == :latest }
 
@@ -42,9 +40,7 @@ Puppet::Type.type(:package).provide :aix, :parent => Puppet::Provider::Package d
                     if updates.key?(current[:name])
                         previous = updates[current[:name]]
 
-                        unless Puppet::Util::Package.versioncmp(previous[:version], current[:version]) == 1
-                            updates[ current[:name] ] = current
-                        end
+                        updates[ current[:name] ] = current unless Puppet::Util::Package.versioncmp(previous[:version], current[:version]) == 1
 
                     else
                         updates[current[:name]] = current
@@ -73,9 +69,7 @@ Puppet::Type.type(:package).provide :aix, :parent => Puppet::Provider::Package d
 
         pkg = @resource[:name]
 
-        if (! @resource.should(:ensure).is_a? Symbol) and useversion
-            pkg << " #{@resource.should(:ensure)}"
-        end
+        pkg << " #{@resource.should(:ensure)}" if (! @resource.should(:ensure).is_a? Symbol) and useversion
 
         installp "-acgwXY", "-d", source, pkg
     end
@@ -118,9 +112,7 @@ Puppet::Type.type(:package).provide :aix, :parent => Puppet::Provider::Package d
         unless upd.nil?
             return "#{upd[:version]}"
         else
-            if properties[:ensure] == :absent
-                raise Puppet::DevError, "Tried to get latest on a missing package"
-            end
+            raise Puppet::DevError, "Tried to get latest on a missing package" if properties[:ensure] == :absent
 
             return properties[:ensure]
         end
diff --git a/lib/puppet/provider/package/apt.rb b/lib/puppet/provider/package/apt.rb
index 622e182ea..afbff6237 100755
--- a/lib/puppet/provider/package/apt.rb
+++ b/lib/puppet/provider/package/apt.rb
@@ -42,9 +42,7 @@ Puppet::Type.type(:package).provide :apt, :parent => :dpkg, :source => :dpkg do
     # Install a package using 'apt-get'.  This function needs to support
     # installing a specific version.
     def install
-        if @resource[:responsefile]
-            self.run_preseed
-        end
+        self.run_preseed if @resource[:responsefile]
         should = @resource[:ensure]
 
         checkforcdrom()
@@ -100,16 +98,12 @@ Puppet::Type.type(:package).provide :apt, :parent => :dpkg, :source => :dpkg do
     end
 
     def uninstall
-        if @resource[:responsefile]
-            self.run_preseed
-        end
+        self.run_preseed if @resource[:responsefile]
         aptget "-y", "-q", :remove, @resource[:name]
     end
 
     def purge
-        if @resource[:responsefile]
-            self.run_preseed
-        end
+        self.run_preseed if @resource[:responsefile]
         aptget '-y', '-q', :remove, '--purge', @resource[:name]
         # workaround a "bug" in apt, that already removed packages are not purged
         super
diff --git a/lib/puppet/provider/package/aptitude.rb b/lib/puppet/provider/package/aptitude.rb
index 607f8c0dc..5529535de 100755
--- a/lib/puppet/provider/package/aptitude.rb
+++ b/lib/puppet/provider/package/aptitude.rb
@@ -11,9 +11,7 @@ Puppet::Type.type(:package).provide :aptitude, :parent => :apt, :source => :dpkg
     def aptget(*args)
         args.flatten!
         # Apparently aptitude hasn't always supported a -q flag.
-        if args.include?("-q")
-            args.delete("-q")
-        end
+        args.delete("-q") if args.include?("-q")
         output = aptitude(*args)
 
         # Yay, stupid aptitude doesn't throw an error when the package is missing.
diff --git a/lib/puppet/provider/package/aptrpm.rb b/lib/puppet/provider/package/aptrpm.rb
index a3ad3b447..42f7e706c 100644
--- a/lib/puppet/provider/package/aptrpm.rb
+++ b/lib/puppet/provider/package/aptrpm.rb
@@ -59,9 +59,7 @@ Puppet::Type.type(:package).provide :aptrpm, :parent => :rpm, :source => :rpm do
 
             if available_versions.length == 0
                 self.debug "No latest version"
-                if Puppet[:debug]
-                    print output
-                end
+                print output if Puppet[:debug]
             end
 
             # Get the latest and greatest version number
diff --git a/lib/puppet/provider/package/blastwave.rb b/lib/puppet/provider/package/blastwave.rb
index ee1e8f73a..d1cfe6f2b 100755
--- a/lib/puppet/provider/package/blastwave.rb
+++ b/lib/puppet/provider/package/blastwave.rb
@@ -2,9 +2,7 @@
 Puppet::Type.type(:package).provide :blastwave, :parent => :sun, :source => :sun do
     desc "Package management using Blastwave.org's ``pkg-get`` command on Solaris."
     pkgget = "pkg-get"
-    if FileTest.executable?("/opt/csw/bin/pkg-get")
-        pkgget = "/opt/csw/bin/pkg-get"
-    end
+    pkgget = "/opt/csw/bin/pkg-get" if FileTest.executable?("/opt/csw/bin/pkg-get")
 
     confine :operatingsystem => :solaris
 
@@ -37,9 +35,7 @@ Puppet::Type.type(:package).provide :blastwave, :parent => :sun, :source => :sun
     def self.blastlist(hash)
         command = ["-c"]
 
-        if hash[:justme]
-            command << hash[:justme]
-        end
+        command << hash[:justme] if hash[:justme]
 
         output = Puppet::Util::Execution::withenv(:PAGER => "/usr/bin/cat") { pkgget command }
 
@@ -74,9 +70,7 @@ Puppet::Type.type(:package).provide :blastwave, :parent => :sun, :source => :sun
             end
             hash[:avail] = $5
 
-            if hash[:avail] == "SAME"
-                hash[:avail] = hash[:ensure]
-            end
+            hash[:avail] = hash[:ensure] if hash[:avail] == "SAME"
 
             # Use the name method, so it works with subclasses.
             hash[:provider] = self.name
diff --git a/lib/puppet/provider/package/darwinport.rb b/lib/puppet/provider/package/darwinport.rb
index 2153d2f83..4b9fdb462 100755
--- a/lib/puppet/provider/package/darwinport.rb
+++ b/lib/puppet/provider/package/darwinport.rb
@@ -56,9 +56,7 @@ Puppet::Type.type(:package).provide :darwinport, :parent => Puppet::Provider::Pa
     def query
         version = nil
         self.class.eachpkgashash do |hash|
-            if hash[:name] == @resource[:name]
-                return hash
-            end
+            return hash if hash[:name] == @resource[:name]
         end
 
         return nil
diff --git a/lib/puppet/provider/package/dpkg.rb b/lib/puppet/provider/package/dpkg.rb
index 3c3141b4f..77f7a61eb 100755
--- a/lib/puppet/provider/package/dpkg.rb
+++ b/lib/puppet/provider/package/dpkg.rb
@@ -52,9 +52,7 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
             elsif ['config-files', 'half-installed', 'unpacked', 'half-configured'].include?(hash[:status])
                 hash[:ensure] = :absent
             end
-            if hash[:desired] == 'hold'
-                hash[:ensure] = :held
-            end
+            hash[:ensure] = :held if hash[:desired] == 'hold'
         else
             Puppet.warning "Failed to match dpkg-query line #{line.inspect}"
             return nil
@@ -91,9 +89,7 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
     def latest
         output = dpkg_deb "--show", @resource[:source]
         matches = /^(\S+)\t(\S+)$/.match(output).captures
-        unless matches[0].match( Regexp.escape(@resource[:name]) )
-            warning "source doesn't contain named package, but #{matches[0]}"
-        end
+        warning "source doesn't contain named package, but #{matches[0]}" unless matches[0].match( Regexp.escape(@resource[:name]) )
         matches[1]
     end
 
diff --git a/lib/puppet/provider/package/fink.rb b/lib/puppet/provider/package/fink.rb
index 3f0d79475..a7310b00a 100755
--- a/lib/puppet/provider/package/fink.rb
+++ b/lib/puppet/provider/package/fink.rb
@@ -22,9 +22,7 @@ Puppet::Type.type(:package).provide :fink, :parent => :dpkg, :source => :dpkg do
     # Install a package using 'apt-get'.  This function needs to support
     # installing a specific version.
     def install
-        if @resource[:responsefile]
-            self.run_preseed
-        end
+        self.run_preseed if @resource[:responsefile]
         should = @resource.should(:ensure)
 
         str = @resource[:name]
diff --git a/lib/puppet/provider/package/freebsd.rb b/lib/puppet/provider/package/freebsd.rb
index 0c816cdb6..20ded987e 100755
--- a/lib/puppet/provider/package/freebsd.rb
+++ b/lib/puppet/provider/package/freebsd.rb
@@ -29,9 +29,7 @@ Puppet::Type.type(:package).provide :freebsd, :parent => :openbsd do
                 end
             end
         else
-            if @resource[:source]
-                Puppet.warning "source is defined but does not have trailing slash, ignoring #{@resource[:source]}"
-            end
+            Puppet.warning "source is defined but does not have trailing slash, ignoring #{@resource[:source]}" if @resource[:source]
             pkgadd "-r", @resource[:name]
         end
     end
diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb
index 1c345097f..53677a9c5 100755
--- a/lib/puppet/provider/package/gem.rb
+++ b/lib/puppet/provider/package/gem.rb
@@ -69,9 +69,7 @@ Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package d
 
     def install(useversion = true)
         command = [command(:gemcmd), "install"]
-        if (! resource[:ensure].is_a? Symbol) and useversion
-            command << "-v" << resource[:ensure]
-        end
+        command << "-v" << resource[:ensure] if (! resource[:ensure].is_a? Symbol) and useversion
         # Always include dependencies
         command << "--include-dependencies"
 
@@ -101,9 +99,7 @@ Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package d
 
         output = execute(command)
         # Apparently some stupid gem versions don't exit non-0 on failure
-        if output.include?("ERROR")
-            self.fail "Could not install: #{output.chomp}"
-        end
+        self.fail "Could not install: #{output.chomp}" if output.include?("ERROR")
     end
 
     def latest
diff --git a/lib/puppet/provider/package/nim.rb b/lib/puppet/provider/package/nim.rb
index a798f338a..8b273767e 100644
--- a/lib/puppet/provider/package/nim.rb
+++ b/lib/puppet/provider/package/nim.rb
@@ -28,9 +28,7 @@ Puppet::Type.type(:package).provide :nim, :parent => :aix, :source => :aix do
 
         pkg = @resource[:name]
 
-        if (! @resource.should(:ensure).is_a? Symbol) and useversion
-            pkg << " " << @resource.should(:ensure)
-        end
+        pkg << " " << @resource.should(:ensure) if (! @resource.should(:ensure).is_a? Symbol) and useversion
 
         nimclient "-o", "cust", "-a", "installp_flags=acgwXY", "-a", "lpp_source=#{source}", "-a", "filesets='#{pkg}'"
     end
diff --git a/lib/puppet/provider/package/openbsd.rb b/lib/puppet/provider/package/openbsd.rb
index 149b557ae..f5f978056 100755
--- a/lib/puppet/provider/package/openbsd.rb
+++ b/lib/puppet/provider/package/openbsd.rb
@@ -65,9 +65,7 @@ Puppet::Type.type(:package).provide :openbsd, :parent => Puppet::Provider::Packa
 
         if @resource[:source] =~ /\/$/
             withenv :PKG_PATH => @resource[:source] do
-                if (@resource[:ensure] = get_version) == nil
-                    @resource[:ensure] = old_ensure
-                end
+                @resource[:ensure] = old_ensure if (@resource[:ensure] = get_version) == nil
                 full_name = [ @resource[:name], @resource[:ensure], @resource[:flavor] ]
                 pkgadd full_name.join('-').chomp('-')
             end
diff --git a/lib/puppet/provider/package/pkg.rb b/lib/puppet/provider/package/pkg.rb
index c0767a7f0..ab8429864 100644
--- a/lib/puppet/provider/package/pkg.rb
+++ b/lib/puppet/provider/package/pkg.rb
@@ -100,9 +100,7 @@ Puppet::Type.type(:package).provide :pkg, :parent => Puppet::Provider::Package d
         hash = self.class.parse_line(output) ||
             {:ensure => :absent, :status => 'missing', :name => @resource[:name], :error => 'ok'}
 
-        if hash[:error] != "ok"
-            raise Puppet::Error.new( "Package #{hash[:name]}, version #{hash[:version]} is in error state: #{hash[:error]}")
-        end
+        raise Puppet::Error.new( "Package #{hash[:name]}, version #{hash[:version]} is in error state: #{hash[:error]}") if hash[:error] != "ok"
 
         return hash
     end
diff --git a/lib/puppet/provider/package/pkgdmg.rb b/lib/puppet/provider/package/pkgdmg.rb
index ac4853d83..b533f1002 100644
--- a/lib/puppet/provider/package/pkgdmg.rb
+++ b/lib/puppet/provider/package/pkgdmg.rb
@@ -92,9 +92,7 @@ Puppet::Type.type(:package).provide :pkgdmg, :parent => Puppet::Provider::Packag
                 File.open(cached_source) do |dmg|
                     xml_str = hdiutil "mount", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", "/tmp", dmg.path
                     hdiutil_info = Plist::parse_xml(xml_str)
-                    unless hdiutil_info.has_key?("system-entities")
-                        raise Puppet::Error.new("No disk entities returned by mount at #{dmg.path}")
-                    end
+                    raise Puppet::Error.new("No disk entities returned by mount at #{dmg.path}") unless hdiutil_info.has_key?("system-entities")
                     mounts = hdiutil_info["system-entities"].collect { |entity|
                         entity["mount-point"]
                     }.compact
diff --git a/lib/puppet/provider/package/portage.rb b/lib/puppet/provider/package/portage.rb
index 7e1119a1a..a8d071f41 100644
--- a/lib/puppet/provider/package/portage.rb
+++ b/lib/puppet/provider/package/portage.rb
@@ -20,9 +20,7 @@ Puppet::Type.type(:package).provide :portage, :parent => Puppet::Provider::Packa
         search_format = "<category> <name> [<installedversions:LASTVERSION>] [<bestversion:LASTVERSION>] <homepage> <description>\n"
 
         begin
-            if !FileUtils.uptodate?("/var/cache/eix", %w{/usr/bin/eix /usr/portage/metadata/timestamp})
-                update_eix
-            end
+            update_eix if !FileUtils.uptodate?("/var/cache/eix", %w{/usr/bin/eix /usr/portage/metadata/timestamp})
 
             search_output = nil
             Puppet::Util::Execution.withenv :LASTVERSION => version_format do
@@ -83,9 +81,7 @@ Puppet::Type.type(:package).provide :portage, :parent => Puppet::Provider::Packa
         search_value = package_name
 
         begin
-            if !FileUtils.uptodate?("/var/cache/eix", %w{/usr/bin/eix /usr/portage/metadata/timestamp})
-                update_eix
-            end
+            update_eix if !FileUtils.uptodate?("/var/cache/eix", %w{/usr/bin/eix /usr/portage/metadata/timestamp})
 
             search_output = nil
             Puppet::Util::Execution.withenv :LASTVERSION => version_format do
diff --git a/lib/puppet/provider/package/ports.rb b/lib/puppet/provider/package/ports.rb
index 286aca8e1..4a925aa8d 100755
--- a/lib/puppet/provider/package/ports.rb
+++ b/lib/puppet/provider/package/ports.rb
@@ -10,9 +10,7 @@ Puppet::Type.type(:package).provide :ports, :parent => :freebsd, :source => :fre
 
     # I hate ports
     %w{INTERACTIVE UNAME}.each do |var|
-        if ENV.include?(var)
-            ENV.delete(var)
-        end
+        ENV.delete(var) if ENV.include?(var)
     end
 
     def install
diff --git a/lib/puppet/provider/package/rpm.rb b/lib/puppet/provider/package/rpm.rb
index 6d5adf2ef..abcfbd3b6 100755
--- a/lib/puppet/provider/package/rpm.rb
+++ b/lib/puppet/provider/package/rpm.rb
@@ -94,9 +94,7 @@ Puppet::Type.type(:package).provide :rpm, :source => :rpm, :parent => Puppet::Pr
         end
 
         flag = "-i"
-        if @property_hash[:ensure] and @property_hash[:ensure] != :absent
-            flag = "-U"
-        end
+        flag = "-U" if @property_hash[:ensure] and @property_hash[:ensure] != :absent
 
         rpm flag, "--oldpackage", source
     end
diff --git a/lib/puppet/provider/package/sun.rb b/lib/puppet/provider/package/sun.rb
index f0a1c6ac5..2d4e1ac3f 100755
--- a/lib/puppet/provider/package/sun.rb
+++ b/lib/puppet/provider/package/sun.rb
@@ -50,9 +50,7 @@ Puppet::Type.type(:package).provide :sun, :parent => Puppet::Provider::Package d
                     name = $1
                     value = $2
                     if names.include?(name)
-                        unless names[name].nil?
-                            hash[names[name]] = value
-                        end
+                        hash[names[name]] = value unless names[name].nil?
                     end
                 when /\s+\d+.+/
                     # nothing; we're ignoring the FILES info
@@ -84,9 +82,7 @@ Puppet::Type.type(:package).provide :sun, :parent => Puppet::Provider::Package d
 
         hash = {}
         cmd = "#{command(:pkginfo)} -l"
-        if device
-            cmd += " -d #{device}"
-        end
+        cmd += " -d #{device}" if device
         cmd += " #{@resource[:name]}"
 
         begin
@@ -101,9 +97,7 @@ Puppet::Type.type(:package).provide :sun, :parent => Puppet::Provider::Package d
                         name = $1
                         value = $2
                         if names.include?(name)
-                            unless names[name].nil?
-                                hash[names[name]] = value
-                            end
+                            hash[names[name]] = value unless names[name].nil?
                         end
                     when /\s+\d+.+/
                         # nothing; we're ignoring the FILES info
@@ -119,18 +113,12 @@ Puppet::Type.type(:package).provide :sun, :parent => Puppet::Provider::Package d
     end
 
     def install
-        unless @resource[:source]
-            raise Puppet::Error, "Sun packages must specify a package source"
-        end
+        raise Puppet::Error, "Sun packages must specify a package source" unless @resource[:source]
         cmd = []
 
-        if @resource[:adminfile]
-            cmd << "-a" << @resource[:adminfile]
-        end
+        cmd << "-a" << @resource[:adminfile] if @resource[:adminfile]
 
-        if @resource[:responsefile]
-            cmd << "-r" << @resource[:responsefile]
-        end
+        cmd << "-r" << @resource[:responsefile] if @resource[:responsefile]
 
         cmd << "-d" << @resource[:source]
         cmd << "-n" << @resource[:name]
@@ -151,9 +139,7 @@ Puppet::Type.type(:package).provide :sun, :parent => Puppet::Provider::Package d
     def uninstall
         command  = ["-n"]
 
-        if @resource[:adminfile]
-            command << "-a" << @resource[:adminfile]
-        end
+        command << "-a" << @resource[:adminfile] if @resource[:adminfile]
 
         command << @resource[:name]
         pkgrm command
@@ -162,9 +148,7 @@ Puppet::Type.type(:package).provide :sun, :parent => Puppet::Provider::Package d
     # Remove the old package, and install the new one.  This will probably
     # often fail.
     def update
-        if (@property_hash[:ensure] || info2hash()[:ensure]) != :absent
-            self.uninstall
-        end
+        self.uninstall if (@property_hash[:ensure] || info2hash()[:ensure]) != :absent
         self.install
     end
 end
diff --git a/lib/puppet/provider/package/yum.rb b/lib/puppet/provider/package/yum.rb
index 4ff365de7..17981f03c 100755
--- a/lib/puppet/provider/package/yum.rb
+++ b/lib/puppet/provider/package/yum.rb
@@ -22,9 +22,7 @@ Puppet::Type.type(:package).provide :yum, :parent => :rpm, :source => :rpm do
     defaultfor :operatingsystem => [:fedora, :centos, :redhat]
 
     def self.prefetch(packages)
-        if Process.euid != 0
-            raise Puppet::Error, "The yum provider can only be used as root"
-        end
+        raise Puppet::Error, "The yum provider can only be used as root" if Process.euid != 0
         super
         return unless packages.detect { |name, package| package.should(:ensure) == :latest }
 
@@ -68,15 +66,11 @@ Puppet::Type.type(:package).provide :yum, :parent => :rpm, :source => :rpm do
         output = yum "-d", "0", "-e", "0", "-y", :install, wanted
 
         is = self.query
-        unless is
-            raise Puppet::Error, "Could not find package #{self.name}"
-        end
+        raise Puppet::Error, "Could not find package #{self.name}" unless is
 
         # FIXME: Should we raise an exception even if should == :latest
         # and yum updated us to a version other than @param_hash[:ensure] ?
-        if should && should != is[:ensure]
-            raise Puppet::Error, "Failed to update to version #{should}, got version #{is[:ensure]} instead"
-        end
+        raise Puppet::Error, "Failed to update to version #{should}, got version #{is[:ensure]} instead" if should && should != is[:ensure]
     end
 
     # What's the latest package version available?
@@ -89,9 +83,7 @@ Puppet::Type.type(:package).provide :yum, :parent => :rpm, :source => :rpm do
         else
             # Yum didn't find updates, pretend the current
             # version is the latest
-            if properties[:ensure] == :absent
-                raise Puppet::DevError, "Tried to get latest on a missing package"
-            end
+            raise Puppet::DevError, "Tried to get latest on a missing package" if properties[:ensure] == :absent
             return properties[:ensure]
         end
     end