Fix Style/SafeNavigation rubocop warnings

.rubocop_todo.yml

@@ -206,28 +206,6 @@ Style/NumericPredicate:
     - '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

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.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
@@ -889,12 +889,10 @@ class AmfController < ApplicationController
   # in the +tags+ hash.
   def strip_non_xml_chars(tags)
     new_tags = {}
-    unless tags.nil?
+    tags&.each do |k, v|
-      tags.each do |k, v|
+      new_k = k.delete "\000-\037\ufffe\uffff", "^\011\012\015"
-        new_k = k.delete "\000-\037\ufffe\uffff", "^\011\012\015"
+      new_v = v.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
-        new_tags[new_k] = new_v
-      end
     end
     new_tags
   end

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
 
-    if request.headers["X-Error-Format"] &&
+    if request.headers["X-Error-Format"]&.casecmp("xml")&.zero?
-       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]}")
@@ -310,7 +309,7 @@ class ApplicationController < ActionController::Base
   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
@@ -435,7 +434,7 @@ class ApplicationController < ActionController::Base
   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

app/controllers/browse_controller.rb

@@ -58,7 +58,7 @@ class BrowseController < ApplicationController
   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)
@@ -77,7 +77,7 @@ class BrowseController < ApplicationController
   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

app/controllers/issues_controller.rb

@@ -18,7 +18,7 @@ class IssuesController < ApplicationController
     @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)
@@ -28,11 +28,11 @@ class IssuesController < ApplicationController
       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

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
-    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|
@@ -286,7 +286,7 @@ class NotesController < ApplicationController
         @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"

app/controllers/old_controller.rb

@@ -70,6 +70,6 @@ class OldController < ApplicationController
   private
 
   def show_redactions?
-    current_user && current_user.moderator? && params[:show_redactions] == "true"
+    current_user&.moderator? && params[:show_redactions] == "true"
   end
 end

app/controllers/traces_controller.rb

@@ -92,8 +92,8 @@ class TracesController < ApplicationController
   def show
     @trace = Trace.find(params[:id])
 
-    if @trace && @trace.visible? &&
+    if @trace&.visible? &&
-       (@trace.public? || @trace.user == current_user)
+       (@trace&.public? || @trace&.user == current_user)
       @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 = if params[:public] && params[:public].to_i.nonzero?
+      visibility = if params[:public]&.to_i&.nonzero?
                      "public"
                    else
                      "private"

app/controllers/user_controller.rb

@@ -29,7 +29,7 @@ class UserController < ApplicationController
     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?
@@ -276,7 +276,7 @@ class UserController < ApplicationController
     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)
@@ -292,7 +292,7 @@ class UserController < ApplicationController
   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?
@@ -349,7 +349,7 @@ class UserController < ApplicationController
   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
@@ -413,7 +413,7 @@ class UserController < ApplicationController
     @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]
@@ -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
-        user.update(:auth_provider => provider, :auth_uid => uid) if user
+        user&.update(:auth_provider => provider, :auth_uid => uid)
       end
 
       if user

app/helpers/user_roles_helper.rb

@@ -6,7 +6,7 @@ module UserRolesHelper
   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}")

app/models/changeset.rb

@@ -208,7 +208,7 @@ class Changeset < ActiveRecord::Base
 
     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

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)
-    token if token && token.authorized?
+    token if token&.authorized?
   end
 
   def self.verify_request(request, options = {}, &block)

app/models/notifier.rb

@@ -181,8 +181,8 @@ class Notifier < ActionMailer::Base
   end
 
   def user_avatar_file_path(user)
-    image = user && user.image
+    image = user&.image
-    if image && image.file?
+    if image&.file?
       return image.path(:small)
     else
       return Rails.root.join("app", "assets", "images", "users", "images", "small.png")

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.
-      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

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 nd.node && nd.node.visible?
+        ordered_nodes[nd.sequence_id] = nd.node_id.to_s if nd.node&.visible?
       end
     end
 

script/deliver-message

@@ -23,7 +23,7 @@ end
 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)