From: Tom Hughes Date: Tue, 24 Feb 2015 20:45:55 +0000 (+0000) Subject: Fix more rubocop style issues X-Git-Tag: live~4746 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/a6b84a0294a2929006ff056f56103be549c5b5a2?ds=sidebyside Fix more rubocop style issues --- diff --git a/.rubocop.yml b/.rubocop.yml index a0533b162..dce00b925 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -9,6 +9,10 @@ Style/FileName: - 'script/locale/reload-languages' - 'script/update-spam-blocks' +Style/GlobalVars: + Exclude: + - 'lib/quad_tile/extconf.rb' + Style/HashSyntax: EnforcedStyle: hash_rockets Exclude: diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e9561cd2d..a83e74d27 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -70,29 +70,10 @@ Style/AccessorMethodName: Style/AsciiComments: Enabled: false -# Offense count: 9 -Style/ClassVars: - Enabled: false - -# Offense count: 12 -# Configuration parameters: Keywords. -Style/CommentAnnotation: - Enabled: false - # Offense count: 306 Style/Documentation: Enabled: false -# Offense count: 8 -# Configuration parameters: EnforcedStyle, SupportedStyles. -Style/FormatString: - Enabled: false - -# Offense count: 1 -# Configuration parameters: AllowedVariables. -Style/GlobalVars: - Enabled: false - # Offense count: 41 # Configuration parameters: MinBodyLength. Style/GuardClause: @@ -133,10 +114,6 @@ Style/RescueModifier: Style/StringLiteralsInInterpolation: Enabled: false -# Offense count: 1 -Style/StructInheritance: - Enabled: false - # Offense count: 3 # Configuration parameters: ExactNameMatch, AllowPredicates, AllowDSLWriters, Whitelist. Style/TrivialAccessors: diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 43afdf74f..72de936d7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -272,7 +272,7 @@ class ApplicationController < ActionController::Base # phrase from that, we can also put the error message into the status # message. For now, rails won't let us) def report_error(message, status = :bad_request) - # Todo: some sort of escaping of problem characters in the message + # TODO: some sort of escaping of problem characters in the message response.headers["Error"] = message if request.headers["X-Error-Format"] && diff --git a/app/controllers/swf_controller.rb b/app/controllers/swf_controller.rb index 51fe4010b..8d907d850 100644 --- a/app/controllers/swf_controller.rb +++ b/app/controllers/swf_controller.rb @@ -119,7 +119,7 @@ class SwfController < ApplicationController def start_and_move(x, y, col) d = "001001" # Line style change, moveTo l = [length_sb(x), length_sb(y)].max - d += sprintf("%05b%0#{l}b%0#{l}b", l, x, y) + d += format("%05b%0*b%0*b", l, l, x, l, y) d += col # Select line style d end @@ -147,9 +147,9 @@ class SwfController < ApplicationController dx = x2 - x1 dy = y2 - y1 l = [length_sb(dx), length_sb(dy)].max - d += sprintf("%04b", l - 2) + d += format("%04b", l - 2) d += "1" # GeneralLine - d += sprintf("%0#{l}b%0#{l}b", dx, dy) + d += format("%0*b%0*b", l, dx, l, dy) d end @@ -176,7 +176,7 @@ class SwfController < ApplicationController length_sb(c), length_sb(d)].max # create binary string (00111001 etc.) - 5-byte length, then bbox - n = sprintf("%05b%0#{l}b%0#{l}b%0#{l}b%0#{l}b", l, a, b, c, d) + n = format("%05b%0*b%0*b%0*b%0*b", l, l, a, l, b, l, c, l, d) # pack into byte string [n].pack("B*") end diff --git a/app/helpers/title_helper.rb b/app/helpers/title_helper.rb index b44628248..3103a339f 100644 --- a/app/helpers/title_helper.rb +++ b/app/helpers/title_helper.rb @@ -1,11 +1,13 @@ require "htmlentities" module TitleHelper - @@coder = HTMLEntities.new + def self.coder + @coder ||= HTMLEntities.new + end def set_title(title = false) if title - @title = @@coder.decode(title.gsub("", "\u202a").gsub("", "\u202c")) + @title = TitleHelper.coder.decode(title.gsub("", "\u202a").gsub("", "\u202c")) response.headers["X-Page-Title"] = t("layouts.project_name.title") + " | " + @title else @title = title diff --git a/app/models/node.rb b/app/models/node.rb index 1b81cc823..ec899fa89 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -141,7 +141,7 @@ class Node < ActiveRecord::Base # update changeset bbox with *old* position first changeset.update_bbox!(bbox) - # FIXME logic needs to be double checked + # FIXME: logic needs to be double checked self.latitude = new_node.latitude self.longitude = new_node.longitude self.tags = new_node.tags diff --git a/app/models/old_way.rb b/app/models/old_way.rb index 92fe19ffb..ed1f867f6 100644 --- a/app/models/old_way.rb +++ b/app/models/old_way.rb @@ -70,7 +70,7 @@ class OldWay < ActiveRecord::Base add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache) - old_nodes.each do |nd| # FIXME need to make sure they come back in the right order + old_nodes.each do |nd| # FIXME: need to make sure they come back in the right order node_el = XML::Node.new "nd" node_el["ref"] = nd.node_id.to_s el << node_el diff --git a/app/models/relation.rb b/app/models/relation.rb index 120fe4723..97241f437 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -135,7 +135,7 @@ class Relation < ActiveRecord::Base el end - # FIXME is this really needed? + # FIXME: is this really needed? def members @members ||= relation_members.map do |member| [member.member_type, member.member_id, member.member_role] diff --git a/app/models/trace.rb b/app/models/trace.rb index 1a935293a..157b63f69 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -219,7 +219,7 @@ class Trace < ActiveRecord::Base end def xml_file - # TODO *nix specific, could do to work on windows... would be functionally inferior though - check for '.gz' + # TODO: *nix specific, could do to work on windows... would be functionally inferior though - check for '.gz' filetype = `/usr/bin/file -bz #{trace_name}`.chomp gzipped = filetype =~ /gzip compressed/ bzipped = filetype =~ /bzip2 compressed/ diff --git a/db/migrate/021_move_to_innodb.rb b/db/migrate/021_move_to_innodb.rb index 57a69b570..b58817ed9 100644 --- a/db/migrate/021_move_to_innodb.rb +++ b/db/migrate/021_move_to_innodb.rb @@ -1,19 +1,19 @@ require "migrate" class MoveToInnodb < ActiveRecord::Migration - @@conv_tables = %w(nodes ways way_tags way_nodes current_way_tags relation_members relations relation_tags current_relation_tags) + @conv_tables = %w(nodes ways way_tags way_nodes current_way_tags relation_members relations relation_tags current_relation_tags) - @@ver_tbl = %w(nodes ways relations) + @ver_tbl = %w(nodes ways relations) def self.up remove_index :current_way_tags, :name => :current_way_tags_v_idx remove_index :current_relation_tags, :name => :current_relation_tags_v_idx - @@ver_tbl.each do |tbl| + @ver_tbl.each do |tbl| change_column tbl, "version", :bigint, :null => false end - @@ver_tbl.each do |tbl| + @ver_tbl.each do |tbl| add_column "current_#{tbl}", "version", :bigint, :null => false # As the initial version of all nodes, ways and relations is 0, we set the # current version to something less so that we can update the version in diff --git a/db/migrate/023_add_changesets.rb b/db/migrate/023_add_changesets.rb index 8bd9a930d..9039bb5d6 100644 --- a/db/migrate/023_add_changesets.rb +++ b/db/migrate/023_add_changesets.rb @@ -1,7 +1,7 @@ require "migrate" class AddChangesets < ActiveRecord::Migration - @@conv_user_tables = %w(current_nodes current_relations current_ways nodes relations ways) + @conv_user_tables = %w(current_nodes current_relations current_ways nodes relations ways) def self.up create_table "changesets", :id => false do |t| @@ -31,7 +31,7 @@ class AddChangesets < ActiveRecord::Migration execute "INSERT INTO changesets (id, user_id, created_at, open)" + "SELECT id, id, creation_time, false from users;" - @@conv_user_tables.each do |tbl| + @conv_user_tables.each do |tbl| rename_column tbl, :user_id, :changeset_id # foreign keys too add_foreign_key tbl, :changesets, :name => "#{tbl}_changeset_id_fkey" diff --git a/lib/country.rb b/lib/country.rb index 16eec1a36..48a721a49 100644 --- a/lib/country.rb +++ b/lib/country.rb @@ -14,7 +14,7 @@ class Country end def self.countries - @@countries ||= load_countries + @countries ||= load_countries end def self.load_countries diff --git a/lib/gpx.rb b/lib/gpx.rb index 9b93f07f2..f1c82cf92 100644 --- a/lib/gpx.rb +++ b/lib/gpx.rb @@ -150,11 +150,11 @@ module GPX private - class TrkPt < Struct.new(:segment, :latitude, :longitude, :altitude, :timestamp) + TrkPt = Struct.new(:segment, :latitude, :longitude, :altitude, :timestamp) do def valid? latitude && longitude && timestamp && - latitude >= -90 && latitude <= 90 && - longitude >= -180 && longitude <= 180 + latitude >= -90 && latitude <= 90 && + longitude >= -180 && longitude <= 180 end end end diff --git a/lib/quova.rb b/lib/quova.rb index 6cfc2ff92..b09f8acea 100644 --- a/lib/quova.rb +++ b/lib/quova.rb @@ -34,13 +34,13 @@ module Quova ## # Create SOAP endpoint - @@soap = SOAP::WSDLDriverFactory.new(WSDL_URL).create_rpc_driver - @@soap.options["protocol.http.basic_auth"] << [WSDL_URL, WSDL_USER, WSDL_PASS] + @soap = SOAP::WSDLDriverFactory.new(WSDL_URL).create_rpc_driver + @soap.options["protocol.http.basic_auth"] << [WSDL_URL, WSDL_USER, WSDL_PASS] ## # Accessor for SOAP endpoint def self.soap - @@soap + @soap end ## diff --git a/lib/rich_text.rb b/lib/rich_text.rb index bb2baddc8..7325a2a28 100644 --- a/lib/rich_text.rb +++ b/lib/rich_text.rb @@ -72,18 +72,19 @@ module RichText class Markdown < Base def to_html - html_parser.render(self).html_safe + Markdown.html_parser.render(self).html_safe end def to_text to_s end - private + def self.html_renderer + @html_renderer ||= Renderer.new(:filter_html => true, :safe_links_only => true) + end - def html_parser - @@html_renderer ||= Renderer.new(:filter_html => true, :safe_links_only => true) - @@html_parser ||= Redcarpet::Markdown.new(@@html_renderer, :no_intra_emphasis => true, :autolink => true, :space_after_headers => true) + def self.html_parser + @html_parser ||= Redcarpet::Markdown.new(html_renderer, :no_intra_emphasis => true, :autolink => true, :space_after_headers => true) end class Renderer < Redcarpet::Render::XHTML diff --git a/lib/validators.rb b/lib/validators.rb index 6338e9ce9..69cabb942 100644 --- a/lib/validators.rb +++ b/lib/validators.rb @@ -1,16 +1,13 @@ module ActiveRecord module Validations module ClassMethods - # error message when invalid UTF-8 is detected - @@invalid_utf8_message = " is invalid UTF-8" - ## # validation method to be included like any other validations methods # in the models definitions. this one checks that the named attribute # is a valid UTF-8 format string. def validates_as_utf8(*attrs) validates_each(attrs) do |record, attr, value| - record.errors.add(attr, @@invalid_utf8_message) unless UTF8.valid? value + record.errors.add(attr, " is invalid UTF-8") unless UTF8.valid? value end end end diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index aad27a06d..8eb933744 100644 --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@ -1381,7 +1381,7 @@ EOF assert_response :success assert_template nil # print @response.body - # FIXME needs more assert_select tests + # FIXME: needs more assert_select tests assert_select "osmChange[version='#{API_VERSION}'][generator='#{GENERATOR}']" do assert_select "create", :count => 5 assert_select "create>node[id='#{nodes(:used_node_2).node_id}'][visible='#{nodes(:used_node_2).visible?}'][version='#{nodes(:used_node_2).version}']" do @@ -1393,7 +1393,7 @@ EOF ## # check that the bounding box of a changeset gets updated correctly - ## FIXME: This should really be moded to a integration test due to the with_controller + # FIXME: This should really be moded to a integration test due to the with_controller def test_changeset_bbox basic_authorization users(:public_user).email, "test" @@ -1769,7 +1769,7 @@ EOF # Now check that all 20 (or however many were returned) changesets are in the html assert_select "li", :count => changesets.size changesets.each do |_changeset| - # FIXME this test needs rewriting - test for table contents + # FIXME: this test needs rewriting - test for table contents end end @@ -1795,7 +1795,7 @@ EOF # Now check that all 20 (or however many were returned) changesets are in the html assert_select "li", :count => changesets.size changesets.each do |_changeset| - # FIXME this test needs rewriting - test for table contents + # FIXME: this test needs rewriting - test for table contents end end @@ -1806,7 +1806,7 @@ EOF get :list, :format => "html", :display_name => user.display_name assert_response :success assert_template "history" - ## FIXME need to add more checks to see which if edits are actually shown if your data is public + # FIXME: need to add more checks to see which if edits are actually shown if your data is public end ## @@ -1829,7 +1829,7 @@ EOF assert_select "feed", :count => 1 assert_select "entry", :count => changesets.size changesets.each do |_changeset| - # FIXME this test needs rewriting - test for feed contents + # FIXME: this test needs rewriting - test for feed contents end end @@ -1841,7 +1841,7 @@ EOF assert_response :success assert_template "list" assert_equal "application/atom+xml", response.content_type - ## FIXME need to add more checks to see which if edits are actually shown if your data is public + # FIXME: need to add more checks to see which if edits are actually shown if your data is public end ## diff --git a/test/controllers/node_controller_test.rb b/test/controllers/node_controller_test.rb index 5ea1dd5a4..8952daade 100644 --- a/test/controllers/node_controller_test.rb +++ b/test/controllers/node_controller_test.rb @@ -499,14 +499,6 @@ class NodeControllerTest < ActionController::TestCase assert apinode.tags.include?("\#{@user.inspect}") end - def basic_authorization(user, pass) - @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}") - end - - def content(c) - @request.env["RAW_POST_DATA"] = c.to_s - end - ## # update the changeset_id of a node element def update_changeset(xml, changeset_id) diff --git a/test/controllers/old_node_controller_test.rb b/test/controllers/old_node_controller_test.rb index bdeec8014..6edb7c553 100644 --- a/test/controllers/old_node_controller_test.rb +++ b/test/controllers/old_node_controller_test.rb @@ -31,7 +31,7 @@ class OldNodeControllerTest < ActionController::TestCase # matching versions of the object. # ## - # FIXME Move this test to being an integration test since it spans multiple controllers + # FIXME: Move this test to being an integration test since it spans multiple controllers def test_version ## First try this with a non-public user basic_authorization(users(:normal_user).email, "test") diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb index 347c0d121..7e10159b4 100644 --- a/test/controllers/relation_controller_test.rb +++ b/test/controllers/relation_controller_test.rb @@ -107,7 +107,7 @@ class RelationControllerTest < ActionController::TestCase # check the "full" mode get :full, :id => current_relations(:visible_relation).id assert_response :success - # FIXME check whether this contains the stuff we want! + # FIXME: check whether this contains the stuff we want! print @response.body if $VERBOSE end diff --git a/test/integration/user_blocks_test.rb b/test/integration/user_blocks_test.rb index 38c64df7d..fcf008c07 100644 --- a/test/integration/user_blocks_test.rb +++ b/test/integration/user_blocks_test.rb @@ -4,7 +4,7 @@ class UserBlocksTest < ActionDispatch::IntegrationTest fixtures :users, :user_blocks, :user_roles def auth_header(user, pass) - { "HTTP_AUTHORIZATION" => "Basic %s" % Base64.encode64("#{user}:#{pass}") } + { "HTTP_AUTHORIZATION" => format("Basic %s", Base64.encode64("#{user}:#{pass}")) } end def test_api_blocked diff --git a/test/integration/user_terms_seen_test.rb b/test/integration/user_terms_seen_test.rb index a00ad39d5..3a42aa6bd 100644 --- a/test/integration/user_terms_seen_test.rb +++ b/test/integration/user_terms_seen_test.rb @@ -70,7 +70,7 @@ class UserTermsSeenTest < ActionDispatch::IntegrationTest private def auth_header(user, pass) - { "HTTP_AUTHORIZATION" => "Basic %s" % Base64.encode64("#{user}:#{pass}") } + { "HTTP_AUTHORIZATION" => format("Basic %s", Base64.encode64("#{user}:#{pass}")) } end def with_terms_seen(value) diff --git a/test/models/message_test.rb b/test/models/message_test.rb index cf5572216..81290a14a 100644 --- a/test/models/message_test.rb +++ b/test/models/message_test.rb @@ -55,7 +55,7 @@ class MessageTest < ActiveSupport::TestCase def test_invalid_utf8 # See e.g http://en.wikipedia.org/wiki/UTF-8 for byte sequences - # FIXME - Invalid Unicode characters can still be encoded into "valid" utf-8 byte sequences - maybe check this too? + # FIXME: Invalid Unicode characters can still be encoded into "valid" utf-8 byte sequences - maybe check this too? invalid_sequences = ["\xC0", # always invalid utf8 "\xC2\x4a", # 2-byte multibyte identifier, followed by plain ASCII "\xC2\xC2", # 2-byte multibyte identifier, followed by another one diff --git a/test/test_helper.rb b/test/test_helper.rb index 8f75a7ab0..ed2e02f63 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -115,7 +115,7 @@ module ActiveSupport end def basic_authorization(user, pass) - @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}") + @request.env["HTTP_AUTHORIZATION"] = format("Basic %s", Base64.encode64("#{user}:#{pass}")) end def error_format(format)