Fix style issues found by new rubocop version

.rubocop_todo.yml

@@ -1,19 +1,19 @@
 # This configuration was generated by `rubocop --auto-gen-config`
-# on 2015-02-25 00:27:05 +0000 using RuboCop version 0.29.1.
+# on 2015-04-14 09:29:11 +0100 using RuboCop version 0.30.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
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 13
+# Offense count: 34
 Lint/AmbiguousOperator:
   Enabled: false
 
-# Offense count: 107
+# Offense count: 125
 Lint/AmbiguousRegexpLiteral:
   Enabled: false
 
-# Offense count: 29
+# Offense count: 31
 # Configuration parameters: AllowSafeAssignment.
 Lint/AssignmentInCondition:
   Enabled: false
@@ -26,29 +26,29 @@ Lint/HandleExceptions:
 Lint/ParenthesesAsGroupedExpression:
   Enabled: false
 
-# Offense count: 542
+# Offense count: 646
 Metrics/AbcSize:
-  Max: 280
+  Max: 277
 
 # Offense count: 12
 Metrics/BlockNesting:
   Max: 5
 
-# Offense count: 60
+# Offense count: 62
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
   Max: 1653
 
 # Offense count: 68
 Metrics/CyclomaticComplexity:
-  Max: 21
+  Max: 20
 
-# Offense count: 2092
+# Offense count: 2454
 # Configuration parameters: AllowURI, URISchemes.
 Metrics/LineLength:
   Max: 694
 
-# Offense count: 520
+# Offense count: 601
 # Configuration parameters: CountComments.
 Metrics/MethodLength:
   Max: 179
@@ -60,13 +60,18 @@ Metrics/ParameterLists:
 
 # Offense count: 70
 Metrics/PerceivedComplexity:
-  Max: 24
+  Max: 23
 
 # Offense count: 2
 # Configuration parameters: Include.
 Rails/HasAndBelongsToMany:
   Enabled: false
 
+# Offense count: 70
+# Configuration parameters: EnforcedStyle, SupportedStyles.
+Rails/TimeZone:
+  Enabled: false
+
 # Offense count: 5
 Style/AccessorMethodName:
   Enabled: false
@@ -75,7 +80,7 @@ Style/AccessorMethodName:
 Style/AsciiComments:
   Enabled: false
 
-# Offense count: 305
+# Offense count: 307
 Style/Documentation:
   Enabled: false
 
@@ -89,12 +94,12 @@ Style/GuardClause:
 Style/LineEndConcatenation:
   Enabled: false
 
-# Offense count: 53
+# Offense count: 69
 # Cop supports --auto-correct.
 Style/NumericLiterals:
   MinDigits: 11
 
-# Offense count: 44
+# Offense count: 41
 # Cop supports --auto-correct.
 Style/PerlBackrefs:
   Enabled: false
@@ -104,7 +109,7 @@ Style/PerlBackrefs:
 Style/PredicateName:
   Enabled: false
 
-# Offense count: 95
+# Offense count: 97
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 Style/RaiseArgs:
   Enabled: false
@@ -119,8 +124,8 @@ Style/RescueModifier:
 Style/StringLiteralsInInterpolation:
   Enabled: false
 
-# Offense count: 4
+# Offense count: 2
 # Cop supports --auto-correct.
-# Configuration parameters: ExactNameMatch, AllowPredicates, AllowDSLWriters, Whitelist.
+# Configuration parameters: ExactNameMatch, AllowPredicates, AllowDSLWriters, IgnoreClassMethods, Whitelist.
 Style/TrivialAccessors:
   Enabled: false

app/controllers/swf_controller.rb

@@ -45,9 +45,9 @@ class SwfController < ApplicationController
 
     if params["token"]
       user = User.authenticate(:token => params[:token])
-      sql = "SELECT gps_points.latitude*0.0000001 AS lat,gps_points.longitude*0.0000001 AS lon,gpx_files.id AS fileid," + 		       "      EXTRACT(EPOCH FROM gps_points.timestamp) AS ts, gps_points.trackid AS trackid " + 			   " FROM gpx_files,gps_points " + 			   "WHERE gpx_files.id=gpx_id " + 			   "  AND gpx_files.user_id=#{user.id} " + 			   "  AND " + OSM.sql_for_area(bbox, "gps_points.") + 			   "  AND (gps_points.timestamp IS NOT NULL) " + 			   "ORDER BY fileid DESC,ts " + 			   "LIMIT 10000 OFFSET #{start}"
+      sql = "SELECT gps_points.latitude*0.0000001 AS lat,gps_points.longitude*0.0000001 AS lon,gpx_files.id AS fileid," + "      EXTRACT(EPOCH FROM gps_points.timestamp) AS ts, gps_points.trackid AS trackid " + " FROM gpx_files,gps_points " + "WHERE gpx_files.id=gpx_id " + "  AND gpx_files.user_id=#{user.id} " + "  AND " + OSM.sql_for_area(bbox, "gps_points.") + "  AND (gps_points.timestamp IS NOT NULL) " + "ORDER BY fileid DESC,ts " + "LIMIT 10000 OFFSET #{start}"
     else
-      sql = "SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,gpx_id AS fileid," + 			     "      EXTRACT(EPOCH FROM timestamp) AS ts, gps_points.trackid AS trackid " + 				 " FROM gps_points " + 				 "WHERE " + OSM.sql_for_area(bbox, "gps_points.") + 				 "  AND (gps_points.timestamp IS NOT NULL) " + 				 "ORDER BY fileid DESC,ts " + 				 "LIMIT 10000 OFFSET #{start}"
+      sql = "SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,gpx_id AS fileid," + "      EXTRACT(EPOCH FROM timestamp) AS ts, gps_points.trackid AS trackid " + 				 " FROM gps_points " + 				 "WHERE " + OSM.sql_for_area(bbox, "gps_points.") + 				 "  AND (gps_points.timestamp IS NOT NULL) " + 				 "ORDER BY fileid DESC,ts " + 				 "LIMIT 10000 OFFSET #{start}"
     end
     gpslist = ActiveRecord::Base.connection.select_all sql
 

app/controllers/user_blocks_controller.rb

@@ -63,7 +63,7 @@ class UserBlocksController < ApplicationController
         :ends_at => Time.now.getutc + @block_period.hours,
         :reason => params[:user_block][:reason],
         :needs_view => params[:user_block][:needs_view]
-            )
+      )
         flash[:notice] = t("user_block.update.success")
         redirect_to(@user_block)
       else

app/controllers/user_controller.rb

@@ -572,7 +572,7 @@ class UserController < ApplicationController
   def openid_expand_url(openid_url)
     if openid_url.nil?
       return nil
-    elsif openid_url.match(/(.*)gmail.com(\/?)$/) || openid_url.match(/(.*)googlemail.com(\/?)$/)
+    elsif openid_url.match(%r{(.*)gmail.com(/?)$}) || openid_url.match(%r{(.*)googlemail.com(/?)$})
       # Special case gmail.com as it is potentially a popular OpenID
       # provider and, unlike yahoo.com, where it works automatically, Google
       # have hidden their OpenID endpoint somewhere obscure this making it

app/controllers/user_preference_controller.rb

@@ -16,7 +16,7 @@ class UserPreferenceController < ApplicationController
     el1 = XML::Node.new "preferences"
 
     prefs.each do |pref|
-      el1 <<  pref.to_xml_node
+      el1 << pref.to_xml_node
     end
 
     doc.root << el1

app/helpers/asset_helper.rb

@@ -3,7 +3,7 @@ module AssetHelper
     assets = {}
 
     Rails.application.assets.index.each_logical_path("#{directory}/*") do |path|
-      assets[path.sub(/^#{directory}\//, "")] = asset_path(path)
+      assets[path.sub(%r{^#{directory}/}, "")] = asset_path(path)
     end
 
     assets

app/helpers/browse_helper.rb

@@ -160,7 +160,7 @@ module BrowseHelper
 
   def telephone_link(_key, value)
     # does it look like a phone number? eg "+1 (234) 567-8901 " ?
-    return nil unless value =~ /^\s*\+[\d\s\(\)\/\.-]{6,25}\s*$/
+    return nil unless value =~ %r{^\s*\+[\d\s\(\)/\.-]{6,25}\s*$}
 
     # remove all whitespace instead of encoding it http://tools.ietf.org/html/rfc3966#section-5.1.1
     # "+1 (234) 567-8901 " -> "+1(234)567-8901"

app/helpers/note_helper.rb

@@ -4,13 +4,13 @@ module NoteHelper
       I18n.t("browse.note." + event + "_by_anonymous",
              :when => friendly_date(at),
              :exact_time => l(at)
-      ).html_safe
+            ).html_safe
     else
       I18n.t("browse.note." + event + "_by",
              :when => friendly_date(at),
              :exact_time => l(at),
              :user => note_author(by)
-      ).html_safe
+            ).html_safe
     end
   end
 

app/models/client_application.rb

@@ -51,7 +51,7 @@ class ClientApplication < ActiveRecord::Base
   end
 
   def access_token_for_user(user)
-    unless token = access_tokens.valid.where(:user_id => user).first
+    unless token = access_tokens.valid.find_by(:user_id => user)
       params = { :user => user }
 
       permissions.each do |p|

app/models/relation.rb

@@ -186,7 +186,7 @@ class Relation < ActiveRecord::Base
       self.lock!
       check_consistency(self, new_relation, user)
       # This will check to see if this relation is used by another relation
-      rel = RelationMember.joins(:relation).where("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id).first
+      rel = RelationMember.joins(:relation).find_by("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id)
       fail OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is used in relation #{rel.relation.id}.") unless rel.nil?
 
       self.changeset_id = new_relation.changeset_id
@@ -247,7 +247,7 @@ class Relation < ActiveRecord::Base
       # use reflection to look up the appropriate class
       model = Kernel.const_get(m[0].capitalize)
       # get the element with that ID
-      element = model.where(:id => m[1]).first
+      element = model.find_by(:id => m[1])
 
       # and check that it is OK to use.
       unless element && element.visible? && element.preconditions_ok?

app/models/tracetag.rb

@@ -4,5 +4,5 @@ class Tracetag < ActiveRecord::Base
   belongs_to :trace, :foreign_key => "gpx_id"
 
   validates :trace, :associated => true
-  validates :tag, :length => 1..255, :format => /\A[^\/;.,?]*\z/
+  validates :tag, :length => 1..255, :format => %r{\A[^/;.,?]*\z}
 end

app/models/user.rb

@@ -39,7 +39,7 @@ class User < ActiveRecord::Base
   validates :display_name, :if => proc { |u| u.display_name_changed? },
                            :uniqueness => { :case_sensitive => false }
   validates :display_name, :if => proc { |u| u.display_name_changed? },
-                           :format => { :with => /\A[^\x00-\x1f\x7f\ufffe\uffff\/;.,?%#]*\z/ }
+                           :format => { :with => %r{\A[^\x00-\x1f\x7f\ufffe\uffff/;.,?%#]*\z} }
   validates :display_name, :if => proc { |u| u.display_name_changed? },
                            :format => { :with => /\A\S/, :message => "has leading whitespace" }
   validates :display_name, :if => proc { |u| u.display_name_changed? },
@@ -51,7 +51,7 @@ class User < ActiveRecord::Base
   validates :home_lat, :home_lon, :allow_nil => true, :numericality => true
   validates :home_zoom, :allow_nil => true, :numericality => { :only_integer => true }
   validates :preferred_editor, :inclusion => Editors::ALL_EDITORS, :allow_nil => true
-  validates :image, :attachment_content_type => { :content_type => /\Aimage\/.*\Z/ }
+  validates :image, :attachment_content_type => { :content_type => %r{\Aimage/.*\Z} }
 
   validates_email_format_of :email, :if => proc { |u| u.email_changed? }
   validates_email_format_of :new_email, :allow_blank => true, :if => proc { |u| u.new_email_changed? }
@@ -62,7 +62,7 @@ class User < ActiveRecord::Base
 
   def self.authenticate(options)
     if options[:username] && options[:password]
-      user = where("email = ? OR display_name = ?", options[:username], options[:username]).first
+      user = find_by("email = ? OR display_name = ?", options[:username], options[:username])
 
       if user.nil?
         users = where("LOWER(email) = LOWER(?) OR LOWER(display_name) = LOWER(?)", options[:username], options[:username])

config/initializers/field_error.rb

@@ -4,6 +4,6 @@ ActionView::Base.field_error_proc = proc do |html_tag, _instance|
   if class_attr_index
     html_tag.insert class_attr_index + 7, "field_with_errors "
   else
-    html_tag.insert html_tag.index(/\/?>/), ' class="field_with_errors"'
+    html_tag.insert html_tag.index(%r{/?>}), ' class="field_with_errors"'
   end
 end

db/migrate/009_way_nodes_node_idx.rb

@@ -1,4 +1,4 @@
-class WayNodesNodeIdx  < ActiveRecord::Migration
+class WayNodesNodeIdx < ActiveRecord::Migration
   def self.up
     add_index "way_nodes", ["node_id"], :name => "way_nodes_node_idx"
   end

lib/bounding_box.rb

@@ -3,8 +3,8 @@ class BoundingBox
 
   LON_LIMIT = 180.0
   LAT_LIMIT = 90.0
-  SCALED_LON_LIMIT = LON_LIMIT *  GeoRecord::SCALE
+  SCALED_LON_LIMIT = LON_LIMIT * GeoRecord::SCALE
-  SCALED_LAT_LIMIT = LAT_LIMIT *  GeoRecord::SCALE
+  SCALED_LAT_LIMIT = LAT_LIMIT * GeoRecord::SCALE
 
   public
 

lib/classic_pagination/pagination.rb

@@ -98,7 +98,7 @@ module ActionController
               unknown_option_keys.empty?
 
       options[:singular_name] ||= ActiveSupport::Inflector.singularize(collection_id.to_s)
-      options[:class_name]  ||= ActiveSupport::Inflector.camelize(options[:singular_name])
+      options[:class_name] ||= ActiveSupport::Inflector.camelize(options[:singular_name])
     end
 
     # Returns a paginator and a collection of Active Record model instances

lib/potlatch.rb

@@ -183,7 +183,7 @@ module Potlatch
       File.open("#{Rails.root}/config/potlatch/presets.txt") do |file|
         file.each_line do|line|
           t = line.chomp
-          if t =~ /(\w+)\/(\w+)/
+          if t =~ %r{(\w+)/(\w+)}
             presettype = $1
             presetcategory = $2
             presetmenus[presettype].push(presetcategory)
@@ -246,7 +246,7 @@ module Potlatch
       autotags = { "point" => {}, "way" => {}, "POI" => {} }
       File.open("#{Rails.root}/config/potlatch/autocomplete.txt") do |file|
         file.each_line do|line|
-          next unless line.chomp =~ /^([\w:]+)\/(\w+)\s+(.+)$/
+          next unless line.chomp =~ %r{^([\w:]+)/(\w+)\s+(.+)$}
 
           tag = $1
           type = $2

lib/short_link.rb

@@ -55,7 +55,7 @@ module ShortLink
   # given a location and zoom, return a short string representing it.
   def self.encode(lon, lat, z)
     code = interleave_bits(((lon + 180.0) * 2**32 / 360.0).to_i,
-                           ((lat +  90.0) * 2**32 / 180.0).to_i)
+                           ((lat + 90.0) * 2**32 / 180.0).to_i)
     str = ""
     # add eight to the zoom level, which approximates an accuracy of
     # one pixel in a tile.

test/controllers/changeset_controller_test.rb

@@ -582,9 +582,9 @@ EOF
     # check that the changeset bbox is within bounds
     cs = Changeset.find(changeset_id)
     assert cs.min_lon >= -180 * GeoRecord::SCALE, "Minimum longitude (#{cs.min_lon / GeoRecord::SCALE}) should be >= -180 to be valid."
-    assert cs.max_lon <=  180 * GeoRecord::SCALE, "Maximum longitude (#{cs.max_lon / GeoRecord::SCALE}) should be <= 180 to be valid."
+    assert cs.max_lon <= 180 * GeoRecord::SCALE, "Maximum longitude (#{cs.max_lon / GeoRecord::SCALE}) should be <= 180 to be valid."
-    assert cs.min_lat >=  -90 * GeoRecord::SCALE, "Minimum latitude (#{cs.min_lat / GeoRecord::SCALE}) should be >= -90 to be valid."
+    assert cs.min_lat >= -90 * GeoRecord::SCALE, "Minimum latitude (#{cs.min_lat / GeoRecord::SCALE}) should be >= -90 to be valid."
-    assert cs.max_lat >=   90 * GeoRecord::SCALE, "Maximum latitude (#{cs.max_lat / GeoRecord::SCALE}) should be <= 90 to be valid."
+    assert cs.max_lat >= 90 * GeoRecord::SCALE, "Maximum latitude (#{cs.max_lat / GeoRecord::SCALE}) should be <= 90 to be valid."
   end
 
   ##

test/controllers/old_node_controller_test.rb

@@ -51,7 +51,7 @@ class OldNodeControllerTest < ActionController::TestCase
     # randomly move the node about
     20.times do
       # move the node somewhere else
-      xml_node["lat"] = precision(rand * 180 -  90).to_s
+      xml_node["lat"] = precision(rand * 180 - 90).to_s
       xml_node["lon"] = precision(rand * 360 - 180).to_s
       with_controller(NodeController.new) do
         content xml_doc
@@ -100,7 +100,7 @@ class OldNodeControllerTest < ActionController::TestCase
     # randomly move the node about
     20.times do
       # move the node somewhere else
-      xml_node["lat"] = precision(rand * 180 -  90).to_s
+      xml_node["lat"] = precision(rand * 180 - 90).to_s
       xml_node["lon"] = precision(rand * 360 - 180).to_s
       with_controller(NodeController.new) do
         content xml_doc

test/controllers/relation_controller_test.rb

@@ -177,7 +177,7 @@ class RelationControllerTest < ActionController::TestCase
     # need a role attribute to be included
     nid = current_nodes(:used_node_1).id
     content "<osm><relation changeset='#{changeset_id}'>" +
-      "<member  ref='#{nid}' type='node'/>" +       "<tag k='test' v='yes' /></relation></osm>"
+      "<member  ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for forbidden due to user
     assert_response :forbidden,
@@ -265,7 +265,7 @@ class RelationControllerTest < ActionController::TestCase
     # need a role attribute to be included
     nid = current_nodes(:used_node_1).id
     content "<osm><relation changeset='#{changeset_id}'>" +
-      "<member  ref='#{nid}' type='node'/>" +       "<tag k='test' v='yes' /></relation></osm>"
+      "<member  ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for success
     assert_response :success,

test/helpers/application_helper_test.rb

@@ -168,19 +168,19 @@ class ApplicationHelperTest < ActionView::TestCase
 
   def test_friendly_date
     date = friendly_date(Time.new(2014, 3, 5, 18, 58, 23))
-    assert_match /^<span title=" *5 March 2014 at 18:58">.*<\/span>$/, date
+    assert_match %r{^<span title=" *5 March 2014 at 18:58">.*</span>$}, date
 
     date = friendly_date(Time.now - 1.hour)
-    assert_match /^<span title=".*">about 1 hour<\/span>$/, date
+    assert_match %r{^<span title=".*">about 1 hour</span>$}, date
 
     date = friendly_date(Time.now - 2.days)
-    assert_match /^<span title=".*">2 days<\/span>$/, date
+    assert_match %r{^<span title=".*">2 days</span>$}, date
 
     date = friendly_date(Time.now - 3.weeks)
-    assert_match /^<span title=".*">21 days<\/span>$/, date
+    assert_match %r{^<span title=".*">21 days</span>$}, date
 
     date = friendly_date(Time.now - 4.months)
-    assert_match /^<span title=".*">4 months<\/span>$/, date
+    assert_match %r{^<span title=".*">4 months</span>$}, date
   end
 
   def test_body_class

test/helpers/changeset_helper_test.rb

@@ -9,7 +9,7 @@ class ChangesetHelperTest < ActionView::TestCase
   end
 
   def test_changeset_details
-    assert_match /^Created <abbr title='Mon, 01 Jan 2007 00:00:00 \+0000'>.*<\/abbr> by anonymous$/, changeset_details(changesets(:normal_user_first_change))
+    assert_match %r{^Created <abbr title='Mon, 01 Jan 2007 00:00:00 \+0000'>.*</abbr> by anonymous$}, changeset_details(changesets(:normal_user_first_change))
     assert_match %r{^Closed <abbr title='Created: Mon, 01 Jan 2007 00:00:00 \+0000&#10;Closed: Tue, 02 Jan 2007 00:00:00 \+0000'>.*</abbr> by <a href="/user/test2">test2</a>$}, changeset_details(changesets(:public_user_closed_change))
   end
 end

test/integration/redirect_test.rb

@@ -1,6 +1,6 @@
 require "test_helper"
 
-class RedirectTest  < ActionDispatch::IntegrationTest
+class RedirectTest < ActionDispatch::IntegrationTest
   def test_legacy_redirects
     get "/index.html"
     assert_response :redirect

test/models/changeset_test.rb

@@ -24,11 +24,11 @@ class ChangesetTest < ActiveSupport::TestCase
     message_create = assert_raise(OSM::APIBadXMLError) do
       Changeset.from_xml(nocs, true)
     end
-    assert_match /XML doesn't contain an osm\/changeset element/, message_create.message
+    assert_match %r{XML doesn't contain an osm/changeset element}, message_create.message
     message_update = assert_raise(OSM::APIBadXMLError) do
       Changeset.from_xml(nocs, false)
     end
-    assert_match /XML doesn't contain an osm\/changeset element/, message_update.message
+    assert_match %r{XML doesn't contain an osm/changeset element}, message_update.message
   end
 
   def test_from_xml_no_k_v

test/models/node_test.rb

@@ -271,11 +271,11 @@ class NodeTest < ActiveSupport::TestCase
     message_create = assert_raise(OSM::APIBadXMLError) do
       Node.from_xml(no_node, true)
     end
-    assert_match /XML doesn't contain an osm\/node element/, message_create.message
+    assert_match %r{XML doesn't contain an osm/node element}, message_create.message
     message_update = assert_raise(OSM::APIBadXMLError) do
       Node.from_xml(no_node, false)
     end
-    assert_match /XML doesn't contain an osm\/node element/, message_update.message
+    assert_match %r{XML doesn't contain an osm/node element}, message_update.message
   end
 
   def test_from_xml_no_k_v