Merge pull request #866 from Floppy/add-rubocop

Add rubocop to lint Rails and RSpec usage

.github/workflows/push.yml

@@ -44,6 +44,6 @@ jobs:
     - name: Install yarn packages
       run: yarn install
     - name: Lint Ruby code
-      run: bundle exec standardrb
+      run: bundle exec rubocop
     - name: Lint Typescript code
       run: yarn run ts-standard

.husky/pre-commit

@@ -1,5 +1,5 @@
 #!/bin/sh
 . "$(dirname "$0")/_/husky.sh"
 
-bundle exec standardrb
-yarn run ts-standard
+bundle exec rubocop --autocorrect
+yarn run lint:ts

.rubocop.yml

@@ -0,0 +1,19 @@
+inherit_from: .rubocop_todo.yml
+
+require:
+  - standard
+  - rubocop-rails
+  - rubocop-rspec
+
+inherit_gem:
+  standard: config/base.yml
+
+AllCops:
+  NewCops: enable
+  Exclude:
+    - db/schema.rb
+    - bin/*
+    - 'node_modules/**/*'
+    - 'tmp/**/*'
+    - 'vendor/**/*'
+    - '.git/**/*'

.rubocop_todo.yml

@@ -0,0 +1,90 @@
+# Offense count: 6
+RSpec/BeforeAfterAll:
+  Exclude:
+    - 'spec/jobs/library_scan_job_spec.rb'
+    - 'spec/jobs/model_file_scan_job_spec.rb'
+    - 'spec/jobs/model_scan_job_spec.rb'
+    - 'spec/requests/creators_request_spec.rb'
+    - 'spec/requests/libraries_request_spec.rb'
+    - 'spec/requests/models_request_spec.rb'
+
+# Offense count: 4
+# Configuration parameters: Prefixes, AllowedPatterns.
+# Prefixes: when, with, without
+RSpec/ContextWording:
+  Exclude:
+    - 'spec/jobs/model_scan_job_spec.rb'
+    - 'spec/models/model_spec.rb'
+
+# Offense count: 6
+# Configuration parameters: CountAsOne.
+RSpec/ExampleLength:
+  Max: 11
+
+# Offense count: 29
+# Configuration parameters: AssignmentOnly.
+RSpec/InstanceVariable:
+  Exclude:
+    - 'spec/models/model_spec.rb'
+    - 'spec/requests/libraries_request_spec.rb'
+    - 'spec/requests/models_request_spec.rb'
+
+# Offense count: 14
+RSpec/MultipleExpectations:
+  Max: 5
+
+# Offense count: 4
+RSpec/RepeatedExample:
+  Exclude:
+    - 'spec/models/model_spec.rb'
+
+# Offense count: 1
+# Configuration parameters: Include.
+# Include: db/migrate/*.rb
+Rails/CreateTableWithTimestamps:
+  Exclude:
+    - 'db/migrate/20220612220116_create_active_storage_variant_records.active_storage.rb'
+
+# Offense count: 2
+Rails/I18nLocaleTexts:
+  Exclude:
+    - 'app/admin/delayed_job.rb'
+
+# Offense count: 1
+# Configuration parameters: Include.
+# Include: db/migrate/*.rb
+Rails/NotNullColumn:
+  Exclude:
+    - 'db/migrate/20220614211256_add_username_to_users.rb'
+
+# Offense count: 4
+Rails/OutputSafety:
+  Exclude:
+    - 'app/admin/delayed_job.rb'
+    - 'app/helpers/application_helper.rb'
+    - 'config/initializers/field_with_errors.rb'
+
+# Offense count: 1
+# Configuration parameters: Include.
+# Include: db/**/*.rb
+Rails/ReversibleMigration:
+  Exclude:
+    - 'db/migrate/20220106220519_remove_images_table.rb'
+
+# Offense count: 2
+# Configuration parameters: ForbiddenMethods, AllowedMethods.
+# ForbiddenMethods: decrement!, decrement_counter, increment!, increment_counter, insert, insert!, insert_all, insert_all!, toggle!, touch, touch_all, update_all, update_attribute, update_column, update_columns, update_counters, upsert, upsert_all
+Rails/SkipsModelValidations:
+  Exclude:
+    - 'app/admin/delayed_job.rb'
+    - 'db/migrate/20220612220115_add_service_name_to_active_storage_blobs.active_storage.rb'
+
+# Offense count: 4
+# Configuration parameters: Include.
+# Include: app/models/**/*.rb
+Rails/UniqueValidationWithoutIndex:
+  Exclude:
+    - 'app/models/library.rb'
+    - 'app/models/model.rb'
+    - 'app/models/model_file.rb'
+    - 'app/models/problem.rb'

Gemfile

@@ -43,6 +43,8 @@ group :development, :test do
   gem "guard", "~> 2.18"
   gem "guard-rspec", "~> 4.7"
   gem "database_cleaner-active_record", "~> 2.0"
+  gem "rubocop-rails", require: false
+  gem "rubocop-rspec", require: false
 end
 
 group :development do

Gemfile.lock

@@ -327,6 +327,12 @@ GEM
     rubocop-performance (1.15.1)
       rubocop (>= 1.7.0, < 2.0)
       rubocop-ast (>= 0.4.0)
+    rubocop-rails (2.17.3)
+      activesupport (>= 4.2.0)
+      rack (>= 1.1)
+      rubocop (>= 1.33.0, < 2.0)
+    rubocop-rspec (2.15.0)
+      rubocop (~> 1.33)
     ruby-progressbar (1.11.0)
     ruby2_keywords (0.0.5)
     sass-rails (6.0.0)
@@ -412,6 +418,8 @@ DEPENDENCIES
   rails-settings-cached (~> 2.8)
   redis (~> 5.0)
   rspec-rails
+  rubocop-rails
+  rubocop-rspec
   sass-rails (>= 6)
   spring
   sqlite3 (~> 1.5)

app/admin/delayed_job.rb

@@ -56,7 +56,7 @@
     column(:created_at, sortable: :created_at) { |job| job.created_at.iso8601.tr("T", " ") }
     column(:run_at, sortable: :run_at) { |post| post.run_at.present? ? post.run_at.iso8601.tr("T", " ") : nil }
     column :queue
-    column("Running", sortable: :locked_at) { |dj| dj.locked_at.present? ? "#{(Time.now - dj.locked_at).round(1)}s by #{dj.locked_by}" : "" }
+    column("Running", sortable: :locked_at) { |dj| dj.locked_at.present? ? "#{(Time.zone.now - dj.locked_at).round(1)}s by #{dj.locked_by}" : "" }
     actions
   end
 

app/admin/log.rb

@@ -4,7 +4,7 @@
     h2 "log/#{Rails.env}.log"
     div style: "padding: 0px 1em; background: #eee; border: 1px solid black; width: 90vw; overflow-x: scroll" do
       pre do
-        File.readlines(Rails.root.join("log/#{Rails.env}.log")).split("\n") do |line|
+        Rails.root.join("log/#{Rails.env}.log").readlines.split("\n") do |line|
           text_node line
         end
       end

app/controllers/creators_controller.rb

@@ -20,26 +20,26 @@ def show
     end
   end
 
-  def edit
-    @creator.links.build if @creator.links.empty? # populate empty link
-  end
-
-  def update
-    @creator.update(creator_params)
-    redirect_to @creator
-  end
-
   def new
     @creator = Creator.new
     @creator.links.build if @creator.links.empty? # populate empty link
     @title = "New Creator"
   end
 
+  def edit
+    @creator.links.build if @creator.links.empty? # populate empty link
+  end
+
   def create
     @creator = Creator.create(creator_params)
     redirect_to @creator
   end
 
+  def update
+    @creator.update(creator_params)
+    redirect_to @creator
+  end
+
   def destroy
     @creator.destroy
     redirect_to creators_path

app/controllers/libraries_controller.rb

@@ -30,7 +30,7 @@ def show
 
     # Filter by tag?
     if params[:tag]
-      @tag = ActsAsTaggableOn::Tag.find_by_name(params[:tag])
+      @tag = ActsAsTaggableOn::Tag.find_by(name: params[:tag])
       @models = @models.tagged_with(@tag) if @tag
     end
   end

app/controllers/settings_controller.rb

@@ -28,7 +28,7 @@ def update
   private
 
   def get_user
-    @user = User.find_by_username(params[:user_id])
+    @user = User.find_by(username: params[:user_id])
   end
 
   def check_owner_permission

app/models/model.rb

@@ -36,7 +36,7 @@ def autogenerate_tags_from_path!
 
   def parents
     Pathname.new(path).parent.descend.map do |path|
-      library.models.find_by_path(path.to_s)
+      library.models.find_by(path: path.to_s)
     end.compact
   end
   memoize :parents

db/data/20220614214418_create_default_admin_user.rb

@@ -10,6 +10,6 @@ def up
   end
 
   def down
-    User.find_by_username("admin").destroy
+    User.find_by(username: "admin").destroy
   end
 end

db/migrate/20210302161202_drop_link_fields_from_creators.rb

@@ -1,8 +1,8 @@
 class DropLinkFieldsFromCreators < ActiveRecord::Migration[6.1]
   def change
-    remove_column :creators, :thingiverse_user
-    remove_column :creators, :cults3d_user
-    remove_column :creators, :mmf_user
-    remove_column :creators, :cgtrader_user
+    remove_column :creators, :thingiverse_user, :string
+    remove_column :creators, :cults3d_user, :string
+    remove_column :creators, :mmf_user, :string
+    remove_column :creators, :cgtrader_user, :string
   end
 end

spec/factories/model.rb

@@ -2,6 +2,6 @@
   factory :model do
     name { Faker::Creature::Animal.name }
     path { Faker::File.dir }
-    library { build :library }
+    library { build(:library) }
   end
 end

spec/factories/model_file.rb

@@ -1,6 +1,6 @@
 FactoryBot.define do
   factory :model_file do
     filename { Faker::File.file_name(ext: "stl") }
-    model { build :model }
+    model { build(:model) }
   end
 end

spec/helpers/collections_helper_spec.rb

@@ -10,6 +10,6 @@
 #     end
 #   end
 # end
-RSpec.describe CollectionsHelper, type: :helper do
+RSpec.describe CollectionsHelper do
   pending "add some examples to (or delete) #{__FILE__}"
 end

spec/helpers/creators_helper_spec.rb

@@ -10,6 +10,6 @@
 #     end
 #   end
 # end
-RSpec.describe CreatorsHelper, type: :helper do
+RSpec.describe CreatorsHelper do
   pending "add some examples to (or delete) #{__FILE__}"
 end

spec/helpers/libraries_helper_spec.rb

@@ -10,6 +10,6 @@
 #     end
 #   end
 # end
-RSpec.describe LibrariesHelper, type: :helper do
+RSpec.describe LibrariesHelper do
   pending "add some examples to (or delete) #{__FILE__}"
 end

spec/helpers/model_files_helper_spec.rb

@@ -10,6 +10,6 @@
 #     end
 #   end
 # end
-RSpec.describe ModelFilesHelper, type: :helper do
+RSpec.describe ModelFilesHelper do
   pending "add some examples to (or delete) #{__FILE__}"
 end

spec/helpers/models_helper_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe ModelsHelper, type: :helper do
+RSpec.describe ModelsHelper do
   describe "string concat" do
     let(:files) do
       %w[

spec/helpers/problems_helper_spec.rb

@@ -10,6 +10,6 @@
 #     end
 #   end
 # end
-RSpec.describe ProblemsHelper, type: :helper do
+RSpec.describe ProblemsHelper do
   pending "add some examples to (or delete) #{__FILE__}"
 end

spec/helpers/search_helper_spec.rb

@@ -10,6 +10,6 @@
 #     end
 #   end
 # end
-RSpec.describe SearchHelper, type: :helper do
+RSpec.describe SearchHelper do
   pending "add some examples to (or delete) #{__FILE__}"
 end

spec/helpers/settings_helper_spec.rb

@@ -10,6 +10,6 @@
 #     end
 #   end
 # end
-RSpec.describe SettingsHelper, type: :helper do
+RSpec.describe SettingsHelper do
   pending "add some examples to (or delete) #{__FILE__}"
 end

spec/jobs/library_scan_job_spec.rb

@@ -1,37 +1,37 @@
 require "rails_helper"
 
-RSpec.describe LibraryScanJob, type: :job do
+RSpec.describe LibraryScanJob do
   before :all do
     ActiveJob::Base.queue_adapter = :test
   end
 
   let(:library) do
-    create(:library, path: File.join(Rails.root, "spec", "fixtures", "library"))
+    create(:library, path: Rails.root.join("spec/fixtures/library"))
   end
 
   it "generates a case-insensitive pattern for files" do
-    expect(LibraryScanJob.file_pattern).to include "*.{stl,STL"
+    expect(described_class.file_pattern).to include "*.{stl,STL"
   end
 
   it "can scan a library directory" do
-    expect { LibraryScanJob.perform_now(library) }.to change { library.models.count }.to(4)
+    expect { described_class.perform_now(library) }.to change { library.models.count }.to(4)
     expect(library.models.map(&:name)).to match_array ["Model One", "Model Two", "Nested Model", "Thingiverse Model"]
     expect(library.models.map(&:path)).to match_array ["/model_one", "/subfolder/model_two", "/model_one/nested_model", "/thingiverse_model"]
   end
 
   it "queues up model scans" do
-    expect { LibraryScanJob.perform_now(library) }.to have_enqueued_job(ModelScanJob).exactly(4).times
+    expect { described_class.perform_now(library) }.to have_enqueued_job(ModelScanJob).exactly(4).times
   end
 
   it "only scans models with changes on rescan" do
     model_one = create(:model, path: "model_one", library: library)
     ModelScanJob.perform_now(model_one)
-    expect { LibraryScanJob.perform_now(library) }.to have_enqueued_job(ModelScanJob).exactly(3).times
+    expect { described_class.perform_now(library) }.to have_enqueued_job(ModelScanJob).exactly(3).times
   end
 
   it "flags models with no files as problems" do
     lib = create(:library, path: File.join("/", "tmp"))
     create(:model, library: lib, path: "missing")
-    expect { LibraryScanJob.perform_now(lib) }.to change { Problem.count }.from(0).to(1)
+    expect { described_class.perform_now(lib) }.to change(Problem, :count).from(0).to(1)
   end
 end