Skip to content

Conversation

@paracycle
Copy link
Member

Motivation

Consumers that define Active Model classes which override any of the attribute models cannot override the signature of the generated attribute methods since we are making it look like the methods are defined on the model class, and not on an included module.

Implementation

Start generating all attribute methods on a GeneratedAttributeMethods module that gets included into the model class.

Tests

Update the existing tests to match

@paracycle paracycle requested a review from bslobodin July 21, 2025 17:36
@paracycle paracycle requested a review from a team as a code owner July 21, 2025 17:36
@paracycle paracycle force-pushed the uk-fix-am-compiler branch from beaab9d to 8322b4a Compare July 21, 2025 17:43
Copy link

@bslobodin bslobodin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to find a bit of time to dig deeper, but a basic test locally ends up with rbis that have a few errors.

I also noticed this during bin/tapioca gem, not sure if red herring or not,

Changed strictness of sorbet/rbi/gems/tapioca@0.17.7-8322b4a3836d2efbef293a9963e1cd5b8e2be8fb.rbi to typed: false (conflicting with DSL files)

@paracycle
Copy link
Member Author

I'll try to find a bit of time to dig deeper, but a basic test locally ends up with rbis that have a few errors.

I took a cursory look at the type checking failures and they all seem to be related to the fact that the signature of the attribute method on the class doesn't match the one coming from the method on the GeneratedAttributeMethods module.

For example:

class Foo
  extend T::Sig
  include ActiveModel::Attributes

  attribute :name, :string

  sig { returns(String) }
  def name = super
end

should fail, since the signature of super is returns(T.nilable(String)), which doesn't match the declared return type of Foo#name.

All in all, I think your errors are now expected and are better signals for you to either change the signature on the custom me|thod to match the attribute one or to do the code change needed to make it match (like super || "No Name", for example).

@paracycle paracycle merged commit 8b4328e into main Jul 21, 2025
17 of 19 checks passed
@paracycle paracycle deleted the uk-fix-am-compiler branch July 21, 2025 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants