Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Commits on Oct 6, 2010
  1. MarkusQ

    Fix for #4945 -- explicitly check os to supress man page installation

    MarkusQ authored
    The fix for #4644 usurped an existing option flag to supress the installation
    of man pages on Microsoft Windows, when the desired behaviour was to only skip
    the installation on MSWin and not change the behaviour on other platforms.
    This patch implements the check explicitly to correctly express the desired
    behaviour.
Commits on Jul 10, 2010
  1. MarkusQ

    Code smell: Two space indentation

    MarkusQ authored
    Replaced 106806 occurances of ^( +)(.*$) with
    
    The ruby community almost universally (i.e. everyone but Luke, Markus, and the other eleven people
    who learned ruby in the 1900s) uses two-space indentation.
    
    3 Examples:
    
        The code:
            end
    
            # Tell getopt which arguments are valid
            def test_get_getopt_args
                element = Setting.new :name => "foo", :desc => "anything", :settings => Puppet::Util::Settings.new
                assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args")
    
        becomes:
            end
    
            # Tell getopt which arguments are valid
            def test_get_getopt_args
              element = Setting.new :name => "foo", :desc => "anything", :settings => Puppet::Util::Settings.new
              assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args")
    
        The code:
                assert_equal(str, val)
    
                assert_instance_of(Float, result)
    
            end
    
            # Now test it with a passed object
        becomes:
              assert_equal(str, val)
    
              assert_instance_of(Float, result)
    
            end
    
            # Now test it with a passed object
        The code:
            end
    
            assert_nothing_raised do
                klass[:Yay] = "boo"
                klass["Cool"] = :yayness
            end
    
        becomes:
            end
    
            assert_nothing_raised do
              klass[:Yay] = "boo"
              klass["Cool"] = :yayness
            end
  2. MarkusQ

    Code smell: Avoid needless decorations

    MarkusQ authored
    * Replaced 704 occurances of (.*)\b([a-z_]+)\(\) with \1\2
    
      3 Examples:
    
          The code:
              ctx = OpenSSL::SSL::SSLContext.new()
          becomes:
              ctx = OpenSSL::SSL::SSLContext.new
          The code:
              skip()
          becomes:
              skip
          The code:
              path = tempfile()
          becomes:
              path = tempfile
    
    * Replaced 31 occurances of ^( *)end *#.* with \1end
    
      3 Examples:
    
          The code:
    
          becomes:
    
          The code:
              end # Dir.foreach
          becomes:
              end
          The code:
              end # def
          becomes:
              end
  3. MarkusQ

    Code smell: Avoid unneeded blocks

    MarkusQ authored
    Replaced 45 occurances of
    
        (DEF)
            begin
                (LINES)
            rescue(.*)
                (LINES)
            end
        end
    
    with
    
    3 Examples:
    
        The code:
            def find(name)
                begin
                    self.const_get(name.to_s.capitalize)
                rescue
                    puts "Unable to find application '#{name.to_s}'."
                    Kernel::exit(1)
                end
            end
        becomes:
            def find(name)
                    self.const_get(name.to_s.capitalize)
            rescue
                    puts "Unable to find application '#{name.to_s}'."
                    Kernel::exit(1)
            end
        The code:
            def exit_on_fail(message, code = 1)
                begin
                    yield
                rescue RuntimeError, NotImplementedError => detail
                    puts detail.backtrace if Puppet[:trace]
                    $stderr.puts "Could not #{message}: #{detail}"
                    exit(code)
                end
            end
        becomes:
            def exit_on_fail(message, code = 1)
                    yield
            rescue RuntimeError, NotImplementedError => detail
                    puts detail.backtrace if Puppet[:trace]
                    $stderr.puts "Could not #{message}: #{detail}"
                    exit(code)
            end
        The code:
            def start
                begin
                    case ssl
                    when :tls
                        @connection = LDAP::SSLConn.new(host, port, true)
                    when true
                        @connection = LDAP::SSLConn.new(host, port)
                    else
                        @connection = LDAP::Conn.new(host, port)
                    end
                    @connection.set_option(LDAP::LDAP_OPT_PROTOCOL_VERSION, 3)
                    @connection.set_option(LDAP::LDAP_OPT_REFERRALS, LDAP::LDAP_OPT_ON)
                    @connection.simple_bind(user, password)
                rescue => detail
                    raise Puppet::Error, "Could not connect to LDAP: #{detail}"
                end
            end
        becomes:
            def start
                    case ssl
                    when :tls
                        @connection = LDAP::SSLConn.new(host, port, true)
                    when true
                        @connection = LDAP::SSLConn.new(host, port)
                    else
                        @connection = LDAP::Conn.new(host, port)
                    end
                    @connection.set_option(LDAP::LDAP_OPT_PROTOCOL_VERSION, 3)
                    @connection.set_option(LDAP::LDAP_OPT_REFERRALS, LDAP::LDAP_OPT_ON)
                    @connection.simple_bind(user, password)
            rescue => detail
                    raise Puppet::Error, "Could not connect to LDAP: #{detail}"
            end
  4. MarkusQ

    Code smell: Booleans are first class values.

    MarkusQ authored
    * Replaced 2 occurances of
    
          def (.*)
              begin
                  (.*) = Integer\((.*)\)
                  return \2
              rescue ArgumentError
                  \2 = nil
              end
              if \2 = (.*)
                  return \2
              else
                  return false
              end
          end
    
      with
    
      2 Examples:
    
          The code:
              def validuser?(value)
                  begin
                      number = Integer(value)
                      return number
                  rescue ArgumentError
                      number = nil
                  end
                  if number = uid(value)
                      return number
                  else
                      return false
                  end
              end
          becomes:
              def validuser?(value)
                  Integer(value) rescue uid(value) || false
              end
          The code:
              def validgroup?(value)
                  begin
                      number = Integer(value)
                      return number
                  rescue ArgumentError
                      number = nil
                  end
                  if number = gid(value)
                      return number
                  else
                      return false
                  end
              end
          becomes:
              def validgroup?(value)
                  Integer(value) rescue gid(value) || false
              end
    
    * Replaced 28 occurances of
    
          return (.*?) if (.*)
          return (.*)
    
      with
    
      3 Examples:
    
          The code:
              return send(options[:mode]) if [:rdoc, :trac, :markdown].include?(options[:mode])
              return other
          becomes:
              return[:rdoc, :trac, :markdown].include?(options[:mode]) ? send(options[:mode]) : other
          The code:
              return true if known_resource_types.definition(name)
              return false
          becomes:
              return(known_resource_types.definition(name) ? true : false)
          The code:
              return :rest if request.protocol == 'https'
              return Puppet::FileBucket::File.indirection.terminus_class
          becomes:
              return(request.protocol == 'https' ? :rest : Puppet::FileBucket::File.indirection.terminus_class)
    
    * Replaced no occurances of
    
          return (.*?) unless (.*)
          return (.*)
    
      with
    
    * Replaced 7 occurances of
    
          if (.*)
              (.*[^:])false
          else
              \2true
          end
    
      with
    
      3 Examples:
    
          The code:
              if RUBY_PLATFORM == "i386-mswin32"
                  InstallOptions.ri  = false
              else
                  InstallOptions.ri  = true
              end
          becomes:
              InstallOptions.ri  = RUBY_PLATFORM != "i386-mswin32"
          The code:
              if options[:references].length > 1
                  with_contents = false
              else
                  with_contents = true
              end
          becomes:
              with_contents = options[:references].length <= 1
          The code:
              if value == false or value == "" or value == :undef
                  return false
              else
                  return true
              end
          becomes:
              return (value != false and value != "" and value != :undef)
    
    * Replaced 19 occurances of
    
          if (.*)
              (.*[^:])true
          else
              \2false
          end
    
      with
    
      3 Examples:
    
          The code:
              if Puppet::Util::Log.level == :debug
                  return true
              else
                  return false
              end
          becomes:
              return Puppet::Util::Log.level == :debug
          The code:
              if satisfies?(*features)
                  return true
              else
                  return false
              end
          becomes:
              return !!satisfies?(*features)
          The code:
              if self.class.parsed_auth_db.has_key?(resource[:name])
                  return true
              else
                  return false
              end
          becomes:
              return !!self.class.parsed_auth_db.has_key?(resource[:name])
    
    * Replaced 1 occurance of
    
          if ([a-z_]) = (.*)
              (.*[^:])\1
          else
              \3(.*)
          end
    
      with
    
      1 Example:
    
          The code:
              if c = self.send(@subclassname, method)
                  return c
              else
                  return nil
              end
          becomes:
              return self.send(@subclassname, method) || nil
    
    * Replaced 2 occurances of
    
          if (.*)
              (.*[^:])\1
          else
              \2false
          end
    
      with
    
      2 Examples:
    
          The code:
              if hash[:Local]
                  @local = hash[:Local]
              else
                  @local = false
              end
          becomes:
              @local = hash[:Local]
          The code:
              if hash[:Local]
                  @local = hash[:Local]
              else
                  @local = false
              end
          becomes:
              @local = hash[:Local]
    
    * Replaced 10 occurances of
    
          if (.*)
              (.*[^:])(.*)
          else
              \2false
          end
    
      with
    
      3 Examples:
    
          The code:
              if defined?(@isnamevar)
                  return @isnamevar
              else
                  return false
              end
          becomes:
              return defined?(@isnamevar) && @isnamevar
          The code:
              if defined?(@required)
                  return @required
              else
                  return false
              end
          becomes:
              return defined?(@required) && @required
          The code:
              if number = uid(value)
                  return number
              else
                  return false
              end
          becomes:
              return (number = uid(value)) && number
    
    * Replaced no occurances of
    
          if (.*)
              (.*[^:])nil
          else
              \2(true)
          end
    
      with
    
    * Replaced no occurances of
    
          if (.*)
              (.*[^:])true
          else
              \2nil
          end
    
      with
    
    * Replaced no occurances of
    
          if (.*)
              (.*[^:])\1
          else
              \2nil
          end
    
      with
    
    * Replaced 23 occurances of
    
          if (.*)
              (.*[^:])(.*)
          else
              \2nil
          end
    
      with
    
      3 Examples:
    
          The code:
              if node = Puppet::Node.find(hostname)
                  env = node.environment
              else
                  env = nil
              end
          becomes:
              env = (node = Puppet::Node.find(hostname)) ? node.environment : nil
          The code:
              if mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files?
                  return @mounts[MODULES].copy(mod.name, mod.file_directory)
              else
                  return nil
              end
          becomes:
              return (mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files?) ? @mounts[MODULES].copy(mod.name, mod.file_directory) : nil
          The code:
              if hash.include?(:CA) and hash[:CA]
                  @ca = Puppet::SSLCertificates::CA.new()
              else
                  @ca = nil
              end
          becomes:
              @ca = (hash.include?(:CA) and hash[:CA]) ? Puppet::SSLCertificates::CA.new() : nil
  5. MarkusQ

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

    MarkusQ authored
    * Replaced 6 occurances of (while .*?) *do$ with
    
      The do is unneeded in the block header form and causes problems
      with the block-to-one-line transformation.
    
      3 Examples:
    
          The code:
              while line = f.gets do
          becomes:
              while line = f.gets
          The code:
              while line = shadow.gets do
          becomes:
              while line = shadow.gets
          The code:
              while wrapper = zeros.pop do
          becomes:
              while wrapper = zeros.pop
    
    * Replaced 19 occurances of ((if|unless) .*?) *then$ with
    
      The then is unneeded in the block header form and causes problems
      with the block-to-one-line transformation.
    
      3 Examples:
    
          The code:
              if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } then
          becomes:
              if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) }
          The code:
              unless defined?(@spec_command) then
          becomes:
              unless defined?(@spec_command)
          The code:
              if c == ?\n then
          becomes:
              if c == ?\n
    
    * Replaced 758 occurances of
    
          ((?:if|unless|while|until) .*)
              (.*)
          end
    
      with
    
      The one-line form is preferable provided:
    
          * The condition is not used to assign a variable
          * The body line is not already modified
          * The resulting line is not too long
    
      3 Examples:
    
          The code:
              if Puppet.features.libshadow?
                  has_feature :manages_passwords
              end
          becomes:
              has_feature :manages_passwords if Puppet.features.libshadow?
          The code:
              unless (defined?(@current_pool) and @current_pool)
                  @current_pool = process_zpool_data(get_pool_data)
              end
          becomes:
              @current_pool = process_zpool_data(get_pool_data) unless (defined?(@current_pool) and @current_pool)
          The code:
              if Puppet[:trace]
                  puts detail.backtrace
              end
          becomes:
              puts detail.backtrace if Puppet[:trace]
  6. MarkusQ

    Code smell: Use string interpolation

    MarkusQ authored
    * Replaced 83 occurances of
    
          (.*)" *[+] *([$@]?[\w_0-9.:]+?)(.to_s\b)?(?! *[*(%\w_0-9.:{\[])
    
      with
    
          \1#{\2}"
    
      3 Examples:
    
          The code:
              puts "PUPPET " + status + ": " + process + ", " + state
          becomes:
              puts "PUPPET " + status + ": " + process + ", #{state}"
          The code:
              puts "PUPPET " + status + ": #{process}" + ", #{state}"
          becomes:
              puts "PUPPET #{status}" + ": #{process}" + ", #{state}"
          The code:
              }.compact.join( "\n" ) + "\n" + t + "]\n"
          becomes:
              }.compact.join( "\n" ) + "\n#{t}" + "]\n"
    
    * Replaced 21 occurances of (.*)" *[+] *" with \1
    
      3 Examples:
    
          The code:
              puts "PUPPET #{status}" + ": #{process}" + ", #{state}"
          becomes:
              puts "PUPPET #{status}" + ": #{process}, #{state}"
          The code:
              puts "PUPPET #{status}" + ": #{process}, #{state}"
          becomes:
              puts "PUPPET #{status}: #{process}, #{state}"
          The code:
              res = self.class.name + ": #{@name}" + "\n"
          becomes:
              res = self.class.name + ": #{@name}\n"
    
    * Don't use string concatenation to split lines unless they would be very long.
    
      Replaced 11 occurances of
    
          (.*)(['"]) *[+]
           *(['"])(.*)
    
      with
    
      3 Examples:
    
          The code:
              o.define_head "The check_puppet Nagios plug-in checks that specified " +
                  "Puppet process is running and the state file is no " +
          becomes:
              o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " +
          The code:
              o.separator   "Mandatory arguments to long options are mandatory for " +
              "short options too."
          becomes:
              o.separator   "Mandatory arguments to long options are mandatory for short options too."
          The code:
              o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " +
                  "older than specified interval."
          becomes:
              o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no older than specified interval."
    
    * Replaced no occurances of do (.*?) end with {\1}
    
    * Replaced 1488 occurances of
    
          "([^"\n]*%s[^"\n]*)" *% *(.+?)(?=$| *\b(do|if|while|until|unless|#)\b)
    
      with
    
      20 Examples:
    
          The code:
              args[0].split(/\./).map do |s| "dc=%s"%[s] end.join(",")
          becomes:
              args[0].split(/\./).map do |s| "dc=#{s}" end.join(",")
          The code:
              puts "%s" % Puppet.version
          becomes:
              puts "#{Puppet.version}"
          The code:
              raise "Could not find information for %s" % node
          becomes:
              raise "Could not find information for #{node}"
          The code:
              raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)]
          becomes:
              raise Puppet::Error, "Cannot create #{dir}: basedir #{File.join(path)} is a file"
          The code:
              Puppet.err "Could not run %s: %s" % [client_class, detail]
          becomes:
              Puppet.err "Could not run #{client_class}: #{detail}"
          The code:
              raise "Could not find handler for %s" % arg
          becomes:
              raise "Could not find handler for #{arg}"
          The code:
              Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig]
          becomes:
              Puppet.err "Will not start without authorization file #{Puppet[:authconfig]}"
          The code:
              raise Puppet::Error, "Could not deserialize catalog from pson: %s" % detail
          becomes:
              raise Puppet::Error, "Could not deserialize catalog from pson: #{detail}"
          The code:
              raise "Could not find facts for %s" % Puppet[:certname]
          becomes:
              raise "Could not find facts for #{Puppet[:certname]}"
          The code:
              raise ArgumentError, "%s is not readable" % path
          becomes:
              raise ArgumentError, "#{path} is not readable"
          The code:
              raise ArgumentError, "Invalid handler %s" % name
          becomes:
              raise ArgumentError, "Invalid handler #{name}"
          The code:
              debug "Executing '%s' in zone %s with '%s'" % [command, @resource[:name], str]
          becomes:
              debug "Executing '#{command}' in zone #{@resource[:name]} with '#{str}'"
          The code:
              raise Puppet::Error, "unknown cert type '%s'" % hash[:type]
          becomes:
              raise Puppet::Error, "unknown cert type '#{hash[:type]}'"
          The code:
              Puppet.info "Creating a new certificate request for %s" % Puppet[:certname]
          becomes:
              Puppet.info "Creating a new certificate request for #{Puppet[:certname]}"
          The code:
              "Cannot create alias %s: object already exists" % [name]
          becomes:
              "Cannot create alias #{name}: object already exists"
          The code:
              return "replacing from source %s with contents %s" % [metadata.source, metadata.checksum]
          becomes:
              return "replacing from source #{metadata.source} with contents #{metadata.checksum}"
          The code:
              it "should have a %s parameter" % param do
          becomes:
              it "should have a #{param} parameter" do
          The code:
              describe "when registring '%s' messages" % log do
          becomes:
              describe "when registring '#{log}' messages" do
          The code:
              paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration%stest" % l }
          becomes:
              paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration#{l}test" }
          The code:
              assert_raise(Puppet::Error, "Check '%s' did not fail on false" % check) do
          becomes:
              assert_raise(Puppet::Error, "Check '#{check}' did not fail on false") do
  7. MarkusQ

    Code smell: English names for special globals rather than line-noise

    MarkusQ authored
    * Replaced 36 occurances of [$][?] with $CHILD_STATUS
    
      3 Examples:
    
          The code:
              print "%s finished with exit code %s\n" % [host, $?.exitstatus]
          becomes:
              print "%s finished with exit code %s\n" % [host, $CHILD_STATUS.exitstatus]
          The code:
              $stderr.puts "Could not find host for PID %s with status %s" % [pid, $?.exitstatus]
          becomes:
              $stderr.puts "Could not find host for PID %s with status %s" % [pid, $CHILD_STATUS.exitstatus]
          The code:
              unless $? == 0
          becomes:
              unless $CHILD_STATUS == 0
    
    * Replaced 3 occurances of [$][$] with $PID
    
      3 Examples:
    
          The code:
              Process.kill(:HUP, $$) if restart_requested?
          becomes:
              Process.kill(:HUP, $PID) if restart_requested?
          The code:
              if pid == $$
          becomes:
              if pid == $PID
          The code:
              host[:name] = "!invalid.hostname.$$$"
          becomes:
              host[:name] = "!invalid.hostname.$PID$"
    
    * Replaced 7 occurances of [$]& with $MATCH
    
      3 Examples:
    
          The code:
              work.slice!(0, $&.length)
          becomes:
              work.slice!(0, $MATCH.length)
          The code:
              if $&
          becomes:
              if $MATCH
          The code:
              if $&
          becomes:
              if $MATCH
    
    * Replaced 28 occurances of [$]:(?!:) with $LOAD_PATH
    
      3 Examples:
    
          The code:
              sitelibdir = $:.find { |x| x =~ /site_ruby/ }
          becomes:
              sitelibdir = $LOAD_PATH.find { |x| x =~ /site_ruby/ }
          The code:
              $:.unshift "lib"
          becomes:
              $LOAD_PATH.unshift "lib"
          The code:
              $:.shift
          becomes:
              $LOAD_PATH.shift
    
    * Replaced 3 occurances of [$]! with $ERROR_INFO
    
      3 Examples:
    
          The code:
              $LOG.fatal("Problem reading #{filepath}: #{$!}")
          becomes:
              $LOG.fatal("Problem reading #{filepath}: #{$ERROR_INFO}")
          The code:
              $stderr.puts "Couldn't build man pages: " + $!
          becomes:
              $stderr.puts "Couldn't build man pages: " + $ERROR_INFO
          The code:
              $stderr.puts $!.message
          becomes:
              $stderr.puts $ERROR_INFO.message
    
    * Replaced 3 occurances of ^(.*)[$]" with \1$LOADED_FEATURES
    
      3 Examples:
    
          The code:
              unless $".index 'racc/parser.rb'
          becomes:
              unless $LOADED_FEATURES.index 'racc/parser.rb'
          The code:
              $".push 'racc/parser.rb'
          becomes:
              $LOADED_FEATURES.push 'racc/parser.rb'
          The code:
              $".should be_include("tmp/myfile.rb")
          becomes:
              $LOADED_FEATURES.should be_include("tmp/myfile.rb")
  8. MarkusQ

    Code smell: Use {} for % notation delimiters wherever practical

    MarkusQ authored
    *
    
    * Replaced 16 occurances of %([qQrwWx])\((.*?)\) with %\1{\2}
    
      3 Examples:
    
          The code:
              # %r(/) != /\//
          becomes:
              # %r{/} != /\//
          The code:
              ri    = glob(%w(bin/*.rb sbin/* lib/**/*.rb)).reject { |e| e=~ /\.(bat|cmd)$/ }
          becomes:
              ri    = glob(%w{bin/*.rb sbin/* lib/**/*.rb}).reject { |e| e=~ /\.(bat|cmd)$/ }
          The code:
              if !FileUtils.uptodate?("/var/cache/eix", %w(/usr/bin/eix /usr/portage/metadata/timestamp))
          becomes:
              if !FileUtils.uptodate?("/var/cache/eix", %w{/usr/bin/eix /usr/portage/metadata/timestamp})
    
    * Replaced 12 occurances of %([qQrwWx])\[(.*?)\] with %\1{\2}
    
      3 Examples:
    
          The code:
              return send(command) if %w[get backup restore].include? command
          becomes:
              return send(command) if %w{get backup restore}.include? command
          The code:
              for iv in %w[indent space space_before object_nl array_nl check_circular allow_nan max_nesting]
          becomes:
              for iv in %w{indent space space_before object_nl array_nl check_circular allow_nan max_nesting}
          The code:
              @puppetd.command_line.stubs(:args).returns(%w[--logdest /my/file])
          becomes:
              @puppetd.command_line.stubs(:args).returns(%w{--logdest /my/file})
    
    * Replaced no occurances of %([qQrwWx])<(.*?)> with %\1{\2}
    
    * Replaced 19 occurances of
    
          %([qQrwWx])([^{\[(<])(.*?)\2
    
      with
    
          %\1{\3}
    
      3 Examples:
    
          The code:
              at.add_mapping(%r%^lib/puppet/(.*)\.rb$%) { |filename, m|
          becomes:
              at.add_mapping(%r{^lib/puppet/(.*)\.rb$}) { |filename, m|
          The code:
              at.add_mapping(%r%^spec/(unit|integration)/.*\.rb$%) { |filename, _|
          becomes:
              at.add_mapping(%r{^spec/(unit|integration)/.*\.rb$}) { |filename, _|
          The code:
              at.add_mapping(%r!^lib/puppet\.rb$!) { |filename, _|
          becomes:
              at.add_mapping(%r{^lib/puppet\.rb$}) { |filename, _|
  9. MarkusQ

    Code smell: Inconsistent indentation and related formatting issues

    MarkusQ authored
    * Replaced 163 occurances of
    
          defined\? +([@a-zA-Z_.0-9?=]+)
    
      with
    
          defined?(\1)
    
      This makes detecting subsequent patterns easier.
    
      3 Examples:
    
          The code:
              if ! defined? @parse_config
          becomes:
              if ! defined?(@parse_config)
          The code:
              return @option_parser if defined? @option_parser
          becomes:
              return @option_parser if defined?(@option_parser)
          The code:
              if defined? @local and @local
          becomes:
              if defined?(@local) and @local
    
    * Eliminate trailing spaces.
    
      Replaced 428 occurances of ^(.*?) +$ with \1
    
      1 file was skipped.
    
          test/ral/providers/host/parsed.rb because 0
    
    * Replace leading tabs with an appropriate number of spaces.
    
      Replaced 306 occurances of ^(\t+)(.*) with
    
      Tabs are not consistently expanded in all environments.
    
    * Don't arbitrarily wrap on sprintf (%) operator.
    
      Replaced 143 occurances of
    
          (.*['"] *%)
           +(.*)
    
      with
    
      Splitting the line does nothing to aid clarity and hinders further refactorings.
    
      3 Examples:
    
          The code:
              raise Puppet::Error, "Cannot create %s: basedir %s is a file" %
                  [dir, File.join(path)]
          becomes:
              raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)]
          The code:
              Puppet.err "Will not start without authorization file %s" %
                  Puppet[:authconfig]
          becomes:
              Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig]
          The code:
              $stderr.puts "Could not find host for PID %s with status %s" %
                  [pid, $?.exitstatus]
          becomes:
              $stderr.puts "Could not find host for PID %s with status %s" % [pid, $?.exitstatus]
    
    * Don't break short arrays/parameter list in two.
    
      Replaced 228 occurances of
    
          (.*)
           +(.*)
    
      with
    
      3 Examples:
    
          The code:
              puts @format.wrap(type.provider(prov).doc,
                                :indent => 4, :scrub => true)
          becomes:
              puts @format.wrap(type.provider(prov).doc, :indent => 4, :scrub => true)
          The code:
              assert(FileTest.exists?(daily),
                  "Did not make daily graph for %s" % type)
          becomes:
              assert(FileTest.exists?(daily), "Did not make daily graph for %s" % type)
          The code:
              assert(prov.target_object(:first).read !~ /^notdisk/,
                  "Did not remove thing from disk")
          becomes:
              assert(prov.target_object(:first).read !~ /^notdisk/, "Did not remove thing from disk")
    
    * If arguments must wrap, treat them all equally
    
      Replaced 510 occurances of
    
          lines ending in things like ...(foo, or ...(bar(1,3),
    
      with
    
          \1
              \2
    
      3 Examples:
    
          The code:
              midscope.to_hash(false),
          becomes:
              assert_equal(
          The code:
              botscope.to_hash(true),
          becomes:
              # bottomscope, then checking that we see the right stuff.
          The code:
              :path => link,
          becomes:
    
    * Replaced 4516 occurances of ^( *)(.*) with
    
      The present code base is supposed to use four-space indentation.  In some places we failed
      to maintain that standard.  These should be fixed regardless of the 2 vs. 4 space question.
    
      15 Examples:
    
          The code:
    
              def run_comp(cmd)
                 puts cmd
                 results = []
                 old_sync = $stdout.sync
                 $stdout.sync = true
                 line = []
                 begin
                    open("| #{cmd}", "r") do |f|
                      until f.eof? do
                        c = f.getc
          becomes:
    
              def run_comp(cmd)
                  puts cmd
                  results = []
                  old_sync = $stdout.sync
                  $stdout.sync = true
                  line = []
                  begin
                      open("| #{cmd}", "r") do |f|
                          until f.eof? do
                              c = f.getc
          The code:
                                  s.gsub!(/.{4}/n, '\\\\u\&')
                                }
                  string.force_encoding(Encoding::UTF_8)
                  string
                rescue Iconv::Failure => e
                  raise GeneratorError, "Caught #{e.class}: #{e}"
                end
              else
                def utf8_to_pson(string) # :nodoc:
                  string = string.gsub(/["\\\x0-\x1f]/) { MAP[$&] }
                  string.gsub!(/(
          becomes:
                                          s.gsub!(/.{4}/n, '\\\\u\&')
                                      }
                      string.force_encoding(Encoding::UTF_8)
                      string
                  rescue Iconv::Failure => e
                      raise GeneratorError, "Caught #{e.class}: #{e}"
                  end
              else
                  def utf8_to_pson(string) # :nodoc:
                      string = string.gsub(/["\\\x0-\x1f]/) { MAP[$&] }
                      string.gsub!(/(
          The code:
                  end
              }
    
              rvalues:      rvalue
                          | rvalues comma rvalue {
                  if val[0].instance_of?(AST::ASTArray)
                      result = val[0].push(val[2])
                  else
                      result = ast AST::ASTArray, :children => [val[0],val[2]]
                  end
              }
          becomes:
                  end
              }
    
              rvalues:      rvalue
                  | rvalues comma rvalue {
                      if val[0].instance_of?(AST::ASTArray)
                      result = val[0].push(val[2])
                  else
                      result = ast AST::ASTArray, :children => [val[0],val[2]]
                  end
              }
          The code:
              #passwdproc = proc { @password }
    
                          keytext = @key.export(
    
                  OpenSSL::Cipher::DES.new(:EDE3, :CBC),
    
                  @password
              )
              File.open(@keyfile, "w", 0400) { |f|
                  f << keytext
              }
          becomes:
              #        passwdproc = proc { @password }
    
                  keytext = @key.export(
    
                      OpenSSL::Cipher::DES.new(:EDE3, :CBC),
    
                      @password
                      )
                      File.open(@keyfile, "w", 0400) { |f|
                          f << keytext
                      }
          The code:
              end
    
              def to_manifest
                  "%s { '%s':\n%s\n}" % [self.type.to_s, self.name,
                       @params.collect { |p, v|
                           if v.is_a? Array
                               "    #{p} => [\'#{v.join("','")}\']"
                           else
                               "    #{p} => \'#{v}\'"
                           end
                       }.join(",\n")
          becomes:
              end
    
              def to_manifest
                  "%s { '%s':\n%s\n}" % [self.type.to_s, self.name,
                      @params.collect { |p, v|
                          if v.is_a? Array
                              "    #{p} => [\'#{v.join("','")}\']"
                          else
                              "    #{p} => \'#{v}\'"
                          end
                      }.join(",\n")
          The code:
              via the augeas tool.
    
               Requires:
                 - augeas to be installed (http://www.augeas.net)
                 - ruby-augeas bindings
    
               Sample usage with a string::
    
                  augeas{\"test1\" :
                         context => \"/files/etc/sysconfig/firstboot\",
                         changes => \"set RUN_FIRSTBOOT YES\",
          becomes:
              via the augeas tool.
    
              Requires:
                  - augeas to be installed (http://www.augeas.net)
                  - ruby-augeas bindings
    
              Sample usage with a string::
    
                  augeas{\"test1\" :
                      context => \"/files/etc/sysconfig/firstboot\",
                      changes => \"set RUN_FIRSTBOOT YES\",
          The code:
                  names.should_not be_include("root")
              end
    
              describe "when generating a purgeable resource" do
                  it "should be included in the generated resources" do
                      Puppet::Type.type(:host).stubs(:instances).returns [@purgeable_resource]
                      @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref)
                  end
              end
    
              describe "when the instance's do not have an ensure property" do
          becomes:
                  names.should_not be_include("root")
              end
    
              describe "when generating a purgeable resource" do
                  it "should be included in the generated resources" do
                      Puppet::Type.type(:host).stubs(:instances).returns [@purgeable_resource]
                      @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref)
                  end
              end
    
              describe "when the instance's do not have an ensure property" do
          The code:
              describe "when the instance's do not have an ensure property" do
                  it "should not be included in the generated resources" do
                      @no_ensure_resource = Puppet::Type.type(:exec).new(:name => '/usr/bin/env echo')
                      Puppet::Type.type(:host).stubs(:instances).returns [@no_ensure_resource]
                      @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref)
                  end
              end
    
              describe "when the instance's ensure property does not accept absent" do
                  it "should not be included in the generated resources" do
                      @no_absent_resource = Puppet::Type.type(:service).new(:name => 'foobar')
          becomes:
              describe "when the instance's do not have an ensure property" do
                  it "should not be included in the generated resources" do
                      @no_ensure_resource = Puppet::Type.type(:exec).new(:name => '/usr/bin/env echo')
                      Puppet::Type.type(:host).stubs(:instances).returns [@no_ensure_resource]
                      @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref)
                  end
              end
    
              describe "when the instance's ensure property does not accept absent" do
                  it "should not be included in the generated resources" do
                      @no_absent_resource = Puppet::Type.type(:service).new(:name => 'foobar')
          The code:
              func = nil
              assert_nothing_raised do
    
                              func = Puppet::Parser::AST::Function.new(
    
                      :name => "template",
                      :ftype => :rvalue,
    
                      :arguments => AST::ASTArray.new(
                          :children => [stringobj(template)]
                      )
          becomes:
              func = nil
              assert_nothing_raised do
    
                  func = Puppet::Parser::AST::Function.new(
    
                      :name => "template",
                      :ftype => :rvalue,
    
                      :arguments => AST::ASTArray.new(
                          :children => [stringobj(template)]
                      )
          The code:
    
                      assert(
                  @store.allowed?("hostname.madstop.com", "192.168.1.50"),
    
              "hostname not allowed")
    
                      assert(
                  ! @store.allowed?("name.sub.madstop.com", "192.168.0.50"),
    
              "subname name allowed")
          becomes:
    
              assert(
                  @store.allowed?("hostname.madstop.com", "192.168.1.50"),
    
              "hostname not allowed")
    
                  assert(
                      ! @store.allowed?("name.sub.madstop.com", "192.168.0.50"),
    
              "subname name allowed")
          The code:
    
              assert_nothing_raised {
    
                              server = Puppet::Network::Handler.fileserver.new(
    
                      :Local => true,
    
                      :Config => false
                  )
              }
    
          becomes:
    
              assert_nothing_raised {
    
                  server = Puppet::Network::Handler.fileserver.new(
    
                      :Local => true,
    
                      :Config => false
                  )
              }
    
          The code:
                   'yay',
                                                   { :failonfail => false,
                                                     :uid => @user.uid,
    
                                                     :gid => @user.gid }
                                                 ).returns('output')
    
              output = Puppet::Util::SUIDManager.run_and_capture 'yay',
                                                                 @user.uid,
                                                                 @user.gid
          becomes:
                      'yay',
                          { :failonfail => false,
                              :uid => @user.uid,
    
                              :gid => @user.gid }
                                  ).returns('output')
    
              output = Puppet::Util::SUIDManager.run_and_capture 'yay',
                  @user.uid,
                  @user.gid
          The code:
                          ).times(1)
              pkg.provider.expects(
                              :aptget
    
                                      ).with(
    
                              '-y',
                              '-q',
                              'remove',
    
                              'faff'
          becomes:
                          ).times(1)
              pkg.provider.expects(
                  :aptget
    
                      ).with(
    
                          '-y',
                          '-q',
                          'remove',
    
                          'faff'
          The code:
              johnny one two
              billy three four\n"
    
                      # Just parse and generate, to make sure it's isomorphic.
                      assert_nothing_raised do
                          assert_equal(text, @parser.to_file(@parser.parse(text)),
                              "parsing was not isomorphic")
                      end
                  end
    
                  def test_valid_attrs
          becomes:
              johnny one two
              billy three four\n"
    
              #         Just parse and generate, to make sure it's isomorphic.
              assert_nothing_raised do
                  assert_equal(text, @parser.to_file(@parser.parse(text)),
                      "parsing was not isomorphic")
                      end
                  end
    
                  def test_valid_attrs
          The code:
                      "testing",
                                  :onboolean => [true, "An on bool"],
    
                                  :string => ["a string", "A string arg"]
                                  )
              result = []
              should = []
              assert_nothing_raised("Add args failed") do
                  @config.addargs(result)
              end
              @config.each do |name, element|
          becomes:
                      "testing",
                          :onboolean => [true, "An on bool"],
    
                          :string => ["a string", "A string arg"]
                          )
              result = []
              should = []
              assert_nothing_raised("Add args failed") do
                  @config.addargs(result)
              end
              @config.each do |name, element|
  10. MarkusQ

    Code smell: Win32 --> MS_windows

    MarkusQ authored
    * Replaced 12 occurances of Win32 with Microsoft Windows
    
      3 Examples:
    
          The code:
              #    and all .rb files in lib/. This is disabled by default on Win32.
          becomes:
              #    and all .rb files in lib/. This is disabled by default on Microsoft Windows.
          The code:
              # We can use Win32 functions
          becomes:
              # We can use Microsoft Windows functions
          The code:
              desc "Uses Win32 functionality to manage file's users and rights."
          becomes:
              desc "Uses Microsoft Windows functionality to manage file's users and rights."
    
    * Replaced 10 occurances of :win32 with :microsoft_windows
    
      3 Examples:
    
          The code:
              Puppet.features.add(:win32, :libs => ["sys/admin", "win32/process", "win32/dir"])
          becomes:
              Puppet.features.add(:microsoft_windows, :libs => ["sys/admin", "win32/process", "win32/dir"])
          The code:
              Puppet::Type.type(:file).provide :win32 do
          becomes:
              Puppet::Type.type(:file).provide :microsoft_windows do
          The code:
              confine :feature => :win32
          becomes:
              confine :feature => :microsoft_windows
    
    * Replaced 13 occurances of win32\? with microsoft_windows?
    
      3 Examples:
    
          The code:
              signals.update({:HUP => :restart, :USR1 => :reload, :USR2 => :reopen_logs }) unless Puppet.features.win32?
          becomes:
              signals.update({:HUP => :restart, :USR1 => :reload, :USR2 => :reopen_logs }) unless Puppet.features.microsoft_windows?
          The code:
              raise Puppet::Error,"Cannot determine basic system flavour" unless Puppet.features.posix? or Puppet.features.win32?
          becomes:
              raise Puppet::Error,"Cannot determine basic system flavour" unless Puppet.features.posix? or Puppet.features.microsoft_windows?
          The code:
              require 'sys/admin' if Puppet.features.win32?
          becomes:
              require 'sys/admin' if Puppet.features.microsoft_windows?
Something went wrong with that request. Please try again.