Merge pull request #9186 from mhashizume/PUP-11768/main/layout-cops

(PUP-11768) Enable and fix Rubocop Layout Cops
puppetlabs · Dec 19, 2023 · 3e8e5c7 · 3e8e5c7
2 parents 3582a39 + c528ea3
commit 3e8e5c7
Show file tree

Hide file tree

Showing 92 changed files with 717 additions and 782 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -22,6 +22,12 @@ Layout/ArgumentAlignment:
   Exclude:
     - 'lib/puppet/defaults.rb'
 
+# The confine statements in package providers unexpectedly affect this cop.
+Layout/BeginEndAlignment:
+  Enabled: true
+  Exclude:
+    - 'lib/puppet/provider/package/*.rb'
+
 # puppet uses symbol booleans in types and providers to work around long standing
 # bugs when trying to manage falsey pararameters and properties
 Lint/BooleanSymbol:

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -22,87 +22,6 @@ I18n/GetText/DecorateStringFormattingUsingPercent:
 I18n/RailsI18n/DecorateString:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyle, IndentationWidth.
-# SupportedStyles: outdent, indent
-Layout/AccessModifierIndentation:
-  Exclude:
-    - 'lib/puppet/module_tool/install_directory.rb'
-    - 'lib/puppet/pops/evaluator/collector_transformer.rb'
-    - 'lib/puppet/provider/service/init.rb'
-    - 'lib/puppet/provider/service/upstart.rb'
-    - 'lib/puppet/settings/config_file.rb'
-    - 'lib/puppet/settings/file_setting.rb'
-    - 'lib/puppet/util/command_line/trollop.rb'
-    - 'lib/puppet/util/run_mode.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyle, IndentationWidth.
-# SupportedStyles: with_first_element, with_fixed_indentation
-Layout/ArrayAlignment:
-  Exclude:
-    - 'lib/puppet/module/plan.rb'
-    - 'lib/puppet/pops/model/model_tree_dumper.rb'
-    - 'lib/puppet/pops/parser/heredoc_support.rb'
-    - 'lib/puppet/pops/parser/lexer2.rb'
-    - 'lib/puppet/provider/service/init.rb'
-    - 'lib/puppet/util/log/destinations.rb'
-    - 'lib/puppet/util/posix.rb'
-    - 'lib/puppet/util/windows/registry.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: IndentationWidth.
-Layout/AssignmentIndentation:
-  Exclude:
-    - 'lib/puppet/application/apply.rb'
-    - 'lib/puppet/ffi/windows/constants.rb'
-    - 'lib/puppet/indirector/request.rb'
-    - 'lib/puppet/parser/e4_parser_adapter.rb'
-    - 'lib/puppet/pops/evaluator/evaluator_impl.rb'
-    - 'lib/puppet/pops/model/model_tree_dumper.rb'
-    - 'lib/puppet/pops/parser/lexer_support.rb'
-    - 'lib/puppet/pops/types/type_parser.rb'
-    - 'lib/puppet/resource/type.rb'
-    - 'lib/puppet/settings.rb'
-    - 'lib/puppet/settings/environment_conf.rb'
-    - 'lib/puppet/type/resources.rb'
-    - 'lib/puppet/util.rb'
-    - 'lib/puppet/util/windows/daemon.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyleAlignWith, Severity.
-# SupportedStylesAlignWith: start_of_line, begin
-Layout/BeginEndAlignment:
-  Exclude:
-    - 'lib/puppet/provider/package/apt.rb'
-    - 'lib/puppet/provider/package/aptrpm.rb'
-    - 'lib/puppet/provider/package/dnf.rb'
-    - 'lib/puppet/provider/package/rpm.rb'
-    - 'lib/puppet/provider/package/tdnf.rb'
-    - 'lib/puppet/provider/package/yum.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyleAlignWith.
-# SupportedStylesAlignWith: either, start_of_block, start_of_line
-Layout/BlockAlignment:
-  Exclude:
-    - 'lib/puppet/defaults.rb'
-    - 'lib/puppet/node/environment.rb'
-    - 'lib/puppet/parser/functions/lookup.rb'
-    - 'lib/puppet/provider/package/dnfmodule.rb'
-    - 'lib/puppet/type/file.rb'
-    - 'lib/puppet/util/windows/principal.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyle, IndentOneStep, IndentationWidth.
-# SupportedStyles: case, end
-Layout/CaseIndentation:
-  Enabled: false
-
-# This cop supports safe auto-correction (--auto-correct).
-Layout/ClosingHeredocIndentation:
-  Enabled: false
-
 # This cop supports safe auto-correction (--auto-correct).
 Layout/ClosingParenthesisIndentation:
   Enabled: false

diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
@@ -234,12 +234,12 @@ def main
         # rule and gets logged.
         #
         catalog =
-        begin
-          Puppet::Resource::Catalog.indirection.find(node.name, :use_node => node)
-        rescue Puppet::Error
-          # already logged and handled by the compiler, including Puppet::ParseErrorWithIssue
-          exit(1)
-        end
+          begin
+            Puppet::Resource::Catalog.indirection.find(node.name, :use_node => node)
+          rescue Puppet::Error
+            # already logged and handled by the compiler, including Puppet::ParseErrorWithIssue
+            exit(1)
+          end
 
         # Resolve all deferred values and replace them / mutate the catalog
         Puppet::Pops::Evaluator::DeferredResolver.resolve_and_replace(node.facts, catalog, apply_environment, Puppet[:preprocess_deferred])

diff --git a/lib/puppet/application/doc.rb b/lib/puppet/application/doc.rb
@@ -112,7 +112,7 @@ module - see https://github.com/puppetlabs/puppetlabs-strings for more informati
 ---------
 Copyright (c) 2011 Puppet Inc., LLC Licensed under the Apache 2.0 License
 
-HELP
+    HELP
   end
 
   def handle_unknown( opt, arg )

diff --git a/lib/puppet/application/ssl.rb b/lib/puppet/application/ssl.rb
@@ -83,7 +83,7 @@ def help
 
  * show:
   Print the full-text version of this host's certificate.
-HELP
+    HELP
   end
 
   option('--target CERTNAME') do |arg|

diff --git a/lib/puppet/datatypes/error.rb b/lib/puppet/datatypes/error.rb
@@ -14,7 +14,7 @@
     functions => {
       message => Callable[[], String[1]]
     }
-    PUPPET
+  PUPPET
 
   require_relative '../../puppet/datatypes/impl/error'
 

diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
@@ -430,7 +430,7 @@ def self.initialize_default_settings!(settings)
       between the configured or resolved path.
 
       When set to false, the resolved paths are reported instead of the configured paths.
-    EOT
+      EOT
     },
     :use_last_environment => {
       :type     => :boolean,
@@ -441,7 +441,7 @@ def self.initialize_default_settings!(settings)
       environment and skip the node request.
 
       When set to false, we will do the node request and ignore the environment data from the last_run_summary file.
-    EOT
+      EOT
     },
     :always_retry_plugins => {
         :type     => :boolean,
@@ -462,7 +462,7 @@ def self.initialize_default_settings!(settings)
         feature.  This behavior is almost always appropriate for the server,
         and can result in a significant performance improvement for types and
         features that are checked frequently.
-      EOT
+        EOT
     },
     :diff_args => {
         :default  => lambda { default_diffargs },
@@ -493,10 +493,10 @@ def self.initialize_default_settings!(settings)
           currently cannot daemonize).",
         :short    => "D",
         :hook     => proc do |value|
-          if value and Puppet::Util::Platform.windows?
-            raise "Cannot daemonize on Windows"
-          end
-      end
+                       if value and Puppet::Util::Platform.windows?
+                         raise "Cannot daemonize on Windows"
+                       end
+                     end
     },
     :maximum_uid => {
         :default  => 4294967290,
@@ -709,7 +709,7 @@ def self.initialize_default_settings!(settings)
 releases this value will only determine if file content is cached.
 
 Valid values are 0 (never cache) and 15 (15 second minimum wait time).
-            WARNING
+          WARNING
         end
       end
     },

diff --git a/lib/puppet/ffi/windows/constants.rb b/lib/puppet/ffi/windows/constants.rb
@@ -47,25 +47,25 @@ module Constants
     FILE_ALL_ACCESS = STANDARD_RIGHTS_REQUIRED | SYNCHRONIZE | 0x1FF
 
     FILE_GENERIC_READ =
-       STANDARD_RIGHTS_READ |
-       FILE_READ_DATA |
-       FILE_READ_ATTRIBUTES |
-       FILE_READ_EA |
-       SYNCHRONIZE
+      STANDARD_RIGHTS_READ |
+      FILE_READ_DATA |
+      FILE_READ_ATTRIBUTES |
+      FILE_READ_EA |
+      SYNCHRONIZE
 
     FILE_GENERIC_WRITE =
-       STANDARD_RIGHTS_WRITE |
-       FILE_WRITE_DATA |
-       FILE_WRITE_ATTRIBUTES |
-       FILE_WRITE_EA |
-       FILE_APPEND_DATA |
-       SYNCHRONIZE
+      STANDARD_RIGHTS_WRITE |
+      FILE_WRITE_DATA |
+      FILE_WRITE_ATTRIBUTES |
+      FILE_WRITE_EA |
+      FILE_APPEND_DATA |
+      SYNCHRONIZE
 
     FILE_GENERIC_EXECUTE =
-       STANDARD_RIGHTS_EXECUTE |
-       FILE_READ_ATTRIBUTES |
-       FILE_EXECUTE |
-       SYNCHRONIZE
+      STANDARD_RIGHTS_EXECUTE |
+      FILE_READ_ATTRIBUTES |
+      FILE_EXECUTE |
+      SYNCHRONIZE
 
     REPLACEFILE_WRITE_THROUGH         = 0x1
     REPLACEFILE_IGNORE_MERGE_ERRORS   = 0x2

diff --git a/lib/puppet/file_system/uniquefile.rb b/lib/puppet/file_system/uniquefile.rb
@@ -100,14 +100,14 @@ def path
 
   def make_tmpname(prefix_suffix, n)
     case prefix_suffix
-      when String
-        prefix = prefix_suffix
-        suffix = ""
-      when Array
-        prefix = prefix_suffix[0]
-        suffix = prefix_suffix[1]
-      else
-        raise ArgumentError, _("unexpected prefix_suffix: %{value}") % { value: prefix_suffix.inspect }
+    when String
+      prefix = prefix_suffix
+      suffix = ""
+    when Array
+      prefix = prefix_suffix[0]
+      suffix = prefix_suffix[1]
+    else
+      raise ArgumentError, _("unexpected prefix_suffix: %{value}") % { value: prefix_suffix.inspect }
     end
     t = Time.now.strftime("%Y%m%d")
     path = "#{prefix}#{t}-#{$$}-#{rand(0x100000000).to_s(36)}"

diff --git a/lib/puppet/http/dns.rb b/lib/puppet/http/dns.rb
@@ -49,9 +49,9 @@ def each_srv_record(domain, service_name = :puppet, &block)
       Puppet.debug "Searching for SRV records for domain: #{domain}"
 
       case service_name
-        when :puppet then service = '_x-puppet'
-        when :file   then service = '_x-puppet-fileserver'
-        else              service = "_x-puppet-#{service_name}"
+      when :puppet then service = '_x-puppet'
+      when :file   then service = '_x-puppet-fileserver'
+      else              service = "_x-puppet-#{service_name}"
       end
       record_name = "#{service}._tcp.#{domain}"
 

diff --git a/lib/puppet/indirector/catalog/compiler.rb b/lib/puppet/indirector/catalog/compiler.rb
@@ -158,17 +158,17 @@ def get_content_uri(metadata, source, environment_path)
   # Also used to profile/log reasons for not inlining.
   def inlineable?(resource, sources)
     case
-      when resource[:ensure] == 'absent'
-        #TRANSLATORS Inlining refers to adding additional metadata (in this case we are not inlining)
-        return Puppet::Util::Profiler.profile(_("Not inlining absent resource"), [:compiler, :static_compile_inlining, :skipped_file_metadata, :absent]) { false }
-      when sources.empty?
-        #TRANSLATORS Inlining refers to adding additional metadata (in this case we are not inlining)
-        return Puppet::Util::Profiler.profile(_("Not inlining resource without sources"), [:compiler, :static_compile_inlining, :skipped_file_metadata, :no_sources]) { false }
-      when (not (sources.all? {|source| source =~ /^puppet:/}))
-        #TRANSLATORS Inlining refers to adding additional metadata (in this case we are not inlining)
-        return Puppet::Util::Profiler.profile(_("Not inlining unsupported source scheme"), [:compiler, :static_compile_inlining, :skipped_file_metadata, :unsupported_scheme]) { false }
-      else
-        return true
+    when resource[:ensure] == 'absent'
+      #TRANSLATORS Inlining refers to adding additional metadata (in this case we are not inlining)
+      return Puppet::Util::Profiler.profile(_("Not inlining absent resource"), [:compiler, :static_compile_inlining, :skipped_file_metadata, :absent]) { false }
+    when sources.empty?
+      #TRANSLATORS Inlining refers to adding additional metadata (in this case we are not inlining)
+      return Puppet::Util::Profiler.profile(_("Not inlining resource without sources"), [:compiler, :static_compile_inlining, :skipped_file_metadata, :no_sources]) { false }
+    when (not (sources.all? {|source| source =~ /^puppet:/}))
+      #TRANSLATORS Inlining refers to adding additional metadata (in this case we are not inlining)
+      return Puppet::Util::Profiler.profile(_("Not inlining unsupported source scheme"), [:compiler, :static_compile_inlining, :skipped_file_metadata, :unsupported_scheme]) { false }
+    else
+      return true
     end
   end
 

diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb
@@ -36,11 +36,11 @@ def environment
 
   def environment=(env)
     @environment =
-    if env.is_a?(Puppet::Node::Environment)
-      env
-    else
-      Puppet.lookup(:environments).get!(env)
-    end
+      if env.is_a?(Puppet::Node::Environment)
+        env
+      else
+        Puppet.lookup(:environments).get!(env)
+      end
   end
 
   # LAK:NOTE This is a messy interface to the cache, and it's only

diff --git a/lib/puppet/module/plan.rb b/lib/puppet/module/plan.rb
@@ -46,11 +46,11 @@ def initialize(plan_name, module_name)
 
     ALLOWED_EXTENSIONS = %w{.pp .yaml}
     RESERVED_WORDS = %w{and application attr case class consumes default else
-        elsif environment false function if import in inherits node or private
-        produces site true type undef unless}
+                        elsif environment false function if import in inherits node or private
+                        produces site true type undef unless}
     RESERVED_DATA_TYPES = %w{any array boolean catalogentry class collection
-        callable data default enum float hash integer numeric optional pattern
-        resource runtime scalar string struct tuple type undef variant}
+                             callable data default enum float hash integer numeric optional pattern
+                             resource runtime scalar string struct tuple type undef variant}
 
     def self.is_plan_name?(name)
       return true if name =~ /^[a-z][a-z0-9_]*$/

diff --git a/lib/puppet/module_tool/install_directory.rb b/lib/puppet/module_tool/install_directory.rb
@@ -26,7 +26,7 @@ def prepare(module_name, version)
         end
       end
 
-    private
+      private
 
       ERROR_MAPPINGS = {
         Errno::EACCES => PermissionDeniedCreateInstallDirectoryError,

diff --git a/lib/puppet/module_tool/metadata.rb b/lib/puppet/module_tool/metadata.rb
@@ -172,14 +172,14 @@ def validate_name(name)
       modname = :namespace_missing if namespace == ''
 
       err = case modname
-      when nil, '', :namespace_missing
-        _("the field must be a namespaced module name")
-      when /[^a-z0-9_]/i
-        _("the module name contains non-alphanumeric (or underscore) characters")
-      when /^[^a-z]/i
-        _("the module name must begin with a letter")
-      else
-        _("the namespace contains non-alphanumeric characters")
+            when nil, '', :namespace_missing
+              _("the field must be a namespaced module name")
+            when /[^a-z0-9_]/i
+              _("the module name contains non-alphanumeric (or underscore) characters")
+            when /^[^a-z]/i
+              _("the module name must begin with a letter")
+            else
+              _("the namespace contains non-alphanumeric characters")
       end
 
       raise ArgumentError, _("Invalid 'name' field in metadata.json: %{err}") % { err: err }

diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
@@ -597,7 +597,7 @@ def perform_initial_import
         parse_results = Puppet::FileSystem::PathPattern.absolute(File.join(file, '**/*')).glob.select {|globbed_file| globbed_file.end_with?('.pp')}.sort.map do | file_to_parse |
                           parser.file = file_to_parse
                           parser.parse
-                        end
+        end
         # Use a parser type specific merger to concatenate the results
         Puppet::Parser::AST::Hostclass.new('', :code => Puppet::Parser::ParserFactory.code_merger.concatenate(parse_results))
       else