Update to enable Rubocop and contains Rubocop fixes in lib/ (#974)

* Rubocop fixes

Author: Helen

.rubocop.yml

@@ -3,3 +3,8 @@ appveyor.yml:
   delete: true
 spec/spec_helper.rb:
   allow_deprecations: true
+.travis.yml:
+  extras:
+  - rvm: 2.1.9
+    bundler_args: --without system_tests
+    script: bundle exec rubocop lib  

.sync.yml

@@ -28,5 +28,8 @@ matrix:
   - rvm: 2.1.9
     bundler_args: --without system_tests
     env: PUPPET_GEM_VERSION="~> 4.0"
+  - rvm: 2.1.9
+    bundler_args: --without system_tests
+    script: bundle exec rubocop lib 
 notifications:
   email: false

.travis.yml

@@ -1,9 +1,13 @@
-def get_mysql_id
-  Facter.value(:macaddress).split(':')[2..-1].inject(0) { |total,value| (total << 6) + value.hex }
+def mysql_id_get
+  Facter.value(:macaddress).split(':')[2..-1].reduce(0) { |total, value| (total << 6) + value.hex }
 end
 
-Facter.add("mysql_server_id") do
+Facter.add('mysql_server_id') do
   setcode do
-    get_mysql_id rescue nil
+    begin
+      mysql_id_get
+    rescue
+      nil
+    end
   end
 end

lib/facter/mysql_server_id.rb

@@ -1,8 +1,6 @@
-Facter.add("mysql_version") do
+Facter.add('mysql_version') do
   setcode do
     mysql_ver = Facter::Util::Resolution.exec('mysql --version')
-    if mysql_ver
-      mysql_ver.match(/\d+\.\d+\.\d+/)[0]
-    end
+    mysql_ver.match(%r{\d+\.\d+\.\d+})[0] if mysql_ver
   end
 end

lib/facter/mysql_version.rb

@@ -1,4 +1,4 @@
-Facter.add("mysqld_version") do
+Facter.add('mysqld_version') do
   setcode do
     Facter::Util::Resolution.exec('mysqld -V 2>/dev/null')
   end

lib/facter/mysqld_version.rb

@@ -1,5 +1,6 @@
+# Recursively merges two or more hashes together and returns the resulting hash.
 module Puppet::Parser::Functions
-  newfunction(:mysql_deepmerge, :type => :rvalue, :doc => <<-'ENDHEREDOC') do |args|
+  newfunction(:mysql_deepmerge, type: :rvalue, doc: <<-'ENDHEREDOC') do |args|
     Recursively merges two or more hashes together and returns the resulting hash.
 
     For example:
@@ -18,12 +19,12 @@ module Puppet::Parser::Functions
     ENDHEREDOC
 
     if args.length < 2
-      raise Puppet::ParseError, ("mysql_deepmerge(): wrong number of arguments (#{args.length}; must be at least 2)")
+      raise Puppet::ParseError, "mysql_deepmerge(): wrong number of arguments (#{args.length}; must be at least 2)"
     end
 
-    result = Hash.new
+    result = {}
     args.each do |arg|
-      next if arg.is_a? String and arg.empty? # empty string is synonym for puppet's undef
+      next if arg.is_a?(String) && arg.empty? # empty string is synonym for puppet's undef
       # If the argument was not a hash, skip it.
       unless arg.is_a?(Hash)
         raise Puppet::ParseError, "mysql_deepmerge: unexpected argument type #{arg.class}, only expects hash arguments"
@@ -32,25 +33,25 @@ module Puppet::Parser::Functions
       # Now we have to traverse our hash assigning our non-hash values
       # to the matching keys in our result while following our hash values
       # and repeating the process.
-      overlay( result, arg )
+      overlay(result, arg)
     end
-    return( result )
+    return(result)
   end
 end
 
-def has_normalized!(hash, key)
-  return true if hash.has_key?( key )
-  return false unless key.match(/-|_/)
-  other_key = key.include?('-') ? key.gsub( '-', '_' ) : key.gsub( '_', '-' )
-  return false unless hash.has_key?( other_key )
-  hash[key] = hash.delete( other_key )
-  return true;
+def normalized?(hash, key)
+  return true if hash.key?(key)
+  return false unless key =~ %r{-|_}
+  other_key = key.include?('-') ? key.tr('-', '_') : key.tr('_', '-')
+  return false unless hash.key?(other_key)
+  hash[key] = hash.delete(other_key)
+  true
 end
 
-def overlay( hash1, hash2 )
+def overlay(hash1, hash2)
   hash2.each do |key, value|
-    if(has_normalized!( hash1, key ) and value.is_a?(Hash) and hash1[key].is_a?(Hash))
-      overlay( hash1[key], value )
+    if normalized?(hash1, key) && value.is_a?(Hash) && hash1[key].is_a?(Hash)
+      overlay(hash1[key], value)
     else
       hash1[key] = value
     end

lib/puppet/parser/functions/mysql_deepmerge.rb

@@ -1,11 +1,14 @@
+# Returns the dirname of a path.
 module Puppet::Parser::Functions
-  newfunction(:mysql_dirname, :type => :rvalue, :doc => <<-EOS
+  newfunction(:mysql_dirname, type: :rvalue, doc: <<-EOS
     Returns the dirname of a path.
     EOS
-  ) do |arguments|
+             ) do |arguments|
 
-    raise(Puppet::ParseError, "mysql_dirname(): Wrong number of arguments " +
-      "given (#{arguments.size} for 1)") if arguments.size < 1
+    if arguments.empty?
+      raise(Puppet::ParseError, 'mysql_dirname(): Wrong number of arguments ' \
+        "given (#{arguments.size} for 1)")
+    end
 
     path = arguments[0]
     return File.dirname(path)

lib/puppet/parser/functions/mysql_dirname.rb

@@ -1,17 +1,19 @@
-# hash a string as mysql's "PASSWORD()" function would do it
 require 'digest/sha1'
-
+# Returns the mysql password hash from the clear text password.
+# Hash a string as mysql's "PASSWORD()" function would do it
 module Puppet::Parser::Functions
-  newfunction(:mysql_password, :type => :rvalue, :doc => <<-EOS
+  newfunction(:mysql_password, type: :rvalue, doc: <<-EOS
     Returns the mysql password hash from the clear text password.
     EOS
-  ) do |args|
+             ) do |args|
 
-    raise(Puppet::ParseError, 'mysql_password(): Wrong number of arguments ' +
-      "given (#{args.size} for 1)") if args.size != 1
+    if args.size != 1
+      raise(Puppet::ParseError, 'mysql_password(): Wrong number of arguments ' \
+        "given (#{args.size} for 1)")
+    end
 
     return '' if args[0].empty?
-    return args[0] if args[0] =~ /\*[A-F0-9]{40}$/
+    return args[0] if args[0] =~ %r{\*[A-F0-9]{40}$}
     '*' + Digest::SHA1.hexdigest(Digest::SHA1.digest(args[0])).upcase
   end
 end

lib/puppet/parser/functions/mysql_password.rb

@@ -1,21 +1,19 @@
+# When given a hash this function strips out all blank entries.
 module Puppet::Parser::Functions
-  newfunction(:mysql_strip_hash, :type => :rvalue, :arity => 1, :doc => <<-EOS
-TEMPORARY FUNCTION: EXPIRES 2014-03-10
-When given a hash this function strips out all blank entries.
+  newfunction(:mysql_strip_hash, type: :rvalue, arity: 1, doc: <<-EOS
+  TEMPORARY FUNCTION: EXPIRES 2014-03-10
+  When given a hash this function strips out all blank entries.
 EOS
-  ) do |args|
+             ) do |args|
 
     hash = args[0]
     unless hash.is_a?(Hash)
       raise(Puppet::ParseError, 'mysql_strip_hash(): Requires hash to work with')
     end
 
     # Filter out all the top level blanks.
-    hash.reject{|k,v| v == ''}.each do |k,v|
-      if v.is_a?(Hash)
-        v.reject!{|ki,vi| vi == '' }
-      end
+    hash.reject { |_k, v| v == '' }.each do |_k, v|
+      v.reject! { |_ki, vi| vi == '' } if v.is_a?(Hash)
     end
-
   end
 end

lib/puppet/parser/functions/mysql_strip_hash.rb

@@ -1,37 +1,37 @@
+# Puppet provider for mysql
 class Puppet::Provider::Mysql < Puppet::Provider
-
   # Without initvars commands won't work.
   initvars
 
   # Make sure we find mysql commands on CentOS and FreeBSD
-  ENV['PATH']=ENV['PATH'] + ':/usr/libexec:/usr/local/libexec:/usr/local/bin'
+  ENV['PATH'] = ENV['PATH'] + ':/usr/libexec:/usr/local/libexec:/usr/local/bin'
 
+  # rubocop:disable Style/HashSyntax
   commands :mysql      => 'mysql'
   commands :mysqld     => 'mysqld'
   commands :mysqladmin => 'mysqladmin'
+  # rubocop:enable Style/HashSyntax
 
   # Optional defaults file
   def self.defaults_file
-    if File.file?("#{Facter.value(:root_home)}/.my.cnf")
-      "--defaults-extra-file=#{Facter.value(:root_home)}/.my.cnf"
-    else
-      nil
-    end
+    "--defaults-extra-file=#{Facter.value(:root_home)}/.my.cnf" if File.file?("#{Facter.value(:root_home)}/.my.cnf")
   end
 
   def self.mysqld_type
     # find the mysql "dialect" like mariadb / mysql etc.
-    mysqld_version_string.scan(/mariadb/i) { return "mariadb" }
-    mysqld_version_string.scan(/\s\(percona/i) { return "percona" }
-    return "mysql"
+    mysqld_version_string.scan(%r{mariadb}i) { return 'mariadb' }
+    mysqld_version_string.scan(%r{\s\(percona}i) { return 'percona' }
+    'mysql'
   end
 
   def mysqld_type
     self.class.mysqld_type
   end
 
   def self.mysqld_version_string
-    # As the possibility of the mysqld being remote we need to allow the version string to be overridden, this can be done by facter.value as seen below. In the case that it has not been set and the facter value is nil we use the mysql -v command to ensure we report the correct version of mysql for later use cases.
+    # As the possibility of the mysqld being remote we need to allow the version string to be overridden,
+    # this can be done by facter.value as seen below. In the case that it has not been set and the facter
+    # value is nil we use the mysql -v command to ensure we report the correct version of mysql for later use cases.
     @mysqld_version_string ||= Facter.value(:mysqld_version) || mysqld('-V')
   end
 
@@ -43,7 +43,7 @@ def self.mysqld_version
     # note: be prepared for '5.7.6-rc-log' etc results
     #       versioncmp detects 5.7.6-log to be newer then 5.7.6
     #       this is why we need the trimming.
-    mysqld_version_string.scan(/\d+\.\d+\.\d+/).first unless mysqld_version_string.nil?
+    mysqld_version_string.scan(%r{\d+\.\d+\.\d+}).first unless mysqld_version_string.nil?
   end
 
   def mysqld_version
@@ -77,39 +77,33 @@ def self.cmd_table(table)
     table_string = ''
 
     # We can't escape *.* so special case this.
-    if table == '*.*'
-      table_string << '*.*'
-    # Special case also for PROCEDURES
-    elsif table.start_with?('PROCEDURE ')
-      table_string << table.sub(/^PROCEDURE (.*)(\..*)/, 'PROCEDURE `\1`\2')
-    else
-      table_string << table.sub(/^(.*)(\..*)/, '`\1`\2')
-    end
+    table_string << if table == '*.*'
+                      '*.*'
+                    # Special case also for PROCEDURES
+                    elsif table.start_with?('PROCEDURE ')
+                      table.sub(%r{^PROCEDURE (.*)(\..*)}, 'PROCEDURE `\1`\2')
+                    else
+                      table.sub(%r{^(.*)(\..*)}, '`\1`\2')
+                    end
     table_string
   end
 
   def self.cmd_privs(privileges)
-    if privileges.include?('ALL')
-      return 'ALL PRIVILEGES'
-    else
-      priv_string = ''
-      privileges.each do |priv|
-        priv_string << "#{priv}, "
-      end
+    return 'ALL PRIVILEGES' if privileges.include?('ALL')
+    priv_string = ''
+    privileges.each do |priv|
+      priv_string << "#{priv}, "
     end
     # Remove trailing , from the last element.
-    priv_string.sub(/, $/, '')
+    priv_string.sub(%r{, $}, '')
   end
 
   # Take in potential options and build up a query string with them.
   def self.cmd_options(options)
     option_string = ''
     options.each do |opt|
-      if opt == 'GRANT'
-        option_string << ' WITH GRANT OPTION'
-      end
+      option_string << ' WITH GRANT OPTION' if opt == 'GRANT'
     end
     option_string
   end
-
 end

lib/puppet/provider/mysql.rb

@@ -1,21 +1,20 @@
 require File.expand_path(File.join(File.dirname(__FILE__), '..', 'mysql'))
-Puppet::Type.type(:mysql_database).provide(:mysql, :parent => Puppet::Provider::Mysql) do
+Puppet::Type.type(:mysql_database).provide(:mysql, parent: Puppet::Provider::Mysql) do
   desc 'Manages MySQL databases.'
 
-  commands :mysql => 'mysql'
+  commands mysql: 'mysql'
 
   def self.instances
-    mysql([defaults_file, '-NBe', 'show databases'].compact).split("\n").collect do |name|
+    mysql([defaults_file, '-NBe', 'show databases'].compact).split("\n").map do |name|
       attributes = {}
       mysql([defaults_file, '-NBe', "show variables like '%_database'", name].compact).split("\n").each do |line|
-        k,v = line.split(/\s/)
+        k, v = line.split(%r{\s})
         attributes[k] = v
       end
-      new(:name    => name,
-          :ensure  => :present,
-          :charset => attributes['character_set_database'],
-          :collate => attributes['collation_database']
-         )
+      new(name: name,
+          ensure: :present,
+          charset: attributes['character_set_database'],
+          collate: attributes['collation_database'])
     end
   end
 
@@ -24,9 +23,8 @@ def self.instances
   def self.prefetch(resources)
     databases = instances
     resources.keys.each do |database|
-      if provider = databases.find { |db| db.name == database }
-        resources[database].provider = provider
-      end
+      provider = databases.find { |db| db.name == database }
+      resources[database].provider = provider if provider
     end
   end
 
@@ -56,13 +54,12 @@ def exists?
   def charset=(value)
     mysql([defaults_file, '-NBe', "alter database `#{resource[:name]}` CHARACTER SET #{value}"].compact)
     @property_hash[:charset] = value
-    charset == value ? (return true) : (return false)
+    (charset == value) ? (return true) : (return false)
   end
 
   def collate=(value)
     mysql([defaults_file, '-NBe', "alter database `#{resource[:name]}` COLLATE #{value}"].compact)
     @property_hash[:collate] = value
-    collate == value ? (return true) : (return false)
+    (collate == value) ? (return true) : (return false)
   end
-
 end