Fix consistency issue between ActiveModel::Conversion and ActiveModel…

…::Lint

activemodel/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   `ActiveModel::Lint` now tests that `#to_key` returns `nil` when model's `#id` returns `nil`.
+
+    This change is done to make `ActiveModel::Conversion` compliant with the ActiveModel API
+    tested by `ActiveModel::Lint`.
+
+    *Santiago Bartesaghi*
+
+
 *   Support infinite ranges for `LengthValidator`s `:in`/`:within` options
 
     ```ruby

activemodel/lib/active_model/conversion.rb

@@ -43,7 +43,7 @@ def to_model
     end
 
     # Returns an Array of all key attributes if any of the attributes is set, whether or not
-    # the object is persisted. Returns +nil+ if there are no key attributes.
+    # the object is persisted. Returns +nil+ if there are no key attributes set.
     #
     #   class Person
     #     include ActiveModel::Conversion

activemodel/lib/active_model/lint.rb

@@ -23,15 +23,15 @@ module Lint
     # +self+.
     module Tests
       # Passes if the object's model responds to <tt>to_key</tt> and if calling
-      # this method returns +nil+ when the object is not persisted.
+      # this method returns +nil+ when there are no key attributes set.
       # Fails otherwise.
       #
       # <tt>to_key</tt> returns an Enumerable of all (primary) key attributes
       # of the model, and is used to a generate unique DOM id for the object.
       def test_to_key
         assert_respond_to model, :to_key
-        def model.persisted?() false end
-        assert model.to_key.nil?, "to_key should return nil when `persisted?` returns false"
+        def model.id() nil end
+        assert model.to_key.nil?, "to_key should return nil when there are no key attributes set"
       end
 
       # Passes if the object's model responds to <tt>to_param</tt> and if

activemodel/test/cases/lint_test.rb

@@ -9,7 +9,34 @@ class CompliantModel
     extend ActiveModel::Naming
     include ActiveModel::Conversion
 
-    def persisted?() false end
+    def persisted?
+      false
+    end
+
+    def errors
+      Hash.new([])
+    end
+  end
+
+  def setup
+    @model = CompliantModel.new
+  end
+end
+
+class PersistedLintTest < ActiveModel::TestCase
+  include ActiveModel::Lint::Tests
+
+  class CompliantModel
+    extend ActiveModel::Naming
+    include ActiveModel::Conversion
+
+    def persisted?
+      true
+    end
+
+    def id
+      1
+    end
 
     def errors
       Hash.new([])