Suggest more-performant alternatives to Enumerable methods

[behnam-oneschema]

At the moment, one might write

• Users.where(email: email).count == 0 or
• Users.where(email: email).count > 0
  to check the existence of users with some email value, which is not an optimized way of doing that.

On top of that, the new Style/CollectionMethods RC rule would recommend writing those as Users.where(email: email).any?, Users.where(email: email).one? or Users.where(email: email).none?. However, none of those calls are actually optimized in Sequel, which will cause regression in the performance of those queries:

1. as before, all the rows are being checked by the SQL engine (no regression here), but
2. all the data fetched will now be transferred to the DB client, which can have real consequences in terms of performance.

From what I see, the proper way of performing those checks seem to be:

• Users.where(email: email).empty?,
• !Users.where(email: email).empty?, and
• Users.where(email: email).limit(2).count == 1.

So, now that Style/CollectionMethods is enabled by default in RC, I think it would be appropriate to have these Sequel-specialized fixes before those generic (and possibly worse) rules to kick in.

---

Side note: I think it's an issue with Sequel's Dataset data model that it does implement the Enumerable API, but enables its methods perform non-optimized queries. I'm not familiar with Sequel enough to tell if this is intentional or not. 🤷

[Simon089]

We started using this custom cop in our codebase for now.

If anyone is interested I can create a PR with the cop and rspec tests.

# Detects inefficient method calls on Sequel datasets and suggests
# more performant alternatives.
#
# @example
#   # bad
#   User.where(active: true).any?
#   dataset.none?
#   Model.select(:id).one?
#
#   # good
#   !User.where(active: true).empty?
#   dataset.empty?
#   Model.select(:id).limit(2).count == 1
class SequelDatasetMethods < Base
  extend AutoCorrector

  MSG_ANY =
    'Use `!.empty?` instead of `.any?` on Sequel datasets for better performance.'
  MSG_NONE =
    'Use `.empty?` instead of `.none?` on Sequel datasets for better performance.'
  MSG_ONE =
    'Use `.limit(2).count == 1` instead of `.one?` on Sequel datasets for better performance.'

  RESTRICT_ON_SEND = %i[any? none? one?].freeze

  # Matches method calls on what appear to be Sequel dataset chains
  def_node_matcher :sequel_dataset_chain?, <<~PATTERN
    (send
      (send $_ {:where :select :order :limit :group :having :join :exclude :distinct} ...)
      {:any? :none? :one?}
    )
  PATTERN

  # Matches direct calls on what might be Sequel models
  def_node_matcher :sequel_model_call?, <<~PATTERN
    (send
      (const $_ $_)
      {:any? :none? :one?}
    )
  PATTERN

  # Matches calls on variables that might be datasets
  def_node_matcher :dataset_variable_call?, <<~PATTERN
    (send
      (lvar $_)
      {:any? :none? :one?}
    )
  PATTERN

  # Matches calls on association datasets (e.g., user.posts_dataset.any?)
  def_node_matcher :association_dataset_call?, <<~PATTERN
    (send
      (send $_ $_ ...)
      {:any? :none? :one?}
    )
  PATTERN

  # Matches calls on constants (should be excluded)
  def_node_matcher :constant_call?, <<~PATTERN
    (send
      (const $_ $_)
      {:any? :none? :one?}
    )
  PATTERN

  def on_send(node)
    return unless likely_sequel_dataset?(node)

    method_name = node.method_name
    message =
      case method_name
      when :any?
        MSG_ANY
      when :none?
        MSG_NONE
      when :one?
        MSG_ONE
      end

    add_offense(node, message: message) { |corrector| autocorrect(corrector, node) }
  end

  private

  def likely_sequel_dataset?(node)
    # Exclude constants (like CONSTANT_LIST.any?)
    return false if constant_call?(node) && !sequel_model_like?(node.receiver)

    # Check for method chains with known Sequel dataset methods
    return true if sequel_dataset_chain?(node)

    # Check for calls on what appear to be Sequel models (constants)
    return true if sequel_model_call?(node)

    # Check for calls on variables that might be datasets
    if dataset_variable_call?(node)
      var_name = node.receiver.children.first.to_s
      return dataset_like_variable_name?(var_name)
    end

    # Check for association dataset calls (e.g., user.posts_dataset.any?)
    if association_dataset_call?(node)
      method_name = node.receiver.method_name.to_s
      return dataset_like_variable_name?(method_name)
    end

    false
  end

  def dataset_like_variable_name?(name)
    # Check if variable name suggests it's a dataset
    name.end_with?('_ds', '_dataset') || name.include?('dataset') ||
      name.end_with?('_query')
  end

  def sequel_model_like?(node)
    # Check if a constant looks like a Sequel model (CamelCase, not ALL_CAPS)
    return false unless node.const_type?

    const_name = node.const_name
    # Sequel models are typically CamelCase, not ALL_CAPS constants
    const_name.match?(/\A[A-Z][a-z]/) && !const_name.match?(/\A[A-Z_]+\z/)
  end

  def autocorrect(corrector, node)
    case node.method_name
    when :any?
      # .any? becomes !.empty?
      corrector.insert_before(node.receiver.source_range, '!')
      corrector.replace(node.loc.selector, 'empty?')
    when :none?
      # .none? becomes .empty? (no negation needed)
      corrector.replace(node.loc.selector, 'empty?')
    when :one?
      # Replace .one? with .limit(2).count == 1
      corrector.replace(node.loc.selector, 'limit(2).count == 1')
    end
  end
end