Fix rubocop warnings

.rubocop.yml

@@ -35,7 +35,7 @@ Naming/FileName:
     - 'script/locale/reload-languages'
     - 'script/update-spam-blocks'
 
-Naming/UncommunicativeMethodParamName:
+Naming/MethodParameterName:
   Enabled: false
 
 Rails/CreateTableWithTimestamps:

.rubocop_todo.yml

@@ -27,7 +27,7 @@ Lint/AssignmentInCondition:
 
 # Offense count: 4
 # Configuration parameters: AllowComments.
-Lint/HandleExceptions:
+Lint/SuppressedException:
   Exclude:
     - 'app/controllers/api/amf_controller.rb'
     - 'app/controllers/users_controller.rb'

app/controllers/api/amf_controller.rb

@@ -226,14 +226,14 @@ module Api
       loaded_lang = "en"
 
       # Load English defaults
-      en = YAML.safe_load(File.open(Rails.root.join("config", "potlatch", "locales", "en.yml")))["en"]
+      en = YAML.safe_load(File.open(Rails.root.join("config/potlatch/locales/en.yml")))["en"]
 
       if lang == "en"
-        return [loaded_lang, en]
+        [loaded_lang, en]
       else
         # Use English as a fallback
         begin
-          other = YAML.safe_load(File.open(Rails.root.join("config", "potlatch", "locales", "#{lang}.yml")))[lang]
+          other = YAML.safe_load(File.open(Rails.root.join("config/potlatch/locales/#{lang}.yml")))[lang]
           loaded_lang = lang
         rescue StandardError
           other = en
@@ -241,7 +241,7 @@ module Api
 
         # We have to return a flat list and some of the keys won't be
         # translated (probably)
-        return [loaded_lang, en.merge(other)]
+        [loaded_lang, en.merge(other)]
       end
     end
 
@@ -876,7 +876,7 @@ module Api
     end
 
     def getlocales
-      @getlocales ||= Locale.list(Dir.glob(Rails.root.join("config", "potlatch", "locales", "*")).collect { |f| File.basename(f, ".yml") })
+      @getlocales ||= Locale.list(Dir.glob(Rails.root.join("config/potlatch/locales/*")).collect { |f| File.basename(f, ".yml") })
     end
 
     ##

app/controllers/api_controller.rb

@@ -12,7 +12,7 @@ class ApiController < ApplicationController
       # no auth, the user does not exist or the password was wrong
       response.headers["WWW-Authenticate"] = "Basic realm=\"#{realm}\""
       render :plain => errormessage, :status => :unauthorized
-      return false
+      false
     end
   end
 

app/controllers/users_controller.rb

@@ -376,7 +376,7 @@ class UsersController < ApplicationController
     @user = User.find_by(:display_name => params[:display_name])
 
     if @user &&
-       (@user.visible? || (current_user&.administrator?))
+       (@user.visible? || current_user&.administrator?)
       @title = @user.display_name
     else
       render_unknown_user params[:display_name]

app/mailers/application_mailer.rb

@@ -0,0 +1,2 @@
+class ApplicationMailer < ActionMailer::Base
+end

app/mailers/notifier.rb

@@ -1,4 +1,4 @@
-class Notifier < ActionMailer::Base
+class Notifier < ApplicationMailer
   include ActionView::Helpers::AssetUrlHelper
 
   self.delivery_job = ActionMailer::MailDeliveryJob
@@ -177,7 +177,7 @@ class Notifier < ActionMailer::Base
   end
 
   def attach_project_logo
-    attachments.inline["logo.png"] = File.read(Rails.root.join("app", "assets", "images", "osm_logo_30.png"))
+    attachments.inline["logo.png"] = File.read(Rails.root.join("app/assets/images/osm_logo_30.png"))
   end
 
   def attach_user_avatar(user)
@@ -187,9 +187,9 @@ class Notifier < ActionMailer::Base
   def user_avatar_file(user)
     avatar = user&.avatar
     if avatar&.attached?
-      return avatar.variant(:resize => "50x50>").blob.download
+      avatar.variant(:resize => "50x50>").blob.download
     else
-      return File.read(Rails.root.join("app", "assets", "images", "avatar_small.png"))
+      File.read(Rails.root.join("app/assets/images/avatar_small.png"))
     end
   end
 

config/environments/development.rb

@@ -14,7 +14,7 @@ Rails.application.configure do
 
   # Enable/disable caching. By default caching is disabled.
   # Run rails dev:cache to toggle caching.
-  if Rails.root.join("tmp", "caching-dev.txt").exist?
+  if Rails.root.join("tmp/caching-dev.txt").exist?
     config.action_controller.perform_caching = true
     config.action_controller.enable_fragment_cache_logging = true
 

config/initializers/assets.rb

@@ -4,7 +4,7 @@
 Rails.application.config.assets.version = "1.0"
 
 # Location of manifest file.
-Rails.application.config.assets.manifest = Rails.root.join("tmp", "manifest.json")
+Rails.application.config.assets.manifest = Rails.root.join("tmp/manifest.json")
 
 # Add additional assets to the asset load path.
 Rails.application.config.assets.paths << Rails.root.join("config")

config/initializers/banners.rb

@@ -1,5 +1,5 @@
 begin
-  BANNERS = YAML.load_file(Rails.root.join("config", "banners.yml")).deep_symbolize_keys
+  BANNERS = YAML.load_file(Rails.root.join("config/banners.yml")).deep_symbolize_keys
 rescue StandardError
   BANNERS = {}.freeze
 end

config/initializers/config.rb

@@ -2,7 +2,7 @@
 # Otherwise, admins might not be aware that they are now silently ignored
 # and major problems could occur
 # rubocop:disable Rails/Output, Rails/Exit
-if File.exist?(Rails.root.join("config", "application.yml"))
+if File.exist?(Rails.root.join("config/application.yml"))
   puts "The config/application.yml file is no longer supported."
   puts ""
   puts "Default settings are now found in config/settings.yml and you"

config/initializers/omniauth.rb

@@ -16,7 +16,7 @@ if Settings.key?(:memcache_servers)
 else
   require "openid/store/filesystem"
 
-  openid_store = OpenID::Store::Filesystem.new(Rails.root.join("tmp", "openids"))
+  openid_store = OpenID::Store::Filesystem.new(Rails.root.join("tmp/openids"))
 end
 
 openid_options = { :name => "openid", :store => openid_store }

config/initializers/wiki_pages.rb

@@ -1 +1 @@
-WIKI_PAGES = YAML.load_file(Rails.root.join("config", "wiki_pages.yml"))
+WIKI_PAGES = YAML.load_file(Rails.root.join("config/wiki_pages.yml"))

db/migrate/034_create_languages.rb

@@ -10,7 +10,7 @@ class CreateLanguages < ActiveRecord::Migration[4.2]
 
     add_primary_key :languages, [:code]
 
-    Language.load(Rails.root.join("config", "languages.yml"))
+    Language.load(Rails.root.join("config/languages.yml"))
 
     add_foreign_key :users, :languages, :column => :locale, :primary_key => :code, :name => "users_locale_fkey"
     add_foreign_key :diary_entries, :languages, :column => :language_code, :primary_key => :code, :name => "diary_entries_language_code_fkey"

lib/country.rb

@@ -19,7 +19,7 @@ class Country
 
   def self.load_countries
     countries = {}
-    xml = REXML::Document.new(File.read(Rails.root.join("config", "countries.xml")))
+    xml = REXML::Document.new(File.read(Rails.root.join("config/countries.xml")))
 
     xml.elements.each("geonames/country") do |ele|
       code = ele.get_text("countryCode").to_s

lib/id.rb

@@ -1,3 +1,3 @@
 module ID
-  LOCALES = Locale.list(Rails.root.join("vendor", "assets", "iD", "iD", "locales").entries.map { |p| p.basename.to_s[/(.*).json/] && Regexp.last_match(1) }.compact)
+  LOCALES = Locale.list(Rails.root.join("vendor/assets/iD/iD/locales").entries.map { |p| p.basename.to_s[/(.*).json/] && Regexp.last_match(1) }.compact)
 end

lib/potlatch.rb

@@ -178,7 +178,7 @@ module Potlatch
       presettype = ""
       presetcategory = ""
       #  StringIO.open(txt) do |file|
-      File.open(Rails.root.join("config", "potlatch", "presets.txt")) do |file|
+      File.open(Rails.root.join("config/potlatch/presets.txt")) do |file|
         file.each_line do |line|
           t = line.chomp
           if t =~ %r{(\w+)/(\w+)}
@@ -202,7 +202,7 @@ module Potlatch
       colours = {}
       casing = {}
       areas = {}
-      File.open(Rails.root.join("config", "potlatch", "colours.txt")) do |file|
+      File.open(Rails.root.join("config/potlatch/colours.txt")) do |file|
         file.each_line do |line|
           next unless line.chomp =~ /(\w+)\s+([^\s]+)\s+([^\s]+)\s+([^\s]+)/
 
@@ -217,7 +217,7 @@ module Potlatch
       relcolours = {}
       relalphas = {}
       relwidths = {}
-      File.open(Rails.root.join("config", "potlatch", "relation_colours.txt")) do |file|
+      File.open(Rails.root.join("config/potlatch/relation_colours.txt")) do |file|
         file.each_line do |line|
           next unless line.chomp =~ /(\w+)\s+([^\s]+)\s+([^\s]+)\s+([^\s]+)/
 
@@ -231,7 +231,7 @@ module Potlatch
       # Read POI presets
       icon_list = []
       icon_tags = {}
-      File.open(Rails.root.join("config", "potlatch", "icon_presets.txt")) do |file|
+      File.open(Rails.root.join("config/potlatch/icon_presets.txt")) do |file|
         file.each_line do |line|
           (icon, tags) = line.chomp.split("\t")
           icon_list.push(icon)
@@ -242,7 +242,7 @@ module Potlatch
 
       # Read auto-complete
       autotags = { "point" => {}, "way" => {}, "POI" => {} }
-      File.open(Rails.root.join("config", "potlatch", "autocomplete.txt")) do |file|
+      File.open(Rails.root.join("config/potlatch/autocomplete.txt")) do |file|
         file.each_line do |line|
           next unless line.chomp =~ %r{^([\w:]+)/(\w+)\s+(.+)$}
 

lib/tasks/add_version_to_nodes.rake

@@ -1,6 +1,6 @@
 namespace "db" do
   desc "Adds a version number to the nodes table"
-  task :node_version do
+  task :node_version => :environment do
     require File.dirname(__FILE__) + "/../../config/environment"
 
     increment = 1000

lib/tasks/auto_annotate_models.rake

@@ -2,7 +2,7 @@
 # NOTE: are sensitive to local FS writes, and besides -- it's just not proper
 # NOTE: to have a dev-mode tool do its thing in production.
 if Rails.env.development?
-  task :set_annotation_options do
+  task :set_annotation_options => :environment do
     # You can override any of these by setting an environment variable of the
     # same name.
     Annotate.set_defaults(

lib/tasks/eslint.rake

@@ -2,11 +2,11 @@ task "eslint" => "eslint:check"
 
 namespace "eslint" do
   def yarn_path
-    Rails.root.join("bin", "yarn").to_s
+    Rails.root.join("bin/yarn").to_s
   end
 
   def config_file
-    Rails.root.join("config", "eslint.json").to_s
+    Rails.root.join("config/eslint.json").to_s
   end
 
   def js_files

lib/tasks/testing.rake

@@ -1,4 +1,4 @@
-task "test" do
+task :test => :environment do
   Rails::TestUnit::Runner.rake_run(["test/system"]) unless ENV.key?("TEST")
 end
 

test/controllers/api/traces_controller_test.rb

@@ -178,7 +178,7 @@ module Api
     # Test creating a trace through the api
     def test_create
       # Get file to use
-      fixture = Rails.root.join("test", "gpx", "fixtures", "a.gpx")
+      fixture = Rails.root.join("test/gpx/fixtures/a.gpx")
       file = Rack::Test::UploadedFile.new(fixture, "application/gpx+xml")
       user = create(:user)
 

test/controllers/traces_controller_test.rb

@@ -532,7 +532,7 @@ class TracesControllerTest < ActionController::TestCase
   # Test creating a trace
   def test_create_post
     # Get file to use
-    fixture = Rails.root.join("test", "gpx", "fixtures", "a.gpx")
+    fixture = Rails.root.join("test/gpx/fixtures/a.gpx")
     file = Rack::Test::UploadedFile.new(fixture, "application/gpx+xml")
     user = create(:user)
 
@@ -564,7 +564,7 @@ class TracesControllerTest < ActionController::TestCase
   # Test creating a trace with validation errors
   def test_create_post_with_validation_errors
     # Get file to use
-    fixture = Rails.root.join("test", "gpx", "fixtures", "a.gpx")
+    fixture = Rails.root.join("test/gpx/fixtures/a.gpx")
     file = Rack::Test::UploadedFile.new(fixture, "application/gpx+xml")
     user = create(:user)
 

test/models/trace_test.rb

@@ -157,7 +157,7 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_large_picture
-    picture = File.read(Rails.root.join("test", "gpx", "fixtures", "a.gif"), :mode => "rb")
+    picture = File.read(Rails.root.join("test/gpx/fixtures/a.gif"), :mode => "rb")
 
     trace = Trace.create
     trace.large_picture = picture
@@ -168,7 +168,7 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_icon_picture
-    picture = File.read(Rails.root.join("test", "gpx", "fixtures", "a_icon.gif"), :mode => "rb")
+    picture = File.read(Rails.root.join("test/gpx/fixtures/a_icon.gif"), :mode => "rb")
 
     trace = Trace.create
     trace.icon_picture = picture
@@ -184,7 +184,7 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_import_removes_previous_tracepoints
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace = create(:trace, :fixture => "a")
       # Tracepoints don't have a primary key, so we use a specific latitude to
       # check for successful deletion
@@ -199,7 +199,7 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_import_creates_tracepoints
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace = create(:trace, :fixture => "a")
       assert_equal 0, Tracepoint.where(:gpx_id => trace.id).count
 
@@ -216,7 +216,7 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_import_creates_icon
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace = create(:trace, :fixture => "a")
       icon_path = File.join(Settings.gpx_image_dir, "#{trace.id}_icon.gif")
       FileUtils.rm(icon_path)
@@ -230,7 +230,7 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_import_creates_large_picture
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace = create(:trace, :fixture => "a")
       large_picture_path = File.join(Settings.gpx_image_dir, "#{trace.id}.gif")
       FileUtils.rm(large_picture_path)
@@ -244,7 +244,7 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_import_handles_bz2
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace = create(:trace, :fixture => "c")
 
       trace.import
@@ -255,7 +255,7 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_import_handles_plain
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace = create(:trace, :fixture => "a")
 
       trace.import
@@ -266,7 +266,7 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_import_handles_plain_with_bom
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace = create(:trace, :fixture => "b")
 
       trace.import
@@ -279,7 +279,7 @@ class TraceTest < ActiveSupport::TestCase
     trace = create(:trace, :fixture => "d")
 
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace.import
 
       assert_equal 1, trace.size
@@ -292,7 +292,7 @@ class TraceTest < ActiveSupport::TestCase
     trace = create(:trace, :fixture => "f")
 
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace.import
 
       assert_equal 2, trace.size
@@ -305,7 +305,7 @@ class TraceTest < ActiveSupport::TestCase
     trace = create(:trace, :fixture => "g")
 
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace.import
 
       assert_equal 2, trace.size
@@ -318,7 +318,7 @@ class TraceTest < ActiveSupport::TestCase
     trace = create(:trace, :fixture => "h")
 
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace.import
 
       assert_equal 2, trace.size
@@ -331,7 +331,7 @@ class TraceTest < ActiveSupport::TestCase
     trace = create(:trace, :fixture => "i")
 
     FakeFS do
-      FakeFS::FileSystem.clone(Rails.root.join("test", "gpx"))
+      FakeFS::FileSystem.clone(Rails.root.join("test/gpx"))
       trace.import
 
       assert_equal 2, trace.size