From: Tom Hughes Date: Wed, 4 Dec 2019 19:25:06 +0000 (+0000) Subject: Fix rubocop warnings X-Git-Tag: live~2934 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/57f5b7840e540fd8b2240fe7786e989fb2f829af Fix rubocop warnings --- diff --git a/.rubocop.yml b/.rubocop.yml index 18bf69b9b..c211cc2fd 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -35,7 +35,7 @@ Naming/FileName: - 'script/locale/reload-languages' - 'script/update-spam-blocks' -Naming/UncommunicativeMethodParamName: +Naming/MethodParameterName: Enabled: false Rails/CreateTableWithTimestamps: diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 518e1eda4..054e10cda 100644 --- a/.rubocop_todo.yml +++ b/.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' diff --git a/app/controllers/api/amf_controller.rb b/app/controllers/api/amf_controller.rb index e5648989b..4dbc16fd2 100644 --- a/app/controllers/api/amf_controller.rb +++ b/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 ## diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index df7cfe93b..44efdc071 100644 --- a/app/controllers/api_controller.rb +++ b/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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 345bae261..a61a10d94 100644 --- a/app/controllers/users_controller.rb +++ b/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] diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb new file mode 100644 index 000000000..ead50cd96 --- /dev/null +++ b/app/mailers/application_mailer.rb @@ -0,0 +1,2 @@ +class ApplicationMailer < ActionMailer::Base +end diff --git a/app/mailers/notifier.rb b/app/mailers/notifier.rb index 6e0c81a39..b12599981 100644 --- a/app/mailers/notifier.rb +++ b/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 diff --git a/config/environments/development.rb b/config/environments/development.rb index 14bf9b54d..778e629eb 100644 --- a/config/environments/development.rb +++ b/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 diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb index 156449635..85ce74f15 100644 --- a/config/initializers/assets.rb +++ b/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") diff --git a/config/initializers/banners.rb b/config/initializers/banners.rb index 8eda290c5..c7ff281f3 100644 --- a/config/initializers/banners.rb +++ b/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 diff --git a/config/initializers/config.rb b/config/initializers/config.rb index 271bd79e7..b74ba57e6 100644 --- a/config/initializers/config.rb +++ b/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" diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index a61fe960c..7889fca81 100644 --- a/config/initializers/omniauth.rb +++ b/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 } diff --git a/config/initializers/wiki_pages.rb b/config/initializers/wiki_pages.rb index c679cd4e2..6720ee778 100644 --- a/config/initializers/wiki_pages.rb +++ b/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")) diff --git a/db/migrate/034_create_languages.rb b/db/migrate/034_create_languages.rb index 459d7f647..20b80ec5b 100644 --- a/db/migrate/034_create_languages.rb +++ b/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" diff --git a/lib/country.rb b/lib/country.rb index ab88072ec..f6f679125 100644 --- a/lib/country.rb +++ b/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 diff --git a/lib/id.rb b/lib/id.rb index 3adfecf65..4e6cf3b7c 100644 --- a/lib/id.rb +++ b/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 diff --git a/lib/potlatch.rb b/lib/potlatch.rb index 0b01daf29..f481e4412 100644 --- a/lib/potlatch.rb +++ b/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+(.+)$} diff --git a/lib/tasks/add_version_to_nodes.rake b/lib/tasks/add_version_to_nodes.rake index 11f639abd..4762116f2 100644 --- a/lib/tasks/add_version_to_nodes.rake +++ b/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 diff --git a/lib/tasks/auto_annotate_models.rake b/lib/tasks/auto_annotate_models.rake index 954acbf22..70bc391a0 100644 --- a/lib/tasks/auto_annotate_models.rake +++ b/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( diff --git a/lib/tasks/eslint.rake b/lib/tasks/eslint.rake index 311d79c76..320651b5a 100644 --- a/lib/tasks/eslint.rake +++ b/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 diff --git a/lib/tasks/testing.rake b/lib/tasks/testing.rake index c9b384cfe..720530256 100644 --- a/lib/tasks/testing.rake +++ b/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 diff --git a/test/controllers/api/traces_controller_test.rb b/test/controllers/api/traces_controller_test.rb index 80b9d1a84..53964b2db 100644 --- a/test/controllers/api/traces_controller_test.rb +++ b/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) diff --git a/test/controllers/traces_controller_test.rb b/test/controllers/traces_controller_test.rb index 059242af9..312d451d7 100644 --- a/test/controllers/traces_controller_test.rb +++ b/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) diff --git a/test/models/trace_test.rb b/test/models/trace_test.rb index 42bfa6dda..e0c65e33e 100644 --- a/test/models/trace_test.rb +++ b/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