Use guard clause and avoid assignment in conditional

5 files changed, 19 insertions, 17 deletions

diff --git a/lib/puppet/provider/augeas/augeas.rb b/lib/puppet/provider/augeas/augeas.rb
index ed3404d..c27aa81 100644
--- a/lib/puppet/provider/augeas/augeas.rb
+++ b/lib/puppet/provider/augeas/augeas.rb
@@ -105,9 +105,9 @@ Puppet::Type.type(:augeas).provide(:augeas) do
           end until ((nbracket == 0 && !inSingleTick && !inDoubleTick && (ch =~ %r{\s})) || sc.eos?)
           len = sc.pos - start
           len -= 1 unless sc.eos?
-          unless p = sc.string[start, len]
-            fail(_('missing path argument %{narg} for %{cmd}') % { narg: narg, cmd: cmd })
-          end
+          p = sc.string[start, len]
+          fail(_('missing path argument %{narg} for %{cmd}') % { narg: narg, cmd: cmd }) if p.nil?
+
           # Rip off any ticks if they are there.
           p = p[1, (p.size - 2)] if p[0, 1] == "'" || p[0, 1] == '"'
           p.chomp!('/')
@@ -180,7 +180,7 @@ Puppet::Type.type(:augeas).provide(:augeas) do
       elsif glob_avail && opt_ctx
         # Optimize loading if the context is given, requires the glob function
         # from Augeas 0.8.2 or up
-        ctx_path = resource[:context].sub(/^\/files(.*?)\/?$/, '\1/')
+        ctx_path = resource[:context].sub(%r{^/files(.*?)/?$}, '\1/')
         load_path = "/augeas/load/*['%s' !~ glob(incl) + regexp('/.*')]" % ctx_path
 
         if aug.match(load_path).size < aug.match('/augeas/load/*').size
@@ -198,11 +198,11 @@ Puppet::Type.type(:augeas).provide(:augeas) do
   end
 
   def close_augeas
-    if @aug
-      @aug.close
-      debug('Closed the augeas connection')
-      @aug = nil
-    end
+    return if @aug.nil?
+
+    @aug.close
+    debug('Closed the augeas connection')
+    @aug = nil
   end
 
   def is_numeric?(s)
@@ -444,8 +444,8 @@ Puppet::Type.type(:augeas).provide(:augeas) do
 
           saved_files = @aug.match('/augeas/events/saved')
           if !saved_files.empty?
-            root = resource[:root].sub(/^\/$/, '')
-            saved_files.map! { |key| @aug.get(key).sub(/^\/files/, root) }
+            root = resource[:root].sub(%r{^/$}, '')
+            saved_files.map! { |key| @aug.get(key).sub(%r{^/files}, root) }
             saved_files.uniq.each do |saved_file|
               if Puppet[:show_diff] && @resource[:show_diff]
                 send(@resource[:loglevel], "\n" + diff(saved_file, saved_file + '.augnew'))
@@ -490,6 +490,7 @@ Puppet::Type.type(:augeas).provide(:augeas) do
   end
 
   # Actually execute the augeas changes.
+  # rubocop:disable Style/GuardClause
   def do_execute_changes
     commands = parse_commands(resource[:changes])
     commands.each do |cmd_array|
@@ -568,4 +569,5 @@ Puppet::Type.type(:augeas).provide(:augeas) do
       end
     end
   end
+  # rubocop:enable Style/GuardClause
 end
diff --git a/lib/puppet/type/augeas.rb b/lib/puppet/type/augeas.rb
index fdde4b8..20366f6 100644
--- a/lib/puppet/type/augeas.rb
+++ b/lib/puppet/type/augeas.rb
@@ -52,12 +52,12 @@ Puppet::Type.newtype(:augeas) do
 
   EOT
 
-  newparam (:name) do
+  newparam(:name) do
     desc 'The name of this task. Used for uniqueness.'
     isnamevar
   end
 
-  newparam (:context) do
+  newparam(:context) do
     desc "Optional context path. This value is prepended to the paths of all
       changes if the path is relative. If the `incl` parameter is set,
       defaults to `/files + incl`; otherwise, defaults to the empty string."
@@ -71,7 +71,7 @@ Puppet::Type.newtype(:augeas) do
     end
   end
 
-  newparam (:onlyif) do
+  newparam(:onlyif) do
     desc "Optional augeas command and comparisons to control the execution of this type.
 
       Note: `values` is not an actual augeas API command. It calls `match` to retrieve an array of paths
diff --git a/spec/acceptance/hosts.rb b/spec/acceptance/hosts.rb
index dabec5a..297d923 100644
--- a/spec/acceptance/hosts.rb
+++ b/spec/acceptance/hosts.rb
@@ -11,7 +11,7 @@ test_name 'Augeas hosts file' do
     'windows',
     'cisco_ios', # PUP-7380
   ]
-  confine :to, {}, hosts.reject { |host| host[:roles].include?('master') }
+  confine(:to, {}, hosts.reject { |host| host[:roles].include?('master') })
 
   step 'Backup the hosts file' do
     on hosts, 'cp /etc/hosts /tmp/hosts.bak'
diff --git a/spec/acceptance/puppet.rb b/spec/acceptance/puppet.rb
index 048a3db..2c17238 100644
--- a/spec/acceptance/puppet.rb
+++ b/spec/acceptance/puppet.rb
@@ -7,7 +7,7 @@ test_name 'Augeas puppet configuration' do
   skip_test 'requires augeas which is included in AIO' if @options[:type] != 'aio'
 
   confine :except, platform: 'windows'
-  confine :to, {}, hosts.reject { |host| host[:roles].include?('master') }
+  confine(:to, {}, hosts.reject { |host| host[:roles].include?('master') })
 
   teardown do
     agents.each do |agent|
diff --git a/spec/acceptance/services.rb b/spec/acceptance/services.rb
index 2e48af0..523b953 100644
--- a/spec/acceptance/services.rb
+++ b/spec/acceptance/services.rb
@@ -9,7 +9,7 @@ test_name 'Augeas services file' do
 
   confine :except, platform: 'windows'
   confine :except, platform: 'osx'
-  confine :to, {}, hosts.reject { |host| host[:roles].include?('master') }
+  confine(:to, {}, hosts.reject { |host| host[:roles].include?('master') })
 
   step 'Backup the services file' do
     on hosts, 'cp /etc/services /tmp/services.bak'