Fix rubocop warnings

.rubocop.yml

@@ -27,15 +27,34 @@ Rails:
 Layout/ExtraSpacing:
   AllowForAlignment: true
 
-Style/BracesAroundHashParameters:
+Lint/PercentStringArray:
-  EnforcedStyle: context_dependent
+  Exclude:
+    - 'config/initializers/secure_headers.rb'
+    - 'app/controllers/site_controller.rb'
 
-Style/FileName:
+Naming/FileName:
   Exclude:
     - 'script/deliver-message'
     - 'script/locale/reload-languages'
     - 'script/update-spam-blocks'
 
+Rails/ApplicationRecord:
+  Enabled: false
+
+Rails/HasManyOrHasOneDependent:
+  Enabled: false
+
+Rails/HttpPositionalArguments:
+  Enabled: false
+
+Rails/SkipsModelValidations:
+  Exclude:
+    - 'db/migrate/*.rb'
+    - 'app/controllers/user_controller.rb'
+
+Style/BracesAroundHashParameters:
+  EnforcedStyle: context_dependent
+
 Style/FormatStringToken:
   EnforcedStyle: template
 
@@ -60,19 +79,3 @@ Style/StringLiterals:
 
 Style/SymbolArray:
   EnforcedStyle: brackets
-
-Rails/ApplicationRecord:
-  Enabled: false
-
-Rails/HttpPositionalArguments:
-  Enabled: false
-
-Rails/SkipsModelValidations:
-  Exclude:
-    - 'db/migrate/*.rb'
-    - 'app/controllers/user_controller.rb'
-
-Lint/PercentStringArray:
-  Exclude:
-    - 'config/initializers/secure_headers.rb'
-    - 'app/controllers/site_controller.rb'

.rubocop_todo.yml

@@ -1,11 +1,19 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2016-10-20 21:45:27 +0100 using RuboCop version 0.44.1.
+# on 2017-10-05 10:04:24 +0100 using RuboCop version 0.50.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: 1
+# Cop supports --auto-correct.
+# Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth.
+# SupportedStyles: aligned, indented
+Layout/MultilineOperationIndentation:
+  Exclude:
+    - 'lib/bounding_box.rb'
+
 # Offense count: 34
 Lint/AmbiguousOperator:
   Exclude:
@@ -14,11 +22,11 @@ Lint/AmbiguousOperator:
     - 'test/lib/bounding_box_test.rb'
     - 'test/lib/country_test.rb'
 
-# Offense count: 117
+# Offense count: 124
 Lint/AmbiguousRegexpLiteral:
   Enabled: false
 
-# Offense count: 30
+# Offense count: 32
 # Configuration parameters: AllowSafeAssignment.
 Lint/AssignmentInCondition:
   Exclude:
@@ -36,52 +44,62 @@ Lint/AssignmentInCondition:
     - 'lib/osm.rb'
     - 'script/deliver-message'
 
-# Offense count: 5
+# Offense count: 4
 Lint/HandleExceptions:
   Exclude:
     - 'app/controllers/amf_controller.rb'
     - 'app/controllers/user_controller.rb'
-    - 'config/initializers/session.rb'
+
+# Offense count: 3
+Lint/InterpolationCheck:
+  Exclude:
+    - 'test/controllers/node_controller_test.rb'
+
+# Offense count: 2
+Lint/RescueWithoutErrorClass:
+  Exclude:
+    - 'app/helpers/browse_helper.rb'
 
 # Offense count: 2
 Lint/ShadowingOuterLocalVariable:
   Exclude:
     - 'app/views/changeset/list.atom.builder'
 
-# Offense count: 630
+# Offense count: 666
 Metrics/AbcSize:
   Max: 280
 
-# Offense count: 35
+# Offense count: 41
-# Configuration parameters: CountComments.
+# Configuration parameters: CountComments, ExcludedMethods.
 Metrics/BlockLength:
-  Max: 295
+  Max: 240
 
 # Offense count: 12
+# Configuration parameters: CountBlocks.
 Metrics/BlockNesting:
   Max: 5
 
-# Offense count: 62
+# Offense count: 63
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
   Max: 1790
 
-# Offense count: 69
+# Offense count: 71
 Metrics/CyclomaticComplexity:
   Max: 20
 
-# Offense count: 2826
+# Offense count: 3004
-# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives.
+# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
 Metrics/LineLength:
-  Max: 1072
+  Max: 1073
 
-# Offense count: 612
+# Offense count: 675
 # Configuration parameters: CountComments.
 Metrics/MethodLength:
   Max: 179
 
-# Offense count: 1
+# Offense count: 2
 # Configuration parameters: CountComments.
 Metrics/ModuleLength:
   Max: 147
@@ -95,6 +113,30 @@ Metrics/ParameterLists:
 Metrics/PerceivedComplexity:
   Max: 23
 
+# Offense count: 5
+Naming/AccessorMethodName:
+  Exclude:
+    - 'app/controllers/application_controller.rb'
+    - 'app/helpers/title_helper.rb'
+    - 'app/models/old_way.rb'
+    - 'lib/osm.rb'
+    - 'lib/potlatch.rb'
+
+# Offense count: 8
+# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist.
+# NamePrefix: is_, has_, have_
+# NamePrefixBlacklist: is_, has_, have_
+# NameWhitelist: is_a?
+Naming/PredicateName:
+  Exclude:
+    - 'spec/**/*'
+    - 'app/models/changeset.rb'
+    - 'app/models/old_node.rb'
+    - 'app/models/old_relation.rb'
+    - 'app/models/old_way.rb'
+    - 'app/models/user.rb'
+    - 'lib/classic_pagination/pagination.rb'
+
 # Offense count: 2
 # Configuration parameters: Include.
 # Include: app/**/*.rb, config/**/*.rb, lib/**/*.rb
@@ -122,7 +164,7 @@ Rails/NotNullColumn:
     - 'db/migrate/025_add_end_time_to_changesets.rb'
     - 'db/migrate/20120404205604_add_user_and_description_to_redaction.rb'
 
-# Offense count: 17
+# Offense count: 20
 Rails/OutputSafety:
   Exclude:
     - 'app/controllers/user_controller.rb'
@@ -136,27 +178,18 @@ Rails/OutputSafety:
     - 'lib/rich_text.rb'
     - 'test/helpers/application_helper_test.rb'
 
-# Offense count: 74
+# Offense count: 86
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: strict, flexible
 Rails/TimeZone:
   Enabled: false
 
-# Offense count: 5
-Style/AccessorMethodName:
-  Exclude:
-    - 'app/controllers/application_controller.rb'
-    - 'app/helpers/title_helper.rb'
-    - 'app/models/old_way.rb'
-    - 'lib/osm.rb'
-    - 'lib/potlatch.rb'
-
 # Offense count: 1
 Style/AsciiComments:
   Exclude:
     - 'test/models/message_test.rb'
 
-# Offense count: 220
+# Offense count: 219
 Style/Documentation:
   Enabled: false
 
@@ -182,27 +215,13 @@ Style/LineEndConcatenation:
     - 'test/controllers/relation_controller_test.rb'
     - 'test/controllers/way_controller_test.rb'
 
-# Offense count: 71
+# Offense count: 75
 # Cop supports --auto-correct.
+# Configuration parameters: Strict.
 Style/NumericLiterals:
   MinDigits: 11
 
-# Offense count: 8
+# Offense count: 95
-# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist.
-# NamePrefix: is_, has_, have_
-# NamePrefixBlacklist: is_, has_, have_
-# NameWhitelist: is_a?
-Style/PredicateName:
-  Exclude:
-    - 'spec/**/*'
-    - 'app/models/changeset.rb'
-    - 'app/models/old_node.rb'
-    - 'app/models/old_relation.rb'
-    - 'app/models/old_way.rb'
-    - 'app/models/user.rb'
-    - 'lib/classic_pagination/pagination.rb'
-
-# Offense count: 97
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: compact, exploded

app/controllers/amf_controller.rb

@@ -151,7 +151,7 @@ class AmfController < ApplicationController
         cs = Changeset.find(closeid.to_i)
         cs.set_closed_time_now
         if cs.user_id != user.id
-          raise OSM::APIUserChangesetMismatchError.new
+          raise OSM::APIUserChangesetMismatchError
         elsif closecomment.empty?
           cs.save!
         else
@@ -229,7 +229,7 @@ class AmfController < ApplicationController
       begin
         other = YAML.safe_load(File.open(Rails.root.join("config", "potlatch", "locales", "#{lang}.yml")))[lang]
         loaded_lang = lang
-      rescue
+      rescue StandardError
         other = en
       end
 
@@ -907,7 +907,7 @@ class AmfController < ApplicationController
   # Alternative SQL queries for getway/whichways
 
   def sql_find_ways_in_area(bbox)
-    sql = <<-EOF
+    sql = <<-SQL
     SELECT DISTINCT current_ways.id AS wayid,current_ways.version AS version
       FROM current_way_nodes
     INNER JOIN current_nodes ON current_nodes.id=current_way_nodes.node_id
@@ -915,20 +915,20 @@ class AmfController < ApplicationController
        WHERE current_nodes.visible=TRUE
        AND current_ways.visible=TRUE
        AND #{OSM.sql_for_area(bbox, 'current_nodes.')}
-    EOF
+    SQL
     ActiveRecord::Base.connection.select_all(sql).collect { |a| [a["wayid"].to_i, a["version"].to_i] }
   end
 
   def sql_find_pois_in_area(bbox)
     pois = []
-    sql = <<-EOF
+    sql = <<-SQL
       SELECT current_nodes.id,current_nodes.latitude*0.0000001 AS lat,current_nodes.longitude*0.0000001 AS lon,current_nodes.version
       FROM current_nodes
        LEFT OUTER JOIN current_way_nodes cwn ON cwn.node_id=current_nodes.id
        WHERE current_nodes.visible=TRUE
        AND cwn.id IS NULL
        AND #{OSM.sql_for_area(bbox, 'current_nodes.')}
-    EOF
+    SQL
     ActiveRecord::Base.connection.select_all(sql).each do |row|
       poitags = {}
       ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_node_tags WHERE id=#{row['id']}").each do |n|
@@ -942,36 +942,36 @@ class AmfController < ApplicationController
   def sql_find_relations_in_area_and_ways(bbox, way_ids)
     # ** It would be more Potlatchy to get relations for nodes within ways
     #    during 'getway', not here
-    sql = <<-EOF
+    sql = <<-SQL
       SELECT DISTINCT cr.id AS relid,cr.version AS version
       FROM current_relations cr
       INNER JOIN current_relation_members crm ON crm.id=cr.id
       INNER JOIN current_nodes cn ON crm.member_id=cn.id AND crm.member_type='Node'
        WHERE #{OSM.sql_for_area(bbox, 'cn.')}
-      EOF
+      SQL
     unless way_ids.empty?
-      sql += <<-EOF
+      sql += <<-SQL
        UNION
         SELECT DISTINCT cr.id AS relid,cr.version AS version
         FROM current_relations cr
         INNER JOIN current_relation_members crm ON crm.id=cr.id
          WHERE crm.member_type='Way'
          AND crm.member_id IN (#{way_ids.join(',')})
-        EOF
+        SQL
     end
     ActiveRecord::Base.connection.select_all(sql).collect { |a| [a["relid"].to_i, a["version"].to_i] }
   end
 
   def sql_get_nodes_in_way(wayid)
     points = []
-    sql = <<-EOF
+    sql = <<-SQL
       SELECT latitude*0.0000001 AS lat,longitude*0.0000001 AS lon,current_nodes.id,current_nodes.version
       FROM current_way_nodes,current_nodes
        WHERE current_way_nodes.id=#{wayid.to_i}
        AND current_way_nodes.node_id=current_nodes.id
        AND current_nodes.visible=TRUE
       ORDER BY sequence_id
-    EOF
+    SQL
     ActiveRecord::Base.connection.select_all(sql).each do |row|
       nodetags = {}
       ActiveRecord::Base.connection.select_all("SELECT k,v FROM current_node_tags WHERE id=#{row['id']}").each do |n|

app/controllers/application_controller.rb

@@ -346,7 +346,7 @@ class ApplicationController < ActionController::Base
   # or raises a suitable error. +method+ should be a symbol, e.g: :put or :get.
   def assert_method(method)
     ok = request.send((method.to_s.downcase + "?").to_sym)
-    raise OSM::APIBadMethodError.new(method) unless ok
+    raise OSM::APIBadMethodError, method unless ok
   end
 
   ##

app/controllers/changeset_controller.rb

@@ -310,8 +310,8 @@ class ChangesetController < ApplicationController
   # Add a comment to a changeset
   def comment
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput, "No id was given" unless params[:id]
-    raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
+    raise OSM::APIBadUserInput, "No text was given" if params[:text].blank?
 
     # Extract the arguments
     id = params[:id].to_i
@@ -319,7 +319,7 @@ class ChangesetController < ApplicationController
 
     # Find the changeset and check it is valid
     changeset = Changeset.find(id)
-    raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
+    raise OSM::APIChangesetNotYetClosedError, changeset if changeset.is_open?
 
     # Add a comment to the changeset
     comment = changeset.comments.create(:changeset => changeset,
@@ -344,15 +344,15 @@ class ChangesetController < ApplicationController
   # Adds a subscriber to the changeset
   def subscribe
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput, "No id was given" unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
 
     # Find the changeset and check it is valid
     changeset = Changeset.find(id)
-    raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
+    raise OSM::APIChangesetNotYetClosedError, changeset if changeset.is_open?
-    raise OSM::APIChangesetAlreadySubscribedError.new(changeset) if changeset.subscribers.exists?(current_user.id)
+    raise OSM::APIChangesetAlreadySubscribedError, changeset if changeset.subscribers.exists?(current_user.id)
 
     # Add the subscriber
     changeset.subscribers << current_user
@@ -365,15 +365,15 @@ class ChangesetController < ApplicationController
   # Removes a subscriber from the changeset
   def unsubscribe
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput, "No id was given" unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
 
     # Find the changeset and check it is valid
     changeset = Changeset.find(id)
-    raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
+    raise OSM::APIChangesetNotYetClosedError, changeset if changeset.is_open?
-    raise OSM::APIChangesetNotSubscribedError.new(changeset) unless changeset.subscribers.exists?(current_user.id)
+    raise OSM::APIChangesetNotSubscribedError, changeset unless changeset.subscribers.exists?(current_user.id)
 
     # Remove the subscriber
     changeset.subscribers.delete(current_user)
@@ -386,7 +386,7 @@ class ChangesetController < ApplicationController
   # Sets visible flag on comment to false
   def hide_comment
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput, "No id was given" unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
@@ -405,7 +405,7 @@ class ChangesetController < ApplicationController
   # Sets visible flag on comment to true
   def unhide_comment
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput, "No id was given" unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
@@ -434,7 +434,7 @@ class ChangesetController < ApplicationController
       @comments = changeset.comments.includes(:author, :changeset).limit(comments_limit)
     else
       # Return comments
-      @comments = ChangesetComment.includes(:author, :changeset).where(:visible => :true).order("created_at DESC").limit(comments_limit).preload(:changeset)
+      @comments = ChangesetComment.includes(:author, :changeset).where(:visible => true).order("created_at DESC").limit(comments_limit).preload(:changeset)
     end
 
     # Render the result
@@ -475,19 +475,19 @@ class ChangesetController < ApplicationController
       changesets
     else
       # shouldn't provide both name and UID
-      raise OSM::APIBadUserInput.new("provide either the user ID or display name, but not both") if user && name
+      raise OSM::APIBadUserInput, "provide either the user ID or display name, but not both" if user && name
 
       # use either the name or the UID to find the user which we're selecting on.
       u = if name.nil?
             # user input checking, we don't have any UIDs < 1
-            raise OSM::APIBadUserInput.new("invalid user ID") if user.to_i < 1
+            raise OSM::APIBadUserInput, "invalid user ID" if user.to_i < 1
             u = User.find(user.to_i)
           else
             u = User.find_by(:display_name => name)
           end
 
       # make sure we found a user
-      raise OSM::APINotFoundError.new if u.nil?
+      raise OSM::APINotFoundError if u.nil?
 
       # should be able to get changesets of public users only, or
       # our own changesets regardless of public-ness.
@@ -514,7 +514,7 @@ class ChangesetController < ApplicationController
 
       # check that we actually have 2 elements in the array
       times = time.split(/,/)
-      raise OSM::APIBadUserInput.new("bad time range") if times.size != 2
+      raise OSM::APIBadUserInput, "bad time range" if times.size != 2
 
       from, to = times.collect { |t| DateTime.parse(t) }
       return changesets.where("closed_at >= ? and created_at <= ?", from, to)
@@ -525,9 +525,9 @@ class ChangesetController < ApplicationController
     # stupid DateTime seems to throw both of these for bad parsing, so
     # we have to catch both and ensure the correct code path is taken.
   rescue ArgumentError => ex
-    raise OSM::APIBadUserInput.new(ex.message.to_s)
+    raise OSM::APIBadUserInput, ex.message.to_s
   rescue RuntimeError => ex
-    raise OSM::APIBadUserInput.new(ex.message.to_s)
+    raise OSM::APIBadUserInput, ex.message.to_s
   end
 
   ##
@@ -563,7 +563,7 @@ class ChangesetController < ApplicationController
     if ids.nil?
       changesets
     elsif ids.empty?
-      raise OSM::APIBadUserInput.new("No changesets were given to search for")
+      raise OSM::APIBadUserInput, "No changesets were given to search for"
     else
       ids = ids.split(",").collect(&:to_i)
       changesets.where(:id => ids)
@@ -584,7 +584,7 @@ class ChangesetController < ApplicationController
       if params[:limit].to_i > 0 && params[:limit].to_i <= 10000
         params[:limit].to_i
       else
-        raise OSM::APIBadUserInput.new("Comments limit must be between 1 and 10000")
+        raise OSM::APIBadUserInput, "Comments limit must be between 1 and 10000"
       end
     else
       100

app/controllers/geocoder_controller.rb

@@ -1,5 +1,3 @@
-# coding: utf-8
-
 class GeocoderController < ApplicationController
   require "cgi"
   require "uri"
@@ -287,7 +285,7 @@ class GeocoderController < ApplicationController
   end
 
   def escape_query(query)
-    URI.escape(query, Regexp.new("[^#{URI::PATTERN::UNRESERVED}]", false, "N"))
+    CGI.escape(query)
   end
 
   def normalize_params
@@ -322,7 +320,7 @@ class GeocoderController < ApplicationController
       Float(captures[0])
       lat = !captures[2].casecmp("s").zero? ? captures[0].to_f : -captures[0].to_f
       lon = !captures[5].casecmp("w").zero? ? captures[3].to_f : -captures[3].to_f
-    rescue
+    rescue StandardError
       lat = !captures[0].casecmp("s").zero? ? captures[1].to_f : -captures[1].to_f
       lon = !captures[3].casecmp("w").zero? ? captures[4].to_f : -captures[4].to_f
     end
@@ -334,7 +332,7 @@ class GeocoderController < ApplicationController
       Float(captures[0])
       lat = !captures[3].casecmp("s").zero? ? captures[0].to_f + captures[1].to_f / 60 : -(captures[0].to_f + captures[1].to_f / 60)
       lon = !captures[7].casecmp("w").zero? ? captures[4].to_f + captures[5].to_f / 60 : -(captures[4].to_f + captures[5].to_f / 60)
-    rescue
+    rescue StandardError
       lat = !captures[0].casecmp("s").zero? ? captures[1].to_f + captures[2].to_f / 60 : -(captures[1].to_f + captures[2].to_f / 60)
       lon = !captures[4].casecmp("w").zero? ? captures[5].to_f + captures[6].to_f / 60 : -(captures[5].to_f + captures[6].to_f / 60)
     end
@@ -346,7 +344,7 @@ class GeocoderController < ApplicationController
       Float(captures[0])
       lat = !captures[4].casecmp("s").zero? ? captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60 : -(captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60)
       lon = !captures[9].casecmp("w").zero? ? captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60 : -(captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60)
-    rescue
+    rescue StandardError
       lat = !captures[0].casecmp("s").zero? ? captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60 : -(captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60)
       lon = !captures[5].casecmp("w").zero? ? captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60 : -(captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60)
     end

app/controllers/node_controller.rb

@@ -41,7 +41,7 @@ class NodeController < ApplicationController
     new_node = Node.from_xml(request.raw_post)
 
     unless new_node && new_node.id == node.id
-      raise OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})")
+      raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})"
     end
 
     node.update_from(new_node, current_user)
@@ -56,7 +56,7 @@ class NodeController < ApplicationController
     new_node = Node.from_xml(request.raw_post)
 
     unless new_node && new_node.id == node.id
-      raise OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})")
+      raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})"
     end
     node.delete_with_history!(new_node, current_user)
     render :plain => node.version.to_s
@@ -65,13 +65,13 @@ class NodeController < ApplicationController
   # Dump the details on many nodes whose ids are given in the "nodes" parameter.
   def nodes
     unless params["nodes"]
-      raise OSM::APIBadUserInput.new("The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]")
+      raise OSM::APIBadUserInput, "The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]"
     end
 
     ids = params["nodes"].split(",").collect(&:to_i)
 
     if ids.empty?
-      raise OSM::APIBadUserInput.new("No nodes were given to search for")
+      raise OSM::APIBadUserInput, "No nodes were given to search for"
     end
     doc = OSM::API.new.get_xml_doc
 

app/controllers/notes_controller.rb

@@ -20,10 +20,10 @@ class NotesController < ApplicationController
     if params[:bbox]
       bbox = BoundingBox.from_bbox_params(params)
     else
-      raise OSM::APIBadUserInput.new("No l was given") unless params[:l]
+      raise OSM::APIBadUserInput, "No l was given" unless params[:l]
-      raise OSM::APIBadUserInput.new("No r was given") unless params[:r]
+      raise OSM::APIBadUserInput, "No r was given" unless params[:r]
-      raise OSM::APIBadUserInput.new("No b was given") unless params[:b]
+      raise OSM::APIBadUserInput, "No b was given" unless params[:b]
-      raise OSM::APIBadUserInput.new("No t was given") unless params[:t]
+      raise OSM::APIBadUserInput, "No t was given" unless params[:t]
 
       bbox = BoundingBox.from_lrbt_params(params)
     end
@@ -56,9 +56,9 @@ class NotesController < ApplicationController
     raise OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip)
 
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No lat was given") unless params[:lat]
+    raise OSM::APIBadUserInput, "No lat was given" unless params[:lat]
-    raise OSM::APIBadUserInput.new("No lon was given") unless params[:lon]
+    raise OSM::APIBadUserInput, "No lon was given" unless params[:lon]
-    raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
+    raise OSM::APIBadUserInput, "No text was given" if params[:text].blank?
 
     # Extract the arguments
     lon = OSM.parse_float(params[:lon], OSM::APIBadUserInput, "lon was not a number")
@@ -69,7 +69,7 @@ class NotesController < ApplicationController
     Note.transaction do
       # Create the note
       @note = Note.create(:lat => lat, :lon => lon)
-      raise OSM::APIBadUserInput.new("The note is outside this world") unless @note.in_world?
+      raise OSM::APIBadUserInput, "The note is outside this world" unless @note.in_world?
 
       # Save the note
       @note.save!
@@ -92,8 +92,8 @@ class NotesController < ApplicationController
     raise OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip)
 
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput, "No id was given" unless params[:id]
-    raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
+    raise OSM::APIBadUserInput, "No text was given" if params[:text].blank?
 
     # Extract the arguments
     id = params[:id].to_i
@@ -103,7 +103,7 @@ class NotesController < ApplicationController
     @note = Note.find(id)
     raise OSM::APINotFoundError unless @note
     raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
-    raise OSM::APINoteAlreadyClosedError.new(@note) if @note.closed?
+    raise OSM::APINoteAlreadyClosedError, @note if @note.closed?
 
     # Add a comment to the note
     Note.transaction do
@@ -121,7 +121,7 @@ class NotesController < ApplicationController
   # Close a note
   def close
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput, "No id was given" unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
@@ -131,7 +131,7 @@ class NotesController < ApplicationController
     @note = Note.find_by(:id => id)
     raise OSM::APINotFoundError unless @note
     raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
-    raise OSM::APINoteAlreadyClosedError.new(@note) if @note.closed?
+    raise OSM::APINoteAlreadyClosedError, @note if @note.closed?
 
     # Close the note and add a comment
     Note.transaction do
@@ -151,7 +151,7 @@ class NotesController < ApplicationController
   # Reopen a note
   def reopen
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput, "No id was given" unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
@@ -161,7 +161,7 @@ class NotesController < ApplicationController
     @note = Note.find_by(:id => id)
     raise OSM::APINotFoundError unless @note
     raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? || current_user.moderator?
-    raise OSM::APINoteAlreadyOpenError.new(@note) unless @note.closed? || !@note.visible?
+    raise OSM::APINoteAlreadyOpenError, @note unless @note.closed? || !@note.visible?
 
     # Reopen the note and add a comment
     Note.transaction do
@@ -206,7 +206,7 @@ class NotesController < ApplicationController
   # Read a note
   def show
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput, "No id was given" unless params[:id]
 
     # Find the note and check it is valid
     @note = Note.find(params[:id])
@@ -226,7 +226,7 @@ class NotesController < ApplicationController
   # Delete (hide) a note
   def destroy
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput, "No id was given" unless params[:id]
 
     # Extract the arguments
     id = params[:id].to_i
@@ -256,7 +256,7 @@ class NotesController < ApplicationController
   # Return a list of notes matching a given string
   def search
     # Check the arguments are sane
-    raise OSM::APIBadUserInput.new("No query string was given") unless params[:q]
+    raise OSM::APIBadUserInput, "No query string was given" unless params[:q]
 
     # Get any conditions that need to be applied
     @notes = closed_condition(Note.all)
@@ -310,7 +310,7 @@ class NotesController < ApplicationController
       if params[:limit].to_i > 0 && params[:limit].to_i <= 10000
         params[:limit].to_i
       else
-        raise OSM::APIBadUserInput.new("Note limit must be between 1 and 10000")
+        raise OSM::APIBadUserInput, "Note limit must be between 1 and 10000"
       end
     else
       100

app/controllers/old_controller.rb

@@ -19,7 +19,7 @@ class OldController < ApplicationController
     # the .where() method used in the lookup_old_element_versions
     # call won't throw an error if no records are found, so we have
     # to do that ourselves.
-    raise OSM::APINotFoundError.new if @elements.empty?
+    raise OSM::APINotFoundError if @elements.empty?
 
     doc = OSM::API.new.get_xml_doc
 

app/controllers/relation_controller.rb

@@ -36,7 +36,7 @@ class RelationController < ApplicationController
     new_relation = Relation.from_xml(request.raw_post)
 
     unless new_relation && new_relation.id == relation.id
-      raise OSM::APIBadUserInput.new("The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})")
+      raise OSM::APIBadUserInput, "The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})"
     end
 
     relation.update_from new_relation, current_user
@@ -128,13 +128,13 @@ class RelationController < ApplicationController
 
   def relations
     unless params["relations"]
-      raise OSM::APIBadUserInput.new("The parameter relations is required, and must be of the form relations=id[,id[,id...]]")
+      raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]"
     end
 
     ids = params["relations"].split(",").collect(&:to_i)
 
     if ids.empty?
-      raise OSM::APIBadUserInput.new("No relations were given to search for")
+      raise OSM::APIBadUserInput, "No relations were given to search for"
     end
 
     doc = OSM::API.new.get_xml_doc

app/controllers/trace_controller.rb

@@ -112,7 +112,7 @@ class TraceController < ApplicationController
         begin
           do_create(params[:trace][:gpx_file], params[:trace][:tagstring],
                     params[:trace][:description], params[:trace][:visibility])
-        rescue => ex
+        rescue StandardError => ex
           logger.debug ex
         end
 

app/controllers/user_controller.rb

@@ -99,7 +99,7 @@ class UserController < ApplicationController
                                        "lat" => m[2],
                                        "lon" => m[3] }.merge(editor))
             end
-          rescue
+          rescue StandardError
             # Use default
           end
 
@@ -723,7 +723,7 @@ class UserController < ApplicationController
 
           begin
             Notifier.email_confirm(user, user.tokens.create).deliver_now
-          rescue
+          rescue StandardError
             # Ignore errors sending email
           end
         else

app/controllers/user_preference_controller.rb

@@ -45,7 +45,7 @@ class UserPreferenceController < ApplicationController
       if preference = old_preferences.delete(pt["k"])
         preference.v = pt["v"]
       elsif new_preferences.include?(pt["k"])
-        raise OSM::APIDuplicatePreferenceError.new(pt["k"])
+        raise OSM::APIDuplicatePreferenceError, pt["k"]
       else
         preference = current_user.preferences.build(:k => pt["k"], :v => pt["v"])
       end

app/controllers/way_controller.rb

@@ -36,7 +36,7 @@ class WayController < ApplicationController
     new_way = Way.from_xml(request.raw_post)
 
     unless new_way && new_way.id == way.id
-      raise OSM::APIBadUserInput.new("The id in the url (#{way.id}) is not the same as provided in the xml (#{new_way.id})")
+      raise OSM::APIBadUserInput, "The id in the url (#{way.id}) is not the same as provided in the xml (#{new_way.id})"
     end
 
     way.update_from(new_way, current_user)
@@ -81,14 +81,12 @@ class WayController < ApplicationController
 
   def ways
     unless params["ways"]
-      raise OSM::APIBadUserInput.new("The parameter ways is required, and must be of the form ways=id[,id[,id...]]")
+      raise OSM::APIBadUserInput, "The parameter ways is required, and must be of the form ways=id[,id[,id...]]"
     end
 
     ids = params["ways"].split(",").collect(&:to_i)
 
-    if ids.empty?
+    raise OSM::APIBadUserInput, "No ways were given to search for" if ids.empty?
-      raise OSM::APIBadUserInput.new("No ways were given to search for")
-    end
 
     doc = OSM::API.new.get_xml_doc
 

app/helpers/banner_helper.rb

@@ -4,7 +4,7 @@ module BannerHelper
       enddate = v[:enddate]
       begin
         parsed = enddate && Date.parse(enddate)
-      rescue
+      rescue StandardError
         parsed = nil
       end
       !parsed.is_a?(Date) || (parsed.is_a?(Date) && parsed.past?)

app/helpers/browse_helper.rb

@@ -1,4 +1,4 @@
-require "uri"
+require "cgi"
 
 module BrowseHelper
   def printable_name(object, version = false)
@@ -114,9 +114,17 @@ module BrowseHelper
     # the correct page.
     lookup_us = lookup.tr(" ", "_")
 
-    if page = WIKI_PAGES[locale][type][lookup_us] rescue nil
+    if page = begin
+                WIKI_PAGES[locale][type][lookup_us]
+              rescue
+                nil
+              end
       url = "http://wiki.openstreetmap.org/wiki/#{page}?uselang=#{locale}"
-    elsif page = WIKI_PAGES["en"][type][lookup_us] rescue nil
+    elsif page = begin
+                   WIKI_PAGES["en"][type][lookup_us]
+                 rescue
+                   nil
+                 end
       url = "http://wiki.openstreetmap.org/wiki/#{page}?uselang=#{locale}"
     end
 
@@ -151,7 +159,7 @@ module BrowseHelper
       # Must break it up to correctly build the url
       value = Regexp.last_match(1)
       section = "#" + Regexp.last_match(2)
-      encoded_section = "#" + URI.encode(Regexp.last_match(2).gsub(/ +/, "_"), /[^A-Za-z0-9:_]/).tr("%", ".")
+      encoded_section = "#" + CGI.escape(Regexp.last_match(2).gsub(/ +/, "_")).tr("%", ".")
     else
       section = ""
       encoded_section = ""

app/helpers/title_helper.rb

@@ -8,10 +8,10 @@ module TitleHelper
   def set_title(title = nil)
     if title
       @title = TitleHelper.coder.decode(title.gsub("<bdi>", "\u202a").gsub("</bdi>", "\u202c"))
-      response.headers["X-Page-Title"] = URI.escape(t("layouts.project_name.title") + " | " + @title)
+      response.headers["X-Page-Title"] = ERB::Util.u(t("layouts.project_name.title") + " | " + @title)
     else
       @title = title
-      response.headers["X-Page-Title"] = URI.escape(t("layouts.project_name.title"))
+      response.headers["X-Page-Title"] = ERB::Util.u(t("layouts.project_name.title"))
     end
   end
 end

app/models/changeset.rb

@@ -133,7 +133,7 @@ class Changeset < ActiveRecord::Base
   attr_writer :tags
 
   def add_tag_keyval(k, v)
-    @tags = {} unless @tags
+    @tags ||= {}
 
     # duplicate tags are now forbidden, so we can't allow values
     # in the hash to be overwritten.
@@ -241,10 +241,10 @@ class Changeset < ActiveRecord::Base
   # bounding box, only the tags of the changeset.
   def update_from(other, user)
     # ensure that only the user who opened the changeset may modify it.
-    raise OSM::APIUserChangesetMismatchError.new unless user.id == user_id
+    raise OSM::APIUserChangesetMismatchError unless user.id == user_id
 
     # can't change a closed changeset
-    raise OSM::APIChangesetAlreadyClosedError.new(self) unless is_open?
+    raise OSM::APIChangesetAlreadyClosedError, self unless is_open?
 
     # copy the other's tags
     self.tags = other.tags

app/models/node.rb

@@ -70,7 +70,7 @@ class Node < ActiveRecord::Base
     raise OSM::APIBadXMLError.new("node", pt, "Changeset id is missing") if pt["changeset"].nil?
     node.changeset_id = pt["changeset"].to_i
 
-    raise OSM::APIBadUserInput.new("The node is outside this world") unless node.in_world?
+    raise OSM::APIBadUserInput, "The node is outside this world" unless node.in_world?
 
     # version must be present unless creating
     raise OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create || !pt["version"].nil?
@@ -81,7 +81,7 @@ class Node < ActiveRecord::Base
       node.id = pt["id"].to_i
       # .to_i will return 0 if there is no number that can be parsed.
       # We want to make sure that there is no id with zero anyway
-      raise OSM::APIBadUserInput.new("ID of node cannot be zero when updating.") if node.id.zero?
+      raise OSM::APIBadUserInput, "ID of node cannot be zero when updating." if node.id.zero?
     end
 
     # We don't care about the time, as it is explicitly set on create/update/delete
@@ -120,10 +120,10 @@ class Node < ActiveRecord::Base
       lock!
       check_consistency(self, new_node, user)
       ways = Way.joins(:way_nodes).where(:visible => true, :current_way_nodes => { :node_id => id }).order(:id)
-      raise OSM::APIPreconditionFailedError.new("Node #{id} is still used by ways #{ways.collect(&:id).join(',')}.") unless ways.empty?
+      raise OSM::APIPreconditionFailedError, "Node #{id} is still used by ways #{ways.collect(&:id).join(',')}." unless ways.empty?
 
       rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Node", :member_id => id }).order(:id)
-      raise OSM::APIPreconditionFailedError.new("Node #{id} is still used by relations #{rels.collect(&:id).join(',')}.") unless rels.empty?
+      raise OSM::APIPreconditionFailedError, "Node #{id} is still used by relations #{rels.collect(&:id).join(',')}." unless rels.empty?
 
       self.changeset_id = new_node.changeset_id
       self.tags = {}
@@ -205,7 +205,7 @@ class Node < ActiveRecord::Base
   attr_writer :tags
 
   def add_tag_key_val(k, v)
-    @tags = {} unless @tags
+    @tags ||= {}
 
     # duplicate tags are now forbidden, so we can't allow values
     # in the hash to be overwritten.

app/models/oauth2_token.rb

@@ -9,9 +9,9 @@ class Oauth2Token < AccessToken
 
   def to_query
     q = "access_token=#{token}&token_type=bearer"
-    q << "&state=#{URI.escape(state)}" if @state
+    q << "&state=#{CGI.escape(state)}" if @state
     q << "&expires_in=#{expires_in}" if expires_at
-    q << "&scope=#{URI.escape(scope)}" if scope
+    q << "&scope=#{CGI.escape(scope)}" if scope
     q
   end
 

app/models/oauth2_verifier.rb

@@ -21,7 +21,7 @@ class Oauth2Verifier < OauthToken
 
   def to_query
     q = "code=#{token}"
-    q << "&state=#{URI.escape(state)}" if @state
+    q << "&state=#{CGI.escape(state)}" if @state
     q
   end
 

app/models/relation.rb

@@ -60,7 +60,7 @@ class Relation < ActiveRecord::Base
       relation.id = pt["id"].to_i
       # .to_i will return 0 if there is no number that can be parsed.
       # We want to make sure that there is no id with zero anyway
-      raise OSM::APIBadUserInput.new("ID of relation cannot be zero when updating.") if relation.id.zero?
+      raise OSM::APIBadUserInput, "ID of relation cannot be zero when updating." if relation.id.zero?
     end
 
     # We don't care about the timestamp nor the visibility as these are either
@@ -92,7 +92,7 @@ class Relation < ActiveRecord::Base
       member["role"] ||= "" # Allow  the upload to not include this, in which case we default to an empty string.
       relation.add_member(member["type"].classify, member["ref"], member["role"])
     end
-    raise OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil?
+    raise OSM::APIBadUserInput, "Some bad xml in relation" if relation.nil?
 
     relation
   end
@@ -155,7 +155,7 @@ class Relation < ActiveRecord::Base
   end
 
   def add_tag_keyval(k, v)
-    @tags = {} unless @tags
+    @tags ||= {}
 
     # duplicate tags are now forbidden, so we can't allow values
     # in the hash to be overwritten.
@@ -187,7 +187,7 @@ class Relation < ActiveRecord::Base
       check_consistency(self, new_relation, user)
       # This will check to see if this relation is used by another relation
       rel = RelationMember.joins(:relation).find_by("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id)
-      raise OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is used in relation #{rel.relation.id}.") unless rel.nil?
+      raise OSM::APIPreconditionFailedError, "The relation #{new_relation.id} is used in relation #{rel.relation.id}." unless rel.nil?
 
       self.changeset_id = new_relation.changeset_id
       self.tags = {}
@@ -202,7 +202,7 @@ class Relation < ActiveRecord::Base
       lock!
       check_consistency(self, new_relation, user)
       unless new_relation.preconditions_ok?(members)
-        raise OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.")
+        raise OSM::APIPreconditionFailedError, "Cannot update relation #{id}: data or member data is invalid."
       end
       self.changeset_id = new_relation.changeset_id
       self.changeset = new_relation.changeset
@@ -216,7 +216,7 @@ class Relation < ActiveRecord::Base
   def create_with_history(user)
     check_create_consistency(self, user)
     unless preconditions_ok?
-      raise OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.")
+      raise OSM::APIPreconditionFailedError, "Cannot create relation: data or member data is invalid."
     end
     self.version = 0
     self.visible = true
@@ -253,7 +253,7 @@ class Relation < ActiveRecord::Base
 
       # and check that it is OK to use.
       unless element && element.visible? && element.preconditions_ok?
-        raise OSM::APIPreconditionFailedError.new("Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}")
+        raise OSM::APIPreconditionFailedError, "Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}"
       end
       hash[m[1]] = true
     end
@@ -270,7 +270,7 @@ class Relation < ActiveRecord::Base
       old_id = id.to_i
       if old_id < 0
         new_id = id_map[type.downcase.to_sym][old_id]
-        raise OSM::APIBadUserInput.new("Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}.") if new_id.nil?
+        raise OSM::APIBadUserInput, "Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}." if new_id.nil?
         [type, new_id, role]
       else
         [type, id, role]

app/models/trace.rb

@@ -194,8 +194,8 @@ class Trace < ActiveRecord::Base
       id = pt["id"].to_i
       # .to_i will return 0 if there is no number that can be parsed.
       # We want to make sure that there is no id with zero anyway
-      raise OSM::APIBadUserInput.new("ID of trace cannot be zero when updating.") if id.zero?
+      raise OSM::APIBadUserInput, "ID of trace cannot be zero when updating." if id.zero?
-      raise OSM::APIBadUserInput.new("The id in the url (#{self.id}) is not the same as provided in the xml (#{id})") unless self.id == id
+      raise OSM::APIBadUserInput, "The id in the url (#{self.id}) is not the same as provided in the xml (#{id})" unless self.id == id
     end
 
     # We don't care about the time, as it is explicitly set on create/update/delete

app/models/way.rb

@@ -58,7 +58,7 @@ class Way < ActiveRecord::Base
       way.id = pt["id"].to_i
       # .to_i will return 0 if there is no number that can be parsed.
       # We want to make sure that there is no id with zero anyway
-      raise OSM::APIBadUserInput.new("ID of way cannot be zero when updating.") if way.id.zero?
+      raise OSM::APIBadUserInput, "ID of way cannot be zero when updating." if way.id.zero?
     end
 
     # We don't care about the timestamp nor the visibility as these are either
@@ -138,12 +138,12 @@ class Way < ActiveRecord::Base
   attr_writer :tags
 
   def add_nd_num(n)
-    @nds = [] unless @nds
+    @nds ||= []
     @nds << n.to_i
   end
 
   def add_tag_keyval(k, v)
-    @tags = {} unless @tags
+    @tags ||= {}
 
     # duplicate tags are now forbidden, so we can't allow values
     # in the hash to be overwritten.
@@ -166,7 +166,7 @@ class Way < ActiveRecord::Base
       lock!
       check_consistency(self, new_way, user)
       unless new_way.preconditions_ok?(nds)
-        raise OSM::APIPreconditionFailedError.new("Cannot update way #{id}: data is invalid.")
+        raise OSM::APIPreconditionFailedError, "Cannot update way #{id}: data is invalid."
       end
 
       self.changeset_id = new_way.changeset_id
@@ -181,7 +181,7 @@ class Way < ActiveRecord::Base
   def create_with_history(user)
     check_create_consistency(self, user)
     unless preconditions_ok?
-      raise OSM::APIPreconditionFailedError.new("Cannot create way: data is invalid.")
+      raise OSM::APIPreconditionFailedError, "Cannot create way: data is invalid."
     end
     self.version = 0
     self.visible = true
@@ -205,7 +205,7 @@ class Way < ActiveRecord::Base
 
       if db_nds.length < new_nds.length
         missing = new_nds - db_nds.collect(&:id)
-        raise OSM::APIPreconditionFailedError.new("Way #{id} requires the nodes with id in (#{missing.join(',')}), which either do not exist, or are not visible.")
+        raise OSM::APIPreconditionFailedError, "Way #{id} requires the nodes with id in (#{missing.join(',')}), which either do not exist, or are not visible."
       end
     end
 
@@ -222,7 +222,7 @@ class Way < ActiveRecord::Base
       lock!
       check_consistency(self, new_way, user)
       rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Way", :member_id => id }).order(:id)
-      raise OSM::APIPreconditionFailedError.new("Way #{id} is still used by relations #{rels.collect(&:id).join(',')}.") unless rels.empty?
+      raise OSM::APIPreconditionFailedError, "Way #{id} is still used by relations #{rels.collect(&:id).join(',')}." unless rels.empty?
 
       self.changeset_id = new_way.changeset_id
       self.changeset = new_way.changeset
@@ -242,7 +242,7 @@ class Way < ActiveRecord::Base
     nds.map! do |node_id|
       if node_id < 0
         new_id = id_map[:node][node_id]
-        raise OSM::APIBadUserInput.new("Placeholder node not found for reference #{node_id} in way #{id.nil? ? placeholder_id : id}") if new_id.nil?
+        raise OSM::APIBadUserInput, "Placeholder node not found for reference #{node_id} in way #{id.nil? ? placeholder_id : id}" if new_id.nil?
         new_id
       else
         node_id

config/initializers/banners.rb

@@ -1,5 +1,5 @@
 begin
   BANNERS = YAML.load_file(Rails.root.join("config", "banners.yml")).deep_symbolize_keys
-rescue
+rescue StandardError
   BANNERS = {}.freeze
 end

db/migrate/006_tile_nodes.rb

@@ -3,33 +3,33 @@ require "migrate"
 class TileNodes < ActiveRecord::Migration
   def self.upgrade_table(from_table, to_table, model)
     if ENV["USE_DB_FUNCTIONS"]
-      execute <<-END_SQL
+      execute <<-SQL
       INSERT INTO #{to_table} (id, latitude, longitude, user_id, visible, tags, timestamp, tile)
       SELECT id, ROUND(latitude * 10000000), ROUND(longitude * 10000000),
              user_id, visible, tags, timestamp,
              tile_for_point(CAST(ROUND(latitude * 10000000) AS INTEGER),
                             CAST(ROUND(longitude * 10000000) AS INTEGER))
       FROM #{from_table}
-      END_SQL
+      SQL
     else
-      execute <<-END_SQL
+      execute <<-SQL
       INSERT INTO #{to_table} (id, latitude, longitude, user_id, visible, tags, timestamp, tile)
       SELECT id, ROUND(latitude * 10000000), ROUND(longitude * 10000000),
              user_id, visible, tags, timestamp, 0
       FROM #{from_table}
-      END_SQL
+      SQL
 
       model.all.each(&:save!)
     end
   end
 
   def self.downgrade_table(from_table, to_table)
-    execute <<-END_SQL
+    execute <<-SQL
     INSERT INTO #{to_table} (id, latitude, longitude, user_id, visible, tags, timestamp)
     SELECT id, latitude / 10000000, longitude / 10000000,
            user_id, visible, tags, timestamp
     FROM #{from_table}
-    END_SQL
+    SQL
   end
 
   def self.up

db/migrate/008_remove_segments.rb

@@ -10,7 +10,7 @@ class RemoveSegments < ActiveRecord::Migration
       cmd = "db/migrate/008_remove_segments_helper"
       src = "#{cmd}.cc"
       if !File.exist?(cmd) || File.mtime(cmd) < File.mtime(src)
-        system("c++ -O3 -Wall `mysql_config --cflags --libs` " +
+        system("c++ -O3 -Wall `mysql_config --cflags --libs` " \
           "#{src} -o #{cmd}") || raise
       end
 

db/migrate/020_populate_node_tags_and_remove.rb

@@ -10,7 +10,7 @@ class PopulateNodeTagsAndRemove < ActiveRecord::Migration
       cmd = "db/migrate/020_populate_node_tags_and_remove_helper"
       src = "#{cmd}.c"
       if !File.exist?(cmd) || File.mtime(cmd) < File.mtime(src)
-        system("cc -O3 -Wall `mysql_config --cflags --libs` " +
+        system("cc -O3 -Wall `mysql_config --cflags --libs` " \
           "#{src} -o #{cmd}") || raise
       end
 

db/migrate/023_add_changesets.rb

@@ -28,7 +28,7 @@ class AddChangesets < ActiveRecord::Migration
     # all edits up to the API change,
     # all the changesets will have the id of the user that made them.
     # We need to generate a changeset for each user in the database
-    execute "INSERT INTO changesets (id, user_id, created_at, open)" +
+    execute "INSERT INTO changesets (id, user_id, created_at, open)" \
             "SELECT id, id, creation_time, false from users;"
 
     @conv_user_tables.each do |tbl|

lib/bounding_box.rb

@@ -66,18 +66,14 @@ class BoundingBox
   def check_boundaries
     # check the bbox is sane
     if min_lon > max_lon
-      raise OSM::APIBadBoundingBox.new(
+      raise OSM::APIBadBoundingBox, "The minimum longitude must be less than the maximum longitude, but it wasn't"
-        "The minimum longitude must be less than the maximum longitude, but it wasn't"
-      )
     end
     if min_lat > max_lat
-      raise OSM::APIBadBoundingBox.new(
+      raise OSM::APIBadBoundingBox, "The minimum latitude must be less than the maximum latitude, but it wasn't"
-        "The minimum latitude must be less than the maximum latitude, but it wasn't"
-      )
     end
     if min_lon < -LON_LIMIT || min_lat < -LAT_LIMIT || max_lon > +LON_LIMIT || max_lat > +LAT_LIMIT
-      raise OSM::APIBadBoundingBox.new("The latitudes must be between #{-LAT_LIMIT} and #{LAT_LIMIT}," +
+      raise OSM::APIBadBoundingBox, "The latitudes must be between #{-LAT_LIMIT} and #{LAT_LIMIT}," \
-                                       " and longitudes between #{-LON_LIMIT} and #{LON_LIMIT}")
+                                       " and longitudes between #{-LON_LIMIT} and #{LON_LIMIT}"
     end
     self
   end
@@ -85,8 +81,8 @@ class BoundingBox
   def check_size(max_area = MAX_REQUEST_AREA)
     # check the bbox isn't too large
     if area > max_area
-      raise OSM::APIBadBoundingBox.new("The maximum bbox size is " + max_area.to_s +
+      raise OSM::APIBadBoundingBox, "The maximum bbox size is " + max_area.to_s +
-        ", and your request was too large. Either request a smaller area, or use planet.osm")
+                                    ", and your request was too large. Either request a smaller area, or use planet.osm"
     end
     self
   end
@@ -171,9 +167,7 @@ class BoundingBox
 
     def from_bbox_array(bbox_array)
       unless bbox_array
-        raise OSM::APIBadUserInput.new(
+        raise OSM::APIBadUserInput, "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat"
-          "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat"
-        )
       end
       # Take an array of length 4, create a bounding box with min_lon, min_lat, max_lon and
       # max_lat within their respective boundaries.

lib/consistency_validations.rb

@@ -7,26 +7,26 @@ module ConsistencyValidations
   # This will throw an exception if there is an inconsistency
   def check_consistency(old, new, user)
     if new.id != old.id || new.id.nil? || old.id.nil?
-      raise OSM::APIPreconditionFailedError.new("New and old IDs don't match on #{new.class}. #{new.id} != #{old.id}.")
+      raise OSM::APIPreconditionFailedError, "New and old IDs don't match on #{new.class}. #{new.id} != #{old.id}."
     elsif new.version != old.version
       raise OSM::APIVersionMismatchError.new(new.id, new.class.to_s, new.version, old.version)
     elsif new.changeset.nil?
-      raise OSM::APIChangesetMissingError.new
+      raise OSM::APIChangesetMissingError
     elsif new.changeset.user_id != user.id
-      raise OSM::APIUserChangesetMismatchError.new
+      raise OSM::APIUserChangesetMismatchError
     elsif !new.changeset.is_open?
-      raise OSM::APIChangesetAlreadyClosedError.new(new.changeset)
+      raise OSM::APIChangesetAlreadyClosedError, new.changeset
     end
   end
 
   # This is similar to above, just some validations don't apply
   def check_create_consistency(new, user)
     if new.changeset.nil?
-      raise OSM::APIChangesetMissingError.new
+      raise OSM::APIChangesetMissingError
     elsif new.changeset.user_id != user.id
-      raise OSM::APIUserChangesetMismatchError.new
+      raise OSM::APIUserChangesetMismatchError
     elsif !new.changeset.is_open?
-      raise OSM::APIChangesetAlreadyClosedError.new(new.changeset)
+      raise OSM::APIChangesetAlreadyClosedError, new.changeset
     end
   end
 
@@ -37,11 +37,11 @@ module ConsistencyValidations
     # check user credentials - only the user who opened a changeset
     # may alter it.
     if changeset.nil?
-      raise OSM::APIChangesetMissingError.new
+      raise OSM::APIChangesetMissingError
     elsif user.id != changeset.user_id
-      raise OSM::APIUserChangesetMismatchError.new
+      raise OSM::APIUserChangesetMismatchError
     elsif !changeset.is_open?
-      raise OSM::APIChangesetAlreadyClosedError.new(changeset)
+      raise OSM::APIChangesetAlreadyClosedError, changeset
     end
   end
 end

lib/daemons/gpx_import_ctl

@@ -6,7 +6,7 @@ require "erb"
 
 class Hash
   def with_symbols!
-    keys.each { |key| self[key.to_s.to_sym] = self[key] }
+    each_key { |key| self[key.to_s.to_sym] = self[key] }
     self
   end
 end

lib/diff_reader.rb

@@ -86,8 +86,8 @@ class DiffReader
     with_element do |model_name, _model_attributes|
       model = MODELS[model_name]
       if model.nil?
-        raise OSM::APIBadUserInput.new("Unexpected element type #{model_name}, " +
+        raise OSM::APIBadUserInput, "Unexpected element type #{model_name}, " \
-                                       "expected node, way or relation.")
+                                       "expected node, way or relation."
       end
       # new in libxml-ruby >= 2, expand returns an element not associated
       # with a document. this means that there's no encoding parameter,
@@ -130,7 +130,7 @@ class DiffReader
 
     # take the first element and check that it is an osmChange element
     @reader.read
-    raise OSM::APIBadUserInput.new("Document element should be 'osmChange'.") if @reader.name != "osmChange"
+    raise OSM::APIBadUserInput, "Document element should be 'osmChange'." if @reader.name != "osmChange"
 
     result = OSM::API.new.get_xml_doc
     result.root.name = "diffResult"
@@ -152,7 +152,7 @@ class DiffReader
           # check if the placeholder ID has been given before and throw
           # an exception if it has - we can't create the same element twice.
           model_sym = model.to_s.downcase.to_sym
-          raise OSM::APIBadUserInput.new("Placeholder IDs must be unique for created elements.") if ids[model_sym].include? placeholder_id
+          raise OSM::APIBadUserInput, "Placeholder IDs must be unique for created elements." if ids[model_sym].include? placeholder_id
 
           # some elements may have placeholders for other elements in the
           # diff, so we must fix these before saving the element.
@@ -252,7 +252,7 @@ class DiffReader
 
       else
         # no other actions to choose from, so it must be the users fault!
-        raise OSM::APIChangesetActionInvalid.new(action_name)
+        raise OSM::APIChangesetActionInvalid, action_name
       end
     end
 

lib/not_redactable.rb

@@ -6,6 +6,6 @@ module NotRedactable
   end
 
   def redact!(_r)
-    raise OSM::APICannotRedactError.new
+    raise OSM::APICannotRedactError
   end
 end

lib/osm.rb

@@ -532,7 +532,7 @@ module OSM
   # Parse a float, raising a specified exception on failure
   def self.parse_float(str, klass, *args)
     Float(str)
-  rescue
+  rescue StandardError
     raise klass.new(*args)
   end
 
@@ -553,7 +553,7 @@ module OSM
     tilesql = QuadTile.sql_for_area(bbox, prefix)
     bbox = bbox.to_scaled
 
-    "#{tilesql} AND #{prefix}latitude BETWEEN #{bbox.min_lat} AND #{bbox.max_lat} " +
+    "#{tilesql} AND #{prefix}latitude BETWEEN #{bbox.min_lat} AND #{bbox.max_lat} " \
       "AND #{prefix}longitude BETWEEN #{bbox.min_lon} AND #{bbox.max_lon}"
   end
 

lib/redactable.rb

@@ -13,7 +13,7 @@ module Redactable
 
   def redact!(redaction)
     # check that this version isn't the current version
-    raise OSM::APICannotRedactError.new if is_latest_version?
+    raise OSM::APICannotRedactError if is_latest_version?
 
     # make the change
     self.redaction = redaction

lib/session_persistence.rb

@@ -55,7 +55,7 @@ module SessionPersistence
     if session[session_persistence_key]
       request.session_options[:expire_after] = session[session_persistence_key]
     end
-  rescue
+  rescue StandardError
     reset_session
   end
 end

test/controllers/changeset_controller_test.rb

@@ -98,16 +98,16 @@ class ChangesetControllerTest < ActionController::TestCase
   def test_create
     basic_authorization create(:user, :data_public => false).email, "test"
     # Create the first user's changeset
-    content "<osm><changeset>" +
+    content "<osm><changeset>" \
-            "<tag k='created_by' v='osm test suite checking changesets'/>" +
+            "<tag k='created_by' v='osm test suite checking changesets'/>" \
             "</changeset></osm>"
     put :create
     assert_require_public_data
 
     basic_authorization create(:user).email, "test"
     # Create the first user's changeset
-    content "<osm><changeset>" +
+    content "<osm><changeset>" \
-            "<tag k='created_by' v='osm test suite checking changesets'/>" +
+            "<tag k='created_by' v='osm test suite checking changesets'/>" \
             "</changeset></osm>"
     put :create
 
@@ -327,7 +327,7 @@ class ChangesetControllerTest < ActionController::TestCase
 
     # simple diff to change a node, way and relation by removing
     # their tags
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <modify>
         <node id='#{node.id}' lon='0' lat='0' changeset='#{changeset_id}' version='1'/>
@@ -343,7 +343,7 @@ class ChangesetControllerTest < ActionController::TestCase
         </relation>
        </modify>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -357,7 +357,7 @@ EOF
 
     # simple diff to change a node, way and relation by removing
     # their tags
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <modify>
         <node id='#{node.id}' lon='0' lat='0' changeset='#{changeset_id}' version='1'/>
@@ -373,7 +373,7 @@ EOF
         </relation>
        </modify>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -387,7 +387,7 @@ EOF
 
     # simple diff to change a node, way and relation by removing
     # their tags
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <modify>
         <node id='#{node.id}' lon='0' lat='0' changeset='#{changeset_id}' version='1'/>
@@ -403,7 +403,7 @@ EOF
         </relation>
        </modify>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -429,7 +429,7 @@ EOF
     basic_authorization user.email, "test"
 
     # simple diff to create a node way and relation using placeholders
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <create>
         <node id='-1' lon='0' lat='0' changeset='#{changeset.id}'>
@@ -448,7 +448,7 @@ EOF
         </relation>
        </create>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -561,8 +561,8 @@ EOF
       basic_authorization create(:user).email, "test"
 
       # create a temporary changeset
-      content "<osm><changeset>" +
+      content "<osm><changeset>" \
-              "<tag k='created_by' v='osm test suite checking changesets'/>" +
+              "<tag k='created_by' v='osm test suite checking changesets'/>" \
               "</changeset></osm>"
       assert_difference "Changeset.count", 1 do
         put :create
@@ -582,7 +582,7 @@ EOF
 
     # upload some widely-spaced nodes, spiralling positive and negative to cause
     # largest bbox over-expansion possible.
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <create>
         <node id='-1' lon='-20' lat='-10' changeset='#{changeset_id}'/>
@@ -605,7 +605,7 @@ EOF
         <node id='-18' lon='179.9'  lat='89.9' changeset='#{changeset_id}'/>
        </create>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it, which used to cause an error like "PGError: ERROR:
     # integer out of range" (bug #2152). but shouldn't any more.
@@ -739,7 +739,7 @@ EOF
     basic_authorization changeset.user.email, "test"
 
     # simple diff to create a node way and relation using placeholders
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <create>
         <node id='-1' lon='0' lat='0' changeset='#{changeset.id}'>
@@ -747,7 +747,7 @@ EOF
         </node>
        </create>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -770,7 +770,7 @@ EOF
     basic_authorization changeset.user.email, "test"
 
     # simple diff to create a node way and relation using placeholders
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <create>
         <node id='-1' lon='0' lat='0' changeset='#{changeset.id}'>
@@ -790,7 +790,7 @@ EOF
         </relation>
        </modify>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -832,7 +832,7 @@ EOF
     basic_authorization changeset.user.email, "test"
 
     # simple diff to create a node way and relation using placeholders
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <modify>
         <node id='#{node.id}' lon='0' lat='0' changeset='#{changeset.id}' version='1'/>
@@ -854,7 +854,7 @@ EOF
         </node>
        </create>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -878,7 +878,7 @@ EOF
     # change the location of a node multiple times, each time referencing
     # the last version. doesn't this depend on version numbers being
     # sequential?
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <modify>
         <node id='#{node.id}' lon='0' lat='0' changeset='#{changeset.id}' version='1'/>
@@ -891,7 +891,7 @@ EOF
         <node id='#{node.id}' lon='9' lat='9' changeset='#{changeset.id}' version='8'/>
        </modify>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -914,14 +914,14 @@ EOF
 
     basic_authorization changeset.user.email, "test"
 
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <modify>
         <node id='#{node.id}' lon='0' lat='0' changeset='#{changeset.id}' version='1'/>
         <node id='#{node.id}' lon='1' lat='1' changeset='#{changeset.id}' version='1'/>
        </modify>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -937,13 +937,13 @@ EOF
 
     basic_authorization changeset.user.email, "test"
 
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <modify>
        <node id='1' lon='1' lat='1' changeset='#{changeset.id}'/>
        </modify>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -959,13 +959,13 @@ EOF
 
     basic_authorization changeset.user.email, "test"
 
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
         <ping>
          <node id='1' lon='1' lat='1' changeset='#{changeset.id}' />
         </ping>
       </osmChange>
-EOF
+CHANGESET
     content diff
     post :upload, :params => { :id => changeset.id }
     assert_response :bad_request, "Shouldn't be able to upload a diff with the action ping"
@@ -985,7 +985,7 @@ EOF
 
     basic_authorization changeset.user.email, "test"
 
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <modify><node id='#{node.id}' lon='0' lat='0' changeset='#{changeset.id}'
         version='1'></node>
@@ -997,7 +997,7 @@ EOF
          <member type='relation' role='some' ref='#{other_relation.id}'/>
         </relation>
        </modify></osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -1021,7 +1021,7 @@ EOF
 
     basic_authorization changeset.user.email, "test"
 
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <create>
         <node id='-1' lon='0' lat='0' changeset='#{changeset.id}'>
@@ -1035,7 +1035,7 @@ EOF
         <node id='-1' lon='2' lat='2' changeset='#{changeset.id}' version='2'/>
        </delete>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -1056,7 +1056,7 @@ EOF
 
     basic_authorization changeset.user.email, "test"
 
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <create>
         <node id='-1' lon='0' lat='0' changeset='#{changeset.id}' version='1'/>
@@ -1064,7 +1064,7 @@ EOF
         <node id='-1' lon='2' lat='2' changeset='#{changeset.id}' version='2'/>
        </create>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -1082,7 +1082,7 @@ EOF
 
     basic_authorization changeset.user.email, "test"
 
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <create>
         <node id="-1" lon="0" lat="0" changeset="#{changeset.id}" version="1"/>
@@ -1096,7 +1096,7 @@ EOF
         </way>
        </create>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -1106,7 +1106,7 @@ EOF
     assert_equal "Placeholder node not found for reference -4 in way -1", @response.body
 
     # the same again, but this time use an existing way
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <create>
         <node id="-1" lon="0" lat="0" changeset="#{changeset.id}" version="1"/>
@@ -1120,7 +1120,7 @@ EOF
         </way>
        </create>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -1139,7 +1139,7 @@ EOF
 
     basic_authorization changeset.user.email, "test"
 
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <create>
         <node id="-1" lon="0" lat="0" changeset="#{changeset.id}" version="1"/>
@@ -1153,7 +1153,7 @@ EOF
         </relation>
        </create>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -1163,7 +1163,7 @@ EOF
     assert_equal "Placeholder Node not found for reference -4 in relation -1.", @response.body
 
     # the same again, but this time use an existing relation
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <create>
         <node id="-1" lon="0" lat="0" changeset="#{changeset.id}" version="1"/>
@@ -1177,7 +1177,7 @@ EOF
         </relation>
        </create>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -1193,8 +1193,8 @@ EOF
   def test_upload_node_move
     basic_authorization create(:user).email, "test"
 
-    content "<osm><changeset>" +
+    content "<osm><changeset>" \
-            "<tag k='created_by' v='osm test suite checking changesets'/>" +
+            "<tag k='created_by' v='osm test suite checking changesets'/>" \
             "</changeset></osm>"
     put :create
     assert_response :success
@@ -1231,8 +1231,8 @@ EOF
   def test_upload_way_extend
     basic_authorization create(:user).email, "test"
 
-    content "<osm><changeset>" +
+    content "<osm><changeset>" \
-            "<tag k='created_by' v='osm test suite checking changesets'/>" +
+            "<tag k='created_by' v='osm test suite checking changesets'/>" \
             "</changeset></osm>"
     put :create
     assert_response :success
@@ -1280,7 +1280,7 @@ EOF
       # upload it
       content diff
       post :upload, :params => { :id => changeset.id }
-      assert_response(:success, "should be able to upload " +
+      assert_response(:success, "should be able to upload " \
                       "empty changeset: " + diff)
     end
   end
@@ -1324,8 +1324,8 @@ EOF
     basic_authorization create(:user, :data_public => false).email, "test"
 
     # create a temporary changeset
-    content "<osm><changeset>" +
+    content "<osm><changeset>" \
-            "<tag k='created_by' v='osm test suite checking changesets'/>" +
+            "<tag k='created_by' v='osm test suite checking changesets'/>" \
             "</changeset></osm>"
     put :create
     assert_response :forbidden
@@ -1334,15 +1334,15 @@ EOF
     basic_authorization create(:user).email, "test"
 
     # create a temporary changeset
-    content "<osm><changeset>" +
+    content "<osm><changeset>" \
-            "<tag k='created_by' v='osm test suite checking changesets'/>" +
+            "<tag k='created_by' v='osm test suite checking changesets'/>" \
             "</changeset></osm>"
     put :create
     assert_response :success
     changeset_id = @response.body.to_i
 
     # add a diff to it
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <modify>
         <node id='#{node.id}' lon='0' lat='0' changeset='#{changeset_id}' version='1'/>
@@ -1355,7 +1355,7 @@ EOF
         <node id='#{node.id}' lon='9' lat='9' changeset='#{changeset_id}' version='8'/>
        </modify>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -1380,8 +1380,8 @@ EOF
     basic_authorization create(:user).email, "test"
 
     # create a temporary changeset
-    content "<osm><changeset>" +
+    content "<osm><changeset>" \
-            "<tag k='created_by' v='osm test suite checking changesets'/>" +
+            "<tag k='created_by' v='osm test suite checking changesets'/>" \
             "</changeset></osm>"
     put :create
     assert_response :success
@@ -1442,15 +1442,15 @@ OSMFILE
     basic_authorization create(:user).email, "test"
 
     # create a temporary changeset
-    content "<osm><changeset>" +
+    content "<osm><changeset>" \
-            "<tag k='created_by' v='osm test suite checking changesets'/>" +
+            "<tag k='created_by' v='osm test suite checking changesets'/>" \
             "</changeset></osm>"
     put :create
     assert_response :success
     changeset_id = @response.body.to_i
 
     # add a diff to it
-    diff = <<EOF.strip_heredoc
+    diff = <<CHANGESET.strip_heredoc
       <osmChange>
        <delete>
         <node id='#{node.id}' lon='0' lat='0' changeset='#{changeset_id}' version='1'/>
@@ -1470,7 +1470,7 @@ OSMFILE
         </way>
        </modify>
       </osmChange>
-EOF
+CHANGESET
 
     # upload it
     content diff
@@ -1880,7 +1880,7 @@ EOF
 
     # check that the changeset is now closed as well
     assert(!changeset.is_open?,
-           "changeset should have been auto-closed by exceeding " +
+           "changeset should have been auto-closed by exceeding " \
            "element limit.")
   end
 

test/controllers/diary_entry_controller_test.rb

@@ -254,7 +254,7 @@ class DiaryEntryControllerTest < ActionController::TestCase
     get :edit,
         :params => { :display_name => entry.user.display_name, :id => entry.id }
     assert_response :redirect
-    assert_redirected_to :controller => :user, :action => :login, :referer => "/user/#{URI.encode(entry.user.display_name)}/diary/#{entry.id}/edit"
+    assert_redirected_to :controller => :user, :action => :login, :referer => "/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit"
 
     # Verify that you get a not found error, when you pass a bogus id
     get :edit,
@@ -284,7 +284,7 @@ class DiaryEntryControllerTest < ActionController::TestCase
       assert_select "h1", :text => /Edit diary entry/, :count => 1
     end
     assert_select "div#content", :count => 1 do
-      assert_select "form[action='/user/#{URI.encode(entry.user.display_name)}/diary/#{entry.id}/edit'][method=post]", :count => 1 do
+      assert_select "form[action='/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit'][method=post]", :count => 1 do
         assert_select "input#diary_entry_title[name='diary_entry[title]'][value='#{entry.title}']", :count => 1
         assert_select "textarea#diary_entry_body[name='diary_entry[body]']", :text => entry.body, :count => 1
         assert_select "select#diary_entry_language_code", :count => 1
@@ -329,7 +329,7 @@ class DiaryEntryControllerTest < ActionController::TestCase
       assert_select "abbr[class='geo'][title='#{number_with_precision(new_latitude, :precision => 4)}; #{number_with_precision(new_longitude, :precision => 4)}']", :count => 1
       # As we're not logged in, check that you cannot edit
       # print @response.body
-      assert_select "a[href='/user/#{URI.encode(entry.user.display_name)}/diary/#{entry.id}/edit']", :text => "Edit this entry", :count => 1
+      assert_select "a[href='/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit']", :text => "Edit this entry", :count => 1
     end
 
     # and when not logged in as the user who wrote the entry
@@ -350,7 +350,7 @@ class DiaryEntryControllerTest < ActionController::TestCase
       assert_select "abbr[class=geo][title='#{number_with_precision(new_latitude, :precision => 4)}; #{number_with_precision(new_longitude, :precision => 4)}']", :count => 1
       # As we're not logged in, check that you cannot edit
       assert_select "li[class='hidden show_if_user_#{entry.user.id}']", :count => 1 do
-        assert_select "a[href='/user/#{URI.encode(entry.user.display_name)}/diary/#{entry.id}/edit']", :text => "Edit this entry", :count => 1
+        assert_select "a[href='/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit']", :text => "Edit this entry", :count => 1
       end
     end
   end
@@ -430,7 +430,7 @@ class DiaryEntryControllerTest < ActionController::TestCase
     assert_response :success
     assert_select ".diary-comment", :count => 1 do
       assert_select "#comment#{comment.id}", :count => 1 do
-        assert_select "a[href='/user/#{URI.encode(other_user.display_name)}']", :text => other_user.display_name, :count => 1
+        assert_select "a[href='/user/#{ERB::Util.u(other_user.display_name)}']", :text => other_user.display_name, :count => 1
       end
       assert_select ".richtext", :text => /New comment/, :count => 1
     end
@@ -890,7 +890,7 @@ class DiaryEntryControllerTest < ActionController::TestCase
     assert_select "div.diary_post", entries.count
 
     entries.each do |entry|
-      assert_select "a[href=?]", "/user/#{URI.encode(entry.user.display_name)}/diary/#{entry.id}"
+      assert_select "a[href=?]", "/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}"
     end
   end
 end

test/controllers/geocoder_controller_test.rb

@@ -1,4 +1,3 @@
-# coding: utf-8
 
 require "test_helper"
 require "geocoder_controller"

test/controllers/node_controller_test.rb

@@ -496,8 +496,8 @@ class NodeControllerTest < ActionController::TestCase
 
     # try and put something into a string that the API might
     # use unquoted and therefore allow code injection...
-    content "<osm><node lat='0' lon='0' changeset='#{private_changeset.id}'>" +
+    content "<osm><node lat='0' lon='0' changeset='#{private_changeset.id}'>" \
-            '<tag k="#{@user.inspect}" v="0"/>' +
+            '<tag k="#{@user.inspect}" v="0"/>' \
             "</node></osm>"
     put :create
     assert_require_public_data "Shouldn't be able to create with non-public user"
@@ -507,8 +507,8 @@ class NodeControllerTest < ActionController::TestCase
 
     # try and put something into a string that the API might
     # use unquoted and therefore allow code injection...
-    content "<osm><node lat='0' lon='0' changeset='#{changeset.id}'>" +
+    content "<osm><node lat='0' lon='0' changeset='#{changeset.id}'>" \
-            '<tag k="#{@user.inspect}" v="0"/>' +
+            '<tag k="#{@user.inspect}" v="0"/>' \
             "</node></osm>"
     put :create
     assert_response :success

test/controllers/old_node_controller_test.rb

@@ -137,7 +137,7 @@ class OldNodeControllerTest < ActionController::TestCase
     end
 
     # check all the versions
-    versions.keys.each do |key|
+    versions.each_key do |key|
       get :version, :params => { :id => nodeid, :version => key.to_i }
 
       assert_response :success,

test/controllers/relation_controller_test.rb

@@ -216,8 +216,8 @@ class RelationControllerTest < ActionController::TestCase
     ###
     # create an relation with a node as member
     # This time try with a role attribute in the relation
-    content "<osm><relation changeset='#{private_changeset.id}'>" +
+    content "<osm><relation changeset='#{private_changeset.id}'>" \
-            "<member  ref='#{node.id}' type='node' role='some'/>" +
+            "<member  ref='#{node.id}' type='node' role='some'/>" \
             "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for forbidden due to user
@@ -227,7 +227,7 @@ class RelationControllerTest < ActionController::TestCase
     ###
     # create an relation with a node as member, this time test that we don't
     # need a role attribute to be included
-    content "<osm><relation changeset='#{private_changeset.id}'>" +
+    content "<osm><relation changeset='#{private_changeset.id}'>" \
             "<member  ref='#{node.id}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for forbidden due to user
@@ -236,9 +236,9 @@ class RelationControllerTest < ActionController::TestCase
 
     ###
     # create an relation with a way and a node as members
-    content "<osm><relation changeset='#{private_changeset.id}'>" +
+    content "<osm><relation changeset='#{private_changeset.id}'>" \
-            "<member type='node' ref='#{node.id}' role='some'/>" +
+            "<member type='node' ref='#{node.id}' role='some'/>" \
-            "<member type='way' ref='#{way.id}' role='other'/>" +
+            "<member type='way' ref='#{way.id}' role='other'/>" \
             "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for forbidden, due to user
@@ -277,8 +277,8 @@ class RelationControllerTest < ActionController::TestCase
     ###
     # create an relation with a node as member
     # This time try with a role attribute in the relation
-    content "<osm><relation changeset='#{changeset.id}'>" +
+    content "<osm><relation changeset='#{changeset.id}'>" \
-            "<member  ref='#{node.id}' type='node' role='some'/>" +
+            "<member  ref='#{node.id}' type='node' role='some'/>" \
             "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for success
@@ -308,7 +308,7 @@ class RelationControllerTest < ActionController::TestCase
     ###
     # create an relation with a node as member, this time test that we don't
     # need a role attribute to be included
-    content "<osm><relation changeset='#{changeset.id}'>" +
+    content "<osm><relation changeset='#{changeset.id}'>" \
             "<member  ref='#{node.id}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for success
@@ -337,9 +337,9 @@ class RelationControllerTest < ActionController::TestCase
 
     ###
     # create an relation with a way and a node as members
-    content "<osm><relation changeset='#{changeset.id}'>" +
+    content "<osm><relation changeset='#{changeset.id}'>" \
-            "<member type='node' ref='#{node.id}' role='some'/>" +
+            "<member type='node' ref='#{node.id}' role='some'/>" \
-            "<member type='way' ref='#{way.id}' role='other'/>" +
+            "<member type='way' ref='#{way.id}' role='other'/>" \
             "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for success
@@ -461,8 +461,8 @@ class RelationControllerTest < ActionController::TestCase
     basic_authorization user.email, "test"
 
     # create a relation with non-existing node as member
-    content "<osm><relation changeset='#{changeset.id}'>" +
+    content "<osm><relation changeset='#{changeset.id}'>" \
-            "<member type='node' ref='0'/><tag k='test' v='yes' />" +
+            "<member type='node' ref='0'/><tag k='test' v='yes' />" \
             "</relation></osm>"
     put :create
     # expect failure
@@ -482,8 +482,8 @@ class RelationControllerTest < ActionController::TestCase
     basic_authorization user.email, "test"
 
     # create some xml that should return an error
-    content "<osm><relation changeset='#{changeset.id}'>" +
+    content "<osm><relation changeset='#{changeset.id}'>" \
-            "<member type='type' ref='#{node.id}' role=''/>" +
+            "<member type='type' ref='#{node.id}' role=''/>" \
             "<tag k='tester' v='yep'/></relation></osm>"
     put :create
     # expect failure
@@ -1063,7 +1063,7 @@ OSM
     assert_equal a_tags.keys, b_tags.keys, "Tag keys should be identical."
     a_tags.each do |k, v|
       assert_equal v, b_tags[k],
-                   "Tags which were not altered should be the same. " +
+                   "Tags which were not altered should be the same. " \
                    "#{a_tags.inspect} != #{b_tags.inspect}"
     end
   end

test/controllers/user_controller_test.rb

@@ -778,7 +778,7 @@ class UserControllerTest < ActionController::TestCase
     # you are not logged in
     get :account, :params => { :display_name => user.display_name }
     assert_response :redirect
-    assert_redirected_to :controller => :user, :action => "login", :referer => "/user/#{URI.encode(user.display_name)}/account"
+    assert_redirected_to :controller => :user, :action => "login", :referer => "/user/#{ERB::Util.u(user.display_name)}/account"
 
     # Make sure that you are blocked when not logged in as the right user
     get :account, :params => { :display_name => user.display_name }, :session => { :user => create(:user) }
@@ -791,7 +791,7 @@ class UserControllerTest < ActionController::TestCase
     assert_select "form#accountForm" do |form|
       assert_equal "post", form.attr("method").to_s
       assert_select "input[name='_method']", false
-      assert_equal "/user/#{URI.encode(user.display_name)}/account", form.attr("action").to_s
+      assert_equal "/user/#{ERB::Util.u(user.display_name)}/account", form.attr("action").to_s
     end
 
     # Updating the description should work
@@ -859,7 +859,7 @@ class UserControllerTest < ActionController::TestCase
     # Adding external authentication should redirect to the auth provider
     post :account, :params => { :display_name => user.display_name, :user => user.attributes.merge(:auth_provider => "openid", :auth_uid => "gmail.com") }, :session => { :user => user }
     assert_response :redirect
-    assert_redirected_to auth_path(:provider => "openid", :openid_url => "https://www.google.com/accounts/o8/id", :origin => "/user/#{URI.encode(user.display_name)}/account")
+    assert_redirected_to auth_path(:provider => "openid", :openid_url => "https://www.google.com/accounts/o8/id", :origin => "/user/#{ERB::Util.u(user.display_name)}/account")
 
     # Changing name to one that exists should fail
     new_attributes = user.attributes.dup.merge(:display_name => create(:user).display_name)
@@ -941,14 +941,14 @@ class UserControllerTest < ActionController::TestCase
     get :view, :params => { :display_name => user.display_name }
     assert_response :success
     assert_select "div#userinformation" do
-      assert_select "a[href^='/user/#{URI.encode(user.display_name)}/history']", 1
+      assert_select "a[href^='/user/#{ERB::Util.u(user.display_name)}/history']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/traces']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/traces']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary/comments']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/account']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/account']", 0
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks_by']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0
-      assert_select "a[href='/blocks/new/#{URI.encode(user.display_name)}']", 0
+      assert_select "a[href='/blocks/new/#{ERB::Util.u(user.display_name)}']", 0
     end
 
     # Test a user who has been blocked
@@ -957,14 +957,14 @@ class UserControllerTest < ActionController::TestCase
     get :view, :params => { :display_name => blocked_user.display_name }
     assert_response :success
     assert_select "div#userinformation" do
-      assert_select "a[href^='/user/#{URI.encode(blocked_user.display_name)}/history']", 1
+      assert_select "a[href^='/user/#{ERB::Util.u(blocked_user.display_name)}/history']", 1
-      assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/traces']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/traces']", 1
-      assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/diary']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/diary']", 1
-      assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/diary/comments']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/diary/comments']", 1
-      assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/account']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/account']", 0
-      assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/blocks']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/blocks']", 1
-      assert_select "a[href='/user/#{URI.encode(blocked_user.display_name)}/blocks_by']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/blocks_by']", 0
-      assert_select "a[href='/blocks/new/#{URI.encode(blocked_user.display_name)}']", 0
+      assert_select "a[href='/blocks/new/#{ERB::Util.u(blocked_user.display_name)}']", 0
     end
 
     # Test a moderator who has applied blocks
@@ -973,14 +973,14 @@ class UserControllerTest < ActionController::TestCase
     get :view, :params => { :display_name => moderator_user.display_name }
     assert_response :success
     assert_select "div#userinformation" do
-      assert_select "a[href^='/user/#{URI.encode(moderator_user.display_name)}/history']", 1
+      assert_select "a[href^='/user/#{ERB::Util.u(moderator_user.display_name)}/history']", 1
-      assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/traces']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/traces']", 1
-      assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/diary']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/diary']", 1
-      assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/diary/comments']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/diary/comments']", 1
-      assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/account']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/account']", 0
-      assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/blocks']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/blocks']", 0
-      assert_select "a[href='/user/#{URI.encode(moderator_user.display_name)}/blocks_by']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/blocks_by']", 1
-      assert_select "a[href='/blocks/new/#{URI.encode(moderator_user.display_name)}']", 0
+      assert_select "a[href='/blocks/new/#{ERB::Util.u(moderator_user.display_name)}']", 0
     end
 
     # Login as a normal user
@@ -990,14 +990,14 @@ class UserControllerTest < ActionController::TestCase
     get :view, :params => { :display_name => user.display_name }
     assert_response :success
     assert_select "div#userinformation" do
-      assert_select "a[href^='/user/#{URI.encode(user.display_name)}/history']", 1
+      assert_select "a[href^='/user/#{ERB::Util.u(user.display_name)}/history']", 1
       assert_select "a[href='/traces/mine']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary/comments']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/account']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/account']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks_by']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0
-      assert_select "a[href='/blocks/new/#{URI.encode(user.display_name)}']", 0
+      assert_select "a[href='/blocks/new/#{ERB::Util.u(user.display_name)}']", 0
     end
 
     # Login as a moderator
@@ -1007,14 +1007,14 @@ class UserControllerTest < ActionController::TestCase
     get :view, :params => { :display_name => user.display_name }
     assert_response :success
     assert_select "div#userinformation" do
-      assert_select "a[href^='/user/#{URI.encode(user.display_name)}/history']", 1
+      assert_select "a[href^='/user/#{ERB::Util.u(user.display_name)}/history']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/traces']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/traces']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/diary/comments']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/account']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/account']", 0
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/blocks_by']", 0
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0
-      assert_select "a[href='/blocks/new/#{URI.encode(user.display_name)}']", 1
+      assert_select "a[href='/blocks/new/#{ERB::Util.u(user.display_name)}']", 1
     end
   end
 

test/controllers/way_controller_test.rb

@@ -128,8 +128,8 @@ class WayControllerTest < ActionController::TestCase
     changeset_id = private_changeset.id
 
     # create a way with pre-existing nodes
-    content "<osm><way changeset='#{changeset_id}'>" +
+    content "<osm><way changeset='#{changeset_id}'>" \
-            "<nd ref='#{node1.id}'/><nd ref='#{node2.id}'/>" +
+            "<nd ref='#{node1.id}'/><nd ref='#{node2.id}'/>" \
             "<tag k='test' v='yes' /></way></osm>"
     put :create
     # hope for failure
@@ -143,8 +143,8 @@ class WayControllerTest < ActionController::TestCase
     changeset_id = changeset.id
 
     # create a way with pre-existing nodes
-    content "<osm><way changeset='#{changeset_id}'>" +
+    content "<osm><way changeset='#{changeset_id}'>" \
-            "<nd ref='#{node1.id}'/><nd ref='#{node2.id}'/>" +
+            "<nd ref='#{node1.id}'/><nd ref='#{node2.id}'/>" \
             "<tag k='test' v='yes' /></way></osm>"
     put :create
     # hope for success
@@ -188,7 +188,7 @@ class WayControllerTest < ActionController::TestCase
 
     # use the first user's open changeset
     # create a way with non-existing node
-    content "<osm><way changeset='#{private_open_changeset.id}'>" +
+    content "<osm><way changeset='#{private_open_changeset.id}'>" \
             "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
     put :create
     # expect failure
@@ -196,7 +196,7 @@ class WayControllerTest < ActionController::TestCase
                     "way upload with invalid node using a private user did not return 'forbidden'"
 
     # create a way with no nodes
-    content "<osm><way changeset='#{private_open_changeset.id}'>" +
+    content "<osm><way changeset='#{private_open_changeset.id}'>" \
             "<tag k='test' v='yes' /></way></osm>"
     put :create
     # expect failure
@@ -204,7 +204,7 @@ class WayControllerTest < ActionController::TestCase
                     "way upload with no node using a private userdid not return 'forbidden'"
 
     # create a way inside a closed changeset
-    content "<osm><way changeset='#{private_closed_changeset.id}'>" +
+    content "<osm><way changeset='#{private_closed_changeset.id}'>" \
             "<nd ref='#{node.id}'/></way></osm>"
     put :create
     # expect failure
@@ -216,7 +216,7 @@ class WayControllerTest < ActionController::TestCase
 
     # use the first user's open changeset
     # create a way with non-existing node
-    content "<osm><way changeset='#{open_changeset.id}'>" +
+    content "<osm><way changeset='#{open_changeset.id}'>" \
             "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
     put :create
     # expect failure
@@ -225,7 +225,7 @@ class WayControllerTest < ActionController::TestCase
     assert_equal "Precondition failed: Way  requires the nodes with id in (0), which either do not exist, or are not visible.", @response.body
 
     # create a way with no nodes
-    content "<osm><way changeset='#{open_changeset.id}'>" +
+    content "<osm><way changeset='#{open_changeset.id}'>" \
             "<tag k='test' v='yes' /></way></osm>"
     put :create
     # expect failure
@@ -234,7 +234,7 @@ class WayControllerTest < ActionController::TestCase
     assert_equal "Precondition failed: Cannot create way: data is invalid.", @response.body
 
     # create a way inside a closed changeset
-    content "<osm><way changeset='#{closed_changeset.id}'>" +
+    content "<osm><way changeset='#{closed_changeset.id}'>" \
             "<nd ref='#{node.id}'/></way></osm>"
     put :create
     # expect failure
@@ -242,9 +242,9 @@ class WayControllerTest < ActionController::TestCase
                     "way upload to closed changeset did not return 'conflict'"
 
     # create a way with a tag which is too long
-    content "<osm><way changeset='#{open_changeset.id}'>" +
+    content "<osm><way changeset='#{open_changeset.id}'>" \
-            "<nd ref='#{node.id}'/>" +
+            "<nd ref='#{node.id}'/>" \
-            "<tag k='foo' v='#{'x' * 256}'/>" +
+            "<tag k='foo' v='#{'x' * 256}'/>" \
             "</way></osm>"
     put :create
     # expect failure

test/helpers/changeset_helper_test.rb

@@ -3,7 +3,7 @@ require "test_helper"
 class ChangesetHelperTest < ActionView::TestCase
   def test_changeset_user_link
     changeset = create(:changeset)
-    assert_equal %(<a href="/user/#{URI.encode(changeset.user.display_name)}">#{changeset.user.display_name}</a>), changeset_user_link(changeset)
+    assert_equal %(<a href="/user/#{ERB::Util.u(changeset.user.display_name)}">#{changeset.user.display_name}</a>), changeset_user_link(changeset)
 
     changeset = create(:changeset, :user => create(:user, :data_public => false))
     assert_equal "anonymous", changeset_user_link(changeset)
@@ -20,7 +20,7 @@ class ChangesetHelperTest < ActionView::TestCase
     assert_match %r{^Created <abbr title='Mon, 01 Jan 2007 00:00:00 \+0000'>.*</abbr> by anonymous$}, changeset_details(changeset)
 
     changeset = create(:changeset, :created_at => Time.utc(2007, 1, 1, 0, 0, 0), :closed_at => Time.utc(2007, 1, 2, 0, 0, 0))
-    user_link = %(<a href="/user/#{URI.encode(changeset.user.display_name)}">#{changeset.user.display_name}</a>)
+    user_link = %(<a href="/user/#{ERB::Util.u(changeset.user.display_name)}">#{changeset.user.display_name}</a>)
 
     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 #{user_link}$}, changeset_details(changeset)
   end

test/helpers/note_helper_test.rb

@@ -9,7 +9,7 @@ class NoteHelperTest < ActionView::TestCase
     user = create(:user)
 
     assert_match %r{^Created by anonymous <abbr title='Wed, 05 Mar 2014 21:37:45 \+0000'><span title=" 5 March 2014 at 21:37">.*</span> ago</abbr>$}, note_event("open", date, nil)
-    assert_match %r{^Resolved by <a href="/user/#{URI.encode(user.display_name)}">#{user.display_name}</a> <abbr title='Wed, 05 Mar 2014 21:37:45 \+0000'><span title=" 5 March 2014 at 21:37">.*</span> ago</abbr>$}, note_event("closed", date, user)
+    assert_match %r{^Resolved by <a href="/user/#{ERB::Util.u(user.display_name)}">#{user.display_name}</a> <abbr title='Wed, 05 Mar 2014 21:37:45 \+0000'><span title=" 5 March 2014 at 21:37">.*</span> ago</abbr>$}, note_event("closed", date, user)
   end
 
   def test_note_author
@@ -18,7 +18,7 @@ class NoteHelperTest < ActionView::TestCase
 
     assert_equal "", note_author(nil)
     assert_equal "deleted", note_author(deleted_user)
-    assert_equal "<a href=\"/user/#{URI.encode(user.display_name)}\">#{user.display_name}</a>", note_author(user)
+    assert_equal "<a href=\"/user/#{ERB::Util.u(user.display_name)}\">#{user.display_name}</a>", note_author(user)
-    assert_equal "<a href=\"http://test.host/user/#{URI.encode(user.display_name)}\">#{user.display_name}</a>", note_author(user, :only_path => false)
+    assert_equal "<a href=\"http://test.host/user/#{ERB::Util.u(user.display_name)}\">#{user.display_name}</a>", note_author(user, :only_path => false)
   end
 end

test/helpers/title_helper_test.rb

@@ -1,4 +1,3 @@
-# coding: utf-8
 
 require "test_helper"
 
@@ -17,11 +16,11 @@ class TitleHelperTest < ActionView::TestCase
     assert_equal "Test Title", @title
 
     set_title("Test & Title")
-    assert_equal "OpenStreetMap%20%7C%20Test%20&%20Title", response.header["X-Page-Title"]
+    assert_equal "OpenStreetMap%20%7C%20Test%20%26%20Title", response.header["X-Page-Title"]
     assert_equal "Test & Title", @title
 
     set_title("Tést & Tïtlè")
-    assert_equal "OpenStreetMap%20%7C%20T%C3%A9st%20&%20T%C3%AFtl%C3%A8", response.header["X-Page-Title"]
+    assert_equal "OpenStreetMap%20%7C%20T%C3%A9st%20%26%20T%C3%AFtl%C3%A8", response.header["X-Page-Title"]
     assert_equal "Tést & Tïtlè", @title
   end
 end

test/helpers/user_roles_helper_test.rb

@@ -18,11 +18,11 @@ class UserRolesHelperTest < ActionView::TestCase
 
     user = create(:user)
     icon = role_icon(user, "moderator")
-    assert_dom_equal %(<a confirm="Are you sure you want to grant the role `moderator&#39; to the user `#{user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{URI.encode(user.display_name)}/role/moderator/grant"><picture><source srcset="/images/roles/blank_moderator.svg" type="image/svg+xml" /><img srcset="/images/roles/blank_moderator.svg" border="0" alt="Grant moderator access" title="Grant moderator access" src="/images/roles/blank_moderator.png" width="20" height="20" /></picture></a>), icon
+    assert_dom_equal %(<a confirm="Are you sure you want to grant the role `moderator&#39; to the user `#{user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{ERB::Util.u(user.display_name)}/role/moderator/grant"><picture><source srcset="/images/roles/blank_moderator.svg" type="image/svg+xml" /><img srcset="/images/roles/blank_moderator.svg" border="0" alt="Grant moderator access" title="Grant moderator access" src="/images/roles/blank_moderator.png" width="20" height="20" /></picture></a>), icon
 
     moderator_user = create(:moderator_user)
     icon = role_icon(moderator_user, "moderator")
-    assert_dom_equal %(<a confirm="Are you sure you want to revoke the role `moderator&#39; from the user `#{moderator_user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{URI.encode(moderator_user.display_name)}/role/moderator/revoke"><picture><source srcset="/images/roles/moderator.svg" type="image/svg+xml" /><img srcset="/images/roles/moderator.svg" border="0" alt="Revoke moderator access" title="Revoke moderator access" src="/images/roles/moderator.png" width="20" height="20" /></picture></a>), icon
+    assert_dom_equal %(<a confirm="Are you sure you want to revoke the role `moderator&#39; from the user `#{moderator_user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{ERB::Util.u(moderator_user.display_name)}/role/moderator/revoke"><picture><source srcset="/images/roles/moderator.svg" type="image/svg+xml" /><img srcset="/images/roles/moderator.svg" border="0" alt="Revoke moderator access" title="Revoke moderator access" src="/images/roles/moderator.png" width="20" height="20" /></picture></a>), icon
   end
 
   def test_role_icons_normal
@@ -43,14 +43,14 @@ class UserRolesHelperTest < ActionView::TestCase
 
     user = create(:user)
     icons = role_icons(user)
-    assert_dom_equal %( <a confirm="Are you sure you want to grant the role `administrator&#39; to the user `#{user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{URI.encode(user.display_name)}/role/administrator/grant"><picture><source srcset="/images/roles/blank_administrator.svg" type="image/svg+xml" /><img srcset="/images/roles/blank_administrator.svg" border="0" alt="Grant administrator access" title="Grant administrator access" src="/images/roles/blank_administrator.png" width="20" height="20" /></picture></a> <a confirm="Are you sure you want to grant the role `moderator&#39; to the user `#{user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{URI.encode(user.display_name)}/role/moderator/grant"><picture><source srcset="/images/roles/blank_moderator.svg" type="image/svg+xml" /><img srcset="/images/roles/blank_moderator.svg" border="0" alt="Grant moderator access" title="Grant moderator access" src="/images/roles/blank_moderator.png" width="20" height="20" /></picture></a>), icons
+    assert_dom_equal %( <a confirm="Are you sure you want to grant the role `administrator&#39; to the user `#{user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{ERB::Util.u(user.display_name)}/role/administrator/grant"><picture><source srcset="/images/roles/blank_administrator.svg" type="image/svg+xml" /><img srcset="/images/roles/blank_administrator.svg" border="0" alt="Grant administrator access" title="Grant administrator access" src="/images/roles/blank_administrator.png" width="20" height="20" /></picture></a> <a confirm="Are you sure you want to grant the role `moderator&#39; to the user `#{user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{ERB::Util.u(user.display_name)}/role/moderator/grant"><picture><source srcset="/images/roles/blank_moderator.svg" type="image/svg+xml" /><img srcset="/images/roles/blank_moderator.svg" border="0" alt="Grant moderator access" title="Grant moderator access" src="/images/roles/blank_moderator.png" width="20" height="20" /></picture></a>), icons
 
     moderator_user = create(:moderator_user)
     icons = role_icons(moderator_user)
-    assert_dom_equal %( <a confirm="Are you sure you want to grant the role `administrator&#39; to the user `#{moderator_user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{URI.encode(moderator_user.display_name)}/role/administrator/grant"><picture><source srcset="/images/roles/blank_administrator.svg" type="image/svg+xml" /><img srcset="/images/roles/blank_administrator.svg" border="0" alt="Grant administrator access" title="Grant administrator access" src="/images/roles/blank_administrator.png" width="20" height="20" /></picture></a> <a confirm="Are you sure you want to revoke the role `moderator&#39; from the user `#{moderator_user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{URI.encode(moderator_user.display_name)}/role/moderator/revoke"><picture><source srcset="/images/roles/moderator.svg" type="image/svg+xml" /><img srcset="/images/roles/moderator.svg" border="0" alt="Revoke moderator access" title="Revoke moderator access" src="/images/roles/moderator.png" width="20" height="20" /></picture></a>), icons
+    assert_dom_equal %( <a confirm="Are you sure you want to grant the role `administrator&#39; to the user `#{moderator_user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{ERB::Util.u(moderator_user.display_name)}/role/administrator/grant"><picture><source srcset="/images/roles/blank_administrator.svg" type="image/svg+xml" /><img srcset="/images/roles/blank_administrator.svg" border="0" alt="Grant administrator access" title="Grant administrator access" src="/images/roles/blank_administrator.png" width="20" height="20" /></picture></a> <a confirm="Are you sure you want to revoke the role `moderator&#39; from the user `#{moderator_user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{ERB::Util.u(moderator_user.display_name)}/role/moderator/revoke"><picture><source srcset="/images/roles/moderator.svg" type="image/svg+xml" /><img srcset="/images/roles/moderator.svg" border="0" alt="Revoke moderator access" title="Revoke moderator access" src="/images/roles/moderator.png" width="20" height="20" /></picture></a>), icons
 
     super_user = create(:super_user)
     icons = role_icons(super_user)
-    assert_dom_equal %( <a confirm="Are you sure you want to revoke the role `administrator&#39; from the user `#{super_user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{URI.encode(super_user.display_name)}/role/administrator/revoke"><picture><source srcset="/images/roles/administrator.svg" type="image/svg+xml" /><img srcset="/images/roles/administrator.svg" border="0" alt="Revoke administrator access" title="Revoke administrator access" src="/images/roles/administrator.png" width="20" height="20" /></picture></a> <a confirm="Are you sure you want to revoke the role `moderator&#39; from the user `#{super_user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{URI.encode(super_user.display_name)}/role/moderator/revoke"><picture><source srcset="/images/roles/moderator.svg" type="image/svg+xml" /><img srcset="/images/roles/moderator.svg" border="0" alt="Revoke moderator access" title="Revoke moderator access" src="/images/roles/moderator.png" width="20" height="20" /></picture></a>), icons
+    assert_dom_equal %( <a confirm="Are you sure you want to revoke the role `administrator&#39; from the user `#{super_user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{ERB::Util.u(super_user.display_name)}/role/administrator/revoke"><picture><source srcset="/images/roles/administrator.svg" type="image/svg+xml" /><img srcset="/images/roles/administrator.svg" border="0" alt="Revoke administrator access" title="Revoke administrator access" src="/images/roles/administrator.png" width="20" height="20" /></picture></a> <a confirm="Are you sure you want to revoke the role `moderator&#39; from the user `#{super_user.display_name}&#39;?" rel="nofollow" data-method="post" href="/user/#{ERB::Util.u(super_user.display_name)}/role/moderator/revoke"><picture><source srcset="/images/roles/moderator.svg" type="image/svg+xml" /><img srcset="/images/roles/moderator.svg" border="0" alt="Revoke moderator access" title="Revoke moderator access" src="/images/roles/moderator.png" width="20" height="20" /></picture></a>), icons
   end
 end

test/integration/client_applications_test.rb

@@ -12,35 +12,35 @@ class ClientApplicationsTest < ActionDispatch::IntegrationTest
     assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true"
     follow_redirect!
     assert_response :success
-    post "/login", :params => { "username" => user.email, "password" => "test", :referer => "/user/#{URI.encode(user.display_name)}" }
+    post "/login", :params => { "username" => user.email, "password" => "test", :referer => "/user/#{ERB::Util.u(user.display_name)}" }
     assert_response :redirect
     follow_redirect!
     assert_response :success
     assert_template "user/view"
-    get "/user/#{URI.encode(user.display_name)}/account"
+    get "/user/#{ERB::Util.u(user.display_name)}/account"
     assert_response :success
     assert_template "user/account"
 
     # check that the form to allow new client application creations exists
     assert_in_heading do
-      assert_select "ul.secondary-actions li a[href='/user/#{URI.encode(user.display_name)}/oauth_clients']"
+      assert_select "ul.secondary-actions li a[href='/user/#{ERB::Util.u(user.display_name)}/oauth_clients']"
     end
 
     # now we follow the link to the oauth client list
-    get "/user/#{URI.encode(user.display_name)}/oauth_clients"
+    get "/user/#{ERB::Util.u(user.display_name)}/oauth_clients"
     assert_response :success
     assert_in_body do
-      assert_select "a[href='/user/#{URI.encode(user.display_name)}/oauth_clients/new']"
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/oauth_clients/new']"
     end
 
     # now we follow the link to the new oauth client page
-    get "/user/#{URI.encode(user.display_name)}/oauth_clients/new"
+    get "/user/#{ERB::Util.u(user.display_name)}/oauth_clients/new"
     assert_response :success
     assert_in_heading do
       assert_select "h1", "Register a new application"
     end
     assert_in_body do
-      assert_select "form[action='/user/#{URI.encode(user.display_name)}/oauth_clients']" do
+      assert_select "form[action='/user/#{ERB::Util.u(user.display_name)}/oauth_clients']" do
         [:name, :url, :callback_url, :support_url].each do |inp|
           assert_select "input[name=?]", "client_application[#{inp}]"
         end
@@ -50,7 +50,7 @@ class ClientApplicationsTest < ActionDispatch::IntegrationTest
       end
     end
 
-    post "/user/#{URI.encode(user.display_name)}/oauth_clients",
+    post "/user/#{ERB::Util.u(user.display_name)}/oauth_clients",
          :params => { "client_application[name]" => "My New App",
                       "client_application[url]" => "http://my.new.app.org/",
                       "client_application[callback_url]" => "http://my.new.app.org/callback",
@@ -62,7 +62,7 @@ class ClientApplicationsTest < ActionDispatch::IntegrationTest
     assert_equal "Registered the information successfully", flash[:notice]
 
     # now go back to the account page and check its listed under this user
-    get "/user/#{URI.encode(user.display_name)}/oauth_clients"
+    get "/user/#{ERB::Util.u(user.display_name)}/oauth_clients"
     assert_response :success
     assert_template "oauth_clients/index"
     assert_in_body { assert_select "div>a", "My New App" }

test/integration/user_roles_test.rb

@@ -32,7 +32,7 @@ class UserRolesTest < ActionDispatch::IntegrationTest
     assert_response :success
 
     target_user = create(:user)
-    post "/user/#{URI.encode(target_user.display_name)}/role/#{role}/#{action}"
+    post "/user/#{ERB::Util.u(target_user.display_name)}/role/#{role}/#{action}"
     assert_redirected_to :controller => "user", :action => "view", :display_name => target_user.display_name
 
     reset!
@@ -50,7 +50,7 @@ class UserRolesTest < ActionDispatch::IntegrationTest
     assert_response :success
 
     target_user = create(:user)
-    post "/user/#{URI.encode(target_user.display_name)}/role/#{role}/#{action}"
+    post "/user/#{ERB::Util.u(target_user.display_name)}/role/#{role}/#{action}"
     assert_redirected_to :controller => "user", :action => "view", :display_name => target_user.display_name
 
     reset!

test/lib/bounding_box_test.rb

@@ -306,7 +306,7 @@ class BoundingBoxTest < ActiveSupport::TestCase
 
   def check_expand(bbox, array_string, margin = 0, result = nil)
     array = array_string.split(",").collect(&:to_f)
-    result = array unless result
+    result ||= array
     bbox.expand!(BoundingBox.new(array[0], array[1], array[2], array[3]), margin)
     check_bbox(bbox, result)
   end

test/lib/i18n_test.rb

@@ -13,7 +13,7 @@ class I18nTest < ActiveSupport::TestCase
         if default_value.is_a?(Hash)
           variables.push("count")
 
-          default_value.each do |_subkey, subvalue|
+          default_value.each_value do |subvalue|
             subvalue.scan(/%\{(\w+)\}/) do
               variables.push(Regexp.last_match(1))
             end

test/models/language_test.rb

@@ -1,5 +1,3 @@
-# coding: utf-8
-
 require "test_helper"
 
 class LanguageTest < ActiveSupport::TestCase