From de29e9b3f59d7a06fbe2e32096328bd8a6ed2b3a Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sat, 22 Sep 2018 17:34:58 +0100 Subject: [PATCH] Fix Style/NumericPredicate rubocop warnings --- .rubocop_todo.yml | 21 --------------------- app/controllers/amf_controller.rb | 10 +++++----- app/controllers/changeset_controller.rb | 2 +- app/controllers/notes_controller.rb | 6 +++--- app/helpers/banner_helper.rb | 2 +- app/helpers/issues_helper.rb | 2 +- app/models/relation.rb | 2 +- app/models/trace.rb | 2 +- app/models/way.rb | 2 +- db/migrate/021_move_to_innodb.rb | 2 +- lib/classic_pagination/pagination.rb | 2 +- lib/daemons/gpx_import.rb | 2 +- lib/gpx.rb | 2 +- test/controllers/amf_controller_test.rb | 6 +++--- 14 files changed, 21 insertions(+), 42 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c59c4efca..293135907 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -185,27 +185,6 @@ Style/IfUnlessModifier: Style/NumericLiterals: MinDigits: 11 -# Offense count: 21 -# Cop supports --auto-correct. -# Configuration parameters: AutoCorrect, EnforcedStyle. -# SupportedStyles: predicate, comparison -Style/NumericPredicate: - Exclude: - - 'spec/**/*' - - 'app/controllers/amf_controller.rb' - - 'app/controllers/changeset_controller.rb' - - 'app/controllers/notes_controller.rb' - - 'app/helpers/banner_helper.rb' - - 'app/helpers/issues_helper.rb' - - 'app/models/relation.rb' - - 'app/models/trace.rb' - - 'app/models/way.rb' - - 'db/migrate/021_move_to_innodb.rb' - - 'lib/classic_pagination/pagination.rb' - - 'lib/daemons/gpx_import.rb' - - 'lib/gpx.rb' - - 'test/controllers/amf_controller_test.rb' - # Offense count: 3080 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index b164eddaf..4f6adae5d 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -472,7 +472,7 @@ class AmfController < ApplicationController return -1, t("application.setup_user_auth.blocked") if user.blocks.active.exists? query = Trace.visible_to(user) - query = if searchterm.to_i > 0 + query = if searchterm.to_i.positive? query.where(:id => searchterm.to_i) else query.where("MATCH(name) AGAINST (?)", searchterm).limit(21) @@ -508,7 +508,7 @@ class AmfController < ApplicationController def findrelations(searchterm) rels = [] - if searchterm.to_i > 0 + if searchterm.to_i.positive? rel = Relation.where(:id => searchterm.to_i).first rels.push([rel.id, rel.tags, rel.members, rel.version]) if rel&.visible else @@ -545,7 +545,7 @@ class AmfController < ApplicationController relation = nil Relation.transaction do # create a new relation, or find the existing one - relation = Relation.find(relid) if relid > 0 + relation = Relation.find(relid) if relid.positive? # We always need a new node, based on the data that has been sent to us new_relation = Relation.new @@ -553,7 +553,7 @@ class AmfController < ApplicationController typedmembers = [] members.each do |m| mid = m[1].to_i - if mid < 0 + if mid.negative? mid = renumberednodes[mid] if m[0] == "Node" mid = renumberedways[mid] if m[0] == "Way" end @@ -741,7 +741,7 @@ class AmfController < ApplicationController node = nil new_node = nil Node.transaction do - if id > 0 + if id.positive? begin node = Node.find(id) rescue ActiveRecord::RecordNotFound diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 90b868129..c03ed9056 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -582,7 +582,7 @@ class ChangesetController < ApplicationController # Get the maximum number of comments to return def comments_limit if params[:limit] - if params[:limit].to_i > 0 && params[:limit].to_i <= 10000 + if params[:limit].to_i.positive? && params[:limit].to_i <= 10000 params[:limit].to_i else raise OSM::APIBadUserInput, "Comments limit must be between 1 and 10000" diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 0c3392d60..19ff725dc 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -307,7 +307,7 @@ class NotesController < ApplicationController # Get the maximum number of results to return def result_limit if params[:limit] - if params[:limit].to_i > 0 && params[:limit].to_i <= 10000 + if params[:limit].to_i.positive? && params[:limit].to_i <= 10000 params[:limit].to_i else raise OSM::APIBadUserInput, "Note limit must be between 1 and 10000" @@ -327,9 +327,9 @@ class NotesController < ApplicationController 7 end - if closed_since < 0 + if closed_since.negative? notes.where.not(:status => "hidden") - elsif closed_since > 0 + elsif closed_since.positive? notes.where(:status => "open") .or(notes.where(:status => "closed") .where(notes.arel_table[:closed_at].gt(Time.now - closed_since.days))) diff --git a/app/helpers/banner_helper.rb b/app/helpers/banner_helper.rb index 661369454..fef6eaa5e 100644 --- a/app/helpers/banner_helper.rb +++ b/app/helpers/banner_helper.rb @@ -26,7 +26,7 @@ module BannerHelper # rotate all banner queue positions index = cval.to_i - cookies[ckey] = index - 1 if index > 0 + cookies[ckey] = index - 1 if index.positive? # pick banner with mininum queue position next if index > min_index diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 4ecd7001a..43ca98c91 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -29,7 +29,7 @@ module IssuesHelper count = Issue.visible_to(current_user).open.limit(100).size if count > 99 content_tag(:span, "99+", :class => "count-number") - elsif count > 0 + elsif count.positive? content_tag(:span, count, :class => "count-number") end end diff --git a/app/models/relation.rb b/app/models/relation.rb index 42ac3d6dc..d5117b7c3 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -275,7 +275,7 @@ class Relation < ActiveRecord::Base def fix_placeholders!(id_map, placeholder_id = nil) members.map! do |type, id, role| old_id = id.to_i - if old_id < 0 + if old_id.negative? new_id = id_map[type.downcase.to_sym][old_id] raise OSM::APIBadUserInput, "Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}." if new_id.nil? diff --git a/app/models/trace.rb b/app/models/trace.rb index 91492404b..5096a81aa 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -306,7 +306,7 @@ class Trace < ActiveRecord::Base tp.save! end - if gpx.actual_points > 0 + if gpx.actual_points.positive? max_lat = Tracepoint.where(:gpx_id => id).maximum(:latitude) min_lat = Tracepoint.where(:gpx_id => id).minimum(:latitude) max_lon = Tracepoint.where(:gpx_id => id).maximum(:longitude) diff --git a/app/models/way.rb b/app/models/way.rb index 2d149863e..c95a12122 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -254,7 +254,7 @@ class Way < ActiveRecord::Base # to IDs +id_map+. def fix_placeholders!(id_map, placeholder_id = nil) nds.map! do |node_id| - if node_id < 0 + if node_id.negative? new_id = id_map[:node][node_id] raise OSM::APIBadUserInput, "Placeholder node not found for reference #{node_id} in way #{id.nil? ? placeholder_id : id}" if new_id.nil? diff --git a/db/migrate/021_move_to_innodb.rb b/db/migrate/021_move_to_innodb.rb index 3232e2741..d1c20bcdb 100644 --- a/db/migrate/021_move_to_innodb.rb +++ b/db/migrate/021_move_to_innodb.rb @@ -19,7 +19,7 @@ class MoveToInnodb < ActiveRecord::Migration[5.0] # current version to something less so that we can update the version in # batches of 10000 tbl.classify.constantize.update_all(:version => -1) - tbl.classify.constantize.update_all("version=(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)", { :version => -1 }, { :limit => 10000 }) while tbl.classify.constantize.where(:version => -1).count > 0 + tbl.classify.constantize.update_all("version=(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)", { :version => -1 }, { :limit => 10000 }) while tbl.classify.constantize.where(:version => -1).count.positive? # execute "UPDATE current_#{tbl} SET version = " + # "(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)" # The above update causes a MySQL error: diff --git a/lib/classic_pagination/pagination.rb b/lib/classic_pagination/pagination.rb index 054b2bb34..811d09239 100644 --- a/lib/classic_pagination/pagination.rb +++ b/lib/classic_pagination/pagination.rb @@ -398,7 +398,7 @@ module ActionController # Sets the window's padding (the number of pages on either side of the # window page). def padding=(padding) - @padding = padding < 0 ? 0 : padding + @padding = padding.negative? ? 0 : padding # Find the beginning and end pages of the window @first = if @paginator.has_page_number?(@page.number - @padding) @paginator[@page.number - @padding] diff --git a/lib/daemons/gpx_import.rb b/lib/daemons/gpx_import.rb index 4445d1ec0..a0344b58c 100755 --- a/lib/daemons/gpx_import.rb +++ b/lib/daemons/gpx_import.rb @@ -20,7 +20,7 @@ loop do begin gpx = trace.import - if gpx.actual_points > 0 + if gpx.actual_points.positive? Notifier.gpx_success(trace, gpx.actual_points).deliver else Notifier.gpx_failure(trace, "0 points parsed ok. Do they all have lat,lng,alt,timestamp?").deliver diff --git a/lib/gpx.rb b/lib/gpx.rb index ee9d53afa..8510df916 100644 --- a/lib/gpx.rb +++ b/lib/gpx.rb @@ -79,7 +79,7 @@ module GPX px = proj.x(p.longitude) py = proj.y(p.latitude) - if m > 0 + if m.positive? frames.times do |n| gc = if n == mm highlightgc.dup diff --git a/test/controllers/amf_controller_test.rb b/test/controllers/amf_controller_test.rb index ec11b38e8..0bdd01bd2 100644 --- a/test/controllers/amf_controller_test.rb +++ b/test/controllers/amf_controller_test.rb @@ -747,7 +747,7 @@ class AmfControllerTest < ActionController::TestCase assert_equal 5, result.size assert_equal 0, result[0], "expected to get the status ok from the amf" assert_equal 0, result[2], "The old id should be 0" - assert result[3] > 0, "The new id should be greater than 0" + assert result[3].positive?, "The new id should be greater than 0" assert_equal 1, result[4], "The new version should be 1" # Finally check that the node that was saved has saved the data correctly @@ -784,7 +784,7 @@ class AmfControllerTest < ActionController::TestCase assert_equal 5, result.size assert_equal 0, result[0], "Expected to get the status ok in the amf" assert_equal 0, result[2], "The old id should be 0" - assert result[3] > 0, "The new id should be greater than 0" + assert result[3].positive?, "The new id should be greater than 0" assert_equal 1, result[4], "The new version should be 1" # Finally check that the node that was saved has saved the data correctly @@ -831,7 +831,7 @@ class AmfControllerTest < ActionController::TestCase assert_equal 5, result.size assert_equal 0, result[0], "Expected to get the status ok in the amf" assert_equal 0, result[2], "The old id should be 0" - assert result[3] > 0, "The new id should be greater than 0" + assert result[3].positive?, "The new id should be greater than 0" assert_equal 1, result[4], "The new version should be 1" # Finally check that the node that was saved has saved the data correctly -- 2.43.2