/lib/rails_best_practices/reviews/use_query_attribute_review.rb

http://github.com/flyerhzm/rails_best_practices · Ruby · 119 lines · 64 code · 13 blank · 42 comment · 13 complexity · 3bc7ac20ed18e20f1a3ce3d4545561bf MD5 · raw file

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