support use_observer_check

README.textile

@@ -43,6 +43,7 @@ NeedlessDeepNestingCheck: { nested_count: 2 }
 NotUseDefaultRouteCheck: {  }
 KeepFindersOnTheirOwnModelCheck: { }
 LawOfDemeterCheck: { }
+UseObserverCheck: { }
 </code></pre>
 
 *************************************************
@@ -73,7 +74,7 @@ h2. Progress
 ## DRY: metaprogramming
 ## Extract into Module
 ## Extract to composed class
-## Use Observer
+## [-Use Observer-]
 
 * Lesson 4. Migration
 ## Isolating Seed Data

lib/rails_best_practices/checks.rb

@@ -12,3 +12,4 @@
 require 'rails_best_practices/checks/not_use_default_route_check'
 require 'rails_best_practices/checks/keep_finders_on_their_own_model_check'
 require 'rails_best_practices/checks/law_of_demeter_check'
+require 'rails_best_practices/checks/use_observer_check'

lib/rails_best_practices/checks/use_observer_check.rb

@@ -0,0 +1,47 @@
+require 'rails_best_practices/checks/check'
+
+module RailsBestPractices
+  module Checks
+    # Check a model file to make sure mail deliver method is in observer not callback.
+    #
+    # Implementation: 
+    # Record :after_create callback
+    # Check method define, if it is a callback and call deliver_xxx message in method body, then it should use observer.
+    class UseObserverCheck < Check
+
+      def interesting_nodes
+        [:defn, :call]
+      end
+
+      def interesting_files
+        /models\/.*rb/
+      end
+
+      def initialize
+        super
+        @callbacks = []
+      end
+
+      def evaluate_start(node)
+        if :after_create == node.message
+          remember_callbacks(node)
+        elsif :defn == node.node_type and @callbacks.include?(node.message_name.to_s)
+          add_error "use observer" if use_observer?(node)
+        end
+      end
+
+      private
+
+      def remember_callbacks(node)
+        @callbacks << eval(node.arguments.to_ruby).to_s
+      end
+
+      def use_observer?(node)
+        node.recursive_children do |child|
+          return true if :call == child.node_type and :const == child.subject.node_type and child.message.to_s =~ /^deliver_/
+        end
+        false
+      end
+    end
+  end
+end

lib/rails_best_practices/core/visitable_sexp.rb

@@ -81,6 +81,12 @@ def false_node
       self[3]
     end
   end
+
+  def message_name
+    if :defn == node_type
+      self[1]
+    end
+  end
 
   def method_body
     if :block == node_type

rails_best_practices.yml

@@ -12,3 +12,4 @@ NeedlessDeepNestingCheck: { nested_count: 2 }
 NotUseDefaultRouteCheck: {  }
 KeepFindersOnTheirOwnModelCheck: { }
 LawOfDemeterCheck: { }
+UseObserverCheck: { }

spec/rails_best_practices/checks/use_observer_check_spec.rb

@@ -0,0 +1,44 @@
+require File.join(File.dirname(__FILE__) + '/../../spec_helper')
+
+describe RailsBestPractices::Checks::UseObserverCheck do
+  before(:each) do
+    @runner = RailsBestPractices::Core::Runner.new(RailsBestPractices::Checks::UseObserverCheck.new)
+  end
+
+  it "should use observer" do
+    content =<<-EOF
+    class Project < ActiveRecord::Base
+      after_create :send_create_notification
+
+      private
+
+      def send_create_notification
+        self.members.each do |member|
+          ProjectMailer.deliver_notification(self, member)
+        end
+      end
+    end
+    EOF
+    @runner.check('app/models/project.rb', content)
+    errors = @runner.errors
+    errors.should_not be_empty
+    errors[0].to_s.should == "app/models/project.rb:6 - use observer"
+  end
+
+  it "should not use observer without callback" do
+    content =<<-EOF
+    class Project < ActiveRecord::Base
+      private
+
+      def send_create_notification
+        self.members.each do |member|
+          ProjectMailer.deliver_notification(self, member)
+        end
+      end
+    end
+    EOF
+    @runner.check('app/models/project.rb', content)
+    errors = @runner.errors
+    errors.should be_empty
+  end
+end