Skip to content

Commit

Permalink
Enforce new RuboCop cops for all Ruby files (#566)
Browse files Browse the repository at this point in the history
* Let RuboCop autocorrect all Ruby files (safe)

Only safe autocorrections used. Command:
bundle exec rubocop --autocorrect --disable-uncorrectable

version: 1.57.2 (using Parser 3.2.2.4,
rubocop-ast 1.30.0, running on ruby 3.1.4) [x86_64-linux]
See the docs: https://docs.rubocop.org/rubocop/usage/auto_correct.html

704 files inspected, 8138 offenses detected, 7399 offenses corrected,
720 more offenses can be corrected with `rubocop -A`.

Note there occurred one error for the file "db/seeds.rb", which will
be fixed manually in subsequent commits.

* Revert "Let RuboCop autocorrect all Ruby files (safe)"

This reverts commit b067b2f.

* Force use of explicit hash literal value (HashSyntax)

* Only correct string literals to double quotes (safe)

704 files inspected, 3912 offenses detected, 3912 offenses corrected

* Allow use of method ".touch"

* Autocorrect all other specified cops (safe)

704 files inspected, 3171 offenses detected, 2432 offenses corrected,
720 more offenses can be corrected with `rubocop -A`

Command:
bundle exec rubocop --autocorrect --disable-uncorrectable

* Fix Layout/IndentationConsistency (manual)

* Fix (or disable) Layout/LineLength

* Fix all Style/ cops

* Fix Lint/ cops (3 still left open)

* Rename `get_votes_count` to `votes_count`

* Fix other Naming/ cops (1 still left open)

* Disable `Rails/HelperInstanceVariable` in `quizzes_helper.rb`

This is since we only use the instance variable to provide a *default*
value for some params, so the helper methods can still be reused.

* Fix all Rails/InverseOf cops

* Disable Rails/ cops in existing db migrations

The respective comments were added manually, not automatically.

* Fix Rails/SkipsModelValidations (1 left open)

Replaced `update_all` by `update`
and `Time.now` by `Time.current`

* Disable Rails/OutputSafety for one line

* Fix Rails/I18n related cops

* Add custom env variable to rubocop

This is to avoid Rails/UnknownEnv error

* Disable Performance/CollectionLiteralInLoop in some tests

* Merge two duplicates groups in Gemfile together

* Automatically autocorrect cops (unsafe)

Command used:
bundle exec rubocop --autocorrect-all --disable-uncorrectable

704 files inspected, 911 offenses detected,
909 offenses corrected

2 errors occurred:
An error occurred while Style/StringConcatenation cop was inspecting
mampf/app/models/medium.rb:871:30.
An error occurred while Layout/LineLength cop was inspecting
mampf/db/seeds.rb.

I manually fixed the error in medium.rb.
 I checked the seeds.rb and dit not find any error there.

* Fix missing `Time.zone.now`

* Add Style/MethodCallWithArgsParentheses and autofix (safe)

Command used: bundle exec rubocop --autocorrect --disable-uncorrectable

704 files inspected, 371 offenses detected, 350 offenses corrected,
13 more offenses can be corrected with `rubocop -A`

* Fix Layout/ cops

* Fix or disable Style/ cops

* Delete weird random character

* Fix Security/ cops

* Fix Performance/ cops

* Fix wrong namespace for a cop

* Manually ignore more db/ cop violations

* Delete empty test files

* Fix line to long

* Temporarily disable Rails/LexicallyScopedActionFilter

* Fix <= logical bug (registration threshold)

* remove duplicated restricted? method

* change private methods to non-private methods

* remove duplicate method

* fix typo

* change inverse_of relation

* fix existing inverse_of associations

* rename duplicate announcements method

* undo premature change

* add namespace to constant

* remove unnecessary logging

* remove non existing action from before filter

* Rewrite new registrations query with timeframe

* Allow touch_all method and replace problematic update calls

See this comment:
#566 (comment)

* Disable Rails/HasManyOrHasOneDependent rule

This is because we think it's cleaner not to have to write
"dependent: nil".

Also removed the respective rubocop:todo comments

* Fix unwanted "," character

* Get rid of weird "foo" check

* remove unused restricted? methods

* Replace problematic touch_all by touch

* Fix wrong manual Style/ correction of 5081aaa

* Fix Rails/OutputSafety

* Remove duplicate dates before save

see #566 (comment)

---------

Co-authored-by: fosterfarrell9 <28628554+fosterfarrell9@users.noreply.github.com>
  • Loading branch information
Splines and fosterfarrell9 authored Dec 19, 2023
1 parent 5469dc1 commit e87caf4
Show file tree
Hide file tree
Showing 493 changed files with 5,572 additions and 5,697 deletions.
26 changes: 26 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,20 @@ Performance:
Performance/FlatMap:
Severity: warning # an error in CI/CD


#############################################
# Rails
#############################################

Rails/HasManyOrHasOneDependent:
Enabled: false

Rails/SkipsModelValidations:
AllowedMethods: ["touch", "touch_all"]

Rails/UnknownEnv:
Environments: ["development", "test", "production", "docker_development"]

#############################################
# Style
#############################################
Expand All @@ -81,6 +95,18 @@ Style/EmptyMethod:
Style/FrozenStringLiteralComment:
EnforcedStyle: never

Style/HashSyntax:
EnforcedShorthandSyntax: never

Style/MethodCallWithArgsParentheses:
Enabled: true
AllowedMethods: ["authorize!", "authorize", "can", "can?", "head", "import",
"include", "not_to", "puts", "render", "require", "to"]
AllowedPatterns: [^redirect_]
# Don't enforce in migrations, as we have methods like `add_column`,
# `change_column` etc. and parentheses would be very annoying there.
Exclude: ["db/**/*"]

Style/RedundantReturn:
AllowMultipleReturnValues: true

Expand Down
133 changes: 66 additions & 67 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
source "https://rubygems.org"
git_source(:github) { |repo| "https://github.com/#{repo}.git" }

ruby '3.1.4'
ruby "3.1.4"

# Bundle edge Rails instead: gem 'rails', github: 'rails/rails'
gem "rails", "~> 7.0.4.3"
Expand Down Expand Up @@ -32,113 +32,112 @@ gem "jbuilder"
# gem 'image_processing', '~> 1.2'

# Reduces boot times through caching; required in config/boot.rb
gem "active_model_serializers"
gem "bootsnap", ">= 1.4.2", require: false
gem "rack"
gem "active_model_serializers"
# Use CoffeeScript for .coffee assets and views
gem "coffee-rails", "~> 5.0.0"

# Use Redis adapter to run Action Cable in production
# gem 'redis', '~> 3.0'
gem "shrine"
gem "fastimage"
gem "streamio-ffmpeg"
gem "pdf-reader"
gem "mini_magick"
gem "image_processing"
gem "mini_magick"
gem "pdf-reader"
gem "shrine"
gem "streamio-ffmpeg"
# Use ActiveModel has_secure_password
# gem 'bcrypt', '~> 3.1.7'
gem "filesize"
# Use Capistrano for deployment
# gem 'capistrano-rails', group: :development
gem "rgl"
gem "responders"
gem "pg"
gem "devise"
gem "erubis"
gem "cancancan"
gem "jquery-rails"
gem "jquery-ui-rails"
gem "js-routes", '1.4.9'
gem "activerecord-import",
git: "https://github.com/zdennis/activerecord-import.git",
branch: "master"
gem "acts_as_list"
gem "acts_as_tree"
gem "acts_as_votable"
gem "barby"
gem "bootstrap", "~>5"
gem "bootstrap_form"
gem "cancancan"
gem "clipboard-rails"
gem "commontator"
gem "coveralls", require: false
gem "devise"
gem "devise-bootstrap-views"
gem "erubis"
gem "exception_handler", "~> 0.8.0.0"
gem "faraday", "~> 1.8"
gem "fuzzy-string-match"
gem "coveralls", require: false
gem "globalize"
gem "globalize-accessors"
gem "jquery-rails"
gem "jquery-ui-rails"
gem "js-routes", "1.4.9"
gem "kaminari"
gem "acts_as_list"
gem "acts_as_tree"
gem "activerecord-import",
git: "https://github.com/zdennis/activerecord-import.git",
branch: "master"
gem "thredded"
gem "kramdown-parser-gfm"
gem "thredded-markdown_katex",
git: "https://github.com/thredded/thredded-markdown_katex.git",
branch: "main"
gem "rails-i18n"
gem "kaminari-i18n"
gem "trix-rails", require: "trix"
gem "sunspot_rails",
github: 'sunspot/sunspot',
glob: 'sunspot_rails/*.gemspec'
gem "sunspot_solr"
gem "kramdown-parser-gfm"
gem "net-smtp"
gem "pg"
gem "premailer-rails"
gem "progress_bar"
gem "barby"
gem "rails-i18n"
gem "responders"
gem "rgl"
gem "rqrcode"
gem "rubyzip", "~> 2.3.0"
gem "sidekiq"
gem "sidekiq-cron", "~> 1.1"
gem "faraday", "~> 1.8"
gem "globalize"
gem "globalize-accessors"
gem "commontator"
gem "acts_as_votable"
gem "sprockets-rails",
git: "https://github.com/rails/sprockets-rails",
branch: "master"
gem "premailer-rails"
gem "clipboard-rails"
gem "rubyzip", "~> 2.3.0"
gem "exception_handler", "~> 0.8.0.0"
gem 'webpacker', '~> 5.x'
gem 'net-smtp'

group :development, :docker_development, :test do
# Call 'byebug' anywhere in the code to stop execution and get a debugger console
gem "byebug", platforms: [:mri, :mingw, :x64_mingw]
gem "rspec-rails"
gem "factory_bot_rails"
end
git: "https://github.com/rails/sprockets-rails",
branch: "master"
gem "sunspot_rails",
github: "sunspot/sunspot",
glob: "sunspot_rails/*.gemspec"
gem "sunspot_solr"
gem "thredded"
gem "thredded-markdown_katex",
git: "https://github.com/thredded/thredded-markdown_katex.git",
branch: "main"
gem "trix-rails", require: "trix"
gem "webpacker", "~> 5.x"

group :development, :docker_development do
# Access an interactive console on exception pages or by calling 'console' anywhere in the code.
gem "web-console", ">= 3.3.0"
gem "listen", ">= 3.0.5", "< 3.2"
gem "rails-erd"
gem "web-console", ">= 3.3.0"
# Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring
gem "marcel"
gem "pgreset"
gem "rubocop", "~> 1.57", require: false
gem "rubocop-performance", "~> 1.16", require: false
gem "rubocop-rails", "~> 2.22", ">= 2.22.1", require: false
gem "spring"
gem "spring-watcher-listen", "~> 2.0.0"
gem "rubocop", "~> 1.57", require: false
gem 'rubocop-performance', '~> 1.16', require: false
gem 'rubocop-rails', '~> 2.22', '>= 2.22.1', require: false
gem "pgreset"
gem "marcel"
# gem 'bullet'
end

group :test do
# Adds support for Capybara system testing and selenium driver
gem "selenium-webdriver"
# Easy installation and use of web drivers to run system tests with browsers
gem 'webdrivers'
gem 'faker'
gem 'database_cleaner'
gem 'launchy'
gem 'simplecov', require: false
gem "database_cleaner"
gem "faker"
gem "launchy"
gem "simplecov", require: false
gem "webdrivers"
end

group :test, :development, :docker_development do
gem 'cypress-on-rails', '~> 1.0'
gem 'simplecov-cobertura'
# Call 'byebug' anywhere in the code to stop execution and get a debugger console
gem "byebug", platforms: [:mri, :mingw, :x64_mingw]
gem "factory_bot_rails"
gem "rspec-rails"

gem "cypress-on-rails", "~> 1.0"
gem "simplecov-cobertura"
end
gem 'prometheus_exporter'

gem "prometheus_exporter"
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Add your own tasks in files placed in lib/tasks ending in .rake,
# for example lib/tasks/capistrano.rake, and they will automatically be available to Rake.

require_relative 'config/application'
require_relative "config/application"

Rails.application.load_tasks
2 changes: 1 addition & 1 deletion app/abilities/clicker_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def initialize(user)
!user.generic?
end

can [:show, :get_votes_count], Clicker
can [:show, :votes_count], Clicker

can [:edit, :open, :close, :set_alternatives], Clicker do |clicker, code|
(user&.admin? || user == clicker.editor) || code == clicker.code
Expand Down
2 changes: 1 addition & 1 deletion app/abilities/main_ability.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class MainAbility
include CanCan::Ability

def initialize(user)
def initialize(_user)
can :start, :main
end
end
6 changes: 3 additions & 3 deletions app/abilities/medium_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def initialize(user)

can [:show, :show_comments], Medium do |medium|
medium.visible_for_user?(user) &&
!(medium.sort.in?(['Question', 'Remark']) && !user.can_edit?(medium))
!(medium.sort.in?(["Question", "Remark"]) && !user.can_edit?(medium))
end

can :inspect, Medium do |medium|
Expand All @@ -18,7 +18,7 @@ def initialize(user)

can [:edit, :update, :enrich, :publish, :destroy, :cancel_publication,
:add_item, :add_reference, :add_screenshot, :remove_screenshot,
:import_script_items, :import_manuscript, :get_statistics,
:import_script_items, :import_manuscript, :statistics,
:render_medium_tags, :fill_quizzable_area,
:fill_reassign_modal], Medium do |medium|
user.can_edit?(medium)
Expand Down Expand Up @@ -46,7 +46,7 @@ def initialize(user)
!user.generic? && user.can_edit?(medium)
end

can [:register_download], Medium do |medium|
can [:register_download], Medium do |_medium|
!user.new_record?
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/abilities/profile_ability.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class ProfileAbility
include CanCan::Ability

def initialize(user)
def initialize(_user)
clear_aliased_actions

can [:edit, :update, :check_for_consent, :add_consent,
Expand Down
2 changes: 1 addition & 1 deletion app/abilities/search_ability.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class SearchAbility
include CanCan::Ability

def initialize(user)
def initialize(_user)
clear_aliased_actions

can :index, :search
Expand Down
2 changes: 1 addition & 1 deletion app/abilities/talk_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def initialize(user)
end

can [:new, :edit, :create, :update, :destroy], Talk do |talk|
(talk.lecture && talk.lecture.edited_by?(user)) || user.admin?
talk.lecture&.edited_by?(user) || user.admin?
end

can [:assemble, :modify], Talk do |talk|
Expand Down
4 changes: 2 additions & 2 deletions app/abilities/tutorial_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ def initialize(user)
user.can_update_personell?(tutorial.lecture)
end

can :overview, Tutorial do |tutorial, lecture|
can :overview, Tutorial do |_tutorial, lecture|
user.editor_or_teacher_in?(lecture)
end

can :index, Tutorial do |tutorial, lecture|
can :index, Tutorial do |_tutorial, lecture|
user.in?(lecture.tutors) || user.editor_or_teacher_in?(lecture)
end

Expand Down
2 changes: 1 addition & 1 deletion app/assets/javascripts/media.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ $(document).on 'turbolinks:load', ->

$(document).on 'click', '#showMediaStatistics', ->
mediumId = $(this).data('medium')
$.ajax Routes.get_statistics_path(mediumId),
$.ajax Routes.statistics_path(mediumId),
type: 'GET'
dataType: 'script'
return
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/administration_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class AdministrationController < ApplicationController
# tell cancancan there is no model for this controller, but authorize
# nevertheless
authorize_resource class: false
layout 'administration'
layout "administration"

def current_ability
@current_ability ||= AdministrationAbility.new(current_user)
Expand All @@ -26,6 +26,6 @@ def classification
end

def search
@tags = params[:sort] == 'tag'
@tags = params[:sort] == "tag"
end
end
Loading

0 comments on commit e87caf4

Please sign in to comment.