Skip to content

Make Config attributes read-only #890

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

Open
amomchilov wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Make Config attributes read-only #890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion lib/spoom/coverage.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def report(context, snapshots, palette:)
#: (Context context) -> FileTree
def file_tree(context)
config = context.sorbet_config
config.ignore += ["test"]
config.ignore << "test"

files = context.srb_files(with_config: config, include_rbis: false)
FileTree.new(files)
Expand Down
47 changes: 29 additions & 18 deletions lib/spoom/sorbet/config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,17 @@ class Config
DEFAULT_ALLOWED_EXTENSIONS = [".rb", ".rbi"].freeze #: Array[String]

#: Array[String]
attr_accessor :paths, :ignore, :allowed_extensions
attr_reader :paths, :ignore, :allowed_extensions

#: bool
attr_accessor :no_stdlib
attr_reader :no_stdlib
Copy link
Copy Markdown
Contributor

@Morriar Morriar Apr 2, 2026

Choose a reason for hiding this comment

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

So we can add new paths/ignore/allowed_extensions but not change this flag?

Should we freeze the arrays then? We could also type them as Enumerable[String].

There are places where we do expect the config to not be read only: https://github.com/Shopify/spoom/blob/main/lib/spoom/coverage.rb#L19

Copy link
Copy Markdown
Contributor Author

@amomchilov amomchilov Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

So we can add new paths/ignore/allowed_extensions but not change this flag?

Yeah basically.

Should we freeze the arrays then?

Not without changing the places like the one you mentioned, or that I change in this PR.

This is very much a half-measure, but still a marginal improvement

Copy link
Copy Markdown
Contributor

@Morriar Morriar Apr 2, 2026

Choose a reason for hiding this comment

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

What are the benefits of making it half-read-only? Can we keep it writable?


#: -> void
def initialize
@paths = [] #: Array[String]
@ignore = [] #: Array[String]
@allowed_extensions = [] #: Array[String]
@no_stdlib = false #: bool
#: (?paths: Array[String], ?ignore: Array[String], ?allowed_extensions: Array[String], ?no_stdlib: bool) -> void
def initialize(paths: [], ignore: [], allowed_extensions: [], no_stdlib: false)
@paths = paths
@ignore = ignore
@allowed_extensions = allowed_extensions
@no_stdlib = no_stdlib
end

#: (Config source) -> void
Expand Down Expand Up @@ -78,35 +78,40 @@ def parse_file(sorbet_config_path)

#: (String sorbet_config) -> Spoom::Sorbet::Config
def parse_string(sorbet_config)
config = Config.new
paths = [] #: Array[String]
ignore = [] #: Array[String]
allowed_extensions = [] #: Array[String]
no_stdlib = false #: bool

state = nil #: Symbol?

sorbet_config.each_line do |line|
line = line.strip
case line
when /^--allowed-extension$/
state = :extension
next
when /^--allowed-extension=/
config.allowed_extensions << parse_option(line)
allowed_extensions << parse_option(line)
next
when /^--ignore$/
state = :ignore
next
when /^--ignore=/
config.ignore << parse_option(line)
ignore << parse_option(line)
next
when /^--file$/
next
when /^--file=/
config.paths << parse_option(line)
paths << parse_option(line)
next
when /^--dir$/
next
when /^--dir=/
config.paths << parse_option(line)
paths << parse_option(line)
next
when /^--no-stdlib(=|$)/
config.no_stdlib = parse_bool_option(line)
no_stdlib = parse_bool_option(line)
next
when /^--.*=/
next
Expand All @@ -121,18 +126,24 @@ def parse_string(sorbet_config)
else
case state
when :ignore
config.ignore << line
ignore << line
when :extension
config.allowed_extensions << line
allowed_extensions << line
when :skip
# nothing
else
config.paths << line
paths << line
end
state = nil
end
end
config

Config.new(
paths:,
ignore:,
allowed_extensions:,
no_stdlib:,
)
end

private
Expand Down
17 changes: 9 additions & 8 deletions rbi/spoom.rbi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2598,27 +2598,28 @@ Spoom::Sorbet::BIN_PATH = T.let(T.unsafe(nil), String)
Spoom::Sorbet::CONFIG_PATH = T.let(T.unsafe(nil), String)

class Spoom::Sorbet::Config
sig { void }
def initialize; end
sig do
params(
paths: T::Array[::String],
ignore: T::Array[::String],
allowed_extensions: T::Array[::String],
no_stdlib: T::Boolean
).void
end
def initialize(paths: T.unsafe(nil), ignore: T.unsafe(nil), allowed_extensions: T.unsafe(nil), no_stdlib: T.unsafe(nil)); end

def allowed_extensions; end
def allowed_extensions=(_arg0); end
def ignore; end
def ignore=(_arg0); end

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

def no_stdlib=(_arg0); end

sig { returns(::String) }
def options_string; end

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

def paths=(_arg0); end

private

sig { params(source: ::Spoom::Sorbet::Config).void }
Expand Down
3 changes: 1 addition & 2 deletions test/spoom/context/sorbet_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,7 @@ def test_context_srb_files_with_custom_config
context.write!("h.js", "")
context.write!("i", "")

config = Spoom::Sorbet::Config.new
config.paths = ["b", "d"]
config = Spoom::Sorbet::Config.new(paths: ["b", "d"])

assert_equal(["b/c.rb", "d/e/f.rbi"], context.srb_files(with_config: config))

Expand Down
Loading