Evaluate if the Rubocop Performance plugin is a reasonable inclusion

[MarcPaquette]

Issue

Incorporating rubocop's performance plugin, and following it's practices may yield more performant code, and PR's as well in the future. https://github.com/rubocop/rubocop-performance

Context

Performance improvements to cloud_controller_ng are an area of focus in 2021.

Possible Fix

Modifying the .rubocop.yml file in cloud_controller_ng, and updating the recommendations in the code would address this issue.

[MarcPaquette]

I was able to get rubocop's performance tests to run. It would require 30+ changes to implement . I'm not entirely sure if the performance benefits would be worth the inclusion and added "commit tax" for every PR and commit for CAPI.

Inspecting 3192 files
..................................................................................................................C..C.C....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................C......C...................................................................................C.................C.....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................C...................................C.........................................C.............................................C.................................................................................................................C.........C.....C......................................C...............................................................................................................................C...........................................................................................................................C........................................................................................................................................C...........................................................................................................................................................................................................................................................................C..........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................C................................................................................................................................C......................................................................................................................................

Offenses:

app/actions/security_group_create.rb:45:14: C: [Correctable] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
          if /column.*name/ =~ error.message
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/actions/security_group_create.rb:47:17: C: [Correctable] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
          elsif /column.*rules/ =~ error.message
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/actions/security_group_update.rb:41:14: C: [Correctable] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
          if /column.*name/ =~ error.message
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/actions/security_group_update.rb:43:17: C: [Correctable] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
          elsif /column.*rules/ =~ error.message
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/actions/service_credential_binding_app_create.rb:45:27: C: [Correctable] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
        already_bound! if e.message =~ /The app is already bound to the service|unique_service_binding_service_instance_guid_app_guid/
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/messages/validators.rb:344:10: C: [Correctable] Performance/RegexpMatch: Use match? instead of !~ when MatchData is not used.
      if timestamp !~ /\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z\Z/
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/messages/validators/security_group_rule_validator.rb:125:8: C: [Correctable] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
    if /\s/ =~ destination
       ^^^^^^^^^^^^^^^^^^^
app/models/runtime/process_model.rb:592:9: C: [Correctable] Performance/Casecmp: Use process_type.casecmp(type).zero? instead of process_type.downcase == type.downcase.
        process_type.downcase == type.downcase
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/runtime/route_mapping_model.rb:86:72: C: [Correctable] Performance/RangeInclude: Use Range#cover? instead of Range#member?.
      errors.add(:weight, 'must be between 1 and 100') unless (1..100).member?(weight)
                                                                       ^^^^^^^
docs/v3/helpers/docs_helpers.rb:6:8: C: [Correctable] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
    if version =~ /release-candidate/
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/cloud_controller/blobstore/fog/fog_client.rb:34:9: C: [Correctable] Performance/Casecmp: Use @connection_config[:provider].casecmp('local').zero? instead of @connection_config[:provider].downcase == 'local'.
        @connection_config[:provider].downcase == 'local'
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/cloud_controller/console.rb:28:3: C: [Correctable] Performance/RedundantMerge: Use config_kwargs[:secrets_hash] = secrets_hash instead of config_kwargs.merge!(secrets_hash: secrets_hash).
  config_kwargs.merge!(secrets_hash: secrets_hash)
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/cloud_controller/diego/environment.rb:43:17: C: [Correctable] Performance/RedundantBlockCall: Use yield instead of blk.call.
          merge(blk.call).
                ^^^^^^^^
lib/cloud_controller/rest_controller/order_applicator.rb:29:7: C: [Correctable] Performance/Casecmp: Use @order_direction.casecmp('desc').zero? instead of @order_direction.downcase == 'desc'.
      @order_direction.downcase == 'desc'
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/cloud_controller/routing_api/routing_api_client.rb:26:17: C: [Correctable] Performance/Casecmp: Use routing_api_uri.scheme.to_s.casecmp('https').zero? instead of routing_api_uri.scheme.to_s.downcase == 'https'.
      use_ssl = routing_api_uri.scheme.to_s.downcase == 'https'
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/cloud_controller/security_context.rb:72:29: C: [Correctable] Performance/Casecmp: Use current_user_email.casecmp(email).zero? instead of current_user_email.downcase == email.downcase.
      current_user_email && current_user_email.downcase == email.downcase
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/kubernetes/update_reapply_client.rb:18:30: C: [Correctable] Performance/RedundantBlockCall: Use yield instead of block.call.
        @client.update_route(block.call(@client.get_route(name, namespace)))
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/kubernetes/update_reapply_client.rb:27:32: C: [Correctable] Performance/RedundantBlockCall: Use yield instead of block.call.
          @client.update_image(block.call(@client.get_image(name, namespace)))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/kubernetes/update_reapply_client.rb:36:32: C: [Correctable] Performance/RedundantBlockCall: Use yield instead of block.call.
        @client.update_builder(block.call(@client.get_builder(name, namespace)))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
middleware/rate_limiter.rb:57:9: C: [Correctable] Performance/StartWith: Use String#start_with? instead of a regex match anchored to the beginning of the string.
        request.fullpath.match(%r{\A/internal})
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/request/lifecycle/service_credential_bindings_synoptic_spec.rb:62:53: C: [Correctable] Performance/Detect: Use find instead of select[0].
    app_binding_guid = parsed_response['resources'].select { |r| r['type'] == 'app' }[0]['guid']
                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/request/lifecycle/service_credential_bindings_synoptic_spec.rb:63:53: C: [Correctable] Performance/Detect: Use find instead of select[0].
    key_binding_guid = parsed_response['resources'].select { |r| r['type'] == 'key' }[0]['guid']
                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/support/stepper.rb:132:7: C: [Correctable] Performance/RedundantBlockCall: Use yield instead of block.call.
      block.call(result) if block
      ^^^^^^^^^^^^^^^^^^
spec/support/stepper.rb:189:5: C: [Correctable] Performance/RedundantBlockCall: Use yield instead of block.call.
    block.call if block
    ^^^^^^^^^^
spec/unit/controllers/v3/errors_controller_spec.rb:17:7: C: [Correctable] Performance/RedundantMerge: Use @request.env['action_dispatch.exception'] = StandardError.new('sad things') instead of @request.env.merge!('action_dispatch.exception' => StandardError.new('sad things')).
      @request.env.merge!('action_dispatch.exception' => StandardError.new('sad things'))
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/unit/messages/service_instance_create_message_spec.rb:97:11: C: [Correctable] Performance/RedundantMerge: Use body[:metadata] = { choppers: 3 } instead of body.merge!({ metadata: { choppers: 3 } }).
          body.merge!({ metadata: { choppers: 3 } })
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/unit/models/runtime/space_spec.rb:487:38: C: [Correctable] Performance/Detect: Use find instead of select.first.
            queried_space_1 = spaces.select { |s| s.guid == space1.guid }.first
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/unit/models/runtime/space_spec.rb:488:38: C: [Correctable] Performance/Detect: Use find instead of select.first.
            queried_space_3 = spaces.select { |s| s.guid == space3.guid }.first
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/unit/models/runtime/space_spec.rb:528:38: C: [Correctable] Performance/Detect: Use find instead of select.first.
            queried_space_1 = spaces.select { |s| s.guid == space1.guid }.first
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/unit/models/runtime/space_spec.rb:529:38: C: [Correctable] Performance/Detect: Use find instead of select.first.
            queried_space_3 = spaces.select { |s| s.guid == space3.guid }.first
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

3192 files inspected, 30 offenses detected, 30 offenses auto-correctable

[MarcPaquette]

Took a second pass at this and incorporated the automatic changes. It was easy enough, but as discussed with colleagues, the root of the performance issues that CAPI has at this time are closely tied with Database performance.