1 files changed, 20 insertions, 22 deletions

diff --git a/lib/chef/provider/user/useradd.rb b/lib/chef/provider/user/useradd.rb
index 35a106b0b6..cf6f4e727f 100644
--- a/lib/chef/provider/user/useradd.rb
+++ b/lib/chef/provider/user/useradd.rb
@@ -23,25 +23,25 @@ class Chef
   class Provider
     class User
       class Useradd < Chef::Provider::User
-        # MAJOR XXX: this should become the base class of all Useradd providers instead of the linux implementation
+        # the linux version of this has been forked off, this is the base class now of solaris and AIX and should be abandoned
+        # and those provider should be rewritten like the linux version.
 
-        UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:password, "-p"], [:shell, "-s"], [:uid, "-u"]]
+        UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:password, "-p"], [:shell, "-s"], [:uid, "-u"]].freeze
 
         def create_user
           command = compile_command("useradd") do |useradd|
             useradd.concat(universal_options)
             useradd.concat(useradd_options)
           end
-          shell_out!(*command)
+          shell_out_compact!(command)
         end
 
         def manage_user
-          unless universal_options.empty?
-            command = compile_command("usermod") do |u|
-              u.concat(universal_options)
-            end
-            shell_out!(*command)
+          return if universal_options.empty?
+          command = compile_command("usermod") do |u|
+            u.concat(universal_options)
           end
+          shell_out_compact!(command)
         end
 
         def remove_user
@@ -49,19 +49,19 @@ class Chef
           command << "-r" if managing_home_dir?
           command << "-f" if new_resource.force
           command << new_resource.username
-          shell_out!(*command)
+          shell_out_compact!(command)
         end
 
         def check_lock
           # we can get an exit code of 1 even when it's successful on
           # rhel/centos (redhat bug 578534). See additional error checks below.
-          passwd_s = shell_out!("passwd", "-S", new_resource.username, :returns => [0, 1])
+          passwd_s = shell_out_compact!("passwd", "-S", new_resource.username, returns: [0, 1])
           if whyrun_mode? && passwd_s.stdout.empty? && passwd_s.stderr.match(/does not exist/)
             # if we're in whyrun mode and the user is not yet created we assume it would be
             return false
           end
 
-          raise Chef::Exceptions::User, "Cannot determine if #{@new_resource} is locked!" if passwd_s.stdout.empty?
+          raise Chef::Exceptions::User, "Cannot determine if #{new_resource} is locked!" if passwd_s.stdout.empty?
 
           status_line = passwd_s.stdout.split(" ")
           case status_line[1]
@@ -76,7 +76,7 @@ class Chef
           unless passwd_s.exitstatus == 0
             raise_lock_error = false
             if %w{redhat centos}.include?(node[:platform])
-              passwd_version_check = shell_out!("rpm -q passwd")
+              passwd_version_check = shell_out_compact!("rpm", "-q", "passwd")
               passwd_version = passwd_version_check.stdout.chomp
 
               unless passwd_version == "passwd-0.73-1"
@@ -93,11 +93,11 @@ class Chef
         end
 
         def lock_user
-          shell_out!("usermod", "-L", new_resource.username)
+          shell_out_compact!("usermod", "-L", new_resource.username)
         end
 
         def unlock_user
-          shell_out!("usermod", "-U", new_resource.username)
+          shell_out_compact!("usermod", "-U", new_resource.username)
         end
 
         def compile_command(base_command)
@@ -130,12 +130,10 @@ class Chef
         end
 
         def update_options(field, option, opts)
-          if @current_resource.send(field).to_s != new_resource.send(field).to_s
-            if new_resource.send(field)
-              Chef::Log.debug("#{new_resource} setting #{field} to #{new_resource.send(field)}")
-              opts << option << new_resource.send(field).to_s
-            end
-          end
+          return unless current_resource.send(field).to_s != new_resource.send(field).to_s
+          return unless new_resource.send(field)
+          Chef::Log.debug("#{new_resource} setting #{field} to #{new_resource.send(field)}")
+          opts << option << new_resource.send(field).to_s
         end
 
         def useradd_options
@@ -150,8 +148,8 @@ class Chef
           # Pathname#cleanpath does a better job than ::File::expand_path (on both unix and windows)
           # ::File.expand_path("///tmp") == ::File.expand_path("/tmp") => false
           # ::File.expand_path("\\tmp") => "C:/tmp"
-          return true if @current_resource.home.nil? && new_resource.home
-          new_resource.home && Pathname.new(@current_resource.home).cleanpath != Pathname.new(new_resource.home).cleanpath
+          return true if current_resource.home.nil? && new_resource.home
+          new_resource.home && Pathname.new(current_resource.home).cleanpath != Pathname.new(new_resource.home).cleanpath
         end
 
       end