Skip to content

Fully qualify extend T::Generic & T::Helpers #1001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
dejmedus wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Fully qualify extend T::Generic & T::Helpers #1001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb8f34b
Fully qualify inserted extend T::Generic and T::Helpers
dejmedus Aug 4, 2026
0828e16
Correctly detect existing extend nodes
dejmedus Aug 5, 2026
3acd62c
Ensure Sorbet extensions precede generated code
dejmedus Aug 5, 2026
90f5578
Update spoom.rbi file
dejmedus Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 27 additions & 18 deletions lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/base_translator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,11 @@ def apply_class_annotations(node)
node.expression.location.end_offset
end

# Only translate (and `extend T::Helpers`) when there's at least one *known* class
# Only translate (and `extend ::T::Helpers`) when there's at least one *known* class
# annotation. A node with only unknown annotations (e.g. `@private`) is left untouched.
if comments.class_annotations.any?
unless already_extends?(node, /^(::)?T::Helpers$/)
extend_with("T::Helpers", into: node, at: insert_pos)
end
find_and_remove_existing_extend(node, /^(::)?T::Helpers$/)

@amomchilov amomchilov Aug 6, 2026 •
edited
Loading

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.

I'm not sure if we should try to fix a pre-existing extend T::Helpers like this.

It's a nice feature in theory, but narrow. There are many other explicit uses of T that would be problematic (T.untyped, T::Boolean, T.any, ...), which we couldn't patch up as easily/reliably. I'm inclined err of inaction, and require people fix their own explicit usages of T.

I suppose the distinction is: who wrote the T?

If our rewriter wrote it, we should be good stewards and ::T it. If devs wrote it explicitly in their own source, they should choose ::T if they want it (or rename their generic arg to not be called T).

@amomchilov amomchilov Aug 6, 2026

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.

@Morriar wdyt?

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

comments.annotations.reverse_each do |annotation|
content = case annotation.string
Expand Down Expand Up @@ -286,9 +285,8 @@ def apply_class_annotations(node)
next
end

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

type_params.each do |type_param|
type_member = "#{type_param.name} = type_member"
Expand Down Expand Up @@ -395,22 +393,33 @@ def rewrite_annotation(annotation, is_known:) = nil # no-op
#: (String mixin_name, into: PrismTypes::anyScopeNode, at: Integer) -> void
def extend_with(mixin_name, into:, at:) = raise

#: (PrismTypes::anyScopeNode, Regexp) -> bool
def already_extends?(node, constant_regex)
node.child_nodes.any? do |c|
next false unless c.is_a?(Prism::CallNode)
next false unless c.message == "extend"
next false unless c.receiver.nil? || c.receiver.is_a?(Prism::SelfNode)
next false unless c.arguments&.arguments&.size == 1
#: (PrismTypes::anyScopeNode, Regexp) -> void
def find_and_remove_existing_extend(node, constant_regex)
scope_statements(node).each do |statement|
next unless statement.is_a?(Prism::CallNode)
next unless statement.message == "extend"
next unless statement.receiver.nil? || statement.receiver.is_a?(Prism::SelfNode)
next unless statement.arguments&.arguments&.size == 1

arg = c.arguments&.arguments&.first
next false unless arg.is_a?(Prism::ConstantPathNode)
next false unless arg.slice.match?(constant_regex)
argument = statement.arguments&.arguments&.first
next unless argument.is_a?(Prism::ConstantPathNode)
next unless argument.slice.match?(constant_regex)

true
remove_extend(statement)
end
end

# @abstract
#: (Prism::CallNode) -> void
def remove_extend(node) = raise

#: (PrismTypes::anyScopeNode) -> Array[Prism::Node]
def scope_statements(node)
body = node.body
statements = body.is_a?(Prism::BeginNode) ? body.statements : body
statements&.body || []
end

#: (Array[Prism::Comment]) -> Array[Spoom::RBS::TypeAlias]
def collect_type_aliases(comments)
type_aliases = [] #: Array[Spoom::RBS::TypeAlias]
Expand Down
9 changes: 9 additions & 0 deletions lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/human_readable_translator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ def extend_with(mixin_name, into:, at:)
# trailing newline can't leave a blank line before `end` (unlike the lines that follow).
@rewriter << Source::Insert.new(at, "\n#{indent}extend #{mixin_name}\n")
end

# @override
#: (Prism::CallNode) -> void
def remove_extend(node)
@rewriter << Source::Delete.new(
adjust_to_line_start(node.location.start_offset),
adjust_to_line_end(node.location.end_offset),
)
end
end
end
end
Expand Down
9 changes: 9 additions & 0 deletions lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/line_matching_translator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ def extend_with(mixin_name, into:, at:)
@rewriter << Source::Insert.new(insert_pos, "; extend #{mixin_name}")
end

# @override
#: (Prism::CallNode) -> void
def remove_extend(node)
@rewriter << Source::Delete.new(
adjust_to_line_start(node.location.start_offset),
node.location.end_offset - 1,
)
end

# @override
#: (of: String, to_height_of: Spoom::RBS::Comment) -> String
def pad_out_line_count(of:, to_height_of:)
Expand Down
32 changes: 24 additions & 8 deletions rbi/spoom.rbi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3369,14 +3369,6 @@ class Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::BaseTranslator < ::Spoo

private

sig do
params(
node: T.any(::Prism::ClassNode, ::Prism::ModuleNode, ::Prism::SingletonClassNode),
constant_regex: ::Regexp
).returns(T::Boolean)
end
def already_extends?(node, constant_regex); end

sig do
abstract
.params(
Expand Down Expand Up @@ -3424,6 +3416,14 @@ class Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::BaseTranslator < ::Spoo
end
def extend_with(mixin_name, into:, at:); end

sig do
params(
node: T.any(::Prism::ClassNode, ::Prism::ModuleNode, ::Prism::SingletonClassNode),
constant_regex: ::Regexp
).void
end
def find_and_remove_existing_extend(node, constant_regex); end

sig do
abstract
.params(
Expand All @@ -3437,6 +3437,9 @@ class Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::BaseTranslator < ::Spoo
sig { overridable.params(of: ::String, to_height_of: ::Spoom::RBS::Comment).returns(::String) }
def pad_out_line_count(of:, to_height_of:); end

sig { abstract.params(node: ::Prism::CallNode).void }
def remove_extend(node); end

sig { overridable.params(annotation: ::Spoom::RBS::Annotation, is_known: T::Boolean).void }
def rewrite_annotation(annotation, is_known:); end

Expand All @@ -3452,6 +3455,13 @@ class Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::BaseTranslator < ::Spoo
sig { abstract.params(signature: ::Spoom::RBS::Signature, type_params: T::Array[::RBS::AST::TypeParam]).void }
def rewrite_type_params_signature(signature, type_params:); end

sig do
params(
node: T.any(::Prism::ClassNode, ::Prism::ModuleNode, ::Prism::SingletonClassNode)
).returns(T::Array[::Prism::Node])
end
def scope_statements(node); end

sig { params(node: ::Prism::CallNode).void }
def visit_attr(node); end
end
Expand Down Expand Up @@ -3496,6 +3506,9 @@ class Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::HumanReadableTranslator
end
def insert_type_member(type_member, parent_node:, insert_pos:); end

sig { override.params(node: ::Prism::CallNode).void }
def remove_extend(node); end

sig { override.params(signature: ::Spoom::RBS::Signature).void }
def rewrite_discarded_overload(signature); end

Expand Down Expand Up @@ -3540,6 +3553,9 @@ class Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::LineMatchingTranslator
sig { override.params(of: ::String, to_height_of: ::Spoom::RBS::Comment).returns(::String) }
def pad_out_line_count(of:, to_height_of:); end

sig { override.params(node: ::Prism::CallNode).void }
def remove_extend(node); end

sig { override.params(annotation: ::Spoom::RBS::Annotation, is_known: T::Boolean).void }
def rewrite_annotation(annotation, is_known:); end

Expand Down
Loading
Loading