Fix some rubocop todos

.rubocop.yml

@@ -67,7 +67,8 @@ Style/Documentation:
   Enabled: false
 
 Style/FormatStringToken:
-  EnforcedStyle: template
+  Exclude:
+    - 'config/routes.rb'
 
 Style/IfInsideElse:
   Enabled: false

.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.

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
 
   ##

app/helpers/application_helper.rb

@@ -79,11 +79,11 @@ module ApplicationHelper
 
     data[:location] = session[:location] if session[:location]
 
-    if @oauth
+    if oauth_token
-      data[:token] = @oauth.token
+      data[:token] = oauth_token.token
-      data[:token_secret] = @oauth.secret
+      data[:token_secret] = oauth_token.secret
-      data[:consumer_key] = @oauth.client_application.key
+      data[:consumer_key] = oauth_token.client_application.key
-      data[:consumer_secret] = @oauth.client_application.secret
+      data[:consumer_secret] = oauth_token.client_application.secret
     end
 
     data

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
   end
 end

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
 

app/views/api/map/_bounds.xml.builder

@@ -1,8 +1,8 @@
 attrs = {
-  "minlat" => format("%.7f", bounds.min_lat),
+  "minlat" => format("%<lat>.7f", :lat => bounds.min_lat),
-  "minlon" => format("%.7f", bounds.min_lon),
+  "minlon" => format("%<lon>.7f", :lon => bounds.min_lon),
-  "maxlat" => format("%.7f", bounds.max_lat),
+  "maxlat" => format("%<lat>.7f", :lat => bounds.max_lat),
-  "maxlon" => format("%.7f", bounds.max_lon)
+  "maxlon" => format("%<lon>.7f", :lon => bounds.max_lon)
 }
 
 xml.bounds(attrs)

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}lat"] = format("%<lat>.7f", :lat => min_lat)
-    hash["min#{underscore}lon"] = format("%.7f", min_lon)
+    hash["min#{underscore}lon"] = format("%<lon>.7f", :lon => min_lon)
-    hash["max#{underscore}lat"] = format("%.7f", max_lat)
+    hash["max#{underscore}lat"] = format("%<lat>.7f", :lat => max_lat)
-    hash["max#{underscore}lon"] = format("%.7f", max_lon)
+    hash["max#{underscore}lon"] = format("%<lon>.7f", :lon => max_lon)
     hash
   end
 

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 "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('%.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
@@ -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

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_lon='#{format('%<lon>.7f', :lon => 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[min_lat='#{format('%<lat>.7f', :lat => 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_lon='#{format('%<lon>.7f', :lon => 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[max_lat='#{format('%<lat>.7f', :lat => bbox.max_lat)}']", 1, "Changeset max_lat wrong in #{@response.body}"
       end
     end
 

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
 

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"
          }

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

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

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("%<lon>.7f", :lon => @min_lon), bounds["minlon"]
-    assert_equal format("%.7f", @min_lat), bounds["minlat"]
+    assert_equal format("%<lat>.7f", :lat => @min_lat), bounds["minlat"]
-    assert_equal format("%.7f", @max_lon), bounds["maxlon"]
+    assert_equal format("%<lon>.7f", :lon => @max_lon), bounds["maxlon"]
-    assert_equal format("%.7f", @max_lat), bounds["maxlat"]
+    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("%<lon>.7f", :lon => @min_lon), bounds["min_lon"]
-    assert_equal format("%.7f", @min_lat), bounds["min_lat"]
+    assert_equal format("%<lat>.7f", :lat => @min_lat), bounds["min_lat"]
-    assert_equal format("%.7f", @max_lon), bounds["max_lon"]
+    assert_equal format("%<lon>.7f", :lon => @max_lon), bounds["max_lon"]
-    assert_equal format("%.7f", @max_lat), bounds["max_lat"]
+    assert_equal format("%<lat>.7f", :lat => @max_lat), bounds["max_lat"]
   end
 
   def test_to_scaled

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

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
 
     ##