Code smell: Inconsistent indentation and related formatting issues

* 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|

8 files changed, 31 insertions, 25 deletions

diff --git a/spec/unit/util/cache_accumulator_spec.rb b/spec/unit/util/cache_accumulator_spec.rb
index 2d976d0b1..3b5d68118 100644
--- a/spec/unit/util/cache_accumulator_spec.rb
+++ b/spec/unit/util/cache_accumulator_spec.rb
@@ -25,16 +25,22 @@ describe Puppet::Util::CacheAccumulator do
     end
 
     it 'should delegate bulk lookups to find with appropriate arguments and returning result count' do
-        @test_class.expects(:find).with(:all,
-                                        :conditions => {:name => ['a', 'b', 'c']}
-                                       ).returns(['a','b','c'].collect {|n| @test_class.new(n)}).once
+
+        @test_class.expects(:find).with(
+            :all,
+
+                :conditions => {:name => ['a', 'b', 'c']}
+                    ).returns(['a','b','c'].collect {|n| @test_class.new(n)}).once
         @test_class.accumulate_by_name('a', 'b', 'c').should == 3
     end
 
     it 'should only need find_or_create_by_name lookup for missing bulk entries' do
-        @test_class.expects(:find).with(:all,
-                                        :conditions => {:name => ['a', 'b']}
-                                       ).returns([ @test_class.new('a') ]).once
+
+        @test_class.expects(:find).with(
+            :all,
+
+                :conditions => {:name => ['a', 'b']}
+                    ).returns([ @test_class.new('a') ]).once
         @test_class.expects(:find_or_create_by_name).with('b').returns(@test_class.new('b')).once
         @test_class.expects(:find_or_create_by_name).with('a').never
         @test_class.accumulate_by_name('a','b').should == 1
diff --git a/spec/unit/util/file_locking_spec.rb b/spec/unit/util/file_locking_spec.rb
index 35e7f781e..9a8fad1f2 100755
--- a/spec/unit/util/file_locking_spec.rb
+++ b/spec/unit/util/file_locking_spec.rb
@@ -30,7 +30,7 @@ describe Puppet::Util::FileLocking do
             File.stubs(:exists?).with('/file').returns true
             File.stubs(:file?).with('/file').returns true
         end
-        
+
         it "should use a global shared mutex" do
             @sync = mock 'sync'
             @sync.expects(:synchronize).with(Sync::SH).once
@@ -66,7 +66,7 @@ describe Puppet::Util::FileLocking do
             File.expects(:exists?).with('/file').returns false
             File.expects(:open).with('/file').once
 
-            Puppet::Util::FileLocking.readlock('/file') 
+            Puppet::Util::FileLocking.readlock('/file')
         end
     end
 
@@ -150,7 +150,7 @@ describe Puppet::Util::FileLocking do
             File.expects(:exists?).with('/file').returns false
             File.expects(:open).with('/file', 'w', 0600).once
 
-            Puppet::Util::FileLocking.writelock('/file') 
+            Puppet::Util::FileLocking.writelock('/file')
         end
     end
 end
diff --git a/spec/unit/util/log_spec.rb b/spec/unit/util/log_spec.rb
index 917ad7dab..6414a37ff 100755
--- a/spec/unit/util/log_spec.rb
+++ b/spec/unit/util/log_spec.rb
@@ -15,7 +15,7 @@ describe Puppet::Util::Log do
 
     describe Puppet::Util::Log::DestConsole do
         before do
-            @console = Puppet::Util::Log::DestConsole.new 
+            @console = Puppet::Util::Log::DestConsole.new
         end
 
         it "should colorize if Puppet[:color] is :ansi" do
@@ -173,7 +173,7 @@ describe Puppet::Util::Log do
             it "should use the source_descriptors" do
                 source = stub "source"
                 source.stubs(:source_descriptors).returns(:tags => ["tag","tag2"], :path => "path", :version => 100)
-                
+
                 log = Puppet::Util::Log.new(:level => "notice", :message => :foo)
                 log.expects(:tag).with("tag")
                 log.expects(:tag).with("tag2")
diff --git a/spec/unit/util/selinux_spec.rb b/spec/unit/util/selinux_spec.rb
index 88f4ac809..3e10b59ed 100755
--- a/spec/unit/util/selinux_spec.rb
+++ b/spec/unit/util/selinux_spec.rb
@@ -19,20 +19,20 @@ describe Puppet::Util::SELinux do
         before do
         end
         it "should return :true if this system has SELinux enabled" do
-             Selinux.expects(:is_selinux_enabled).returns 1
-             selinux_support?.should be_true
+            Selinux.expects(:is_selinux_enabled).returns 1
+            selinux_support?.should be_true
         end
 
         it "should return :false if this system lacks SELinux" do
-             Selinux.expects(:is_selinux_enabled).returns 0
-             selinux_support?.should be_false
+            Selinux.expects(:is_selinux_enabled).returns 0
+            selinux_support?.should be_false
         end
- 
+
         it "should return nil if /proc/mounts does not exist" do
             File.stubs(:open).with("/proc/mounts").raises("No such file or directory - /proc/mounts")
             read_mounts.should == nil
         end
-   end
+    end
 
     describe "filesystem detection" do
         before :each do
@@ -77,7 +77,7 @@ describe Puppet::Util::SELinux do
     describe "realpath" do
         it "should handle files that don't exist" do
 
-            # Since I'm stubbing Pathname.new for this test, 
+            # Since I'm stubbing Pathname.new for this test,
             # I need to also stub the internal calls to Pathname.new,
             # which happen in Pathname.dirname and Parthname.basename
             # I want those to return real Pathname objects,
@@ -194,7 +194,7 @@ describe Puppet::Util::SELinux do
     end
 
     describe "set_selinux_context" do
-         before :each do
+        before :each do
             fh = stub 'fh', :close => nil
             File.stubs(:open).with("/proc/mounts").returns fh
             fh.stubs(:read_nonblock).returns(
@@ -206,7 +206,7 @@ describe Puppet::Util::SELinux do
                 ).then.raises EOFError
         end
 
-       it "should return nil if there is no SELinux support" do
+        it "should return nil if there is no SELinux support" do
             self.expects(:selinux_support?).returns false
             set_selinux_context("/foo", "user_u:role_r:type_t:s0").should be_nil
         end
diff --git a/spec/unit/util/settings/file_setting_spec.rb b/spec/unit/util/settings/file_setting_spec.rb
index 55ad2df8e..1fe372ccf 100755
--- a/spec/unit/util/settings/file_setting_spec.rb
+++ b/spec/unit/util/settings/file_setting_spec.rb
@@ -61,7 +61,7 @@ describe Puppet::Util::Settings::FileSetting do
 
     describe "when reading the owner" do
         it "should be root when the setting specifies root" do
-            setting = FileSetting.new(:settings => mock("settings"), :owner => "root", :desc => "a setting") 
+            setting = FileSetting.new(:settings => mock("settings"), :owner => "root", :desc => "a setting")
             setting.owner.should == "root"
         end
 
diff --git a/spec/unit/util/settings_spec.rb b/spec/unit/util/settings_spec.rb
index 56769af71..d7925101c 100755
--- a/spec/unit/util/settings_spec.rb
+++ b/spec/unit/util/settings_spec.rb
@@ -122,7 +122,7 @@ describe Puppet::Util::Settings do
             @settings[:bool].should == true
         end
 
-       it "should not consider a cli setting of a non boolean with a boolean as an argument to be a boolean" do
+        it "should not consider a cli setting of a non boolean with a boolean as an argument to be a boolean" do
             # Turn it off first
             @settings[:myval] = "bob"
             @settings.handlearg("--no-myval", "")
diff --git a/spec/unit/util/user_attr_spec.rb b/spec/unit/util/user_attr_spec.rb
index 57787b1a6..88f538bb8 100644
--- a/spec/unit/util/user_attr_spec.rb
+++ b/spec/unit/util/user_attr_spec.rb
@@ -20,7 +20,7 @@ describe UserAttr do
         end
 
         it "should return a hash with the name value from /etc/user_attr" do
-             UserAttr.get_attributes_by_name('foo')[:name].should == 'foo'
+            UserAttr.get_attributes_by_name('foo')[:name].should == 'foo'
         end
 
         #this test is contrived
diff --git a/spec/unit/util/zaml_spec.rb b/spec/unit/util/zaml_spec.rb
index 1f21c4e29..e39dbdf99 100644
--- a/spec/unit/util/zaml_spec.rb
+++ b/spec/unit/util/zaml_spec.rb
@@ -12,7 +12,7 @@ describe "Pure ruby yaml implementation" do
     []           => "--- []",
     :symbol      => "--- !ruby/sym symbol",
     {:a => "A"}  => "--- \n  !ruby/sym a: A"
-    }.each { |o,y| 
+    }.each { |o,y|
         it "should convert the #{o.class} #{o.inspect} to yaml" do
             o.to_yaml.should == y
         end
@@ -26,7 +26,7 @@ describe "Pure ruby yaml implementation" do
     Object.new                   => "--- !ruby/object {}",
     [Object.new]                 => "--- \n  - !ruby/object {}",
     {Object.new => Object.new}   => "--- \n  ? !ruby/object {}\n  : !ruby/object {}"
-    }.each { |o,y| 
+    }.each { |o,y|
         it "should convert the #{o.class} #{o.inspect} to yaml" do
             o.to_yaml.should == y
         end