diff --git a/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/base_translator.rb b/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/base_translator.rb index 406f70b1..011727f9 100644 --- a/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/base_translator.rb +++ b/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/base_translator.rb @@ -92,51 +92,154 @@ def visit_call_node(node) #: (Prism::CallNode) -> void def visit_attr(node) comments = node_rbs_comments(node) - return if comments.empty? + signatures = comments.signatures + return if signatures.empty? - return if comments.signatures.empty? + if signatures.size != 1 + raise Error, "RBS signatures for attributes do not support overloads" + end - signatures = apply_overloads_strategy( - comments.signatures, - method_name: node.message.to_s, - location: "#{@file}:#{node.location.start_line}", - ) + signature = signatures.first + return unless signature - known_annotations = nil #: Array[Spoom::RBS::Annotation]? + attr_name_nodes = node.arguments&.arguments || [] - signatures.each do |signature| - attr_type = ::RBS::Parser.parse_type(signature.string) - sig = RBI::Sig.new + if node.message == "attr_writer" && attr_name_nodes.size != 1 + raise Error, "RBS signatures for attr_writer require exactly one argument" + end - if node.message == "attr_writer" - if node.arguments&.arguments&.size != 1 - raise Error, "AttrWriter must have exactly one name" - end + first_attr_name_node, *additional_attr_name_nodes = attr_name_nodes + return unless first_attr_name_node - name = node.arguments&.arguments&.first #: as Prism::SymbolNode - sig.params << RBI::SigParam.new( - name.slice[1..-1], #: as String - @type_translator.translate(attr_type), - ) - end + rbs_type = ::RBS::Parser.parse_type(signature.string) + attr_type = @type_translator.translate(rbs_type) + first_attr_sig = build_attr_sig(node, first_attr_name_node, attr_type) + known_annotations = apply_member_annotations(comments.method_annotations, first_attr_sig) - sig.return_type = @type_translator.translate(attr_type) + # Format the signature with the correct indentation. + indent = node.location.start_column + first_attr_sig_source = format_attr_sig(first_attr_sig, indent:) + first_attr_sig_source.concat("\n") - known_annotations = apply_member_annotations(comments.method_annotations, sig) + # Replace the RBS comment in place to preserve the source layout. + @rewriter << Source::Replace.new( + signature.location.start_offset, + signature.location.end_offset, + pad_out_line_count(of: first_attr_sig_source, to_height_of: signature), + ) - @rewriter << Source::Replace.new( - signature.location.start_offset, - signature.location.end_offset, - pad_out_line_count(of: sig.string(max_line_length: @max_line_length), to_height_of: signature), + # Split multi-name calls so each additional attribute gets its own signature. + if additional_attr_name_nodes.any? + rewrite_multi_name_attr( + node, + first_attr_name_node, + additional_attr_name_nodes, + attr_type, + annotations: comments.method_annotations, ) - rescue ::RBS::ParsingError, ::RBI::Error - # Ignore signatures with errors - next end - if known_annotations - rewrite_member_annotations(comments.method_annotations, known: known_annotations) + # Handle member annotations like `# @without_runtime`. + rewrite_member_annotations(comments.method_annotations, known: known_annotations) + rescue ::RBS::ParsingError, ::RBI::Error + # Ignore signatures with errors + end + + #: (Prism::CallNode, Prism::Node, RBI::Type) -> RBI::Sig + def build_attr_sig(node, attr_name_node, attr_type) + sig = RBI::Sig.new + sig.return_type = attr_type + + if node.message == "attr_writer" + name = attr_name_node #: as Prism::SymbolNode + sig.params << RBI::SigParam.new( + name.slice[1..-1], #: as String + attr_type, + ) end + + sig + end + + #: (RBI::Sig, indent: Integer) -> String + def format_attr_sig(sig, indent:) + sig.string(indent:, max_line_length: @max_line_length) + .delete_prefix(" " * indent) + .chomp + end + + # `attr_reader(:a, :b)` becomes a single attr call: `attr_reader(:a)`. + #: (Prism::CallNode, Prism::Node) -> String + def single_attr_call_source(node, attr_name_node) + receiver = node.receiver + call = if receiver + "#{receiver.slice}#{node.call_operator}#{node.message}" + else + node.message.to_s + end + + if node.opening_loc + "#{call}(#{attr_name_node.slice})" + else + "#{call} #{attr_name_node.slice}" + end + end + + # Split a multi-name attr into one call per name. The first signature is already written above the call, + # so only the remaining names need signatures here. + #: ( + #| Prism::CallNode, + #| Prism::Node, + #| Array[Prism::Node], + #| RBI::Type, + #| annotations: Array[Spoom::RBS::Annotation] + #| ) -> void + def rewrite_multi_name_attr(node, first_attr_name_node, additional_attr_name_nodes, attr_type, annotations:) + indent = node.location.start_column + # Replacing the original call removes every attr, so include the first attr in its replacement. + located_statements = [ + [single_attr_call_source(node, first_attr_name_node), first_attr_name_node.location.start_line], + ] + + additional_attr_name_nodes.each do |attr_name_node| + sig = build_attr_sig(node, attr_name_node, attr_type) + apply_member_annotations(annotations, sig) + located_statements << [format_attr_sig(sig, indent:), attr_name_node.location.start_line] + located_statements << [single_attr_call_source(node, attr_name_node), attr_name_node.location.start_line] + end + + # Replacing the original call removes its trailing comments, so keep them with the final attr call. + last_attr_name_node = additional_attr_name_nodes.last #: as Prism::Node + last_attr_source = located_statements.last.first + indented_newline = "\n#{" " * indent}" + @comments.each do |comment| + next unless comment.location.start_offset > last_attr_name_node.location.end_offset + next unless comment.location.start_offset < node.location.end_offset + + separator = if comment.location.start_line == last_attr_name_node.location.end_line + " " + else + indented_newline + end + last_attr_source.concat(separator).concat(comment.slice) + end + + replace_multi_name_attr(node, located_statements:) + end + + # Replace the original multi-name call with the generated single-name calls and signatures, one per line. + # @overridable + #: (Prism::CallNode, located_statements: Array[[String, Integer]]) -> void + def replace_multi_name_attr(node, located_statements:) + indent = " " * node.location.start_column + statements = located_statements.map { |source, _line| source } + + @rewriter << Source::Replace.new( + node.location.start_offset, + # Prism ends are exclusive; Source::Replace ends are inclusive. + node.location.end_offset - 1, + statements.join("\n#{indent}"), + ) end #: (Prism::DefNode, Spoom::RBS::Comments) -> void diff --git a/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/line_matching_translator.rb b/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/line_matching_translator.rb index f4c159b6..4fcb5ce4 100644 --- a/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/line_matching_translator.rb +++ b/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs/line_matching_translator.rb @@ -72,6 +72,43 @@ def insert_type_member(type_member, parent_node:, insert_pos:) @rewriter << Source::Insert.new(insert_pos, "; #{type_member}") end + # Keep each attr on its original line so Sorbet errors point to the correct source. + # @override + #: (Prism::CallNode, located_statements: Array[[String, Integer]]) -> void + def replace_multi_name_attr(node, located_statements:) + replacement = String.new + current_line = node.location.start_line + indent = " " * node.location.start_column + + located_statements.each do |source, source_line| + line_gap = source_line - current_line + if line_gap.positive? + replacement.concat("\n" * line_gap).concat(indent) + elsif !replacement.empty? + replacement.concat("; ") + end + + replacement.concat(source) + current_line = source_line + end + + remaining_lines = node.location.end_line - node.location.start_line - replacement.count("\n") + replacement.concat("\n" * remaining_lines) + + # Remove the indentation when moving the first attr off the original call line. + start_offset = node.location.start_offset + if located_statements.first&.last != node.location.start_line + start_offset = adjust_to_line_start(start_offset) + end + + @rewriter << Source::Replace.new( + start_offset, + # Prism ends are exclusive; Source::Replace ends are inclusive. + node.location.end_offset - 1, + replacement, + ) + end + # @override #: (Spoom::RBS::Annotation, is_known: bool) -> void def rewrite_annotation(annotation, is_known:) diff --git a/rbi/spoom.rbi b/rbi/spoom.rbi index 4d8abf11..118dcfe9 100644 --- a/rbi/spoom.rbi +++ b/rbi/spoom.rbi @@ -491,6 +491,7 @@ class Spoom::Coverage::Cards::Card < ::Spoom::Coverage::Template sig { params(template: ::String, title: T.nilable(::String), body: T.nilable(::String)).void } def initialize(template: T.unsafe(nil), title: T.unsafe(nil), body: T.unsafe(nil)); end + sig { returns(T.nilable(::String)) } def body; end sig { returns(T.nilable(::String)) } @@ -2354,7 +2355,10 @@ class Spoom::Location sig { override.params(other: ::BasicObject).returns(T.nilable(::Integer)) } def <=>(other); end + sig { returns(T.nilable(::Integer)) } def end_column; end + + sig { returns(T.nilable(::Integer)) } def end_line; end sig { returns(::String) } @@ -2363,6 +2367,7 @@ class Spoom::Location sig { params(other: ::Spoom::Location).returns(T::Boolean) } def include?(other); end + sig { returns(T.nilable(::Integer)) } def start_column; end sig { returns(T.nilable(::Integer)) } @@ -2843,16 +2848,19 @@ class Spoom::Poset::Element sig { returns(T::Array[E]) } def descendants; end + sig { returns(T::Set[Spoom::Poset::Element[E]]) } def dfroms; end sig { returns(T::Set[Spoom::Poset::Element[E]]) } def dtos; end + sig { returns(T::Set[Spoom::Poset::Element[E]]) } def froms; end sig { returns(T::Array[E]) } def parents; end + sig { returns(T::Set[Spoom::Poset::Element[E]]) } def tos; end sig { returns(E) } @@ -2966,9 +2974,14 @@ class Spoom::Sorbet::Config sig { void } def initialize; end + sig { returns(T::Array[::String]) } def allowed_extensions; end + def allowed_extensions=(_arg0); end + + sig { returns(T::Array[::String]) } def ignore; end + def ignore=(_arg0); end sig { returns(T::Boolean) } @@ -2983,7 +2996,10 @@ class Spoom::Sorbet::Config def paths; end def paths=(_arg0); end + + sig { returns(T::Array[::String]) } def typed_overrides; end + def typed_overrides=(_arg0); end private @@ -3050,6 +3066,7 @@ class Spoom::Sorbet::Errors::Error sig { params(other: T.untyped).returns(::Integer) } def <=>(other); end + sig { returns(T.nilable(::Integer)) } def code; end sig { returns(T.nilable(::String)) } @@ -3061,6 +3078,7 @@ class Spoom::Sorbet::Errors::Error sig { returns(T.nilable(::Integer)) } def line; end + sig { returns(T.nilable(::String)) } def message; end sig { returns(T::Array[::String]) } @@ -3409,6 +3427,9 @@ class Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::BaseTranslator < ::Spoo sig { params(comments: T::Array[::Prism::Comment]).void } def apply_type_aliases(comments); end + sig { params(node: ::Prism::CallNode, attr_name_node: ::Prism::Node, attr_type: ::RBI::Type).returns(::RBI::Sig) } + def build_attr_sig(node, attr_name_node, attr_type); end + sig { params(comments: T::Array[::Prism::Comment]).returns(T::Array[::Spoom::RBS::TypeAlias]) } def collect_type_aliases(comments); end @@ -3422,6 +3443,9 @@ class Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::BaseTranslator < ::Spoo end def extend_with(mixin_name, into:, at:); end + sig { params(sig: ::RBI::Sig, indent: ::Integer).returns(::String) } + def format_attr_sig(sig, indent:); end + sig do abstract .params( @@ -3435,6 +3459,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 { overridable.params(node: ::Prism::CallNode, located_statements: T::Array[[::String, ::Integer]]).void } + def replace_multi_name_attr(node, located_statements:); end + sig { overridable.params(annotation: ::Spoom::RBS::Annotation, is_known: T::Boolean).void } def rewrite_annotation(annotation, is_known:); end @@ -3447,9 +3474,23 @@ class Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::BaseTranslator < ::Spoo sig { params(annotations: T::Array[::Spoom::RBS::Annotation], known: T::Array[::Spoom::RBS::Annotation]).void } def rewrite_member_annotations(annotations, known:); end + sig do + params( + node: ::Prism::CallNode, + first_attr_name_node: ::Prism::Node, + additional_attr_name_nodes: T::Array[::Prism::Node], + attr_type: ::RBI::Type, + annotations: T::Array[::Spoom::RBS::Annotation] + ).void + end + def rewrite_multi_name_attr(node, first_attr_name_node, additional_attr_name_nodes, attr_type, annotations:); end + 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 { params(node: ::Prism::CallNode, attr_name_node: ::Prism::Node).returns(::String) } + def single_attr_call_source(node, attr_name_node); end + sig { params(node: ::Prism::CallNode).void } def visit_attr(node); end end @@ -3538,6 +3579,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, located_statements: T::Array[[::String, ::Integer]]).void } + def replace_multi_name_attr(node, located_statements:); end + sig { override.params(annotation: ::Spoom::RBS::Annotation, is_known: T::Boolean).void } def rewrite_annotation(annotation, is_known:); end @@ -3815,6 +3859,7 @@ class Spoom::Source::Delete < ::Spoom::Source::Edit sig { override.returns([::Integer, ::Integer]) } def range; end + sig { returns(::Integer) } def to; end sig { override.returns(::String) } @@ -3869,6 +3914,7 @@ class Spoom::Source::Replace < ::Spoom::Source::Edit sig { returns(::String) } def text; end + sig { returns(::Integer) } def to; end sig { override.returns(::String) } diff --git a/test/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs_test.rb b/test/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs_test.rb index bd00c79e..5c750758 100644 --- a/test/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs_test.rb +++ b/test/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs_test.rb @@ -317,24 +317,196 @@ class A to_pretty_format_for_humans: <<~RUBY, class A sig { returns(Integer) } - attr_accessor :a, :b + attr_accessor :a + sig { returns(Integer) } + attr_accessor :b sig { returns(Integer) } - attr_reader :c, :d + attr_reader :c + sig { returns(Integer) } + attr_reader :d sig { params(e: Integer).returns(Integer) } attr_writer :e end RUBY - to_line_matched_format_for_machines: :same_as_pretty_output, + to_line_matched_format_for_machines: <<~RUBY, + class A + sig { returns(Integer) } + attr_accessor :a; sig { returns(Integer) }; attr_accessor :b + + sig { returns(Integer) } + attr_reader :c; sig { returns(Integer) }; attr_reader :d + + sig { params(e: Integer).returns(Integer) } + attr_writer :e + end + RUBY + ) + end + + def test_translate_to_rbi_attr_sigs_with_annotations_and_multiple_names + assert_rewrites_rbs( + from: <<~RUBY, + class A + # @without_runtime + #: Integer + attr_accessor :a, :b + end + RUBY + + to_pretty_format_for_humans: <<~RUBY, + class A + # @without_runtime + ::T::Sig::WithoutRuntime.sig { returns(Integer) } + attr_accessor :a + ::T::Sig::WithoutRuntime.sig { returns(Integer) } + attr_accessor :b + end + RUBY + + to_line_matched_format_for_machines: <<~RUBY, + class A + # RBS_REWRITTEN_ANNOTATION: @without_runtime + ::T::Sig::WithoutRuntime.sig { returns(Integer) } + attr_accessor :a; ::T::Sig::WithoutRuntime.sig { returns(Integer) }; attr_accessor :b + end + RUBY + ) + end + + def test_translate_to_rbi_parenthesized_attr_sigs_with_multiple_names + assert_rewrites_rbs( + from: <<~RUBY, + class A + #: Array[String] + attr_reader(:a, :b, :c) + end + RUBY + + to_pretty_format_for_humans: <<~RUBY, + class A + sig do + returns(::T::Array[String]) + end + attr_reader(:a) + sig do + returns(::T::Array[String]) + end + attr_reader(:b) + sig do + returns(::T::Array[String]) + end + attr_reader(:c) + end + RUBY + + to_line_matched_format_for_machines: <<~RUBY, + class A + sig { returns(::T::Array[String]) } + attr_reader(:a); sig { returns(::T::Array[String]) }; attr_reader(:b); sig { returns(::T::Array[String]) }; attr_reader(:c) + end + RUBY + + max_line_length: 20, ) end - def test_translate_to_rbi_attr_sigs_raises_on_writer_with_multiple_names + def test_translate_to_rbi_multiline_attr_sigs_with_multiple_names + assert_rewrites_rbs( + from: <<~RUBY, + class A + #: Integer + attr_reader( + :a, + b, + ) + + def after; end + end + RUBY + + to_pretty_format_for_humans: <<~RUBY, + class A + sig { returns(Integer) } + attr_reader(:a) + sig { returns(Integer) } + attr_reader(b) + + def after; end + end + RUBY + + to_line_matched_format_for_machines: <<~RUBY, + class A + sig { returns(Integer) } + + attr_reader(:a) + sig { returns(Integer) }; attr_reader(b) + + + def after; end + end + RUBY + ) + end + + def test_translate_to_rbi_preserves_trailing_comments_in_multi_name_attrs + assert_rewrites_rbs( + from: <<~RUBY, + class A + #: Integer + attr_reader( + # first attribute + :a, # explanation + # second attribute + :b, # last attribute + # after attributes + ) + end + RUBY + + to_pretty_format_for_humans: <<~RUBY, + class A + sig { returns(Integer) } + attr_reader(:a) + sig { returns(Integer) } + attr_reader(:b) # last attribute + # after attributes + end + RUBY + + to_line_matched_format_for_machines: <<~RUBY, + class A + sig { returns(Integer) } + + + attr_reader(:a) + + sig { returns(Integer) }; attr_reader(:b) # last attribute + # after attributes + + end + RUBY + ) + end + + def test_translate_to_rbi_attr_writer_sigs_raises_with_multiple_names + contents = <<~RB + #: Integer + attr_writer :a, :b + RB + + assert_raises(Translate::Error) do + rbs_comments_to_sorbet_sigs(contents) + end + end + + def test_translate_to_rbi_attr_writer_sigs_raises_without_name contents = <<~RB #: Integer - attr_writer :a, b + attr_writer RB assert_raises(Translate::Error) do