Avoid duplicated names on help description and show proper error message if trying to load a Rails 2.x generator.

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

3 files changed, 49 insertions, 16 deletions

diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb
index 49f32aa0db..a6a59f039f 100644
--- a/railties/lib/rails/generators.rb
+++ b/railties/lib/rails/generators.rb
@@ -152,7 +152,18 @@ module Rails
     end
     load_paths # Cache load paths. Needed to avoid __FILE__ pointing to wrong paths.
 
-    # Receives a namespace and tries different combinations to find a generator.
+    # Rails finds namespaces exactly as thor, with three conveniences:
+    #
+    #  1) If your generator name ends with generator, as WebratGenerator, it sets
+    #     its namespace to "webrat", so it can be invoked as "webrat" and not
+    #     "webrat_generator";
+    #
+    #  2) If your generator has a generators namespace, as Rails::Generators::WebratGenerator,
+    #     the namespace is set to "rails:generators:webrat", but Rails allows it
+    #     to be invoked simply as "rails:webrat". The "generators" is added
+    #     automatically when doing the lookup;
+    #
+    #  3) Rails looks in load paths and loads the generator just before it's going to be used.
     #
     # ==== Examples
     #
@@ -162,30 +173,29 @@ module Rails
     #
     #   "rails:generators:webrat", "webrat:generators:integration", "webrat"
     #
-    # If the namespace has ":" included we consider that a absolute namespace
-    # was given and the lookup above does not happen. Just the name is searched.
-    #
-    # Finally, it deals with one kind of shortcut:
+    # On the other hand, if "rails:webrat" is given, it will search for:
     #
-    #   find_by_namespace "test_unit:model"
+    #   "rails:generators:webrat", "rails:webrat"
     #
-    # It will search for generators at:
-    #
-    #   "test_unit:generators:model", "test_unit:model"
+    # Notice that the "generators" namespace is handled automatically by Rails,
+    # so you don't need to type it when you want to invoke a generator in specific.
     #
     def self.find_by_namespace(name, base=nil, context=nil) #:nodoc:
-      name, attempts = name.to_s, []
+      name, attempts = name.to_s, [ ]
 
       case name.count(':')
         when 1
           base, name = name.split(':')
           return find_by_namespace(name, base)
         when 0
-          attempts << "#{base}:generators:#{name}"    if base
-          attempts << "#{name}:generators:#{context}" if context
+          attempts += generator_names(base, name)    if base
+          attempts += generator_names(name, context) if context
       end
 
       attempts << name
+      attempts += generator_names(name, name) unless name.include?(?:)
+      attempts.uniq!
+
       unloaded = attempts - namespaces
       lookup(unloaded)
 
@@ -231,7 +241,10 @@ module Rails
 
         until tail.empty?
           others += Dir[File.join(path, *tail)].collect do |file|
-            file.split('/')[-tail.size, 2].join(':').sub(/_generator\.rb$/, '')
+            name = file.split('/')[-tail.size, 2]
+            name.last.sub!(/_generator\.rb$/, '')
+            name.uniq!
+            name.join(':')
           end
           tail.shift
         end
@@ -246,7 +259,7 @@ module Rails
       # Return all defined namespaces.
       #
       def self.namespaces #:nodoc:
-        Thor::Base.subclasses.map{ |klass| klass.namespace }
+        Thor::Base.subclasses.map { |klass| klass.namespace }
       end
 
       # Keep builtin generators in an Array[Array[group, name]].
@@ -257,6 +270,12 @@ module Rails
         end
       end
 
+      # By default, Rails strips the generator namespace to make invocations
+      # easier. This method generaters the both possibilities names.
+      def self.generator_names(first, second)
+        [ "#{first}:generators:#{second}", "#{first}:#{second}" ]
+      end
+
       # Try callbacks for the given base.
       #
       def self.invoke_fallbacks_for(name, base)
@@ -285,6 +304,9 @@ module Rails
           Dir[File.join(path, '**', attempts)].each do |file|
             begin
               require file
+            rescue NameError => e
+              raise unless e.message =~ /Rails::Generator/
+              warn "[WARNING] Could not load generator at #{file.inspect} because it's a Rails 2.x generator, which is not supported anymore"
             rescue Exception => e
               warn "[WARNING] Could not load generator at #{file.inspect}. Error: #{e.message}"
             end
diff --git a/railties/test/fixtures/lib/generators/foobar/foobar_generator.rb b/railties/test/fixtures/lib/generators/foobar/foobar_generator.rb
new file mode 100644
index 0000000000..d1de8c56fa
--- /dev/null
+++ b/railties/test/fixtures/lib/generators/foobar/foobar_generator.rb
@@ -0,0 +1,4 @@
+module Foobar
+  class FoobarGenerator < Rails::Generators::Base
+  end
+end
diff --git a/railties/test/generators_test.rb b/railties/test/generators_test.rb
index 8f1984c7d2..07f51eca01 100644
--- a/railties/test/generators_test.rb
+++ b/railties/test/generators_test.rb
@@ -45,6 +45,12 @@ class GeneratorsTest < GeneratorsTestCase
     assert_equal "test_unit:generators: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_add_generators_to_raw_lookups
     klass = Rails::Generators.find_by_namespace("test_unit:model")
     assert klass
@@ -101,14 +107,15 @@ class GeneratorsTest < GeneratorsTestCase
 
   def test_rails_generators_with_others_information
     output = capture(:stdout){ Rails::Generators.help }.split("\n").last
-    assert_equal "Others: active_record:fixjour, fixjour, mspec, rails:javascripts.", output
+    assert_equal "Others: active_record:fixjour, fixjour, foobar, mspec, rails:javascripts.", 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 /Error: uninitialized constant Rails::Generator/, output
+    assert_match /Rails 2\.x generator/, output
   end
 
   def test_no_color_sets_proper_shell