From 43f756778f4e3c0682f99f4877a93637b836e86c Mon Sep 17 00:00:00 2001 From: Hartley McGuire Date: Wed, 4 Aug 2021 00:30:05 -0400 Subject: [PATCH] Move comment after newline in installed migrations When an engine's migration is installed in a rails application, a comment is inserted after any magic comments indicating the migration's source. However, the current implementation does not take into account whether there is an empty line after magic comments, and the generated migration will not pass rubocop's Layout/EmptyLineAfterMagicComment even if the engine's migration did. This commit changes the implementation to insert the new comment after a newline occuring after magic comments, if it exists. Example Engine Migration: ```ruby # frozen_string_literal: true # coding: ISO-8859-15 class CurrenciesHaveSymbols < ActiveRecord::Migration::Current end ``` Before change: ```ruby # frozen_string_literal: true # coding: ISO-8859-15 # This migration comes from bukkits (originally 1) class CurrenciesHaveSymbols < ActiveRecord::Migration::Current end ``` After change: ```ruby # frozen_string_literal: true # coding: ISO-8859-15 # This migration comes from bukkits (originally 1) class CurrenciesHaveSymbols < ActiveRecord::Migration::Current end ``` --- activerecord/lib/active_record/migration.rb | 6 ++++++ activerecord/test/cases/migration_test.rb | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 4d205d2a3a..99357f3f1a 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -957,6 +957,12 @@ module ActiveRecord magic_comments << magic_comment; "" end || break end + + if !magic_comments.empty? && source.start_with?("\n") + magic_comments << "\n" + source = source[1..-1] + end + source = "#{magic_comments}#{inserted_comment}#{source}" if duplicate = destination_migrations.detect { |m| m.name == migration.name } diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index b48feccd77..57afea8ef6 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -1426,7 +1426,7 @@ class CopyMigrationsTest < ActiveRecord::TestCase assert_equal [@migrations_path + "/4_people_have_hobbies.bukkits.rb", @migrations_path + "/5_people_have_descriptions.bukkits.rb"], copied.map(&:filename) expected = "# This migration comes from bukkits (originally 1)" - assert_equal expected, IO.readlines(@migrations_path + "/4_people_have_hobbies.bukkits.rb")[1].chomp + assert_equal expected, IO.readlines(@migrations_path + "/4_people_have_hobbies.bukkits.rb")[2].chomp files_count = Dir[@migrations_path + "/*.rb"].length copied = ActiveRecord::Migration.copy(@migrations_path, bukkits: MIGRATIONS_ROOT + "/to_copy") @@ -1529,8 +1529,8 @@ class CopyMigrationsTest < ActiveRecord::TestCase assert File.exist?(@migrations_path + "/4_currencies_have_symbols.bukkits.rb") assert_equal [@migrations_path + "/4_currencies_have_symbols.bukkits.rb"], copied.map(&:filename) - expected = "# frozen_string_literal: true\n# coding: ISO-8859-15\n# This migration comes from bukkits (originally 1)" - assert_equal expected, IO.readlines(@migrations_path + "/4_currencies_have_symbols.bukkits.rb")[0..2].join.chomp + expected = "# frozen_string_literal: true\n# coding: ISO-8859-15\n\n# This migration comes from bukkits (originally 1)" + assert_equal expected, IO.readlines(@migrations_path + "/4_currencies_have_symbols.bukkits.rb")[0..3].join.chomp files_count = Dir[@migrations_path + "/*.rb"].length copied = ActiveRecord::Migration.copy(@migrations_path, bukkits: MIGRATIONS_ROOT + "/magic")