More cleaning up on rails generators load path.

8 files changed, 84 insertions, 114 deletions

diff --git a/railties/bin/rails b/railties/bin/rails
index b8b2d6188f..afcd9fd0be 100755
--- a/railties/bin/rails
+++ b/railties/bin/rails
@@ -22,6 +22,6 @@ ARGV << "--help" if ARGV.empty?
 
 
 require 'rails/generators'
-require 'rails/generators/rails/app/app_generator'
+require 'generators/rails/app/app_generator'
 
 Rails::Generators::AppGenerator.start
diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb
index 45ae1fd30a..34fe39f47e 100644
--- a/railties/lib/rails/generators.rb
+++ b/railties/lib/rails/generators.rb
@@ -131,51 +131,35 @@ module Rails
     # Rails looks for is the first and last parts of the namespace.
     #
     def self.find_by_namespace(name, base=nil, context=nil) #:nodoc:
-      base = "rails" unless base || context || name.to_s.include?(?:)
-
       lookups = []
       lookups << "#{base}:#{name}"    if base
       lookups << "#{name}:#{context}" if context
+      lookups << "#{name}:#{name}"    unless name.to_s.include?(?:)
       lookups << "#{name}"
       lookups << "rails:#{name}"      unless base || context || name.to_s.include?(?:)
 
-      # Mount regexps to lookup
-      regexps = []
-      regexps << /^#{base}:[\w:]*#{name}$/    if base
-      regexps << /^#{name}:[\w:]*#{context}$/ if context
-      regexps << /^[(#{name}):]+$/
-      regexps.uniq!
-
       # Check if generator happens to be loaded
-      checked = subclasses.dup
-      klass   = find_by_regexps(regexps, checked)
+      klass = subclasses.find { |k| lookups.include?(k.namespace) }
       return klass if klass
 
-      # Try to require other generators by looking in load_paths
+      # Try to load generator from $LOAD_PATH
+      checked = subclasses.dup
       lookup(lookups)
+
       unchecked = subclasses - checked
-      klass = find_by_regexps(regexps, unchecked)
+      klass     = unchecked.find { |k| lookups.include?(k.namespace) }
       return klass if klass
 
-      # Invoke fallbacks
       invoke_fallbacks_for(name, base) || invoke_fallbacks_for(context, name)
     end
 
-    # Tries to find a generator which the namespace match the regexp.
-    def self.find_by_regexps(regexps, klasses)
-      klasses.find do |klass|
-        namespace = klass.namespace
-        regexps.find { |r| namespace =~ r }
-      end
-    end
-
     # Receives a namespace, arguments and the behavior to invoke the generator.
     # It's used as the default entry point for generate, destroy and update
     # commands.
     def self.invoke(namespace, args=ARGV, config={})
       names = namespace.to_s.split(':')
       if klass = find_by_namespace(names.pop, names.shift)
-        args << "--help" if klass.arguments.any? { |a| a.required? } && args.empty?
+        args << "--help" if args.empty? && klass.arguments.any? { |a| a.required? }
         klass.start(args, config)
       else
         puts "Could not find generator #{namespace}."
@@ -187,7 +171,8 @@ module Rails
       builtin = Rails::Generators.builtin.each { |n| n.sub!(/^rails:/, '') }
       builtin.sort!
 
-      lookup("*")
+      # TODO Fix me
+      # lookup("*")
       others  = subclasses.map{ |k| k.namespace }
       others -= Rails::Generators.builtin
       others.sort!
@@ -224,28 +209,38 @@ module Rails
 
       # Receives namespaces in an array and tries to find matching generators
       # in the load path.
-      def self.lookup(attempts) #:nodoc:
-        attempts.compact!
-        attempts.uniq!
-
-        attempts.each do |attempt|
-          last = attempt.split(':').last
-
-          # TODO Support rails_generators
-          require_generator "generators/#{attempt.gsub(':', '/')}/#{last}_generator"
-          require_generator "generators/#{attempt.gsub(':', '/')}_generator"
+      def self.lookup(namespaces) #:nodoc:
+        paths = namespaces_to_paths(namespaces)
+
+        paths.each do |path|
+          ["generators", "rails_generators"].each do |base|
+            path = "#{base}/#{path}_generator"
+
+            begin
+              require path
+              return
+            rescue LoadError => e
+              raise unless e.message =~ /#{Regexp.escape(path)}$/
+            rescue NameError => e
+              raise unless e.message =~ /Rails::Generator([\s(::)]|$)/
+              warn "[WARNING] Could not load generator #{path.inspect} because it's a Rails 2.x generator, which is not supported anymore. Error: #{e.message}"
+            end
+          end
         end
       end
 
-      def self.require_generator(path)
-        begin
-          require path
-        rescue LoadError => e
-          raise unless e.message =~ /#{Regexp.escape(path)}$/
-        rescue NameError => e
-          raise unless e.message =~ /Rails::Generator/
-          warn "[WARNING] Could not load generator at #{path.inspect} because it's a Rails 2.x generator, which is not supported anymore. Error: #{e.message}"
+      # Convert namespaces to paths by replacing ":" for "/" and adding
+      # an extra lookup. For example, "rails:model" should be searched
+      # in both: "rails/model/model_generator" and "rails/model_generator".
+      def self.namespaces_to_paths(namespaces) #:nodoc:
+        paths = []
+        namespaces.each do |namespace|
+          pieces = namespace.split(":")
+          paths << pieces.dup.push(pieces.last).join("/")
+          paths << pieces.join("/")
         end
+        paths.uniq!
+        paths
       end
 
   end
diff --git a/railties/test/fixtures/lib/generators/active_record/fixjour/fixjour_generator.rb b/railties/test/fixtures/lib/generators/active_record/fixjour_generator.rb
index a7d079a1bc..7a4edb8bcb 100644
--- a/railties/test/fixtures/lib/generators/active_record/fixjour/fixjour_generator.rb
+++ b/railties/test/fixtures/lib/generators/active_record/fixjour_generator.rb
@@ -1,4 +1,4 @@
-require 'rails/generators/active_record'
+require 'generators/active_record'
 
 module ActiveRecord
   module Generators
diff --git a/railties/test/fixtures/lib/generators/rails/javascripts_generator.rb b/railties/test/fixtures/lib/generators/rails/javascripts_generator.rb
deleted file mode 100644
index cad5e96784..0000000000
--- a/railties/test/fixtures/lib/generators/rails/javascripts_generator.rb
+++ /dev/null
@@ -1,4 +0,0 @@
-module Rails::Generators
-  class JavascriptsGenerator < Rails::Generators::NamedBase
-  end
-end
diff --git a/railties/test/fixtures/vendor/gems/gems/wrong/lib/generators/wrong_generator.rb b/railties/test/fixtures/lib/generators/wrong_generator.rb
index 6aa7cb052e..6aa7cb052e 100644
--- a/railties/test/fixtures/vendor/gems/gems/wrong/lib/generators/wrong_generator.rb
+++ b/railties/test/fixtures/lib/generators/wrong_generator.rb
diff --git a/railties/test/fixtures/vendor/another_gem_path/xspec/lib/generators/xspec_generator.rb b/railties/test/fixtures/vendor/another_gem_path/xspec/lib/generators/xspec_generator.rb
deleted file mode 100644
index cd477eb4c9..0000000000
--- a/railties/test/fixtures/vendor/another_gem_path/xspec/lib/generators/xspec_generator.rb
+++ /dev/null
@@ -1,2 +0,0 @@
-class XspecGenerator < Rails::Generators::NamedBase
-end
diff --git a/railties/test/fixtures/vendor/plugins/mspec/lib/rails_generators/mspec_generator.rb b/railties/test/fixtures/vendor/plugins/mspec/lib/rails_generators/mspec_generator.rb
deleted file mode 100644
index 191bdbf2fc..0000000000
--- a/railties/test/fixtures/vendor/plugins/mspec/lib/rails_generators/mspec_generator.rb
+++ /dev/null
@@ -1,2 +0,0 @@
-class MspecGenerator < Rails::Generators::NamedBase
-end
diff --git a/railties/test/generators_test.rb b/railties/test/generators_test.rb
index 65d7cd2746..fdd72d6361 100644
--- a/railties/test/generators_test.rb
+++ b/railties/test/generators_test.rb
@@ -1,15 +1,20 @@
 require 'generators/generators_test_helper'
-require 'rails/generators/rails/model/model_generator'
-require 'rails/generators/test_unit/model/model_generator'
+require 'generators/rails/model/model_generator'
+require 'generators/test_unit/model/model_generator'
 require 'mocha'
 
 class GeneratorsTest < GeneratorsTestCase
+
   def setup
-    Rails::Generators.instance_variable_set(:@load_paths, nil)
-    Gem.stubs(:respond_to?).with(:loaded_specs).returns(false)
+    @path = File.expand_path("lib", Rails.root)
+    $LOAD_PATH.unshift(@path)
+  end
+
+  def teardown
+    $LOAD_PATH.delete(@path)
   end
 
-  def test_invoke_add_generators_to_raw_lookups
+  def test_simple_invoke
     TestUnit::Generators::ModelGenerator.expects(:start).with(["Account"], {})
     Rails::Generators.invoke("test_unit:model", ["Account"])
   end
@@ -34,8 +39,10 @@ class GeneratorsTest < GeneratorsTestCase
     Rails::Generators.invoke :model, ["Account"], :behavior => :skip
   end
 
-  def test_find_by_namespace_without_base_or_context_looks_into_rails_namespace
-    assert Rails::Generators.find_by_namespace(:model)
+  def test_find_by_namespace
+    klass = Rails::Generators.find_by_namespace("rails:model")
+    assert klass
+    assert_equal "rails:model", klass.namespace
   end
 
   def test_find_by_namespace_with_base
@@ -50,48 +57,32 @@ class GeneratorsTest < GeneratorsTestCase
     assert_equal "test_unit:model", klass.namespace
   end
 
-  def test_find_by_namespace_with_duplicated_name
-    klass = Rails::Generators.find_by_namespace(:foobar)
-    assert klass
-    assert_equal "foobar:foobar", klass.namespace
-  end
-
-  def test_find_by_namespace_lookup_to_the_rails_root_folder
+  def test_find_by_namespace_with_generator_on_root
     klass = Rails::Generators.find_by_namespace(:fixjour)
     assert klass
     assert_equal "fixjour", klass.namespace
   end
 
-  def test_find_by_namespace_lookup_to_deep_rails_root_folders
+  def test_find_by_namespace_in_subfolder
     klass = Rails::Generators.find_by_namespace(:fixjour, :active_record)
     assert klass
     assert_equal "active_record:fixjour", klass.namespace
   end
 
-  def test_find_by_namespace_lookup_traverse_folders
-    klass = Rails::Generators.find_by_namespace(:javascripts, :rails)
+  def test_find_by_namespace_with_duplicated_name
+    klass = Rails::Generators.find_by_namespace(:foobar)
     assert klass
-    assert_equal "rails:javascripts", klass.namespace
+    assert_equal "foobar:foobar", klass.namespace
   end
 
-  def test_find_by_namespace_lookup_to_vendor_folders
-    klass = Rails::Generators.find_by_namespace(:mspec)
-    assert klass
-    assert_equal "mspec", klass.namespace
+  def test_find_by_namespace_without_base_or_context_looks_into_rails_namespace
+    assert Rails::Generators.find_by_namespace(:model)
   end
 
-  def test_find_by_namespace_lookup_with_gem_specification
-    assert_nil Rails::Generators.find_by_namespace(:xspec)
-    Rails::Generators.instance_variable_set(:@load_paths, nil)
-
-    spec = Gem::Specification.new
-    spec.expects(:full_gem_path).returns(File.join(Rails.root, 'vendor', 'another_gem_path', 'xspec'))
-    Gem.expects(:respond_to?).with(:loaded_specs).returns(true)
-    Gem.expects(:loaded_specs).returns(:spec => spec)
-
-    klass = Rails::Generators.find_by_namespace(:xspec)
-    assert klass
-    assert_equal "xspec", klass.namespace
+  def test_find_by_namespace_show_warning_if_generator_cant_be_loaded
+    output = capture(:stderr) { Rails::Generators.find_by_namespace(:wrong) }
+    assert_match /\[WARNING\] Could not load generator/, output
+    assert_match /Rails 2\.x generator/, output
   end
 
   def test_builtin_generators
@@ -109,14 +100,6 @@ class GeneratorsTest < GeneratorsTestCase
     assert_equal "Others: active_record:fixjour, fixjour, foobar:foobar, mspec, rails:javascripts, xspec.", output
   end
 
-  def test_warning_is_shown_if_generator_cant_be_loaded
-    Rails::Generators.load_paths << File.join(Rails.root, "vendor", "gems", "gems", "wrong")
-    output = capture(:stderr){ Rails::Generators.find_by_namespace(:wrong) }
-
-    assert_match /\[WARNING\] Could not load generator at/, output
-    assert_match /Rails 2\.x generator/, output
-  end
-
   def test_no_color_sets_proper_shell
     Rails::Generators.no_color!
     assert_equal Thor::Shell::Basic, Thor::Base.shell
@@ -124,24 +107,6 @@ class GeneratorsTest < GeneratorsTestCase
     Thor::Base.shell = Thor::Shell::Color
   end
 
-  def test_rails_root_templates
-    template = File.join(Rails.root, "lib", "templates", "active_record", "model", "model.rb")
-
-    # Create template
-    mkdir_p(File.dirname(template))
-    File.open(template, 'w'){ |f| f.write "empty" }
-
-    output = capture(:stdout) do
-      Rails::Generators.invoke :model, ["user"], :destination_root => destination_root
-    end
-
-    assert_file "app/models/user.rb" do |content|
-      assert_equal "empty", content
-    end
-  ensure
-    rm_rf File.dirname(template)
-  end
-
   def test_fallbacks_for_generators_on_find_by_namespace
     Rails::Generators.fallbacks[:remarkable] = :test_unit
     klass = Rails::Generators.find_by_namespace(:plugin, :remarkable)
@@ -181,8 +146,26 @@ class GeneratorsTest < GeneratorsTestCase
     Rails::Generators.subclasses.delete(klass)
   end
 
+  def test_rails_root_templates
+    template = File.join(Rails.root, "lib", "templates", "active_record", "model", "model.rb")
+
+    # Create template
+    mkdir_p(File.dirname(template))
+    File.open(template, 'w'){ |f| f.write "empty" }
+
+    output = capture(:stdout) do
+      Rails::Generators.invoke :model, ["user"], :destination_root => destination_root
+    end
+
+    assert_file "app/models/user.rb" do |content|
+      assert_equal "empty", content
+    end
+  ensure
+    rm_rf File.dirname(template)
+  end
+
   def test_source_paths_for_not_namespaced_generators
-    mspec = Rails::Generators.find_by_namespace :mspec
-    assert mspec.source_paths.include?(File.join(Rails.root, "lib", "templates", "mspec"))
+    mspec = Rails::Generators.find_by_namespace :fixjour
+    assert mspec.source_paths.include?(File.join(Rails.root, "lib", "templates", "fixjour"))
   end
 end