From: Tom Hughes Date: Fri, 20 Feb 2015 20:01:27 +0000 (+0000) Subject: Fix some more rubocop style issues X-Git-Tag: live~4827 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/8e404f3a468a2636481d52f245b816c41e9d5ac0?ds=inline Fix some more rubocop style issues --- diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 418e7265d..e9561cd2d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -108,10 +108,6 @@ Style/LineEndConcatenation: Style/NumericLiterals: MinDigits: 11 -# Offense count: 42 -Style/OneLineConditional: - Enabled: false - # Offense count: 44 # Cop supports --auto-correct. Style/PerlBackrefs: @@ -127,10 +123,6 @@ Style/PredicateName: Style/RaiseArgs: Enabled: false -# Offense count: 11 -Style/RegexpLiteral: - MaxSlashes: 5 - # Offense count: 2 Style/RescueModifier: Enabled: false diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 6860056e7..6c3e2c5cf 100644 --- 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) } - 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) - 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 @@ -137,12 +137,12 @@ class AmfController < ApplicationController 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 - 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 @@ -413,11 +413,11 @@ class AmfController < ApplicationController 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) - 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 @@ -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) - 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 @@ -616,13 +617,13 @@ class AmfController < ApplicationController # -- 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 @@ -639,8 +640,8 @@ class AmfController < ApplicationController 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] @@ -651,7 +652,7 @@ class AmfController < ApplicationController 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") @@ -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) - 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 @@ -739,8 +740,8 @@ class AmfController < ApplicationController 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 @@ -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) - 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 = {} diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 1a7f3810a..6cb0b60db 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -89,7 +89,7 @@ class UserController < ApplicationController 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], @@ -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) - 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 ## diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb index 4027ddb2f..793baf07b 100644 --- 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 - return nil if value =~ /^https?:\/\// + return nil if value =~ %r{^https?://} 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 84f5b978c..e1358028d 100644 --- 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_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 diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index 75f6d2371..e27c46ea7 100644 --- 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) - if url =~ /^\/assets\/(.*)$/ + if url =~ %r{^/assets/(.*)$} asset_path($1) else url diff --git a/lib/classic_pagination/pagination.rb b/lib/classic_pagination/pagination.rb index 990a53d8b..32995a5f6 100644 --- 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 - 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 - 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 diff --git a/test/helpers/changeset_helper_test.rb b/test/helpers/changeset_helper_test.rb index f8ffa1824..c014fad3a 100644 --- 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> by anonymous$/, changeset_details(changesets(:normal_user_first_change)) - assert_match /^Closed .*<\/abbr> by test2<\/a>$/, changeset_details(changesets(:public_user_closed_change)) + assert_match %r{^Closed .* by test2$}, changeset_details(changesets(:public_user_closed_change)) end end diff --git a/test/helpers/note_helper_test.rb b/test/helpers/note_helper_test.rb index ccfcadc93..eb7b99938 100644 --- 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") - assert_match /^Created by anonymous .*<\/span> ago<\/abbr>$/, note_event("open", date, nil) - assert_match /^Resolved by test2<\/a> .*<\/span> ago<\/abbr>$/, note_event("closed", date, users(:public_user)) + assert_match %r{^Created by anonymous .* ago$}, note_event("open", date, nil) + assert_match %r{^Resolved by test2 .* ago$}, note_event("closed", date, users(:public_user)) end def test_note_author