Skip to content

Remove already_extends? helper method - #1014

Open
dejmedus wants to merge 1 commit into
mainfrom
jb-handle-already-extends-bug
Open

Remove already_extends? helper method#1014
dejmedus wants to merge 1 commit into
mainfrom
jb-handle-already-extends-bug

Conversation

@dejmedus

Copy link
Copy Markdown
Contributor

This PR removes the already_extends? method and its checks. Of the options to handle the already_extends? bug detailed in the issue, I believe removal is the most straightforward (maintains current behaviour) and least fragile

`already_extends?` looks through child nodes to find
pre-existing `extend`s. As they are deeper in the tree
they are not being detected

However, this logic isn't necessary as we will want
to insert an `extend` statement prior to anything
requiring it regardless. This commit removes the
helper method and its checks
@dejmedus
dejmedus marked this pull request as ready for review August 11, 2026 22:04
@dejmedus
dejmedus requested a review from a team as a code owner August 11, 2026 22:04
unless already_extends?(node, /^(::)?T::Helpers$/)
extend_with("T::Helpers", into: node, at: insert_pos)
end
extend_with("T::Helpers", into: node, at: insert_pos)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be fully qualified?

Suggested change
extend_with("T::Helpers", into: node, at: insert_pos)
extend_with("::T::Helpers", into: node, at: insert_pos)

unless already_extends?(node, /^(::)?T::Generic$/)
extend_with("T::Generic", into: node, at: insert_pos)
end
extend_with("T::Generic", into: node, at: insert_pos)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
extend_with("T::Generic", into: node, at: insert_pos)
extend_with("::T::Generic", into: node, at: insert_pos)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

already_extends? misses pre-existing extend calls

2 participants