Will now output the relative path to a file rather than an absolute

lib/application_checker.rb

@@ -41,7 +41,7 @@ module Rails
         lines = grep_for("named_scope", "app/models/")
         files = extract_filenames(lines)
 
-        if files
+        unless files.empty?
           alert(
             "named_scope is now just scope",
             "The named_scope method has been renamed to just scope.",
@@ -173,9 +173,10 @@ module Rails
       # Check for old-style config.gem calls
       def check_gems
         lines = grep_for("config.gem ", "config/*.rb")
+        lines += grep_for("config.gem ", "config/**/*.rb")
         files = extract_filenames(lines)
 
-        if files
+        unless files.empty?
           alert(
             "Old gem bundling (config.gems)",
             "The old way of bundling is gone now.  You need a Gemfile for bundler.",
@@ -191,7 +192,7 @@ module Rails
         lines = grep_for("deliver_", "app/models/ #{base_path}app/controllers/ #{base_path}app/observers/")
         files = extract_filenames(lines)
 
-        if files
+        unless files.empty?
           alert(
             "Deprecated ActionMailer API",
             "You're using the old ActionMailer API to send e-mails in a controller, model, or observer.",
@@ -225,7 +226,7 @@ module Rails
                     grep_for("def manifest", g).empty? 
                   end.compact
 
-          if !files.empty?
+          unless files.empty?
             alert(
               "Old Rails generator API",
               "A plugin in the app is using the old generator API (a new one may be available at http://github.com/trydionel/rails3-generators).",
@@ -269,7 +270,7 @@ module Rails
         
         files = extract_filenames(lines)
 
-        if files
+        if !files.blank?
           alert(
             "Deprecated ERb helper calls",
             "Block helpers that use concat (e.g., form_for) should use <%= instead of <%.  The current form will continue to work for now, but you will get deprecation warnings since this form will go away in the future.",
@@ -303,7 +304,7 @@ module Rails
         lines = grep_for("ActionController::Base.cookie_verifier_secret = ", "config/**/*")
         files = extract_filenames(lines)
 
-        if files
+        unless files.empty?
           alert(
             "Deprecated cookie secret setting",
             "Previously, cookie secret was set directly on ActionController::Base; it's now config.secret_token.",
@@ -317,7 +318,7 @@ module Rails
         lines = grep_for("ActionController::Base.session = {", "config/**/*")
         files = extract_filenames(lines)
 
-        if files
+        unless files.empty?
           alert(
             "Deprecated session secret setting",
             "Previously, session secret was set directly on ActionController::Base; it's now config.secret_token.",
@@ -332,7 +333,7 @@ module Rails
         lines = grep_for("ActionController::Base.session_store", "config/**/*")
         files = extract_filenames(lines)
 
-        if files
+        unless files.empty?
           alert(
             "Old session store setting",
             "Previously, session store was set directly on ActionController::Base; it's now config.session_store :whatever.",
@@ -401,14 +402,18 @@ module Rails
       # Extract the filenames from the grep output
       def extract_filenames(output)
         if @probably_has_grep
-          extract_filenames_from_grep(output)
+          filenames = extract_filenames_from_grep(output)
         else
-          extract_filenames_from_rak(output)
+          filenames = extract_filenames_from_rak(output)
+        end
+
+        filenames.compact.map do |f|
+          f.gsub(base_path, "")
         end
       end
 
       def extract_filenames_from_grep(output)
-        return nil if output.empty?
+        return [] if output.empty?
 
         output.split("\n").map do |fn|
           if m = fn.match(/^(.+?):/)
@@ -418,7 +423,7 @@ module Rails
       end
 
       def extract_filenames_from_rak(output)
-        return nil if output.empty?
+        return [] if output.empty?
 
         output.split("\n").uniq
       end

test/application_checker_test.rb

@@ -15,7 +15,7 @@ FileUtils.mkdir_p BASE_ROOT
 module Rails
   module Upgrading
     class ApplicationChecker
-      attr_reader :alerts
+      attr_reader :alerts, :culprits
 
       def base_path
         BASE_ROOT + "/"
@@ -27,10 +27,12 @@ module Rails
 
       def initialize
         @alerts = {}
+        @culprits = {}
       end
 
       def alert(title, text, more_info_url, culprits)
-        @alerts[title] = [text, more_info_url, culprits]
+        @alerts[title] = [text, more_info_url]
+        @culprits[title] = culprits
       end
     end
   end
@@ -55,7 +57,9 @@ class ApplicationCheckerTest < ActiveSupport::TestCase
     make_file("app/models", "post.rb", "Post.find(:all)")
     @checker.check_ar_methods
 
-    assert @checker.alerts.has_key?("Soon-to-be-deprecated ActiveRecord calls")
+    key = "Soon-to-be-deprecated ActiveRecord calls"
+    assert @checker.alerts.has_key?(key)
+    assert_equal "app/models/post.rb", @checker.culprits[key].first
   end
   
   def test_check_svn_subdirs_are_not_included
@@ -132,6 +136,18 @@ class ApplicationCheckerTest < ActiveSupport::TestCase
 
     assert @checker.alerts.has_key?("Old gem bundling (config.gems)")
   end
+  
+  def test_check_gems_finds_nothing
+    @checker.check_gems
+
+    assert_equal false, @checker.alerts.has_key?("Old gem bundling (config.gems)")
+  end
+  
+  def test_check_mailer_finds_nothing
+    @checker.check_mailers
+
+    assert_equal false, @checker.alerts.has_key?("Old ActionMailer class API")
+  end
 
   def test_check_mailer_syntax
     make_file("app/models/", "notifications.rb", "def signup\nrecipients @users\n end")
@@ -223,6 +239,11 @@ class ApplicationCheckerTest < ActiveSupport::TestCase
 
     assert @checker.alerts.has_key?("Deprecated constant(s)")
   end
+  
+  def test_check_deprecated_cookie_finds_nothing
+    @checker.check_old_cookie_secret
+    assert_equal false, @checker.alerts.has_key?("Deprecated cookie secret setting")
+  end
 
   def test_check_deprecated_cookie_settings
     make_file("config/initializers/", "more_settings.rb", "ActionController::Base.cookie_verifier_secret = 'OMG'")
@@ -231,12 +252,22 @@ class ApplicationCheckerTest < ActiveSupport::TestCase
     assert @checker.alerts.has_key?("Deprecated cookie secret setting")
   end
 
+  def test_check_deprecated_session_finds_nothing
+    @checker.check_old_session_secret
+    assert_equal false, @checker.alerts.has_key?("Deprecated session secret setting")
+  end
+
   def test_check_deprecated_session_secret
     make_file("config/initializers/", "more_settings.rb", "ActionController::Base.session = {\n:whatever => 'woot'\n}")
     @checker.check_old_session_secret
 
     assert @checker.alerts.has_key?("Deprecated session secret setting")
   end
+  
+  def test_check_old_session_setting_finds_nothing
+    @checker.check_old_session_setting
+    assert_equal false, @checker.alerts.has_key?("Old session store setting")
+  end
 
   def test_check_deprecated_session_settings
     make_file("config/initializers/", "more_settings.rb", "ActionController::Base.session_store = :cookie\nthings.awesome(:whatever)")
@@ -264,7 +295,7 @@ class ApplicationCheckerTest < ActiveSupport::TestCase
     make_file("app/views/users/", "another_test.html.erb", "<b>blah blah blah</b><% @some_items.each do |item| %> <label>doo dah</label> <%= item %> <% end %>")
     @checker.check_old_helpers
 
-    assert_equal @checker.alerts.has_key?("Deprecated ERb helper calls"), false
+    assert_equal false, @checker.alerts.has_key?("Deprecated ERb helper calls")
   end
 
   def test_check_old_ajax_helpers

test/routes_upgrader_test.rb

@@ -158,19 +158,19 @@ end
 
   def test_generates_code_for_delete_route
     routes_code = %Q{
-      ActionController::Routing::Routes.draw do |map|
+ActionController::Routing::Routes.draw do |map|
-        map.sign_out '/sign_out', :controller => 'sessions', :action => 'destroy', :method => :delete
+  map.sign_out '/sign_out', :controller => 'sessions', :action => 'destroy', :method => :delete
-      end
+end
     }
 
     new_routes_code = %Q{
-      MyApplication::Application.routes.draw do
+MyApplication::Application.routes.draw do
-        match '/sign_out' => 'sessions#destroy', :as => :sign_out, :via => 'delete'
+  match '/sign_out' => 'sessions#destroy', :as => :sign_out, :via => 'delete'
-      end
+end
     }
 
     upgrader = Rails::Upgrading::RoutesUpgrader.new
     upgrader.routes_code = routes_code
-    assert_equal new_routes_code, upgrader.generate_new_routes
+    assert_equal new_routes_code.strip, upgrader.generate_new_routes.strip
   end
 end