From a6b1e04824ffa746d875d60c656d7e7d205f1ba4 Mon Sep 17 00:00:00 2001 From: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com> Date: Sat, 29 Aug 2026 18:42:49 +0200 Subject: [PATCH 1/2] Run collection cleaner from CLI --- lib/rbs/cli.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rbs/cli.rb b/lib/rbs/cli.rb index 79a9bd682..52e483e79 100644 --- a/lib/rbs/cli.rb +++ b/lib/rbs/cli.rb @@ -1158,7 +1158,7 @@ def run_collection(args, options) stderr.puts "#{lock_path} should exist to clean" return 1 end - Collection::Cleaner.new(lockfile_path: lock_path) + Collection::Cleaner.new(lockfile_path: lock_path).clean when 'help', 'hel', 'he', 'h' stdout.puts opts.help else From d1fdb1bb13dae3ea3e1e6b02635c20261df1ccb7 Mon Sep 17 00:00:00 2001 From: Masataka Pocke Kuwabara Date: Mon, 7 Sep 2026 19:02:49 +0900 Subject: [PATCH 2/2] Test that `rbs collection clean` removes unneeded directories `rbs collection clean` built a `Collection::Cleaner` and never called `#clean` on it, so the command walked away with exit status 0 while leaving every directory in place. The bug dates back to 38fd8b2d, the commit that introduced `rbs collection`, so the command had never once removed anything. `test/rbs/collection/cleaner_test.rb` exercises `Cleaner#clean` directly, which is why the missing call went unnoticed for so long. This test goes through `RBS::CLI#run` instead, so it covers the wiring between the subcommand and the cleaner. The lockfile lists `ast 2.4`. The collection directory also holds `ast 2.3` and `rainbow 3.0`, covering both reasons `Cleaner#needed?` rejects a directory: a gem locked at another version, and a gem absent from the lockfile. `ast/2.4/ast.rbs` is written so that the assertion on the version that survives shows its RBS files are left alone rather than only that a directory still exists. The collection is named with `--collection` rather than by changing the working directory. `Config.find_config_path` walks from the working directory up to the filesystem root, so a `Dir.chdir` based test stays inside its temporary directory only as long as the config file is written before `cli.run`. With `TMPDIR` pointed inside a real project, a reordering would let the search reach an ancestor's lockfile and delete that project's `.gem_rbs_collection`. `--collection` expands the path it is given and skips the search. The config file is written empty. `clean` reads `path` and `gems` from the lockfile and never opens the config, and `path` appears in both files, so giving the config a `path` of its own would suggest that the command consults it. Its one remaining job is to be what `--collection` names and what the lockfile path is derived from, which is why the lockfile is written through `Config.to_lockfile_path` instead of a hardcoded name that happens to match. The `source` block in the lockfile stays even though `Cleaner#needed?` does not read it. `Lockfile.from_lockfile` passes `gem["source"]` to `Sources.from_config_entry`, so a lockfile without it raises `NoMethodError` when read through `Lockfile` rather than `Config`. https://github.com/ruby/rbs/pull/3127 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HSq3kKMB8ULhZrnL5urD7y --- test/rbs/cli_test.rb | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/rbs/cli_test.rb b/test/rbs/cli_test.rb index e9f6672a0..85fe26d7f 100644 --- a/test/rbs/cli_test.rb +++ b/test/rbs/cli_test.rb @@ -1734,6 +1734,42 @@ def test_collection_install__set_pathname__manifest end end + def test_collection_clean + Dir.mktmpdir do |dir| + dir = Pathname(dir) + + config_path = dir + RBS::Collection::Config::PATH + config_path.write("") + + RBS::Collection::Config.to_lockfile_path(config_path).write(<<~YAML) + path: .gem_rbs_collection + gems: + - name: ast + version: "2.4" + source: + type: git + name: ruby/gem_rbs_collection + remote: https://github.com/ruby/gem_rbs_collection.git + revision: b4d3b346d9657543099a35a1fd20347e75b8c523 + repo_dir: gems + YAML + + collection_dir = dir + ".gem_rbs_collection" + (collection_dir + "ast/2.4").mkpath + (collection_dir + "ast/2.4/ast.rbs").write("class Ast end") + (collection_dir + "ast/2.3").mkpath + (collection_dir + "rainbow/3.0").mkpath + + with_cli do |cli| + assert_cli_success cli.run(["--collection", config_path.to_s, "collection", "clean"]) + end + + assert_predicate(collection_dir + "ast/2.4/ast.rbs", :file?) + refute_predicate(collection_dir + "ast/2.3", :exist?) + refute_predicate(collection_dir + "rainbow/3.0", :exist?) + end + end + def test_subtract Dir.mktmpdir do |dir| dir = Pathname(dir)