/lib/rails_best_practices/reviews/use_query_attribute_review.rb

  1# frozen_string_literal: true
  2
  3module RailsBestPractices
  4  module Reviews
  5    # Make sure to use query attribute instead of nil?, blank? and present?.
  6    #
  7    # See the best practice details here https://rails-bestpractices.com/posts/2010/10/03/use-query-attribute/
  8    #
  9    # Implementation:
 10    #
 11    # Review process:
 12    #   check all method calls within conditional statements, like @user.login.nil?
 13    #   if their receivers are one of the model names
 14    #   and their messages of first call are not pluralize and not in any of the association names
 15    #   and their messages of second call are one of nil?, blank?, present?, or they are == ""
 16    #   then you can use query attribute instead.
 17    class UseQueryAttributeReview < Review
 18      interesting_nodes :if, :unless, :elsif, :ifop, :if_mod, :unless_mod
 19      interesting_files ALL_FILES
 20      url 'https://rails-bestpractices.com/posts/2010/10/03/use-query-attribute/'
 21
 22      QUERY_METHODS = %w[nil? blank? present?].freeze
 23
 24      # check if node to see whose conditional statement nodes contain nodes that can use query attribute instead.
 25      #
 26      # it will check every call nodes in the if nodes. If the call node is
 27      #
 28      # 1. two method calls, like @user.login.nil?
 29      # 2. the receiver is one of the model names
 30      # 3. the message of first call is the model's attribute,
 31      #    the message is not in any of associations name and is not pluralize
 32      # 4. the message of second call is one of nil?, blank? or present? or
 33      #    the message is == and the argument is ""
 34      #
 35      # then the call node can use query attribute instead.
 36      add_callback :start_if, :start_unless, :start_elsif, :start_ifop, :start_if_mod, :start_unless_mod do |node|
 37        all_conditions =
 38          if node.conditional_statement == node.conditional_statement.all_conditions
 39            [node.conditional_statement]
 40          else
 41            node.conditional_statement.all_conditions
 42          end
 43        all_conditions.each do |condition_node|
 44          next unless query_attribute_node = query_attribute_node(condition_node)
 45
 46          receiver_node = query_attribute_node.receiver
 47          add_error "use query attribute (#{receiver_node.receiver}.#{receiver_node.message}?)",
 48                    node.file,
 49                    query_attribute_node.line_number
 50        end
 51      end
 52
 53      private
 54
 55      # recursively check conditional statement nodes to see if there is a call node that may be
 56      # possible query attribute.
 57      def query_attribute_node(conditional_statement_node)
 58        case conditional_statement_node.sexp_type
 59        when :and, :or
 60          node =
 61            query_attribute_node(conditional_statement_node[1]) || query_attribute_node(conditional_statement_node[2])
 62          node.file = conditional_statement_code.file
 63          return node
 64        when :not
 65          node = query_attribute_node(conditional_statement_node[1])
 66          node.file = conditional_statement_node.file
 67        when :call
 68          return conditional_statement_node if possible_query_attribute?(conditional_statement_node)
 69        when :binary
 70          return conditional_statement_node if possible_query_attribute?(conditional_statement_node)
 71        end
 72        nil
 73      end
 74
 75      # check if the node may use query attribute instead.
 76      #
 77      # if the node contains two method calls, e.g. @user.login.nil?
 78      #
 79      # for the first call, the receiver should be one of the class names and
 80      # the message should be one of the attribute name.
 81      #
 82      # for the second call, the message should be one of nil?, blank? or present? or
 83      # it is compared with an empty string.
 84      #
 85      # the node that may use query attribute.
 86      def possible_query_attribute?(node)
 87        return false unless node.receiver.sexp_type == :call
 88
 89        variable_node = variable(node)
 90        message_node = node.grep_node(receiver: variable_node.to_s).message
 91
 92        is_model?(variable_node) && model_attribute?(variable_node, message_node.to_s) &&
 93          (QUERY_METHODS.include?(node.message.to_s) || compare_with_empty_string?(node))
 94      end
 95
 96      # check if the receiver is one of the models.
 97      def is_model?(variable_node)
 98        return false if variable_node.const?
 99
100        class_name = variable_node.to_s.sub(/^@/, '').classify
101        models.include?(class_name)
102      end
103
104      # check if the receiver and message is one of the model's attribute.
105      # the receiver should match one of the class model name, and the message should match one of attribute name.
106      def model_attribute?(variable_node, message)
107        class_name = variable_node.to_s.sub(/^@/, '').classify
108        attribute_type = model_attributes.get_attribute_type(class_name, message)
109        attribute_type && !%w[integer float].include?(attribute_type)
110      end
111
112      # check if the node is with node type :binary, node message :== and node argument is empty string.
113      def compare_with_empty_string?(node)
114        node.sexp_type == :binary && ['==', '!='].include?(node.message.to_s) &&
115          s(:string_literal, s(:string_content)) == node.argument
116      end
117    end
118  end
119end