if define reader method of ActiveModle::AttributeSet that named attributes, respond_to? raise NoMethodError

[troter]

Steps to reproduce

if define reader method of ActiveModle::AttributeSet that named attributes, respond_to? raise NoMethodError.

class Hoge
  include ActiveModel::Model
  include ActiveModel::Attributes
  attribute :hoge
end

Hoge.new.respond_to?(:hoge) # => true
Hoge.new.respond_to?(:foo) # => false

class Hoge
  attr_reader :attributes # @attributes is instance of ActiveModel::AttributeSet
end

Hoge.new.respond_to?(:hoge) # => true
Hoge.new.respond_to?(:foo) # => NoMethodError
# NoMethodError: undefined method `include?' for #<ActiveModel::AttributeSet:0x00007f8e2e621538>
# from /Users/iino/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/activemodel-5.2.0.rc1/lib/active_model/attribute_methods.rb:460:in `attribute_method?'

Expected behavior

It should return false.

use key? instead of include?

• https://github.com/rails/rails/blob/v5.2.0.rc1/activemodel/lib/active_model/attribute_methods.rb#L460

Actual behavior

NoMethodError

System configuration

Rails-5.2.0.rc1
Ruby-2.4.3

[composerinteralia]

ActiveModel::Attributes is not part of the public api, so you will have to use it at your own risk. Internally it sets an @attributes instance variable, and your attr_reader is accessing that variable.

ActiveModel::Attributes includes ActiveModel::AttributeMethods, which is part of the public api. The documentation for that module explains that in order to use ActiveModel::AttributeMethods you need to:

Define an attributes method which returns a hash with each attribute name in your model as hash key and the attribute value as hash value. Hash keys must be strings.

ActiveRecord takes advantage of the internal @attributes variable and defines the required attributes method like this:

def attributes
  @attributes.to_hash
end

You could actually do the same, but to be safe you should follow the public documentation and write:

def attributes
  { 'hoge' => hoge }
end

The short version: I don't think this is actually a bug.

@rafaelfranca would it make sense to use @_attributes or @internal_attributes or something along those lines so people are less likely to do something like this? Or maybe we can just close this issue.

[rafaelfranca]

I think this is a valid usage and this is how the system was designed to work. Once you add ActiveModel::Attributes to you module your source of truth is the AttributeSet. It looks like it is missing include? method. I'd also change ActiveModel::Attributes to define an attributes reader.

[rafaelfranca]

Closed by #31926