Code cleanups (forem#659)

* Initial automatic cleanup with rubocop

* Fix syntax error introduced by rubocop

* Cleanup seeds file

* Cleanup lib folder

* Exclude bin folder because it contains auto generated files

* Make Rubocop a little bit more chatty

* Block length should not include comments in the count

* Cleanup config folder

* Cleanup specs

* Updated Rubocop version and generated a todo file

* Fix broken ArticlesApi spec

* Fix tests

* Restored rubocop pre-commit hook

Author: rhymes

.rubocop.yml

@@ -1,3 +1,5 @@
+inherit_from: .rubocop_todo.yml
+
 require: rubocop-rspec
 
 # our custom config
@@ -19,7 +21,13 @@ Metrics/LineLength:
 
 AllCops:
   Exclude:
+    - bin/**/*
     - db/schema.rb
+    - db/migrate/*.rb
+    - node_modules/**/*
+  DisplayStyleGuide: true
+  ExtraDetails: true
+  TargetRubyVersion: 2.5
 
 Naming/AccessorMethodName:
   Description: Check the naming of accessor methods for get_/set_.
@@ -110,7 +118,7 @@ Metrics/AbcSize:
   Enabled: false
 
 Metrics/BlockLength:
-  CountComments: true  # count full line comments?
+  CountComments: false  # count full line comments?
   Max: 25
   ExcludedMethods: []
   Exclude:

.rubocop_todo.yml

@@ -0,0 +1,200 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2018-08-05 17:42:25 +0200 using RuboCop version 0.58.2.
+# 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: 9
+# Cop supports --auto-correct.
+# Configuration parameters: EnforcedStyleAlignWith, AutoCorrect, Severity.
+# SupportedStylesAlignWith: keyword, variable, start_of_line
+Layout/EndAlignment:
+  Exclude:
+    - 'app/controllers/api/v0/articles_controller.rb'
+    - 'app/controllers/articles_controller.rb'
+    - 'app/controllers/chat_channels_controller.rb'
+    - 'app/controllers/image_uploads_controller.rb'
+    - 'app/helpers/application_helper.rb'
+    - 'app/models/user.rb'
+    - 'app/services/article_api_index_service.rb'
+
+# Offense count: 1
+# Cop supports --auto-correct.
+# Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBrackets.
+# SupportedStyles: space, no_space, compact
+# SupportedStylesForEmptyBrackets: space, no_space
+Layout/SpaceInsideArrayLiteralBrackets:
+  Exclude:
+    - 'app/dashboards/email_message_dashboard.rb'
+
+# Offense count: 3
+Lint/AmbiguousBlockAssociation:
+  Exclude:
+    - 'app/controllers/podcast_episodes_controller.rb'
+    - 'app/views/api/v0/articles/show.json.jbuilder'
+
+# Offense count: 4
+Lint/IneffectiveAccessModifier:
+  Exclude:
+    - 'app/black_box/black_box.rb'
+    - 'app/models/tweet.rb'
+
+# Offense count: 3
+Lint/ShadowingOuterLocalVariable:
+  Exclude:
+    - 'app/views/api/v0/comments/index.json.jbuilder'
+    - 'app/views/api/v0/comments/show.json.jbuilder'
+
+# Offense count: 2
+# Configuration parameters: ContextCreatingMethods, MethodCreatingMethods.
+Lint/UselessAccessModifier:
+  Exclude:
+    - 'app/black_box/black_box.rb'
+    - 'app/models/tweet.rb'
+
+# Offense count: 10
+Lint/UselessAssignment:
+  Exclude:
+    - 'app/controllers/stories_controller.rb'
+    - 'app/labor/cache_buster.rb'
+    - 'app/labor/markdown_parser.rb'
+    - 'app/labor/rate_limit_checker.rb'
+    - 'app/liquid_tags/comment_tag.rb'
+    - 'app/liquid_tags/glitch_tag.rb'
+    - 'app/liquid_tags/tweet_tag.rb'
+
+# Offense count: 8
+# Configuration parameters: CountComments, ExcludedMethods.
+# ExcludedMethods: refine
+Metrics/BlockLength:
+  Max: 68
+
+# Offense count: 132
+# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
+# URISchemes: http, https
+Metrics/LineLength:
+  Max: 274
+
+# Offense count: 14
+Metrics/PerceivedComplexity:
+  Max: 16
+
+# Offense count: 1
+# Configuration parameters: EnforcedStyleForLeadingUnderscores.
+# SupportedStylesForLeadingUnderscores: disallowed, required, optional
+Naming/MemoizedInstanceVariableName:
+  Exclude:
+    - 'app/controllers/admin/application_controller.rb'
+
+# Offense count: 2
+# Configuration parameters: EnforcedStyle.
+# SupportedStyles: snake_case, camelCase
+Naming/MethodName:
+  Exclude:
+    - 'app/helpers/articles_helper.rb'
+    - 'app/liquid_tags/podcast_tag.rb'
+
+# Offense count: 4
+# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist, MethodDefinitionMacros.
+# NamePrefix: is_, has_, have_
+# NamePrefixBlacklist: is_, has_, have_
+# NameWhitelist: is_a?
+# MethodDefinitionMacros: define_method, define_singleton_method
+Naming/PredicateName:
+  Exclude:
+    - 'spec/**/*'
+    - 'app/controllers/application_controller.rb'
+    - 'app/helpers/articles_helper.rb'
+    - 'app/models/user.rb'
+
+# Offense count: 9
+# Configuration parameters: MinNameLength, AllowNamesEndingInNumbers, AllowedNames, ForbiddenNames.
+# AllowedNames: io, id, to, by, on, in, at, ip
+Naming/UncommunicativeMethodParamName:
+  Exclude:
+    - 'app/controllers/followed_articles_controller.rb'
+    - 'app/controllers/internal/dogfood_controller.rb'
+    - 'app/labor/mailchimp_bot.rb'
+    - 'app/labor/podcast_feed.rb'
+    - 'app/models/chat_channel.rb'
+    - 'app/models/tweet.rb'
+
+# Offense count: 2
+# Configuration parameters: EnforcedStyle.
+# SupportedStyles: snake_case, camelCase
+Naming/VariableName:
+  Exclude:
+    - 'app/liquid_tags/github_tag/github_issue_tag.rb'
+
+# Offense count: 3
+Security/Open:
+  Exclude:
+    - 'app/labor/podcast_feed.rb'
+    - 'app/labor/rss_reader.rb'
+
+# Offense count: 14
+# Cop supports --auto-correct.
+# Configuration parameters: AutoCorrect, EnforcedStyle.
+# SupportedStyles: nested, compact
+Style/ClassAndModuleChildren:
+  Exclude:
+    - 'app/controllers/api/v0/api_controller.rb'
+    - 'app/controllers/internal/application_controller.rb'
+    - 'app/controllers/internal/articles_controller.rb'
+    - 'app/controllers/internal/broadcasts_controller.rb'
+    - 'app/controllers/internal/buffer_updates_controller.rb'
+    - 'app/controllers/internal/comments_controller.rb'
+    - 'app/controllers/internal/dogfood_controller.rb'
+    - 'app/controllers/internal/feedback_messages_controller.rb'
+    - 'app/controllers/internal/members_controller.rb'
+    - 'app/controllers/internal/tags_controller.rb'
+    - 'app/controllers/internal/users_controller.rb'
+    - 'app/controllers/internal/welcome_controller.rb'
+    - 'app/controllers/notifications/counts_controller.rb'
+    - 'app/controllers/notifications/reads_controller.rb'
+
+# Offense count: 1
+Style/DateTime:
+  Exclude:
+    - 'app/controllers/comments_controller.rb'
+
+# Offense count: 1
+Style/EvalWithLocation:
+  Exclude:
+    - 'app/controllers/omniauth_callbacks_controller.rb'
+
+# Offense count: 1
+# Configuration parameters: EnforcedStyle.
+# SupportedStyles: each, for
+Style/For:
+  Exclude:
+    - 'app/views/articles/feed.rss.builder'
+
+# Offense count: 1
+Style/IfInsideElse:
+  Exclude:
+    - 'app/models/comment.rb'
+
+# Offense count: 3
+Style/MixinUsage:
+  Exclude:
+    - 'app/models/organization.rb'
+    - 'app/models/podcast_episode.rb'
+
+# Offense count: 1
+# Cop supports --auto-correct.
+# Configuration parameters: AutoCorrect, EnforcedStyle.
+# SupportedStyles: predicate, comparison
+Style/NumericPredicate:
+  Exclude:
+    - 'spec/**/*'
+    - 'app/liquid_tags/github_tag/github_issue_tag.rb'
+
+# Offense count: 3
+# Cop supports --auto-correct.
+# Configuration parameters: AllowAsExpressionSeparator.
+Style/Semicolon:
+  Exclude:
+    - 'app/controllers/stories_controller.rb'

Gemfile

@@ -10,7 +10,7 @@ git_source(:github) do |repo_name|
 end
 
 group :production do
-  gem 'nakayoshi_fork'
+  gem "nakayoshi_fork"
 end
 
 gem "actionpack-action_caching", "~> 1.2"

Gemfile.lock

@@ -617,7 +617,7 @@ GEM
     parallel (1.12.1)
     parallel_tests (2.21.3)
       parallel
-    parser (2.5.1.0)
+    parser (2.5.1.2)
       ast (~> 2.4.0)
     pg (0.21.0)
     powerpack (0.1.2)
@@ -736,10 +736,10 @@ GEM
     rspec-retry (0.6.1)
       rspec-core (> 3.3)
     rspec-support (3.7.1)
-    rubocop (0.57.2)
+    rubocop (0.58.2)
       jaro_winkler (~> 1.5.1)
       parallel (~> 1.10)
-      parser (>= 2.5)
+      parser (>= 2.5, != 2.5.1.1)
       powerpack (~> 0.1)
       rainbow (>= 2.2.2, < 4.0)
       ruby-progressbar (~> 1.7)

Guardfile

@@ -27,7 +27,7 @@
 ignore([%r{^bin/*}, %r{^config/*}, %r{^db/*}, %r{^lib/*}, %r{^log/*}, %r{^public/*}, %r{^tmp/*}, %r{^node_modules/*}])
 
 rspec_options = {
-  results_file: File.expand_path('tmp/guard_rspec_results.txt'),
+  results_file: File.expand_path("tmp/guard_rspec_results.txt"),
   #############################
   # BECAUSE spring doesn't seem to work well with simplecov, choose
   # between the following two.
@@ -37,7 +37,7 @@ rspec_options = {
   cmd: "bin/spring rspec -p",
   #############################
   failed_mode: :focus,
-  bundler_env: :clean_env
+  bundler_env: :clean_env,
 }
 
 guard :rspec, rspec_options do
@@ -65,7 +65,7 @@ guard :rspec, rspec_options do
     [
       rspec.spec.call("routing/#{m[1]}_routing"),
       rspec.spec.call("controllers/#{m[1]}_controller"),
-      rspec.spec.call("acceptance/#{m[1]}")
+      rspec.spec.call("acceptance/#{m[1]}"),
     ]
   end
 

Rakefile

@@ -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 File.expand_path('../config/application', __FILE__)
+require File.expand_path("config/application", __dir__)
 
 Rails.application.load_tasks

app/black_box/black_box.rb

@@ -1,35 +1,35 @@
 class BlackBox
   def self.article_hotness_score(article)
-    return (article.featured_number|| 10000)/10000 unless Rails.env.production?
+    return (article.featured_number || 10000) / 10000 unless Rails.env.production?
     reaction_points = article.reactions.sum(:points)
     recency_bonus = article.published_at > 12.hours.ago ? 50 : 0
     today_bonus = article.published_at > 36.hours.ago ? 250 : 0
     FunctionCaller.new("blackbox-production-articleHotness",
-      {article: article, user: article.user}.to_json).call +
+      { article: article, user: article.user }.to_json).call +
       reaction_points + recency_bonus + today_bonus
   end
 
   def self.comment_quality_score(comment)
-    descendants_points = (comment.descendants.size/2)
+    descendants_points = (comment.descendants.size / 2)
     rep_points = comment.reactions.sum(:points)
     bonus_points = calculate_bonus_score(comment.body_markdown)
     spaminess_rating = calculate_spaminess(comment)
     (rep_points + descendants_points + bonus_points - spaminess_rating).to_i
   end
 
   def self.calculate_spaminess(story)
-    #accepts comment or article as story
+    # accepts comment or article as story
     return 0 unless Rails.env.production?
     return 100 unless story.user
     FunctionCaller.new("blackbox-production-spamScore",
-      {story: story, user: story.user}.to_json).call
+      { story: story, user: story.user }.to_json).call
   end
 
   private
 
   def self.calculate_bonus_score(body_markdown)
     size_bonus = body_markdown.size > 200 ? 2 : 0
-    code_bonus = body_markdown.include?('`') ? 1 : 0
+    code_bonus = body_markdown.include?("`") ? 1 : 0
     size_bonus + code_bonus
   end
 end

app/controllers/additional_content_boxes_controller.rb

@@ -14,11 +14,11 @@ def index
       @boosted_article = Suggester::Articles::Boosted.new(
         current_user,
         @article,
-        {not_ids: (article_ids + [@for_user_article&.id]), area: "additional_articles"},
+        not_ids: (article_ids + [@for_user_article&.id]), area: "additional_articles",
       ).suggest
     else
       @alt_classic = Suggester::Articles::Classic.
-        new(@article, {not_ids: (article_ids + [@for_user_article&.id])}).get
+        new(@article, not_ids: (article_ids + [@for_user_article&.id])).get
     end
     set_surrogate_key_header "additional_content_boxes_" + params.to_s unless current_user
     render "boxes", layout: false

app/controllers/admin/application_controller.rb

@@ -10,7 +10,7 @@ class ApplicationController < Administrate::ApplicationController
     before_action :authorize_admin
 
     def order
-      @_order ||= Administrate::Order.new(params[:order] || "id",params[:direction] || "desc")
+      @_order ||= Administrate::Order.new(params[:order] || "id", params[:direction] || "desc")
     end
 
     def valid_request_origin?

app/controllers/api/v0/api_controller.rb

@@ -1,19 +1,19 @@
 class Api::V0::ApiController < ApplicationController
   def cors_set_access_control_headers
-    headers['Access-Control-Allow-Origin'] = '*'
-    headers['Access-Control-Allow-Methods'] = 'POST, GET, PUT, DELETE, OPTIONS'
-    headers['Access-Control-Allow-Headers'] = 'Origin, Content-Type, Accept, Authorization, Token'
-    headers['Access-Control-Max-Age'] = "1728000"
+    headers["Access-Control-Allow-Origin"] = "*"
+    headers["Access-Control-Allow-Methods"] = "POST, GET, PUT, DELETE, OPTIONS"
+    headers["Access-Control-Allow-Headers"] = "Origin, Content-Type, Accept, Authorization, Token"
+    headers["Access-Control-Max-Age"] = "1728000"
   end
 
   def cors_preflight_check
-    if request.method == 'OPTIONS'
-      headers['Access-Control-Allow-Origin'] = '*'
-      headers['Access-Control-Allow-Methods'] = 'POST, GET, PUT, DELETE, OPTIONS'
-      headers['Access-Control-Allow-Headers'] = 'X-Requested-With, X-Prototype-Version, Token'
-      headers['Access-Control-Max-Age'] = '1728000'
+    if request.method == "OPTIONS"
+      headers["Access-Control-Allow-Origin"] = "*"
+      headers["Access-Control-Allow-Methods"] = "POST, GET, PUT, DELETE, OPTIONS"
+      headers["Access-Control-Allow-Headers"] = "X-Requested-With, X-Prototype-Version, Token"
+      headers["Access-Control-Max-Age"] = "1728000"
 
-      render :text => '', :content_type => 'text/plain'
+      render text: "", content_type: "text/plain"
     end
   end
-end
+end