From 2d3972249c18fc5cd193a8b2f2efe9b46badb217 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sun, 2 Aug 2020 19:38:58 +0100 Subject: [PATCH 1/1] Fix some rubocop todos --- .rubocop.yml | 3 ++- .rubocop_todo.yml | 27 +++---------------- app/controllers/application_controller.rb | 4 ++- app/helpers/application_helper.rb | 10 +++---- app/helpers/trace_helper.rb | 6 +---- app/models/concerns/geo_record.rb | 4 +-- app/views/api/map/_bounds.xml.builder | 8 +++--- lib/bounding_box.rb | 8 +++--- test/controllers/api/map_controller_test.rb | 6 ++--- .../api/relations_controller_test.rb | 8 +++--- test/controllers/api/ways_controller_test.rb | 2 +- test/integration/compressed_requests_test.rb | 8 +++--- test/integration/user_blocks_test.rb | 2 +- test/integration/user_terms_seen_test.rb | 2 +- test/lib/bounding_box_test.rb | 16 +++++------ test/models/message_test.rb | 2 +- test/test_helper.rb | 2 +- 17 files changed, 48 insertions(+), 70 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 334c8112b..ac29f4fc8 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -67,7 +67,8 @@ Style/Documentation: Enabled: false Style/FormatStringToken: - EnforcedStyle: template + Exclude: + - 'config/routes.rb' Style/IfInsideElse: Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 02369943f..66942d6a4 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2020-07-31 18:09:40 UTC using RuboCop version 0.86.0. +# on 2020-08-02 18:37:55 UTC using RuboCop version 0.86.0. # 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,7 +13,7 @@ require: - rubocop-performance - rubocop-rails -# Offense count: 565 +# Offense count: 568 # Cop supports --auto-correct. # Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https @@ -123,14 +123,12 @@ Rails/HasAndBelongsToMany: - 'app/models/changeset.rb' - 'app/models/user.rb' -# Offense count: 11 +# Offense count: 4 # Configuration parameters: Include. # Include: app/helpers/**/*.rb Rails/HelperInstanceVariable: Exclude: - - 'app/helpers/application_helper.rb' - 'app/helpers/title_helper.rb' - - 'app/helpers/trace_helper.rb' # Offense count: 5 # Configuration parameters: Include. @@ -163,25 +161,6 @@ Rails/OutputSafety: Rails/TimeZone: Enabled: false -# Offense count: 1 -# Configuration parameters: AllowedChars. -Style/AsciiComments: - Exclude: - - 'test/models/message_test.rb' - -# Offense count: 32 -# Configuration parameters: EnforcedStyle. -# SupportedStyles: annotated, template, unannotated -Style/FormatStringToken: - Exclude: - - 'app/models/concerns/geo_record.rb' - - 'app/views/api/map/_bounds.xml.builder' - - 'lib/bounding_box.rb' - - 'test/controllers/api/map_controller_test.rb' - - 'test/controllers/api/relations_controller_test.rb' - - 'test/controllers/api/ways_controller_test.rb' - - 'test/lib/bounding_box_test.rb' - # Offense count: 572 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f305e14d7..f1e847e10 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -12,8 +12,10 @@ class ApplicationController < ActionController::Base around_action :better_errors_allow_inline, :if => proc { Rails.env.development? } attr_accessor :current_user + attr_accessor :oauth_token helper_method :current_user + helper_method :oauth_token helper_method :preferred_langauges private @@ -58,7 +60,7 @@ class ApplicationController < ActionController::Base end def require_oauth - @oauth = current_user.access_token(Settings.oauth_key) if current_user && Settings.key?(:oauth_key) + @oauth_token = current_user.access_token(Settings.oauth_key) if current_user && Settings.key?(:oauth_key) end ## diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 5f36e262b..2c8b00c17 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -79,11 +79,11 @@ module ApplicationHelper data[:location] = session[:location] if session[:location] - if @oauth - data[:token] = @oauth.token - data[:token_secret] = @oauth.secret - data[:consumer_key] = @oauth.client_application.key - data[:consumer_secret] = @oauth.client_application.secret + if oauth_token + data[:token] = oauth_token.token + data[:token_secret] = oauth_token.secret + data[:consumer_key] = oauth_token.client_application.key + data[:consumer_secret] = oauth_token.client_application.secret end data diff --git a/app/helpers/trace_helper.rb b/app/helpers/trace_helper.rb index 15bc32313..aec1146cb 100644 --- a/app/helpers/trace_helper.rb +++ b/app/helpers/trace_helper.rb @@ -1,9 +1,5 @@ module TraceHelper def link_to_tag(tag) - if @action == "mine" - link_to(tag, :tag => tag, :page => nil) - else - link_to(tag, :tag => tag, :display_name => @display_name, :page => nil) - end + link_to(tag, :tag => tag, :page => nil) end end diff --git a/app/models/concerns/geo_record.rb b/app/models/concerns/geo_record.rb index 447ee19df..a5635c172 100644 --- a/app/models/concerns/geo_record.rb +++ b/app/models/concerns/geo_record.rb @@ -10,11 +10,11 @@ module GeoRecord end def to_s - format("%.7f", self) + format("%.7f", :coord => self) end def as_json(_) - format("%.7f", self).to_f + format("%.7f", :coord => self).to_f end end diff --git a/app/views/api/map/_bounds.xml.builder b/app/views/api/map/_bounds.xml.builder index 8a38e21ea..0a4181c37 100644 --- a/app/views/api/map/_bounds.xml.builder +++ b/app/views/api/map/_bounds.xml.builder @@ -1,8 +1,8 @@ attrs = { - "minlat" => format("%.7f", bounds.min_lat), - "minlon" => format("%.7f", bounds.min_lon), - "maxlat" => format("%.7f", bounds.max_lat), - "maxlon" => format("%.7f", bounds.max_lon) + "minlat" => format("%.7f", :lat => bounds.min_lat), + "minlon" => format("%.7f", :lon => bounds.min_lon), + "maxlat" => format("%.7f", :lat => bounds.max_lat), + "maxlon" => format("%.7f", :lon => bounds.max_lon) } xml.bounds(attrs) diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb index d1c39f1f4..630518649 100644 --- a/lib/bounding_box.rb +++ b/lib/bounding_box.rb @@ -124,10 +124,10 @@ class BoundingBox # there are two forms used for bounds with and without an underscore, # cater for both forms eg minlon and min_lon def add_bounds_to(hash, underscore = "") - hash["min#{underscore}lat"] = format("%.7f", min_lat) - hash["min#{underscore}lon"] = format("%.7f", min_lon) - hash["max#{underscore}lat"] = format("%.7f", max_lat) - hash["max#{underscore}lon"] = format("%.7f", max_lon) + hash["min#{underscore}lat"] = format("%.7f", :lat => min_lat) + hash["min#{underscore}lon"] = format("%.7f", :lon => min_lon) + hash["max#{underscore}lat"] = format("%.7f", :lat => max_lat) + hash["max#{underscore}lon"] = format("%.7f", :lon => max_lon) hash end diff --git a/test/controllers/api/map_controller_test.rb b/test/controllers/api/map_controller_test.rb index fb64b0716..dd7bb2cb3 100644 --- a/test/controllers/api/map_controller_test.rb +++ b/test/controllers/api/map_controller_test.rb @@ -131,8 +131,8 @@ module Api end assert_response :success, "Expected scucess with the map call" assert_select "osm[version='#{Settings.api_version}'][generator='#{Settings.generator}']", :count => 1 do - assert_select "bounds[minlon='#{format('%.7f', minlon)}'][minlat='#{format('%.7f', minlat)}'][maxlon='#{format('%.7f', maxlon)}'][maxlat='#{format('%.7f', maxlat)}']", :count => 1 - assert_select "node[id='#{node.id}'][lat='#{format('%.7f', node.lat)}'][lon='#{format('%.7f', node.lon)}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do + assert_select "bounds[minlon='#{format('%.7f', :lon => minlon)}'][minlat='#{format('%.7f', :lat => minlat)}'][maxlon='#{format('%.7f', :lon => maxlon)}'][maxlat='#{format('%.7f', :lat => maxlat)}']", :count => 1 + assert_select "node[id='#{node.id}'][lat='#{format('%.7f', :lat => node.lat)}'][lon='#{format('%.7f', :lon => node.lon)}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do # This should really be more generic assert_select "tag[k='#{tag.k}'][v='#{tag.v}']" end @@ -206,7 +206,7 @@ module Api assert_response :success, "The map call should have succeeded" assert_select "osm[version='#{Settings.api_version}'][generator='#{Settings.generator}']", :count => 1 do assert_select "bounds[minlon='#{node.lon}'][minlat='#{node.lat}'][maxlon='#{node.lon}'][maxlat='#{node.lat}']", :count => 1 - assert_select "node[id='#{node.id}'][lat='#{format('%.7f', node.lat)}'][lon='#{format('%.7f', node.lon)}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do + assert_select "node[id='#{node.id}'][lat='#{format('%.7f', :lat => node.lat)}'][lon='#{format('%.7f', :lon => node.lon)}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do # This should really be more generic assert_select "tag[k='#{tag.k}'][v='#{tag.v}']" end diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb index cff8d65e1..b04ad3c19 100644 --- a/test/controllers/api/relations_controller_test.rb +++ b/test/controllers/api/relations_controller_test.rb @@ -993,10 +993,10 @@ module Api assert_response :success, "can't re-read changeset for modify test" assert_select "osm>changeset", 1, "Changeset element doesn't exist in #{@response.body}" assert_select "osm>changeset[id='#{changeset_id}']", 1, "Changeset id=#{changeset_id} doesn't exist in #{@response.body}" - assert_select "osm>changeset[min_lon='#{format('%.7f', bbox.min_lon)}']", 1, "Changeset min_lon wrong in #{@response.body}" - assert_select "osm>changeset[min_lat='#{format('%.7f', bbox.min_lat)}']", 1, "Changeset min_lat wrong in #{@response.body}" - assert_select "osm>changeset[max_lon='#{format('%.7f', bbox.max_lon)}']", 1, "Changeset max_lon wrong in #{@response.body}" - assert_select "osm>changeset[max_lat='#{format('%.7f', bbox.max_lat)}']", 1, "Changeset max_lat wrong in #{@response.body}" + assert_select "osm>changeset[min_lon='#{format('%.7f', :lon => bbox.min_lon)}']", 1, "Changeset min_lon wrong in #{@response.body}" + assert_select "osm>changeset[min_lat='#{format('%.7f', :lat => bbox.min_lat)}']", 1, "Changeset min_lat wrong in #{@response.body}" + assert_select "osm>changeset[max_lon='#{format('%.7f', :lon => bbox.max_lon)}']", 1, "Changeset max_lon wrong in #{@response.body}" + assert_select "osm>changeset[max_lat='#{format('%.7f', :lat => bbox.max_lat)}']", 1, "Changeset max_lat wrong in #{@response.body}" end end diff --git a/test/controllers/api/ways_controller_test.rb b/test/controllers/api/ways_controller_test.rb index a6f8fae82..672a282a9 100644 --- a/test/controllers/api/ways_controller_test.rb +++ b/test/controllers/api/ways_controller_test.rb @@ -77,7 +77,7 @@ module Api # reference and as the node element. way.nodes.each do |n| assert_select "osm way nd[ref='#{n.id}']", 1 - assert_select "osm node[id='#{n.id}'][version='1'][lat='#{format('%.7f', n.lat)}'][lon='#{format('%.7f', n.lon)}']", 1 + assert_select "osm node[id='#{n.id}'][version='1'][lat='#{format('%.7f', :lat => n.lat)}'][lon='#{format('%.7f', :lon => n.lon)}']", 1 end end diff --git a/test/integration/compressed_requests_test.rb b/test/integration/compressed_requests_test.rb index 55db373d0..ecffe3c46 100644 --- a/test/integration/compressed_requests_test.rb +++ b/test/integration/compressed_requests_test.rb @@ -38,7 +38,7 @@ class CompressedRequestsTest < ActionDispatch::IntegrationTest post "/api/0.6/changeset/#{changeset.id}/upload", :params => diff, :headers => { - "HTTP_AUTHORIZATION" => format("Basic %{auth}", :auth => Base64.encode64("#{user.display_name}:test")), + "HTTP_AUTHORIZATION" => format("Basic %s", :auth => Base64.encode64("#{user.display_name}:test")), "HTTP_CONTENT_TYPE" => "application/xml" } assert_response :success, @@ -87,7 +87,7 @@ class CompressedRequestsTest < ActionDispatch::IntegrationTest post "/api/0.6/changeset/#{changeset.id}/upload", :params => gzip_content(diff), :headers => { - "HTTP_AUTHORIZATION" => format("Basic %{auth}", :auth => Base64.encode64("#{user.display_name}:test")), + "HTTP_AUTHORIZATION" => format("Basic %s", :auth => Base64.encode64("#{user.display_name}:test")), "HTTP_CONTENT_ENCODING" => "gzip", "HTTP_CONTENT_TYPE" => "application/xml" } @@ -137,7 +137,7 @@ class CompressedRequestsTest < ActionDispatch::IntegrationTest post "/api/0.6/changeset/#{changeset.id}/upload", :params => deflate_content(diff), :headers => { - "HTTP_AUTHORIZATION" => format("Basic %{auth}", :auth => Base64.encode64("#{user.display_name}:test")), + "HTTP_AUTHORIZATION" => format("Basic %s", :auth => Base64.encode64("#{user.display_name}:test")), "HTTP_CONTENT_ENCODING" => "deflate", "HTTP_CONTENT_TYPE" => "application/xml" } @@ -158,7 +158,7 @@ class CompressedRequestsTest < ActionDispatch::IntegrationTest post "/api/0.6/changeset/#{changeset.id}/upload", :params => "", :headers => { - "HTTP_AUTHORIZATION" => format("Basic %{auth}", :auth => Base64.encode64("#{user.display_name}:test")), + "HTTP_AUTHORIZATION" => format("Basic %s", :auth => Base64.encode64("#{user.display_name}:test")), "HTTP_CONTENT_ENCODING" => "unknown", "HTTP_CONTENT_TYPE" => "application/xml" } diff --git a/test/integration/user_blocks_test.rb b/test/integration/user_blocks_test.rb index 3bc6ded98..ce67e74ba 100644 --- a/test/integration/user_blocks_test.rb +++ b/test/integration/user_blocks_test.rb @@ -2,7 +2,7 @@ require "test_helper" class UserBlocksTest < ActionDispatch::IntegrationTest def auth_header(user, pass) - { "HTTP_AUTHORIZATION" => format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}")) } + { "HTTP_AUTHORIZATION" => format("Basic %s", :auth => Base64.encode64("#{user}:#{pass}")) } end def test_api_blocked diff --git a/test/integration/user_terms_seen_test.rb b/test/integration/user_terms_seen_test.rb index 51eab5b18..4bffd99de 100644 --- a/test/integration/user_terms_seen_test.rb +++ b/test/integration/user_terms_seen_test.rb @@ -64,6 +64,6 @@ class UserTermsSeenTest < ActionDispatch::IntegrationTest private def auth_header(user, pass) - { "HTTP_AUTHORIZATION" => format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}")) } + { "HTTP_AUTHORIZATION" => format("Basic %s", :auth => Base64.encode64("#{user}:#{pass}")) } end end diff --git a/test/lib/bounding_box_test.rb b/test/lib/bounding_box_test.rb index 56c681aae..866820a6e 100644 --- a/test/lib/bounding_box_test.rb +++ b/test/lib/bounding_box_test.rb @@ -265,19 +265,19 @@ class BoundingBoxTest < ActiveSupport::TestCase def test_add_bounds_to_no_underscore bounds = @bbox_from_string.add_bounds_to({}) assert_equal 4, bounds.size - assert_equal format("%.7f", @min_lon), bounds["minlon"] - assert_equal format("%.7f", @min_lat), bounds["minlat"] - assert_equal format("%.7f", @max_lon), bounds["maxlon"] - assert_equal format("%.7f", @max_lat), bounds["maxlat"] + assert_equal format("%.7f", :lon => @min_lon), bounds["minlon"] + assert_equal format("%.7f", :lat => @min_lat), bounds["minlat"] + assert_equal format("%.7f", :lon => @max_lon), bounds["maxlon"] + assert_equal format("%.7f", :lat => @max_lat), bounds["maxlat"] end def test_add_bounds_to_with_underscore bounds = @bbox_from_string.add_bounds_to({}, "_") assert_equal 4, bounds.size - assert_equal format("%.7f", @min_lon), bounds["min_lon"] - assert_equal format("%.7f", @min_lat), bounds["min_lat"] - assert_equal format("%.7f", @max_lon), bounds["max_lon"] - assert_equal format("%.7f", @max_lat), bounds["max_lat"] + assert_equal format("%.7f", :lon => @min_lon), bounds["min_lon"] + assert_equal format("%.7f", :lat => @min_lat), bounds["min_lat"] + assert_equal format("%.7f", :lon => @max_lon), bounds["max_lon"] + assert_equal format("%.7f", :lat => @max_lat), bounds["max_lat"] end def test_to_scaled diff --git a/test/models/message_test.rb b/test/models/message_test.rb index 60ff19e44..73ae5f76c 100644 --- a/test/models/message_test.rb +++ b/test/models/message_test.rb @@ -186,7 +186,7 @@ class MessageTest < ActiveSupport::TestCase def assert_message_ok(char, count) message = make_message(char, count) assert message.save! - response = message.class.find(message.id) # stand by for some über-generalisation... + response = message.class.find(message.id) # stand by for some uber-generalisation... assert_equal char * count, response.title, "message with #{count} #{char} chars (i.e. #{char.length * count} bytes) fails" end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 9354e6646..40b2b50bc 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -118,7 +118,7 @@ module ActiveSupport ## # return request header for HTTP Basic Authorization def basic_authorization_header(user, pass) - { "Authorization" => format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}")) } + { "Authorization" => format("Basic %s", :auth => Base64.encode64("#{user}:#{pass}")) } end ## -- 2.39.5