Fix Style/SafeNavigation rubocop warnings

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index fbae24baac1c482ecc6e8233f88525626fbc0716..c59c4efca2ef6e04cd52dc0a1c5979b316148a97 100644 (file)
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -206,28 +206,6 @@ Style/NumericPredicate:
     - 'lib/gpx.rb'
     - 'test/controllers/amf_controller_test.rb'
 
     - 'lib/gpx.rb'
     - 'test/controllers/amf_controller_test.rb'
 

-# Offense count: 30
-# Cop supports --auto-correct.
-# Configuration parameters: ConvertCodeThatCanStartToReturnNil, Whitelist.
-# Whitelist: present?, blank?, presence, try, try!
-Style/SafeNavigation:
-  Exclude:
-    - 'app/controllers/amf_controller.rb'
-    - 'app/controllers/application_controller.rb'
-    - 'app/controllers/browse_controller.rb'
-    - 'app/controllers/issues_controller.rb'
-    - 'app/controllers/notes_controller.rb'
-    - 'app/controllers/old_controller.rb'
-    - 'app/controllers/traces_controller.rb'
-    - 'app/controllers/user_controller.rb'
-    - 'app/helpers/user_roles_helper.rb'
-    - 'app/models/changeset.rb'
-    - 'app/models/client_application.rb'
-    - 'app/models/notifier.rb'
-    - 'app/models/relation.rb'
-    - 'app/models/way.rb'
-    - 'script/deliver-message'
-

 # Offense count: 3080
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
 # 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 8fbd8dea9c707bc22941c2ac442e603f50784007..b164eddafffc8714b119f522bf6fb47e8f934554 100644 (file)
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -510,7 +510,7 @@ class AmfController < ApplicationController
     rels = []
     if searchterm.to_i > 0
       rel = Relation.where(:id => searchterm.to_i).first
     rels = []
     if searchterm.to_i > 0
       rel = Relation.where(:id => searchterm.to_i).first

-      rels.push([rel.id, rel.tags, rel.members, rel.version]) if rel && rel.visible
+      rels.push([rel.id, rel.tags, rel.members, rel.version]) if rel&.visible

     else
       RelationTag.where("v like ?", "%#{searchterm}%").limit(11).each do |t|
         rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version]) if t.relation.visible
     else
       RelationTag.where("v like ?", "%#{searchterm}%").limit(11).each do |t|
         rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version]) if t.relation.visible


@@ -889,12 +889,10 @@ class AmfController < ApplicationController
   # in the +tags+ hash.
   def strip_non_xml_chars(tags)
     new_tags = {}
   # in the +tags+ hash.
   def strip_non_xml_chars(tags)
     new_tags = {}

-    unless tags.nil?
-      tags.each do |k, v|
-        new_k = k.delete "\000-\037\ufffe\uffff", "^\011\012\015"
-        new_v = v.delete "\000-\037\ufffe\uffff", "^\011\012\015"
-        new_tags[new_k] = new_v
-      end
+    tags&.each do |k, v|
+      new_k = k.delete "\000-\037\ufffe\uffff", "^\011\012\015"
+      new_v = v.delete "\000-\037\ufffe\uffff", "^\011\012\015"
+      new_tags[new_k] = new_v

     end
     new_tags
   end
     end
     new_tags
   end

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index db4ae9ad392f20f91a60d6c251f8244633cc7d34..f35493b26bd6da15b038f56b1cffe42d417fe337 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -283,8 +283,7 @@ class ApplicationController < ActionController::Base
     # TODO: some sort of escaping of problem characters in the message
     response.headers["Error"] = message
 
     # TODO: some sort of escaping of problem characters in the message
     response.headers["Error"] = message
 

-    if request.headers["X-Error-Format"] &&
-       request.headers["X-Error-Format"].casecmp("xml").zero?
+    if request.headers["X-Error-Format"]&.casecmp("xml")&.zero?

       result = OSM::API.new.get_xml_doc
       result.root.name = "osmError"
       result.root << (XML::Node.new("status") << "#{Rack::Utils.status_code(status)} #{Rack::Utils::HTTP_STATUS_CODES[status]}")
       result = OSM::API.new.get_xml_doc
       result.root.name = "osmError"
       result.root << (XML::Node.new("status") << "#{Rack::Utils.status_code(status)} #{Rack::Utils::HTTP_STATUS_CODES[status]}")


@@ -310,7 +309,7 @@ class ApplicationController < ActionController::Base
   helper_method :preferred_languages
 
   def set_locale(reset = false)
   helper_method :preferred_languages
 
   def set_locale(reset = false)

-    if current_user && current_user.languages.empty? && !http_accept_language.user_preferred_languages.empty?
+    if current_user&.languages&.empty? && !http_accept_language.user_preferred_languages.empty?

       current_user.languages = http_accept_language.user_preferred_languages
       current_user.save
     end
       current_user.languages = http_accept_language.user_preferred_languages
       current_user.save
     end


@@ -435,7 +434,7 @@ class ApplicationController < ActionController::Base
   def preferred_editor
     editor = if params[:editor]
                params[:editor]
   def preferred_editor
     editor = if params[:editor]
                params[:editor]

-             elsif current_user && current_user.preferred_editor
+             elsif current_user&.preferred_editor

                current_user.preferred_editor
              else
                DEFAULT_EDITOR
                current_user.preferred_editor
              else
                DEFAULT_EDITOR

diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb
index b4cb4594fb43e7dcffa9694e7b8919961e4cdfdf..89b9c6ca3078aa4f8552918ae5714993dc8cbe23 100644 (file)
--- a/app/controllers/browse_controller.rb
+++ b/app/controllers/browse_controller.rb
@@ -58,7 +58,7 @@ class BrowseController < ApplicationController
   def changeset
     @type = "changeset"
     @changeset = Changeset.find(params[:id])
   def changeset
     @type = "changeset"
     @changeset = Changeset.find(params[:id])

-    @comments = if current_user && current_user.moderator?
+    @comments = if current_user&.moderator?

                   @changeset.comments.unscope(:where => :visible).includes(:author)
                 else
                   @changeset.comments.includes(:author)
                   @changeset.comments.unscope(:where => :visible).includes(:author)
                 else
                   @changeset.comments.includes(:author)


@@ -77,7 +77,7 @@ class BrowseController < ApplicationController
   def note
     @type = "note"
 
   def note
     @type = "note"
 

-    if current_user && current_user.moderator?
+    if current_user&.moderator?

       @note = Note.find(params[:id])
       @note_comments = @note.comments.unscope(:where => :visible)
     else
       @note = Note.find(params[:id])
       @note_comments = @note.comments.unscope(:where => :visible)
     else

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index d4e9a3bdfa4bec2c7e745b200c67e52a307ff086..ad38454f0650ba62ebd2dae40c1ab383c462c12a 100644 (file)
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -18,7 +18,7 @@ class IssuesController < ApplicationController
     @issues = Issue.visible_to(current_user)
 
     # If search
     @issues = Issue.visible_to(current_user)
 
     # If search

-    if params[:search_by_user] && params[:search_by_user].present?
+    if params[:search_by_user]&.present?

       @find_user = User.find_by(:display_name => params[:search_by_user])
       if @find_user
         @issues = @issues.where(:reported_user_id => @find_user.id)
       @find_user = User.find_by(:display_name => params[:search_by_user])
       if @find_user
         @issues = @issues.where(:reported_user_id => @find_user.id)


@@ -28,11 +28,11 @@ class IssuesController < ApplicationController
       end
     end
 
       end
     end
 

-    @issues = @issues.where(:status => params[:status]) if params[:status] && params[:status].present?
+    @issues = @issues.where(:status => params[:status]) if params[:status]&.present?

 
 

-    @issues = @issues.where(:reportable_type => params[:issue_type]) if params[:issue_type] && params[:issue_type].present?
+    @issues = @issues.where(:reportable_type => params[:issue_type]) if params[:issue_type]&.present?

 
 

-    if params[:last_updated_by] && params[:last_updated_by].present?
+    if params[:last_updated_by]&.present?

       last_updated_by = params[:last_updated_by].to_s == "nil" ? nil : params[:last_updated_by].to_i
       @issues = @issues.where(:updated_by => last_updated_by)
     end
       last_updated_by = params[:last_updated_by].to_s == "nil" ? nil : params[:last_updated_by].to_i
       @issues = @issues.where(:updated_by => last_updated_by)
     end

diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index 94d0fdb55343cd4f708da802ead7921f63e0969a..0c3392d60c32f1e415c6dae6f86309769b853a78 100644 (file)
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -211,7 +211,7 @@ class NotesController < ApplicationController
     # Find the note and check it is valid
     @note = Note.find(params[:id])
     raise OSM::APINotFoundError unless @note
     # Find the note and check it is valid
     @note = Note.find(params[:id])
     raise OSM::APINotFoundError unless @note

-    raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? || (current_user && current_user.moderator?)
+    raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? || current_user&.moderator?

 
     # Render the result
     respond_to do |format|
 
     # Render the result
     respond_to do |format|


@@ -286,7 +286,7 @@ class NotesController < ApplicationController
         @page = (params[:page] || 1).to_i
         @page_size = 10
         @notes = @user.notes
         @page = (params[:page] || 1).to_i
         @page_size = 10
         @notes = @user.notes

-        @notes = @notes.visible unless current_user && current_user.moderator?
+        @notes = @notes.visible unless current_user&.moderator?

         @notes = @notes.order("updated_at DESC, id").distinct.offset((@page - 1) * @page_size).limit(@page_size).preload(:comments => :author).to_a
       else
         @title = t "user.no_such_user.title"
         @notes = @notes.order("updated_at DESC, id").distinct.offset((@page - 1) * @page_size).limit(@page_size).preload(:comments => :author).to_a
       else
         @title = t "user.no_such_user.title"

diff --git a/app/controllers/old_controller.rb b/app/controllers/old_controller.rb
index 9adf141d959f4a319ec39c64a97826a573faf49b..4f01b1e2a1da41b5bd4d44fb1d5ba8c623ef1f4d 100644 (file)
--- a/app/controllers/old_controller.rb
+++ b/app/controllers/old_controller.rb
@@ -70,6 +70,6 @@ class OldController < ApplicationController
   private
 
   def show_redactions?
   private
 
   def show_redactions?

-    current_user && current_user.moderator? && params[:show_redactions] == "true"
+    current_user&.moderator? && params[:show_redactions] == "true"

   end
 end
   end
 end

diff --git a/app/controllers/traces_controller.rb b/app/controllers/traces_controller.rb
index c77d884b391d3a237e69b5d2b0d2d0832ed8b898..b78ae295916dce42ef7ca294dec170977e14cdad 100644 (file)
--- a/app/controllers/traces_controller.rb
+++ b/app/controllers/traces_controller.rb
@@ -92,8 +92,8 @@ class TracesController < ApplicationController
   def show
     @trace = Trace.find(params[:id])
 
   def show
     @trace = Trace.find(params[:id])
 

-    if @trace && @trace.visible? &&
-       (@trace.public? || @trace.user == current_user)
+    if @trace&.visible? &&
+       (@trace&.public? || @trace&.user == current_user)

       @title = t ".title", :name => @trace.name
     else
       flash[:error] = t ".trace_not_found"
       @title = t ".title", :name => @trace.name
     else
       flash[:error] = t ".trace_not_found"


@@ -318,7 +318,7 @@ class TracesController < ApplicationController
     visibility = params[:visibility]
 
     if visibility.nil?
     visibility = params[:visibility]
 
     if visibility.nil?

-      visibility = if params[:public] && params[:public].to_i.nonzero?
+      visibility = if params[:public]&.to_i&.nonzero?

                      "public"
                    else
                      "private"
                      "public"
                    else
                      "private"

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index be261e8c89423715214abe362f3c3d674591ae45..78299dccf2abe5342c4f82f7bc76acf1aca52f66 100644 (file)
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -29,7 +29,7 @@ class UserController < ApplicationController
     else
       @title = t "user.terms.title"
 
     else
       @title = t "user.terms.title"
 

-      if current_user && current_user.terms_agreed?
+      if current_user&.terms_agreed?

         # Already agreed to terms, so just show settings
         redirect_to :action => :account, :display_name => current_user.display_name
       elsif current_user.nil? && session[:new_user].nil?
         # Already agreed to terms, so just show settings
         redirect_to :action => :account, :display_name => current_user.display_name
       elsif current_user.nil? && session[:new_user].nil?


@@ -276,7 +276,7 @@ class UserController < ApplicationController
     if params[:session] == session.id
       if session[:token]
         token = UserToken.find_by(:token => session[:token])
     if params[:session] == session.id
       if session[:token]
         token = UserToken.find_by(:token => session[:token])

-        token.destroy if token
+        token&.destroy

         session.delete(:token)
       end
       session.delete(:user)
         session.delete(:token)
       end
       session.delete(:user)


@@ -292,7 +292,7 @@ class UserController < ApplicationController
   def confirm
     if request.post?
       token = UserToken.find_by(:token => params[:confirm_string])
   def confirm
     if request.post?
       token = UserToken.find_by(:token => params[:confirm_string])

-      if token && token.user.active?
+      if token&.user&.active?

         flash[:error] = t("user.confirm.already active")
         redirect_to :action => "login"
       elsif !token || token.expired?
         flash[:error] = t("user.confirm.already active")
         redirect_to :action => "login"
       elsif !token || token.expired?


@@ -349,7 +349,7 @@ class UserController < ApplicationController
   def confirm_email
     if request.post?
       token = UserToken.find_by(:token => params[:confirm_string])
   def confirm_email
     if request.post?
       token = UserToken.find_by(:token => params[:confirm_string])

-      if token && token.user.new_email?
+      if token&.user&.new_email?

         self.current_user = token.user
         current_user.email = current_user.new_email
         current_user.new_email = nil
         self.current_user = token.user
         current_user.email = current_user.new_email
         current_user.new_email = nil


@@ -413,7 +413,7 @@ class UserController < ApplicationController
     @user = User.find_by(:display_name => params[:display_name])
 
     if @user &&
     @user = User.find_by(:display_name => params[:display_name])
 
     if @user &&

-       (@user.visible? || (current_user && current_user.administrator?))
+       (@user.visible? || (current_user&.administrator?))

       @title = @user.display_name
     else
       render_unknown_user params[:display_name]
       @title = @user.display_name
     else
       render_unknown_user params[:display_name]


@@ -552,7 +552,7 @@ class UserController < ApplicationController
       if user.nil? && provider == "google"
         openid_url = auth_info[:extra][:id_info]["openid_id"]
         user = User.find_by(:auth_provider => "openid", :auth_uid => openid_url) if openid_url
       if user.nil? && provider == "google"
         openid_url = auth_info[:extra][:id_info]["openid_id"]
         user = User.find_by(:auth_provider => "openid", :auth_uid => openid_url) if openid_url

-        user.update(:auth_provider => provider, :auth_uid => uid) if user
+        user&.update(:auth_provider => provider, :auth_uid => uid)

       end
 
       if user
       end
 
       if user

diff --git a/app/helpers/user_roles_helper.rb b/app/helpers/user_roles_helper.rb
index 93e090b0fd7c2d3941ddfe06f69250d997732b1e..0b7e9c6162c2c2365a558594daf40a7ac3572b05 100644 (file)
--- a/app/helpers/user_roles_helper.rb
+++ b/app/helpers/user_roles_helper.rb
@@ -6,7 +6,7 @@ module UserRolesHelper
   end
 
   def role_icon(user, role)
   end
 
   def role_icon(user, role)

-    if current_user && current_user.administrator?
+    if current_user&.administrator?

       if user.has_role?(role)
         image = "roles/#{role}"
         alt = t("user.show.role.revoke.#{role}")
       if user.has_role?(role)
         image = "roles/#{role}"
         alt = t("user.show.role.revoke.#{role}")

diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index 4d305ea4460fabc165d5f8d3e8905e0082ff393a..5cdfeb994544444f08edb72cc4f8ba42fead569d 100644 (file)
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -208,7 +208,7 @@ class Changeset < ActiveRecord::Base
 
     user_display_name_cache = {} if user_display_name_cache.nil?
 
 
     user_display_name_cache = {} if user_display_name_cache.nil?
 

-    if user_display_name_cache && user_display_name_cache.key?(user_id)
+    if user_display_name_cache&.key?(user_id)

       # use the cache if available
     elsif user.data_public?
       user_display_name_cache[user_id] = user.display_name
       # use the cache if available
     elsif user.data_public?
       user_display_name_cache[user_id] = user.display_name

diff --git a/app/models/client_application.rb b/app/models/client_application.rb
index 1d5922d8e52073df6b55c3b1823b77aaae795ece..d11942beb6434e7c393db40ad3cfd5f3c3c4849b 100644 (file)
--- a/app/models/client_application.rb
+++ b/app/models/client_application.rb
@@ -51,7 +51,7 @@ class ClientApplication < ActiveRecord::Base
 
   def self.find_token(token_key)
     token = OauthToken.includes(:client_application).find_by(:token => token_key)
 
   def self.find_token(token_key)
     token = OauthToken.includes(:client_application).find_by(:token => token_key)

-    token if token && token.authorized?
+    token if token&.authorized?

   end
 
   def self.verify_request(request, options = {}, &block)
   end
 
   def self.verify_request(request, options = {}, &block)

diff --git a/app/models/notifier.rb b/app/models/notifier.rb
index 554d1e364ffc5c1e01b83ee63f903fa61709bfba..3cc1be1e66548ef2d98d0695db89ecf163829cc3 100644 (file)
--- a/app/models/notifier.rb
+++ b/app/models/notifier.rb
@@ -181,8 +181,8 @@ class Notifier < ActionMailer::Base
   end
 
   def user_avatar_file_path(user)
   end
 
   def user_avatar_file_path(user)

-    image = user && user.image
-    if image && image.file?
+    image = user&.image
+    if image&.file?

       return image.path(:small)
     else
       return Rails.root.join("app", "assets", "images", "users", "images", "small.png")
       return image.path(:small)
     else
       return Rails.root.join("app", "assets", "images", "users", "images", "small.png")

diff --git a/app/models/relation.rb b/app/models/relation.rb
index b17a3b604674abe85e166aeb8da09d34e9fecfc9..42ac3d6dc337ce11633ac4a68765b621b2b51d4d 100644 (file)
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -260,7 +260,7 @@ class Relation < ActiveRecord::Base
       element = model.lock("for share").find_by(:id => m[1])
 
       # and check that it is OK to use.
       element = model.lock("for share").find_by(:id => m[1])
 
       # and check that it is OK to use.

-      raise OSM::APIPreconditionFailedError, "Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}" unless element && element.visible? && element.preconditions_ok?
+      raise OSM::APIPreconditionFailedError, "Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}" unless element&.visible? && element&.preconditions_ok?

 
       hash[m[1]] = true
     end
 
       hash[m[1]] = true
     end

diff --git a/app/models/way.rb b/app/models/way.rb
index d6f0011d9c7d61b4ba0609a736b759e1e3dc6197..2d149863e359b12cad01bf22315a5e658ef216b9 100644 (file)
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -127,7 +127,7 @@ class Way < ActiveRecord::Base
         ordered_nodes[nd.sequence_id] = nd.node_id.to_s if visible_nodes[nd.node_id]
       else
         # otherwise, manually go to the db to check things
         ordered_nodes[nd.sequence_id] = nd.node_id.to_s if visible_nodes[nd.node_id]
       else
         # otherwise, manually go to the db to check things

-        ordered_nodes[nd.sequence_id] = nd.node_id.to_s if nd.node && nd.node.visible?
+        ordered_nodes[nd.sequence_id] = nd.node_id.to_s if nd.node&.visible?

       end
     end
 
       end
     end
 

diff --git a/script/deliver-message b/script/deliver-message
index 98e00d3328bdbab946a0e07f33f529b9182cc4ea..2c440c3ec60144b00d6cb571dae9ec8e3569df4e 100755 (executable)
--- a/script/deliver-message
+++ b/script/deliver-message
@@ -23,7 +23,7 @@ end
 exit 0 unless token == digest[0, 6]
 exit 0 if date < 1.month.ago
 
 exit 0 unless token == digest[0, 6]
 exit 0 if date < 1.month.ago
 

-message.update(:message_read => true) if message
+message&.update(:message_read => true)

 
 mail = Mail.new(STDIN.read
                      .encode(:universal_newline => true)
 
 mail = Mail.new(STDIN.read
                      .encode(:universal_newline => true)