Fix some rubocop warnings

openstreetmap · Sep 15, 2016 · 5f4dcd3 · 5f4dcd3
1 parent fd9bf6b
commit 5f4dcd3
Show file tree

Hide file tree

Showing 11 changed files with 36 additions and 59 deletions.
diff --git 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
@@ -868,7 +868,7 @@ def deleteway(usertoken, changeset_id, way_id, way_version, deletednodes) #:doc:
 
   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 @@ def sql_find_ways_in_area(bbox)
     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 @@ def sql_find_pois_in_area(bbox)
        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 @@ def sql_find_relations_in_area_and_ways(bbox, way_ids)
       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
@@ -133,14 +133,14 @@ def wikipedia_link(key, value)
       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 @@ def wikipedia_link(key, value)
     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
@@ -120,10 +120,10 @@ def delete_with_history!(new_node, user)
       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
@@ -222,7 +222,7 @@ def delete_with_history!(new_way, user)
       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
@@ -6,7 +6,7 @@ def for(style_name, options)
       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
@@ -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
@@ -29,7 +29,7 @@ def remove_primary_key(table_name)
         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
@@ -182,17 +182,17 @@ def self.get_presets
         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 @@ def self.get_presets
         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 @@ def self.get_presets
         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 @@ def self.get_presets
         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
@@ -675,7 +675,7 @@ def test_upload_invalid_too_long_tag
 <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
@@ -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