Merge pull request #87 from aetherknight/add-rubocop

Add rubocop and address most lints

Author: aetherknight

.github/dependabot.yml

@@ -4,7 +4,9 @@ updates:
     directory: "/"
     schedule:
       interval: "quarterly"
-  - package-ecosystem: "bundler"
-    directory: "/"
-    schedule:
-      interval: "quarterly"
+  # TODO: the bundler dependency updater doesn't seem to handle the Gemfile
+  # using the gemspec? Figure this out and re-enable, if possible
+  # - package-ecosystem: "bundler"
+  #   directory: "/"
+  #   schedule:
+  #     interval: "quarterly"

.rubocop.yml

@@ -0,0 +1,56 @@
+# The behavior of RuboCop can be controlled via the .rubocop.yml
+# configuration file. It makes it possible to enable/disable
+# certain cops (checks) and to alter their behavior if they accept
+# any parameters. The file can be placed either in your home
+# directory or in some project directory.
+#
+# RuboCop will start looking for the configuration file in the directory
+# where the inspected file is and continue its way up to the root directory.
+#
+# See https://docs.rubocop.org/rubocop/configuration
+plugins:
+  - 'rubocop-rake'
+  - 'rubocop-rspec'
+
+Layout/LineLength:
+  Max: 120
+
+Metrics/BlockLength:
+  Exclude:
+    - "spec/**/*.rb"
+    - "*.gemspec"
+
+Metrics/MethodLength:
+  Enabled: false
+
+Naming/FileName:
+  Exclude:
+    # This file is deliberately named with dashes, to match the package name
+    - 'lib/recursive-open-struct.rb'
+
+Style/CommentedKeyword:
+  Exclude:
+    - "spec/**/*.rb"
+
+Style/Documentation:
+  Enabled: false
+
+# Can't really disable this in the gemspec file (it is reported on line 1, but
+# line 1 has the frozen string literal comment)
+Gemspec/RequiredRubyVersion:
+  Enabled: false
+# TODO: should I set a required ruby version? At the moment, the project policy
+# is to officially support supported Ruby versions (and JRuby), but to not
+# officially support no-longer-maintained Ruby versions.
+
+RSpec/ExampleLength:
+  Enabled: false
+RSpec/NestedGroups:
+  Enabled: false
+RSpec/MultipleExpectations:
+  Enabled: false
+
+# Not sure why this one still fails -- the specs are all in a
+# recursive_open_struct directory.
+RSpec/SpecFilePathFormat:
+  Enabled: false

.travis.yml

@@ -49,3 +49,31 @@ When contributing code that changes behavior or fixes bugs, please include unit
 tests to cover the new behavior or to provide regression testing for bugs.
 Also, treat the unit tests as documentation --- make sure they are clean,
 clear, and concise, and well organized.
+
+## Testing and Development
+
+- This project uses RSpec to both document and test behavior. Please structure
+  your tests help someone understand what is being tested.
+- It also uses rubocop to autoformat and lint the project. Run it locally
+  before committing.
+
+You can run both the test suite and the linter with the following commands:
+
+```sh
+# install/update dependencies
+bundle
+
+# run the test suite and linter
+bundle exec rake
+
+# have the linter apply all safe fixes (eg, autoformatting)
+bundle exec rake rubocop:autocorrect
+```
+
+## Release Process
+
+1. Update CHANGELOG.md and lib/recursive_open_struct/version.rb
+2. Run `bundle exec rake update_authors`
+3. Make a version release commit
+4. Run the release task (it will tag, build, push code, push package):
+   `bundle exec rake release`

CONTRIBUTING.md

@@ -1,2 +1,4 @@
+# frozen_string_literal: true
+
 source 'https://rubygems.org'
 gemspec

Gemfile

@@ -1,4 +1,4 @@
-# encoding: utf-8
+# frozen_string_literal: true
 
 require 'rubygems'
 require 'bundler/gem_tasks'
@@ -9,52 +9,56 @@ RSpec::Core::RakeTask.new(:spec) do |spec|
 end
 namespace :spec do
   if RUBY_VERSION =~ /^1\.8/
-    desc "Rspec code coverage (1.8.7)"
+    desc 'Rspec code coverage (1.8.7)'
     RSpec::Core::RakeTask.new(:coverage) do |spec|
       spec.pattern = 'spec/**/*_spec.rb'
       spec.rcov = true
     end
   else
-    desc "Rspec code coverage (1.9+)"
+    desc 'Rspec code coverage (1.9+)'
     task :coverage do
       ENV['COVERAGE'] = 'true'
-      Rake::Task["spec"].execute
+      Rake::Task['spec'].execute
     end
   end
 end
 
+require 'rubocop/rake_task'
+RuboCop::RakeTask.new
+
 require 'rdoc/task'
 Rake::RDocTask.new do |rdoc|
-  version = File.exist?('VERSION') ? File.read('VERSION') : ""
+  version = File.exist?('VERSION') ? File.read('VERSION') : ''
 
   rdoc.rdoc_dir = 'rdoc'
   rdoc.title = "recursive-open-struct #{version}"
   rdoc.rdoc_files.include('README*')
   rdoc.rdoc_files.include('lib/**/*.rb')
 end
 
-task :default => :spec
+task default: %i[spec rubocop]
 
+desc 'Ensure all files have appropriate permissions before building a package'
 task :fix_permissions do
-  File.umask 0022
+  File.umask 0o022
   filelist = `git ls-files`.split("\n")
-  FileUtils.chmod 0644, filelist, :verbose => true
-  FileUtils.chmod 0755, ['lib','spec'], :verbose => true
+  FileUtils.chmod 0o644, filelist, verbose: true
+  FileUtils.chmod 0o755, %w[lib spec], verbose: true
 end
 
-desc "Update the AUTHORS.txt file"
+desc 'Update the AUTHORS.txt file'
 task :update_authors do
   authors = `git log --format="%aN <%aE>"|sort -f|uniq`
   File.open('AUTHORS.txt', 'w') do |f|
     f.write("Recursive-open-struct was written by these fine people:\n\n")
-    f.write(authors.split("\n").map { |a| "* #{a}" }.join( "\n" ))
+    f.write(authors.split("\n").map { |a| "* #{a}" }.join("\n"))
     f.write("\n")
   end
 end
 
-task :build => [:update_authors, :fix_permissions]
+task build: %i[update_authors fix_permissions]
 
-desc "Run an interactive pry shell with ros required"
+desc 'Run an interactive pry shell with ros required'
 task :pry do
-  sh "pry -I lib -r recursive-open-struct"
+  sh 'pry -I lib -r recursive-open-struct'
 end

Rakefile

@@ -1 +1,3 @@
+# frozen_string_literal: true
+
 require 'recursive_open_struct'

lib/recursive-open-struct.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'ostruct'
 require 'recursive_open_struct/version'
 
@@ -12,6 +14,7 @@
 # TODO: `#*_as_a_hash` deprecated. Nested hashes can be referenced using
 # `#to_h`.
 
+# rubocop:disable Metrics/ClassLength
 class RecursiveOpenStruct < OpenStruct
   include Dig if OpenStruct.public_instance_methods.include? :dig
 
@@ -28,7 +31,9 @@ def self.default_options
     }
   end
 
-  def initialize(hash=nil, passed_options={})
+  # rubocop:disable Lint/MissingSuper
+  # Intentionally doesn't call super and initializes +@table+ itself.
+  def initialize(hash = nil, passed_options = {})
     hash = hash.to_h if [hash.is_a?(RecursiveOpenStruct), hash.is_a?(OpenStruct)].any?
     hash ||= {}
 
@@ -40,6 +45,7 @@ def initialize(hash=nil, passed_options={})
 
     @sub_elements = {}
   end
+  # rubocop:enable Lint/MissingSuper
 
   def marshal_load(attributes)
     hash, @options = attributes
@@ -69,17 +75,17 @@ def to_h
 
   # TODO: deprecated, unsupported by OpenStruct. OpenStruct does not consider
   # itself to be a "kind of" Hash.
-  alias_method :to_hash, :to_h
+  alias to_hash to_h
 
   # Continue supporting older rubies -- JRuby 9.1.x.x is still considered
   # stable, but is based on Ruby
   # 2.3.x and so uses :modifiable instead of :modifiable?. Furthermore, if
   # :modifiable is private, then make :modifiable? private too.
-  if !OpenStruct.private_instance_methods.include?(:modifiable?)
+  unless OpenStruct.private_instance_methods.include?(:modifiable?)
     if OpenStruct.private_instance_methods.include?(:modifiable)
-      alias_method :modifiable?, :modifiable
+      alias modifiable? modifiable
     elsif OpenStruct.public_instance_methods.include?(:modifiable)
-      alias_method :modifiable?, :modifiable
+      alias modifiable? modifiable
       private :modifiable?
     end
   end
@@ -89,7 +95,7 @@ def [](name)
     v = @table[key_name]
     if v.is_a?(Hash)
       @sub_elements[key_name] ||= _create_sub_element_(v, mutate_input_hash: true)
-    elsif v.is_a?(Array) and @options[:recurse_over_arrays]
+    elsif v.is_a?(Array) && @options[:recurse_over_arrays]
       @sub_elements[key_name] ||= recurse_over_array(v)
       @sub_elements[key_name] = recurse_over_array(@sub_elements[key_name])
     else
@@ -100,7 +106,7 @@ def [](name)
   if private_instance_methods.include?(:modifiable?) || public_instance_methods.include?(:modifiable?)
     def []=(name, value)
       key_name = _get_key_from_table_(name)
-      tbl = modifiable?  # Ensure we are modifiable
+      tbl = modifiable? # Ensure we are modifiable
       @sub_elements.delete(key_name)
       tbl[key_name] = value
     end
@@ -120,19 +126,20 @@ def respond_to_missing?(mid, include_private = false)
 
   # Adapted implementation of method_missing to accommodate the differences
   # between ROS and OS.
+  # rubocop:disable Metrics/AbcSize
+  # rubocop:disable Metrics/PerceivedComplexity
   def method_missing(mid, *args)
     len = args.length
     if mid =~ /^(.*)=$/
-      if len != 1
-        raise ArgumentError, "wrong number of arguments (#{len} for 1)", caller(1)
-      end
+      raise ArgumentError, "wrong number of arguments (#{len} for 1)", caller(1) if len != 1
+
       # self[$1.to_sym] = args[0]
       # modifiable?[new_ostruct_member!($1.to_sym)] = args[0]
-      new_ostruct_member!($1.to_sym)
+      new_ostruct_member!(::Regexp.last_match(1).to_sym)
       public_send(mid, args[0])
-    elsif len == 0
+    elsif len.zero?
       key = mid
-      key = $1 if key =~ /^(.*)_as_a_hash$/
+      key = ::Regexp.last_match(1) if key =~ /^(.*)_as_a_hash$/
       if @table.key?(_get_key_from_table_(key))
         new_ostruct_member!(key)
         public_send(mid)
@@ -147,6 +154,8 @@ def method_missing(mid, *args)
       raise err
     end
   end
+  # rubocop:enable Metrics/PerceivedComplexity
+  # rubocop:enable Metrics/AbcSize
 
   def freeze
     @table.each_key do |key|
@@ -160,7 +169,7 @@ def freeze
   # 2.4.0.
   def new_ostruct_member(name)
     key_name = _get_key_from_table_(name)
-    unless self.singleton_class.method_defined?(name.to_sym)
+    unless singleton_class.method_defined?(name.to_sym)
       class << self; self; end.class_eval do
         define_method(name) do
           self[key_name]
@@ -185,7 +194,12 @@ class << self; self; end.class_eval do
 
   def delete_field(name)
     sym = _get_key_from_table_(name)
-    singleton_class.__send__(:remove_method, sym, "#{sym}=") rescue NoMethodError # ignore if methods not yet generated.
+    begin
+      singleton_class.__send__(:remove_method, sym, "#{sym}=")
+    rescue StandardError
+      # ignore if methods not yet generated.
+      NoMethodError
+    end
     @sub_elements.delete(sym)
     @table.delete(sym)
   end
@@ -203,8 +217,9 @@ def initialize_dup(orig)
   end
 
   def _get_key_from_table_(name)
-    return name.to_s if @table.has_key?(name.to_s)
-    return name.to_sym if @table.has_key?(name.to_sym)
+    return name.to_s if @table.key?(name.to_s)
+    return name.to_sym if @table.key?(name.to_sym)
+
     name
   end
 
@@ -222,5 +237,5 @@ def recurse_over_array(array)
     end
     array
   end
-
 end
+# rubocop:enable Metrics/ClassLength