Improve rubocop rules and fix issues (#1281)

* Update rubocop config based on ReactOnRails config

* Fix auto-correctable rubocop issues

* Manually resolving additional linting issues

---------

Co-authored-by: Judah Meek <[email protected]>

Author: ahangarha

.github/workflows/rubocop.yml

@@ -0,0 +1,23 @@
+name: Rubocop
+
+on: [push]
+
+jobs:
+  rubocop:
+    name: Rubocop
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ubuntu-latest]
+        ruby: ['2.7', '3.0']
+
+    steps:
+      - uses: actions/checkout@v2
+      - uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: ${{ matrix.ruby }}
+          bundler-cache: true
+      - name: Install linting gems
+        run: gem install rubocop rubocop-performance rubocop-minitest
+      - name: Run rubocop
+        run: rubocop

.rubocop.yml

@@ -1,10 +1,89 @@
+inherit_from: .rubocop_todo.yml
+
+require:
+  - rubocop-performance
+  - rubocop-minitest
+
 AllCops:
-  TargetRubyVersion: 2.3
+  NewCops: enable
+  TargetRubyVersion: 2.5
   DisplayCopNames: true
+
+  Include:
+    - '**/Rakefile'
+    - '**/config.ru'
+    - 'Gemfile'
+    - '**/*.rb'
+    - '**/*.rake'
+
   Exclude:
-    - test/dummy_sprockets/**/*
-    - test/dummy_webpacker1/**/*
-    - test/dummy_webpacker2/**/*
-    - test/dummy_webpacker3/**/*
-    - node_modules/**/*
-    - react_ujs/**/*
+  <% `git status --ignored --porcelain`.lines.grep(/^!! /).each do |path| %>
+    - <%= path.sub(/^!! /, '') %>
+  <% end %>
+    - '**/*.js'
+    - '**/node_modules/**/*'
+    - '**/public/**/*'
+    - '**/tmp/**/*'
+    - 'vendor/**/*'
+    - 'test/dummy_sprockets/**/*'
+    - 'test/dummy_webpacker1/**/*'
+    - 'test/dummy_webpacker2/**/*'
+    - 'test/dummy_webpacker3/**/*'
+    - 'react_ujs/**/*'
+
+Naming/FileName:
+  Exclude:
+    - '**/Gemfile'
+    - '**/Rakefile'
+    - 'lib/react-rails.rb'
+
+Layout/LineLength:
+  Max: 120
+
+Style/StringLiterals:
+  EnforcedStyle: double_quotes
+
+Style/Documentation:
+  Enabled: false
+
+Style/HashEachMethods:
+  Enabled: true
+
+Style/HashTransformKeys:
+  Enabled: true
+
+Style/HashTransformValues:
+  Enabled: true
+
+Metrics/AbcSize:
+  Max: 28
+
+Metrics/CyclomaticComplexity:
+  Max: 7
+
+Metrics/PerceivedComplexity:
+  Max: 10
+
+Metrics/ClassLength:
+  Max: 150
+
+Metrics/ParameterLists:
+  Max: 5
+  CountKeywordArgs: false
+
+Metrics/MethodLength:
+  Max: 41
+
+Metrics/ModuleLength:
+  Max: 180
+
+Naming/RescuedExceptionsVariableName:
+  Enabled: false
+
+# Style/GlobalVars:
+  # Exclude:
+  #   - 'spec/dummy/config/environments/development.rb'
+
+Metrics/BlockLength:
+  Exclude:
+    - 'test/**/*_test.rb'

.rubocop_todo.yml

@@ -0,0 +1,18 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2023-06-30 00:26:13 UTC using RuboCop version 1.53.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.
+
+# Offense count: 2
+Lint/IneffectiveAccessModifier:
+  Exclude:
+    - 'lib/generators/react/component_generator.rb'
+
+# Offense count: 1
+# Configuration parameters: CountComments, CountAsOne.
+Metrics/ClassLength:
+  Exclude:
+    - 'lib/generators/react/component_generator.rb'

Gemfile

@@ -1,4 +1,6 @@
-source 'http://rubygems.org'
+# frozen_string_literal: true
+
+source "http://rubygems.org"
 
 gemspec
 # This is an optional dev-dependency, required whenever sprockets is required

Gemfile.lock

@@ -103,11 +103,9 @@ GEM
     matrix (0.4.2)
     method_source (1.0.0)
     mini_mime (1.0.2)
-    mini_portile2 (2.8.1)
     minitest (5.17.0)
     nenv (0.3.0)
-    nokogiri (1.14.3)
-      mini_portile2 (~> 2.8.0)
+    nokogiri (1.14.3-x86_64-darwin)
       racc (~> 1.4)
     nokogiri (1.14.3-x86_64-linux)
       racc (~> 1.4)
@@ -168,7 +166,7 @@ GEM
     zeitwerk (2.6.6)
 
 PLATFORMS
-  ruby
+  x86_64-darwin-20
   x86_64-linux
 
 DEPENDENCIES

Rakefile

@@ -1,7 +1,9 @@
+# frozen_string_literal: true
+
 begin
-  require 'bundler/setup'
+  require "bundler/setup"
 rescue LoadError
-  puts 'You must `gem install bundler` and `bundle install` to run rake tasks'
+  puts "You must `gem install bundler` and `bundle install` to run rake tasks"
 end
 
 Bundler::GemHelper.install_tasks
@@ -20,22 +22,22 @@ def yarn_run_in(dirname, cmd)
 end
 
 namespace :react do
-  desc 'Run the JS build process to put files in the gem source'
-  task update: [:install, :build, :copy]
+  desc "Run the JS build process to put files in the gem source"
+  task update: %i[install build copy]
 
-  desc 'Install the JavaScript dependencies'
+  desc "Install the JavaScript dependencies"
   task :install do
-    yarn_run_in('react-builds', 'install')
+    yarn_run_in("react-builds", "install")
   end
 
-  desc 'Build the JS bundles with Webpack'
+  desc "Build the JS bundles with Webpack"
   task :build do
-    yarn_run_in('react-builds', 'build')
+    yarn_run_in("react-builds", "build")
   end
 
   desc "Copy browser-ready JS files to the gem's asset paths"
   task :copy do
-    environments = ['development', 'production']
+    environments = %w[development production]
     environments.each do |environment|
       copy_react_asset("#{environment}/react-browser.js", "#{environment}/react.js")
       copy_react_asset("#{environment}/react-server.js", "#{environment}/react-server.js")
@@ -44,48 +46,48 @@ namespace :react do
 end
 
 namespace :ujs do
-  desc 'Run the JS build process to put files in the gem source'
-  task update: [:install, :build, :copy]
+  desc "Run the JS build process to put files in the gem source"
+  task update: %i[install build copy]
 
-  desc 'Install the JavaScript dependencies'
+  desc "Install the JavaScript dependencies"
   task :install do
     `yarn install`
   end
 
-  desc 'Build the JS bundles with Webpack'
+  desc "Build the JS bundles with Webpack"
   task :build do
     `yarn build`
   end
 
   desc "Copy browser-ready JS files to the gem's asset paths"
   task :copy do
-    full_webpack_path = File.expand_path('../react_ujs/dist/react_ujs.js', __FILE__)
-    full_destination_path = File.expand_path('../lib/assets/javascripts/react_ujs.js', __FILE__)
+    full_webpack_path = File.expand_path("react_ujs/dist/react_ujs.js", __dir__)
+    full_destination_path = File.expand_path("lib/assets/javascripts/react_ujs.js", __dir__)
     FileUtils.cp(full_webpack_path, full_destination_path)
   end
 
-  desc 'Publish the package in ./react_ujs/ to npm as `react_ujs`'
+  desc "Publish the package in ./react_ujs/ to npm as `react_ujs`"
   task publish: :update do
     `npm publish`
   end
 end
 
-require 'appraisal'
-require 'minitest/test_task'
+require "appraisal"
+require "minitest/test_task"
 
 Minitest::TestTask.create(:test) do |t|
-  t.libs << 'lib'
-  t.libs << 'test'
-  t.test_globs = ENV['TEST_PATTERN'] || 'test/**/*_test.rb'
-  t.verbose = ENV['TEST_VERBOSE'] == '1'
+  t.libs << "lib"
+  t.libs << "test"
+  t.test_globs = ENV["TEST_PATTERN"] || "test/**/*_test.rb"
+  t.verbose = ENV["TEST_VERBOSE"] == "1"
   t.warning = false
 end
 
 task default: :test
 
 task :test_setup do
-  load 'webdrivers/Rakefile'
-  Dir.chdir('./test/dummy') do
+  load "webdrivers/Rakefile"
+  Dir.chdir("./test/dummy") do
     `yarn install`
   end
 end