Merge pull request #5674 from chef/lcg/user-provider-cleanup

user provider cleanup

13 files changed, 293 insertions, 308 deletions

diff --git a/lib/chef/provider/user.rb b/lib/chef/provider/user.rb
index 4b05ac8f5e..de94871a87 100644
--- a/lib/chef/provider/user.rb
+++ b/lib/chef/provider/user.rb
@@ -39,7 +39,7 @@ class Chef
         if @new_resource.gid.is_a? String
           @new_resource.gid(Etc.getgrnam(@new_resource.gid).gid)
         end
-      rescue ArgumentError => e
+      rescue ArgumentError
         @group_name_resolved = false
       end
 
@@ -53,7 +53,7 @@ class Chef
 
         begin
           user_info = Etc.getpwnam(@new_resource.username)
-        rescue ArgumentError => e
+        rescue ArgumentError
           @user_exists = false
           Chef::Log.debug("#{@new_resource} user does not exist")
           user_info = nil
@@ -97,7 +97,7 @@ class Chef
         requirements.assert(:all_actions) do |a|
           a.assertion { @shadow_lib_ok }
           a.failure_message Chef::Exceptions::MissingLibrary, "You must have ruby-shadow installed for password support!"
-          a.whyrun "ruby-shadow is not installed. Attempts to set user password will cause failure.  Assuming that this gem will have been previously installed." +
+          a.whyrun "ruby-shadow is not installed. Attempts to set user password will cause failure.  Assuming that this gem will have been previously installed." \
             "Note that user update converge may report false-positive on the basis of mismatched password. "
         end
         requirements.assert(:modify, :lock, :unlock) do |a|
@@ -137,34 +137,31 @@ class Chef
       end
 
       def action_remove
-        if @user_exists
-          converge_by("remove user #{@new_resource.username}") do
-            remove_user
-            Chef::Log.info("#{@new_resource} removed")
-          end
+        return unless @user_exists
+        converge_by("remove user #{@new_resource.username}") do
+          remove_user
+          Chef::Log.info("#{@new_resource} removed")
         end
       end
 
       def action_manage
-        if @user_exists && compare_user
-          converge_by("manage user #{@new_resource.username}") do
-            manage_user
-            Chef::Log.info("#{@new_resource} managed")
-          end
+        return unless @user_exists && compare_user
+        converge_by("manage user #{@new_resource.username}") do
+          manage_user
+          Chef::Log.info("#{@new_resource} managed")
         end
       end
 
       def action_modify
-        if compare_user
-          converge_by("modify user #{@new_resource.username}") do
-            manage_user
-            Chef::Log.info("#{@new_resource} modified")
-          end
+        return unless compare_user
+        converge_by("modify user #{@new_resource.username}") do
+          manage_user
+          Chef::Log.info("#{@new_resource} modified")
         end
       end
 
       def action_lock
-        if check_lock() == false
+        if check_lock == false
           converge_by("lock the user #{@new_resource.username}") do
             lock_user
             Chef::Log.info("#{@new_resource} locked")
@@ -175,7 +172,7 @@ class Chef
       end
 
       def action_unlock
-        if check_lock() == true
+        if check_lock == true
           converge_by("unlock user #{@new_resource.username}") do
             unlock_user
             Chef::Log.info("#{@new_resource} unlocked")
diff --git a/lib/chef/provider/user/aix.rb b/lib/chef/provider/user/aix.rb
index 8ac229ae4d..0e81c76bbc 100644
--- a/lib/chef/provider/user/aix.rb
+++ b/lib/chef/provider/user/aix.rb
@@ -23,7 +23,7 @@ class Chef
         provides :user, os: "aix"
         provides :aix_user
 
-        UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:shell, "-s"], [:uid, "-u"]]
+        UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:shell, "-s"], [:uid, "-u"]].freeze
 
         def create_user
           super
@@ -44,53 +44,52 @@ class Chef
         end
 
         def check_lock
-          lock_info = shell_out!("lsuser -a account_locked #{new_resource.username}")
+          lock_info = shell_out_compact!("lsuser", "-a", "account_locked", new_resource.username)
           if whyrun_mode? && passwd_s.stdout.empty? && lock_info.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 lock_info.stdout.empty?
+          raise Chef::Exceptions::User, "Cannot determine if #{new_resource} is locked!" if lock_info.stdout.empty?
 
           status = /\S+\s+account_locked=(\S+)/.match(lock_info.stdout)
-          if status && status[1] == "true"
-            @locked = true
-          else
-            @locked = false
-          end
+          @locked =
+            if status && status[1] == "true"
+              true
+            else
+              false
+            end
 
           @locked
         end
 
         def lock_user
-          shell_out!("chuser account_locked=true #{new_resource.username}")
+          shell_out_compact!("chuser", "account_locked=true", new_resource.username)
         end
 
         def unlock_user
-          shell_out!("chuser account_locked=false #{new_resource.username}")
+          shell_out_compact!("chuser", "account_locked=false", new_resource.username)
         end
 
         private
 
         def add_password
-          if @current_resource.password != @new_resource.password && @new_resource.password
-            Chef::Log.debug("#{@new_resource.username} setting password to #{@new_resource.password}")
-            command = "echo '#{@new_resource.username}:#{@new_resource.password}' | chpasswd -e"
-            shell_out!(command)
-          end
+          return unless current_resource.password != new_resource.password && new_resource.password
+          Chef::Log.debug("#{new_resource.username} setting password to #{new_resource.password}")
+          command = "echo '#{new_resource.username}:#{new_resource.password}' | chpasswd -e"
+          shell_out!(command)
         end
 
         # Aix specific handling to update users home directory.
         def manage_home
+          return unless updating_home? && managing_home_dir?
           # -m option does not work on aix, so move dir.
-          if updating_home? && managing_home_dir?
-            universal_options.delete("-m")
-            if ::File.directory?(@current_resource.home)
-              Chef::Log.debug("Changing users home directory from #{@current_resource.home} to #{new_resource.home}")
-              shell_out!("mv #{@current_resource.home} #{new_resource.home}")
-            else
-              Chef::Log.debug("Creating users home directory #{new_resource.home}")
-              shell_out!("mkdir -p #{new_resource.home}")
-            end
+          universal_options.delete("-m")
+          if ::File.directory?(current_resource.home)
+            Chef::Log.debug("Changing users home directory from #{current_resource.home} to #{new_resource.home}")
+            FileUtils.mv current_resource.home, new_resource.home
+          else
+            Chef::Log.debug("Creating users home directory #{new_resource.home}")
+            FileUtils.mkdir_p new_resource.home
           end
         end
 
diff --git a/lib/chef/provider/user/dscl.rb b/lib/chef/provider/user/dscl.rb
index 16d60ba116..2302a874e2 100644
--- a/lib/chef/provider/user/dscl.rb
+++ b/lib/chef/provider/user/dscl.rb
@@ -65,12 +65,12 @@ class Chef
           end
 
           requirements.assert(:all_actions) do |a|
-            a.assertion { ::File.exists?("/usr/bin/dscl") }
+            a.assertion { ::File.exist?("/usr/bin/dscl") }
             a.failure_message(Chef::Exceptions::User, "Cannot find binary '/usr/bin/dscl' on the system for #{new_resource}!")
           end
 
           requirements.assert(:all_actions) do |a|
-            a.assertion { ::File.exists?("/usr/bin/plutil") }
+            a.assertion { ::File.exist?("/usr/bin/plutil") }
             a.failure_message(Chef::Exceptions::User, "Cannot find binary '/usr/bin/plutil' on the system for #{new_resource}!")
           end
 
@@ -199,7 +199,7 @@ user password using shadow hash.")
         # Create a user using dscl
         #
         def dscl_create_user
-          run_dscl("create /Users/#{new_resource.username}")
+          run_dscl("create", "/Users/#{new_resource.username}")
         end
 
         #
@@ -208,7 +208,7 @@ user password using shadow hash.")
         #
         def dscl_create_comment
           comment = new_resource.comment || new_resource.username
-          run_dscl("create /Users/#{new_resource.username} RealName '#{comment}'")
+          run_dscl("create", "/Users/#{new_resource.username}", "RealName", comment)
         end
 
         #
@@ -224,7 +224,7 @@ user password using shadow hash.")
             raise(Chef::Exceptions::RequestedUIDUnavailable, "uid #{new_resource.uid} is already in use")
           end
 
-          run_dscl("create /Users/#{new_resource.username} UniqueID #{new_resource.uid}")
+          run_dscl("create", "/Users/#{new_resource.username}", "UniqueID", new_resource.uid)
         end
 
         #
@@ -235,7 +235,7 @@ user password using shadow hash.")
           uid = nil
           base_uid = new_resource.system ? 200 : 500
           next_uid_guess = base_uid
-          users_uids = run_dscl("list /Users uid")
+          users_uids = run_dscl("list", "/Users", "uid")
           while next_uid_guess < search_limit + base_uid
             if users_uids =~ Regexp.new("#{Regexp.escape(next_uid_guess.to_s)}\n")
               next_uid_guess += 1
@@ -244,7 +244,7 @@ user password using shadow hash.")
               break
             end
           end
-          return uid || raise("uid not found. Exhausted. Searched #{search_limit} times")
+          uid || raise("uid not found. Exhausted. Searched #{search_limit} times")
         end
 
         #
@@ -252,18 +252,18 @@ user password using shadow hash.")
         #
         def uid_used?(uid)
           return false unless uid
-          users_uids = run_dscl("list /Users uid").split("\n")
-          uid_map = users_uids.inject({}) do |tmap, tuid|
+          users_uids = run_dscl("list", "/Users", "uid").split("\n")
+          uid_map = users_uids.each_with_object({}) do |tuid, tmap|
             x = tuid.split
             tmap[x[1]] = x[0]
             tmap
           end
           if uid_map[uid.to_s]
-            unless uid_map[uid.to_s] == new_resource.username.to_s
+            unless uid_map[uid.to_s] == new_resource.username
               return true
             end
           end
-          return false
+          false
         end
 
         #
@@ -277,14 +277,14 @@ user password using shadow hash.")
             new_resource.gid(STAFF_GROUP_ID)
           elsif !new_resource.gid.to_s.match(/^\d+$/)
             begin
-              possible_gid = run_dscl("read /Groups/#{new_resource.gid} PrimaryGroupID").split(" ").last
-            rescue Chef::Exceptions::DsclCommandFailed => e
-              raise Chef::Exceptions::GroupIDNotFound.new("Group not found for #{new_resource.gid} when creating user #{new_resource.username}")
+              possible_gid = run_dscl("read", "/Groups/#{new_resource.gid}", "PrimaryGroupID").split(" ").last
+            rescue Chef::Exceptions::DsclCommandFailed
+              raise Chef::Exceptions::GroupIDNotFound, "Group not found for #{new_resource.gid} when creating user #{new_resource.username}"
             end
             # XXX: mutates the new resource
             new_resource.gid(possible_gid) if possible_gid && possible_gid.match(/^\d+$/)
           end
-          run_dscl("create /Users/#{new_resource.username} PrimaryGroupID '#{new_resource.gid}'")
+          run_dscl("create", "/Users/#{new_resource.username}", "PrimaryGroupID", new_resource.gid)
         end
 
         #
@@ -293,7 +293,7 @@ user password using shadow hash.")
         #
         def dscl_set_home
           if new_resource.home.nil? || new_resource.home.empty?
-            run_dscl("delete /Users/#{new_resource.username} NFSHomeDirectory")
+            run_dscl("delete", "/Users/#{new_resource.username}", "NFSHomeDirectory")
             return
           end
 
@@ -308,7 +308,7 @@ user password using shadow hash.")
               move_home
             end
           end
-          run_dscl("create /Users/#{new_resource.username} NFSHomeDirectory '#{new_resource.home}'")
+          run_dscl("create", "/Users/#{new_resource.username}", "NFSHomeDirectory", new_resource.home)
         end
 
         def validate_home_dir_specification!
@@ -318,17 +318,17 @@ user password using shadow hash.")
         end
 
         def current_home_exists?
-          ::File.exist?("#{current_resource.home}")
+          ::File.exist?(current_resource.home)
         end
 
         def new_home_exists?
-          ::File.exist?("#{new_resource.home}")
+          ::File.exist?(new_resource.home)
         end
 
         def ditto_home
           skel = "/System/Library/User Template/English.lproj"
-          raise(Chef::Exceptions::User, "can't find skel at: #{skel}") unless ::File.exists?(skel)
-          shell_out! "ditto '#{skel}' '#{new_resource.home}'"
+          raise(Chef::Exceptions::User, "can't find skel at: #{skel}") unless ::File.exist?(skel)
+          shell_out_compact!("ditto", skel, new_resource.home)
           ::FileUtils.chown_R(new_resource.username, new_resource.gid.to_s, new_resource.home)
         end
 
@@ -338,7 +338,7 @@ user password using shadow hash.")
           src = current_resource.home
           FileUtils.mkdir_p(new_resource.home)
           files = ::Dir.glob("#{Chef::Util::PathHelper.escape_glob_dir(src)}/*", ::File::FNM_DOTMATCH) - ["#{src}/.", "#{src}/.."]
-          ::FileUtils.mv(files, new_resource.home, :force => true)
+          ::FileUtils.mv(files, new_resource.home, force: true)
           ::FileUtils.rmdir(src)
           ::FileUtils.chown_R(new_resource.username, new_resource.gid.to_s, new_resource.home)
         end
@@ -347,10 +347,10 @@ user password using shadow hash.")
         # Sets the shell for the user using dscl.
         #
         def dscl_set_shell
-          if new_resource.shell || ::File.exists?("#{new_resource.shell}")
-            run_dscl("create /Users/#{new_resource.username} UserShell '#{new_resource.shell}'")
+          if new_resource.shell
+            run_dscl("create", "/Users/#{new_resource.username}", "UserShell", new_resource.shell)
           else
-            run_dscl("create /Users/#{new_resource.username} UserShell '/usr/bin/false'")
+            run_dscl("create", "/Users/#{new_resource.username}", "UserShell", "/usr/bin/false")
           end
         end
 
@@ -367,9 +367,8 @@ user password using shadow hash.")
 
           # Shadow info is saved as binary plist. Convert the info to binary plist.
           shadow_info_binary = StringIO.new
-          command = Mixlib::ShellOut.new("plutil -convert binary1 -o - -",
-                                         :input => shadow_info.to_plist, :live_stream => shadow_info_binary)
-          command.run_command
+          shell_out_compact("plutil", "-convert", "binary1", "-o", "-", "-",
+                            input: shadow_info.to_plist, live_stream: shadow_info_binary)
 
           if user_info.nil?
             # User is  just created. read_user_info() will read the fresh information
@@ -401,7 +400,7 @@ user password using shadow hash.")
                            # Create a random 4 byte salt
                            salt = OpenSSL::Random.random_bytes(4)
                            encoded_password = OpenSSL::Digest::SHA512.hexdigest(salt + new_resource.password)
-                           hash_value = salt.unpack("H*").first + encoded_password
+                           salt.unpack("H*").first + encoded_password
                          end
 
             shadow_info["SALTED-SHA512"] = StringIO.new
@@ -449,21 +448,21 @@ user password using shadow hash.")
           end
 
           # Remove the user from its groups
-          run_dscl("list /Groups").each_line do |group|
+          run_dscl("list", "/Groups").each_line do |group|
             if member_of_group?(group.chomp)
-              run_dscl("delete /Groups/#{group.chomp} GroupMembership '#{new_resource.username}'")
+              run_dscl("delete", "/Groups/#{group.chomp}", "GroupMembership", new_resource.username)
             end
           end
 
           # Remove user account
-          run_dscl("delete /Users/#{new_resource.username}")
+          run_dscl("delete", "/Users/#{new_resource.username}")
         end
 
         #
         # Locks the user.
         #
         def lock_user
-          run_dscl("append /Users/#{new_resource.username} AuthenticationAuthority ';DisabledUser;'")
+          run_dscl("append", "/Users/#{new_resource.username}", "AuthenticationAuthority", ";DisabledUser;")
         end
 
         #
@@ -471,7 +470,7 @@ user password using shadow hash.")
         #
         def unlock_user
           auth_string = authentication_authority.gsub(/AuthenticationAuthority: /, "").gsub(/;DisabledUser;/, "").strip
-          run_dscl("create /Users/#{new_resource.username} AuthenticationAuthority '#{auth_string}'")
+          run_dscl("create", "/Users/#{new_resource.username}", "AuthenticationAuthority", auth_string)
         end
 
         #
@@ -489,7 +488,7 @@ user password using shadow hash.")
         # This is the interface base User provider requires to provide idempotency.
         #
         def check_lock
-          return @locked = locked?
+          @locked = locked?
         end
 
         #
@@ -501,11 +500,11 @@ user password using shadow hash.")
         # given attribute.
         #
         def diverged?(parameter)
-          parameter_updated?(parameter) && (not new_resource.send(parameter).nil?)
+          parameter_updated?(parameter) && !new_resource.send(parameter).nil?
         end
 
         def parameter_updated?(parameter)
-          not (new_resource.send(parameter) == current_resource.send(parameter))
+          !(new_resource.send(parameter) == current_resource.send(parameter))
         end
 
         #
@@ -551,7 +550,7 @@ user password using shadow hash.")
         def member_of_group?(group_name)
           membership_info = ""
           begin
-            membership_info = run_dscl("read /Groups/#{group_name}")
+            membership_info = run_dscl("read", "/Groups/#{group_name}")
           rescue Chef::Exceptions::DsclCommandFailed
             # Raised if the group doesn't contain any members
           end
@@ -568,14 +567,14 @@ user password using shadow hash.")
 
         # A simple map of Chef's terms to DSCL's terms.
         DSCL_PROPERTY_MAP = {
-          :uid => "uid",
-          :gid => "gid",
-          :home => "home",
-          :shell => "shell",
-          :comment => "realname",
-          :password => "passwd",
-          :auth_authority => "authentication_authority",
-          :shadow_hash => "ShadowHashData",
+          uid: "uid",
+          gid: "gid",
+          home: "home",
+          shell: "shell",
+          comment: "realname",
+          password: "passwd",
+          auth_authority: "authentication_authority",
+          shadow_hash: "ShadowHashData",
         }.freeze
 
         # Directory where the user plist files are stored for versions 10.7 and above
@@ -590,11 +589,11 @@ user password using shadow hash.")
 
           # We flush the cache here in order to make sure that we read fresh information
           # for the user.
-          shell_out("dscacheutil '-flushcache'")
+          shell_out_compact("dscacheutil", "-flushcache") # FIXME: this is MacOS version dependent
 
           begin
             user_plist_file = "#{USER_PLIST_DIRECTORY}/#{new_resource.username}.plist"
-            user_plist_info = run_plutil("convert xml1 -o - #{user_plist_file}")
+            user_plist_info = run_plutil("convert", "xml1", "-o", "-", user_plist_file)
             user_info = Plist.parse_xml(user_plist_info)
           rescue Chef::Exceptions::PlistUtilCommandFailed
           end
@@ -609,7 +608,7 @@ user password using shadow hash.")
         def save_user_info(user_info)
           user_plist_file = "#{USER_PLIST_DIRECTORY}/#{new_resource.username}.plist"
           Plist::Emit.save_plist(user_info, user_plist_file)
-          run_plutil("convert binary1 #{user_plist_file}")
+          run_plutil("convert", "binary1", user_plist_file)
         end
 
         #
@@ -658,7 +657,9 @@ user password using shadow hash.")
         end
 
         def run_dscl(*args)
-          result = shell_out("dscl . -#{args.join(' ')}")
+          argdup = args.dup
+          cmd = argdup.shift
+          result = shell_out_compact("dscl", ".", "-#{cmd}", argdup)
           return "" if ( args.first =~ /^delete/ ) && ( result.exitstatus != 0 )
           raise(Chef::Exceptions::DsclCommandFailed, "dscl error: #{result.inspect}") unless result.exitstatus == 0
           raise(Chef::Exceptions::DsclCommandFailed, "dscl error: #{result.inspect}") if result.stdout =~ /No such key: /
@@ -666,17 +667,19 @@ user password using shadow hash.")
         end
 
         def run_plutil(*args)
-          result = shell_out("plutil -#{args.join(' ')}")
+          argdup = args.dup
+          cmd = argdup.shift
+          result = shell_out_compact("plutil", "-#{cmd}", argdup)
           raise(Chef::Exceptions::PlistUtilCommandFailed, "plutil error: #{result.inspect}") unless result.exitstatus == 0
           if result.stdout.encoding == Encoding::ASCII_8BIT
-            result.stdout.encode("utf-8", "binary", :undef => :replace, :invalid => :replace, :replace => "?")
+            result.stdout.encode("utf-8", "binary", undef: :replace, invalid: :replace, replace: "?")
           else
             result.stdout
           end
         end
 
         def convert_binary_plist_to_xml(binary_plist_string)
-          Mixlib::ShellOut.new("plutil -convert xml1 -o - -", :input => binary_plist_string).run_command.stdout
+          shell_out_compact("plutil", "-convert", "xml1", "-o", "-", "-", input: binary_plist_string).stdout
         end
 
         def convert_to_binary(string)
diff --git a/lib/chef/provider/user/linux.rb b/lib/chef/provider/user/linux.rb
index 968cf771e4..445421ad38 100644
--- a/lib/chef/provider/user/linux.rb
+++ b/lib/chef/provider/user/linux.rb
@@ -24,23 +24,23 @@ class Chef
         provides :user, os: "linux"
 
         def create_user
-          shell_out!(*clean_array("useradd", universal_options, useradd_options, new_resource.username))
+          shell_out_compact!("useradd", universal_options, useradd_options, new_resource.username)
         end
 
         def manage_user
-          shell_out!(*clean_array("usermod", universal_options, usermod_options, new_resource.username))
+          shell_out_compact!("usermod", universal_options, usermod_options, new_resource.username)
         end
 
         def remove_user
-          shell_out!(*clean_array("userdel", userdel_options, new_resource.username))
+          shell_out_compact!("userdel", userdel_options, new_resource.username)
         end
 
         def lock_user
-          shell_out!(*clean_array("usermod", "-L", new_resource.username))
+          shell_out_compact!("usermod", "-L", new_resource.username)
         end
 
         def unlock_user
-          shell_out!(*clean_array("usermod", "-U", new_resource.username))
+          shell_out_compact!("usermod", "-U", new_resource.username)
         end
 
         # common to usermod and useradd
@@ -69,11 +69,11 @@ class Chef
         def useradd_options
           opts = []
           opts << "-r" if new_resource.system
-          if managing_home_dir?
-            opts << "-m"
-          else
-            opts << "-M"
-          end
+          opts << if managing_home_dir?
+                    "-m"
+                  else
+                    "-M"
+                  end
           opts
         end
 
@@ -97,15 +97,12 @@ class Chef
         def check_lock
           # there's an old bug in rhel (https://bugzilla.redhat.com/show_bug.cgi?id=578534)
           # which means that both 0 and 1 can be success.
-          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 ])
 
           # checking "does not exist" has to come before exit code handling since centos and ubuntu differ in exit codes
           if passwd_s.stderr =~ /does not exist/
-            if whyrun_mode?
-              return false
-            else
-              raise Chef::Exceptions::User, "User #{new_resource.username} does not exist when checking lock status for #{new_resource}"
-            end
+            return false if whyrun_mode?
+            raise Chef::Exceptions::User, "User #{new_resource.username} does not exist when checking lock status for #{new_resource}"
           end
 
           # now raise if we didn't get a 0 or 1 (see above)
diff --git a/lib/chef/provider/user/pw.rb b/lib/chef/provider/user/pw.rb
index b210374eb9..42d862a983 100644
--- a/lib/chef/provider/user/pw.rb
+++ b/lib/chef/provider/user/pw.rb
@@ -27,49 +27,45 @@ class Chef
 
         def load_current_resource
           super
-          raise Chef::Exceptions::User, "Could not find binary /usr/sbin/pw for #{@new_resource}" unless ::File.exists?("/usr/sbin/pw")
+          raise Chef::Exceptions::User, "Could not find binary /usr/sbin/pw for #{new_resource}" unless ::File.exist?("/usr/sbin/pw")
         end
 
         def create_user
-          command = "pw useradd"
-          command << set_options
-          run_command(:command => command)
+          shell_out_compact!("pw", "useradd", set_options)
           modify_password
         end
 
         def manage_user
-          command = "pw usermod"
-          command << set_options
-          run_command(:command => command)
+          shell_out_compact!("pw", "usermod", set_options)
           modify_password
         end
 
         def remove_user
-          command = "pw userdel #{@new_resource.username}"
-          command << " -r" if managing_home_dir?
-          run_command(:command => command)
+          command = [ "pw", "userdel", new_resource.username ]
+          command << "-r" if managing_home_dir?
+          shell_out_compact!(command)
         end
 
         def check_lock
-          case @current_resource.password
-          when /^\*LOCKED\*/
-            @locked = true
-          else
-            @locked = false
-          end
+          @locked = case current_resource.password
+                    when /^\*LOCKED\*/
+                      true
+                    else
+                      false
+                    end
           @locked
         end
 
         def lock_user
-          run_command(:command => "pw lock #{@new_resource.username}")
+          shell_out_compact!("pw", "lock", new_resource.username)
         end
 
         def unlock_user
-          run_command(:command => "pw unlock #{@new_resource.username}")
+          shell_out_compact!("pw", "unlock", new_resource.username)
         end
 
         def set_options
-          opts = " #{@new_resource.username}"
+          opts = [ new_resource.username ]
 
           field_list = {
             "comment" => "-c",
@@ -80,26 +76,26 @@ class Chef
           }
           field_list.sort { |a, b| a[0] <=> b[0] }.each do |field, option|
             field_symbol = field.to_sym
-            if @current_resource.send(field_symbol) != @new_resource.send(field_symbol)
-              if @new_resource.send(field_symbol)
-                Chef::Log.debug("#{@new_resource} setting #{field} to #{@new_resource.send(field_symbol)}")
-                opts << " #{option} '#{@new_resource.send(field_symbol)}'"
-              end
+            next unless current_resource.send(field_symbol) != new_resource.send(field_symbol)
+            if new_resource.send(field_symbol)
+              Chef::Log.debug("#{new_resource} setting #{field} to #{new_resource.send(field_symbol)}")
+              opts << option
+              opts << new_resource.send(field_symbol)
             end
           end
           if managing_home_dir?
-            Chef::Log.debug("#{@new_resource} is managing the users home directory")
-            opts << " -m"
+            Chef::Log.debug("#{new_resource} is managing the users home directory")
+            opts << "-m"
           end
           opts
         end
 
         def modify_password
-          if (not @new_resource.password.nil?) && (@current_resource.password != @new_resource.password)
+          if !new_resource.password.nil? && (current_resource.password != new_resource.password)
             Chef::Log.debug("#{new_resource} updating password")
-            command = "pw usermod #{@new_resource.username} -H 0"
-            status = popen4(command, :waitlast => true) do |pid, stdin, stdout, stderr|
-              stdin.puts "#{@new_resource.password}"
+            command = "pw usermod #{new_resource.username} -H 0"
+            status = popen4(command, waitlast: true) do |pid, stdin, stdout, stderr|
+              stdin.puts new_resource.password.to_s
             end
 
             unless status.exitstatus == 0
diff --git a/lib/chef/provider/user/solaris.rb b/lib/chef/provider/user/solaris.rb
index 7aa0ceb93a..4e772312ae 100644
--- a/lib/chef/provider/user/solaris.rb
+++ b/lib/chef/provider/user/solaris.rb
@@ -26,7 +26,7 @@ class Chef
       class Solaris < Chef::Provider::User::Useradd
         provides :solaris_user
         provides :user, os: %w{omnios solaris2}
-        UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:shell, "-s"], [:uid, "-u"]]
+        UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:shell, "-s"], [:uid, "-u"]].freeze
 
         attr_writer :password_file
 
@@ -46,22 +46,22 @@ class Chef
         end
 
         def check_lock
-          user = IO.read(@password_file).match(/^#{Regexp.escape(@new_resource.username)}:([^:]*):/)
+          user = IO.read(@password_file).match(/^#{Regexp.escape(new_resource.username)}:([^:]*):/)
 
           # If we're in whyrun mode, and the user is not created, we assume it will be
           return false if whyrun_mode? && user.nil?
 
-          raise Chef::Exceptions::User, "Cannot determine if #{@new_resource} is locked!" if user.nil?
+          raise Chef::Exceptions::User, "Cannot determine if #{new_resource} is locked!" if user.nil?
 
           @locked = user[1].start_with?("*LK*")
         end
 
         def lock_user
-          shell_out!("passwd", "-l", new_resource.username)
+          shell_out_compact!("passwd", "-l", new_resource.username)
         end
 
         def unlock_user
-          shell_out!("passwd", "-u", new_resource.username)
+          shell_out_compact!("passwd", "-u", new_resource.username)
         end
 
         private
@@ -82,10 +82,9 @@ class Chef
         end
 
         def manage_password
-          if @current_resource.password != @new_resource.password && @new_resource.password
-            Chef::Log.debug("#{@new_resource} setting password to #{@new_resource.password}")
-            write_shadow_file
-          end
+          return unless current_resource.password != new_resource.password && new_resource.password
+          Chef::Log.debug("#{new_resource} setting password to #{new_resource.password}")
+          write_shadow_file
         end
 
         def write_shadow_file
@@ -93,7 +92,7 @@ class Chef
           ::File.open(@password_file) do |shadow_file|
             shadow_file.each do |entry|
               user = entry.split(":").first
-              if user == @new_resource.username
+              if user == new_resource.username
                 buffer.write(updated_password(entry))
               else
                 buffer.write(entry)
@@ -104,7 +103,7 @@ class Chef
 
           # FIXME: mostly duplicates code with file provider deploying a file
           s = ::File.stat(@password_file)
-          mode = s.mode & 07777
+          mode = s.mode & 0o7777
           uid  = s.uid
           gid  = s.gid
 
@@ -116,7 +115,7 @@ class Chef
 
         def updated_password(entry)
           fields = entry.split(":")
-          fields[1] = @new_resource.password
+          fields[1] = new_resource.password
           fields[2] = days_since_epoch
           fields.join(":")
         end
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
diff --git a/lib/chef/provider/user/windows.rb b/lib/chef/provider/user/windows.rb
index b086a1e32b..0afa8fa14a 100644
--- a/lib/chef/provider/user/windows.rb
+++ b/lib/chef/provider/user/windows.rb
@@ -31,31 +31,30 @@ class Chef
 
         def initialize(new_resource, run_context)
           super
-          @net_user = Chef::Util::Windows::NetUser.new(@new_resource.username)
+          @net_user = Chef::Util::Windows::NetUser.new(new_resource.username)
         end
 
         def load_current_resource
-          if @new_resource.gid
+          if new_resource.gid
             Chef::Log.warn("The 'gid' attribute is not implemented by the Windows platform. Please use the 'group' resource to assign a user to a group.")
           end
 
-          @current_resource = Chef::Resource::User.new(@new_resource.name)
-          @current_resource.username(@new_resource.username)
-          user_info = nil
+          @current_resource = Chef::Resource::User.new(new_resource.name)
+          current_resource.username(new_resource.username)
           begin
             user_info = @net_user.get_info
 
-            @current_resource.uid(user_info[:user_id])
-            @current_resource.comment(user_info[:full_name])
-            @current_resource.home(user_info[:home_dir])
-            @current_resource.shell(user_info[:script_path])
+            current_resource.uid(user_info[:user_id])
+            current_resource.comment(user_info[:full_name])
+            current_resource.home(user_info[:home_dir])
+            current_resource.shell(user_info[:script_path])
           rescue Chef::Exceptions::UserIDNotFound => e
             # e.message should be "The user name could not be found" but checking for that could cause a localization bug
             @user_exists = false
-            Chef::Log.debug("#{@new_resource} does not exist (#{e.message})")
+            Chef::Log.debug("#{new_resource} does not exist (#{e.message})")
           end
 
-          @current_resource
+          current_resource
         end
 
         # Check to see if the user needs any changes
@@ -64,12 +63,12 @@ class Chef
         # <true>:: If a change is required
         # <false>:: If the users are identical
         def compare_user
-          unless @net_user.validate_credentials(@new_resource.password)
-            Chef::Log.debug("#{@new_resource} password has changed")
+          unless @net_user.validate_credentials(new_resource.password)
+            Chef::Log.debug("#{new_resource} password has changed")
             return true
           end
           [ :uid, :comment, :home, :shell ].any? do |user_attrib|
-            !@new_resource.send(user_attrib).nil? && @new_resource.send(user_attrib) != @current_resource.send(user_attrib)
+            !new_resource.send(user_attrib).nil? && new_resource.send(user_attrib) != current_resource.send(user_attrib)
           end
         end
 
@@ -98,7 +97,7 @@ class Chef
         end
 
         def set_options
-          opts = { :name => @new_resource.username }
+          opts = { name: new_resource.username }
 
           field_list = {
             "comment" => "full_name",
@@ -110,14 +109,12 @@ class Chef
 
           field_list.sort { |a, b| a[0] <=> b[0] }.each do |field, option|
             field_symbol = field.to_sym
-            if @current_resource.send(field_symbol) != @new_resource.send(field_symbol)
-              if @new_resource.send(field_symbol)
-                unless field_symbol == :password
-                  Chef::Log.debug("#{@new_resource} setting #{field} to #{@new_resource.send(field_symbol)}")
-                end
-                opts[option.to_sym] = @new_resource.send(field_symbol)
-              end
+            next unless current_resource.send(field_symbol) != new_resource.send(field_symbol)
+            next unless new_resource.send(field_symbol)
+            unless field_symbol == :password
+              Chef::Log.debug("#{new_resource} setting #{field} to #{new_resource.send(field_symbol)}")
             end
+            opts[option.to_sym] = new_resource.send(field_symbol)
           end
           opts
         end
diff --git a/spec/unit/provider/user/dscl_spec.rb b/spec/unit/provider/user/dscl_spec.rb
index 7b8be02f3a..f59709e717 100644
--- a/spec/unit/provider/user/dscl_spec.rb
+++ b/spec/unit/provider/user/dscl_spec.rb
@@ -116,38 +116,38 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30"
   describe "when shelling out to dscl" do
     it "should run dscl with the supplied cmd /Path args" do
       shell_return = shellcmdresult.new("stdout", "err", 0)
-      expect(provider).to receive(:shell_out).with("dscl . -cmd /Path args").and_return(shell_return)
-      expect(provider.run_dscl("cmd /Path args")).to eq("stdout")
+      expect(provider).to receive(:shell_out).with("dscl", ".", "-cmd", "/Path", "args").and_return(shell_return)
+      expect(provider.run_dscl("cmd", "/Path", "args")).to eq("stdout")
     end
 
     it "returns an empty string from delete commands" do
       shell_return = shellcmdresult.new("out", "err", 23)
-      expect(provider).to receive(:shell_out).with("dscl . -delete /Path args").and_return(shell_return)
-      expect(provider.run_dscl("delete /Path args")).to eq("")
+      expect(provider).to receive(:shell_out).with("dscl", ".", "-delete", "/Path", "args").and_return(shell_return)
+      expect(provider.run_dscl("delete", "/Path", "args")).to eq("")
     end
 
     it "should raise an exception for any other command" do
       shell_return = shellcmdresult.new("out", "err", 23)
-      expect(provider).to receive(:shell_out).with("dscl . -cmd /Path arguments").and_return(shell_return)
-      expect { provider.run_dscl("cmd /Path arguments") }.to raise_error(Chef::Exceptions::DsclCommandFailed)
+      expect(provider).to receive(:shell_out).with("dscl", ".", "-cmd", "/Path", "arguments").and_return(shell_return)
+      expect { provider.run_dscl("cmd", "/Path", "arguments") }.to raise_error(Chef::Exceptions::DsclCommandFailed)
     end
 
     it "raises an exception when dscl reports 'no such key'" do
       shell_return = shellcmdresult.new("No such key: ", "err", 23)
-      expect(provider).to receive(:shell_out).with("dscl . -cmd /Path args").and_return(shell_return)
-      expect { provider.run_dscl("cmd /Path args") }.to raise_error(Chef::Exceptions::DsclCommandFailed)
+      expect(provider).to receive(:shell_out).with("dscl", ".", "-cmd", "/Path", "args").and_return(shell_return)
+      expect { provider.run_dscl("cmd", "/Path", "args") }.to raise_error(Chef::Exceptions::DsclCommandFailed)
     end
 
     it "raises an exception when dscl reports 'eDSRecordNotFound'" do
       shell_return = shellcmdresult.new("<dscl_cmd> DS Error: -14136 (eDSRecordNotFound)", "err", -14136)
-      expect(provider).to receive(:shell_out).with("dscl . -cmd /Path args").and_return(shell_return)
-      expect { provider.run_dscl("cmd /Path args") }.to raise_error(Chef::Exceptions::DsclCommandFailed)
+      expect(provider).to receive(:shell_out).with("dscl", ".", "-cmd", "/Path", "args").and_return(shell_return)
+      expect { provider.run_dscl("cmd", "/Path", "args") }.to raise_error(Chef::Exceptions::DsclCommandFailed)
     end
   end
 
   describe "get_free_uid" do
     before do
-      expect(provider).to receive(:run_dscl).with("list /Users uid").and_return("\nwheel      200\nstaff      201\nbrahms      500\nchopin      501\n")
+      expect(provider).to receive(:run_dscl).with("list", "/Users", "uid").and_return("\nwheel      200\nstaff      201\nbrahms      500\nchopin      501\n")
     end
 
     describe "when resource is configured as system" do
@@ -177,7 +177,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30"
 
     describe "when called with a user id" do
       before do
-        expect(provider).to receive(:run_dscl).with("list /Users uid").and_return("\naj      500\n")
+        expect(provider).to receive(:run_dscl).with("list", "/Users", "uid").and_return("\naj      500\n")
       end
 
       it "should return true for a used uid number" do
@@ -198,8 +198,8 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30"
     end
 
     it "finds a valid, unused uid when none is specified" do
-      expect(provider).to receive(:run_dscl).with("list /Users uid").and_return("")
-      expect(provider).to receive(:run_dscl).with("create /Users/toor UniqueID 501")
+      expect(provider).to receive(:run_dscl).with("list", "/Users", "uid").and_return("")
+      expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "UniqueID", 501)
       expect(provider).to receive(:get_free_uid).and_return(501)
       provider.dscl_set_uid
       expect(new_resource.uid).to eq(501)
@@ -207,8 +207,8 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30"
 
     it "sets the uid specified in the resource" do
       new_resource.uid(1000)
-      expect(provider).to receive(:run_dscl).with("create /Users/toor UniqueID 1000").and_return(true)
-      expect(provider).to receive(:run_dscl).with("list /Users uid").and_return("")
+      expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "UniqueID", 1000).and_return(true)
+      expect(provider).to receive(:run_dscl).with("list", "/Users", "uid").and_return("")
       provider.dscl_set_uid
     end
   end
@@ -219,9 +219,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30"
     end
 
     before do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      new_resource.supports({ :manage_home => true })
+      new_resource.manage_home true
       new_resource.home("/Users/toor")
 
       provider.current_resource = current_resource
@@ -229,7 +227,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30"
 
     it "deletes the home directory when resource#home is nil" do
       new_resource.instance_variable_set(:@home, nil)
-      expect(provider).to receive(:run_dscl).with("delete /Users/toor NFSHomeDirectory").and_return(true)
+      expect(provider).to receive(:run_dscl).with("delete", "/Users/toor", "NFSHomeDirectory").and_return(true)
       provider.dscl_set_home
     end
 
@@ -239,40 +237,40 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30"
     end
 
     it "moves the users home to the new location if it exists and the target location is different" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      new_resource.supports(:manage_home => true)
+      new_resource.manage_home true
 
       current_home = CHEF_SPEC_DATA + "/old_home_dir"
       current_home_files = [current_home + "/my-dot-emacs", current_home + "/my-dot-vim"]
       current_resource.home(current_home)
       new_resource.gid(23)
-      allow(::File).to receive(:exists?).with("/old/home/toor").and_return(true)
-      allow(::File).to receive(:exists?).with("/Users/toor").and_return(true)
+      allow(::File).to receive(:exist?).with("/old/home/toor").and_return(true)
+      allow(::File).to receive(:exist?).with("/Users/toor").and_return(true)
+      allow(::File).to receive(:exist?).with(current_home).and_return(true)
 
       expect(FileUtils).to receive(:mkdir_p).with("/Users/toor").and_return(true)
       expect(FileUtils).to receive(:rmdir).with(current_home)
       expect(::Dir).to receive(:glob).with("#{CHEF_SPEC_DATA}/old_home_dir/*", ::File::FNM_DOTMATCH).and_return(current_home_files)
-      expect(FileUtils).to receive(:mv).with(current_home_files, "/Users/toor", :force => true)
+      expect(FileUtils).to receive(:mv).with(current_home_files, "/Users/toor", force: true)
       expect(FileUtils).to receive(:chown_R).with("toor", "23", "/Users/toor")
 
-      expect(provider).to receive(:run_dscl).with("create /Users/toor NFSHomeDirectory '/Users/toor'")
+      expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "NFSHomeDirectory", "/Users/toor")
       provider.dscl_set_home
     end
 
     it "should raise an exception when the systems user template dir (skel) cannot be found" do
-      allow(::File).to receive(:exists?).and_return(false, false, false)
+      allow(::File).to receive(:exist?).and_return(false, false, false)
       expect { provider.dscl_set_home }.to raise_error(Chef::Exceptions::User)
     end
 
     it "should run ditto to copy any missing files from skel to the new home dir" do
-      expect(::File).to receive(:exists?).with("/System/Library/User\ Template/English.lproj").and_return(true)
+      expect(::File).to receive(:exist?).with("/System/Library/User\ Template/English.lproj").and_return(true)
       expect(FileUtils).to receive(:chown_R).with("toor", "", "/Users/toor")
-      expect(provider).to receive(:shell_out!).with("ditto '/System/Library/User Template/English.lproj' '/Users/toor'")
+      expect(provider).to receive(:shell_out!).with("ditto", "/System/Library/User Template/English.lproj", "/Users/toor")
       provider.ditto_home
     end
 
     it "creates the user's NFSHomeDirectory and home directory" do
-      expect(provider).to receive(:run_dscl).with("create /Users/toor NFSHomeDirectory '/Users/toor'").and_return(true)
+      expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "NFSHomeDirectory", "/Users/toor").and_return(true)
       expect(provider).to receive(:ditto_home)
       provider.dscl_set_home
     end
@@ -283,8 +281,8 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30"
     let(:plutil_exists) { true }
 
     before do
-      allow(::File).to receive(:exists?).with("/usr/bin/dscl").and_return(dscl_exists)
-      allow(::File).to receive(:exists?).with("/usr/bin/plutil").and_return(plutil_exists)
+      allow(::File).to receive(:exist?).with("/usr/bin/dscl").and_return(dscl_exists)
+      allow(::File).to receive(:exist?).with("/usr/bin/plutil").and_return(plutil_exists)
     end
 
     def run_requirements
@@ -382,8 +380,8 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30"
     let(:user_plist_file) { nil }
 
     before do
-      expect(provider).to receive(:shell_out).with("dscacheutil '-flushcache'")
-      expect(provider).to receive(:shell_out).with("plutil -convert xml1 -o - /var/db/dslocal/nodes/Default/users/toor.plist") do
+      expect(provider).to receive(:shell_out).with("dscacheutil", "-flushcache")
+      expect(provider).to receive(:shell_out).with("plutil", "-convert", "xml1", "-o", "-", "/var/db/dslocal/nodes/Default/users/toor.plist") do
         if user_plist_file.nil?
           shellcmdresult.new("Can not find the file", "Sorry!!", 1)
         else
@@ -391,7 +389,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30"
         end
       end
 
-      if !user_plist_file.nil?
+      unless user_plist_file.nil?
         expect(provider).to receive(:convert_binary_plist_to_xml).and_return(File.read(File.join(CHEF_SPEC_DATA, "mac_users/#{user_plist_file}.shadow.xml")))
       end
     end
@@ -726,7 +724,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30")
       expect(provider).to receive(:prepare_password_shadow_info).and_return({})
       mock_shellout = double("Mock::Shellout")
       allow(mock_shellout).to receive(:run_command)
-      expect(Mixlib::ShellOut).to receive(:new).and_return(mock_shellout)
+      expect(provider).to receive(:shell_out).and_return(mock_shellout)
       expect(provider).to receive(:read_user_info)
       expect(provider).to receive(:dscl_set)
       expect(provider).to receive(:sleep).with(3)
@@ -754,29 +752,29 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30")
       end
 
       it "creates the user and sets the comment field" do
-        expect(provider).to receive(:run_dscl).with("create /Users/toor").and_return(true)
+        expect(provider).to receive(:run_dscl).with("create", "/Users/toor").and_return(true)
         provider.dscl_create_user
       end
 
       it "sets the comment field" do
-        expect(provider).to receive(:run_dscl).with("create /Users/toor RealName '#mockssuck'").and_return(true)
+        expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "RealName", "#mockssuck").and_return(true)
         provider.dscl_create_comment
       end
 
       it "sets the comment field to username" do
         new_resource.comment nil
-        expect(provider).to receive(:run_dscl).with("create /Users/toor RealName '#mockssuck'").and_return(true)
+        expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "RealName", "#mockssuck").and_return(true)
         provider.dscl_create_comment
         expect(new_resource.comment).to eq("#mockssuck")
       end
 
       it "should run run_dscl with create /Users/user PrimaryGroupID to set the users primary group" do
-        expect(provider).to receive(:run_dscl).with("create /Users/toor PrimaryGroupID '1001'").and_return(true)
+        expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "PrimaryGroupID", 1001).and_return(true)
         provider.dscl_set_gid
       end
 
       it "should run run_dscl with create /Users/user UserShell to set the users login shell" do
-        expect(provider).to receive(:run_dscl).with("create /Users/toor UserShell '/usr/bin/false'").and_return(true)
+        expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "UserShell", "/usr/bin/false").and_return(true)
         provider.dscl_set_shell
       end
     end
@@ -788,21 +786,21 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30")
       end
 
       it "should map the group name to a numeric ID when the group exists" do
-        expect(provider).to receive(:run_dscl).with("read /Groups/newgroup PrimaryGroupID").ordered.and_return("PrimaryGroupID: 1001\n")
-        expect(provider).to receive(:run_dscl).with("create /Users/toor PrimaryGroupID '1001'").ordered.and_return(true)
+        expect(provider).to receive(:run_dscl).with("read", "/Groups/newgroup", "PrimaryGroupID").ordered.and_return("PrimaryGroupID: 1001\n")
+        expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "PrimaryGroupID", "1001").ordered.and_return(true)
         provider.dscl_set_gid
       end
 
       it "should raise an exception when the group does not exist" do
         shell_return = shellcmdresult.new("<dscl_cmd> DS Error: -14136 (eDSRecordNotFound)", "err", -14136)
-        expect(provider).to receive(:shell_out).with("dscl . -read /Groups/newgroup PrimaryGroupID").and_return(shell_return)
+        expect(provider).to receive(:shell_out).with("dscl", ".", "-read", "/Groups/newgroup", "PrimaryGroupID").and_return(shell_return)
         expect { provider.dscl_set_gid }.to raise_error(Chef::Exceptions::GroupIDNotFound)
       end
     end
 
     it "should set group ID to 20 if it's not specified" do
       new_resource.gid nil
-      expect(provider).to receive(:run_dscl).with("create /Users/toor PrimaryGroupID '20'").ordered.and_return(true)
+      expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "PrimaryGroupID", 20).ordered.and_return(true)
       provider.dscl_set_gid
       expect(new_resource.gid).to eq(20)
     end
@@ -850,8 +848,8 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30")
 
   describe "when the user exists" do
     before do
-      expect(provider).to receive(:shell_out).with("dscacheutil '-flushcache'")
-      expect(provider).to receive(:shell_out).with("plutil -convert xml1 -o - /var/db/dslocal/nodes/Default/users/toor.plist") do
+      expect(provider).to receive(:shell_out).with("dscacheutil", "-flushcache")
+      expect(provider).to receive(:shell_out).with("plutil", "-convert", "xml1", "-o", "-", "/var/db/dslocal/nodes/Default/users/toor.plist") do
         shellcmdresult.new(File.read(File.join(CHEF_SPEC_DATA, "mac_users/10.9.plist.xml")), "", 0)
       end
       provider.load_current_resource
@@ -859,14 +857,13 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30")
 
     describe "when Chef is removing the user" do
       it "removes the user from the groups and deletes home directory when the resource is configured to manage home" do
-        Chef::Config[:treat_deprecation_warnings_as_errors] = false
-        new_resource.supports({ :manage_home => true })
-        expect(provider).to receive(:run_dscl).with("list /Groups").and_return("my_group\nyour_group\nreal_group\n")
-        expect(provider).to receive(:run_dscl).with("read /Groups/my_group").and_raise(Chef::Exceptions::DsclCommandFailed) # Empty group
-        expect(provider).to receive(:run_dscl).with("read /Groups/your_group").and_return("GroupMembership: not_you")
-        expect(provider).to receive(:run_dscl).with("read /Groups/real_group").and_return("GroupMembership: toor")
-        expect(provider).to receive(:run_dscl).with("delete /Groups/real_group GroupMembership 'toor'")
-        expect(provider).to receive(:run_dscl).with("delete /Users/toor")
+        new_resource.manage_home true
+        expect(provider).to receive(:run_dscl).with("list", "/Groups").and_return("my_group\nyour_group\nreal_group\n")
+        expect(provider).to receive(:run_dscl).with("read", "/Groups/my_group").and_raise(Chef::Exceptions::DsclCommandFailed) # Empty group
+        expect(provider).to receive(:run_dscl).with("read", "/Groups/your_group").and_return("GroupMembership: not_you")
+        expect(provider).to receive(:run_dscl).with("read", "/Groups/real_group").and_return("GroupMembership: toor")
+        expect(provider).to receive(:run_dscl).with("delete", "/Groups/real_group", "GroupMembership", "toor")
+        expect(provider).to receive(:run_dscl).with("delete", "/Users/toor")
         expect(FileUtils).to receive(:rm_rf).with("/Users/vagrant")
         provider.remove_user
       end
@@ -889,7 +886,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30")
       end
 
       it "can unlock the user" do
-        expect(provider).to receive(:run_dscl).with("create /Users/toor AuthenticationAuthority ';ShadowHash;HASHLIST:<SALTED-SHA512-PBKDF2>'")
+        expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "AuthenticationAuthority", ";ShadowHash;HASHLIST:<SALTED-SHA512-PBKDF2>")
         provider.unlock_user
       end
     end
@@ -897,7 +894,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30")
 
   describe "when locking the user" do
     it "should run run_dscl with append /Users/user AuthenticationAuthority ;DisabledUser; to lock the user account" do
-      expect(provider).to receive(:run_dscl).with("append /Users/toor AuthenticationAuthority ';DisabledUser;'")
+      expect(provider).to receive(:run_dscl).with("append", "/Users/toor", "AuthenticationAuthority", ";DisabledUser;")
       provider.lock_user
     end
   end
diff --git a/spec/unit/provider/user/linux_spec.rb b/spec/unit/provider/user/linux_spec.rb
index ce9b84ec92..b04ada2511 100644
--- a/spec/unit/provider/user/linux_spec.rb
+++ b/spec/unit/provider/user/linux_spec.rb
@@ -55,7 +55,7 @@ describe Chef::Provider::User::Linux do
 
     # supports is a method on the superclass so can't totally be removed, but we should aggressively NOP it to decisively break it
     it "disables the supports API", chef: ">= 13" do
-      @new_resource.supports( { manage_home: true } )
+      @new_resource.supports( manage_home: true )
       expect( @new_resource.supports.key?(:manage_home) ).to be false
     end
 
@@ -70,13 +70,13 @@ describe Chef::Provider::User::Linux do
     it "throws a deprecation warning on setting supports[:non_unique]" do
       Chef::Config[:treat_deprecation_warnings_as_errors] = false
       expect(Chef).to receive(:deprecated).with(:supports_property, "supports { non_unique: true } on the user resource is deprecated and will be removed in Chef 13, set non_unique true instead")
-      @new_resource.supports( { :non_unique => true } )
+      @new_resource.supports( non_unique: true )
     end
 
     it "throws a deprecation warning on setting supports[:manage_home]" do
       Chef::Config[:treat_deprecation_warnings_as_errors] = false
       expect(Chef).to receive(:deprecated).with(:supports_property, "supports { manage_home: true } on the user resource is deprecated and will be removed in Chef 13, set manage_home true instead")
-      @new_resource.supports( { :manage_home => true } )
+      @new_resource.supports( manage_home: true )
     end
 
     it "defaults manage_home to false" do
@@ -85,13 +85,13 @@ describe Chef::Provider::User::Linux do
 
     it "supports[:manage_home] (incorectly) acts like manage_home" do
       Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      @new_resource.supports({ manage_home: true })
+      @new_resource.supports(manage_home: true)
       expect( provider.useradd_options ).to eql(["-m"])
     end
 
     it "supports[:manage_home] does not change behavior of manage_home: false", chef: ">= 13" do
       Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      @new_resource.supports({ manage_home: true })
+      @new_resource.supports(manage_home: true)
       expect( provider.useradd_options ).to eql(["-M"])
     end
 
diff --git a/spec/unit/provider/user/pw_spec.rb b/spec/unit/provider/user/pw_spec.rb
index fb7c9211a1..2f44d6f3e3 100644
--- a/spec/unit/provider/user/pw_spec.rb
+++ b/spec/unit/provider/user/pw_spec.rb
@@ -34,7 +34,7 @@ describe Chef::Provider::User::Pw do
 
     # XXX: rip out in Chef-13
     Chef::Config[:treat_deprecation_warnings_as_errors] = false
-    @new_resource.supports :manage_home => true
+    @new_resource.supports manage_home: true
 
     @current_resource = Chef::Resource::User::PwUser.new("adam")
     @current_resource.comment  "Adam Jacob"
@@ -65,35 +65,36 @@ describe Chef::Provider::User::Pw do
 
       it "should set the option for #{attribute} if the new resources #{attribute} is not null" do
         allow(@new_resource).to receive(attribute).and_return("hola")
-        expect(@provider.set_options).to eql(" #{@new_resource.username} #{option} '#{@new_resource.send(attribute)}' -m")
+        expect(@provider.set_options).to eql([ @new_resource.username, option, @new_resource.send(attribute), "-m"])
       end
 
       it "should set the option for #{attribute} if the new resources #{attribute} is not null, without homedir management" do
-        allow(@new_resource).to receive(:supports).and_return({ :manage_home => false })
+        allow(@new_resource).to receive(:supports).and_return(manage_home: false)
         allow(@new_resource).to receive(attribute).and_return("hola")
-        expect(@provider.set_options).to eql(" #{@new_resource.username} #{option} '#{@new_resource.send(attribute)}'")
+        expect(@provider.set_options).to eql([@new_resource.username, option, @new_resource.send(attribute)])
       end
     end
 
     it "should combine all the possible options" do
-      match_string = " adam"
+      match_array = [ "adam" ]
       field_list.sort { |a, b| a[0] <=> b[0] }.each do |attribute, option|
         allow(@new_resource).to receive(attribute).and_return("hola")
-        match_string << " #{option} 'hola'"
+        match_array << option
+        match_array << "hola"
       end
-      match_string << " -m"
-      expect(@provider.set_options).to eql(match_string)
+      match_array << "-m"
+      expect(@provider.set_options).to eql(match_array)
     end
   end
 
   describe "create_user" do
     before(:each) do
-      allow(@provider).to receive(:run_command).and_return(true)
+      allow(@provider).to receive(:shell_out!).and_return(true)
       allow(@provider).to receive(:modify_password).and_return(true)
     end
 
     it "should run pw useradd with the return of set_options" do
-      expect(@provider).to receive(:run_command).with({ :command => "pw useradd adam -m" }).and_return(true)
+      expect(@provider).to receive(:shell_out!).with("pw", "useradd", "adam", "-m").and_return(true)
       @provider.create_user
     end
 
@@ -105,12 +106,12 @@ describe Chef::Provider::User::Pw do
 
   describe "manage_user" do
     before(:each) do
-      allow(@provider).to receive(:run_command).and_return(true)
+      allow(@provider).to receive(:shell_out!).and_return(true)
       allow(@provider).to receive(:modify_password).and_return(true)
     end
 
     it "should run pw usermod with the return of set_options" do
-      expect(@provider).to receive(:run_command).with({ :command => "pw usermod adam -m" }).and_return(true)
+      expect(@provider).to receive(:shell_out!).with("pw", "usermod", "adam", "-m").and_return(true)
       @provider.manage_user
     end
 
@@ -122,13 +123,13 @@ describe Chef::Provider::User::Pw do
 
   describe "remove_user" do
     it "should run pw userdel with the new resources user name" do
-      @new_resource.supports :manage_home => false
-      expect(@provider).to receive(:run_command).with({ :command => "pw userdel #{@new_resource.username}" }).and_return(true)
+      @new_resource.supports manage_home: false
+      expect(@provider).to receive(:shell_out!).with("pw", "userdel", @new_resource.username).and_return(true)
       @provider.remove_user
     end
 
     it "should run pw userdel with the new resources user name and -r if manage_home is true" do
-      expect(@provider).to receive(:run_command).with({ :command => "pw userdel #{@new_resource.username} -r" }).and_return(true)
+      expect(@provider).to receive(:shell_out!).with("pw", "userdel", @new_resource.username, "-r").and_return(true)
       @provider.remove_user
     end
   end
@@ -147,23 +148,26 @@ describe Chef::Provider::User::Pw do
 
   describe "when locking the user" do
     it "should run pw lock with the new resources username" do
-      expect(@provider).to receive(:run_command).with({ :command => "pw lock #{@new_resource.username}" })
+      expect(@provider).to receive(:shell_out!).with("pw", "lock", @new_resource.username)
       @provider.lock_user
     end
   end
 
   describe "when unlocking the user" do
     it "should run pw unlock with the new resources username" do
-      expect(@provider).to receive(:run_command).with({ :command => "pw unlock #{@new_resource.username}" })
+      expect(@provider).to receive(:shell_out!).with("pw", "unlock", @new_resource.username)
       @provider.unlock_user
     end
   end
 
   describe "when modifying the password" do
     before(:each) do
-      @status = double("Status", :exitstatus => 0)
+      @status = double("Status", exitstatus: 0)
       allow(@provider).to receive(:popen4).and_return(@status)
-      @pid, @stdin, @stdout, @stderr = nil, nil, nil, nil
+      @pid = nil
+      @stdin = nil
+      @stdout = nil
+      @stderr = nil
     end
 
     describe "and the new password has not been specified" do
@@ -210,7 +214,7 @@ describe Chef::Provider::User::Pw do
       end
 
       it "should run pw usermod with the username and the option -H 0" do
-        expect(@provider).to receive(:popen4).with("pw usermod adam -H 0", :waitlast => true).and_return(@status)
+        expect(@provider).to receive(:popen4).with("pw usermod adam -H 0", waitlast: true).and_return(@status)
         @provider.modify_password
       end
 
@@ -239,12 +243,12 @@ describe Chef::Provider::User::Pw do
     end
 
     it "should raise an error if the required binary /usr/sbin/pw doesn't exist" do
-      expect(File).to receive(:exists?).with("/usr/sbin/pw").and_return(false)
+      expect(File).to receive(:exist?).with("/usr/sbin/pw").and_return(false)
       expect { @provider.load_current_resource }.to raise_error(Chef::Exceptions::User)
     end
 
     it "shouldn't raise an error if /usr/sbin/pw exists" do
-      allow(File).to receive(:exists?).and_return(true)
+      allow(File).to receive(:exist?).and_return(true)
       expect { @provider.load_current_resource }.not_to raise_error
     end
   end
diff --git a/spec/unit/provider/user/windows_spec.rb b/spec/unit/provider/user/windows_spec.rb
index 324b5f503f..82f3c1ab2a 100644
--- a/spec/unit/provider/user/windows_spec.rb
+++ b/spec/unit/provider/user/windows_spec.rb
@@ -127,19 +127,19 @@ describe Chef::Provider::User::Windows do
 
   describe "when creating the user" do
     it "should call @net_user.add with the return of set_options" do
-      allow(@provider).to receive(:set_options).and_return(:name => "monkey")
-      expect(@net_user).to receive(:add).with(:name => "monkey")
+      allow(@provider).to receive(:set_options).and_return(name: "monkey")
+      expect(@net_user).to receive(:add).with(name: "monkey")
       @provider.create_user
     end
   end
 
   describe "manage_user" do
     before(:each) do
-      allow(@provider).to receive(:set_options).and_return(:name => "monkey")
+      allow(@provider).to receive(:set_options).and_return(name: "monkey")
     end
 
     it "should call @net_user.update with the return of set_options" do
-      expect(@net_user).to receive(:update).with(:name => "monkey")
+      expect(@net_user).to receive(:update).with(name: "monkey")
       @provider.manage_user
     end
   end
diff --git a/spec/unit/provider/user_spec.rb b/spec/unit/provider/user_spec.rb
index 719dc8d492..489597445f 100644
--- a/spec/unit/provider/user_spec.rb
+++ b/spec/unit/provider/user_spec.rb
@@ -58,7 +58,7 @@ describe Chef::Provider::User do
   describe "executing load_current_resource" do
     before(:each) do
       @node = Chef::Node.new
-      #@new_resource = double("Chef::Resource::User",
+      # @new_resource = double("Chef::Resource::User",
       #  :null_object => true,
       #  :username => "adam",
       #  :comment => "Adam Jacob",
@@ -68,7 +68,7 @@ describe Chef::Provider::User do
       #  :shell => "/usr/bin/zsh",
       #  :password => nil,
       #  :updated => nil
-      #)
+      # )
       allow(Chef::Resource::User).to receive(:new).and_return(@current_resource)
       @pw_user = EtcPwnamIsh.new
       @pw_user.name = "adam"
@@ -110,11 +110,11 @@ describe Chef::Provider::User do
 
     # The mapping between the Chef::Resource::User and Getpwnam struct
     user_attrib_map = {
-      :uid => :uid,
-      :gid => :gid,
-      :comment => :gecos,
-      :home => :dir,
-      :shell => :shell,
+      uid: :uid,
+      gid: :gid,
+      comment: :gecos,
+      home: :dir,
+      shell: :shell,
     }
     user_attrib_map.each do |user_attrib, getpwnam_attrib|
       it "should set the current resources #{user_attrib} based on getpwnam #{getpwnam_attrib}" do
@@ -140,18 +140,16 @@ describe Chef::Provider::User do
 
     describe "and running assertions" do
       def self.shadow_lib_unavail?
-        begin
-          require "rubygems"
-          require "shadow"
-        rescue LoadError
-          skip "ruby-shadow gem not installed for dynamic load test"
-          true
-        else
-          false
-        end
+        require "rubygems"
+        require "shadow"
+      rescue LoadError
+        skip "ruby-shadow gem not installed for dynamic load test"
+        true
+      else
+        false
       end
 
-      before (:each) do
+      before(:each) do
         user = @pw_user.dup
         user.name = "root"
         user.passwd = "x"
@@ -161,15 +159,15 @@ describe Chef::Provider::User do
 
       unless shadow_lib_unavail?
         context "and we have the ruby-shadow gem" do
-          skip "and we are not root (rerun this again as root)", :requires_unprivileged_user => true
+          skip "and we are not root (rerun this again as root)", requires_unprivileged_user: true
 
-          context "and we are root", :requires_root => true do
+          context "and we are root", requires_root: true do
             it "should pass assertions when ruby-shadow can be loaded" do
               @provider.action = "create"
               original_method = @provider.method(:require)
               expect(@provider).to receive(:require) { |*args| original_method.call(*args) }
-              passwd_info = Struct::PasswdEntry.new(:sp_namp => "adm ", :sp_pwdp => "$1$T0N0Q.lc$nyG6pFI3Dpqa5cxUz/57j0", :sp_lstchg => 14861, :sp_min => 0, :sp_max => 99999,
-                                                    :sp_warn => 7, :sp_inact => -1, :sp_expire => -1, :sp_flag => -1)
+              passwd_info = Struct::PasswdEntry.new(sp_namp: "adm ", sp_pwdp: "$1$T0N0Q.lc$nyG6pFI3Dpqa5cxUz/57j0", sp_lstchg: 14861, sp_min: 0, sp_max: 99999,
+                                                    sp_warn: 7, sp_inact: -1, sp_expire: -1, sp_flag: -1)
               expect(Shadow::Passwd).to receive(:getspnam).with("adam").and_return(passwd_info)
               @provider.load_current_resource
               @provider.define_resource_requirements