From 81e283b28cdd91d259e3b60687aee7ea66e9d05d Mon Sep 17 00:00:00 2001 From: Markus Roberts Date: Fri, 9 Jul 2010 18:06:06 -0700 Subject: Code smell: Line modifiers are preferred to one-line blocks. * 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] --- lib/puppet/parser/functions/generate.rb | 4 +--- lib/puppet/parser/functions/include.rb | 4 +--- lib/puppet/parser/functions/shellquote.rb | 4 +--- lib/puppet/parser/functions/split.rb | 4 +--- lib/puppet/parser/functions/sprintf.rb | 4 +--- 5 files changed, 5 insertions(+), 15 deletions(-) (limited to 'lib/puppet/parser/functions') diff --git a/lib/puppet/parser/functions/generate.rb b/lib/puppet/parser/functions/generate.rb index cf46775ab..8430f03a6 100644 --- a/lib/puppet/parser/functions/generate.rb +++ b/lib/puppet/parser/functions/generate.rb @@ -11,9 +11,7 @@ Puppet::Parser::Functions::newfunction(:generate, :type => :rvalue, generators, so all shell metacharacters are passed directly to the generator.") do |args| - unless args[0] =~ /^#{File::SEPARATOR}/ - raise Puppet::ParseError, "Generators must be fully qualified" - end + raise Puppet::ParseError, "Generators must be fully qualified" unless args[0] =~ /^#{File::SEPARATOR}/ unless args[0] =~ /^[-#{File::SEPARATOR}\w.]+$/ raise Puppet::ParseError, diff --git a/lib/puppet/parser/functions/include.rb b/lib/puppet/parser/functions/include.rb index a8b6f175b..d1bafa54a 100644 --- a/lib/puppet/parser/functions/include.rb +++ b/lib/puppet/parser/functions/include.rb @@ -12,9 +12,7 @@ Puppet::Parser::Functions::newfunction(:include, :doc => "Evaluate one or more c unless missing.empty? # Throw an error if we didn't evaluate all of the classes. str = "Could not find class" - if missing.length > 1 - str += "es" - end + str += "es" if missing.length > 1 str += " " + missing.join(", ") diff --git a/lib/puppet/parser/functions/shellquote.rb b/lib/puppet/parser/functions/shellquote.rb index 0a7ae5958..96feaa1ee 100644 --- a/lib/puppet/parser/functions/shellquote.rb +++ b/lib/puppet/parser/functions/shellquote.rb @@ -26,9 +26,7 @@ module Puppet::Parser::Functions else r = '"' word.each_byte() do |c| - if Dangerous.include?(c) - r += "\\" - end + r += "\\" if Dangerous.include?(c) r += c.chr() end r += '"' diff --git a/lib/puppet/parser/functions/split.rb b/lib/puppet/parser/functions/split.rb index c3466ba4a..09caa18aa 100644 --- a/lib/puppet/parser/functions/split.rb +++ b/lib/puppet/parser/functions/split.rb @@ -22,9 +22,7 @@ a regexp meta-character (.), and that needs protection. A simple way to do that for a single character is to enclose it in square brackets.") do |args| - if args.length != 2 - raise Puppet::ParseError, ("split(): wrong number of arguments (#{args.length}; must be 2)") - end + raise Puppet::ParseError, ("split(): wrong number of arguments (#{args.length}; must be 2)") if args.length != 2 return args[0].split(Regexp.compile(args[1])) end diff --git a/lib/puppet/parser/functions/sprintf.rb b/lib/puppet/parser/functions/sprintf.rb index 4a53a9736..4a3916c89 100644 --- a/lib/puppet/parser/functions/sprintf.rb +++ b/lib/puppet/parser/functions/sprintf.rb @@ -6,9 +6,7 @@ module Puppet::Parser::Functions :doc => "Perform printf-style formatting of text. The first parameter is format string describing how the rest of the parameters should be formatted. See the documentation for the ``Kernel::sprintf()`` function in Ruby for all the details.") do |args| - if args.length < 1 - raise Puppet::ParseError, 'sprintf() needs at least one argument' - end + raise Puppet::ParseError, 'sprintf() needs at least one argument' if args.length < 1 fmt = args.shift() return sprintf(fmt, *args) end -- cgit