Merge pull request #48 from ripienaar/misc_rubocop

(misc) update some rubocop styles

lib/mcollective/ddl/agentddl.rb

@@ -37,7 +37,7 @@ module DDL
     #         aggregate summary(:value)
     #     end
     # end
-    class AgentDDL<Base
+    class AgentDDL < Base
       def initialize(plugin, plugintype=:agent, loadddl=true)
         @process_aggregate_functions = nil
 
@@ -65,18 +65,18 @@ def input(argument, properties)
       def summarize(&block)
         unless @config.mode == :server
           @process_aggregate_functions = true
-          block.call
+          yield
           @process_aggregate_functions = nil
         end
       end
 
       # Sets the aggregate array for the given action
-      def aggregate(function, format = {:format => nil})
+      def aggregate(function, format={:format => nil})
         raise(DDLValidationError, "Formats supplied to aggregation functions should be a hash") unless format.is_a?(Hash)
         raise(DDLValidationError, "Formats supplied to aggregation functions must have a :format key") unless format.keys.include?(:format)
         raise(DDLValidationError, "Functions supplied to aggregate should be a hash") unless function.is_a?(Hash)
 
-        unless (function.keys.include?(:args)) && function[:args]
+        unless function.keys.include?(:args) && function[:args]
           raise DDLValidationError, "aggregate method for action '%s' missing a function parameter" % entities[@current_entity][:action]
         end
 
@@ -133,19 +133,21 @@ def action(name, input, &block)
         # we set @current_entity so the input block can know what its talking
         # to, this is probably an epic hack, need to improve.
         @current_entity = name
-        block.call if block_given?
+        yield if block_given?
         @current_entity = nil
       end
 
       # If the method name matches a # aggregate function, we return the function
       # with args as a hash.  This will only be active if the @process_aggregate_functions
       # is set to true which only happens in the #summarize block
       def method_missing(name, *args, &block)
-        unless @process_aggregate_functions || is_function?(name)
-          raise NoMethodError, "undefined local variable or method `#{name}'", caller
-        end
+        super unless @process_aggregate_functions || is_function?(name)
+
+        {:function => name, :args => args}
+      end
 
-        return {:function => name, :args => args}
+      def respond_to_missing?(method, *)
+        @process_aggregate_functions || is_function?(name) || super
       end
 
       # Checks if a method name matches a aggregate plugin.
@@ -169,7 +171,7 @@ def set_default_input_arguments(action, arguments)
 
         return unless input
 
-        input.keys.each do |key|
+        input.each_key do |key|
           if key.is_a?(Symbol) && arguments.include?(key.to_s) && !input.include?(key.to_s)
             compat_arg = key.to_s
           else
@@ -228,7 +230,7 @@ def validate_rpc_request(action, arguments)
         input = action_interface(action)[:input] || {}
         compatible_args = symbolize_basic_input_arguments(input, arguments)
 
-        input.keys.each do |key|
+        input.each_key do |key|
           unless input[key][:optional]
             unless compatible_args.include?(key)
               raise DDLValidationError, "Action #{action} needs a #{key} argument"

lib/mcollective/ddl/base.rb

@@ -42,7 +42,7 @@ def initialize(plugin, plugintype=:agent, loadddl=true)
       # the config dir and if that does not exist, default to
       # /etc/mcollective
       def help(template=nil)
-        template = template_for_plugintype unless template
+        template ||= template_for_plugintype
         template = Util.templatepath(template) unless Util.absolute_path?(template)
 
         template = File.read(template)
@@ -52,28 +52,29 @@ def help(template=nil)
         unless template == "metadata-help.erb"
           metadata_template = Util.templatepath("metadata-help.erb")
           metadata_template = File.read(metadata_template)
-          metastring = ERB.new(metadata_template, 0, '%')
+          metastring = ERB.new(metadata_template, 0, "%")
           metastring = metastring.result(binding)
         end
 
-        erb = ERB.new(template, 0, '%')
+        erb = ERB.new(template, 0, "%")
         erb.result(binding)
       end
 
+      # rubocop:disable Lint/DuplicateMethods, Style/TrivialAccessors
       def usage(usage_text)
         @usage = usage_text
       end
 
       def template_for_plugintype
         case @plugintype
         when :agent
-          return "rpc-help.erb"
+          "rpc-help.erb"
         else
-          if File.exists?(Util.templatepath("#{@plugintype}-help.erb"))
-            return "#{@plugintype}-help.erb"
+          if File.exist?(Util.templatepath("#{@plugintype}-help.erb"))
+            "#{@plugintype}-help.erb"
           else
             # Default help template gets loaded if plugintype-help does not exist.
-            return "metadata-help.erb"
+            "metadata-help.erb"
           end
         end
       end
@@ -91,8 +92,8 @@ def loadddlfile
       end
 
       def findddlfile(ddlname=nil, ddltype=nil)
-        ddlname = @pluginname unless ddlname
-        ddltype = @plugintype unless ddltype
+        ddlname ||= @pluginname
+        ddltype ||= @plugintype
 
         @config.libdir.each do |libdir|
           ddlfile = File.join([libdir, "mcollective", ddltype.to_s, "#{ddlname}.ddl"])
@@ -170,17 +171,17 @@ def input(argument, properties)
                                                :optional => properties[:optional]}
 
         case properties[:type]
-          when :string
-            raise "Input type :string needs a :validation argument" unless properties.include?(:validation)
-            raise "Input type :string needs a :maxlength argument" unless properties.include?(:maxlength)
+        when :string
+          raise "Input type :string needs a :validation argument" unless properties.include?(:validation)
+          raise "Input type :string needs a :maxlength argument" unless properties.include?(:maxlength)
 
-            @entities[entity][:input][argument][:validation] = properties[:validation]
-            @entities[entity][:input][argument][:maxlength] = properties[:maxlength]
+          @entities[entity][:input][argument][:validation] = properties[:validation]
+          @entities[entity][:input][argument][:maxlength] = properties[:maxlength]
 
-          when :list
-            raise "Input type :list needs a :list argument" unless properties.include?(:list)
+        when :list
+          raise "Input type :list needs a :list argument" unless properties.include?(:list)
 
-            @entities[entity][:input][argument][:list] = properties[:list]
+          @entities[entity][:input][argument][:list] = properties[:list]
         end
       end
 
@@ -206,7 +207,7 @@ def requires(requirement)
 
         valid_requirements = [:mcollective]
 
-        requirement.keys.each do |key|
+        requirement.each_key do |key|
           unless valid_requirements.include?(key)
             raise "Requirement %s is not a valid requirement, only %s is supported" % [key, valid_requirements.join(", ")]
           end

lib/mcollective/ddl/dataddl.rb

@@ -27,17 +27,17 @@ module DDL
     #              :display_as => item.to_s.capitalize
     #     end
     # end
-    class DataDDL<Base
+    class DataDDL < Base
       def dataquery(input, &block)
         raise "Data queries need a :description" unless input.include?(:description)
         raise "Data queries can only have one definition" if @entities[:data]
 
-        @entities[:data]  = {:description => input[:description],
-                             :input => {},
-                             :output => {}}
+        @entities[:data] = {:description => input[:description],
+                            :input => {},
+                            :output => {}}
 
         @current_entity = :data
-        block.call if block_given?
+        yield if block_given?
         @current_entity = nil
       end
 

lib/mcollective/ddl/discoveryddl.rb

@@ -13,7 +13,7 @@ module DDL
     # discovery do
     #     capabilities [:classes, :facts, :identity, :agents, :compound]
     # end
-    class DiscoveryDDL<Base
+    class DiscoveryDDL < Base
       def discovery_interface
         @entities[:discovery]
       end
@@ -44,7 +44,7 @@ def discovery(&block)
         @entities[:discovery] = {:capabilities => []}
 
         @current_entity = :discovery
-        block.call if block_given?
+        yield if block_given?
         @current_entity = nil
       end
     end

lib/mcollective/ddl/validatorddl.rb

@@ -1,6 +1,6 @@
 module MCollective
   module DDL
-    class ValidatorDDL<Base
+    class ValidatorDDL < Base
     end
   end
 end

lib/mcollective/pluginpackager/agent_definition.rb

@@ -16,7 +16,7 @@ def initialize(configuration, mcdependency, plugintype)
         @dependencies = configuration[:dependency] || []
         @target_path = File.expand_path(@path)
         @metadata, mcversion = PluginPackager.get_metadata(@path, "agent")
-        @mcname = mcdependency[:mcname] ||  "mcollective"
+        @mcname = mcdependency[:mcname] || "mcollective"
         @mcversion = mcdependency[:mcversion] || mcversion
         @metadata[:version] = (configuration[:version] || @metadata[:version])
         @dependencies << {:name => "#{@mcname}-common", :version => @mcversion}
@@ -76,7 +76,7 @@ def client
       # Obtain common package files and dependencies.
       def common
         common = {
-          :files =>[],
+          :files => [],
           :dependencies => @dependencies.clone,
           :description => "Common libraries for #{@metadata[:name]}"
         }
@@ -92,7 +92,7 @@ def common
 
         # We fail if there is no ddl file present
         if common[:files].grep(/^.*\.ddl$/).empty?
-          raise "cannot create package - No ddl file found in #{File.join(@path, "agent")}"
+          raise "cannot create package - No ddl file found in #{File.join(@path, 'agent')}"
         end
 
         common[:files].uniq!

lib/mcollective/pluginpackager/forge_packager.rb

@@ -3,11 +3,11 @@
 module MCollective
   module PluginPackager
     class ForgePackager
-      def initialize(plugin, pluginpath = nil, signature = nil, verbose = false, keep_artifacts = nil, module_template = nil)
+      def initialize(plugin, pluginpath=nil, signature=nil, verbose=false, keep_artifacts=nil, module_template=nil)
         @plugin = plugin
         @verbose = verbose
         @keep_artifacts = keep_artifacts
-        @module_template = module_template || File.join(File.dirname(__FILE__), 'templates', 'forge')
+        @module_template = module_template || File.join(File.dirname(__FILE__), "templates", "forge")
       end
 
       def which(cmd)
@@ -18,7 +18,7 @@ def which(cmd)
             return exe if File.executable?(exe) && !File.directory?(exe)
           end
         end
-        return nil
+        nil
       end
 
       def create_packages
@@ -44,7 +44,7 @@ def create_packages
         ensure
           if @keep_artifacts
             puts("Keeping build artifacts")
-            puts("Build artifacts saved in %s" %  @tmpdir)
+            puts("Build artifacts saved in %s" % @tmpdir)
           else
             cleanup_tmpdirs
           end
@@ -113,7 +113,7 @@ def plugin_hiera_data
           hierakey(:server_files) => filelist(:agent),
           hierakey(:server_directories) => dirlist(:agent),
           hierakey(:client_files) => filelist(:client),
-          hierakey(:client_directories) => dirlist(:client),
+          hierakey(:client_directories) => dirlist(:client)
         }.merge(module_override_data)
       end
 
@@ -132,7 +132,7 @@ def copy_additional_files
       end
 
       def copy_module_files
-        @plugin.packagedata.each do |klass, data|
+        @plugin.packagedata.each_value do |data|
           data[:files].each do |file|
             clean_dest_file = file.gsub("./lib/mcollective", "")
             dest_dir = File.expand_path(File.join(@tmpdir, "files", "mcollective", File.dirname(clean_dest_file)))
@@ -162,7 +162,7 @@ def generate_agent_json_ddls
             data = {
               "$schema" => "https://choria.io/schemas/mcorpc/ddl/v1/agent.json",
               "metadata" => ddl.meta,
-              "actions" => [],
+              "actions" => []
             }
 
             ddl.actions.sort.each do |action|
@@ -179,7 +179,7 @@ def generate_agent_json_ddls
       end
 
       def render_templates
-        templates = Dir.chdir(@module_template) do |path|
+        templates = Dir.chdir(@module_template) do |_path|
           Dir.glob("**/*.erb")
         end
 
@@ -191,25 +191,23 @@ def render_templates
       end
 
       def render_template(infile, outfile)
-        begin
-          erb = ERB.new(File.read(infile), nil, "-")
-          File.open(outfile, "w") do |f|
-            f.puts erb.result(binding)
-          end
-        rescue
-          STDERR.puts("Could not render template %s to %s" % [infile, outfile])
-          raise
+        erb = ERB.new(File.read(infile), nil, "-")
+        File.open(outfile, "w") do |f|
+          f.puts erb.result(binding)
         end
+      rescue
+        STDERR.puts("Could not render template %s to %s" % [infile, outfile])
+        raise
       end
 
       def validate_environment
         raise("Supplying a vendor is required, please use --vendor") if @plugin.vendor == "Puppet Labs"
-        raise("Vendor names may not have a space in them, please specify a valid vendor using --vendor") if @plugin.vendor.match(" ")
+        raise("Vendor names may not have a space in them, please specify a valid vendor using --vendor") if @plugin.vendor.include?(" ")
       end
 
       def pdk_path
         pdk_bin = which("pdk")
-        pdk_bin = "/opt/puppetlabs/pdk/bin/pdk" unless pdk_bin
+        pdk_bin ||= "/opt/puppetlabs/pdk/bin/pdk"
 
         pdk_bin
       end
@@ -226,35 +224,29 @@ def assert_new_enough_pdk
       end
 
       def run_build
-        begin
-          PluginPackager.execute_verbosely(@verbose) do
-            Dir.chdir(@tmpdir) do
-              PluginPackager.safe_system("#{pdk_path} build --force")
-            end
+        PluginPackager.execute_verbosely(@verbose) do
+          Dir.chdir(@tmpdir) do
+            PluginPackager.safe_system("#{pdk_path} build --force")
           end
-        rescue
-          STDERR.puts("Build process has failed")
-          raise
         end
+      rescue
+        STDERR.puts("Build process has failed")
+        raise
       end
 
       def move_package
-        begin
-          package_file = File.join(@tmpdir, "pkg", module_file_name)
-          FileUtils.cp(package_file, ".")
-        rescue
-          STDERR.puts("Could not copy package to working directory")
-          raise
-        end
+        package_file = File.join(@tmpdir, "pkg", module_file_name)
+        FileUtils.cp(package_file, ".")
+      rescue
+        STDERR.puts("Could not copy package to working directory")
+        raise
       end
 
       def cleanup_tmpdirs
-        begin
-          FileUtils.rm_r(@tmpdir) if File.directory?(@tmpdir)
-        rescue
-          STDERR.puts("Could not remove temporary build directory %s" % [@tmpdir])
-          raise
-        end
+        FileUtils.rm_r(@tmpdir) if File.directory?(@tmpdir)
+      rescue
+        STDERR.puts("Could not remove temporary build directory %s" % [@tmpdir])
+        raise
       end
     end
   end