Prevent duplicate entries in plugin Gemfile

This commit refactors the plugin `Gemfile.tt` to abstract much of the
logic into the `gemfile_entries` helper.  Doing so avoids adding
duplicate `Gemfile` entries (e.g. `gem "sqlite3"`), as well as
application-only `Gemfile` entries (e.g. `gem "puma"`), when generating
a plugin with a prerelease flag (e.g. `--dev`).

railties/lib/rails/generators/app_base.rb

@@ -108,16 +108,18 @@ module Rails
 
     private
       def gemfile_entries # :doc:
-        [rails_gemfile_entry,
-         asset_pipeline_gemfile_entry,
-         database_gemfile_entry,
-         web_server_gemfile_entry,
-         javascript_gemfile_entry,
-         hotwire_gemfile_entry,
-         css_gemfile_entry,
-         jbuilder_gemfile_entry,
-         psych_gemfile_entry,
-         cable_gemfile_entry].flatten.find_all(&@gem_filter)
+        [
+          rails_gemfile_entry,
+          asset_pipeline_gemfile_entry,
+          database_gemfile_entry,
+          web_server_gemfile_entry,
+          javascript_gemfile_entry,
+          hotwire_gemfile_entry,
+          css_gemfile_entry,
+          jbuilder_gemfile_entry,
+          psych_gemfile_entry,
+          cable_gemfile_entry,
+        ].flatten.compact.select(&@gem_filter)
       end
 
       def builder # :doc:
@@ -159,7 +161,8 @@ module Rails
       end
 
       def database_gemfile_entry # :doc:
-        return [] if options[:skip_active_record]
+        return if options[:skip_active_record]
+
         gem_name, gem_version = gem_for_database
         GemfileEntry.version gem_name, gem_version,
           "Use #{options[:database]} as the database for Active Record"
@@ -262,20 +265,13 @@ module Rails
           new(name, nil, comment, path: path)
         end
 
-        def version
-          version = super
-
-          if version.is_a?(Array)
-            version.join('", "')
-          else
-            version
-          end
-        end
-
         def to_s
-          [ ("# #{comment}\n" if comment),
-            ("# " if commented_out), "gem \"#{name}\"", (", \"#{version}\"" if version),
-            *options.map { |key, val| ", #{key}: #{val.inspect}" }
+          [
+            (comment.gsub(/^/, "# ").chomp + "\n" if comment),
+            ("# " if commented_out),
+            "gem \"#{name}\"",
+            *Array(version).map { |constraint| ", \"#{constraint}\"" },
+            *options.map { |key, value| ", #{key}: #{value.inspect}" },
           ].compact.join
         end
       end
@@ -312,12 +308,12 @@ module Rails
       end
 
       def jbuilder_gemfile_entry
-        return [] if options[:skip_jbuilder]
+        return if options[:skip_jbuilder]
         GemfileEntry.new "jbuilder", nil, "Build JSON APIs with ease [https://github.com/rails/jbuilder]", {}, options[:api]
       end
 
       def javascript_gemfile_entry
-        return [] if options[:skip_javascript]
+        return if options[:skip_javascript]
 
         if adjusted_javascript_option == "importmap"
           GemfileEntry.floats "importmap-rails", "Use JavaScript with ESM import maps [https://github.com/rails/importmap-rails]"
@@ -327,7 +323,7 @@ module Rails
       end
 
       def hotwire_gemfile_entry
-        return [] if options[:skip_javascript] || options[:skip_hotwire]
+        return if options[:skip_javascript] || options[:skip_hotwire]
 
         turbo_rails_entry =
           GemfileEntry.floats "turbo-rails", "Hotwire's SPA-like page accelerator [https://turbo.hotwired.dev]"
@@ -353,7 +349,7 @@ module Rails
       end
 
       def css_gemfile_entry
-        return [] unless options[:css]
+        return unless options[:css]
 
         if !using_node? && options[:css] == "tailwind"
           GemfileEntry.floats "tailwindcss-rails", "Use Tailwind CSS [https://github.com/rails/tailwindcss-rails]"
@@ -363,7 +359,7 @@ module Rails
       end
 
       def psych_gemfile_entry
-        return [] unless defined?(Rubinius)
+        return unless defined?(Rubinius)
 
         comment = "Use Psych as the YAML engine, instead of Syck, so serialized " \
                   "data can be read safely from different rubies (see http://git.io/uuLVag)"
@@ -371,11 +367,10 @@ module Rails
       end
 
       def cable_gemfile_entry
-        return [] if options[:skip_action_cable]
+        return if options[:skip_action_cable]
+
         comment = "Use Redis adapter to run Action Cable in production"
-        gems = []
-        gems << GemfileEntry.new("redis", "~> 4.0", comment, {}, true)
-        gems
+        GemfileEntry.new("redis", "~> 4.0", comment, {}, true)
       end
 
       def bundle_command(command, env = {})

railties/lib/rails/generators/rails/plugin/plugin_generator.rb

@@ -313,6 +313,33 @@ module Rails
       end
 
     private
+      def gemfile_entries
+        [
+          rails_gemfile_entry,
+          simplify_gemfile_entries(
+            database_gemfile_entry,
+            asset_pipeline_gemfile_entry,
+          ),
+        ].flatten.compact
+      end
+
+      def rails_gemfile_entry
+        if options[:skip_gemspec]
+          super
+        elsif rails_prerelease?
+          super.dup.tap do |entry|
+            entry.comment = <<~COMMENT
+              Your gem is dependent on a prerelease version of Rails. Once you can lock this
+              dependency down to a specific version, move it to your gemspec.
+            COMMENT
+          end
+        end
+      end
+
+      def simplify_gemfile_entries(*gemfile_entries)
+        gemfile_entries.flatten.compact.map { |entry| GemfileEntry.floats(entry.name) }
+      end
+
       def create_dummy_app(path = nil)
         dummy_path(path) if path
 

railties/lib/rails/generators/rails/plugin/templates/Gemfile.tt

@@ -1,36 +1,19 @@
 source "https://rubygems.org"
 git_source(:github) { |repo| "https://github.com/#{repo}.git" }
+<% unless options[:skip_gemspec] -%>
 
-<% if options[:skip_gemspec] -%>
-<%= "# " if rails_prerelease? -%>gem "rails", "<%= Array(rails_version_specifier).join("', '") %>"
-<% else -%>
 # Specify your gem's dependencies in <%= name %>.gemspec.
 gemspec
 <% end -%>
-<% unless options[:skip_active_record] -%>
-
-group :development do
-  gem "<%= gem_for_database[0] %>"
-end
-<% end -%>
-
-<% if engine? && !skip_sprockets? -%>
-<%= asset_pipeline_gemfile_entry %>
-
-<% end -%>
-<% if rails_prerelease? -%>
-# Your gem is dependent on a prerelease version of Rails. Once you can lock this
-# dependency down to a specific version, move it to your gemspec.
-<% gemfile_entries.each do |gemfile_entry| -%>
+<% gemfile_entries.each do |gemfile_entry| %>
 <%= gemfile_entry %>
-<% end -%>
-
 <% end -%>
 <% if RUBY_ENGINE == "ruby" -%>
-# Start debugger with binding.b -- Read more: https://github.com/ruby/debug
-# gem "debug", ">= 1.0.0", group: %i[ development test ]
-<% end -%>
-<% if RUBY_PLATFORM.match(/bccwin|cygwin|emx|mingw|mswin|wince|java/) -%>
 
-gem "tzinfo-data", platforms: %i[ mingw mswin x64_mingw jruby]
+# Start debugger with binding.b [https://github.com/ruby/debug]
+# gem "debug", ">= 1.0.0"
+<% end -%>
+<% if RUBY_PLATFORM.match?(/bccwin|cygwin|emx|mingw|mswin|wince|java/) -%>
+
+gem "tzinfo-data", platforms: %i[ mingw mswin x64_mingw jruby ]
 <% end -%>

railties/test/generators/plugin_generator_test.rb

@@ -201,8 +201,10 @@ class PluginGeneratorTest < Rails::Generators::TestCase
   def test_default_database_dependency_is_sqlite
     run_generator
     assert_file "test/dummy/config/database.yml", /sqlite/
-    assert_file "Gemfile" do |contents|
-      assert_match_sqlite3(contents)
+    if defined?(JRUBY_VERSION)
+      assert_gem "activerecord-jdbcsqlite3-adapter"
+    else
+      assert_gem "sqlite3"
     end
   end
 
@@ -231,6 +233,14 @@ class PluginGeneratorTest < Rails::Generators::TestCase
     assert_file "test/dummy/config/database.yml", /postgres/
   end
 
+  def test_no_duplicate_gemfile_entries_when_using_prerelease
+    run_generator_using_prerelease [destination_root, "--dev"]
+
+    assert_file "Gemfile" do |content|
+      assert_equal 1, content.scan(/gem "sqlite3"/).length
+    end
+  end
+
   def test_generation_runs_bundle_install
     generator([destination_root])
     run_generator_instance
@@ -651,6 +661,17 @@ class PluginGeneratorTest < Rails::Generators::TestCase
     end
   end
 
+  def test_dummy_application_respects_asset_pipeline_gem_choice
+    run_generator [destination_root, "--mountable", "--asset-pipeline=propshaft"]
+
+    assert_gem "propshaft"
+    assert_no_gem "sprockets-rails"
+    assert_no_file "test/dummy/config/initializers/assets.rb"
+    assert_file "test/dummy/config/environments/development.rb" do |content|
+      assert_no_match "config.assets", content
+    end
+  end
+
   def test_no_asset_pipeline_gem_when_no_dummy_application
     run_generator [destination_root, "--mountable", "--skip-test"]
 
@@ -892,14 +913,6 @@ class PluginGeneratorTest < Rails::Generators::TestCase
       ::DEFAULT_PLUGIN_FILES
     end
 
-    def assert_match_sqlite3(contents)
-      if defined?(JRUBY_VERSION)
-        assert_match(/group :development do\n  gem 'activerecord-jdbcsqlite3-adapter'\nend/, contents)
-      else
-        assert_match(/group :development do\n  gem "sqlite3"\nend/, contents)
-      end
-    end
-
     def with_simulated_app
       gemfile_path = "#{Rails.root}/Gemfile"
       Object.const_set("APP_PATH", Rails.root)