Fix some more rubocop style issues

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 418e7265d05228b0771e945e86d150365608b1ad..e9561cd2d8f2147f3db73251c07da5ec3d60f1a4 100644 (file)
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -108,10 +108,6 @@ Style/LineEndConcatenation:
 Style/NumericLiterals:
   MinDigits: 11
 
 Style/NumericLiterals:
   MinDigits: 11
 

-# Offense count: 42
-Style/OneLineConditional:
-  Enabled: false
-

 # Offense count: 44
 # Cop supports --auto-correct.
 Style/PerlBackrefs:
 # Offense count: 44
 # Cop supports --auto-correct.
 Style/PerlBackrefs:


@@ -127,10 +123,6 @@ Style/PredicateName:
 Style/RaiseArgs:
   Enabled: false
 
 Style/RaiseArgs:
   Enabled: false
 

-# Offense count: 11
-Style/RegexpLiteral:
-  MaxSlashes: 5
-

 # Offense count: 2
 Style/RescueModifier:
   Enabled: false
 # Offense count: 2
 Style/RescueModifier:
   Enabled: false

diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb
index 6860056e7716036d49197097c3870d5dfc504fdc..6c3e2c5cf2d272f4ee120153acb83a1fda3142b7 100644 (file)
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -86,14 +86,14 @@ class AmfController < ApplicationController
           orn = renumberednodes.dup
           result = putway(renumberednodes, *args)
           result[4] = renumberednodes.reject { |k, _v| orn.key?(k) }
           orn = renumberednodes.dup
           result = putway(renumberednodes, *args)
           result[4] = renumberednodes.reject { |k, _v| orn.key?(k) }

-          if result[0] == 0 && result[2] != result[3] then renumberedways[result[2]] = result[3] end
+          renumberedways[result[2]] = result[3] if result[0] == 0 && result[2] != result[3]

         when "putrelation" then
           result = putrelation(renumberednodes, renumberedways, *args)
         when "deleteway" then
           result = deleteway(*args)
         when "putpoi" then
           result = putpoi(*args)
         when "putrelation" then
           result = putrelation(renumberednodes, renumberedways, *args)
         when "deleteway" then
           result = deleteway(*args)
         when "putpoi" then
           result = putpoi(*args)

-          if result[0] == 0 && result[2] != result[3] then renumberednodes[result[2]] = result[3] end
+          renumberednodes[result[2]] = result[3] if result[0] == 0 && result[2] != result[3]

         when "startchangeset" then
           result = startchangeset(*args)
         end
         when "startchangeset" then
           result = startchangeset(*args)
         end


@@ -137,12 +137,12 @@ class AmfController < ApplicationController
   def startchangeset(usertoken, cstags, closeid, closecomment, opennew)
     amf_handle_error("'startchangeset'", nil, nil) do
       user = getuser(usertoken)
   def startchangeset(usertoken, cstags, closeid, closecomment, opennew)
     amf_handle_error("'startchangeset'", nil, nil) do
       user = getuser(usertoken)

-      unless user then return -1, "You are not logged in, so Potlatch can't write any changes to the database." end
-      if user.blocks.active.exists? then return -1, t("application.setup_user_auth.blocked") end
-      if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? then return -1, "You must accept the contributor terms before you can edit." end
+      return -1, "You are not logged in, so Potlatch can't write any changes to the database." unless user
+      return -1, t("application.setup_user_auth.blocked") if user.blocks.active.exists?
+      return -1, "You must accept the contributor terms before you can edit." if REQUIRE_TERMS_AGREED && user.terms_agreed.nil?

 
       if cstags
 
       if cstags

-        unless tags_ok(cstags) then return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
+        return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(cstags)

         cstags = strip_non_xml_chars cstags
       end
 
         cstags = strip_non_xml_chars cstags
       end
 


@@ -413,11 +413,11 @@ class AmfController < ApplicationController
     revusers = {}
     Way.find(wayid).old_ways.unredacted.collect do |a|
       revdates.push(a.timestamp)
     revusers = {}
     Way.find(wayid).old_ways.unredacted.collect do |a|
       revdates.push(a.timestamp)

-      unless revusers.key?(a.timestamp.to_i) then revusers[a.timestamp.to_i] = change_user(a) end
+      revusers[a.timestamp.to_i] = change_user(a) unless revusers.key?(a.timestamp.to_i)

       a.nds.each do |n|
         Node.find(n).old_nodes.unredacted.collect do |o|
           revdates.push(o.timestamp)
       a.nds.each do |n|
         Node.find(n).old_nodes.unredacted.collect do |o|
           revdates.push(o.timestamp)

-          unless revusers.key?(o.timestamp.to_i) then revusers[o.timestamp.to_i] = change_user(o) end
+          revusers[o.timestamp.to_i] = change_user(o) unless revusers.key?(o.timestamp.to_i)

         end
       end
     end
         end
       end
     end


@@ -528,11 +528,12 @@ class AmfController < ApplicationController
   def putrelation(renumberednodes, renumberedways, usertoken, changeset_id, version, relid, tags, members, visible) #:doc:
     amf_handle_error("'putrelation' #{relid}", "relation", relid)  do
       user = getuser(usertoken)
   def putrelation(renumberednodes, renumberedways, usertoken, changeset_id, version, relid, tags, members, visible) #:doc:
     amf_handle_error("'putrelation' #{relid}", "relation", relid)  do
       user = getuser(usertoken)

-      unless user then return -1, "You are not logged in, so the relation could not be saved." end
-      if user.blocks.active.exists? then return -1, t("application.setup_user_auth.blocked") end
-      if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? then return -1, "You must accept the contributor terms before you can edit." end

 
 

-      unless tags_ok(tags) then return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
+      return -1, "You are not logged in, so the relation could not be saved." unless user
+      return -1, t("application.setup_user_auth.blocked") if user.blocks.active.exists?
+      return -1, "You must accept the contributor terms before you can edit." if REQUIRE_TERMS_AGREED && user.terms_agreed.nil?
+
+      return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(tags)

       tags = strip_non_xml_chars tags
 
       relid = relid.to_i
       tags = strip_non_xml_chars tags
 
       relid = relid.to_i


@@ -616,13 +617,13 @@ class AmfController < ApplicationController
       # -- Initialise
 
       user = getuser(usertoken)
       # -- Initialise
 
       user = getuser(usertoken)

-      unless user then return -1, "You are not logged in, so the way could not be saved." end
-      if user.blocks.active.exists? then return -1, t("application.setup_user_auth.blocked") end
-      if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? then return -1, "You must accept the contributor terms before you can edit." end
+      return -1, "You are not logged in, so the way could not be saved." unless user
+      return -1, t("application.setup_user_auth.blocked") if user.blocks.active.exists?
+      return -1, "You must accept the contributor terms before you can edit." if REQUIRE_TERMS_AGREED && user.terms_agreed.nil?

 
 

-      if pointlist.length < 2 then return -2, "Server error - way is only #{points.length} points long." end
+      return -2, "Server error - way is only #{points.length} points long." if pointlist.length < 2

 
 

-      unless tags_ok(attributes) then return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
+      return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(attributes)

       attributes = strip_non_xml_chars attributes
 
       originalway = originalway.to_i
       attributes = strip_non_xml_chars attributes
 
       originalway = originalway.to_i


@@ -639,8 +640,8 @@ class AmfController < ApplicationController
           id = a[2].to_i
           version = a[3].to_i
 
           id = a[2].to_i
           version = a[3].to_i
 

-          if id == 0  then return -2, "Server error - node with id 0 found in way #{originalway}." end
-          if lat == 90 then return -2, "Server error - node with latitude -90 found in way #{originalway}." end
+          return -2, "Server error - node with id 0 found in way #{originalway}." if id == 0
+          return -2, "Server error - node with latitude -90 found in way #{originalway}." if lat == 90

 
           id = renumberednodes[id]  if renumberednodes[id]
 
 
           id = renumberednodes[id]  if renumberednodes[id]
 


@@ -651,7 +652,7 @@ class AmfController < ApplicationController
           node.tags = a[4]
 
           # fixup node tags in a way as well
           node.tags = a[4]
 
           # fixup node tags in a way as well

-          unless tags_ok(node.tags) then return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
+          return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(node.tags)

           node.tags = strip_non_xml_chars node.tags
 
           node.tags.delete("created_by")
           node.tags = strip_non_xml_chars node.tags
 
           node.tags.delete("created_by")


@@ -724,11 +725,11 @@ class AmfController < ApplicationController
   def putpoi(usertoken, changeset_id, version, id, lon, lat, tags, visible) #:doc:
     amf_handle_error("'putpoi' #{id}", "node", id) do
       user = getuser(usertoken)
   def putpoi(usertoken, changeset_id, version, id, lon, lat, tags, visible) #:doc:
     amf_handle_error("'putpoi' #{id}", "node", id) do
       user = getuser(usertoken)

-      unless user then return -1, "You are not logged in, so the point could not be saved." end
-      if user.blocks.active.exists? then return -1, t("application.setup_user_auth.blocked") end
-      if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? then return -1, "You must accept the contributor terms before you can edit." end
+      return -1, "You are not logged in, so the point could not be saved." unless user
+      return -1, t("application.setup_user_auth.blocked") if user.blocks.active.exists?
+      return -1, "You must accept the contributor terms before you can edit." if REQUIRE_TERMS_AGREED && user.terms_agreed.nil?

 
 

-      unless tags_ok(tags) then return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
+      return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(tags)

       tags = strip_non_xml_chars tags
 
       id = id.to_i
       tags = strip_non_xml_chars tags
 
       id = id.to_i


@@ -739,8 +740,8 @@ class AmfController < ApplicationController
         if id > 0
           node = Node.find(id)
 
         if id > 0
           node = Node.find(id)
 

-          unless visible
-            unless node.ways.empty? then return -1, "Point #{id} has since become part of a way, so you cannot save it as a POI.", id, id, version end
+          unless visible || node.ways.empty?
+            return -1, "Point #{id} has since become part of a way, so you cannot save it as a POI.", id, id, version

           end
         end
         # We always need a new node, based on the data that has been sent to us
           end
         end
         # We always need a new node, based on the data that has been sent to us


@@ -808,9 +809,9 @@ class AmfController < ApplicationController
   def deleteway(usertoken, changeset_id, way_id, way_version, deletednodes) #:doc:
     amf_handle_error("'deleteway' #{way_id}", "way", way_id) do
       user = getuser(usertoken)
   def deleteway(usertoken, changeset_id, way_id, way_version, deletednodes) #:doc:
     amf_handle_error("'deleteway' #{way_id}", "way", way_id) do
       user = getuser(usertoken)

-      unless user then return -1, "You are not logged in, so the way could not be deleted." end
-      if user.blocks.active.exists? then return -1, t("application.setup_user_auth.blocked") end
-      if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? then return -1, "You must accept the contributor terms before you can edit." end
+      return -1, "You are not logged in, so the way could not be deleted." unless user
+      return -1, t("application.setup_user_auth.blocked") if user.blocks.active.exists?
+      return -1, "You must accept the contributor terms before you can edit." if REQUIRE_TERMS_AGREED && user.terms_agreed.nil?

 
       way_id = way_id.to_i
       nodeversions = {}
 
       way_id = way_id.to_i
       nodeversions = {}

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index 1a7f3810a0e1842a50f60b7ab1eb069badd5fab1..6cb0b60dba9f64c8f0d86cefcc61798b69c39330 100644 (file)
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -89,7 +89,7 @@ class UserController < ApplicationController
 
           begin
             uri = URI(session[:referer])
 
           begin
             uri = URI(session[:referer])

-            /map=(.*)\/(.*)\/(.*)/.match(uri.fragment) do |m|
+            %r{map=(.*)/(.*)/(.*)}.match(uri.fragment) do |m|

               editor = Rack::Utils.parse_query(uri.query).slice("editor")
               referer = welcome_path({ "zoom" => m[1],
                                        "lat" => m[2],
               editor = Rack::Utils.parse_query(uri.query).slice("editor")
               referer = welcome_path({ "zoom" => m[1],
                                        "lat" => m[2],


@@ -625,8 +625,8 @@ class UserController < ApplicationController
   # check if we trust an OpenID provider to return a verified
   # email, so that we can skpi verifying it ourselves
   def openid_email_verified(openid_url)
   # check if we trust an OpenID provider to return a verified
   # email, so that we can skpi verifying it ourselves
   def openid_email_verified(openid_url)

-    openid_url.match(/https:\/\/www.google.com\/accounts\/o8\/id?(.*)/) ||
-      openid_url.match(/https:\/\/me.yahoo.com\/(.*)/)
+    openid_url.match(%r{https://www.google.com/accounts/o8/id?(.*)}) ||
+      openid_url.match(%r{https://me.yahoo.com/(.*)})

   end
 
   ##
   end
 
   ##

diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb
index 4027ddb2f8f79fb54aeccd968f157ca84d42c434..793baf07b749a0abc14a090fbccf027f2d61340d 100644 (file)
--- a/app/helpers/browse_helper.rb
+++ b/app/helpers/browse_helper.rb
@@ -112,7 +112,7 @@ module BrowseHelper
 
   def wikipedia_link(key, value)
     # Some k/v's are wikipedia=http://en.wikipedia.org/wiki/Full%20URL
 
   def wikipedia_link(key, value)
     # Some k/v's are wikipedia=http://en.wikipedia.org/wiki/Full%20URL

-    return nil if value =~ /^https?:\/\//
+    return nil if value =~ %r{^https?://}

 
     if key == "wikipedia"
       # This regex should match Wikipedia language codes, everything
 
     if key == "wikipedia"
       # This regex should match Wikipedia language codes, everything

diff --git a/app/models/client_application.rb b/app/models/client_application.rb
index 84f5b978c180fd31ce246f7e5957df1ddf467724..e1358028d98ae6fe7197b922cc081194528e759c 100644 (file)
--- a/app/models/client_application.rb
+++ b/app/models/client_application.rb
@@ -9,9 +9,9 @@ class ClientApplication < ActiveRecord::Base
 
   validates_presence_of :name, :url, :key, :secret
   validates_uniqueness_of :key
 
   validates_presence_of :name, :url, :key, :secret
   validates_uniqueness_of :key

-  validates_format_of :url, :with => /\Ahttp(s?):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i
-  validates_format_of :support_url, :with => /\Ahttp(s?):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i, :allow_blank => true
-  validates_format_of :callback_url, :with => /\A[a-z][a-z0-9.+-]*:\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i, :allow_blank => true
+  validates_format_of :url, :with => %r{\Ahttp(s?)://(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(/|/([\w#!:.?+=&%@!\-/]))?}i
+  validates_format_of :support_url, :with => %r{\Ahttp(s?)://(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(/|/([\w#!:.?+=&%@!\-/]))?}i, :allow_blank => true
+  validates_format_of :callback_url, :with => %r{\A[a-z][a-z0-9.+-]*://(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(/|/([\w#!:.?+=&%@!\-/]))?}i, :allow_blank => true

 
   before_validation :generate_keys, :on => :create
 
 
   before_validation :generate_keys, :on => :create
 

diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb
index 75f6d237115ce98db0f511c7da073d434a502e7a..e27c46ea70a406fd08425cec7be982de97021e88 100644 (file)
--- a/config/initializers/paperclip.rb
+++ b/config/initializers/paperclip.rb
@@ -5,7 +5,7 @@ module Paperclip
     def for(style_name, options)
       url = super(style_name, options)
 
     def for(style_name, options)
       url = super(style_name, options)
 

-      if url =~ /^\/assets\/(.*)$/
+      if url =~ %r{^/assets/(.*)$}

         asset_path($1)
       else
         url
         asset_path($1)
       else
         url

diff --git a/lib/classic_pagination/pagination.rb b/lib/classic_pagination/pagination.rb
index 990a53d8b1d55be2b7561782854f510898d86e80..32995a5f65fa28c290833cf0a2b356e8e6a229c2 100644 (file)
--- a/lib/classic_pagination/pagination.rb
+++ b/lib/classic_pagination/pagination.rb
@@ -359,13 +359,13 @@ module ActionController
         # Returns a new Page object representing the page just before this
         # page, or nil if this is the first page.
         def previous
         # Returns a new Page object representing the page just before this
         # page, or nil if this is the first page.
         def previous

-          if first? then nil else @paginator[@number - 1] end
+          first? ? nil : @paginator[@number - 1]

         end
 
         # Returns a new Page object representing the page just after this
         # page, or nil if this is the last page.
         def next
         end
 
         # Returns a new Page object representing the page just after this
         # page, or nil if this is the last page.
         def next

-          if last? then nil else @paginator[@number + 1] end
+          last? ? nil : @paginator[@number + 1]

         end
 
         # Returns a new Window object for this page with the specified
         end
 
         # Returns a new Window object for this page with the specified

diff --git a/test/helpers/changeset_helper_test.rb b/test/helpers/changeset_helper_test.rb
index f8ffa18246227537d70cbc4a09525e034b82c2ce..c014fad3a84cc9aeeb9a68faa18d2523851b3aed 100644 (file)
--- a/test/helpers/changeset_helper_test.rb
+++ b/test/helpers/changeset_helper_test.rb
@@ -10,6 +10,6 @@ class ChangesetHelperTest < ActionView::TestCase
 
   def test_changeset_details
     assert_match /^Created <abbr title='Mon, 01 Jan 2007 00:00:00 \+0000'>.*<\/abbr> by anonymous$/, changeset_details(changesets(:normal_user_first_change))
 
   def test_changeset_details
     assert_match /^Created <abbr title='Mon, 01 Jan 2007 00:00:00 \+0000'>.*<\/abbr> by anonymous$/, changeset_details(changesets(:normal_user_first_change))

-    assert_match /^Closed <abbr title='Created: Mon, 01 Jan 2007 00:00:00 \+0000&#10;Closed: Tue, 02 Jan 2007 00:00:00 \+0000'>.*<\/abbr> by <a href="\/user\/test2">test2<\/a>$/, changeset_details(changesets(:public_user_closed_change))
+    assert_match %r{^Closed <abbr title='Created: Mon, 01 Jan 2007 00:00:00 \+0000&#10;Closed: Tue, 02 Jan 2007 00:00:00 \+0000'>.*</abbr> by <a href="/user/test2">test2</a>$}, changeset_details(changesets(:public_user_closed_change))

   end
 end
   end
 end

diff --git a/test/helpers/note_helper_test.rb b/test/helpers/note_helper_test.rb
index ccfcadc93afa8956168bdd4bd906794579aba5bc..eb7b999385fb78268c187c78abf1acf3ced63d92 100644 (file)
--- a/test/helpers/note_helper_test.rb
+++ b/test/helpers/note_helper_test.rb
@@ -9,8 +9,8 @@ class NoteHelperTest < ActionView::TestCase
   def test_note_event
     date = Time.new(2014, 3, 5, 21, 37, 45, "+00:00")
 
   def test_note_event
     date = Time.new(2014, 3, 5, 21, 37, 45, "+00:00")
 

-    assert_match /^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 /^Resolved by <a href="\/user\/test2">test2<\/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, users(:public_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/test2">test2</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, users(:public_user))

   end
 
   def test_note_author
   end
 
   def test_note_author