Fix some rubocop todos

diff --git a/.rubocop.yml b/.rubocop.yml
index 334c8112b498c45d58694fab7757cefcc6a003f1..ac29f4fc8769d9aec2d1142f33eef65bf698383e 100644 (file)
--- 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 02369943fba04f9c1969702f84a148c2caecdc61..66942d6a47c1c01168ab854b187dea49eb2c2caf 100644 (file)
--- 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 f305e14d7524182cf3e5ddb12f1ce84cbb8d62b5..f1e847e109dc48afef4f834bb3232ad6e551344e 100644 (file)
--- 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 5f36e262b9d24677f2cff1497e0d967718b471fe..2c8b00c17460504ddb49319b6506992a25e2be20 100644 (file)
--- 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 15bc3231316bb5340252be3d36fb35cfa016dff3..aec1146cb8da9c68fd100364be0b7a626a98ecd5 100644 (file)
--- 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 447ee19df61050e83f229619db8968351498a62a..a5635c1729b2610442df10a8019f30f2ad614029 100644 (file)
--- 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("%<coord>.7f", :coord => self)
     end
 
     def as_json(_)
-      format("%.7f", self).to_f
+      format("%<coord>.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 8a38e21ea8cf16ee52d5a9f8a6d01cda503b431e..0a4181c37bd4cd96732723c374e1e6a62d9bd91b 100644 (file)
--- 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("%<lat>.7f", :lat => bounds.min_lat),
+  "minlon" => format("%<lon>.7f", :lon => bounds.min_lon),
+  "maxlat" => format("%<lat>.7f", :lat => bounds.max_lat),
+  "maxlon" => format("%<lon>.7f", :lon => bounds.max_lon)
 }
 
 xml.bounds(attrs)

diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb
index d1c39f1f4dbf73674f6f875d4489c86be17a0c1a..630518649d5814894d4de0e44b09c4977fe6f493 100644 (file)
--- 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("%<lat>.7f", :lat => min_lat)
+    hash["min#{underscore}lon"] = format("%<lon>.7f", :lon => min_lon)
+    hash["max#{underscore}lat"] = format("%<lat>.7f", :lat => max_lat)
+    hash["max#{underscore}lon"] = format("%<lon>.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 fb64b07163581259ea6b8cd0ce6bb47b312f0ac0..dd7bb2cb3be9861e7752d60b0ac022299ec34b3f 100644 (file)
--- 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('%<lon>.7f', :lon => minlon)}'][minlat='#{format('%<lat>.7f', :lat => minlat)}'][maxlon='#{format('%<lon>.7f', :lon => maxlon)}'][maxlat='#{format('%<lat>.7f', :lat => maxlat)}']", :count => 1
+        assert_select "node[id='#{node.id}'][lat='#{format('%<lat>.7f', :lat => node.lat)}'][lon='#{format('%<lon>.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('%<lat>.7f', :lat => node.lat)}'][lon='#{format('%<lon>.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 cff8d65e1fb0771914453b2ef362054531c7f2d8..b04ad3c19b1aeca6a5f0639ce9ebfabffd53424c 100644 (file)
--- 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('%<lon>.7f', :lon => bbox.min_lon)}']", 1, "Changeset min_lon wrong in #{@response.body}"
+        assert_select "osm>changeset[min_lat='#{format('%<lat>.7f', :lat => bbox.min_lat)}']", 1, "Changeset min_lat wrong in #{@response.body}"
+        assert_select "osm>changeset[max_lon='#{format('%<lon>.7f', :lon => bbox.max_lon)}']", 1, "Changeset max_lon wrong in #{@response.body}"
+        assert_select "osm>changeset[max_lat='#{format('%<lat>.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 a6f8fae825a40663df2f19058230be3b3d3ef0b7..672a282a9659e908f2508e008c2c30bed430f477 100644 (file)
--- 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('%<lat>.7f', :lat => n.lat)}'][lon='#{format('%<lon>.7f', :lon => n.lon)}']", 1
       end
     end
 

diff --git a/test/integration/compressed_requests_test.rb b/test/integration/compressed_requests_test.rb
index 55db373d00963dfb4655cd636860f8d955978076..ecffe3c46781e9edfac50e1208300489986e3e13 100644 (file)
--- 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 %<auth>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 %<auth>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 %<auth>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 %<auth>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 3bc6ded98b74b164f9af4ba62cbfc6e8879713cf..ce67e74ba5455efef6efce6e91e869a8c747678b 100644 (file)
--- 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 %<auth>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 51eab5b18947ca0d0d53c635a8aa9a305b17d228..4bffd99de598c9fddde9f754b019908e5cf6ac26 100644 (file)
--- 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 %<auth>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 56c681aae90f02f3c0214cedae21731c78aa6bd1..866820a6e2fecf799a9eb18e285e250085409662 100644 (file)
--- 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("%<lon>.7f", :lon => @min_lon), bounds["minlon"]
+    assert_equal format("%<lat>.7f", :lat => @min_lat), bounds["minlat"]
+    assert_equal format("%<lon>.7f", :lon => @max_lon), bounds["maxlon"]
+    assert_equal format("%<lat>.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("%<lon>.7f", :lon => @min_lon), bounds["min_lon"]
+    assert_equal format("%<lat>.7f", :lat => @min_lat), bounds["min_lat"]
+    assert_equal format("%<lon>.7f", :lon => @max_lon), bounds["max_lon"]
+    assert_equal format("%<lat>.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 60ff19e4467e616c1cb2a91608dddbf016fdcd6d..73ae5f76c8350d2b63da9a2ff0e8f75ba479ade6 100644 (file)
--- 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 9354e664600cd87eccfe4d3107b125bb4b692005..40b2b50bc7911f985121862aa6b9dea8fa6ad812 100644 (file)
--- 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 %<auth>s", :auth => Base64.encode64("#{user}:#{pass}")) }
     end
 
     ##