Fix more rubocop style issues

.rubocop.yml

@@ -9,6 +9,10 @@ Style/FileName:
     - 'script/locale/reload-languages'
     - 'script/update-spam-blocks'
 
+Style/GlobalVars:
+  Exclude:
+    - 'lib/quad_tile/extconf.rb'
+    
 Style/HashSyntax:
   EnforcedStyle: hash_rockets
   Exclude:

.rubocop_todo.yml

@@ -70,29 +70,10 @@ Style/AccessorMethodName:
 Style/AsciiComments:
   Enabled: false
 
-# Offense count: 9
-Style/ClassVars:
-  Enabled: false
-
-# Offense count: 12
-# Configuration parameters: Keywords.
-Style/CommentAnnotation:
-  Enabled: false
-
 # Offense count: 306
 Style/Documentation:
   Enabled: false
 
-# Offense count: 8
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/FormatString:
-  Enabled: false
-
-# Offense count: 1
-# Configuration parameters: AllowedVariables.
-Style/GlobalVars:
-  Enabled: false
-
 # Offense count: 41
 # Configuration parameters: MinBodyLength.
 Style/GuardClause:
@@ -133,10 +114,6 @@ Style/RescueModifier:
 Style/StringLiteralsInInterpolation:
   Enabled: false
 
-# Offense count: 1
-Style/StructInheritance:
-  Enabled: false
-
 # Offense count: 3
 # Configuration parameters: ExactNameMatch, AllowPredicates, AllowDSLWriters, Whitelist.
 Style/TrivialAccessors:

app/controllers/application_controller.rb

@@ -272,7 +272,7 @@ class ApplicationController < ActionController::Base
   #  phrase from that, we can also put the error message into the status
   #  message. For now, rails won't let us)
   def report_error(message, status = :bad_request)
-    # Todo: some sort of escaping of problem characters in the message
+    # TODO: some sort of escaping of problem characters in the message
     response.headers["Error"] = message
 
     if request.headers["X-Error-Format"] &&

app/controllers/swf_controller.rb

@@ -119,7 +119,7 @@ class SwfController < ApplicationController
   def start_and_move(x, y, col)
     d = "001001"					# Line style change, moveTo
     l = [length_sb(x), length_sb(y)].max
-    d += sprintf("%05b%0#{l}b%0#{l}b", l, x, y)
+    d += format("%05b%0*b%0*b", l, l, x, l, y)
     d += col						# Select line style
     d
   end
@@ -147,9 +147,9 @@ class SwfController < ApplicationController
     dx = x2 - x1
     dy = y2 - y1
     l = [length_sb(dx), length_sb(dy)].max
-    d += sprintf("%04b", l - 2)
+    d += format("%04b", l - 2)
     d += "1"											# GeneralLine
-    d += sprintf("%0#{l}b%0#{l}b", dx, dy)
+    d += format("%0*b%0*b", l, dx, l, dy)
     d
   end
 
@@ -176,7 +176,7 @@ class SwfController < ApplicationController
          length_sb(c),
          length_sb(d)].max
     # create binary string (00111001 etc.) - 5-byte length, then bbox
-    n = sprintf("%05b%0#{l}b%0#{l}b%0#{l}b%0#{l}b", l, a, b, c, d)
+    n = format("%05b%0*b%0*b%0*b%0*b", l, l, a, l, b, l, c, l, d)
     # pack into byte string
     [n].pack("B*")
   end

app/helpers/title_helper.rb

@@ -1,11 +1,13 @@
 require "htmlentities"
 
 module TitleHelper
-  @@coder = HTMLEntities.new
+  def self.coder
+    @coder ||= HTMLEntities.new
+  end
 
   def set_title(title = false)
     if title
-      @title = @@coder.decode(title.gsub("<bdi>", "\u202a").gsub("</bdi>", "\u202c"))
+      @title = TitleHelper.coder.decode(title.gsub("<bdi>", "\u202a").gsub("</bdi>", "\u202c"))
       response.headers["X-Page-Title"] = t("layouts.project_name.title") + " | " + @title
     else
       @title = title

app/models/node.rb

@@ -141,7 +141,7 @@ class Node < ActiveRecord::Base
       # update changeset bbox with *old* position first
       changeset.update_bbox!(bbox)
 
-      # FIXME logic needs to be double checked
+      # FIXME: logic needs to be double checked
       self.latitude = new_node.latitude
       self.longitude = new_node.longitude
       self.tags = new_node.tags

app/models/old_way.rb

@@ -70,7 +70,7 @@ class OldWay < ActiveRecord::Base
 
     add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
 
-    old_nodes.each do |nd| # FIXME need to make sure they come back in the right order
+    old_nodes.each do |nd| # FIXME: need to make sure they come back in the right order
       node_el = XML::Node.new "nd"
       node_el["ref"] = nd.node_id.to_s
       el << node_el

app/models/relation.rb

@@ -135,7 +135,7 @@ class Relation < ActiveRecord::Base
     el
   end
 
-  # FIXME is this really needed?
+  # FIXME: is this really needed?
   def members
     @members ||= relation_members.map do |member|
       [member.member_type, member.member_id, member.member_role]

app/models/trace.rb

@@ -219,7 +219,7 @@ class Trace < ActiveRecord::Base
   end
 
   def xml_file
-    # TODO *nix specific, could do to work on windows... would be functionally inferior though - check for '.gz'
+    # TODO: *nix specific, could do to work on windows... would be functionally inferior though - check for '.gz'
     filetype = `/usr/bin/file -bz #{trace_name}`.chomp
     gzipped = filetype =~ /gzip compressed/
     bzipped = filetype =~ /bzip2 compressed/

db/migrate/021_move_to_innodb.rb

@@ -1,19 +1,19 @@
 require "migrate"
 
 class MoveToInnodb < ActiveRecord::Migration
-  @@conv_tables = %w(nodes ways way_tags way_nodes current_way_tags relation_members relations relation_tags current_relation_tags)
+  @conv_tables = %w(nodes ways way_tags way_nodes current_way_tags relation_members relations relation_tags current_relation_tags)
 
-  @@ver_tbl = %w(nodes ways relations)
+  @ver_tbl = %w(nodes ways relations)
 
   def self.up
     remove_index :current_way_tags, :name => :current_way_tags_v_idx
     remove_index :current_relation_tags, :name => :current_relation_tags_v_idx
 
-    @@ver_tbl.each do |tbl|
+    @ver_tbl.each do |tbl|
       change_column tbl, "version", :bigint, :null => false
     end
 
-    @@ver_tbl.each do |tbl|
+    @ver_tbl.each do |tbl|
       add_column "current_#{tbl}", "version", :bigint, :null => false
       # As the initial version of all nodes, ways and relations is 0, we set the
       # current version to something less so that we can update the version in

db/migrate/023_add_changesets.rb

@@ -1,7 +1,7 @@
 require "migrate"
 
 class AddChangesets < ActiveRecord::Migration
-  @@conv_user_tables = %w(current_nodes current_relations current_ways nodes relations ways)
+  @conv_user_tables = %w(current_nodes current_relations current_ways nodes relations ways)
 
   def self.up
     create_table "changesets", :id => false do |t|
@@ -31,7 +31,7 @@ class AddChangesets < ActiveRecord::Migration
     execute "INSERT INTO changesets (id, user_id, created_at, open)" +
       "SELECT id, id, creation_time, false from users;"
 
-    @@conv_user_tables.each do |tbl|
+    @conv_user_tables.each do |tbl|
       rename_column tbl, :user_id, :changeset_id
       # foreign keys too
       add_foreign_key tbl, :changesets, :name => "#{tbl}_changeset_id_fkey"

lib/country.rb

@@ -14,7 +14,7 @@ class Country
   end
 
   def self.countries
-    @@countries ||= load_countries
+    @countries ||= load_countries
   end
 
   def self.load_countries

lib/gpx.rb

@@ -150,11 +150,11 @@ module GPX
 
   private
 
-  class TrkPt < Struct.new(:segment, :latitude, :longitude, :altitude, :timestamp)
+  TrkPt = Struct.new(:segment, :latitude, :longitude, :altitude, :timestamp) do
     def valid?
       latitude && longitude && timestamp &&
-        latitude >= -90 && latitude <= 90 &&
+      latitude >= -90 && latitude <= 90 &&
-        longitude >= -180 && longitude <= 180
+      longitude >= -180 && longitude <= 180
     end
   end
 end

lib/quova.rb

@@ -34,13 +34,13 @@ module Quova
 
   ##
   # Create SOAP endpoint
-  @@soap = SOAP::WSDLDriverFactory.new(WSDL_URL).create_rpc_driver
+  @soap = SOAP::WSDLDriverFactory.new(WSDL_URL).create_rpc_driver
-  @@soap.options["protocol.http.basic_auth"] << [WSDL_URL, WSDL_USER, WSDL_PASS]
+  @soap.options["protocol.http.basic_auth"] << [WSDL_URL, WSDL_USER, WSDL_PASS]
 
   ##
   # Accessor for SOAP endpoint
   def self.soap
-    @@soap
+    @soap
   end
 
   ##

lib/rich_text.rb

@@ -72,18 +72,19 @@ module RichText
 
   class Markdown < Base
     def to_html
-      html_parser.render(self).html_safe
+      Markdown.html_parser.render(self).html_safe
     end
 
     def to_text
       to_s
     end
 
-    private
+    def self.html_renderer
+      @html_renderer ||= Renderer.new(:filter_html => true, :safe_links_only => true)
+    end
 
-    def html_parser
+    def self.html_parser
-      @@html_renderer ||= Renderer.new(:filter_html => true, :safe_links_only => true)
+      @html_parser ||= Redcarpet::Markdown.new(html_renderer, :no_intra_emphasis => true, :autolink => true, :space_after_headers => true)
-      @@html_parser ||= Redcarpet::Markdown.new(@@html_renderer,         :no_intra_emphasis => true, :autolink => true, :space_after_headers => true)
     end
 
     class Renderer < Redcarpet::Render::XHTML

lib/validators.rb

@@ -1,16 +1,13 @@
 module ActiveRecord
   module Validations
     module ClassMethods
-      # error message when invalid UTF-8 is detected
-      @@invalid_utf8_message = " is invalid UTF-8"
-
       ##
       # validation method to be included like any other validations methods
       # in the models definitions. this one checks that the named attribute
       # is a valid UTF-8 format string.
       def validates_as_utf8(*attrs)
         validates_each(attrs) do |record, attr, value|
-          record.errors.add(attr, @@invalid_utf8_message) unless UTF8.valid? value
+          record.errors.add(attr, " is invalid UTF-8") unless UTF8.valid? value
         end
       end
     end

test/controllers/changeset_controller_test.rb

@@ -1381,7 +1381,7 @@ EOF
     assert_response :success
     assert_template nil
     # print @response.body
-    # FIXME needs more assert_select tests
+    # FIXME: needs more assert_select tests
     assert_select "osmChange[version='#{API_VERSION}'][generator='#{GENERATOR}']" do
       assert_select "create", :count => 5
       assert_select "create>node[id='#{nodes(:used_node_2).node_id}'][visible='#{nodes(:used_node_2).visible?}'][version='#{nodes(:used_node_2).version}']" do
@@ -1393,7 +1393,7 @@ EOF
 
   ##
   # check that the bounding box of a changeset gets updated correctly
-  ## FIXME: This should really be moded to a integration test due to the with_controller
+  # FIXME: This should really be moded to a integration test due to the with_controller
   def test_changeset_bbox
     basic_authorization users(:public_user).email, "test"
 
@@ -1769,7 +1769,7 @@ EOF
     # Now check that all 20 (or however many were returned) changesets are in the html
     assert_select "li", :count => changesets.size
     changesets.each do |_changeset|
-      # FIXME this test needs rewriting - test for table contents
+      # FIXME: this test needs rewriting - test for table contents
     end
   end
 
@@ -1795,7 +1795,7 @@ EOF
     # Now check that all 20 (or however many were returned) changesets are in the html
     assert_select "li", :count => changesets.size
     changesets.each do |_changeset|
-      # FIXME this test needs rewriting - test for table contents
+      # FIXME: this test needs rewriting - test for table contents
     end
   end
 
@@ -1806,7 +1806,7 @@ EOF
     get :list, :format => "html", :display_name => user.display_name
     assert_response :success
     assert_template "history"
-    ## FIXME need to add more checks to see which if edits are actually shown if your data is public
+    # FIXME: need to add more checks to see which if edits are actually shown if your data is public
   end
 
   ##
@@ -1829,7 +1829,7 @@ EOF
     assert_select "feed", :count => 1
     assert_select "entry", :count => changesets.size
     changesets.each do |_changeset|
-      # FIXME this test needs rewriting - test for feed contents
+      # FIXME: this test needs rewriting - test for feed contents
     end
   end
 
@@ -1841,7 +1841,7 @@ EOF
     assert_response :success
     assert_template "list"
     assert_equal "application/atom+xml", response.content_type
-    ## FIXME need to add more checks to see which if edits are actually shown if your data is public
+    # FIXME: need to add more checks to see which if edits are actually shown if your data is public
   end
 
   ##

test/controllers/node_controller_test.rb

@@ -499,14 +499,6 @@ class NodeControllerTest < ActionController::TestCase
     assert apinode.tags.include?("\#{@user.inspect}")
   end
 
-  def basic_authorization(user, pass)
-    @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}")
-  end
-
-  def content(c)
-    @request.env["RAW_POST_DATA"] = c.to_s
-  end
-
   ##
   # update the changeset_id of a node element
   def update_changeset(xml, changeset_id)

test/controllers/old_node_controller_test.rb

@@ -31,7 +31,7 @@ class OldNodeControllerTest < ActionController::TestCase
   # matching versions of the object.
   #
   ##
-  # FIXME Move this test to being an integration test since it spans multiple controllers
+  # FIXME: Move this test to being an integration test since it spans multiple controllers
   def test_version
     ## First try this with a non-public user
     basic_authorization(users(:normal_user).email, "test")

test/controllers/relation_controller_test.rb

@@ -107,7 +107,7 @@ class RelationControllerTest < ActionController::TestCase
     # check the "full" mode
     get :full, :id => current_relations(:visible_relation).id
     assert_response :success
-    # FIXME check whether this contains the stuff we want!
+    # FIXME: check whether this contains the stuff we want!
     print @response.body if $VERBOSE
   end
 

test/integration/user_blocks_test.rb

@@ -4,7 +4,7 @@ class UserBlocksTest < ActionDispatch::IntegrationTest
   fixtures :users, :user_blocks, :user_roles
 
   def auth_header(user, pass)
-    { "HTTP_AUTHORIZATION" => "Basic %s" % Base64.encode64("#{user}:#{pass}") }
+    { "HTTP_AUTHORIZATION" => format("Basic %s", Base64.encode64("#{user}:#{pass}")) }
   end
 
   def test_api_blocked

test/integration/user_terms_seen_test.rb

@@ -70,7 +70,7 @@ class UserTermsSeenTest < ActionDispatch::IntegrationTest
   private
 
   def auth_header(user, pass)
-    { "HTTP_AUTHORIZATION" => "Basic %s" % Base64.encode64("#{user}:#{pass}") }
+    { "HTTP_AUTHORIZATION" => format("Basic %s", Base64.encode64("#{user}:#{pass}")) }
   end
 
   def with_terms_seen(value)

test/models/message_test.rb

@@ -55,7 +55,7 @@ class MessageTest < ActiveSupport::TestCase
 
   def test_invalid_utf8
     # See e.g http://en.wikipedia.org/wiki/UTF-8 for byte sequences
-    # FIXME - Invalid Unicode characters can still be encoded into "valid" utf-8 byte sequences - maybe check this too?
+    # FIXME: Invalid Unicode characters can still be encoded into "valid" utf-8 byte sequences - maybe check this too?
     invalid_sequences = ["\xC0",         # always invalid utf8
                          "\xC2\x4a",     # 2-byte multibyte identifier, followed by plain ASCII
                          "\xC2\xC2",     # 2-byte multibyte identifier, followed by another one

test/test_helper.rb

@@ -115,7 +115,7 @@ module ActiveSupport
     end
 
     def basic_authorization(user, pass)
-      @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}")
+      @request.env["HTTP_AUTHORIZATION"] = format("Basic %s", Base64.encode64("#{user}:#{pass}"))
     end
 
     def error_format(format)