From b3862c1da8806e4a437921ce295e4c2cfa5da165 Mon Sep 17 00:00:00 2001 From: Julia Boutin Date: Tue, 12 May 2026 13:20:22 -0600 Subject: [PATCH 1/3] Skip RBS rewrite when file contains no RBS syntax When a file is not typed or contains no RBS comment syntax, we can skip running the RBS rewriter on it Co-authored-by: Matt Kubej --- lib/spoom/sorbet/sigils.rb | 7 + lib/spoom/sorbet/translate.rb | 2 +- .../translate/rbs_comments_to_sorbet_sigs.rb | 27 ++++ rbi/spoom.rbi | 14 ++ test/spoom/sorbet/sigils_test.rb | 6 + .../rbs_comments_to_sorbet_sigs_test.rb | 135 ++++++++++++++++++ 6 files changed, 190 insertions(+), 1 deletion(-) diff --git a/lib/spoom/sorbet/sigils.rb b/lib/spoom/sorbet/sigils.rb index 2ad4f77e..74c2f06c 100644 --- a/lib/spoom/sorbet/sigils.rb +++ b/lib/spoom/sorbet/sigils.rb @@ -44,6 +44,13 @@ def strictness_in_content(content) SIGIL_REGEXP.match(content)&.[](1) end + # returns true if the passed content contains a valid sigil + #: (String content) -> bool + def contains_valid_sigil?(content) + strictness = strictness_in_content(content) + !!strictness && valid_strictness?(strictness) + end + # returns a string which is the passed content but with the sigil updated to a new strictness #: (String content, String new_strictness) -> String def update_sigil(content, new_strictness) diff --git a/lib/spoom/sorbet/translate.rb b/lib/spoom/sorbet/translate.rb index deb8dd40..74cac87a 100644 --- a/lib/spoom/sorbet/translate.rb +++ b/lib/spoom/sorbet/translate.rb @@ -55,7 +55,7 @@ def sorbet_sigs_to_rbs_comments( # It also handles type members and class annotations. #: (String ruby_contents, file: String, ?max_line_length: Integer?) -> String def rbs_comments_to_sorbet_sigs(ruby_contents, file:, max_line_length: nil) - RBSCommentsToSorbetSigs.new(ruby_contents, file: file, max_line_length: max_line_length).rewrite + RBSCommentsToSorbetSigs.rewrite_if_needed(ruby_contents, file: file, max_line_length: max_line_length) end # Converts all `T.let` and `T.cast` nodes to RBS comments in the given Ruby code. diff --git a/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs.rb b/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs.rb index edbe318b..3929a2fb 100644 --- a/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs.rb +++ b/lib/spoom/sorbet/translate/rbs_comments_to_sorbet_sigs.rb @@ -7,6 +7,33 @@ module Translate class RBSCommentsToSorbetSigs < Translator include Spoom::RBS::ExtractRBSComments + RBS_ANNOTATION_MARKERS = [ + "# @abstract", + "# @interface", + "# @sealed", + "# @final", + "# @requires_ancestor:", + "# @override", + "# @overridable", + "# @without_runtime", + ].freeze #: Array[String] + RBS_REWRITE_PATTERN = Regexp.union(["#:", "#|", *RBS_ANNOTATION_MARKERS]).freeze #: Regexp + private_constant :RBS_ANNOTATION_MARKERS, :RBS_REWRITE_PATTERN + + class << self + #: (String source) -> bool + def contains_rbs_syntax?(source) + Sigils.contains_valid_sigil?(source) && source.match?(RBS_REWRITE_PATTERN) + end + + #: (String ruby_contents, file: String, ?max_line_length: Integer?) -> String + def rewrite_if_needed(ruby_contents, file:, max_line_length: nil) + return ruby_contents unless contains_rbs_syntax?(ruby_contents) + + new(ruby_contents, file:, max_line_length:).rewrite + end + end + #: (String, file: String, ?max_line_length: Integer?) -> void def initialize(ruby_contents, file:, max_line_length: nil) super(ruby_contents, file: file) diff --git a/rbi/spoom.rbi b/rbi/spoom.rbi index ea216e3d..d8f8b9f2 100644 --- a/rbi/spoom.rbi +++ b/rbi/spoom.rbi @@ -2823,6 +2823,9 @@ module Spoom::Sorbet::Sigils sig { params(path_list: T::Array[::String], new_strictness: ::String).returns(T::Array[::String]) } def change_sigil_in_files(path_list, new_strictness); end + sig { params(content: ::String).returns(T::Boolean) } + def contains_valid_sigil?(content); end + sig { params(path: T.any(::Pathname, ::String)).returns(T.nilable(::String)) } def file_strictness(path); end @@ -2938,8 +2941,19 @@ class Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs < ::Spoom::Sorbet::Trans sig { params(node: ::Prism::CallNode).void } def visit_attr(node); end + + class << self + sig { params(source: ::String).returns(T::Boolean) } + def contains_rbs_syntax?(source); end + + sig { params(ruby_contents: ::String, file: ::String, max_line_length: T.nilable(::Integer)).returns(::String) } + def rewrite_if_needed(ruby_contents, file:, max_line_length: T.unsafe(nil)); end + end end +Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::RBS_ANNOTATION_MARKERS = T.let(T.unsafe(nil), Array) +Spoom::Sorbet::Translate::RBSCommentsToSorbetSigs::RBS_REWRITE_PATTERN = T.let(T.unsafe(nil), Regexp) + class Spoom::Sorbet::Translate::SorbetAssertionsToRBSComments < ::Spoom::Sorbet::Translate::Translator sig do params( diff --git a/test/spoom/sorbet/sigils_test.rb b/test/spoom/sorbet/sigils_test.rb index f076ea24..175af98d 100644 --- a/test/spoom/sorbet/sigils_test.rb +++ b/test/spoom/sorbet/sigils_test.rb @@ -85,6 +85,12 @@ class A; end refute(Sigils.valid_strictness?(T.must(strictness))) end + def test_contains_valid_sigil + assert(Sigils.contains_valid_sigil?("# typed: true\nclass A; end\n")) + refute(Sigils.contains_valid_sigil?("class A; end\n")) + refute(Sigils.contains_valid_sigil?("# typed: foo\nclass A; end\n")) + end + def test_update_sigil_to_use_valid_strictness content = <<~STR # typed: ignore 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 bf0d2d8c..f584e5f9 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 @@ -678,6 +678,141 @@ class Range RB end + def test_contains_rbs_syntax_returns_true_for_supported_rbs_annotations + [ + "# @abstract", + "# @interface", + "# @sealed", + "# @final", + "# @requires_ancestor:", + "# @override", + "# @override(allow_incompatible: true)", + "# @override(allow_incompatible: false)", + "# @override(allow_incompatible: :visibility)", + "# @overridable", + "# @without_runtime", + ].each do |marker| + assert( + RBSCommentsToSorbetSigs.contains_rbs_syntax?(<<~RB), + # typed: true + + #{marker} + class Foo; end + RB + "#contains_rbs_syntax? should return true for files containing #{marker}", + ) + end + end + + def test_contains_rbs_syntax_returns_true_for_supported_typed_sigils + [ + "# typed: ignore", + "# typed: false", + "# typed: true", + "# typed: strict", + "# typed: strong", + "# typed: __STDLIB_INTERNAL", + ].each do |sigil| + assert( + RBSCommentsToSorbetSigs.contains_rbs_syntax?(<<~RB), + #{sigil} + + #: -> void + def foo; end + RB + "#contains_rbs_syntax? should return true for files containing #{sigil}", + ) + end + end + + def test_contains_rbs_syntax_returns_true_when_typed_sigil_is_after_other_magic_comments + assert(RBSCommentsToSorbetSigs.contains_rbs_syntax?(<<~RB)) + # frozen_string_literal: true + # typed: true + + class Foo + #: -> String + def foo; end + end + RB + + assert(RBSCommentsToSorbetSigs.contains_rbs_syntax?(<<~RB)) + # frozen_string_literal: true + + # typed: true + + class Foo + #: -> String + def foo; end + end + RB + end + + def test_contains_rbs_syntax_returns_true_for_rbs_comments + assert(RBSCommentsToSorbetSigs.contains_rbs_syntax?(<<~RB)) + # typed: true + + class Foo + #: -> String + def foo; end + end + RB + end + + def test_contains_rbs_syntax_returns_true_for_multiline_rbs_comments + assert(RBSCommentsToSorbetSigs.contains_rbs_syntax?(<<~RB)) + # typed: true + + class Foo + #: -> Array[ + #| String + #| ] + def foo; end + end + RB + end + + def test_contains_rbs_syntax_returns_false_for_files_without_typed_sigil + refute(RBSCommentsToSorbetSigs.contains_rbs_syntax?(<<~RB)) + #: -> void + def foo; end + RB + end + + def test_contains_rbs_syntax_returns_false_for_files_without_rbs_syntax + refute(RBSCommentsToSorbetSigs.contains_rbs_syntax?(<<~RB)) + # typed: true + + class Foo + def foo; end + end + RB + end + + def test_contains_rbs_syntax_returns_false_for_unrelated_yard_tags + refute(RBSCommentsToSorbetSigs.contains_rbs_syntax?(<<~RB)) + # typed: true + + # @param value [String] + # @return [String] + def foo(value); end + RB + end + + def test_rewrite_does_not_call_new_for_files_without_rbs_syntax + source = <<~RB + # typed: true + + class Foo + def foo; end + end + RB + + RBSCommentsToSorbetSigs.stub(:new, ->(*) { flunk("should not be called") }) do + assert_equal(source, RBSCommentsToSorbetSigs.rewrite_if_needed(source, file: "test.rb")) + end + end + private #: (String, ?max_line_length: Integer?) -> String From ca8a1fdb376b902ffc16921db67fda9be788b4be Mon Sep 17 00:00:00 2001 From: Julia Boutin Date: Tue, 12 May 2026 13:27:57 -0600 Subject: [PATCH 2/3] Add typed: true to test example files --- test/spoom/cli/srb/sigs_test.rb | 18 ++++++++++++++++++ .../rbs_comments_to_sorbet_sigs_test.rb | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/test/spoom/cli/srb/sigs_test.rb b/test/spoom/cli/srb/sigs_test.rb index 63cbf94c..68d72957 100644 --- a/test/spoom/cli/srb/sigs_test.rb +++ b/test/spoom/cli/srb/sigs_test.rb @@ -351,6 +351,8 @@ def foo = raise NotImplementedError, "Abstract method called" def test_translate_from_rbs_to_rbi @project.write!("file.rb", <<~RB) + # typed: true + #: -> void def foo; end RB @@ -366,6 +368,8 @@ def foo; end assert(result.status) assert_equal(<<~RB, @project.read("file.rb")) + # typed: true + sig { void } def foo; end RB @@ -373,6 +377,8 @@ def foo; end def test_translate_to_rbi_with_max_line_length_by_default @project.write!("file.rb", <<~RB) + # typed: true + #: ( #| param1: AVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongType, #| param2: AVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongType @@ -386,6 +392,8 @@ def foo(param1:, param2:); end assert(result.status) assert_equal(<<~RB, @project.read("file.rb")) + # typed: true + sig do params( param1: AVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongType, @@ -398,6 +406,8 @@ def foo(param1:, param2:); end def test_translate_to_rbi_without_max_line_length @project.write!("file.rb", <<~RB) + # typed: true + #: ( #| param1: AVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongType, #| param2: AVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongType @@ -411,6 +421,8 @@ def foo(param1:, param2:); end assert(result.status) assert_equal(<<~RB, @project.read("file.rb")) + # typed: true + sig { params(param1: AVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongType, param2: AVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongType).void } def foo(param1:, param2:); end RB @@ -444,6 +456,8 @@ def test_export_cant_locate_entry_point def test_export_create_rbi_file @project.write!("foo.gemspec", GEMSPEC) @project.write!("lib/foo.rb", <<~RB) + # typed: true + class Foo # ignored comment #: -> void @@ -473,6 +487,8 @@ def foo; end def test_export_check_sync_raises_if_rbi_is_not_up_to_date @project.write!("foo.gemspec", GEMSPEC) @project.write!("lib/foo.rb", <<~RB) + # typed: true + class Foo #: -> void def bar; end @@ -524,6 +540,8 @@ def foo; end def test_export_check_sync_does_not_raise_if_rbi_is_up_to_date @project.write!("foo.gemspec", GEMSPEC) @project.write!("lib/foo.rb", <<~RB) + # typed: true + class Foo #: -> void def bar; end 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 f584e5f9..38b3330d 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 @@ -817,7 +817,7 @@ def foo; end #: (String, ?max_line_length: Integer?) -> String def rbs_comments_to_sorbet_sigs(ruby_contents, max_line_length: nil) - Translate.rbs_comments_to_sorbet_sigs(ruby_contents, file: "test.rb", max_line_length: max_line_length) + RBSCommentsToSorbetSigs.new(ruby_contents, file: "test.rb", max_line_length: max_line_length).rewrite end end end From 5d5eb6a6e15eeda3ba0068f55dcdad71fa9ab8d0 Mon Sep 17 00:00:00 2001 From: Julia Boutin Date: Tue, 12 May 2026 13:24:16 -0600 Subject: [PATCH 3/3] Don't count unchanged files in translated file number --- lib/spoom/cli/srb/sigs.rb | 6 ++++-- test/spoom/cli/srb/sigs_test.rb | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/spoom/cli/srb/sigs.rb b/lib/spoom/cli/srb/sigs.rb index 8039290e..078744bc 100644 --- a/lib/spoom/cli/srb/sigs.rb +++ b/lib/spoom/cli/srb/sigs.rb @@ -213,8 +213,10 @@ def transform_files(files, &block) contents = contents.force_encoding(encoding) end - contents = block.call(file, contents) - File.write(file, contents) + new_contents = block.call(file, contents) + next if new_contents == contents + + File.write(file, new_contents) transformed_count += 1 rescue RBI::Error => error say_warning("Can't parse #{file}: #{error.message}") diff --git a/test/spoom/cli/srb/sigs_test.rb b/test/spoom/cli/srb/sigs_test.rb index 68d72957..831bb886 100644 --- a/test/spoom/cli/srb/sigs_test.rb +++ b/test/spoom/cli/srb/sigs_test.rb @@ -76,6 +76,24 @@ def test_translate_no_files refute(result.status) end + def test_translate_does_not_count_files_with_nothing_to_translate + @project.write!("file.rb", <<~RB) + # typed: true + + def foo; end + RB + + result = @project.spoom("srb sigs translate --from rbs --to rbi --no-color") + + assert_empty(result.err) + assert(result.status) + assert_equal(<<~OUT, result.out) + Translating signatures from `rbs` to `rbi` in `1` file... + + Translated signatures in `0` files. + OUT + end + def test_translate_only_selected_files @project.write!("a/file1.rb", <<~RB) sig { void }