RuboCop lint rules, linting, TODO config added

.rubocop.yml

@@ -0,0 +1,22 @@
+inherit_from: .rubocop_todo.yml
+
+AllCops:
+  DisplayCopNames: true
+  TargetRubyVersion: 2.2
+  Exclude:
+    - 'features/**/*'
+    - 'vendor/**/*'
+Metrics/LineLength:
+  Max: 120
+Metrics/BlockLength:
+  Exclude:
+    - 'spec/**/*'
+Style/Documentation:
+  Enabled: false
+Style/FrozenStringLiteralComment:
+  Enabled: false
+# Use double quotes everywhere by default.
+Style/StringLiterals:
+  EnforcedStyle: double_quotes
+
+

.rubocop_todo.yml

@@ -0,0 +1,7 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2017-08-17 12:53:11 +0200 using RuboCop version 0.49.1.
+# The point is for the user to remove these configuration records
+# one by one as the offenses are removed from the code base.
+# Note that changes in the inspected code, or installation of new
+# versions of RuboCop, may require this file to be generated again.

Gemfile

@@ -3,6 +3,7 @@ source "https://rubygems.org"
 gemspec
 
 group :development do
-  gem "rdoc"
   gem "bundler"
+  gem "rdoc"
+  gem "rubocop", "~> 0.49.1"
 end

Rakefile

@@ -7,10 +7,10 @@ RSpec::Core::RakeTask.new(:spec) do |spec|
   spec.pattern = "spec/**/*_spec.rb"
 end
 
-task default: [:style, :spec, :features]
+task default: %i{style spec features}
 
 # For rubygems-test
-task :test => :spec
+task test: :spec
 
 RDoc::Task.new do |rdoc|
   rdoc.rdoc_dir = "rdoc"
@@ -39,5 +39,6 @@ GitHubChangelogGenerator::RakeTask.new :changelog do |config|
   config.future_release = Mixlib::Log::VERSION
   config.enhancement_labels = "enhancement,Enhancement,New Feature,Feature".split(",")
   config.bug_labels = "bug,Bug,Improvement,Upstream Bug".split(",")
-  config.exclude_labels = "duplicate,question,invalid,wontfix,no_changelog,Exclude From Changelog,Question,Discussion".split(",")
+  config.exclude_labels = ["duplicate", "question", "invalid", "wontfix",
+                           "no_changelog", "Exclude From Changelog", "Question", "Discussion"]
 end

lib/mixlib/log.rb

@@ -22,15 +22,16 @@
 
 module Mixlib
   module Log
-
     @logger, @loggers = nil
 
-    LEVELS = { :debug => Logger::DEBUG, :info => Logger::INFO, :warn => Logger::WARN, :error => Logger::ERROR, :fatal => Logger::FATAL }.freeze
+    LEVELS = { debug: Logger::DEBUG, info: Logger::INFO,
+               warn: Logger::WARN, error: Logger::ERROR, fatal: Logger::FATAL }.freeze
     LEVEL_NAMES = LEVELS.invert.freeze
 
     def reset!
       close!
-      @logger, @loggers = nil, nil
+      @logger = nil
+      @loggers = nil
     end
 
     # An Array of log devices that will be logged to. Defaults to just the default
@@ -58,13 +59,12 @@ def use_log_devices(other)
       if other.respond_to?(:loggers) && other.respond_to?(:logger)
         @loggers = other.loggers
         @logger = other.logger
-      elsif other.kind_of?(Array)
+      elsif other.is_a?(Array)
         @loggers = other
         @logger = other.first
       else
-        msg = "#use_log_devices takes a Mixlib::Log object or array of log devices. " <<
-          "You gave: #{other.inspect}"
-        raise ArgumentError, msg
+        raise ArgumentError, "#use_log_devices takes a Mixlib::Log object or array of log devices. " \
+              "You gave: #{other.inspect}"
       end
       @configured = true
     end
@@ -79,7 +79,7 @@ def use_log_devices(other)
     def init(*opts)
       reset!
       @logger = logger_for(*opts)
-      @logger.formatter = Mixlib::Log::Formatter.new() if @logger.respond_to?(:formatter=)
+      @logger.formatter = Mixlib::Log::Formatter.new if @logger.respond_to?(:formatter=)
       @logger.level = Logger::WARN
       @configured = true
       @logger
@@ -109,13 +109,13 @@ def level(new_level = nil)
       if new_level.nil?
         LEVEL_NAMES[logger.level]
       else
-        self.level = (new_level)
+        self.level = new_level
       end
     end
 
     # Define the standard logger methods on this class programmatically.
     # No need to incur method_missing overhead on every log call.
-    [:debug, :info, :warn, :error, :fatal].each do |method_name|
+    %i{debug info warn error fatal}.each do |method_name|
       class_eval(<<-METHOD_DEFN, __FILE__, __LINE__)
         def #{method_name}(msg=nil, &block)
           loggers.each {|l| l.#{method_name}(msg, &block) }
@@ -127,7 +127,7 @@ def #{method_name}(msg=nil, &block)
     # Note that we *only* query the default logger (@logger) and not any other
     # loggers that may have been added, even though it is possible to configure
     # two (or more) loggers at different log levels.
-    [:debug?, :info?, :warn?, :error?, :fatal?].each do |method_name|
+    %i{debug? info? warn? error? fatal?}.each do |method_name|
       class_eval(<<-METHOD_DEFN, __FILE__, __LINE__)
         def #{method_name}
           logger.#{method_name}
@@ -143,7 +143,7 @@ def add(severity, message = nil, progname = nil, &block)
       loggers.each { |l| l.add(severity, message, progname, &block) }
     end
 
-    alias :log :add
+    alias log add
 
     # Passes any other method calls on directly to the underlying Logger object created with init. If
     # this method gets hit before a call to Mixlib::Logger.init has been made, it will call
@@ -185,9 +185,12 @@ def loggers_to_close
     def close!
       # try to close all file loggers
       loggers_to_close.each do |l|
-        l.close rescue nil
+        begin
+          l.close
+        rescue
+          nil
+        end
       end
     end
-
   end
 end

lib/mixlib/log/formatter.rb

@@ -29,9 +29,9 @@ def self.show_time=(show = false)
 
       # Prints a log message as '[time] severity: message' if Chef::Log::Formatter.show_time == true.
       # Otherwise, doesn't print the time.
-      def call(severity, time, progname, msg)
+      def call(severity, time, _progname, msg)
         if @@show_time
-          sprintf("[%s] %s: %s\n", time.iso8601(), severity, msg2str(msg))
+          sprintf("[%s] %s: %s\n", time.iso8601, severity, msg2str(msg))
         else
           sprintf("%s: %s\n", severity, msg2str(msg))
         end

lib/mixlib/log/version.rb

@@ -1,5 +1,5 @@
 module Mixlib
   module Log
-    VERSION = "1.7.1"
+    VERSION = "1.7.1".freeze
   end
 end

mixlib-log.gemspec

@@ -1,4 +1,4 @@
-$:.unshift File.expand_path("../lib", __FILE__)
+$LOAD_PATH.unshift File.expand_path("../lib", __FILE__)
 require "mixlib/log/version"
 
 Gem::Specification.new do |gem|
@@ -16,7 +16,7 @@ Gem::Specification.new do |gem|
   gem.required_ruby_version = ">= 2.2"
   gem.add_development_dependency "rake"
   gem.add_development_dependency "rspec", "~> 3.4"
-  gem.add_development_dependency "chefstyle"
+  gem.add_development_dependency "chefstyle", ">= 0.6"
   gem.add_development_dependency "cucumber"
   gem.add_development_dependency "github_changelog_generator", ">= 1.11.3"
 end

spec/mixlib/log/formatter_spec.rb

@@ -34,7 +34,7 @@
   end
 
   it "should format random objects via inspect with msg2str(Object)" do
-    expect(@formatter.msg2str([ "black thought", "?uestlove" ])).to eq('["black thought", "?uestlove"]')
+    expect(@formatter.msg2str(["black thought", "?uestlove"])).to eq('["black thought", "?uestlove"]')
   end
 
   it "should return a formatted string with call" do
@@ -47,5 +47,4 @@
     Mixlib::Log::Formatter.show_time = false
     expect(@formatter.call("monkey", Time.new, "test", "mos def")).to eq("monkey: mos def\n")
   end
-
 end

spec/mixlib/log_spec.rb

@@ -28,7 +28,7 @@ def initialize
     @messages = ""
   end
 
-  [:debug, :info, :warn, :error, :fatal].each do |method_name|
+  %i{debug info warn error fatal}.each do |method_name|
     class_eval(<<-E)
       def #{method_name}(message)
         @messages << message
@@ -38,7 +38,6 @@ def #{method_name}(message)
 end
 
 describe Mixlib::Log do
-
   # Since we are testing class behaviour for an instance variable
   # that gets set once, we need to reset it prior to each example [cb]
   before(:each) do
@@ -69,7 +68,8 @@ def #{method_name}(message)
   end
 
   it "should re-initialize the logger if init is called again" do
-    first_logdev, second_logdev = StringIO.new, StringIO.new
+    first_logdev = StringIO.new
+    second_logdev = StringIO.new
     Logit.init(first_logdev)
     Logit.fatal "FIRST"
     expect(first_logdev.string).to match(/FIRST/)
@@ -86,11 +86,11 @@ def #{method_name}(message)
 
   it "should set the log level using the binding form,  with :debug, :info, :warn, :error, or :fatal" do
     levels = {
-      :debug => Logger::DEBUG,
-      :info  => Logger::INFO,
-      :warn  => Logger::WARN,
-      :error => Logger::ERROR,
-      :fatal => Logger::FATAL,
+      debug: Logger::DEBUG,
+      info: Logger::INFO,
+      warn: Logger::WARN,
+      error: Logger::ERROR,
+      fatal: Logger::FATAL,
     }
     levels.each do |symbol, constant|
       Logit.level = symbol
@@ -108,11 +108,11 @@ def #{method_name}(message)
 
   it "should set the log level using the method form, with :debug, :info, :warn, :error, or :fatal" do
     levels = {
-      :debug => Logger::DEBUG,
-      :info  => Logger::INFO,
-      :warn  => Logger::WARN,
-      :error => Logger::ERROR,
-      :fatal => Logger::FATAL,
+      debug: Logger::DEBUG,
+      info: Logger::INFO,
+      warn: Logger::WARN,
+      error: Logger::ERROR,
+      fatal: Logger::FATAL,
     }
     levels.each do |symbol, constant|
       Logit.level(symbol)
@@ -121,25 +121,25 @@ def #{method_name}(message)
   end
 
   it "should raise an ArgumentError if you try and set the level to something strange using the binding form" do
-    expect(lambda { Logit.level = :the_roots }).to raise_error(ArgumentError)
+    expect(-> { Logit.level = :the_roots }).to raise_error(ArgumentError)
   end
 
   it "should raise an ArgumentError if you try and set the level to something strange using the method form" do
-    expect(lambda { Logit.level(:the_roots) }).to raise_error(ArgumentError)
+    expect(-> { Logit.level(:the_roots) }).to raise_error(ArgumentError)
   end
 
   it "should pass other method calls directly to logger" do
     Logit.level = :debug
     expect(Logit).to be_debug
-    expect(lambda { Logit.debug("Gimme some sugar!") }).to_not raise_error
+    expect(-> { Logit.debug("Gimme some sugar!") }).to_not raise_error
   end
 
   it "should pass add method calls directly to logger" do
     logdev = StringIO.new
     Logit.init(logdev)
     Logit.level = :debug
     expect(Logit).to be_debug
-    expect(lambda { Logit.add(Logger::DEBUG, "Gimme some sugar!") }).to_not raise_error
+    expect(-> { Logit.add(Logger::DEBUG, "Gimme some sugar!") }).to_not raise_error
     expect(logdev.string).to match(/Gimme some sugar/)
   end
 
@@ -186,5 +186,4 @@ def #{method_name}(message)
     end
     expect(opened_files_count_after).to eq(opened_files_count_before + 1)
   end
-
 end

spec/spec_helper.rb

@@ -18,7 +18,7 @@
 #
 
 $TESTING = true
-$:.push File.join(File.dirname(__FILE__), "..", "lib")
+$LOAD_PATH.push File.join(File.dirname(__FILE__), "..", "lib")
 
 require "rspec"
 require "mixlib/log"