Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 34 additions & 1 deletion lib/spoom/sorbet/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,30 @@ class Config
#: bool
attr_accessor :no_stdlib

#: String?
attr_accessor :cache_dir

#: Symbol?
attr_accessor :parser

#: bool
attr_accessor :use_rbs

#: -> bool
def parse_with_prism? = @parser == :prism

#: -> bool
def use_rbs? = @use_rbs

#: -> void
def initialize
@paths = [] #: Array[String]
@ignore = [] #: Array[String]
@allowed_extensions = [] #: Array[String]
@no_stdlib = false #: bool
@cache_dir = nil #: String?
@parser = nil #: Symbol?
@use_rbs = false #: bool
end

#: -> Config
Expand All @@ -47,6 +65,9 @@ def copy
new_config.ignore.concat(@ignore)
new_config.allowed_extensions.concat(@allowed_extensions)
new_config.no_stdlib = @no_stdlib
new_config.cache_dir = @cache_dir
new_config.parser = @parser
new_config.use_rbs = @use_rbs
new_config
end

Expand All @@ -69,6 +90,8 @@ def options_string
opts.concat(ignore.map { |p| "--ignore '#{p}'" })
opts.concat(allowed_extensions.map { |ext| "--allowed-extension '#{ext}'" })
opts << "--no-stdlib" if @no_stdlib
opts << "--cache-dir='#{@cache_dir}'" if @cache_dir
opts << "--parser=#{@parser}" if @parser
opts.join(" ")
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.

use_rbs is missing from options_string. Both cache_dir and parser are included, but use_rbs is not, so a parse → options_string round-trip silently drops the RBS config.

Should we add something like:

Suggested change
opts.join(" ")
opts << "--parser=#{@parser}" if @parser
opts << "--enable-experimental-rbs-comments" if @use_rbs

(or whichever RBS flag makes the most sense as canonical)

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.

We may need to map Sorbet version <:> feature like we do in Tapioca

end

Expand Down Expand Up @@ -110,6 +133,16 @@ def parse_string(sorbet_config)
when /^--no-stdlib(=|$)/
config.no_stdlib = parse_bool_option(line)
next
when /^--cache-dir=/
value = parse_option(line)
config.cache_dir = value.empty? ? nil : value
next
when /^--parser=/
config.parser = parse_option(line).to_sym
next
when /^--enable-experimental-rbs-(comments|signatures|assertions)(=|$)/
config.use_rbs = parse_bool_option(line)
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 wondering about the three separate RBS flags (comments, signatures, assertions) collapsing into a single use_rbs boolean. If a config has:

--enable-experimental-rbs-comments
--enable-experimental-rbs-signatures=false

The last one wins and use_rbs becomes false. Is that intentional? If we only need a single boolean for Spoom's purposes, maybe worth a comment explaining the simplification.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was trying to keep it simple, since the whole point of the deprecation was to not have to worry about the distinction (like signatures=true and assetions=false).

I think I'll change this to use the ORing of values, and raise if there's a mix of trues/falses

next
when /^--.*=/
next
when /^--/
Expand Down Expand Up @@ -141,7 +174,7 @@ def parse_string(sorbet_config)

#: (String line) -> String
def parse_option(line)
T.must(line.split("=").last).strip
T.must(line.split("=", 2).last).strip
end

#: (String line) -> bool
Expand Down
21 changes: 21 additions & 0 deletions rbi/spoom.rbi
Original file line number Diff line number Diff line change
Expand Up @@ -2604,6 +2604,11 @@ class Spoom::Sorbet::Config
def allowed_extensions; end
def allowed_extensions=(_arg0); end

sig { returns(T.nilable(::String)) }
def cache_dir; end

def cache_dir=(_arg0); end

sig { returns(::Spoom::Sorbet::Config) }
def copy; end

Expand All @@ -2618,11 +2623,27 @@ class Spoom::Sorbet::Config
sig { returns(::String) }
def options_string; end

sig { returns(T::Boolean) }
def parse_with_prism?; end

sig { returns(T.nilable(::Symbol)) }
def parser; end

def parser=(_arg0); end

sig { returns(T::Array[::String]) }
def paths; end

def paths=(_arg0); end

sig { returns(T::Boolean) }
def use_rbs; end

def use_rbs=(_arg0); end

sig { returns(T::Boolean) }
def use_rbs?; end

class << self
sig { params(sorbet_config_path: ::String).returns(::Spoom::Sorbet::Config) }
def parse_file(sorbet_config_path); end
Expand Down
69 changes: 69 additions & 0 deletions test/spoom/sorbet/config_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,75 @@ def test_parses_no_stdlib_equals_false
refute_predicate(config, :no_stdlib)
end

def test_cache_dir_defaults_to_nil
config = Spoom::Sorbet::Config.parse_string(".")
assert_nil(config.cache_dir)
end

def test_parses_cache_dir
config = Spoom::Sorbet::Config.parse_string("--cache-dir=/tmp/sorbet-cache")
assert_equal("/tmp/sorbet-cache", config.cache_dir)
end

def test_parses_empty_cache_dir_as_nil
config = Spoom::Sorbet::Config.parse_string("--cache-dir=")
assert_nil(config.cache_dir)
end

def test_parses_parser_option_prism
config = Spoom::Sorbet::Config.parse_string(<<~CONFIG)
.
--parser=prism
CONFIG
assert_equal(:prism, config.parser)
assert_predicate(config, :parse_with_prism?)
end

def test_parses_parser_option_original
config = Spoom::Sorbet::Config.parse_string(<<~CONFIG)
.
--parser=original
CONFIG
assert_equal(:original, config.parser)
refute_predicate(config, :parse_with_prism?)
end

def test_parser_defaults_to_nil
config = Spoom::Sorbet::Config.parse_string(".")
assert_nil(config.parser)
refute_predicate(config, :parse_with_prism?)
end

def test_use_rbs_defaults_to_false
config = Spoom::Sorbet::Config.parse_string(".")
refute_predicate(config, :use_rbs?)
end

def test_use_rbs_with_rbs_comments
config = Spoom::Sorbet::Config.parse_string("--enable-experimental-rbs-comments")
assert_predicate(config, :use_rbs?)
end

def test_use_rbs_with_rbs_signatures
config = Spoom::Sorbet::Config.parse_string("--enable-experimental-rbs-signatures")
assert_predicate(config, :use_rbs?)
end

def test_use_rbs_with_rbs_assertions
config = Spoom::Sorbet::Config.parse_string("--enable-experimental-rbs-assertions")
assert_predicate(config, :use_rbs?)
end

def test_use_rbs_equals_false
config = Spoom::Sorbet::Config.parse_string("--enable-experimental-rbs-comments=false")
refute_predicate(config, :use_rbs?)
end

def test_use_rbs_equals_true
config = Spoom::Sorbet::Config.parse_string("--enable-experimental-rbs-comments=true")
assert_predicate(config, :use_rbs?)
end

def test_parses_a_config_string_with_mixed_options
config = Spoom::Sorbet::Config.parse_string(<<~CONFIG)
a
Expand Down
Loading