From: Tom Hughes <tom@compton.nu>
Date: Thu, 15 Sep 2016 21:41:07 +0000 (+0100)
Subject: Fix some rubocop warnings
X-Git-Tag: live~3741
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/5f4dcd34ff5943ff4de39bc8322013376a6de8b9

Fix some rubocop warnings
---

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 3a3008e9e..8d81a7c56 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -164,17 +164,6 @@ Style/LineEndConcatenation:
 Style/NumericLiterals:
   MinDigits: 11
 
-# Offense count: 38
-# Cop supports --auto-correct.
-Style/PerlBackrefs:
-  Exclude:
-    - 'app/controllers/amf_controller.rb'
-    - 'app/helpers/browse_helper.rb'
-    - 'config/initializers/paperclip.rb'
-    - 'lib/id.rb'
-    - 'lib/potlatch.rb'
-    - 'test/lib/i18n_test.rb'
-
 # Offense count: 8
 # Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist.
 # NamePrefix: is_, has_, have_
@@ -202,15 +191,3 @@ Style/RaiseArgs:
 Style/RescueModifier:
   Exclude:
     - 'app/helpers/browse_helper.rb'
-
-# Offense count: 8
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-# SupportedStyles: single_quotes, double_quotes
-Style/StringLiteralsInInterpolation:
-  Exclude:
-    - 'app/controllers/amf_controller.rb'
-    - 'app/models/node.rb'
-    - 'app/models/way.rb'
-    - 'lib/migrate.rb'
-    - 'test/controllers/changeset_controller_test.rb'
diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb
index ac6da0566..632974f4f 100644
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -868,7 +868,7 @@ class AmfController < ApplicationController
 
   def getuser(token) #:doc:
     if token =~ /^(.+)\:(.+)$/
-      User.authenticate(:username => $1, :password => $2)
+      User.authenticate(:username => Regexp.last_match(1), :password => Regexp.last_match(2))
     else
       User.authenticate(:token => token)
     end
@@ -914,7 +914,7 @@ class AmfController < ApplicationController
     INNER JOIN current_ways  ON current_ways.id =current_way_nodes.id
        WHERE current_nodes.visible=TRUE
        AND current_ways.visible=TRUE
-       AND #{OSM.sql_for_area(bbox, "current_nodes.")}
+       AND #{OSM.sql_for_area(bbox, 'current_nodes.')}
     EOF
     ActiveRecord::Base.connection.select_all(sql).collect { |a| [a["wayid"].to_i, a["version"].to_i] }
   end
@@ -927,7 +927,7 @@ class AmfController < ApplicationController
        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.")}
+       AND #{OSM.sql_for_area(bbox, 'current_nodes.')}
     EOF
     ActiveRecord::Base.connection.select_all(sql).each do |row|
       poitags = {}
@@ -947,7 +947,7 @@ class AmfController < ApplicationController
       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.")}
+       WHERE #{OSM.sql_for_area(bbox, 'cn.')}
       EOF
     unless way_ids.empty?
       sql += <<-EOF
diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb
index 76a6d5eb1..ef7b3f554 100644
--- a/app/helpers/browse_helper.rb
+++ b/app/helpers/browse_helper.rb
@@ -133,14 +133,14 @@ module BrowseHelper
       lang = if value =~ /^([a-z-]{2,12}):(.+)$/i
                # Value is <lang>:<title> so split it up
                # Note that value is always left as-is, see: https://trac.openstreetmap.org/ticket/4315
-               $1
+               Regexp.last_match(1)
              else
                # Value is <title> so default to English Wikipedia
                "en"
              end
     elsif key =~ /^wikipedia:(\S+)$/
       # Language is in the key, so assume value is the title
-      lang = $1
+      lang = Regexp.last_match(1)
     else
       # Not a wikipedia key!
       return nil
@@ -149,9 +149,9 @@ module BrowseHelper
     if value =~ /^([^#]*)#(.*)/
       # Contains a reference to a section of the wikipedia article
       # Must break it up to correctly build the url
-      value = $1
-      section = "#" + $2
-      encoded_section = "#" + URI.encode($2.gsub(/ +/, "_"), /[^A-Za-z0-9:_]/).tr("%", ".")
+      value = Regexp.last_match(1)
+      section = "#" + Regexp.last_match(2)
+      encoded_section = "#" + URI.encode(Regexp.last_match(2).gsub(/ +/, "_"), /[^A-Za-z0-9:_]/).tr("%", ".")
     else
       section = ""
       encoded_section = ""
diff --git a/app/models/node.rb b/app/models/node.rb
index a5becbf4d..578735688 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -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.new("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.new("Node #{id} is still used by relations #{rels.collect(&:id).join(',')}.") unless rels.empty?
 
       self.changeset_id = new_node.changeset_id
       self.tags = {}
diff --git a/app/models/way.rb b/app/models/way.rb
index b31ffc1d1..ecbdd2f9c 100644
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -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.new("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
diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb
index e27c46ea7..4db3166ae 100644
--- a/config/initializers/paperclip.rb
+++ b/config/initializers/paperclip.rb
@@ -6,7 +6,7 @@ module Paperclip
       url = super(style_name, options)
 
       if url =~ %r{^/assets/(.*)$}
-        asset_path($1)
+        asset_path(Regexp.last_match(1))
       else
         url
       end
diff --git a/lib/id.rb b/lib/id.rb
index 858bc53c3..4e6cf3b7c 100644
--- a/lib/id.rb
+++ b/lib/id.rb
@@ -1,3 +1,3 @@
 module ID
-  LOCALES = Locale.list(Rails.root.join("vendor/assets/iD/iD/locales").entries.map { |p| p.basename.to_s[/(.*).json/] && $1 }.compact)
+  LOCALES = Locale.list(Rails.root.join("vendor/assets/iD/iD/locales").entries.map { |p| p.basename.to_s[/(.*).json/] && Regexp.last_match(1) }.compact)
 end
diff --git a/lib/migrate.rb b/lib/migrate.rb
index 9008b0a9f..21c1b2cf2 100644
--- a/lib/migrate.rb
+++ b/lib/migrate.rb
@@ -29,7 +29,7 @@ module ActiveRecord
         end
 
         def alter_primary_key(table_name, new_columns)
-          execute "ALTER TABLE #{quote_table_name(table_name)} DROP CONSTRAINT #{quote_table_name(table_name + "_pkey")}"
+          execute "ALTER TABLE #{quote_table_name(table_name)} DROP CONSTRAINT #{quote_table_name(table_name + '_pkey')}"
           execute "ALTER TABLE #{quote_table_name(table_name)} ADD PRIMARY KEY (#{quote_column_name(new_columns)})"
         end
 
diff --git a/lib/potlatch.rb b/lib/potlatch.rb
index f7661e39d..4b56414a4 100644
--- a/lib/potlatch.rb
+++ b/lib/potlatch.rb
@@ -182,17 +182,17 @@ module Potlatch
         file.each_line do |line|
           t = line.chomp
           if t =~ %r{(\w+)/(\w+)}
-            presettype = $1
-            presetcategory = $2
+            presettype = Regexp.last_match(1)
+            presetcategory = Regexp.last_match(2)
             presetmenus[presettype].push(presetcategory)
             presetnames[presettype][presetcategory] = ["(no preset)"]
           elsif t =~ /^([\w\s]+):\s?(.+)$/
-            pre = $1
-            kv = $2
+            pre = Regexp.last_match(1)
+            kv = Regexp.last_match(2)
             presetnames[presettype][presetcategory].push(pre)
             presets[pre] = {}
             kv.split(",").each do |a|
-              presets[pre][$1] = $2 if a =~ /^(.+)=(.*)$/
+              presets[pre][Regexp.last_match(1)] = Regexp.last_match(2) if a =~ /^(.+)=(.*)$/
             end
           end
         end
@@ -206,10 +206,10 @@ module Potlatch
         file.each_line do |line|
           next unless line.chomp =~ /(\w+)\s+([^\s]+)\s+([^\s]+)\s+([^\s]+)/
 
-          tag = $1
-          colours[tag] = $2.hex if $2 != "-"
-          casing[tag] = $3.hex if $3 != "-"
-          areas[tag] = $4.hex if $4 != "-"
+          tag = Regexp.last_match(1)
+          colours[tag] = Regexp.last_match(2).hex if Regexp.last_match(2) != "-"
+          casing[tag] = Regexp.last_match(3).hex if Regexp.last_match(3) != "-"
+          areas[tag] = Regexp.last_match(4).hex if Regexp.last_match(4) != "-"
         end
       end
 
@@ -221,10 +221,10 @@ module Potlatch
         file.each_line do |line|
           next unless line.chomp =~ /(\w+)\s+([^\s]+)\s+([^\s]+)\s+([^\s]+)/
 
-          tag = $1
-          relcolours[tag] = $2.hex if $2 != "-"
-          relalphas[tag] = $3.to_i if $3 != "-"
-          relwidths[tag] = $4.to_i if $4 != "-"
+          tag = Regexp.last_match(1)
+          relcolours[tag] = Regexp.last_match(2).hex if Regexp.last_match(2) != "-"
+          relalphas[tag] = Regexp.last_match(3).to_i if Regexp.last_match(3) != "-"
+          relwidths[tag] = Regexp.last_match(4).to_i if Regexp.last_match(4) != "-"
         end
       end
 
@@ -246,9 +246,9 @@ module Potlatch
         file.each_line do |line|
           next unless line.chomp =~ %r{^([\w:]+)/(\w+)\s+(.+)$}
 
-          tag = $1
-          type = $2
-          values = $3
+          tag = Regexp.last_match(1)
+          type = Regexp.last_match(2)
+          values = Regexp.last_match(3)
           autotags[type][tag] = if values == "-"
                                   []
                                 else
diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb
index ecccb1f5f..e1960d4e8 100644
--- a/test/controllers/changeset_controller_test.rb
+++ b/test/controllers/changeset_controller_test.rb
@@ -675,7 +675,7 @@ EOF
 <osmChange>
  <create>
   <node id='-1' lon='0' lat='0' changeset='#{cs_id}'>
-   <tag k='foo' v='#{"x" * 256}'/>
+   <tag k='foo' v='#{'x' * 256}'/>
   </node>
  </create>
 </osmChange>
diff --git a/test/lib/i18n_test.rb b/test/lib/i18n_test.rb
index 09965e950..c18c2f346 100644
--- a/test/lib/i18n_test.rb
+++ b/test/lib/i18n_test.rb
@@ -15,12 +15,12 @@ class I18nTest < ActiveSupport::TestCase
 
           default_value.each do |_subkey, subvalue|
             subvalue.scan(/%\{(\w+)\}/) do
-              variables.push($1)
+              variables.push(Regexp.last_match(1))
             end
           end
         else
           default_value.scan(/%\{(\w+)\}/) do
-            variables.push($1)
+            variables.push(Regexp.last_match(1))
           end
         end
 
@@ -37,14 +37,14 @@ class I18nTest < ActiveSupport::TestCase
             next if subvalue.nil?
 
             subvalue.scan(/%\{(\w+)\}/) do
-              assert variables.include?($1), "#{key}.#{subkey} uses unknown interpolation variable #{$1}"
+              assert variables.include?(Regexp.last_match(1)), "#{key}.#{subkey} uses unknown interpolation variable #{Regexp.last_match(1)}"
             end
           end
         else
           assert value.is_a?(String), "#{key} is not a string"
 
           value.scan(/%\{(\w+)\}/) do
-            assert variables.include?($1), "#{key} uses unknown interpolation variable #{$1}"
+            assert variables.include?(Regexp.last_match(1)), "#{key} uses unknown interpolation variable #{Regexp.last_match(1)}"
           end
         end
       end