Merge pull request #135 from aws/rubocop

Run rubocop and clean up

.rubocop.yml

@@ -1,9 +1,17 @@
+inherit_from: .rubocop_todo.yml
+
 AllCops:
-  TargetRubyVersion: 2.0.0
+  NewCops: enable
+  TargetRubyVersion: 2.3
+  Exclude:
+    - 'tasks/release/**/*'
 
-Documentation:
+Gemspec/RequireMFA:
   Enabled: false
 
+Layout/LineLength:
+  Max: 120
+
 Metrics/AbcSize:
   Enabled: false
 
@@ -15,9 +23,6 @@ Metrics/BlockLength:
 Metrics/ClassLength:
   Enabled: false
 
-Metrics/LineLength:
-  Max: 120
-
 Metrics/MethodLength:
   Enabled: false
 
@@ -36,5 +41,11 @@ Style/BlockDelimiters:
   Exclude:
     - 'spec/**/*'
 
+Style/Documentation:
+  Enabled: false
+
+Style/GlobalVars:
+  Enabled: false
+
 Style/TrivialAccessors:
   Enabled: false

.rubocop_todo.yml

@@ -0,0 +1,54 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2023-09-27 19:58:03 UTC using RuboCop version 1.56.3.
+# 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.
+
+# Offense count: 1
+# Configuration parameters: Severity, Include.
+# Include: **/*.gemspec
+Gemspec/RequiredRubyVersion:
+  Exclude:
+    - 'aws-record.gemspec'
+
+# Offense count: 3
+# Configuration parameters: AllowedMethods.
+# AllowedMethods: enums
+Lint/ConstantDefinitionInBlock:
+  Exclude:
+    - 'features/searching/step_definitions.rb'
+    - 'features/table_config/step_definitions.rb'
+    - 'spec/aws-record/record/batch_spec.rb'
+
+# Offense count: 3
+# Configuration parameters: AllowedMethods, AllowedPatterns.
+Metrics/CyclomaticComplexity:
+  Max: 11
+
+# Offense count: 3
+# Configuration parameters: AllowedMethods, AllowedPatterns.
+Metrics/PerceivedComplexity:
+  Max: 12
+
+# Offense count: 1
+# Configuration parameters: ExpectMatchingDefinition, CheckDefinitionPathHierarchy, CheckDefinitionPathHierarchyRoots, Regex, IgnoreExecutableScripts, AllowedAcronyms.
+# CheckDefinitionPathHierarchyRoots: lib, spec, test, src
+# AllowedAcronyms: CLI, DSL, ACL, API, ASCII, CPU, CSS, DNS, EOF, GUID, HTML, HTTP, HTTPS, ID, IP, JSON, LHS, QPS, RAM, RHS, RPC, SLA, SMTP, SQL, SSH, TCP, TLS, TTL, UDP, UI, UID, UUID, URI, URL, UTF8, VM, XML, XMPP, XSRF, XSS
+Naming/FileName:
+  Exclude:
+    - 'lib/aws-record.rb'
+
+# Offense count: 4
+# Configuration parameters: MinNameLength, AllowNamesEndingInNumbers, AllowedNames, ForbiddenNames.
+# AllowedNames: as, at, by, cc, db, id, if, in, io, ip, of, on, os, pp, to
+Naming/MethodParameterName:
+  Exclude:
+    - 'lib/aws-record/record/buildable_search.rb'
+    - 'lib/aws-record/record/table_config.rb'
+
+# Offense count: 1
+Style/OpenStructUse:
+  Exclude:
+    - 'lib/aws-record/record/transactions.rb'

CHANGELOG.md

@@ -1,7 +1,9 @@
 Unreleased Changes
 ------------------
 
-* Issue - Fix default value for String/Numeric Sets to be unset. (#133) 
+* Issue - Fix default value for String/Numeric Sets to be unset. (#133)
+
+* Issue - Run Rubocop on all files. (#135)
 
 2.11.0 (2023-06-02)
 ------------------

Gemfile

@@ -1,20 +1,20 @@
+# frozen_string_literal: true
+
 source 'https://rubygems.org'
 
 gemspec
 
 gem 'rake', require: false
 
 group :test do
-  gem 'rspec'
   gem 'cucumber'
-
+  gem 'rspec'
+
   gem 'simplecov', require: false
 
-  if RUBY_VERSION >= '3.0'
-    gem 'rexml'
-  end
+  gem 'rexml' if RUBY_VERSION >= '3.0'
 
-  if ENV["NEW_RAILS"]
+  if ENV['NEW_RAILS']
     gem 'activemodel'
   else
     gem 'activemodel', '< 5.0'
@@ -31,7 +31,5 @@ group :release do
 end
 
 group :development do
-  # rubocop's TargetRubyVersion is 2.0.0
-  # Ruby version required less than 3.0
-  # gem 'rubocop', '0.50.0'
+  gem 'rubocop'
 end

Rakefile

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'rspec/core/rake_task'
 
 $REPO_ROOT = File.dirname(__FILE__)
@@ -13,7 +15,7 @@ RSpec::Core::RakeTask.new do |t|
   t.rspec_opts = "-I #{$REPO_ROOT}/lib -I #{$REPO_ROOT}/spec"
   t.pattern = "#{$REPO_ROOT}/spec"
 end
-task :spec => 'test:coverage:clear'
+task spec: 'test:coverage:clear'
 task 'test:unit' => :spec # alias old names
 
 task 'cucumber' do
@@ -22,7 +24,7 @@ end
 
 # Ensure the test:integration task behaves as it always has
 desc 'run integration tests'
-task 'test:integration' do |t|
+task 'test:integration' do
   if ENV['AWS_INTEGRATION']
     Rake::Task['cucumber'].invoke
   else
@@ -33,8 +35,8 @@ end
 
 # Setup alias for old task names
 task 'test:unit' => :spec
-task :test => %w[test:unit test:integration]
-task :default => :test
+task test: %w[test:unit test:integration]
+task default: :test
 
 task 'release:test' => :spec
 

aws-record.gemspec

@@ -1,16 +1,16 @@
 # frozen_string_literal: true
 
 Gem::Specification.new do |spec|
-  spec.name          = "aws-record"
-  spec.version       = File.read(File.expand_path('../VERSION', __FILE__)).strip
-  spec.authors       = ["Amazon Web Services"]
-  spec.email         = ["mamuller@amazon.com", "alexwoo@amazon.com"]
-  spec.summary       = "AWS Record library for Amazon DynamoDB"
-  spec.description   = "Provides an object mapping abstraction for Amazon DynamoDB."
-  spec.homepage      = "https://github.com/aws/aws-sdk-ruby-record"
-  spec.license       = "Apache 2.0"
+  spec.name          = 'aws-record'
+  spec.version       = File.read(File.expand_path('VERSION', __dir__)).strip
+  spec.authors       = ['Amazon Web Services']
+  spec.email         = ['aws-dr-rubygems@amazon.com']
+  spec.summary       = 'AWS Record library for Amazon DynamoDB'
+  spec.description   = 'Provides an object mapping abstraction for Amazon DynamoDB.'
+  spec.homepage      = 'https://github.com/aws/aws-sdk-ruby-record'
+  spec.license       = 'Apache 2.0'
 
-  spec.require_paths = ["lib"]
+  spec.require_paths = ['lib']
   spec.files = Dir['lib/**/*.rb', 'LICENSE', 'CHANGELOG.md', 'VERSION']
 
   # Require 1.85.0 for user_agent_frameworks config

features/indexes/step_definitions.rb

@@ -10,7 +10,7 @@
 Then(/^the table should have a local secondary index named "([^"]*)"$/) do |expected|
   resp = @client.describe_table(table_name: @table_name)
   lsis = resp.table.local_secondary_indexes
-  exists = lsis && lsis.any? { |index| index.index_name == expected }
+  exists = lsis&.any? { |index| index.index_name == expected }
   expect(exists).to eq(true)
 end
 
@@ -25,6 +25,6 @@
 Then(/^the table should have a global secondary index named "([^"]*)"$/) do |expected|
   resp = @client.describe_table(table_name: @table_name)
   gsis = resp.table.global_secondary_indexes
-  exists = gsis && gsis.any? { |index| index.index_name == expected }
+  exists = gsis&.any? { |index| index.index_name == expected }
   expect(exists).to eq(true)
 end

features/items/step_definitions.rb

@@ -23,18 +23,18 @@
   @model.update(data)
 end
 
-Then(/^we should receive an aws\-record item with attribute data:$/) do |string|
+Then(/^we should receive an aws-record item with attribute data:$/) do |string|
   data = JSON.parse(string, symbolize_names: true)
   data.each do |key, value|
     expect(@instance.send(key)).to eq(value)
   end
 end
 
-When(/^we call 'delete!' on the aws\-record item instance$/) do
+When(/^we call 'delete!' on the aws-record item instance$/) do
   @instance.delete!
 end
 
-When(/^we call 'update' on the aws\-record item instance with parameter data:$/) do |string|
+When(/^we call 'update' on the aws-record item instance with parameter data:$/) do |string|
   data = JSON.parse(string, symbolize_names: true)
   @instance.update(data)
 end
@@ -63,7 +63,7 @@
   expect(actual).to eq(expected)
 end
 
-When(/^we call "([^"]*)" on aws\-record item instance(?: with an integer value of "(-?\d+)")?$/) do |method, value|
+When(/^we call "([^"]*)" on aws-record item instance(?: with an integer value of "(-?\d+)")?$/) do |method, value|
   if value
     @instance.send(method, value)
   else

features/migrations/step_definitions.rb

@@ -18,7 +18,7 @@
 end
 
 Then(/^calling 'table_exists\?' on the model should return "([^"]*)"$/) do |b|
-  boolean = b == 'false' || b.nil? ? false : true
+  boolean = !(b == 'false' || b.nil?)
   expect(@model.table_exists?).to eq(boolean)
 end
 

features/searching/step_definitions.rb

@@ -5,7 +5,7 @@
   @collection = @model.query(data)
 end
 
-Then(/^we should receive an aws\-record collection with members:$/) do |string|
+Then(/^we should receive an aws-record collection with members:$/) do |string|
   expected = JSON.parse(string, symbolize_names: true)
   # Ensure that we have the same number of items, and no pagination.
   expect(expected.size).to eq(@collection.to_a.size)
@@ -29,14 +29,14 @@
   @instance = @collection.first
 end
 
-Then(/^we should receive an aws\-record page with 2 values from members:$/) do |string|
+Then(/^we should receive an aws-record page with 2 values from members:$/) do |string|
   expected = JSON.parse(string, symbolize_names: true)
   page = @collection.page
   @last_evaluated_key = @collection.last_evaluated_key
   # This is definitely a hack which takes advantage of an accident in test
   # design. In the future, we'll need to have some sort of shared collection
   # state to cope with the fact that scan order is not guaranteed.
-  page.size == 2 # rubocop:disable Void
+  page.size == 2 # rubocop:disable Lint/Void
   # Results do not have guaranteed order, check each expected value individually
   page.each do |item|
     h = item.to_h

features/step_definitions.rb

@@ -93,7 +93,7 @@ def cleanup_table
   expect(resp.item).to eq(nil)
 end
 
-Given(/^an aws\-record model with data:$/) do |string|
+Given(/^an aws-record model with data:$/) do |string|
   data = JSON.parse(string)
   @model = Class.new do
     include(Aws::Record)
@@ -115,7 +115,7 @@ def cleanup_table
   expect(@model.send(method)).to eq(expected)
 end
 
-Given(/^an aws\-record model with definition:$/) do |string|
+Given(/^an aws-record model with definition:$/) do |string|
   @model = Class.new do
     include(Aws::Record)
   end

lib/aws-record/record.rb

@@ -117,14 +117,13 @@ module RecordClassMethods
       #   MyOtherTable.table_name # => "test_MyTable"
       def table_name
         # rubocop:disable Style/RedundantSelf
-        @table_name ||= begin
-          if Aws::Record.extends_record?(self) &&
-             default_table_name(self.superclass) != self.superclass.table_name
-            self.superclass.instance_variable_get('@table_name')
-          else
-            default_table_name(self)
-          end
-        end
+        @table_name ||= if Aws::Record.extends_record?(self) &&
+                           default_table_name(self.superclass) != self.superclass.table_name
+                          self.superclass.instance_variable_get('@table_name')
+                        else
+                          default_table_name(self)
+                        end
+
         # rubocop:enable Style/RedundantSelf
       end
 
@@ -247,6 +246,7 @@ def model_valid?
 
       def default_table_name(klass)
         return unless klass.name
+
         klass.name.split('::').join('_')
       end
     end

lib/aws-record/record/attribute.rb

@@ -41,11 +41,9 @@ def initialize(name, options = {})
         @marshaler = options[:marshaler] || DefaultMarshaler
         @persist_nil = options[:persist_nil]
         @default_value_or_lambda = if options.key?(:default_value)
-          dv = options[:default_value]
-          _is_lambda?(dv) ? dv : type_cast(dv)
-        else
-          nil
-        end
+                                     dv = options[:default_value]
+                                     _is_lambda?(dv) ? dv : type_cast(dv)
+                                   end
       end
 
       # Attempts to type cast a raw value into the attribute's type. This call