From 0654be27f9826e6cca4b88c03b39517214612351 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Mon, 11 Jan 2021 19:14:00 +0000 Subject: [PATCH 1/1] Fix new rubocop warnings --- .rubocop_todo.yml | 62 +++++++++++-------- app/controllers/api/changesets_controller.rb | 4 +- app/controllers/geocoder_controller.rb | 24 +++---- app/controllers/redactions_controller.rb | 4 +- app/models/old_node.rb | 2 +- app/models/old_relation.rb | 2 +- app/models/old_way.rb | 2 +- app/models/report.rb | 3 +- app/views/users/_contact.html.erb | 2 +- config/initializers/oauth.rb | 8 +-- config/initializers/strong_migrations.rb | 2 +- script/statistics | 2 +- .../api/tracepoints_controller_test.rb | 2 +- test/controllers/geocoder_controller_test.rb | 2 +- test/controllers/traces_controller_test.rb | 6 +- test/factories/diary_entry_subscriptions.rb | 3 +- 16 files changed, 67 insertions(+), 63 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index afce190d4..9aea439cb 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2020-09-02 06:16:56 UTC using RuboCop version 0.90.0. +# on 2021-01-11 19:00:54 UTC using RuboCop version 1.8.1. # 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 @@ -13,12 +13,12 @@ require: - rubocop-performance - rubocop-rails -# Offense count: 568 +# Offense count: 544 # Cop supports --auto-correct. # Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Layout/LineLength: - Max: 250 + Max: 248 # Offense count: 36 # Configuration parameters: AllowSafeAssignment. @@ -39,14 +39,24 @@ Lint/AssignmentInCondition: - 'lib/osm.rb' - 'script/deliver-message' -# Offense count: 682 -# Configuration parameters: IgnoredMethods. +# Offense count: 8 +# Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches. +Lint/DuplicateBranch: + Exclude: + - 'app/controllers/api_controller.rb' + - 'app/controllers/diary_entries_controller.rb' + - 'app/controllers/geocoder_controller.rb' + - 'app/helpers/browse_tags_helper.rb' + - 'lib/password_hash.rb' + +# Offense count: 487 +# Configuration parameters: IgnoredMethods, CountRepeatedAttributes. Metrics/AbcSize: Max: 194 -# Offense count: 72 -# Configuration parameters: CountComments, CountAsOne, ExcludedMethods. -# ExcludedMethods: refine +# Offense count: 62 +# Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods. +# IgnoredMethods: refine Metrics/BlockLength: Max: 71 @@ -55,41 +65,40 @@ Metrics/BlockLength: Metrics/BlockNesting: Max: 5 -# Offense count: 25 +# Offense count: 24 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 644 + Max: 582 -# Offense count: 68 +# Offense count: 52 # Configuration parameters: IgnoredMethods. Metrics/CyclomaticComplexity: Max: 26 -# Offense count: 735 -# Configuration parameters: CountComments, CountAsOne, ExcludedMethods. +# Offense count: 553 +# Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods. Metrics/MethodLength: Max: 179 -# Offense count: 4 -# Configuration parameters: CountKeywordArgs. +# Offense count: 1 +# Configuration parameters: CountKeywordArgs, MaxOptionalParameters. Metrics/ParameterLists: - Max: 9 + Max: 6 -# Offense count: 69 +# Offense count: 56 # Configuration parameters: IgnoredMethods. Metrics/PerceivedComplexity: Max: 26 -# Offense count: 540 +# Offense count: 365 Minitest/MultipleAssertions: Max: 81 -# Offense count: 6 +# Offense count: 4 Naming/AccessorMethodName: Exclude: - 'app/controllers/application_controller.rb' - 'app/helpers/title_helper.rb' - - 'app/models/old_way.rb' - 'lib/osm.rb' # Offense count: 8 @@ -100,7 +109,6 @@ Naming/AccessorMethodName: # MethodDefinitionMacros: define_method, define_singleton_method Naming/PredicateName: Exclude: - - 'spec/**/*' - 'app/models/changeset.rb' - 'app/models/old_node.rb' - 'app/models/old_relation.rb' @@ -126,7 +134,7 @@ Rails/HasAndBelongsToMany: - 'app/models/changeset.rb' - 'app/models/user.rb' -# Offense count: 4 +# Offense count: 3 # Configuration parameters: Include. # Include: app/helpers/**/*.rb Rails/HelperInstanceVariable: @@ -144,7 +152,7 @@ Rails/NotNullColumn: - 'db/migrate/025_add_end_time_to_changesets.rb' - 'db/migrate/20120404205604_add_user_and_description_to_redaction.rb' -# Offense count: 19 +# Offense count: 8 Rails/OutputSafety: Exclude: - 'app/controllers/users_controller.rb' @@ -152,22 +160,22 @@ Rails/OutputSafety: - 'lib/rich_text.rb' - 'test/helpers/application_helper_test.rb' -# Offense count: 95 +# Offense count: 80 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. # SupportedStyles: strict, flexible Rails/TimeZone: Enabled: false -# Offense count: 572 +# Offense count: 558 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. # SupportedStyles: always, always_true, never Style/FrozenStringLiteralComment: Enabled: false -# Offense count: 78 +# Offense count: 54 # Cop supports --auto-correct. # Configuration parameters: Strict. Style/NumericLiterals: - MinDigits: 11 + MinDigits: 15 diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb index a236083db..29a57570d 100644 --- a/app/controllers/api/changesets_controller.rb +++ b/app/controllers/api/changesets_controller.rb @@ -319,9 +319,7 @@ module Api end # stupid Time seems to throw both of these for bad parsing, so # we have to catch both and ensure the correct code path is taken. - rescue ArgumentError => e - raise OSM::APIBadUserInput, e.message.to_s - rescue RuntimeError => e + rescue ArgumentError, RuntimeError => e raise OSM::APIBadUserInput, e.message.to_s end diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb index 7d05b6765..bc1220af8 100644 --- a/app/controllers/geocoder_controller.rb +++ b/app/controllers/geocoder_controller.rb @@ -319,11 +319,11 @@ class GeocoderController < ApplicationController def nsew_to_decdeg(captures) begin Float(captures[0]) - lat = !captures[2].casecmp("s").zero? ? captures[0].to_f : -captures[0].to_f - lon = !captures[5].casecmp("w").zero? ? captures[3].to_f : -captures[3].to_f + lat = captures[2].casecmp("s").zero? ? -captures[0].to_f : captures[0].to_f + lon = captures[5].casecmp("w").zero? ? -captures[3].to_f : captures[3].to_f rescue StandardError - lat = !captures[0].casecmp("s").zero? ? captures[1].to_f : -captures[1].to_f - lon = !captures[3].casecmp("w").zero? ? captures[4].to_f : -captures[4].to_f + lat = captures[0].casecmp("s").zero? ? -captures[1].to_f : captures[1].to_f + lon = captures[3].casecmp("w").zero? ? -captures[4].to_f : captures[4].to_f end { :lat => lat, :lon => lon } end @@ -331,11 +331,11 @@ class GeocoderController < ApplicationController def ddm_to_decdeg(captures) begin Float(captures[0]) - lat = !captures[3].casecmp("s").zero? ? captures[0].to_f + captures[1].to_f / 60 : -(captures[0].to_f + captures[1].to_f / 60) - lon = !captures[7].casecmp("w").zero? ? captures[4].to_f + captures[5].to_f / 60 : -(captures[4].to_f + captures[5].to_f / 60) + lat = captures[3].casecmp("s").zero? ? -(captures[0].to_f + captures[1].to_f / 60) : captures[0].to_f + captures[1].to_f / 60 + lon = captures[7].casecmp("w").zero? ? -(captures[4].to_f + captures[5].to_f / 60) : captures[4].to_f + captures[5].to_f / 60 rescue StandardError - lat = !captures[0].casecmp("s").zero? ? captures[1].to_f + captures[2].to_f / 60 : -(captures[1].to_f + captures[2].to_f / 60) - lon = !captures[4].casecmp("w").zero? ? captures[5].to_f + captures[6].to_f / 60 : -(captures[5].to_f + captures[6].to_f / 60) + lat = captures[0].casecmp("s").zero? ? -(captures[1].to_f + captures[2].to_f / 60) : captures[1].to_f + captures[2].to_f / 60 + lon = captures[4].casecmp("w").zero? ? -(captures[5].to_f + captures[6].to_f / 60) : captures[5].to_f + captures[6].to_f / 60 end { :lat => lat, :lon => lon } end @@ -343,11 +343,11 @@ class GeocoderController < ApplicationController def dms_to_decdeg(captures) begin Float(captures[0]) - lat = !captures[4].casecmp("s").zero? ? captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60 : -(captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60) - lon = !captures[9].casecmp("w").zero? ? captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60 : -(captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60) + lat = captures[4].casecmp("s").zero? ? -(captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60) : captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60 + lon = captures[9].casecmp("w").zero? ? -(captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60) : captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60 rescue StandardError - lat = !captures[0].casecmp("s").zero? ? captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60 : -(captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60) - lon = !captures[5].casecmp("w").zero? ? captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60 : -(captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60) + lat = captures[0].casecmp("s").zero? ? -(captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60) : captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60 + lon = captures[5].casecmp("w").zero? ? -(captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60) : captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60 end { :lat => lat, :lon => lon } end diff --git a/app/controllers/redactions_controller.rb b/app/controllers/redactions_controller.rb index 45a41058c..30425bd42 100644 --- a/app/controllers/redactions_controller.rb +++ b/app/controllers/redactions_controller.rb @@ -23,7 +23,7 @@ class RedactionsController < ApplicationController @redaction.user = current_user @redaction.title = params[:redaction][:title] @redaction.description = params[:redaction][:description] - # note that the description format will default to 'markdown' + # NOTE: the description format will default to 'markdown' if @redaction.save flash[:notice] = t(".flash") @@ -38,7 +38,7 @@ class RedactionsController < ApplicationController def edit; end def update - # note - don't update the user ID + # NOTE: don't update the user ID @redaction.title = params[:redaction][:title] @redaction.description = params[:redaction][:description] diff --git a/app/models/old_node.rb b/app/models/old_node.rb index 12099498e..fe1a90013 100644 --- a/app/models/old_node.rb +++ b/app/models/old_node.rb @@ -32,7 +32,7 @@ class OldNode < ApplicationRecord self.table_name = "nodes" self.primary_keys = "node_id", "version" - # note this needs to be included after the table name changes, or + # NOTE: this needs to be included after the table name changes, or # the queries generated by Redactable will use the wrong table name. include Redactable diff --git a/app/models/old_relation.rb b/app/models/old_relation.rb index f35678a19..bfbe673b0 100644 --- a/app/models/old_relation.rb +++ b/app/models/old_relation.rb @@ -27,7 +27,7 @@ class OldRelation < ApplicationRecord self.table_name = "relations" self.primary_keys = "relation_id", "version" - # note this needs to be included after the table name changes, or + # NOTE: this needs to be included after the table name changes, or # the queries generated by Redactable will use the wrong table name. include Redactable diff --git a/app/models/old_way.rb b/app/models/old_way.rb index 1ee6935ed..247123005 100644 --- a/app/models/old_way.rb +++ b/app/models/old_way.rb @@ -27,7 +27,7 @@ class OldWay < ApplicationRecord self.table_name = "ways" self.primary_keys = "way_id", "version" - # note this needs to be included after the table name changes, or + # NOTE: this needs to be included after the table name changes, or # the queries generated by Redactable will use the wrong table name. include Redactable diff --git a/app/models/report.rb b/app/models/report.rb index 9afedb04e..346c5eea9 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -32,8 +32,7 @@ class Report < ApplicationRecord def self.categories_for(reportable) case reportable.class.name - when "DiaryEntry" then %w[spam offensive threat other] - when "DiaryComment" then %w[spam offensive threat other] + when "DiaryEntry", "DiaryComment" then %w[spam offensive threat other] when "User" then %w[spam offensive threat vandal other] when "Note" then %w[spam personal abusive other] else %w[other] diff --git a/app/views/users/_contact.html.erb b/app/views/users/_contact.html.erb index d46a61984..a5b29411c 100644 --- a/app/views/users/_contact.html.erb +++ b/app/views/users/_contact.html.erb @@ -22,7 +22,7 @@ <% changeset = contact.changesets.first %> <% if changeset %> <%= t("users.show.latest edit", :ago => time_ago_in_words(changeset.created_at, :scope => :'datetime.distance_in_words_ago')) %> - <% comment = changeset.tags["comment"].to_s != "" ? changeset.tags["comment"] : t("browse.no_comment") %> + <% comment = changeset.tags["comment"].to_s == "" ? t("browse.no_comment") : changeset.tags["comment"] %> <%= link_to(comment, { :controller => "browse", :action => "changeset", :id => changeset.id }, { :title => t("changesets.changeset.view_changeset_details") }) %> diff --git a/config/initializers/oauth.rb b/config/initializers/oauth.rb index 2e2f45ab8..c89778551 100644 --- a/config/initializers/oauth.rb +++ b/config/initializers/oauth.rb @@ -35,12 +35,12 @@ module OpenStreetMap module ClassMethods def included(controller) controller.class_eval do - def self.before_filter(*names, &blk) - before_action(*names, &blk) + def self.before_filter(...) + before_action(...) end - def self.skip_before_filter(*names, &blk) - skip_before_action(*names, &blk) + def self.skip_before_filter(...) + skip_before_action(...) end end diff --git a/config/initializers/strong_migrations.rb b/config/initializers/strong_migrations.rb index b1f4707b5..a2a56074f 100644 --- a/config/initializers/strong_migrations.rb +++ b/config/initializers/strong_migrations.rb @@ -1 +1 @@ -StrongMigrations.start_after = 20190518115041 # rubocop:disable Style/NumericLiterals +StrongMigrations.start_after = 20190518115041 diff --git a/script/statistics b/script/statistics index 33205fe62..cd9e9afa7 100755 --- a/script/statistics +++ b/script/statistics @@ -86,7 +86,7 @@ rescue StandardError => e puts "

Exception: #{e}
#{e.backtrace.join('
')}

" end -puts "

Report took #{(Time.new - start_time)} seconds to run

" +puts "

Report took #{Time.new - start_time} seconds to run

" puts "" puts "" diff --git a/test/controllers/api/tracepoints_controller_test.rb b/test/controllers/api/tracepoints_controller_test.rb index fbe0450f2..d3fb195fe 100644 --- a/test/controllers/api/tracepoints_controller_test.rb +++ b/test/controllers/api/tracepoints_controller_test.rb @@ -105,7 +105,7 @@ module Api assert_equal "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat", @response.body, "A bbox param was expected" end - def test_traces_page_less_than_0 + def test_traces_page_less_than_zero -10.upto(-1) do |i| get trackpoints_path(:page => i, :bbox => "-0.1,-0.1,0.1,0.1") assert_response :bad_request diff --git a/test/controllers/geocoder_controller_test.rb b/test/controllers/geocoder_controller_test.rb index a7541c075..d0f0ce086 100644 --- a/test/controllers/geocoder_controller_test.rb +++ b/test/controllers/geocoder_controller_test.rb @@ -479,7 +479,7 @@ class GeocoderControllerTest < ActionDispatch::IntegrationTest assert_select "li.search_results_entry", results.count results.each do |result| - attrs = result.collect { |k, v| "[data-#{k}='#{v}']" }.join("") + attrs = result.collect { |k, v| "[data-#{k}='#{v}']" }.join assert_select "li.search_results_entry a.set_position#{attrs}", result[:name] end end diff --git a/test/controllers/traces_controller_test.rb b/test/controllers/traces_controller_test.rb index 364142dd9..36bee9c95 100644 --- a/test/controllers/traces_controller_test.rb +++ b/test/controllers/traces_controller_test.rb @@ -761,7 +761,9 @@ class TracesControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_template "index" - if !traces.empty? + if traces.empty? + assert_select "h4", /Nothing here yet/ + else assert_select "table#trace_list tbody", :count => 1 do assert_select "tr", :count => traces.length do |rows| traces.zip(rows).each do |trace, row| @@ -772,8 +774,6 @@ class TracesControllerTest < ActionDispatch::IntegrationTest end end end - else - assert_select "h4", /Nothing here yet/ end end diff --git a/test/factories/diary_entry_subscriptions.rb b/test/factories/diary_entry_subscriptions.rb index ced345479..b5fc8405d 100644 --- a/test/factories/diary_entry_subscriptions.rb +++ b/test/factories/diary_entry_subscriptions.rb @@ -1,4 +1,3 @@ FactoryBot.define do - factory :diary_entry_subscription do - end + factory :diary_entry_subscription end -- 2.39.5