Deprecate locking of dirty records

3 files changed, 18 insertions, 2 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index e4fb363da5..0be9784d8c 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Deprecate locking records with unpersisted changes.
+
+    *Marc Schütz*
+
 *   Deprecate `ColumnDumper#migration_keys`.
 
     *Ryuta Kamizono*
diff --git a/activerecord/lib/active_record/locking/pessimistic.rb b/activerecord/lib/active_record/locking/pessimistic.rb
index e73cb4fc12..263e2a5f7f 100644
--- a/activerecord/lib/active_record/locking/pessimistic.rb
+++ b/activerecord/lib/active_record/locking/pessimistic.rb
@@ -59,7 +59,16 @@ module ActiveRecord
       # or pass true for "FOR UPDATE" (the default, an exclusive row lock). Returns
       # the locked record.
       def lock!(lock = true)
-        reload(lock: lock) if persisted?
+        if persisted?
+          if changed?
+            ActiveSupport::Deprecation.warn(<<-MSG.squish)
+              Locking a record with unpersisted changes is deprecated and will raise an
+              exception in Rails 5.2. Use `save` to persist the changes, or `reload` to
+              discard them explicitly.
+            MSG
+          end
+          reload(lock: lock)
+        end
         self
       end
 
diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
index 9e42242e55..23095618a4 100644
--- a/activerecord/test/cases/locking_test.rb
+++ b/activerecord/test/cases/locking_test.rb
@@ -536,7 +536,10 @@ unless in_memory_db?
         Person.transaction do
           person = Person.find 1
           old, person.first_name = person.first_name, "fooman"
-          person.lock!
+          # Locking a dirty record is deprecated
+          assert_deprecated do
+            person.lock!
+          end
           assert_equal old, person.first_name
         end
       end