refactoring can definition matching behavior

lib/cancan/ability.rb

@@ -50,12 +50,10 @@ module CanCan
     # Also see the RSpec Matchers to aid in testing.
     def can?(action, subject, *extra_args)
       raise Error, "Nom nom nom. I eated it." if action == :has && subject == :cheezburger
-      matching_can_definition(action, subject) do |can_definition|
-        unless (can = can_definition.can?(action, subject, extra_args)) == :_NOT_MATCHED
-          return can
-        end
+      match = relevant_can_definitions(action, subject).detect do |can_definition|
+        can_definition.matches_conditions?(action, subject, extra_args)
       end
-      false
+      match ? match.base_behavior : false
     end
 
     # Convenience method which works the same as "can?" but returns the opposite value.
@@ -200,12 +198,12 @@ module CanCan
     # If the ability is defined using a block then this will raise an exception since a hash of conditions cannot be
     # determined from that.
     def conditions(action, subject, options = {})
-      matched = matching_can_definition(action, subject)
-      unless matched.empty?
-        if matched.any?{|can_definition| can_definition.only_block? }
+      relevant = relevant_can_definitions(action, subject)
+      unless relevant.empty?
+        if relevant.any?{|can_definition| can_definition.only_block? }
           raise Error, "Cannot determine ability conditions from block for #{action.inspect} #{subject.inspect}"
         end
-        matched.map{|can_definition|
+        relevant.map{|can_definition|
             [can_definition.base_behavior, can_definition.conditions(options)]
         }
       else
@@ -255,7 +253,7 @@ module CanCan
     # Returns the associations used in conditions. This is usually used in the :joins option for a search.
     # See ActiveRecordAdditions#accessible_by for use in Active Record.
     def association_joins(action, subject)
-      can_definitions = matching_can_definition(action, subject)
+      can_definitions = relevant_can_definitions(action, subject)
       unless can_definitions.empty?
         if can_definitions.any?{|can_definition| can_definition.only_block? }
           raise Error, "Cannot determine association joins from block for #{action.inspect} #{subject.inspect}"
@@ -281,19 +279,12 @@ module CanCan
       @can_definitions ||= []
     end
 
-    def matching_can_definition(action, subject)
-      if block_given?
-        can_definitions.reverse.each do |can_definition|
-          can_definition.expanded_actions = expand_actions(can_definition.actions)
-          if can_definition.matches? action, subject
-            yield can_definition
-            break if can_definition.definitive?
-          end
-        end
-      else
-        matched = []
-        matching_can_definition(action, subject){|can_definition| matched << can_definition}
-        matched
+    # Returns an array of CanDefinition instances which match the action and subject
+    # This does not take into consideration any hash conditions or block statements
+    def relevant_can_definitions(action, subject)
+      can_definitions.reverse.select do |can_definition|
+        can_definition.expanded_actions = expand_actions(can_definition.actions)
+        can_definition.relevant? action, subject
       end
     end
 

lib/cancan/can_definition.rb

@@ -3,7 +3,7 @@ module CanCan
   # it holds the information about a "can" call made on Ability and provides
   # helpful methods to determine permission checking and conditions hash generation.
   class CanDefinition # :nodoc:
-    attr_reader :conditions, :block, :base_behavior, :definitive
+    attr_reader :conditions, :block, :base_behavior
     include ActiveSupport::Inflector
     attr_reader :block
     attr_reader :actions
@@ -21,14 +21,20 @@ module CanCan
       @block = block
     end
 
-    def matches?(action, subject)
+    # Matches both the subject and action, not necessarily the conditions
+    def relevant?(action, subject)
       matches_action?(action) && matches_subject?(subject)
     end
 
-    def can?(action, subject, extra_args)
-      result = can_without_base_behavior?(action, subject, extra_args)
-      return :_NOT_MATCHED if result == :_NOT_MATCHED || !result
-      @base_behavior ? result : !result
+    # Matches the block or conditions hash
+    def matches_conditions?(action, subject, extra_args)
+      if @block
+        call_block(action, subject, extra_args)
+      elsif @conditions.kind_of?(Hash) && subject.class != Class
+        matches_conditions_hash?(subject)
+      else
+        true
+      end
     end
 
     # Returns a hash of conditions. If the ":tableize => true" option is passed
@@ -45,10 +51,6 @@ module CanCan
       end
     end
 
-    def definitive?
-      conditions_empty? && @block.nil?
-    end
-
     def only_block?
       conditions_empty? && !@block.nil?
     end
@@ -83,24 +85,14 @@ module CanCan
       @subjects.include?(:all) || @subjects.include?(subject) || @subjects.any? { |sub| sub.kind_of?(Class) && subject.kind_of?(sub) }
     end
 
-    def can_without_base_behavior?(action, subject, extra_args)
-      if @block
-        call_block(action, subject, extra_args)
-      elsif @conditions.kind_of?(Hash) && subject.class != Class
-        matches_conditions?(subject)
-      else
-        true
-      end
-    end
-
-    def matches_conditions?(subject, conditions = @conditions)
+    def matches_conditions_hash?(subject, conditions = @conditions)
       conditions.all? do |name, value|
         attribute = subject.send(name)
         if value.kind_of?(Hash)
           if attribute.kind_of? Array
-            attribute.any? { |element| matches_conditions? element, value }
+            attribute.any? { |element| matches_conditions_hash? element, value }
           else
-            matches_conditions? attribute, value
+            matches_conditions_hash? attribute, value
           end
         elsif value.kind_of?(Array) || value.kind_of?(Range)
           value.include? attribute