Add a cop to disallow delegate to predicate methods with allow_nil option

Description

As suggested by @kerrizor,

Predicate methods should return a boolean value, however, when we delegate with allow_nil to properly handle non-extant relations, we end up with true, false, and nil as possible responses. While we generally are loose about nil being equivalent to false in Ruby, I feel like a predicate method is a special signal that we very much care about the boolean-nature of its response.

Proposal

Add a new cop to enforce this pattern across our codebase

# This cop looks for delegations to predicate methods with
# :allow_nil option. Define a method to handle `nil` and
# returns the correct Boolean value instead.

# bad
delegate :is_foo?, to: :bar, allow_nil: true

# good 1
def is_foo?
  return false unless bar
  bar.is_foo?
end

# good 2
def is_foo?
  !!bar&.is_foo?
end

Existing offenses

Offenses:

app/models/clusters/cluster.rb:96:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: on_creation?.
    delegate :on_creation?, to: :provider, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/cluster.rb:97:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: knative_pre_installed?.
    delegate :knative_pre_installed?, to: :provider, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/cluster.rb:99:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: active?.
    delegate :active?, to: :platform_kubernetes, prefix: true, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/cluster.rb:100:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: rbac?.
    delegate :rbac?, to: :platform_kubernetes, prefix: true, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/cluster.rb:101:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: available?.
    delegate :available?, to: :application_helm, prefix: true, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/cluster.rb:102:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: available?.
    delegate :available?, to: :application_ingress, prefix: true, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/cluster.rb:103:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: available?.
    delegate :available?, to: :application_prometheus, prefix: true, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/cluster.rb:104:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: available?.
    delegate :available?, to: :application_knative, prefix: true, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/cluster.rb:105:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: available?.
    delegate :available?, to: :application_elastic_stack, prefix: true, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/platforms/kubernetes.rb:53:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: enabled?.
      delegate :enabled?, to: :cluster, allow_nil: true
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/platforms/kubernetes.rb:54:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: provided_by_user?.
      delegate :provided_by_user?, to: :cluster, allow_nil: true
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/clusters/platforms/kubernetes.rb:55:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: allow_user_defined_namespace?.
      delegate :allow_user_defined_namespace?, to: :cluster, allow_nil: true
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/concerns/ci/metadatable.rb:21:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: has_exposed_artifacts?.
      delegate :has_exposed_artifacts?, to: :metadata, prefix: false, allow_nil: true
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/concerns/diff_positionable_note.rb:6:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: on_text?, on_image?.
    delegate :on_text?, :on_image?, to: :position, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/concerns/resolvable_discussion.rb:31:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: resolved_by_push?.
    delegate  :resolved_at, ...
    ^^^^^^^^^^^^^^^^^^^^^^^
app/models/concerns/services/data_fields.rb:10:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: activated?.
      delegate :activated?, to: :service, allow_nil: true
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/merge_request.rb:379:3: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: active?.
  delegate :active?, :builds_with_coverage, to: :head_pipeline, prefix: true, allow_nil: true
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/merge_request.rb:380:3: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: success?, active?.
  delegate :success?, :active?, to: :actual_head_pipeline, prefix: true, allow_nil: true
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/project.rb:389:3: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: feature_available?, builds_enabled?, wiki_enabled?, merge_requests_enabled?, forking_enabled?, issues_enabled?, pages_enabled?, analytics_enabled?, snippets_enabled?, public_pages?, private_pages?, operations_enabled?.
  delegate :feature_available?, :builds_enabled?, :wiki_enabled?, ...
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/project.rb:397:3: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: show_default_award_emojis?.
  delegate :show_default_award_emojis, :show_default_award_emojis=, ...
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/project.rb:400:3: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: scheduled?, started?, in_progress?, failed?, finished?.
  delegate :scheduled?, :started?, :in_progress?, :failed?, :finished?, ...
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/project.rb:403:3: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: no_import?.
  delegate :no_import?, to: :import_state, allow_nil: true
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/project.rb:422:3: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: active?.
  delegate :active?, to: :prometheus_service, allow_nil: true, prefix: true
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ee/app/models/concerns/ee/ci/metadatable.rb:9:9: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: secrets?.
        delegate :secrets?, :secrets, to: :metadata, prefix: false, allow_nil: true
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ee/app/models/ee/group.rb:52:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: enforced_group_managed_accounts?, enforced_sso?.
      delegate :enforced_group_managed_accounts?, :enforced_sso?, to: :saml_provider, allow_nil: true
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ee/app/models/ee/group.rb:53:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: repository_read_only?.
      delegate :repository_read_only, :repository_read_only?, to: :namespace_settings, allow_nil: true
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ee/app/models/ee/merge_request.rb:55:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: merge_requests_author_approval?.
      delegate :merge_requests_author_approval?, to: :target_project, allow_nil: true
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ee/app/models/ee/merge_request.rb:56:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: merge_requests_disable_committers_approval?.
      delegate :merge_requests_disable_committers_approval?, to: :target_project, allow_nil: true
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ee/app/models/ee/namespace.rb:74:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: temporary_storage_increase_enabled?, eligible_for_temporary_storage_increase?.
      delegate :additional_purchased_storage_size, :additional_purchased_storage_size=, ...
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ee/app/models/ee/namespace.rb:91:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: trial?, upgradable?.
      delegate :trial?, :trial_ends_on, :trial_starts_on, :trial_days_remaining, ...
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ee/app/models/ee/project.rb:194:7: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: last_update_succeeded?, last_update_failed?, ever_updated_successfully?, hard_failed?.
      delegate :last_update_succeeded?, :last_update_failed?, ...
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ee/app/models/license.rb:281:5: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: block_changes?, feature_available?.
    delegate :block_changes?, :feature_available?, to: :current, allow_nil: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/gitlab/ci/trace/stream.rb:15:9: C: Gitlab/DelegatePredicateMethods: Using delegate with allow_nil on the following predicate methods are discouraged: valid?.
        delegate :valid?, to: :stream, allow_nil: true
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

21725 files inspected, 33 offenses detected, 33 offenses auto-correctable

Related effort

• !54262 (merged)
• !54610 (merged)