From 6674c844b9d1d116cc5c5237c4d0d1d3c152a0cd Mon Sep 17 00:00:00 2001
From: Pat George <pat.george@gmail.com>
Date: Sat, 5 Jun 2010 05:50:16 +0800
Subject: [PATCH] Mail API checker no longer reports false positives from
 updated Mail files.

---
 lib/application_checker.rb       | 18 ++++++++------
 test/application_checker_test.rb | 42 ++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/lib/application_checker.rb b/lib/application_checker.rb
index 21dce00..da43304 100644
--- a/lib/application_checker.rb
+++ b/lib/application_checker.rb
@@ -165,8 +165,8 @@ module Rails
         end
 
         files = []
-        ["recipients ", "attachment ", "subject ", "from "].each do |v|
-          lines = grep_for(v, "app/models/")
+        ["recipients ", "attachment(?!s) ", "(?<!:)subject ", "(?<!:)from "].each do |v|
+          lines = grep_for_with_perl_regex(v, "app/models/")
           files += extract_filenames(lines) || []
         end
 
@@ -300,16 +300,20 @@ module Rails
       end
 
     private
+      def grep_for_with_perl_regex(text, where = "./", double_quote = false)
+        grep_for(text, where, double_quote, true)
+      end
+
       # Find a string in a set of files; calls +find_with_grep+ and +find_with_rak+
       # depending on platform.
       #
       # TODO: Figure out if this works on Windows.
-      def grep_for(text, where = "./", double_quote = false)
+      def grep_for(text, where = "./", double_quote = false, perl_regex = false)
         # If they're on Windows, they probably don't have grep.
         @probably_has_grep ||= (Config::CONFIG['host_os'].downcase =~ /mswin|windows|mingw/).nil?
 
         lines = if @probably_has_grep
-          find_with_grep(text, base_path + where, double_quote)
+          find_with_grep(text, base_path + where, double_quote, perl_regex)
         else
           find_with_rak(text, base_path + where, double_quote)
         end
@@ -326,13 +330,13 @@ module Rails
       end
 
       # Use the grep utility to find a string in a set of files
-      def find_with_grep(text, where, double_quote)
+      def find_with_grep(text, where, double_quote, perl_regex = false)
         value = ""
         # Specifically double quote for finding 'test_help'
         command = if double_quote
-                    "grep -r --exclude=\*.svn\* \"#{text}\" #{where}"
+                    "grep -r #{"-P" if perl_regex} --exclude=\*.svn\* \"#{text}\" #{where}"
                   else
-                    "grep -r --exclude=\*.svn\* '#{text}' #{where}"
+                    "grep -r #{"-P" if perl_regex} --exclude=\*.svn\* '#{text}' #{where}"
                   end
 
         Open3.popen3(command) do |stdin, stdout, stderr|
diff --git a/test/application_checker_test.rb b/test/application_checker_test.rb
index 40031d5..53bdb39 100644
--- a/test/application_checker_test.rb
+++ b/test/application_checker_test.rb
@@ -120,6 +120,48 @@ class ApplicationCheckerTest < ActiveSupport::TestCase
     assert @checker.alerts.has_key?("Old ActionMailer class API")
   end
 
+  def test_check_mailer_syntax_from
+    make_file("app/models/", "notifications.rb", "def signup\nfrom @user\n end")
+    @checker.check_mailers
+
+    assert @checker.alerts.has_key?("Old ActionMailer class API")
+  end
+
+  def test_check_mailer_syntax_subject
+    make_file("app/models/", "notifications.rb", "def signup\nsubject @subject\n end")
+    @checker.check_mailers
+
+    assert @checker.alerts.has_key?("Old ActionMailer class API")
+  end
+
+  def test_check_mailer_syntax_attachment
+    make_file("app/models/", "notifications.rb", "def signup\nattachment 'application/pdf' do |a|\n end")
+    @checker.check_mailers
+
+    assert @checker.alerts.has_key?("Old ActionMailer class API")
+  end
+
+  def test_new_check_mailer_syntax_from
+    make_file("app/models/", "notifications.rb", "def signup\n:from => @users\n end")
+    @checker.check_mailers
+
+    assert ! @checker.alerts.has_key?("Old ActionMailer class API")
+  end
+
+  def test_new_check_mailer_syntax_subject
+    make_file("app/models/", "notifications.rb", "def signup\n:subject => @users\n end")
+    @checker.check_mailers
+
+    assert ! @checker.alerts.has_key?("Old ActionMailer class API")
+  end
+
+  def test_new_check_mailer_syntax_attachments
+    make_file("app/models/", "notifications.rb", "def signup\nattachments['an-image.jp'] = File.read('an-image.jpg')\n end")
+    @checker.check_mailers
+
+    assert ! @checker.alerts.has_key?("Old ActionMailer class API")
+  end
+
   def test_check_mailer_api
     make_file("app/controllers/", "thing_controller.rb", "def signup\n Notifications.deliver_signup\n end")
     @checker.check_mailers