Resolve our chefstyle warningsredundant_begins

Now that knife has been moved we can resolve the current warnings

Signed-off-by: Tim Smith <tsmith@chef.io>

19 files changed, 127 insertions, 174 deletions

diff --git a/knife/lib/chef/knife.rb b/knife/lib/chef/knife.rb
index 67f5f7d54d..e5f8c36e32 100644
--- a/knife/lib/chef/knife.rb
+++ b/knife/lib/chef/knife.rb
@@ -642,9 +642,7 @@ class Chef
     end
 
     def rest
-      @rest ||= begin
-        Chef::ServerAPI.new(Chef::Config[:chef_server_url])
-      end
+      @rest ||= Chef::ServerAPI.new(Chef::Config[:chef_server_url])
     end
 
     def noauth_rest
diff --git a/knife/lib/chef/knife/bootstrap/chef_vault_handler.rb b/knife/lib/chef/knife/bootstrap/chef_vault_handler.rb
index 20759d6fdf..d3fff12eec 100644
--- a/knife/lib/chef/knife/bootstrap/chef_vault_handler.rb
+++ b/knife/lib/chef/knife/bootstrap/chef_vault_handler.rb
@@ -108,13 +108,11 @@ class Chef
         # @return [Hash] deserialized ruby hash with all the vault items
         def vault_json
           @vault_json ||=
-            begin
-              if bootstrap_vault_item
-                bootstrap_vault_item
-              else
-                json = bootstrap_vault_json || File.read(bootstrap_vault_file)
-                Chef::JSONCompat.from_json(json)
-              end
+            if bootstrap_vault_item
+              bootstrap_vault_item
+            else
+              json = bootstrap_vault_json || File.read(bootstrap_vault_file)
+              Chef::JSONCompat.from_json(json)
             end
         end
 
diff --git a/knife/lib/chef/knife/bootstrap/train_connector.rb b/knife/lib/chef/knife/bootstrap/train_connector.rb
index a220ece5bc..f155e32225 100644
--- a/knife/lib/chef/knife/bootstrap/train_connector.rb
+++ b/knife/lib/chef/knife/bootstrap/train_connector.rb
@@ -115,24 +115,22 @@ class Chef
         #
         # @return [String] the temporary path created on the remote host.
         def temp_dir
-          @tmpdir ||= begin
-            if windows?
-              run_command!(MKTEMP_WIN_COMMAND).stdout.split.last
-            else
-              # Get a 6 chars string using secure random
-              # eg. /tmp/chef_XXXXXX.
-              # Use mkdir to create TEMP dir to get rid of mktemp
-              dir = "#{DEFAULT_REMOTE_TEMP}/chef_#{SecureRandom.alphanumeric(6)}"
-              run_command!("mkdir -p '#{dir}'")
-              # Ensure that dir has the correct owner.  We are possibly
-              # running with sudo right now - so this directory would be owned by root.
-              # File upload is performed over SCP as the current logged-in user,
-              # so we'll set ownership to ensure that works.
-              run_command!("chown #{config[:user]} '#{dir}'") if config[:sudo]
-
-              dir
-            end
-          end
+          @tmpdir ||= if windows?
+                        run_command!(MKTEMP_WIN_COMMAND).stdout.split.last
+                      else
+                        # Get a 6 chars string using secure random
+                        # eg. /tmp/chef_XXXXXX.
+                        # Use mkdir to create TEMP dir to get rid of mktemp
+                        dir = "#{DEFAULT_REMOTE_TEMP}/chef_#{SecureRandom.alphanumeric(6)}"
+                        run_command!("mkdir -p '#{dir}'")
+                        # Ensure that dir has the correct owner.  We are possibly
+                        # running with sudo right now - so this directory would be owned by root.
+                        # File upload is performed over SCP as the current logged-in user,
+                        # so we'll set ownership to ensure that works.
+                        run_command!("chown #{config[:user]} '#{dir}'") if config[:sudo]
+
+                        dir
+                      end
         end
 
         #
diff --git a/knife/lib/chef/knife/supermarket_download.rb b/knife/lib/chef/knife/supermarket_download.rb
index 5acd733b78..88948f131d 100644
--- a/knife/lib/chef/knife/supermarket_download.rb
+++ b/knife/lib/chef/knife/supermarket_download.rb
@@ -75,9 +75,7 @@ class Chef
       end
 
       def current_cookbook_data
-        @current_cookbook_data ||= begin
-                                     noauth_rest.get "#{cookbooks_api_url}/#{@name_args[0]}"
-                                   end
+        @current_cookbook_data ||= noauth_rest.get "#{cookbooks_api_url}/#{@name_args[0]}"
       end
 
       def current_cookbook_deprecated?
diff --git a/lib/chef/cookbook/synchronizer.rb b/lib/chef/cookbook/synchronizer.rb
index 53e874d0e8..e0ce5d7f41 100644
--- a/lib/chef/cookbook/synchronizer.rb
+++ b/lib/chef/cookbook/synchronizer.rb
@@ -143,11 +143,9 @@ class Chef
     end
 
     def files_remaining_by_cookbook
-      @files_remaining_by_cookbook ||= begin
-        files_by_cookbook.inject({}) do |memo, (cookbook, files)|
-          memo[cookbook] = files.size
-          memo
-        end
+      @files_remaining_by_cookbook ||= files_by_cookbook.inject({}) do |memo, (cookbook, files)|
+        memo[cookbook] = files.size
+        memo
       end
     end
 
diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb
index f7efb2646e..c1d35f7d7e 100644
--- a/lib/chef/cookbook_loader.rb
+++ b/lib/chef/cookbook_loader.rb
@@ -195,10 +195,8 @@ class Chef
 
     def all_files_in_repo_paths
       @all_files_in_repo_paths ||=
-        begin
-          repo_paths.inject([]) do |all_children, repo_path|
-            all_children + Dir[File.join(Chef::Util::PathHelper.escape_glob_dir(repo_path), "*")]
-          end
+        repo_paths.inject([]) do |all_children, repo_path|
+          all_children + Dir[File.join(Chef::Util::PathHelper.escape_glob_dir(repo_path), "*")]
         end
     end
 
diff --git a/lib/chef/formatters/error_inspectors/resource_failure_inspector.rb b/lib/chef/formatters/error_inspectors/resource_failure_inspector.rb
index 8ddae24e6d..4010bd72d5 100644
--- a/lib/chef/formatters/error_inspectors/resource_failure_inspector.rb
+++ b/lib/chef/formatters/error_inspectors/resource_failure_inspector.rb
@@ -64,36 +64,34 @@ class Chef
         def recipe_snippet
           return nil if dynamic_resource?
 
-          @snippet ||= begin
-            if (file = parse_source) && (line = parse_line(file))
-              return nil unless ::File.exist?(file)
+          @snippet ||= if (file = parse_source) && (line = parse_line(file))
+                         return nil unless ::File.exist?(file)
 
-              lines = IO.readlines(file)
+                         lines = IO.readlines(file)
 
-              relevant_lines = ["# In #{file}\n\n"]
+                         relevant_lines = ["# In #{file}\n\n"]
 
-              current_line = line - 1
-              current_line = 0 if current_line < 0
-              nesting = 0
+                         current_line = line - 1
+                         current_line = 0 if current_line < 0
+                         nesting = 0
 
-              loop do
+                         loop do
 
-                # low rent parser. try to gracefully handle nested blocks in resources
-                nesting += 1 if /\s+do\s*/.match?(lines[current_line])
-                nesting -= 1 if /end\s*$/.match?(lines[current_line])
+                           # low rent parser. try to gracefully handle nested blocks in resources
+                           nesting += 1 if /\s+do\s*/.match?(lines[current_line])
+                           nesting -= 1 if /end\s*$/.match?(lines[current_line])
 
-                relevant_lines << format_line(current_line, lines[current_line])
+                           relevant_lines << format_line(current_line, lines[current_line])
 
-                break if lines[current_line + 1].nil?
-                break if current_line >= (line + 50)
-                break if nesting <= 0
+                           break if lines[current_line + 1].nil?
+                           break if current_line >= (line + 50)
+                           break if nesting <= 0
 
-                current_line += 1
-              end
-              relevant_lines << format_line(current_line + 1, lines[current_line + 1]) if lines[current_line + 1]
-              relevant_lines.join("")
-            end
-          end
+                           current_line += 1
+                         end
+                         relevant_lines << format_line(current_line + 1, lines[current_line + 1]) if lines[current_line + 1]
+                         relevant_lines.join("")
+                       end
         end
 
         def dynamic_resource?
diff --git a/lib/chef/provider/git.rb b/lib/chef/provider/git.rb
index 568a56d8fb..1c0b9f562e 100644
--- a/lib/chef/provider/git.rb
+++ b/lib/chef/provider/git.rb
@@ -281,12 +281,10 @@ class Chef
 
       def target_revision
         @target_revision ||=
-          begin
-            if sha_hash?(new_resource.revision)
-              @target_revision = new_resource.revision
-            else
-              @target_revision = remote_resolve_reference
-            end
+          if sha_hash?(new_resource.revision)
+            @target_revision = new_resource.revision
+          else
+            @target_revision = remote_resolve_reference
           end
       end
 
diff --git a/lib/chef/provider/package.rb b/lib/chef/provider/package.rb
index 9c2ee4bb3f..65d2254258 100644
--- a/lib/chef/provider/package.rb
+++ b/lib/chef/provider/package.rb
@@ -598,12 +598,10 @@ class Chef
       # @return [Array] new_resource.source as an array
       def source_array
         @source_array ||=
-          begin
-            if new_resource.source.nil?
-              package_name_array.map { nil }
-            else
-              [ new_resource.source ].flatten
-            end
+          if new_resource.source.nil?
+            package_name_array.map { nil }
+          else
+            [ new_resource.source ].flatten
           end
       end
 
@@ -612,16 +610,14 @@ class Chef
       # @return [Array] Array of sources with package_names converted to sources
       def resolved_source_array
         @resolved_source_array ||=
-          begin
-            source_array.each_with_index.map do |source, i|
-              package_name = package_name_array[i]
-              # we require at least one '/' in the package_name to avoid [XXX_]package 'foo' breaking due to a random 'foo' file in cwd
-              if use_package_name_for_source? && source.nil? && package_name.match(/#{::File::SEPARATOR}/) && ::File.exist?(package_name)
-                logger.trace("No package source specified, but #{package_name} exists on filesystem, using #{package_name} as source.")
-                package_name
-              else
-                source
-              end
+          source_array.each_with_index.map do |source, i|
+            package_name = package_name_array[i]
+            # we require at least one '/' in the package_name to avoid [XXX_]package 'foo' breaking due to a random 'foo' file in cwd
+            if use_package_name_for_source? && source.nil? && package_name.match(/#{::File::SEPARATOR}/) && ::File.exist?(package_name)
+              logger.trace("No package source specified, but #{package_name} exists on filesystem, using #{package_name} as source.")
+              package_name
+            else
+              source
             end
           end
       end
diff --git a/lib/chef/provider/package/rubygems.rb b/lib/chef/provider/package/rubygems.rb
index e427cc0d24..be143d80dd 100644
--- a/lib/chef/provider/package/rubygems.rb
+++ b/lib/chef/provider/package/rubygems.rb
@@ -479,9 +479,7 @@ class Chef
         end
 
         def all_installed_versions
-          @all_installed_versions ||= begin
-                                        @gem_env.installed_versions(Gem::Dependency.new(gem_dependency.name, ">= 0"))
-                                      end
+          @all_installed_versions ||= @gem_env.installed_versions(Gem::Dependency.new(gem_dependency.name, ">= 0"))
         end
 
         ##
@@ -521,13 +519,11 @@ class Chef
         end
 
         def candidate_version
-          @candidate_version ||= begin
-                                  if source_is_remote?
-                                    @gem_env.candidate_version_from_remote(gem_dependency, *gem_sources).to_s
-                                  else
-                                    @gem_env.candidate_version_from_file(gem_dependency, new_resource.source).to_s
-                                  end
-                                end
+          @candidate_version ||= if source_is_remote?
+                                   @gem_env.candidate_version_from_remote(gem_dependency, *gem_sources).to_s
+                                 else
+                                   @gem_env.candidate_version_from_file(gem_dependency, new_resource.source).to_s
+                                 end
         end
 
         def version_requirement_satisfied?(current_version, new_version)
diff --git a/lib/chef/provider/package/windows.rb b/lib/chef/provider/package/windows.rb
index c722d8222c..4350eb6d12 100644
--- a/lib/chef/provider/package/windows.rb
+++ b/lib/chef/provider/package/windows.rb
@@ -70,8 +70,7 @@ class Chef
         end
 
         def package_provider
-          @package_provider ||= begin
-            case installer_type
+          @package_provider ||= case installer_type
             when :msi
               logger.trace("#{new_resource} is MSI")
               require_relative "windows/msi"
@@ -80,8 +79,7 @@ class Chef
               logger.trace("#{new_resource} is EXE with type '#{installer_type}'")
               require_relative "windows/exe"
               Chef::Provider::Package::Windows::Exe.new(resource_for_provider, installer_type, uninstall_registry_entries)
-            end
-          end
+                                end
         end
 
         def installer_type
diff --git a/lib/chef/provider/service/freebsd.rb b/lib/chef/provider/service/freebsd.rb
index 648dc40bf6..9c1cdba39f 100644
--- a/lib/chef/provider/service/freebsd.rb
+++ b/lib/chef/provider/service/freebsd.rb
@@ -130,27 +130,21 @@ class Chef
         # The variable name used in /etc/rc.conf for enabling this service
         def service_enable_variable_name
           @service_enable_variable_name ||=
-            begin
-              # Look for name="foo" in the shell script @init_command. Use this for determining the variable name in /etc/rc.conf
-              # corresponding to this service
-              # For example: to enable the service mysql-server with the init command /usr/local/etc/rc.d/mysql-server, you need
-              # to set mysql_enable="YES" in /etc/rc.conf$
-              if init_command
-                ::File.open(init_command) do |rcscript|
-                  rcscript.each_line do |line|
-                    if line =~ /^name="?(\w+)"?/
-                      return $1 + "_enable"
-                    end
+            if init_command
+              ::File.open(init_command) do |rcscript|
+                rcscript.each_line do |line|
+                  if line =~ /^name="?(\w+)"?/
+                    return $1 + "_enable"
                   end
                 end
-                # some scripts support multiple instances through symlinks such as openvpn.
-                # We should get the service name from rcvar.
-                logger.trace("name=\"service\" not found at #{init_command}. falling back to rcvar")
-                shell_out!("#{init_command} rcvar").stdout[/(\w+_enable)=/, 1]
-              else
-                # for why-run mode when the rcd_script is not there yet
-                new_resource.service_name
               end
+              # some scripts support multiple instances through symlinks such as openvpn.
+              # We should get the service name from rcvar.
+              logger.trace("name=\"service\" not found at #{init_command}. falling back to rcvar")
+              shell_out!("#{init_command} rcvar").stdout[/(\w+_enable)=/, 1]
+            else
+              # for why-run mode when the rcd_script is not there yet
+              new_resource.service_name
             end
         end
 
diff --git a/lib/chef/provider/subversion.rb b/lib/chef/provider/subversion.rb
index 2cc4cb7ab3..7f49e2139f 100644
--- a/lib/chef/provider/subversion.rb
+++ b/lib/chef/provider/subversion.rb
@@ -121,16 +121,14 @@ class Chef
       # up the revision id by asking the server
       # If the specified revision is an integer, trust it.
       def revision_int
-        @revision_int ||= begin
-          if /^\d+$/.match?(new_resource.revision)
-            new_resource.revision
-          else
-            command = scm(:info, new_resource.repository, new_resource.svn_info_args, authentication, "-r#{new_resource.revision}")
-            svn_info = shell_out!(command, run_options(cwd: cwd, returns: [0, 1])).stdout
-
-            extract_revision_info(svn_info)
-          end
-        end
+        @revision_int ||= if /^\d+$/.match?(new_resource.revision)
+                            new_resource.revision
+                          else
+                            command = scm(:info, new_resource.repository, new_resource.svn_info_args, authentication, "-r#{new_resource.revision}")
+                            svn_info = shell_out!(command, run_options(cwd: cwd, returns: [0, 1])).stdout
+
+                            extract_revision_info(svn_info)
+                          end
       end
 
       alias :revision_slug :revision_int
diff --git a/lib/chef/provider/template/content.rb b/lib/chef/provider/template/content.rb
index a0977b5523..fc8f30a7dc 100644
--- a/lib/chef/provider/template/content.rb
+++ b/lib/chef/provider/template/content.rb
@@ -29,9 +29,7 @@ class Chef
         include Chef::Mixin::Template
 
         def template_location
-          @template_file_cache_location ||= begin
-            template_finder.find(new_resource.source, local: new_resource.local, cookbook: new_resource.cookbook)
-          end
+          @template_file_cache_location ||= template_finder.find(new_resource.source, local: new_resource.local, cookbook: new_resource.cookbook)
         end
 
         private
@@ -84,9 +82,7 @@ class Chef
         end
 
         def template_finder
-          @template_finder ||= begin
-            TemplateFinder.new(run_context, new_resource.cookbook_name, run_context.node)
-          end
+          @template_finder ||= TemplateFinder.new(run_context, new_resource.cookbook_name, run_context.node)
         end
       end
     end
diff --git a/lib/chef/provider/user/mac.rb b/lib/chef/provider/user/mac.rb
index 573d28644b..94b0ce0b21 100644
--- a/lib/chef/provider/user/mac.rb
+++ b/lib/chef/provider/user/mac.rb
@@ -393,23 +393,21 @@ class Chef
         # associated group resource. If a group exists we'll modify it, otherwise
         # create it.
         def user_group_info
-          @user_group_info ||= begin
-            if new_resource.gid.is_a?(String)
-              begin
-                g = Etc.getgrnam(new_resource.gid)
-                [g.name, g.gid.to_s, :modify]
-              rescue
-                [new_resource.gid, nil, :create]
-              end
-            else
-              begin
-                g = Etc.getgrgid(new_resource.gid)
-                [g.name, g.gid.to_s, :modify]
-              rescue
-                [g.username, nil, :create]
-              end
-            end
-          end
+          @user_group_info ||= if new_resource.gid.is_a?(String)
+                                 begin
+                                   g = Etc.getgrnam(new_resource.gid)
+                                   [g.name, g.gid.to_s, :modify]
+                                 rescue
+                                   [new_resource.gid, nil, :create]
+                                 end
+                               else
+                                 begin
+                                   g = Etc.getgrgid(new_resource.gid)
+                                   [g.name, g.gid.to_s, :modify]
+                                 rescue
+                                   [g.username, nil, :create]
+                                 end
+                               end
         end
 
         def secure_token_enabled?
diff --git a/lib/chef/resource/dmg_package.rb b/lib/chef/resource/dmg_package.rb
index f294515895..9484de7fe8 100644
--- a/lib/chef/resource/dmg_package.rb
+++ b/lib/chef/resource/dmg_package.rb
@@ -169,13 +169,11 @@ class Chef
       action_class do
         # @return [String] the path to the dmg file
         def dmg_file
-          @dmg_file ||= begin
-            if new_resource.file.nil?
-              "#{Chef::Config[:file_cache_path]}/#{new_resource.dmg_name}.dmg"
-            else
-              new_resource.file
-            end
-          end
+          @dmg_file ||= if new_resource.file.nil?
+                          "#{Chef::Config[:file_cache_path]}/#{new_resource.dmg_name}.dmg"
+                        else
+                          new_resource.file
+                        end
         end
 
         # @return [String] the hdiutil flag for handling DMGs with a password
diff --git a/lib/chef/resource/windows_share.rb b/lib/chef/resource/windows_share.rb
index 0984fab924..4b9cfc483e 100644
--- a/lib/chef/resource/windows_share.rb
+++ b/lib/chef/resource/windows_share.rb
@@ -271,14 +271,11 @@ class Chef
         # users/groups will have their permissions updated with the same command that
         # sets it, but removes must be performed with Revoke-SmbShareAccess
         def users_to_revoke
-          @users_to_revoke ||= begin
-            # if the resource doesn't exist then nothing needs to be revoked
-            if current_resource.nil?
-              []
-            else # if it exists then calculate the current to new resource diffs
-              (current_resource.full_users + current_resource.change_users + current_resource.read_users) - (new_resource.full_users + new_resource.change_users + new_resource.read_users)
-            end
-          end
+          @users_to_revoke ||= if current_resource.nil?
+                                 []
+                               else # if it exists then calculate the current to new resource diffs
+                                 (current_resource.full_users + current_resource.change_users + current_resource.read_users) - (new_resource.full_users + new_resource.change_users + new_resource.read_users)
+                               end
         end
 
         # update existing permissions on a share
diff --git a/spec/support/matchers/leak.rb b/spec/support/matchers/leak.rb
index bd59a2755c..89d238c88e 100644
--- a/spec/support/matchers/leak.rb
+++ b/spec/support/matchers/leak.rb
@@ -59,15 +59,13 @@ module Matchers
     end
 
     def profiler
-      @profiler ||= begin
-        if ChefUtils.windows?
-          require File.join(__dir__, "..", "platforms", "prof", "win32")
-          RSpec::Prof::Win32::Profiler.new
-        else
-          require File.join(__dir__, "..", "prof", "gc")
-          RSpec::Prof::GC::Profiler.new
-        end
-      end
+      @profiler ||= if ChefUtils.windows?
+                      require File.join(__dir__, "..", "platforms", "prof", "win32")
+                      RSpec::Prof::Win32::Profiler.new
+                    else
+                      require File.join(__dir__, "..", "prof", "gc")
+                      RSpec::Prof::GC::Profiler.new
+                    end
     end
 
   end
diff --git a/spec/unit/knife/core/gem_glob_loader_spec.rb b/spec/unit/knife/core/gem_glob_loader_spec.rb
index a6a94cc57a..072dac3986 100644
--- a/spec/unit/knife/core/gem_glob_loader_spec.rb
+++ b/spec/unit/knife/core/gem_glob_loader_spec.rb
@@ -71,7 +71,7 @@ describe Chef::Knife::SubcommandLoader::GemGlobLoader do
     ]
     expected_files = [
       "/usr/lib/ruby/gems/knife-ec2-0.5.12/lib/chef/knife/ec2_base.rb",
-      "/usr/lib/ruby/gems/knife-ec2-0.5.12/lib/chef/knife/ec2_otherstuff.rb"
+      "/usr/lib/ruby/gems/knife-ec2-0.5.12/lib/chef/knife/ec2_otherstuff.rb",
     ]
 
     expect($LOAD_PATH).to receive(:map).and_return([])