From 553be1825b1e249abeb2714aa1b3e00b16bb8c3b Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:31:06 +0900 Subject: [PATCH 01/24] Add EnvironmentLoader for ruby-rbs crate Mirrors RBS::EnvironmentLoader: resolves core, library (with manifest.yaml dependency expansion), and explicit-directory sources in Ruby's load order, then parses each .rbs file once into an Environment. https://github.com/ruby/rbs/issues/2954 --- rust/Cargo.lock | 521 ++++++++++++++++++- rust/ruby-rbs/Cargo.toml | 9 + rust/ruby-rbs/src/ast/convert.rs | 1 + rust/ruby-rbs/src/buffer.rs | 199 +++++++ rust/ruby-rbs/src/environment/mod.rs | 82 +++ rust/ruby-rbs/src/environment/source.rs | 33 ++ rust/ruby-rbs/src/file_finder.rs | 165 ++++++ rust/ruby-rbs/src/gem_version.rs | 317 +++++++++++ rust/ruby-rbs/src/interners.rs | 43 ++ rust/ruby-rbs/src/lib.rs | 7 + rust/ruby-rbs/src/loader/gem_sig_resolver.rs | 27 + rust/ruby-rbs/src/loader/manifest.rs | 420 +++++++++++++++ rust/ruby-rbs/src/loader/mod.rs | 397 ++++++++++++++ rust/ruby-rbs/src/repository.rs | 252 +++++++++ rust/ruby-rbs/tests/loader.rs | 305 +++++++++++ 15 files changed, 2775 insertions(+), 3 deletions(-) create mode 100644 rust/ruby-rbs/src/buffer.rs create mode 100644 rust/ruby-rbs/src/environment/mod.rs create mode 100644 rust/ruby-rbs/src/environment/source.rs create mode 100644 rust/ruby-rbs/src/file_finder.rs create mode 100644 rust/ruby-rbs/src/gem_version.rs create mode 100644 rust/ruby-rbs/src/interners.rs create mode 100644 rust/ruby-rbs/src/loader/gem_sig_resolver.rs create mode 100644 rust/ruby-rbs/src/loader/manifest.rs create mode 100644 rust/ruby-rbs/src/loader/mod.rs create mode 100644 rust/ruby-rbs/src/repository.rs create mode 100644 rust/ruby-rbs/tests/loader.rs diff --git a/rust/Cargo.lock b/rust/Cargo.lock index d2d0d79f54..a3d777c02b 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -11,6 +11,24 @@ dependencies = [ "memchr", ] +[[package]] +name = "anes" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b46cbb362ab8752921c97e041f5e366ee6297bd428a31275b9fcf1e380f7299" + +[[package]] +name = "anstyle" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "940b3a0ca603d1eade50a4846a2afffd5ef57a9feac2c0e2ec2e14f9ead76000" + +[[package]] +name = "autocfg" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" + [[package]] name = "bindgen" version = "0.72.0" @@ -20,7 +38,7 @@ dependencies = [ "bitflags", "cexpr", "clang-sys", - "itertools", + "itertools 0.13.0", "log", "prettyplease", "proc-macro2", @@ -37,6 +55,18 @@ version = "2.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1b8e56985ec62d17e9c1001dc89c88ecd7dc08e47eba5ec7c29c7b5eeecde967" +[[package]] +name = "bumpalo" +version = "3.20.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" + +[[package]] +name = "cast" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" + [[package]] name = "cc" version = "1.2.29" @@ -61,6 +91,33 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9555578bc9e57714c812a1f84e4fc5b4d21fcb063490c624de019f7464c91268" +[[package]] +name = "ciborium" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42e69ffd6f0917f5c029256a24d0161db17cea3997d185db0d35926308770f0e" +dependencies = [ + "ciborium-io", + "ciborium-ll", + "serde", +] + +[[package]] +name = "ciborium-io" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05afea1e0a06c9be33d539b876f1ce3692f4afea2cb41f740e7743225ed1c757" + +[[package]] +name = "ciborium-ll" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57663b653d948a338bfb3eeba9bb2fd5fcfaecb9e199e87e1eda4d9e8b240fd9" +dependencies = [ + "ciborium-io", + "half", +] + [[package]] name = "clang-sys" version = "1.8.1" @@ -72,6 +129,98 @@ dependencies = [ "libloading", ] +[[package]] +name = "clap" +version = "4.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d91e0c145792ef73a6ad36d27c75ac09f1832222a3c209689d90f534685ee5b7" +dependencies = [ + "clap_builder", +] + +[[package]] +name = "clap_builder" +version = "4.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f09628afdcc538b57f3c6341e9c8e9970f18e4a481690a64974d7023bd33548b" +dependencies = [ + "anstyle", + "clap_lex", +] + +[[package]] +name = "clap_lex" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" + +[[package]] +name = "criterion" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2b12d017a929603d80db1831cd3a24082f8137ce19c69e6447f54f5fc8d692f" +dependencies = [ + "anes", + "cast", + "ciborium", + "clap", + "criterion-plot", + "is-terminal", + "itertools 0.10.5", + "num-traits", + "once_cell", + "oorandom", + "plotters", + "rayon", + "regex", + "serde", + "serde_derive", + "serde_json", + "tinytemplate", + "walkdir", +] + +[[package]] +name = "criterion-plot" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6b50826342786a51a89e2da3a28f1c32b06e387201bc2d19791f622c673706b1" +dependencies = [ + "cast", + "itertools 0.10.5", +] + +[[package]] +name = "crossbeam-deque" +version = "0.8.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5181e0de7b61eb03a81e347d6dd8797bae9da5146707b51077e2d71a54ec0ceb" +dependencies = [ + "crossbeam-epoch", + "crossbeam-utils", +] + +[[package]] +name = "crossbeam-epoch" +version = "0.9.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d6914041f254d6e9176c01941b21115dcfb7089e55135a35411081bd106ef3f" +dependencies = [ + "crossbeam-utils", +] + +[[package]] +name = "crossbeam-utils" +version = "0.8.22" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61803da095bee82a81bb1a452ecc25d3b2f1416d1897eb86430c6159ef717c17" + +[[package]] +name = "crunchy" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "460fbee9c2c2f33933d720630a6a0bac33ba7053db5344fac858d4b8952d77d5" + [[package]] name = "either" version = "1.15.0" @@ -84,18 +233,86 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" +[[package]] +name = "errno" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" +dependencies = [ + "libc", + "windows-sys", +] + +[[package]] +name = "fastrand" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" + +[[package]] +name = "futures-core" +version = "0.3.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2cd50c473c80f6d7c3670a752354b8e569b1a7cbfdc0419ec88e5edad85e0dc7" + +[[package]] +name = "futures-task" +version = "0.3.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b231ed28831efb4a61a08580c4bc233ec56bc009f4cd8f52da2c3cb97df0c109" + +[[package]] +name = "futures-util" +version = "0.3.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a77a90a256fce34da66415271e30f94ee91c57b04b8a2c042d9cf3220179deaa" +dependencies = [ + "futures-core", + "futures-task", + "pin-project-lite", + "slab", +] + +[[package]] +name = "getrandom" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "300e883d756b2e4ec94e02791f39b04b522276138852cfc41d9fb7e904106099" +dependencies = [ + "cfg-if", + "libc", + "r-efi", +] + [[package]] name = "glob" version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8d1add55171497b4705a648c6b583acafb01d58050a51727785f0b2c8e0a2b2" +[[package]] +name = "half" +version = "2.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ea2d84b969582b4b1864a92dc5d27cd2b77b622a8d79306834f1be5ba20d84b" +dependencies = [ + "cfg-if", + "crunchy", + "zerocopy", +] + [[package]] name = "hashbrown" version = "0.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" +[[package]] +name = "hermit-abi" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" + [[package]] name = "indexmap" version = "2.14.0" @@ -106,6 +323,26 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "is-terminal" +version = "0.4.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46" +dependencies = [ + "hermit-abi", + "libc", + "windows-sys", +] + +[[package]] +name = "itertools" +version = "0.10.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" +dependencies = [ + "either", +] + [[package]] name = "itertools" version = "0.13.0" @@ -121,11 +358,22 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" +[[package]] +name = "js-sys" +version = "0.3.103" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53b44bfcdb3f8d5837a46dae1ca9660a837176eee74a28b229bc626816589102" +dependencies = [ + "cfg-if", + "futures-util", + "wasm-bindgen", +] + [[package]] name = "libc" -version = "0.2.174" +version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1171693293099992e19cddea4e8b849964e9846f4acee11b3948bcc337be8776" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "libloading" @@ -137,6 +385,12 @@ dependencies = [ "windows-targets", ] +[[package]] +name = "linux-raw-sys" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" + [[package]] name = "log" version = "0.4.27" @@ -165,6 +419,61 @@ dependencies = [ "minimal-lexical", ] +[[package]] +name = "num-traits" +version = "0.2.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" +dependencies = [ + "autocfg", +] + +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + +[[package]] +name = "oorandom" +version = "11.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6790f58c7ff633d8771f42965289203411a5e5c68388703c06e14f24770b41e" + +[[package]] +name = "pin-project-lite" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" + +[[package]] +name = "plotters" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5aeb6f403d7a4911efb1e33402027fc44f29b5bf6def3effcc22d7bb75f2b747" +dependencies = [ + "num-traits", + "plotters-backend", + "plotters-svg", + "wasm-bindgen", + "web-sys", +] + +[[package]] +name = "plotters-backend" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df42e13c12958a16b3f7f4386b9ab1f3e7933914ecea48da7139435263a4172a" + +[[package]] +name = "plotters-svg" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51bae2ac328883f7acdfea3d66a7c35751187f870bc81f94563733a154d7a670" +dependencies = [ + "plotters-backend", +] + [[package]] name = "prettyplease" version = "0.2.35" @@ -193,6 +502,32 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "r-efi" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" + +[[package]] +name = "rayon" +version = "1.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb39b166781f92d482534ef4b4b1b2568f42613b53e5b6c160e24cfbfa30926d" +dependencies = [ + "either", + "rayon-core", +] + +[[package]] +name = "rayon-core" +version = "1.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22e18b0f0062d30d4230b2e85ff77fdfe4326feb054b9783a3460d8435c8ab91" +dependencies = [ + "crossbeam-deque", + "crossbeam-utils", +] + [[package]] name = "regex" version = "1.11.1" @@ -226,9 +561,11 @@ checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" name = "ruby-rbs" version = "0.3.0" dependencies = [ + "criterion", "ruby-rbs-sys", "serde", "serde_yaml", + "tempfile", "xxhash-rust", ] @@ -246,12 +583,40 @@ version = "2.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "357703d41365b4b27c590e3ed91eabb1b663f07c4c084095e60cbed4362dff0d" +[[package]] +name = "rustix" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys", + "windows-sys", +] + +[[package]] +name = "rustversion" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf54715a573b99ac80df0bc206da022bcd442c974952c7b9720069370852e21f" + [[package]] name = "ryu" version = "1.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f" +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "serde" version = "1.0.219" @@ -272,6 +637,18 @@ dependencies = [ "syn", ] +[[package]] +name = "serde_json" +version = "1.0.143" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d401abef1d108fbd9cbaebc3e46611f4b1021f714a0597a71f41ee463f5f4a5a" +dependencies = [ + "itoa", + "memchr", + "ryu", + "serde", +] + [[package]] name = "serde_yaml" version = "0.9.34+deprecated" @@ -291,6 +668,12 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" +[[package]] +name = "slab" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" + [[package]] name = "syn" version = "2.0.104" @@ -302,6 +685,29 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tempfile" +version = "3.27.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" +dependencies = [ + "fastrand", + "getrandom", + "once_cell", + "rustix", + "windows-sys", +] + +[[package]] +name = "tinytemplate" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "be4d6b5f19ff7664e8c98d03e2139cb510db9b0a60b55f8e8709b689d939b6bc" +dependencies = [ + "serde", + "serde_json", +] + [[package]] name = "unicode-ident" version = "1.0.18" @@ -314,6 +720,95 @@ version = "0.2.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" +[[package]] +name = "walkdir" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29790946404f91d9c5d06f9874efddea1dc06c5efe94541a7d6863108e3a5e4b" +dependencies = [ + "same-file", + "winapi-util", +] + +[[package]] +name = "wasm-bindgen" +version = "0.2.126" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b067c0c11094aef6b7a801c1e34a26affafdf3d051dba08456b868789aaf9a4" +dependencies = [ + "cfg-if", + "once_cell", + "rustversion", + "wasm-bindgen-macro", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-macro" +version = "0.2.126" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "167ce5e579f6bcf889c4f7175a8a5a585de84e8ff93976ce393efa5f2837aab1" +dependencies = [ + "quote", + "wasm-bindgen-macro-support", +] + +[[package]] +name = "wasm-bindgen-macro-support" +version = "0.2.126" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3997c7839262f4ef12cf90b818d6340c18e80f263f1a94bf157d0ec4420380e" +dependencies = [ + "bumpalo", + "proc-macro2", + "quote", + "syn", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-shared" +version = "0.2.126" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc1b4cb0cc549fcf58d7dfc081778139b3d283a081644e833e84682ad71cea24" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "web-sys" +version = "0.3.103" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8622dcb61c0bcc9fffa6938bed81210af2da9a7e4a1a834b2e37a59b6dfb6141" +dependencies = [ + "js-sys", + "wasm-bindgen", +] + +[[package]] +name = "winapi-util" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" +dependencies = [ + "windows-sys", +] + +[[package]] +name = "windows-link" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" + +[[package]] +name = "windows-sys" +version = "0.61.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc" +dependencies = [ + "windows-link", +] + [[package]] name = "windows-targets" version = "0.53.2" @@ -383,3 +878,23 @@ name = "xxhash-rust" version = "0.8.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fdd20c5420375476fbd4394763288da7eb0cc0b8c11deed431a91562af7335d3" + +[[package]] +name = "zerocopy" +version = "0.8.55" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5a105cd7b140f6eeec8acff2ea38135d3cab283ada58540f629fe51e46696eb" +dependencies = [ + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.8.55" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fe976fb70c78cd64cccfe3a6fc142244e8a77b70959b30faf9d0ac37ee228eb" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] diff --git a/rust/ruby-rbs/Cargo.toml b/rust/ruby-rbs/Cargo.toml index 0ba4dd4114..e68b0131c7 100644 --- a/rust/ruby-rbs/Cargo.toml +++ b/rust/ruby-rbs/Cargo.toml @@ -10,6 +10,7 @@ readme = "../../README.md" include = [ "src/**/*", "vendor/**/*", + "benches/**/*", "build.rs", "Cargo.toml", ] @@ -21,3 +22,11 @@ xxhash-rust = { version = "0.8", features = ["xxh3"] } [build-dependencies] serde = { version = "1.0", features = ["derive"] } serde_yaml = "0.9" + +[dev-dependencies] +criterion = "0.5" +tempfile = "3" + +[[bench]] +name = "load" +harness = false diff --git a/rust/ruby-rbs/src/ast/convert.rs b/rust/ruby-rbs/src/ast/convert.rs index 628f0720b3..29820a6fa7 100644 --- a/rust/ruby-rbs/src/ast/convert.rs +++ b/rust/ruby-rbs/src/ast/convert.rs @@ -1251,5 +1251,6 @@ fn node_kind(node: &Node<'_>) -> &'static str { Node::UnionType(_) => "UnionType", Node::UntypedFunctionType(_) => "UntypedFunctionType", Node::VariableType(_) => "VariableType", + Node::ModuleSelfAnnotation(_) => "ModuleSelfAnnotation", } } diff --git a/rust/ruby-rbs/src/buffer.rs b/rust/ruby-rbs/src/buffer.rs new file mode 100644 index 0000000000..c046924575 --- /dev/null +++ b/rust/ruby-rbs/src/buffer.rs @@ -0,0 +1,199 @@ +use std::cell::OnceCell; +use std::ops::Range; +use std::path::{Path, PathBuf}; + +/// A signature file's content with byte position <-> line/column conversion, +/// mirroring `RBS::Buffer`. +/// +/// Positions and columns are byte offsets, unlike Ruby's character offsets; +/// they agree on ASCII-only content. Use the parser's `RBSLocationRange` when +/// character offsets are needed. +/// +/// The line table is built on first use, not in [`Buffer::new`]: a normal +/// load never converts a position, and eagerly scanning every file would +/// cost more than the parse it accompanies. +#[derive(Debug, Clone)] +pub struct Buffer { + name: PathBuf, + content: String, + line_ranges: OnceCell>>, +} + +impl Buffer { + pub fn new(name: PathBuf, content: String) -> Self { + Self { + name, + content, + line_ranges: OnceCell::new(), + } + } + + pub fn name(&self) -> &Path { + &self.name + } + + pub fn content(&self) -> &str { + &self.content + } + + fn line_ranges(&self) -> &[Range] { + self.line_ranges + .get_or_init(|| compute_line_ranges(&self.content)) + } + + pub fn line_count(&self) -> usize { + self.line_ranges().len() + } + + /// Returns the 1-origin `line` without its line terminator. + pub fn line(&self, line: usize) -> Option<&str> { + let range = self.line_ranges().get(line.checked_sub(1)?)?; + Some(&self.content[range.clone()]) + } + + /// Converts a byte position into `(line, column)`. + /// + /// A position past the end of the content maps to `(line_count + 1, 0)`, + /// same as the Ruby implementation. + pub fn pos_to_loc(&self, pos: usize) -> (usize, usize) { + let ranges = self.line_ranges(); + let index = ranges.partition_point(|range| range.end < pos); + match ranges.get(index) { + // saturating_sub: positions inside a line terminator do not occur + // for parser-produced token boundaries. + Some(range) => (index + 1, pos.saturating_sub(range.start)), + None => (ranges.len() + 1, 0), + } + } + + /// Converts `(line, column)` into a byte position. Line 0 maps into the + /// last line, mirroring the Ruby implementation's `Array#fetch(-1)` + /// negative indexing; lines past the end map to `last_position`. + pub fn loc_to_pos(&self, line: usize, column: usize) -> usize { + let ranges = self.line_ranges(); + let range = match line.checked_sub(1) { + None => ranges.last(), + Some(index) => ranges.get(index), + }; + match range { + Some(range) => range.start + column, + None => self.last_position(), + } + } + + pub fn last_position(&self) -> usize { + self.line_ranges().last().map_or(0, |range| range.end) + } +} + +fn compute_line_ranges(content: &str) -> Vec> { + let mut ranges = Vec::new(); + let mut offset = 0; + + for line in content.split_inclusive('\n') { + // Like Ruby's String#chomp: strip "\r\n" or "\n", and a bare + // trailing "\r" (which can only occur on the final line). + let without_terminator = match line.strip_suffix('\n') { + Some(l) => l.strip_suffix('\r').unwrap_or(l), + None => line.strip_suffix('\r').unwrap_or(line), + }; + ranges.push(offset..offset + without_terminator.len()); + offset += line.len(); + } + + // Empty content is one empty line, and content ending in a newline has an + // empty line after it; both are the same trailing empty range. + if content.is_empty() || content.ends_with('\n') { + ranges.push(offset..offset); + } + + ranges +} + +#[cfg(test)] +mod tests { + use super::*; + + fn buffer(content: &str) -> Buffer { + Buffer::new(PathBuf::from("a.rbs"), content.to_string()) + } + + #[test] + fn lines_of_content_with_trailing_newline() { + let buffer = buffer("123\nabc\n"); + assert_eq!(buffer.line_count(), 3); + assert_eq!(buffer.line(1), Some("123")); + assert_eq!(buffer.line(2), Some("abc")); + assert_eq!(buffer.line(3), Some("")); + assert_eq!(buffer.line(4), None); + assert_eq!(buffer.line(0), None); + } + + #[test] + fn lines_of_content_without_trailing_newline() { + let buffer = buffer("123\nabc"); + assert_eq!(buffer.line_count(), 2); + assert_eq!(buffer.line(2), Some("abc")); + assert_eq!(buffer.last_position(), 7); + } + + #[test] + fn empty_content_has_one_empty_line() { + let buffer = buffer(""); + assert_eq!(buffer.line_count(), 1); + assert_eq!(buffer.line(1), Some("")); + assert_eq!(buffer.last_position(), 0); + assert_eq!(buffer.pos_to_loc(0), (1, 0)); + } + + #[test] + fn crlf_is_excluded_from_line_ranges() { + let buffer = buffer("abc\r\ndef\r\n"); + assert_eq!(buffer.line(1), Some("abc")); + assert_eq!(buffer.line(2), Some("def")); + assert_eq!(buffer.pos_to_loc(5), (2, 0)); + } + + #[test] + fn pos_to_loc_matches_ruby_buffer_for_ascii() { + let buffer = buffer("123\nabc\n"); + assert_eq!(buffer.pos_to_loc(0), (1, 0)); + assert_eq!(buffer.pos_to_loc(3), (1, 3)); + assert_eq!(buffer.pos_to_loc(4), (2, 0)); + assert_eq!(buffer.pos_to_loc(7), (2, 3)); + assert_eq!(buffer.pos_to_loc(8), (3, 0)); + assert_eq!(buffer.pos_to_loc(9), (4, 0)); + } + + #[test] + fn loc_to_pos_matches_ruby_buffer_for_ascii() { + let buffer = buffer("123\nabc\n"); + assert_eq!(buffer.loc_to_pos(1, 0), 0); + assert_eq!(buffer.loc_to_pos(2, 3), 7); + assert_eq!(buffer.loc_to_pos(3, 0), 8); + assert_eq!(buffer.loc_to_pos(10, 5), 8); + assert_eq!(buffer.last_position(), 8); + } + + #[test] + fn trailing_carriage_return_without_newline_is_chomped() { + let buffer = buffer("abc\ndef\r"); + assert_eq!(buffer.line(2), Some("def")); + assert_eq!(buffer.last_position(), 7); + } + + #[test] + fn line_zero_maps_to_the_last_line() { + let buffer = buffer("123\nabc"); + assert_eq!(buffer.loc_to_pos(0, 2), 6); + } + + #[test] + fn the_line_table_is_built_only_on_demand() { + let buffer = buffer("123\nabc\n"); + assert!(buffer.line_ranges.get().is_none()); + + buffer.pos_to_loc(4); + assert!(buffer.line_ranges.get().is_some()); + } +} diff --git a/rust/ruby-rbs/src/environment/mod.rs b/rust/ruby-rbs/src/environment/mod.rs new file mode 100644 index 0000000000..2da5c52657 --- /dev/null +++ b/rust/ruby-rbs/src/environment/mod.rs @@ -0,0 +1,82 @@ +pub mod source; + +pub use source::{Source, SourceKind}; + +use crate::interners::Interners; +use crate::loader::{EnvironmentLoader, LoadError}; + +/// Owns the interners and the loaded sources. +/// +/// Owning the interners here gives a single `Environment` value the same +/// role as the Ruby implementation's global name pool: names interned while +/// loading stay resolvable and displayable for the environment's lifetime. +pub struct Environment { + interners: Interners, + sources: Vec, +} + +impl Environment { + pub fn new() -> Self { + Environment { + interners: Interners::new(), + sources: Vec::new(), + } + } + + pub fn interners(&self) -> &Interners { + &self.interners + } + + pub fn sources(&self) -> &[Source] { + &self.sources + } + + /// Crate-private: only a caller that interned the source's names into + /// *this* environment can safely add it. + pub(crate) fn interners_mut(&mut self) -> &mut Interners { + &mut self.interners + } + + pub(crate) fn add_source(&mut self, source: Source) { + self.sources.push(source); + } + + /// Loads every signature file the loader resolves, in load order + /// (equivalent to Ruby's `Environment.from_loader(loader)`). + /// + /// On `Err` the environment is dropped, so the partially loaded state + /// `load` leaves behind is not observable here. + pub fn from_loader(loader: &EnvironmentLoader) -> Result { + let mut env = Environment::new(); + loader.load(&mut env)?; + Ok(env) + } +} + +impl Default for Environment { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn environment_owns_interners() { + let mut env = Environment::new(); + + let interners = env.interners_mut(); + let symbol = interners.strings.intern("Foo"); + let root = interners.type_names.absolute_root(); + let name = interners.type_names.append(root, symbol); + + let interners = env.interners(); + assert_eq!( + interners.type_names.display(name, &interners.strings), + "::Foo" + ); + assert!(env.sources().is_empty()); + } +} diff --git a/rust/ruby-rbs/src/environment/source.rs b/rust/ruby-rbs/src/environment/source.rs new file mode 100644 index 0000000000..7ed2aa3763 --- /dev/null +++ b/rust/ruby-rbs/src/environment/source.rs @@ -0,0 +1,33 @@ +use std::path::PathBuf; + +use crate::ast::{Declaration, Directive}; +use crate::buffer::Buffer; + +/// Where a loaded signature file came from, corresponding to the `source` +/// values yielded by `RBS::EnvironmentLoader#each_dir`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SourceKind { + /// Core library signatures (`:core` in Ruby). + Core, + /// A library resolved through the repository or a `GemSigResolver`. + /// + /// Carries the requested version as well as the name: Ruby keys its + /// library set on the `(name, version)` pair, so `uri` and `uri` 1.0 are + /// distinct entries yielded separately by `each_dir`. + Library { + name: String, + version: Option, + }, + /// An explicitly added signature directory. + Dir { path: PathBuf }, +} + +/// A parsed signature file: its buffer, `use` directives, and declarations +/// (`RBS::Source::RBS` equivalent). +#[derive(Debug)] +pub struct Source { + pub buffer: Buffer, + pub directives: Vec, + pub declarations: Vec, + pub kind: SourceKind, +} diff --git a/rust/ruby-rbs/src/file_finder.rs b/rust/ruby-rbs/src/file_finder.rs new file mode 100644 index 0000000000..05ba46e42c --- /dev/null +++ b/rust/ruby-rbs/src/file_finder.rs @@ -0,0 +1,165 @@ +use std::io; +use std::path::{Path, PathBuf}; + +/// Enumerates `.rbs` entries under `path` in a deterministic (path-sorted) +/// order, mirroring `RBS::FileFinder.each_file` and its `Dir.glob` +/// semantics: entries whose name starts with `.` are neither listed nor +/// descended into, symlinked directories are not descended into, and any +/// entry named `*.rbs` is listed regardless of its file type. +/// +/// When `path` is a file it is returned as-is. When `skip_hidden` is true, +/// entries under a directory whose name starts with `_` are skipped; the +/// entry's own name is not checked, same as the Ruby implementation. +/// +/// One divergence: Ruby sorts the `/`-joined strings `Dir.glob` returns, while +/// the sort here compares the platform separator. The two agree except on +/// Windows for names containing a character below `/`. +pub fn each_file(path: &Path, skip_hidden: bool) -> io::Result> { + let mut files = Vec::new(); + + if path.is_file() { + files.push(path.to_path_buf()); + } else if path.is_dir() { + collect(path, skip_hidden, &mut files)?; + files.sort_by(|a, b| a.as_os_str().cmp(b.as_os_str())); + } + + Ok(files) +} + +fn collect(dir: &Path, skip_hidden: bool, files: &mut Vec) -> io::Result<()> { + for entry in std::fs::read_dir(dir)? { + let entry = entry?; + let path = entry.path(); + + if entry.file_name().to_string_lossy().starts_with('.') { + continue; + } + + if path.extension().is_some_and(|ext| ext == "rbs") { + files.push(path.clone()); + } + + // `file_type()` does not follow symlinks, so this naturally skips + // symlinked directories. + if entry.file_type()?.is_dir() { + let hidden = path + .file_name() + .is_some_and(|name| name.to_string_lossy().starts_with('_')); + if skip_hidden && hidden { + continue; + } + collect(&path, skip_hidden, files)?; + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + fn fixture(files: &[&str]) -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + for file in files { + let path = dir.path().join(file); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(&path, "").unwrap(); + } + dir + } + + /// Paths relative to `root`, always `/`-joined so the expectations below + /// read the same on Windows. + fn relative(paths: Vec, root: &Path) -> Vec { + paths + .iter() + .map(|path| { + path.strip_prefix(root) + .unwrap() + .components() + .map(|component| component.as_os_str().to_string_lossy()) + .collect::>() + .join("/") + }) + .collect() + } + + #[test] + fn returns_single_file_as_is() { + let dir = fixture(&["a.txt"]); + let file = dir.path().join("a.txt"); + assert_eq!(each_file(&file, true).unwrap(), vec![file]); + } + + #[test] + fn collects_rbs_files_sorted() { + let dir = fixture(&["b.rbs", "a.rbs", "nested/c.rbs", "ignored.txt"]); + let found = each_file(dir.path(), false).unwrap(); + assert_eq!( + relative(found, dir.path()), + vec!["a.rbs", "b.rbs", "nested/c.rbs"] + ); + } + + #[test] + fn skip_hidden_skips_underscore_directories_only() { + let dir = fixture(&[ + "a.rbs", + "_private/b.rbs", + "nested/_private/c.rbs", + "_top.rbs", + ]); + + let found = each_file(dir.path(), true).unwrap(); + assert_eq!(relative(found, dir.path()), vec!["_top.rbs", "a.rbs"]); + + let found = each_file(dir.path(), false).unwrap(); + assert_eq!( + relative(found, dir.path()), + vec![ + "_private/b.rbs", + "_top.rbs", + "a.rbs", + "nested/_private/c.rbs" + ] + ); + } + + #[test] + fn missing_path_yields_nothing() { + let dir = fixture(&[]); + let found = each_file(&dir.path().join("no_such"), true).unwrap(); + assert!(found.is_empty()); + } + + #[cfg(unix)] + #[test] + fn symlinked_directories_are_not_traversed() { + let dir = fixture(&["a.rbs", "real/b.rbs"]); + std::os::unix::fs::symlink(dir.path().join("real"), dir.path().join("linked")).unwrap(); + + let found = each_file(dir.path(), false).unwrap(); + assert_eq!(relative(found, dir.path()), vec!["a.rbs", "real/b.rbs"]); + } + + #[test] + fn dot_entries_are_neither_listed_nor_traversed() { + let dir = fixture(&[".hidden.rbs", ".git/objects/x.rbs", "normal/ok.rbs"]); + + let found = each_file(dir.path(), false).unwrap(); + assert_eq!(relative(found, dir.path()), vec!["normal/ok.rbs"]); + } + + #[test] + fn entries_named_rbs_match_regardless_of_file_type() { + let dir = fixture(&["dir.rbs/inner.rbs"]); + + let found = each_file(dir.path(), false).unwrap(); + assert_eq!( + relative(found, dir.path()), + vec!["dir.rbs", "dir.rbs/inner.rbs"] + ); + } +} diff --git a/rust/ruby-rbs/src/gem_version.rs b/rust/ruby-rbs/src/gem_version.rs new file mode 100644 index 0000000000..77d374e2c9 --- /dev/null +++ b/rust/ruby-rbs/src/gem_version.rs @@ -0,0 +1,317 @@ +use std::cmp::Ordering; +use std::fmt; + +/// A version number compatible with RubyGems' `Gem::Version`. +/// +/// Comparison follows `Gem::Version#<=>`: trailing zero segments are ignored +/// (`1.0 == 1.0.0`) and prerelease versions sort before their release +/// (`1.0.b1 < 1.0`). Numeric segments are kept as normalized digit strings +/// and compared by length then lexicographically, equivalent to RubyGems' +/// arbitrary-precision comparison. +#[derive(Debug, Clone)] +pub struct GemVersion { + segments: Vec, + prerelease: bool, + version: String, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum Segment { + /// Decimal digits with leading zeros stripped (`"0"` for zero), matching + /// Ruby's `"007".to_i`. Compared by length, then lexicographically. + Number(String), + Str(String), +} + +impl Segment { + fn is_zero(&self) -> bool { + matches!(self, Segment::Number(digits) if digits == "0") + } +} + +impl GemVersion { + /// Parses a version string. Returns `None` when the string is not a valid + /// `Gem::Version` (i.e. `Gem::Version.correct?` would return false). + pub fn parse(source: &str) -> Option { + let trimmed = source.trim(); + if !is_correct(trimmed) { + return None; + } + + // Same normalization as Gem::Version#initialize. + let version = if trimmed.is_empty() { + "0".to_string() + } else { + trimmed.replace('-', ".pre.") + }; + + let prerelease = version.bytes().any(|b| b.is_ascii_alphabetic()); + let segments = scan_segments(&version); + + Some(GemVersion { + segments, + prerelease, + version, + }) + } + + pub fn is_prerelease(&self) -> bool { + self.prerelease + } + + /// Returns the release version, dropping prerelease segments + /// (`1.2.0.a` -> `1.2.0`), same as `Gem::Version#release`. + pub fn release(&self) -> GemVersion { + if !self.prerelease { + return self.clone(); + } + + let end = self + .segments + .iter() + .position(|segment| matches!(segment, Segment::Str(_))) + .unwrap_or(self.segments.len()); + let segments: Vec = self.segments[..end].to_vec(); + let version = segments + .iter() + .map(|segment| match segment { + Segment::Number(digits) => digits.clone(), + Segment::Str(s) => s.clone(), + }) + .collect::>() + .join("."); + + GemVersion { + segments, + prerelease: false, + version, + } + } + + /// Canonical segments for comparison: the numeric prefix and the part + /// starting at the first string segment, each with trailing zeros removed + /// (`Gem::Version#canonical_segments`). + fn canonical_segments(&self) -> Vec<&Segment> { + let split = self + .segments + .iter() + .position(|segment| matches!(segment, Segment::Str(_))) + .unwrap_or(self.segments.len()); + + let mut result = Vec::new(); + for part in [&self.segments[..split], &self.segments[split..]] { + let end = part + .iter() + .rposition(|segment| !segment.is_zero()) + .map_or(0, |index| index + 1); + result.extend(&part[..end]); + } + result + } +} + +impl fmt::Display for GemVersion { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.version) + } +} + +impl PartialEq for GemVersion { + fn eq(&self, other: &Self) -> bool { + self.cmp(other) == Ordering::Equal + } +} + +impl Eq for GemVersion {} + +impl PartialOrd for GemVersion { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for GemVersion { + fn cmp(&self, other: &Self) -> Ordering { + let zero = Segment::Number("0".to_string()); + + let lhs = self.canonical_segments(); + let rhs = other.canonical_segments(); + + for i in 0..lhs.len().max(rhs.len()) { + let l = lhs.get(i).copied().unwrap_or(&zero); + let r = rhs.get(i).copied().unwrap_or(&zero); + + let ordering = match (l, r) { + // Digit strings are leading-zero free: longer means larger. + (Segment::Number(a), Segment::Number(b)) => { + (a.len(), a.as_str()).cmp(&(b.len(), b.as_str())) + } + (Segment::Str(a), Segment::Str(b)) => a.cmp(b), + (Segment::Str(_), Segment::Number(_)) => Ordering::Less, + (Segment::Number(_), Segment::Str(_)) => Ordering::Greater, + }; + if ordering != Ordering::Equal { + return ordering; + } + } + + Ordering::Equal + } +} + +/// `Gem::Version.correct?`: optional `digits ('.' alnum+)* ('-' pre)?`, +/// surrounded by optional whitespace. +fn is_correct(trimmed: &str) -> bool { + if trimmed.is_empty() { + return true; + } + + let (main, pre) = match trimmed.split_once('-') { + Some((main, pre)) => (main, Some(pre)), + None => (trimmed, None), + }; + + let mut parts = main.split('.'); + let first = parts.next().unwrap_or(""); + if first.is_empty() || !first.bytes().all(|b| b.is_ascii_digit()) { + return false; + } + for part in parts { + if part.is_empty() || !part.bytes().all(|b| b.is_ascii_alphanumeric()) { + return false; + } + } + + if let Some(pre) = pre { + for part in pre.split('.') { + if part.is_empty() || !part.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-') { + return false; + } + } + } + + true +} + +/// Splits into numeric and alphabetic runs, like `@version.scan(/[0-9]+|[a-z]+/i)`. +fn scan_segments(version: &str) -> Vec { + let bytes = version.as_bytes(); + let mut segments = Vec::new(); + let mut i = 0; + + while i < bytes.len() { + if bytes[i].is_ascii_digit() { + let start = i; + while i < bytes.len() && bytes[i].is_ascii_digit() { + i += 1; + } + let digits = version[start..i].trim_start_matches('0'); + let digits = if digits.is_empty() { "0" } else { digits }; + segments.push(Segment::Number(digits.to_string())); + } else if bytes[i].is_ascii_alphabetic() { + let start = i; + while i < bytes.len() && bytes[i].is_ascii_alphabetic() { + i += 1; + } + segments.push(Segment::Str(version[start..i].to_string())); + } else { + i += 1; + } + } + + segments +} + +#[cfg(test)] +mod tests { + use super::*; + + fn v(source: &str) -> GemVersion { + GemVersion::parse(source).unwrap() + } + + #[test] + fn parse_rejects_malformed_versions() { + let bad = [ + "junk", + "1.0\n2.0", + "1..2", + "1.2 3.4", + "1.2.3+build", + "2.3422222.222.222222222.22222.ads0as.dasd0.ddd2222.2.qd3e.", + ]; + for source in bad { + assert!( + GemVersion::parse(source).is_none(), + "{source:?} should be invalid" + ); + } + + let good = [ + "", + "0", + "1.0", + "1.0.0-beta", + "5.2.4", + "1.0.0.a.1", + " 1.0 ", + "1.8.2.a10", + ]; + for source in good { + assert!( + GemVersion::parse(source).is_some(), + "{source:?} should be valid" + ); + } + } + + #[test] + fn equality_ignores_trailing_zero_segments() { + assert_eq!(v("1.0"), v("1.0.0")); + assert_eq!(v("1.0"), v("1.0.0.0")); + assert_eq!(v(""), v("0")); + assert_ne!(v("1.0"), v("1.1")); + assert_ne!(v("1.0"), v("1.0.b1")); + } + + #[test] + fn ordering_matches_gem_version() { + assert!(v("1.0") < v("1.1")); + assert!(v("1.0.b1") < v("1.0")); + assert!(v("1.0.a.2") < v("1.0.b1")); + assert!(v("1.0.beta.1") < v("1.0.beta.2")); + assert!(v("1.0.beta.2") < v("1.0.rc.1")); + assert!(v("1.9.3") < v("1.10.0")); + assert!(v("0.9") < v("1.0.a.2")); + assert!(v("1.0.a.2") < v("1.0.0")); + assert_eq!(v("1.0").cmp(&v("1.0.0")), Ordering::Equal); + } + + #[test] + fn prerelease_detection() { + assert!(v("1.2.0.a").is_prerelease()); + assert!(v("1.0.0-beta").is_prerelease()); + assert!(v("2.9.b").is_prerelease()); + assert!(!v("1.2.0").is_prerelease()); + assert!(!v("0").is_prerelease()); + } + + #[test] + fn release_drops_prerelease_segments() { + assert_eq!(v("1.2.0.a").release(), v("1.2.0")); + assert_eq!(v("1.1.rc10").release(), v("1.1")); + assert_eq!(v("1.0.0-beta").release(), v("1.0.0")); + assert_eq!(v("1.9.3").release(), v("1.9.3")); + assert!(!v("1.2.0.a").release().is_prerelease()); + } + + #[test] + fn numeric_segments_compare_with_arbitrary_precision() { + // Beyond u64::MAX; RubyGems compares these as bignums. + assert!(v("18446744073709551616") > v("18446744073709551615")); + assert!(v("1.99999999999999999999999999") < v("1.100000000000000000000000000")); + // Leading zeros are normalized like Ruby's `"007".to_i`. + assert_eq!(v("1.007"), v("1.7.0")); + assert_eq!(v("1.007.a").release(), v("1.7")); + } +} diff --git a/rust/ruby-rbs/src/interners.rs b/rust/ruby-rbs/src/interners.rs new file mode 100644 index 0000000000..b9061c8490 --- /dev/null +++ b/rust/ruby-rbs/src/interners.rs @@ -0,0 +1,43 @@ +use crate::interner::StringInterner; +use crate::type_name::TypeNameInterner; + +/// The pair of interners that an owned AST's ids refer to. +/// +/// An [`crate::environment::Environment`] owns one of these, giving a single +/// value the same role as the Ruby implementation's global name pool. +#[derive(Default)] +pub struct Interners { + pub strings: StringInterner, + pub type_names: TypeNameInterner, +} + +impl Interners { + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Absorbs every entry of `other`, keeping the ones already present. + pub fn merge(&mut self, other: Interners) { + self.strings.merge(other.strings); + self.type_names.merge(other.type_names); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn merge_makes_worker_local_ids_resolvable() { + let mut local = Interners::new(); + let foo = local.strings.intern("Foo"); + let root = local.type_names.absolute_root(); + let name = local.type_names.append(root, foo); + + let mut global = Interners::new(); + global.merge(local); + + assert_eq!(global.type_names.display(name, &global.strings), "::Foo"); + } +} diff --git a/rust/ruby-rbs/src/lib.rs b/rust/ruby-rbs/src/lib.rs index 328898158b..985f507f6b 100644 --- a/rust/ruby-rbs/src/lib.rs +++ b/rust/ruby-rbs/src/lib.rs @@ -1,5 +1,12 @@ pub mod ast; +pub mod buffer; +pub mod environment; +pub mod file_finder; +pub mod gem_version; pub mod ids; pub mod interner; +pub mod interners; +pub mod loader; pub mod node; +pub mod repository; pub mod type_name; diff --git a/rust/ruby-rbs/src/loader/gem_sig_resolver.rs b/rust/ruby-rbs/src/loader/gem_sig_resolver.rs new file mode 100644 index 0000000000..926837d192 --- /dev/null +++ b/rust/ruby-rbs/src/loader/gem_sig_resolver.rs @@ -0,0 +1,27 @@ +use std::path::PathBuf; + +/// Resolves the `sig/` directory of an installed gem. +/// +/// The Ruby implementation asks `Gem::Specification`; Rust tools inject their +/// own strategy here (spawning Ruby, reading a cache, ...). The default +/// [`NoGemSigs`] never finds anything, so only the repository is consulted. +/// +/// Implementations must be `Send + Sync`. Loading is planned to run its parse +/// stage on worker threads, which requires the loader holding this resolver to +/// be `Sync`; a supertrait cannot be added later without breaking downstream +/// implementations, so the bound is here from the start. +pub trait GemSigResolver: Send + Sync { + /// Returns the path to the gem's `sig/` directory, or `None` when the gem + /// is not installed or does not ship signatures. + fn sig_path(&self, name: &str, version: Option<&str>) -> Option; +} + +/// Default resolver that never finds installed gems. +#[derive(Debug, Clone, Copy, Default)] +pub struct NoGemSigs; + +impl GemSigResolver for NoGemSigs { + fn sig_path(&self, _name: &str, _version: Option<&str>) -> Option { + None + } +} diff --git a/rust/ruby-rbs/src/loader/manifest.rs b/rust/ruby-rbs/src/loader/manifest.rs new file mode 100644 index 0000000000..451106aee4 --- /dev/null +++ b/rust/ruby-rbs/src/loader/manifest.rs @@ -0,0 +1,420 @@ +use std::path::Path; + +use super::LoadError; + +/// Reads dependency library names from `manifest.yaml` directly under `dir`, +/// as used by stdlib and gem signature directories. Returns an empty list +/// when the file does not exist. +/// +/// A deliberately small hand-written reader rather than a YAML parser +/// dependency: its acceptance criterion is Ruby's, since Ruby looks only at +/// `manifest['dependencies']` and never at the notation. So all of this is +/// accepted, in one file: +/// +/// ```yaml +/// --- +/// dependencies: +/// - name: bigdecimal +/// - name: singleton +/// - name: uri +/// kind: gem +/// - +/// name: csv +/// ``` +/// +/// as are a single-line flow sequence (`dependencies: [{name: uri}]`, `[]`), +/// blank lines, comments, quoted names, and unrelated top-level keys. +/// +/// Anything outside that is a [`LoadError::Manifest`] rather than silently +/// ignored: a gem's `sig/manifest.yaml` is written by a third party, and a +/// dropped dependency would otherwise surface much later as a confusing +/// missing-type error. +pub fn dependencies(dir: &Path) -> Result, LoadError> { + let path = dir.join("manifest.yaml"); + + let content = match std::fs::read_to_string(&path) { + Ok(content) => content, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(source) => return Err(LoadError::Io { path, source }), + }; + + parse(&content).map_err(|message| LoadError::Manifest { path, message }) +} + +fn parse(content: &str) -> Result, String> { + let mut names = Vec::new(); + let mut in_dependencies = false; + let mut entry: Option = None; + + for (index, raw) in content.lines().enumerate() { + let line_number = index + 1; + let line = strip_comment(raw).trim_end(); + let body = line.trim_start(); + if body.is_empty() { + continue; + } + + // Document and directive markers belong to no key. + if body == "---" || body == "..." || body.starts_with("--- ") || body.starts_with('%') { + finish_entry(entry.take(), &mut names)?; + in_dependencies = false; + continue; + } + + let indented = line.len() != body.len(); + + if in_dependencies { + // The dash may sit at any indent, including column 0 at the + // `dependencies:` key's own level — that is ordinary YAML, and + // reading it as the next top-level key would drop the dependency + // without a word. + if body == "-" || body.starts_with("- ") || body.starts_with("-\t") { + finish_entry(entry.take(), &mut names)?; + let mut started = Entry::new(line_number); + let item = body[1..].trim_start(); + if !item.is_empty() { + started.read_pair(item, line_number)?; + } + entry = Some(started); + continue; + } + + if indented { + let Some(entry) = entry.as_mut() else { + return Err(format!( + "line {line_number}: `dependencies:` must be a sequence of `name:` entries, found {body:?}" + )); + }; + entry.read_pair(body, line_number)?; + continue; + } + + finish_entry(entry.take(), &mut names)?; + in_dependencies = false; + } + + // Inside some other key's block; Ruby reads only the top-level + // `dependencies`. + if indented { + continue; + } + + let Some((key, rest)) = body.split_once(':') else { + return Err(format!( + "line {line_number}: expected `key:`, found {body:?}" + )); + }; + in_dependencies = unquote(key.trim()) == "dependencies"; + + // `dependencies: [...]` is a flow sequence on this line, so no block + // items follow. + if in_dependencies && !rest.trim().is_empty() { + names.extend(parse_flow_sequence(rest.trim(), line_number)?); + in_dependencies = false; + } + } + + finish_entry(entry.take(), &mut names)?; + + Ok(names) +} + +/// One in-progress `dependencies` entry. An entry spans its `-` line plus any +/// deeper continuation lines, so its name is only known once the next item, +/// the next top-level key, or the end of the file arrives. +struct Entry { + name: Option, + line: usize, +} + +impl Entry { + fn new(line: usize) -> Self { + Entry { name: None, line } + } + + /// Reads one `key: value` pair of the entry's mapping, keeping only + /// `name` — Ruby looks at nothing else. + fn read_pair(&mut self, body: &str, line: usize) -> Result<(), String> { + let Some((key, value)) = body.split_once(':') else { + return Err(format!( + "line {line}: expected `- name: `, found {body:?}" + )); + }; + if unquote(key.trim()) != "name" { + return Ok(()); + } + + let value = unquote(value.trim()); + if value.is_empty() { + return Err(format!("line {line}: empty dependency name")); + } + if self.name.is_none() { + self.name = Some(value.to_string()); + } + Ok(()) + } +} + +fn finish_entry(entry: Option, names: &mut Vec) -> Result<(), String> { + let Some(entry) = entry else { + return Ok(()); + }; + match entry.name { + Some(name) => { + names.push(name); + Ok(()) + } + None => Err(format!("line {}: entry has no `name` key", entry.line)), + } +} + +/// Cuts a trailing comment, applying YAML's rule that `#` starts one only at +/// the start of a line or after whitespace, and never inside quotes. +fn strip_comment(line: &str) -> &str { + let bytes = line.as_bytes(); + let mut quote: Option = None; + + for (index, &byte) in bytes.iter().enumerate() { + match quote { + Some(open) if byte == open => quote = None, + Some(_) => {} + None => match byte { + b'\'' | b'"' => quote = Some(byte), + b'#' if index == 0 || bytes[index - 1].is_ascii_whitespace() => { + return &line[..index]; + } + _ => {} + }, + } + } + + line +} + +/// Reads `[{name: a}, {name: b}]` and `[]`. Only single-line flow sequences +/// are supported; anything else is an error rather than a silent skip. +fn parse_flow_sequence(rest: &str, line: usize) -> Result, String> { + let Some(inner) = rest.strip_prefix('[') else { + return Err(format!( + "line {line}: `dependencies:` must be a sequence of `name:` entries, found {rest:?}" + )); + }; + let Some(inner) = inner.strip_suffix(']') else { + return Err(format!( + "line {line}: unterminated flow sequence; multi-line flow style is not supported" + )); + }; + + let mut names = Vec::new(); + let mut rest = inner.trim(); + + while !rest.is_empty() { + let item = rest + .strip_prefix('{') + .ok_or_else(|| format!("line {line}: expected `{{name: }}`, found {rest:?}"))?; + let (body, tail) = item + .split_once('}') + .ok_or_else(|| format!("line {line}: unterminated flow mapping"))?; + + names.push(flow_mapping_name(body, line)?); + + rest = tail.trim_start(); + match rest.strip_prefix(',') { + Some(next) => rest = next.trim_start(), + None if rest.is_empty() => {} + None => { + return Err(format!( + "line {line}: expected `,` between entries, found {rest:?}" + )); + } + } + } + + Ok(names) +} + +/// Picks the `name` value out of a flow mapping body (`name: uri, kind: x`), +/// mirroring Ruby reading only `dep['name']`. +fn flow_mapping_name(body: &str, line: usize) -> Result { + for pair in body.split(',') { + let Some((key, value)) = pair.split_once(':') else { + return Err(format!( + "line {line}: expected `key: value`, found {pair:?}" + )); + }; + if unquote(key.trim()) == "name" { + let name = unquote(value.trim()); + if name.is_empty() { + return Err(format!("line {line}: empty dependency name")); + } + return Ok(name.to_string()); + } + } + + Err(format!("line {line}: entry has no `name` key: {body:?}")) +} + +/// Strips one pair of matching quotes. Escape sequences are not supported; +/// no signature manifest uses them. +fn unquote(value: &str) -> &str { + for quote in ['\'', '"'] { + if let Some(inner) = value + .strip_prefix(quote) + .and_then(|v| v.strip_suffix(quote)) + { + return inner; + } + } + value +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + fn manifest(content: &str) -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("manifest.yaml"), content).unwrap(); + dir + } + + #[test] + fn reads_dependency_names() { + let dir = manifest("dependencies:\n - name: bigdecimal\n - name: singleton\n"); + assert_eq!( + dependencies(dir.path()).unwrap(), + vec!["bigdecimal", "singleton"] + ); + } + + #[test] + fn missing_manifest_means_no_dependencies() { + let dir = tempfile::tempdir().unwrap(); + assert_eq!(dependencies(dir.path()).unwrap(), Vec::::new()); + } + + #[test] + fn comments_blank_lines_and_quotes_are_accepted() { + let dir = manifest("# a comment\n\ndependencies:\n - name: 'uri'\n - name: \"csv\"\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri", "csv"]); + } + + #[test] + fn unrelated_top_level_keys_are_ignored() { + let dir = manifest("type: stdlib\ndependencies:\n - name: uri\nother:\n - name: nope\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); + } + + #[test] + fn empty_flow_sequence_means_no_dependencies() { + let dir = manifest("dependencies: []\n"); + assert_eq!(dependencies(dir.path()).unwrap(), Vec::::new()); + } + + #[test] + fn non_sequence_dependencies_is_an_error() { + let dir = manifest("dependencies: 42\n"); + assert!(matches!( + dependencies(dir.path()), + Err(LoadError::Manifest { .. }) + )); + } + + #[test] + fn flow_style_dependencies_are_accepted() { + // Rejecting this would break a gem that loads fine under Ruby. + let dir = manifest("dependencies: [{name: uri}, {name: 'csv'}]\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri", "csv"]); + } + + #[test] + fn multi_line_flow_style_is_an_error() { + let dir = manifest("dependencies: [\n {name: uri}\n]\n"); + assert!(matches!( + dependencies(dir.path()), + Err(LoadError::Manifest { .. }) + )); + } + + #[test] + fn unexpected_entry_shape_is_an_error() { + let dir = manifest("dependencies:\n - uri\n"); + assert!(matches!( + dependencies(dir.path()), + Err(LoadError::Manifest { .. }) + )); + } + + // The tests below all cover input `YAML.safe_load` reads without + // complaint, verified against Ruby. Rejecting or silently dropping any of + // it would make a gem that loads under the Ruby implementation fail here. + + #[test] + fn sequence_at_the_keys_own_indent_is_accepted() { + // Reading `- name: uri` as the next top-level key silently returned + // no dependencies at all. + let dir = manifest("dependencies:\n- name: uri\n- name: csv\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri", "csv"]); + } + + #[test] + fn document_and_directive_markers_are_accepted() { + // `YAML.dump` always emits `---`. + let dir = manifest("%YAML 1.1\n---\ndependencies:\n - name: uri\n...\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); + } + + #[test] + fn entry_keys_other_than_name_are_ignored() { + let dir = + manifest("dependencies:\n - name: uri\n kind: gem\n - kind: gem\n name: csv\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri", "csv"]); + } + + #[test] + fn an_entrys_mapping_may_start_on_the_next_line() { + let dir = manifest("dependencies:\n -\n name: uri\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); + } + + #[test] + fn trailing_comments_are_stripped() { + let dir = manifest("dependencies:\n - name: uri # needed by Foo\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); + + let dir = manifest("dependencies: [{name: uri}] # needed by Foo\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); + } + + #[test] + fn a_hash_inside_quotes_is_not_a_comment() { + let dir = manifest("dependencies:\n - name: \"x # y\"\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["x # y"]); + } + + #[test] + fn an_entry_without_a_name_is_an_error() { + // Ruby yields a nil name here, which fails later and confusingly. + let dir = manifest("dependencies:\n - kind: gem\n"); + assert!(matches!( + dependencies(dir.path()), + Err(LoadError::Manifest { .. }) + )); + } + + #[test] + fn a_mapping_instead_of_a_sequence_is_an_error() { + let dir = manifest("dependencies:\n foo: bar\n"); + assert!(matches!( + dependencies(dir.path()), + Err(LoadError::Manifest { .. }) + )); + } + + #[test] + fn a_trailing_entry_is_not_lost() { + let dir = manifest("dependencies:\n - name: uri"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); + } +} diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs new file mode 100644 index 0000000000..2e33d56634 --- /dev/null +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -0,0 +1,397 @@ +pub mod gem_sig_resolver; +pub mod manifest; + +pub use gem_sig_resolver::{GemSigResolver, NoGemSigs}; + +use std::collections::HashSet; +use std::fmt; +use std::io; +use std::path::{Path, PathBuf}; + +use crate::ast::AstConverter; +use crate::buffer::Buffer; +use crate::environment::{Environment, Source, SourceKind}; +use crate::file_finder; +use crate::gem_version::GemVersion; +use crate::interners::Interners; +use crate::node; +use crate::repository::Repository; + +/// Errors raised while resolving and loading signature files, +/// mirroring `RBS::EnvironmentLoader::UnknownLibraryError` and friends. +#[derive(Debug)] +#[non_exhaustive] +pub enum LoadError { + /// No signature directory was found for the library + /// (`RBS::EnvironmentLoader::UnknownLibraryError`). + UnknownLibrary { + name: String, + version: Option, + }, + /// A requested library version is not a valid `Gem::Version`, where the + /// Ruby implementation raises `ArgumentError`. + InvalidVersion { + name: String, + version: String, + }, + Io { + path: PathBuf, + source: io::Error, + }, + Parse { + path: PathBuf, + message: String, + }, + Manifest { + path: PathBuf, + message: String, + }, +} + +impl fmt::Display for LoadError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + LoadError::UnknownLibrary { name, version } => write!( + f, + "Cannot find type definitions for library: {} ({})", + name, + version.as_deref().unwrap_or("[nil]") + ), + LoadError::InvalidVersion { name, version } => { + write!( + f, + "Malformed version number string {version} for library {name}" + ) + } + LoadError::Io { path, source } => { + write!(f, "IO error on {}: {}", path.display(), source) + } + LoadError::Parse { path, message } => { + write!(f, "Syntax error in {}: {}", path.display(), message) + } + LoadError::Manifest { path, message } => { + write!(f, "Invalid manifest {}: {}", path.display(), message) + } + } + } +} + +impl std::error::Error for LoadError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + LoadError::Io { source, .. } => Some(source), + _ => None, + } + } +} + +/// A library requested by name and optional version +/// (`RBS::EnvironmentLoader::Library` equivalent). +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct Library { + pub name: String, + pub version: Option, +} + +/// A file loaded by [`EnvironmentLoader::load`], in load order. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LoadedFile { + pub path: PathBuf, + pub kind: SourceKind, +} + +/// Resolves core, library, and explicit signature directories, parses every +/// `.rbs` file exactly once, and feeds the results into an [`Environment`], +/// mirroring `RBS::EnvironmentLoader`. +/// +/// `Send + Sync`, because [`GemSigResolver`] is: a future parallel parse stage +/// hands `&self` to worker threads. +pub struct EnvironmentLoader { + core_root: Option, + stdlib_root: Option, + repository: Repository, + libs: Vec, + dirs: Vec, + resolver: Box, +} + +impl EnvironmentLoader { + pub fn new() -> Self { + EnvironmentLoader { + core_root: None, + stdlib_root: None, + repository: Repository::new(), + libs: Vec::new(), + dirs: Vec::new(), + resolver: Box::new(NoGemSigs), + } + } + + /// Sets the directory containing core signatures. `None` (the default) + /// skips core. + pub fn core_root(mut self, path: Option) -> Self { + self.core_root = path; + self + } + + /// Sets the stdlib signature directory used for dependency expansion, + /// mirroring `Collection::Sources::Stdlib` (which Ruby pins to its + /// bundled `stdlib/`). Independent of [`EnvironmentLoader::repository`]: + /// add the same directory there to make the expanded libraries loadable. + pub fn stdlib_root(mut self, path: Option) -> Self { + self.stdlib_root = path; + self + } + + pub fn repository(mut self, repository: Repository) -> Self { + self.repository = repository; + self + } + + /// Requests a library. Its `manifest.yaml` dependencies are resolved + /// transitively at load time. + pub fn add_library(mut self, name: &str, version: Option<&str>) -> Self { + self.libs.push(Library { + name: name.to_string(), + version: version.map(str::to_string), + }); + self + } + + /// Adds an explicit signature directory. Unlike libraries, `_`-prefixed + /// subdirectories are not skipped. + pub fn add_dir(mut self, path: PathBuf) -> Self { + self.dirs.push(path); + self + } + + pub fn gem_sig_resolver(mut self, resolver: impl GemSigResolver + 'static) -> Self { + self.resolver = Box::new(resolver); + self + } + + /// Loads every signature file into `env` and returns the loaded files in + /// load order (`RBS::EnvironmentLoader#load` equivalent). + /// + /// On `Err` the sources read before the failure are already in `env`, same + /// as the Ruby implementation adding sources as it walks the directories. + pub fn load(&self, env: &mut Environment) -> Result, LoadError> { + let stdlib = self.stdlib_repository()?; + + let mut loaded = Vec::new(); + let mut seen_files: HashSet = HashSet::new(); + + for (kind, dir, skip_hidden) in self.each_dir(&stdlib)? { + let files = + file_finder::each_file(&dir, skip_hidden).map_err(|source| LoadError::Io { + path: dir.clone(), + source, + })?; + + for path in files { + if !seen_files.insert(path.clone()) { + continue; + } + // `parse_one` is a free function so this loop can later be + // spread over worker threads with worker-local `Interners`; + // `add_source` stays in file order. + let source = parse_one(&path, &kind, env.interners_mut())?; + env.add_source(source); + loaded.push(LoadedFile { + path, + kind: kind.clone(), + }); + } + } + + Ok(loaded) + } + + /// Resolves directories in the Ruby implementation's order: + /// core, then libraries (with dependencies), then explicit directories. + fn each_dir(&self, stdlib: &Repository) -> Result, LoadError> { + let mut result = Vec::new(); + + if let Some(core) = &self.core_root { + result.push((SourceKind::Core, core.clone(), true)); + } + + for library in self.resolved_libraries(stdlib)? { + let dir = self + .library_dir(&library) + .ok_or_else(|| LoadError::UnknownLibrary { + name: library.name.clone(), + version: library.version.clone(), + })?; + let kind = SourceKind::Library { + name: library.name, + version: library.version, + }; + result.push((kind, dir, true)); + } + + for dir in &self.dirs { + result.push((SourceKind::Dir { path: dir.clone() }, dir.clone(), false)); + } + + Ok(result) + } + + /// The repository backing dependency expansion, mirroring + /// `Collection::Sources::Stdlib`'s dedicated repository. Built from + /// `stdlib_root` on each load; independent of the loading repository. + fn stdlib_repository(&self) -> Result { + let mut repository = Repository::new(); + if let Some(root) = &self.stdlib_root { + repository.add(root).map_err(|source| LoadError::Io { + path: root.clone(), + source, + })?; + } + Ok(repository) + } + + /// Expands manifest dependencies depth-first in request order, matching + /// the Ruby implementation's insertion-ordered `Set` of libraries. + fn resolved_libraries(&self, stdlib: &Repository) -> Result, LoadError> { + let mut seen: HashSet = HashSet::new(); + let mut result = Vec::new(); + + for library in &self.libs { + self.add_library_recursive(library.clone(), stdlib, &mut seen, &mut result)?; + } + + // Loading core implies stringio (stdlib migration), unconditionally, + // same as Ruby; an unresolvable stringio is an UnknownLibrary error. + if self.core_root.is_some() && !seen.iter().any(|library| library.name == "stringio") { + let stringio = Library { + name: "stringio".to_string(), + version: None, + }; + self.add_library_recursive(stringio, stdlib, &mut seen, &mut result)?; + } + + Ok(result) + } + + fn add_library_recursive( + &self, + library: Library, + stdlib: &Repository, + seen: &mut HashSet, + result: &mut Vec, + ) -> Result<(), LoadError> { + if let Some(version) = &library.version { + // Ruby raises ArgumentError from Gem::Version/Gem::Requirement + // while resolving; fail before any lookup can misinterpret it. + if GemVersion::parse(version).is_none() { + return Err(LoadError::InvalidVersion { + name: library.name.clone(), + version: version.clone(), + }); + } + } + + if !seen.insert(library.clone()) { + return Ok(()); + } + result.push(library.clone()); + + // Mirrors Ruby's resolve_dependencies: the gem sig resolver first, + // then the stdlib repository. The loading repository is deliberately + // not consulted, so a library that only exists in a custom + // repository loads without dependency expansion, same as Ruby. + let dependency_dir = self + .resolver + .sig_path(&library.name, library.version.as_deref()) + .or_else(|| { + stdlib + .lookup(&library.name, library.version.as_deref()) + .map(Path::to_path_buf) + }); + if let Some(dir) = dependency_dir { + for name in manifest::dependencies(&dir)? { + self.add_library_recursive( + Library { + name, + version: None, + }, + stdlib, + seen, + result, + )?; + } + } + + Ok(()) + } + + /// The gem sig resolver wins over the repository, same as + /// `RBS::EnvironmentLoader#each_dir`. + fn library_dir(&self, library: &Library) -> Option { + if let Some(path) = self + .resolver + .sig_path(&library.name, library.version.as_deref()) + { + return Some(path); + } + self.repository + .lookup(&library.name, library.version.as_deref()) + .map(Path::to_path_buf) + } +} + +impl Default for EnvironmentLoader { + fn default() -> Self { + Self::new() + } +} + +/// Reads, parses, and converts a single signature file into an owned +/// [`Source`]. +/// +/// Deliberately a free function taking only the interners, so the load loop +/// can later be parallelised by handing each worker its own [`Interners`]. +/// The parser's `SignatureNode` holds raw pointers and is not `Send`, so it +/// must not escape this function — only the owned `Source` does. +/// +/// Crate-private: a caller outside the crate has no way to merge its +/// worker-local [`Interners`] into the environment, so the `Source` it +/// produced would carry ids that environment cannot resolve. +pub(crate) fn parse_one( + path: &Path, + kind: &SourceKind, + interners: &mut Interners, +) -> Result { + let content = std::fs::read_to_string(path).map_err(|source| LoadError::Io { + path: path.to_path_buf(), + source, + })?; + + let signature = node::parse(&content).map_err(|message| LoadError::Parse { + path: path.to_path_buf(), + message, + })?; + + let mut converter = AstConverter::new(&mut interners.strings, &mut interners.type_names); + let directives = signature + .directives() + .iter() + .map(|node| converter.convert_directive(&node)) + .collect(); + let declarations = signature + .declarations() + .iter() + .map(|node| converter.convert_declaration(&node)) + .collect(); + // SignatureNode borrows `content` and has a Drop impl; drop it + // explicitly before moving `content` into the Buffer. + drop(signature); + + Ok(Source { + buffer: Buffer::new(path.to_path_buf(), content), + directives, + declarations, + kind: kind.clone(), + }) +} diff --git a/rust/ruby-rbs/src/repository.rs b/rust/ruby-rbs/src/repository.rs new file mode 100644 index 0000000000..664f133747 --- /dev/null +++ b/rust/ruby-rbs/src/repository.rs @@ -0,0 +1,252 @@ +use std::collections::{BTreeMap, HashMap}; +use std::io; +use std::path::{Path, PathBuf}; + +use crate::gem_version::GemVersion; + +/// An index of versioned RBS directories laid out as `{dir}/{gem}/{version}/`, +/// mirroring `RBS::Repository`. +/// +/// Unlike the Ruby implementation, no default stdlib directory is registered; +/// callers add repository roots explicitly via [`Repository::add`]. +#[derive(Debug, Default)] +pub struct Repository { + dirs: Vec, + gems: HashMap, +} + +#[derive(Debug, Default)] +struct GemRbs { + /// Keyed by the raw version directory name: Ruby's versions hash uses + /// `Gem::Version` string equality, keeping `1.0` and `1.0.0` distinct. + versions: BTreeMap, +} + +#[derive(Debug)] +struct VersionPath { + version: GemVersion, + path: PathBuf, +} + +impl Repository { + pub fn new() -> Self { + Self::default() + } + + /// Directories registered via [`Repository::add`], in insertion order. + pub fn dirs(&self) -> &[PathBuf] { + &self.dirs + } + + /// Indexes `{dir}/{gem}/{version}/` entries. Non-version and prerelease + /// directory names are ignored. Later additions overwrite earlier ones + /// for the same gem and version. + pub fn add(&mut self, dir: &Path) -> io::Result<()> { + self.dirs.push(dir.to_path_buf()); + + for gem_entry in std::fs::read_dir(dir)? { + let gem_entry = gem_entry?; + if !gem_entry.path().is_dir() { + continue; + } + + let gem_name = gem_entry.file_name().to_string_lossy().into_owned(); + let gem = self.gems.entry(gem_name).or_default(); + + for version_entry in std::fs::read_dir(gem_entry.path())? { + let version_entry = version_entry?; + let name = version_entry.file_name().to_string_lossy().into_owned(); + let Some(version) = GemVersion::parse(&name) else { + continue; + }; + if version.is_prerelease() { + continue; + } + gem.versions.insert( + name, + VersionPath { + version, + path: version_entry.path(), + }, + ); + } + } + + Ok(()) + } + + /// Returns the signature directory for `gem`, choosing the newest version + /// `<= version`, or, when `version` is `None`, the version whose *name + /// string* sorts last — Ruby's `latest_version` sorts by the version + /// string, so `1.2` beats `1.11`. + /// + /// Returns `None` when `version` is not a valid `Gem::Version`; the + /// loader reports this as `InvalidVersion` instead of silently using the + /// latest version. + pub fn lookup(&self, gem: &str, version: Option<&str>) -> Option<&Path> { + let gem = self.gems.get(gem)?; + if gem.versions.is_empty() { + return None; + } + + match version { + Some(version) => { + let requested = GemVersion::parse(version)?; + find_best_version(&gem.versions, &requested.release()) + } + // Ruby's `latest_version` sorts by the version *string* + // (`sort_by(&:version)`): the string-wise greatest key wins. + None => gem + .versions + .last_key_value() + .map(|(_, vp)| vp.path.as_path()), + } + } +} + +/// Returns the semantically newest version that is `<= requested`, or the +/// semantically oldest one when every candidate is newer +/// (`RBS::Repository.find_best_version` compatible). Versions that compare +/// equal (`1.0` vs `1.0.0`) tie-break deterministically on the string-wise +/// last key; Ruby's unstable sort leaves that case unspecified. +fn find_best_version<'a>( + versions: &'a BTreeMap, + requested: &GemVersion, +) -> Option<&'a Path> { + versions + .values() + .filter(|vp| vp.version <= *requested) + .max_by(|a, b| a.version.cmp(&b.version)) + .or_else(|| versions.values().min_by(|a, b| a.version.cmp(&b.version))) + .map(|vp| vp.path.as_path()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + fn fixture(entries: &[&str]) -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + for entry in entries { + fs::create_dir_all(dir.path().join(entry)).unwrap(); + } + dir + } + + fn lookup_name(repository: &Repository, gem: &str, version: Option<&str>) -> Option { + repository + .lookup(gem, version) + .map(|path| path.file_name().unwrap().to_string_lossy().into_owned()) + } + + #[test] + fn lookup_returns_latest_version_without_request() { + let dir = fixture(&["uri/0", "uri/1.0", "csv/2.0", "csv/3.0.pre"]); + let mut repository = Repository::new(); + repository.add(dir.path()).unwrap(); + + assert_eq!( + lookup_name(&repository, "uri", None), + Some("1.0".to_string()) + ); + // prerelease directories are ignored + assert_eq!( + lookup_name(&repository, "csv", None), + Some("2.0".to_string()) + ); + assert_eq!(repository.lookup("no_such_gem", None), None); + } + + #[test] + fn lookup_finds_best_version() { + let dir = fixture(&["uri/0", "uri/1.0", "uri/2.0"]); + let mut repository = Repository::new(); + repository.add(dir.path()).unwrap(); + + assert_eq!( + lookup_name(&repository, "uri", Some("1.5")), + Some("1.0".to_string()) + ); + assert_eq!( + lookup_name(&repository, "uri", Some("1.0")), + Some("1.0".to_string()) + ); + assert_eq!( + lookup_name(&repository, "uri", Some("3.0")), + Some("2.0".to_string()) + ); + // older than every candidate: fall back to the oldest + assert_eq!( + lookup_name(&repository, "uri", Some("0.0.1")), + Some("0".to_string()) + ); + // prerelease requests match on their release version + assert_eq!( + lookup_name(&repository, "uri", Some("1.0.pre")), + Some("1.0".to_string()) + ); + // an invalid version string finds nothing rather than the latest + assert_eq!(repository.lookup("uri", Some("junk")), None); + } + + #[test] + fn non_version_directories_are_ignored() { + let dir = fixture(&["uri/not_a_version", "uri/1.0"]); + let mut repository = Repository::new(); + repository.add(dir.path()).unwrap(); + + assert_eq!( + lookup_name(&repository, "uri", None), + Some("1.0".to_string()) + ); + } + + #[test] + fn later_add_overwrites_same_version() { + let dir1 = fixture(&["uri/1.0"]); + let dir2 = fixture(&["uri/1.0"]); + let mut repository = Repository::new(); + repository.add(dir1.path()).unwrap(); + repository.add(dir2.path()).unwrap(); + + let path = repository.lookup("uri", None).unwrap(); + assert!(path.starts_with(dir2.path())); + assert_eq!(repository.dirs().len(), 2); + } + + #[test] + fn lookup_without_version_uses_string_order() { + let dir = fixture(&["uri/1.2", "uri/1.11"]); + let mut repository = Repository::new(); + repository.add(dir.path()).unwrap(); + + // Ruby's latest_version sorts by version string: "1.11" < "1.2". + assert_eq!( + lookup_name(&repository, "uri", None), + Some("1.2".to_string()) + ); + // A requested version searches semantically instead. + assert_eq!( + lookup_name(&repository, "uri", Some("99")), + Some("1.11".to_string()) + ); + } + + #[test] + fn equal_versions_with_different_spellings_stay_distinct() { + let dir = fixture(&["uri/1.0", "uri/1.0.0"]); + let mut repository = Repository::new(); + repository.add(dir.path()).unwrap(); + + // Deterministic tie-break: the string-wise last spelling wins. + assert_eq!( + lookup_name(&repository, "uri", Some("1.0")), + Some("1.0.0".to_string()) + ); + assert_eq!( + lookup_name(&repository, "uri", None), + Some("1.0.0".to_string()) + ); + } +} diff --git a/rust/ruby-rbs/tests/loader.rs b/rust/ruby-rbs/tests/loader.rs new file mode 100644 index 0000000000..834c4013df --- /dev/null +++ b/rust/ruby-rbs/tests/loader.rs @@ -0,0 +1,305 @@ +use std::fs; +use std::path::{Path, PathBuf}; + +use ruby_rbs::ast::{Declaration, Directive}; +use ruby_rbs::environment::{Environment, SourceKind}; +use ruby_rbs::loader::{EnvironmentLoader, GemSigResolver, LoadError}; +use ruby_rbs::repository::Repository; + +fn repo_root() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("../..") +} + +fn stdlib_repository() -> Repository { + let mut repository = Repository::new(); + repository.add(&repo_root().join("stdlib")).unwrap(); + repository +} + +fn lib(name: &str) -> SourceKind { + SourceKind::Library { + name: name.to_string(), + version: None, + } +} + +/// Writes `files` (relative path to content) under a fresh temporary +/// directory. +fn tree(files: &[(&str, &str)]) -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + for (path, content) in files { + let path = dir.path().join(path); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, content).unwrap(); + } + dir +} + +/// Stands in for `Gem::Specification.find_by_name(...).full_gem_path + "/sig"`. +/// +/// Matching on the requested version as well as the name keeps the tests +/// honest about what the loader passes through. +struct FakeGemSigs(Vec<(&'static str, Option<&'static str>, PathBuf)>); + +impl GemSigResolver for FakeGemSigs { + fn sig_path(&self, name: &str, version: Option<&str>) -> Option { + self.0 + .iter() + .find(|(gem, gem_version, _)| *gem == name && *gem_version == version) + .map(|(_, _, path)| path.clone()) + } +} + +#[test] +fn loads_core_stdlib_and_dependencies_in_ruby_order() { + let loader = EnvironmentLoader::new() + .core_root(Some(repo_root().join("core"))) + .stdlib_root(Some(repo_root().join("stdlib"))) + .repository(stdlib_repository()) + .add_library("bigdecimal-math", None); + + let mut env = Environment::new(); + let loaded = loader.load(&mut env).unwrap(); + + assert_eq!(loaded.first().unwrap().kind, SourceKind::Core); + // bigdecimal-math pulls bigdecimal via manifest.yaml, after itself + let first_math = loaded + .iter() + .position(|f| f.kind == lib("bigdecimal-math")) + .unwrap(); + let first_dep = loaded + .iter() + .position(|f| f.kind == lib("bigdecimal")) + .unwrap(); + assert!(first_math < first_dep); + // loading core implies stringio (stdlib migration compatibility) + assert!(loaded.iter().any(|f| f.kind == lib("stringio"))); + assert_eq!(env.sources().len(), loaded.len()); + assert!(!env.interners().strings.is_empty()); +} + +#[test] +fn from_loader_is_the_primary_entry_point() { + let loader = EnvironmentLoader::new() + .core_root(Some(repo_root().join("core"))) + .stdlib_root(Some(repo_root().join("stdlib"))) + .repository(stdlib_repository()); + + let env = Environment::from_loader(&loader).unwrap(); + + assert!(!env.sources().is_empty()); +} + +#[test] +fn files_are_loaded_once_first_wins() { + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("a.rbs"), "class Foo\nend\n").unwrap(); + + let loader = EnvironmentLoader::new() + .add_dir(dir.path().to_path_buf()) + .add_dir(dir.path().to_path_buf()); + + let mut env = Environment::new(); + let loaded = loader.load(&mut env).unwrap(); + + assert_eq!(loaded.len(), 1); + assert_eq!(env.sources().len(), 1); +} + +#[test] +fn explicit_dirs_do_not_skip_underscore_directories() { + let dir = tempfile::tempdir().unwrap(); + fs::create_dir(dir.path().join("_private")).unwrap(); + fs::write(dir.path().join("_private/a.rbs"), "class Foo\nend\n").unwrap(); + + let loader = EnvironmentLoader::new().add_dir(dir.path().to_path_buf()); + + let mut env = Environment::new(); + let loaded = loader.load(&mut env).unwrap(); + + assert_eq!(loaded.len(), 1); +} + +#[test] +fn unknown_library_is_an_error() { + let loader = EnvironmentLoader::new().add_library("no_such_gem", None); + + let mut env = Environment::new(); + let error = loader.load(&mut env).unwrap_err(); + + assert!(matches!( + error, + LoadError::UnknownLibrary { ref name, version: None } if name == "no_such_gem" + )); +} + +#[test] +fn parse_errors_carry_the_file_path() { + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("broken.rbs"), "class\n").unwrap(); + + let loader = EnvironmentLoader::new().add_dir(dir.path().to_path_buf()); + + let mut env = Environment::new(); + let error = loader.load(&mut env).unwrap_err(); + + assert!(matches!( + error, + LoadError::Parse { ref path, .. } if path.ends_with("broken.rbs") + )); +} + +#[test] +fn core_requires_resolvable_stringio() { + let loader = EnvironmentLoader::new().core_root(Some(repo_root().join("core"))); + + let mut env = Environment::new(); + let error = loader.load(&mut env).unwrap_err(); + + assert!(matches!( + error, + LoadError::UnknownLibrary { ref name, version: None } if name == "stringio" + )); +} + +#[test] +fn custom_repository_manifests_do_not_expand_dependencies() { + // Without stdlib_root the bigdecimal-math manifest is ignored even + // though the loading repository resolves the library itself. + let loader = EnvironmentLoader::new() + .repository(stdlib_repository()) + .add_library("bigdecimal-math", None); + + let mut env = Environment::new(); + let loaded = loader.load(&mut env).unwrap(); + + assert!(!loaded.is_empty()); + assert!(loaded.iter().all(|f| f.kind == lib("bigdecimal-math"))); +} + +#[test] +fn loaded_sources_carry_converted_declarations_and_directives() { + let dir = tree(&[( + "person.rbs", + "use Foo::Bar\n\nclass Person\n def name: () -> String\nend\n", + )]); + + let loader = EnvironmentLoader::new().add_dir(dir.path().to_path_buf()); + let env = Environment::from_loader(&loader).unwrap(); + + let source = &env.sources()[0]; + assert!(source.buffer.name().ends_with("person.rbs")); + assert!(matches!(source.directives.as_slice(), [Directive::Use(_)])); + + let [Declaration::Class(class)] = source.declarations.as_slice() else { + panic!( + "expected one class declaration, got {:?}", + source.declarations + ); + }; + // Names stay as written; Ruby absolutises them in `insert_rbs_decl`, + // which comes with declaration indexing later. + let interners = env.interners(); + assert_eq!( + interners.type_names.display(class.name, &interners.strings), + "Person" + ); + assert_eq!(class.members.len(), 1); +} + +#[test] +fn library_dirs_skip_underscore_directories() { + // Ruby passes skip_hidden: true for libraries, unlike explicit dirs. + let dir = tree(&[ + ("gem1/1.2.3/a.rbs", "class Person\nend\n"), + ("gem1/1.2.3/_private/b.rbs", "class Person::Internal\nend\n"), + ]); + + let mut repository = Repository::new(); + repository.add(dir.path()).unwrap(); + + let loader = EnvironmentLoader::new() + .repository(repository) + .add_library("gem1", Some("1.2.3")); + + let mut env = Environment::new(); + let loaded = loader.load(&mut env).unwrap(); + + assert_eq!(loaded.len(), 1); + assert!(loaded[0].path.ends_with("a.rbs")); + assert_eq!( + loaded[0].kind, + SourceKind::Library { + name: "gem1".to_string(), + version: Some("1.2.3".to_string()), + } + ); +} + +#[test] +fn gem_sig_resolver_wins_over_the_repository() { + let gem_sigs = tree(&[("sig/from_resolver.rbs", "class FromResolver\nend\n")]); + let repository_root = tree(&[("gem1/1.0.0/from_repository.rbs", "class FromRepo\nend\n")]); + + let mut repository = Repository::new(); + repository.add(repository_root.path()).unwrap(); + + let loader = EnvironmentLoader::new() + .repository(repository) + .gem_sig_resolver(FakeGemSigs(vec![( + "gem1", + None, + gem_sigs.path().join("sig"), + )])) + .add_library("gem1", None); + + let mut env = Environment::new(); + let loaded = loader.load(&mut env).unwrap(); + + assert_eq!(loaded.len(), 1); + assert!(loaded[0].path.ends_with("from_resolver.rbs")); +} + +#[test] +fn gem_sig_resolver_manifests_expand_dependencies() { + // resolve_dependencies consults the resolver first, so a manifest in an + // installed gem's sig/ directory pulls its dependencies in too. + let gem_sigs = tree(&[ + ("gem1/sig/manifest.yaml", "dependencies:\n - name: gem2\n"), + ("gem1/sig/a.rbs", "class Gem1\nend\n"), + ("gem2/sig/b.rbs", "class Gem2\nend\n"), + ]); + + let loader = EnvironmentLoader::new() + .gem_sig_resolver(FakeGemSigs(vec![ + ("gem1", Some("1.2.3"), gem_sigs.path().join("gem1/sig")), + ("gem2", None, gem_sigs.path().join("gem2/sig")), + ])) + .add_library("gem1", Some("1.2.3")); + + let mut env = Environment::new(); + let loaded = loader.load(&mut env).unwrap(); + + assert_eq!(loaded.len(), 2); + assert_eq!( + loaded[0].kind, + SourceKind::Library { + name: "gem1".to_string(), + version: Some("1.2.3".to_string()), + } + ); + assert_eq!(loaded[1].kind, lib("gem2")); +} + +#[test] +fn invalid_library_version_is_reported_distinctly() { + let loader = EnvironmentLoader::new().add_library("uri", Some("junk")); + + let mut env = Environment::new(); + let error = loader.load(&mut env).unwrap_err(); + + assert!(matches!( + error, + LoadError::InvalidVersion { ref name, ref version } if name == "uri" && version == "junk" + )); +} From 10bdb711593755135d9b03047f2ea1fede6cf475 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:08:30 +0900 Subject: [PATCH 02/24] Remove GemSigResolver trait from EnvironmentLoader The library client passes resolved gem paths directly; the resolver indirection was unnecessary. Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/src/environment/source.rs | 2 +- rust/ruby-rbs/src/loader/gem_sig_resolver.rs | 27 -------- rust/ruby-rbs/src/loader/mod.rs | 40 ++--------- rust/ruby-rbs/tests/loader.rs | 72 +------------------- 4 files changed, 9 insertions(+), 132 deletions(-) delete mode 100644 rust/ruby-rbs/src/loader/gem_sig_resolver.rs diff --git a/rust/ruby-rbs/src/environment/source.rs b/rust/ruby-rbs/src/environment/source.rs index 7ed2aa3763..d909b8117e 100644 --- a/rust/ruby-rbs/src/environment/source.rs +++ b/rust/ruby-rbs/src/environment/source.rs @@ -9,7 +9,7 @@ use crate::buffer::Buffer; pub enum SourceKind { /// Core library signatures (`:core` in Ruby). Core, - /// A library resolved through the repository or a `GemSigResolver`. + /// A library resolved through the repository. /// /// Carries the requested version as well as the name: Ruby keys its /// library set on the `(name, version)` pair, so `uri` and `uri` 1.0 are diff --git a/rust/ruby-rbs/src/loader/gem_sig_resolver.rs b/rust/ruby-rbs/src/loader/gem_sig_resolver.rs deleted file mode 100644 index 926837d192..0000000000 --- a/rust/ruby-rbs/src/loader/gem_sig_resolver.rs +++ /dev/null @@ -1,27 +0,0 @@ -use std::path::PathBuf; - -/// Resolves the `sig/` directory of an installed gem. -/// -/// The Ruby implementation asks `Gem::Specification`; Rust tools inject their -/// own strategy here (spawning Ruby, reading a cache, ...). The default -/// [`NoGemSigs`] never finds anything, so only the repository is consulted. -/// -/// Implementations must be `Send + Sync`. Loading is planned to run its parse -/// stage on worker threads, which requires the loader holding this resolver to -/// be `Sync`; a supertrait cannot be added later without breaking downstream -/// implementations, so the bound is here from the start. -pub trait GemSigResolver: Send + Sync { - /// Returns the path to the gem's `sig/` directory, or `None` when the gem - /// is not installed or does not ship signatures. - fn sig_path(&self, name: &str, version: Option<&str>) -> Option; -} - -/// Default resolver that never finds installed gems. -#[derive(Debug, Clone, Copy, Default)] -pub struct NoGemSigs; - -impl GemSigResolver for NoGemSigs { - fn sig_path(&self, _name: &str, _version: Option<&str>) -> Option { - None - } -} diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index 2e33d56634..565828e355 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -1,8 +1,5 @@ -pub mod gem_sig_resolver; pub mod manifest; -pub use gem_sig_resolver::{GemSigResolver, NoGemSigs}; - use std::collections::HashSet; use std::fmt; use std::io; @@ -103,16 +100,12 @@ pub struct LoadedFile { /// Resolves core, library, and explicit signature directories, parses every /// `.rbs` file exactly once, and feeds the results into an [`Environment`], /// mirroring `RBS::EnvironmentLoader`. -/// -/// `Send + Sync`, because [`GemSigResolver`] is: a future parallel parse stage -/// hands `&self` to worker threads. pub struct EnvironmentLoader { core_root: Option, stdlib_root: Option, repository: Repository, libs: Vec, dirs: Vec, - resolver: Box, } impl EnvironmentLoader { @@ -123,7 +116,6 @@ impl EnvironmentLoader { repository: Repository::new(), libs: Vec::new(), dirs: Vec::new(), - resolver: Box::new(NoGemSigs), } } @@ -165,11 +157,6 @@ impl EnvironmentLoader { self } - pub fn gem_sig_resolver(mut self, resolver: impl GemSigResolver + 'static) -> Self { - self.resolver = Box::new(resolver); - self - } - /// Loads every signature file into `env` and returns the loaded files in /// load order (`RBS::EnvironmentLoader#load` equivalent). /// @@ -297,18 +284,13 @@ impl EnvironmentLoader { } result.push(library.clone()); - // Mirrors Ruby's resolve_dependencies: the gem sig resolver first, - // then the stdlib repository. The loading repository is deliberately - // not consulted, so a library that only exists in a custom - // repository loads without dependency expansion, same as Ruby. - let dependency_dir = self - .resolver - .sig_path(&library.name, library.version.as_deref()) - .or_else(|| { - stdlib - .lookup(&library.name, library.version.as_deref()) - .map(Path::to_path_buf) - }); + // Mirrors Ruby's resolve_dependencies: the stdlib repository is + // consulted, not the loading repository, so a library that only + // exists in a custom repository loads without dependency expansion, + // same as Ruby. + let dependency_dir = stdlib + .lookup(&library.name, library.version.as_deref()) + .map(Path::to_path_buf); if let Some(dir) = dependency_dir { for name in manifest::dependencies(&dir)? { self.add_library_recursive( @@ -326,15 +308,7 @@ impl EnvironmentLoader { Ok(()) } - /// The gem sig resolver wins over the repository, same as - /// `RBS::EnvironmentLoader#each_dir`. fn library_dir(&self, library: &Library) -> Option { - if let Some(path) = self - .resolver - .sig_path(&library.name, library.version.as_deref()) - { - return Some(path); - } self.repository .lookup(&library.name, library.version.as_deref()) .map(Path::to_path_buf) diff --git a/rust/ruby-rbs/tests/loader.rs b/rust/ruby-rbs/tests/loader.rs index 834c4013df..862310b429 100644 --- a/rust/ruby-rbs/tests/loader.rs +++ b/rust/ruby-rbs/tests/loader.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use ruby_rbs::ast::{Declaration, Directive}; use ruby_rbs::environment::{Environment, SourceKind}; -use ruby_rbs::loader::{EnvironmentLoader, GemSigResolver, LoadError}; +use ruby_rbs::loader::{EnvironmentLoader, LoadError}; use ruby_rbs::repository::Repository; fn repo_root() -> PathBuf { @@ -35,21 +35,6 @@ fn tree(files: &[(&str, &str)]) -> tempfile::TempDir { dir } -/// Stands in for `Gem::Specification.find_by_name(...).full_gem_path + "/sig"`. -/// -/// Matching on the requested version as well as the name keeps the tests -/// honest about what the loader passes through. -struct FakeGemSigs(Vec<(&'static str, Option<&'static str>, PathBuf)>); - -impl GemSigResolver for FakeGemSigs { - fn sig_path(&self, name: &str, version: Option<&str>) -> Option { - self.0 - .iter() - .find(|(gem, gem_version, _)| *gem == name && *gem_version == version) - .map(|(_, _, path)| path.clone()) - } -} - #[test] fn loads_core_stdlib_and_dependencies_in_ruby_order() { let loader = EnvironmentLoader::new() @@ -236,61 +221,6 @@ fn library_dirs_skip_underscore_directories() { ); } -#[test] -fn gem_sig_resolver_wins_over_the_repository() { - let gem_sigs = tree(&[("sig/from_resolver.rbs", "class FromResolver\nend\n")]); - let repository_root = tree(&[("gem1/1.0.0/from_repository.rbs", "class FromRepo\nend\n")]); - - let mut repository = Repository::new(); - repository.add(repository_root.path()).unwrap(); - - let loader = EnvironmentLoader::new() - .repository(repository) - .gem_sig_resolver(FakeGemSigs(vec![( - "gem1", - None, - gem_sigs.path().join("sig"), - )])) - .add_library("gem1", None); - - let mut env = Environment::new(); - let loaded = loader.load(&mut env).unwrap(); - - assert_eq!(loaded.len(), 1); - assert!(loaded[0].path.ends_with("from_resolver.rbs")); -} - -#[test] -fn gem_sig_resolver_manifests_expand_dependencies() { - // resolve_dependencies consults the resolver first, so a manifest in an - // installed gem's sig/ directory pulls its dependencies in too. - let gem_sigs = tree(&[ - ("gem1/sig/manifest.yaml", "dependencies:\n - name: gem2\n"), - ("gem1/sig/a.rbs", "class Gem1\nend\n"), - ("gem2/sig/b.rbs", "class Gem2\nend\n"), - ]); - - let loader = EnvironmentLoader::new() - .gem_sig_resolver(FakeGemSigs(vec![ - ("gem1", Some("1.2.3"), gem_sigs.path().join("gem1/sig")), - ("gem2", None, gem_sigs.path().join("gem2/sig")), - ])) - .add_library("gem1", Some("1.2.3")); - - let mut env = Environment::new(); - let loaded = loader.load(&mut env).unwrap(); - - assert_eq!(loaded.len(), 2); - assert_eq!( - loaded[0].kind, - SourceKind::Library { - name: "gem1".to_string(), - version: Some("1.2.3".to_string()), - } - ); - assert_eq!(loaded[1].kind, lib("gem2")); -} - #[test] fn invalid_library_version_is_reported_distinctly() { let loader = EnvironmentLoader::new().add_library("uri", Some("junk")); From 2420a3cd460c54efa1512a90f5b4952e5697ad0b Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:11:58 +0900 Subject: [PATCH 03/24] Make core_root and stdlib_root required constructor arguments Optional builder setters let core and stdlib be skipped by silent omission instead of explicit choice. Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/src/loader/mod.rs | 37 +++++++++++--------------------- rust/ruby-rbs/tests/loader.rs | 38 +++++++++++++++++---------------- 2 files changed, 32 insertions(+), 43 deletions(-) diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index 565828e355..6421ff8d5c 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -109,32 +109,25 @@ pub struct EnvironmentLoader { } impl EnvironmentLoader { - pub fn new() -> Self { + /// `core_root` is `None` to skip core; `stdlib_root` is the stdlib + /// signature directory used for dependency expansion, mirroring + /// `Collection::Sources::Stdlib` (which Ruby pins to its bundled + /// `stdlib/`) — `None` disables manifest-based dependency expansion. + /// Both are required arguments, not builder defaults, so callers decide + /// them explicitly instead of silently getting `None`. + /// + /// `stdlib_root` is independent of [`EnvironmentLoader::repository`]: + /// add the same directory there to make the expanded libraries loadable. + pub fn new(core_root: Option, stdlib_root: Option) -> Self { EnvironmentLoader { - core_root: None, - stdlib_root: None, + core_root, + stdlib_root, repository: Repository::new(), libs: Vec::new(), dirs: Vec::new(), } } - /// Sets the directory containing core signatures. `None` (the default) - /// skips core. - pub fn core_root(mut self, path: Option) -> Self { - self.core_root = path; - self - } - - /// Sets the stdlib signature directory used for dependency expansion, - /// mirroring `Collection::Sources::Stdlib` (which Ruby pins to its - /// bundled `stdlib/`). Independent of [`EnvironmentLoader::repository`]: - /// add the same directory there to make the expanded libraries loadable. - pub fn stdlib_root(mut self, path: Option) -> Self { - self.stdlib_root = path; - self - } - pub fn repository(mut self, repository: Repository) -> Self { self.repository = repository; self @@ -315,12 +308,6 @@ impl EnvironmentLoader { } } -impl Default for EnvironmentLoader { - fn default() -> Self { - Self::new() - } -} - /// Reads, parses, and converts a single signature file into an owned /// [`Source`]. /// diff --git a/rust/ruby-rbs/tests/loader.rs b/rust/ruby-rbs/tests/loader.rs index 862310b429..3eea3eca67 100644 --- a/rust/ruby-rbs/tests/loader.rs +++ b/rust/ruby-rbs/tests/loader.rs @@ -37,11 +37,12 @@ fn tree(files: &[(&str, &str)]) -> tempfile::TempDir { #[test] fn loads_core_stdlib_and_dependencies_in_ruby_order() { - let loader = EnvironmentLoader::new() - .core_root(Some(repo_root().join("core"))) - .stdlib_root(Some(repo_root().join("stdlib"))) - .repository(stdlib_repository()) - .add_library("bigdecimal-math", None); + let loader = EnvironmentLoader::new( + Some(repo_root().join("core")), + Some(repo_root().join("stdlib")), + ) + .repository(stdlib_repository()) + .add_library("bigdecimal-math", None); let mut env = Environment::new(); let loaded = loader.load(&mut env).unwrap(); @@ -65,10 +66,11 @@ fn loads_core_stdlib_and_dependencies_in_ruby_order() { #[test] fn from_loader_is_the_primary_entry_point() { - let loader = EnvironmentLoader::new() - .core_root(Some(repo_root().join("core"))) - .stdlib_root(Some(repo_root().join("stdlib"))) - .repository(stdlib_repository()); + let loader = EnvironmentLoader::new( + Some(repo_root().join("core")), + Some(repo_root().join("stdlib")), + ) + .repository(stdlib_repository()); let env = Environment::from_loader(&loader).unwrap(); @@ -80,7 +82,7 @@ fn files_are_loaded_once_first_wins() { let dir = tempfile::tempdir().unwrap(); fs::write(dir.path().join("a.rbs"), "class Foo\nend\n").unwrap(); - let loader = EnvironmentLoader::new() + let loader = EnvironmentLoader::new(None, None) .add_dir(dir.path().to_path_buf()) .add_dir(dir.path().to_path_buf()); @@ -97,7 +99,7 @@ fn explicit_dirs_do_not_skip_underscore_directories() { fs::create_dir(dir.path().join("_private")).unwrap(); fs::write(dir.path().join("_private/a.rbs"), "class Foo\nend\n").unwrap(); - let loader = EnvironmentLoader::new().add_dir(dir.path().to_path_buf()); + let loader = EnvironmentLoader::new(None, None).add_dir(dir.path().to_path_buf()); let mut env = Environment::new(); let loaded = loader.load(&mut env).unwrap(); @@ -107,7 +109,7 @@ fn explicit_dirs_do_not_skip_underscore_directories() { #[test] fn unknown_library_is_an_error() { - let loader = EnvironmentLoader::new().add_library("no_such_gem", None); + let loader = EnvironmentLoader::new(None, None).add_library("no_such_gem", None); let mut env = Environment::new(); let error = loader.load(&mut env).unwrap_err(); @@ -123,7 +125,7 @@ fn parse_errors_carry_the_file_path() { let dir = tempfile::tempdir().unwrap(); fs::write(dir.path().join("broken.rbs"), "class\n").unwrap(); - let loader = EnvironmentLoader::new().add_dir(dir.path().to_path_buf()); + let loader = EnvironmentLoader::new(None, None).add_dir(dir.path().to_path_buf()); let mut env = Environment::new(); let error = loader.load(&mut env).unwrap_err(); @@ -136,7 +138,7 @@ fn parse_errors_carry_the_file_path() { #[test] fn core_requires_resolvable_stringio() { - let loader = EnvironmentLoader::new().core_root(Some(repo_root().join("core"))); + let loader = EnvironmentLoader::new(Some(repo_root().join("core")), None); let mut env = Environment::new(); let error = loader.load(&mut env).unwrap_err(); @@ -151,7 +153,7 @@ fn core_requires_resolvable_stringio() { fn custom_repository_manifests_do_not_expand_dependencies() { // Without stdlib_root the bigdecimal-math manifest is ignored even // though the loading repository resolves the library itself. - let loader = EnvironmentLoader::new() + let loader = EnvironmentLoader::new(None, None) .repository(stdlib_repository()) .add_library("bigdecimal-math", None); @@ -169,7 +171,7 @@ fn loaded_sources_carry_converted_declarations_and_directives() { "use Foo::Bar\n\nclass Person\n def name: () -> String\nend\n", )]); - let loader = EnvironmentLoader::new().add_dir(dir.path().to_path_buf()); + let loader = EnvironmentLoader::new(None, None).add_dir(dir.path().to_path_buf()); let env = Environment::from_loader(&loader).unwrap(); let source = &env.sources()[0]; @@ -203,7 +205,7 @@ fn library_dirs_skip_underscore_directories() { let mut repository = Repository::new(); repository.add(dir.path()).unwrap(); - let loader = EnvironmentLoader::new() + let loader = EnvironmentLoader::new(None, None) .repository(repository) .add_library("gem1", Some("1.2.3")); @@ -223,7 +225,7 @@ fn library_dirs_skip_underscore_directories() { #[test] fn invalid_library_version_is_reported_distinctly() { - let loader = EnvironmentLoader::new().add_library("uri", Some("junk")); + let loader = EnvironmentLoader::new(None, None).add_library("uri", Some("junk")); let mut env = Environment::new(); let error = loader.load(&mut env).unwrap_err(); From eb5befc5ced0935000cbecc99c51491b652d8a26 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:58:20 +0900 Subject: [PATCH 04/24] Store resolved path instead of version in SourceKind::Library version alone can't recover where a library's signatures came from; path can. Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/src/environment/source.rs | 11 ++++------- rust/ruby-rbs/src/loader/mod.rs | 2 +- rust/ruby-rbs/tests/loader.rs | 7 +++++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/rust/ruby-rbs/src/environment/source.rs b/rust/ruby-rbs/src/environment/source.rs index d909b8117e..eb05893801 100644 --- a/rust/ruby-rbs/src/environment/source.rs +++ b/rust/ruby-rbs/src/environment/source.rs @@ -11,13 +11,10 @@ pub enum SourceKind { Core, /// A library resolved through the repository. /// - /// Carries the requested version as well as the name: Ruby keys its - /// library set on the `(name, version)` pair, so `uri` and `uri` 1.0 are - /// distinct entries yielded separately by `each_dir`. - Library { - name: String, - version: Option, - }, + /// `path` is the resolved version directory + /// ([`Repository::lookup`](crate::repository::Repository::lookup)), which + /// holds the library's RBS files directly. + Library { name: String, path: PathBuf }, /// An explicitly added signature directory. Dir { path: PathBuf }, } diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index 6421ff8d5c..51a9d6ae07 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -205,7 +205,7 @@ impl EnvironmentLoader { })?; let kind = SourceKind::Library { name: library.name, - version: library.version, + path: dir.clone(), }; result.push((kind, dir, true)); } diff --git a/rust/ruby-rbs/tests/loader.rs b/rust/ruby-rbs/tests/loader.rs index 3eea3eca67..b61f04371e 100644 --- a/rust/ruby-rbs/tests/loader.rs +++ b/rust/ruby-rbs/tests/loader.rs @@ -19,7 +19,10 @@ fn stdlib_repository() -> Repository { fn lib(name: &str) -> SourceKind { SourceKind::Library { name: name.to_string(), - version: None, + path: stdlib_repository() + .lookup(name, None) + .expect("test library must resolve in the stdlib repository") + .to_path_buf(), } } @@ -218,7 +221,7 @@ fn library_dirs_skip_underscore_directories() { loaded[0].kind, SourceKind::Library { name: "gem1".to_string(), - version: Some("1.2.3".to_string()), + path: dir.path().join("gem1/1.2.3"), } ); } From 01998cbfa4100b57ebcad57c801d8d4e452899db Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Tue, 11 Aug 2026 18:23:24 +0900 Subject: [PATCH 05/24] Drop the temporary bench configuration Co-Authored-By: Claude Sonnet 5 --- rust/Cargo.lock | 430 +-------------------------------------- rust/ruby-rbs/Cargo.toml | 6 - 2 files changed, 1 insertion(+), 435 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index a3d777c02b..7f49f12a81 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -11,24 +11,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "anes" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4b46cbb362ab8752921c97e041f5e366ee6297bd428a31275b9fcf1e380f7299" - -[[package]] -name = "anstyle" -version = "1.0.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "940b3a0ca603d1eade50a4846a2afffd5ef57a9feac2c0e2ec2e14f9ead76000" - -[[package]] -name = "autocfg" -version = "1.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" - [[package]] name = "bindgen" version = "0.72.0" @@ -38,7 +20,7 @@ dependencies = [ "bitflags", "cexpr", "clang-sys", - "itertools 0.13.0", + "itertools", "log", "prettyplease", "proc-macro2", @@ -55,18 +37,6 @@ version = "2.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1b8e56985ec62d17e9c1001dc89c88ecd7dc08e47eba5ec7c29c7b5eeecde967" -[[package]] -name = "bumpalo" -version = "3.20.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" - -[[package]] -name = "cast" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" - [[package]] name = "cc" version = "1.2.29" @@ -91,33 +61,6 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9555578bc9e57714c812a1f84e4fc5b4d21fcb063490c624de019f7464c91268" -[[package]] -name = "ciborium" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42e69ffd6f0917f5c029256a24d0161db17cea3997d185db0d35926308770f0e" -dependencies = [ - "ciborium-io", - "ciborium-ll", - "serde", -] - -[[package]] -name = "ciborium-io" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05afea1e0a06c9be33d539b876f1ce3692f4afea2cb41f740e7743225ed1c757" - -[[package]] -name = "ciborium-ll" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57663b653d948a338bfb3eeba9bb2fd5fcfaecb9e199e87e1eda4d9e8b240fd9" -dependencies = [ - "ciborium-io", - "half", -] - [[package]] name = "clang-sys" version = "1.8.1" @@ -129,98 +72,6 @@ dependencies = [ "libloading", ] -[[package]] -name = "clap" -version = "4.6.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d91e0c145792ef73a6ad36d27c75ac09f1832222a3c209689d90f534685ee5b7" -dependencies = [ - "clap_builder", -] - -[[package]] -name = "clap_builder" -version = "4.6.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f09628afdcc538b57f3c6341e9c8e9970f18e4a481690a64974d7023bd33548b" -dependencies = [ - "anstyle", - "clap_lex", -] - -[[package]] -name = "clap_lex" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" - -[[package]] -name = "criterion" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2b12d017a929603d80db1831cd3a24082f8137ce19c69e6447f54f5fc8d692f" -dependencies = [ - "anes", - "cast", - "ciborium", - "clap", - "criterion-plot", - "is-terminal", - "itertools 0.10.5", - "num-traits", - "once_cell", - "oorandom", - "plotters", - "rayon", - "regex", - "serde", - "serde_derive", - "serde_json", - "tinytemplate", - "walkdir", -] - -[[package]] -name = "criterion-plot" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b50826342786a51a89e2da3a28f1c32b06e387201bc2d19791f622c673706b1" -dependencies = [ - "cast", - "itertools 0.10.5", -] - -[[package]] -name = "crossbeam-deque" -version = "0.8.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5181e0de7b61eb03a81e347d6dd8797bae9da5146707b51077e2d71a54ec0ceb" -dependencies = [ - "crossbeam-epoch", - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-epoch" -version = "0.9.20" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d6914041f254d6e9176c01941b21115dcfb7089e55135a35411081bd106ef3f" -dependencies = [ - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-utils" -version = "0.8.22" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61803da095bee82a81bb1a452ecc25d3b2f1416d1897eb86430c6159ef717c17" - -[[package]] -name = "crunchy" -version = "0.2.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "460fbee9c2c2f33933d720630a6a0bac33ba7053db5344fac858d4b8952d77d5" - [[package]] name = "either" version = "1.15.0" @@ -249,30 +100,6 @@ version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" -[[package]] -name = "futures-core" -version = "0.3.33" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cd50c473c80f6d7c3670a752354b8e569b1a7cbfdc0419ec88e5edad85e0dc7" - -[[package]] -name = "futures-task" -version = "0.3.33" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b231ed28831efb4a61a08580c4bc233ec56bc009f4cd8f52da2c3cb97df0c109" - -[[package]] -name = "futures-util" -version = "0.3.33" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a77a90a256fce34da66415271e30f94ee91c57b04b8a2c042d9cf3220179deaa" -dependencies = [ - "futures-core", - "futures-task", - "pin-project-lite", - "slab", -] - [[package]] name = "getrandom" version = "0.4.3" @@ -290,29 +117,12 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8d1add55171497b4705a648c6b583acafb01d58050a51727785f0b2c8e0a2b2" -[[package]] -name = "half" -version = "2.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ea2d84b969582b4b1864a92dc5d27cd2b77b622a8d79306834f1be5ba20d84b" -dependencies = [ - "cfg-if", - "crunchy", - "zerocopy", -] - [[package]] name = "hashbrown" version = "0.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" -[[package]] -name = "hermit-abi" -version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" - [[package]] name = "indexmap" version = "2.14.0" @@ -323,26 +133,6 @@ dependencies = [ "hashbrown", ] -[[package]] -name = "is-terminal" -version = "0.4.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46" -dependencies = [ - "hermit-abi", - "libc", - "windows-sys", -] - -[[package]] -name = "itertools" -version = "0.10.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" -dependencies = [ - "either", -] - [[package]] name = "itertools" version = "0.13.0" @@ -358,17 +148,6 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" -[[package]] -name = "js-sys" -version = "0.3.103" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53b44bfcdb3f8d5837a46dae1ca9660a837176eee74a28b229bc626816589102" -dependencies = [ - "cfg-if", - "futures-util", - "wasm-bindgen", -] - [[package]] name = "libc" version = "0.2.189" @@ -419,61 +198,12 @@ dependencies = [ "minimal-lexical", ] -[[package]] -name = "num-traits" -version = "0.2.19" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" -dependencies = [ - "autocfg", -] - [[package]] name = "once_cell" version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" -[[package]] -name = "oorandom" -version = "11.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6790f58c7ff633d8771f42965289203411a5e5c68388703c06e14f24770b41e" - -[[package]] -name = "pin-project-lite" -version = "0.2.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" - -[[package]] -name = "plotters" -version = "0.3.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5aeb6f403d7a4911efb1e33402027fc44f29b5bf6def3effcc22d7bb75f2b747" -dependencies = [ - "num-traits", - "plotters-backend", - "plotters-svg", - "wasm-bindgen", - "web-sys", -] - -[[package]] -name = "plotters-backend" -version = "0.3.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "df42e13c12958a16b3f7f4386b9ab1f3e7933914ecea48da7139435263a4172a" - -[[package]] -name = "plotters-svg" -version = "0.3.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "51bae2ac328883f7acdfea3d66a7c35751187f870bc81f94563733a154d7a670" -dependencies = [ - "plotters-backend", -] - [[package]] name = "prettyplease" version = "0.2.35" @@ -508,26 +238,6 @@ version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" -[[package]] -name = "rayon" -version = "1.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb39b166781f92d482534ef4b4b1b2568f42613b53e5b6c160e24cfbfa30926d" -dependencies = [ - "either", - "rayon-core", -] - -[[package]] -name = "rayon-core" -version = "1.13.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22e18b0f0062d30d4230b2e85ff77fdfe4326feb054b9783a3460d8435c8ab91" -dependencies = [ - "crossbeam-deque", - "crossbeam-utils", -] - [[package]] name = "regex" version = "1.11.1" @@ -561,7 +271,6 @@ checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" name = "ruby-rbs" version = "0.3.0" dependencies = [ - "criterion", "ruby-rbs-sys", "serde", "serde_yaml", @@ -596,27 +305,12 @@ dependencies = [ "windows-sys", ] -[[package]] -name = "rustversion" -version = "1.0.23" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf54715a573b99ac80df0bc206da022bcd442c974952c7b9720069370852e21f" - [[package]] name = "ryu" version = "1.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f" -[[package]] -name = "same-file" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" -dependencies = [ - "winapi-util", -] - [[package]] name = "serde" version = "1.0.219" @@ -637,18 +331,6 @@ dependencies = [ "syn", ] -[[package]] -name = "serde_json" -version = "1.0.143" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d401abef1d108fbd9cbaebc3e46611f4b1021f714a0597a71f41ee463f5f4a5a" -dependencies = [ - "itoa", - "memchr", - "ryu", - "serde", -] - [[package]] name = "serde_yaml" version = "0.9.34+deprecated" @@ -668,12 +350,6 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" -[[package]] -name = "slab" -version = "0.4.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" - [[package]] name = "syn" version = "2.0.104" @@ -698,16 +374,6 @@ dependencies = [ "windows-sys", ] -[[package]] -name = "tinytemplate" -version = "1.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be4d6b5f19ff7664e8c98d03e2139cb510db9b0a60b55f8e8709b689d939b6bc" -dependencies = [ - "serde", - "serde_json", -] - [[package]] name = "unicode-ident" version = "1.0.18" @@ -720,80 +386,6 @@ version = "0.2.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" -[[package]] -name = "walkdir" -version = "2.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "29790946404f91d9c5d06f9874efddea1dc06c5efe94541a7d6863108e3a5e4b" -dependencies = [ - "same-file", - "winapi-util", -] - -[[package]] -name = "wasm-bindgen" -version = "0.2.126" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4b067c0c11094aef6b7a801c1e34a26affafdf3d051dba08456b868789aaf9a4" -dependencies = [ - "cfg-if", - "once_cell", - "rustversion", - "wasm-bindgen-macro", - "wasm-bindgen-shared", -] - -[[package]] -name = "wasm-bindgen-macro" -version = "0.2.126" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "167ce5e579f6bcf889c4f7175a8a5a585de84e8ff93976ce393efa5f2837aab1" -dependencies = [ - "quote", - "wasm-bindgen-macro-support", -] - -[[package]] -name = "wasm-bindgen-macro-support" -version = "0.2.126" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3997c7839262f4ef12cf90b818d6340c18e80f263f1a94bf157d0ec4420380e" -dependencies = [ - "bumpalo", - "proc-macro2", - "quote", - "syn", - "wasm-bindgen-shared", -] - -[[package]] -name = "wasm-bindgen-shared" -version = "0.2.126" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc1b4cb0cc549fcf58d7dfc081778139b3d283a081644e833e84682ad71cea24" -dependencies = [ - "unicode-ident", -] - -[[package]] -name = "web-sys" -version = "0.3.103" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8622dcb61c0bcc9fffa6938bed81210af2da9a7e4a1a834b2e37a59b6dfb6141" -dependencies = [ - "js-sys", - "wasm-bindgen", -] - -[[package]] -name = "winapi-util" -version = "0.1.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" -dependencies = [ - "windows-sys", -] - [[package]] name = "windows-link" version = "0.2.1" @@ -878,23 +470,3 @@ name = "xxhash-rust" version = "0.8.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fdd20c5420375476fbd4394763288da7eb0cc0b8c11deed431a91562af7335d3" - -[[package]] -name = "zerocopy" -version = "0.8.55" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5a105cd7b140f6eeec8acff2ea38135d3cab283ada58540f629fe51e46696eb" -dependencies = [ - "zerocopy-derive", -] - -[[package]] -name = "zerocopy-derive" -version = "0.8.55" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fe976fb70c78cd64cccfe3a6fc142244e8a77b70959b30faf9d0ac37ee228eb" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] diff --git a/rust/ruby-rbs/Cargo.toml b/rust/ruby-rbs/Cargo.toml index e68b0131c7..973162288b 100644 --- a/rust/ruby-rbs/Cargo.toml +++ b/rust/ruby-rbs/Cargo.toml @@ -10,7 +10,6 @@ readme = "../../README.md" include = [ "src/**/*", "vendor/**/*", - "benches/**/*", "build.rs", "Cargo.toml", ] @@ -24,9 +23,4 @@ serde = { version = "1.0", features = ["derive"] } serde_yaml = "0.9" [dev-dependencies] -criterion = "0.5" tempfile = "3" - -[[bench]] -name = "load" -harness = false From 70b438c976229748d4cd2aecad49ad3fa9d8aaf5 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:00:04 +0900 Subject: [PATCH 06/24] Replace hand-rolled manifest.yaml parser with serde_yaml Fixes rejection of YAML that Ruby's YAML.safe_load accepts. Co-Authored-By: Claude Fable 5 --- rust/Cargo.lock | 43 +++- rust/ruby-rbs/Cargo.toml | 2 + rust/ruby-rbs/src/loader/manifest.rs | 296 +++++---------------------- 3 files changed, 87 insertions(+), 254 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 7f49f12a81..7b1becdd29 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -28,7 +28,7 @@ dependencies = [ "regex", "rustc-hash", "shlex", - "syn", + "syn 2.0.104", ] [[package]] @@ -144,9 +144,9 @@ dependencies = [ [[package]] name = "itoa" -version = "1.0.15" +version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" +checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" [[package]] name = "libc" @@ -211,7 +211,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "061c1221631e079b26479d25bbf2275bfe5917ae8419cd7e34f13bfc2aa7539a" dependencies = [ "proc-macro2", - "syn", + "syn 2.0.104", ] [[package]] @@ -307,28 +307,38 @@ dependencies = [ [[package]] name = "ryu" -version = "1.0.20" +version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f" +checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" [[package]] name = "serde" -version = "1.0.219" +version = "1.0.229" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4148590afebada386688f18773da617792bf2ef03ffc1e4cbd2b1d45b023e0ba" +dependencies = [ + "serde_core", + "serde_derive", +] + +[[package]] +name = "serde_core" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f0e2c6ed6606019b4e29e69dbaba95b11854410e5347d525002456dbbb786b6" +checksum = "67dca2c9c51e58a4791a4b1ed58308b39c64224d349a935ab5039aa360942a48" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.219" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b0276cf7f2c73365f7157c8123c21cd9a50fbbd844757af28ca1f5925fc2a00" +checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 3.0.3", ] [[package]] @@ -361,6 +371,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "tempfile" version = "3.27.0" diff --git a/rust/ruby-rbs/Cargo.toml b/rust/ruby-rbs/Cargo.toml index 973162288b..c6d3ede5a0 100644 --- a/rust/ruby-rbs/Cargo.toml +++ b/rust/ruby-rbs/Cargo.toml @@ -17,6 +17,8 @@ include = [ [dependencies] ruby-rbs-sys = { version = "0.3", path = "../ruby-rbs-sys" } xxhash-rust = { version = "0.8", features = ["xxh3"] } +serde = { version = "1.0", features = ["derive"] } +serde_yaml = "0.9" [build-dependencies] serde = { version = "1.0", features = ["derive"] } diff --git a/rust/ruby-rbs/src/loader/manifest.rs b/rust/ruby-rbs/src/loader/manifest.rs index 451106aee4..44030e6160 100644 --- a/rust/ruby-rbs/src/loader/manifest.rs +++ b/rust/ruby-rbs/src/loader/manifest.rs @@ -1,29 +1,16 @@ use std::path::Path; +use serde::Deserialize; + use super::LoadError; /// Reads dependency library names from `manifest.yaml` directly under `dir`, /// as used by stdlib and gem signature directories. Returns an empty list /// when the file does not exist. /// -/// A deliberately small hand-written reader rather than a YAML parser -/// dependency: its acceptance criterion is Ruby's, since Ruby looks only at -/// `manifest['dependencies']` and never at the notation. So all of this is -/// accepted, in one file: -/// -/// ```yaml -/// --- -/// dependencies: -/// - name: bigdecimal -/// - name: singleton -/// - name: uri -/// kind: gem -/// - -/// name: csv -/// ``` -/// -/// as are a single-line flow sequence (`dependencies: [{name: uri}]`, `[]`), -/// blank lines, comments, quoted names, and unrelated top-level keys. +/// Reads exactly what Ruby reads — `manifest['dependencies'][*]['name']` — +/// and nothing else about the notation, so any YAML that `YAML.safe_load` +/// accepts is accepted here too. /// /// Anything outside that is a [`LoadError::Manifest`] rather than silently /// ignored: a gem's `sig/manifest.yaml` is written by a third party, and a @@ -41,231 +28,37 @@ pub fn dependencies(dir: &Path) -> Result, LoadError> { parse(&content).map_err(|message| LoadError::Manifest { path, message }) } -fn parse(content: &str) -> Result, String> { - let mut names = Vec::new(); - let mut in_dependencies = false; - let mut entry: Option = None; - - for (index, raw) in content.lines().enumerate() { - let line_number = index + 1; - let line = strip_comment(raw).trim_end(); - let body = line.trim_start(); - if body.is_empty() { - continue; - } - - // Document and directive markers belong to no key. - if body == "---" || body == "..." || body.starts_with("--- ") || body.starts_with('%') { - finish_entry(entry.take(), &mut names)?; - in_dependencies = false; - continue; - } - - let indented = line.len() != body.len(); - - if in_dependencies { - // The dash may sit at any indent, including column 0 at the - // `dependencies:` key's own level — that is ordinary YAML, and - // reading it as the next top-level key would drop the dependency - // without a word. - if body == "-" || body.starts_with("- ") || body.starts_with("-\t") { - finish_entry(entry.take(), &mut names)?; - let mut started = Entry::new(line_number); - let item = body[1..].trim_start(); - if !item.is_empty() { - started.read_pair(item, line_number)?; - } - entry = Some(started); - continue; - } - - if indented { - let Some(entry) = entry.as_mut() else { - return Err(format!( - "line {line_number}: `dependencies:` must be a sequence of `name:` entries, found {body:?}" - )); - }; - entry.read_pair(body, line_number)?; - continue; - } - - finish_entry(entry.take(), &mut names)?; - in_dependencies = false; - } - - // Inside some other key's block; Ruby reads only the top-level - // `dependencies`. - if indented { - continue; - } - - let Some((key, rest)) = body.split_once(':') else { - return Err(format!( - "line {line_number}: expected `key:`, found {body:?}" - )); - }; - in_dependencies = unquote(key.trim()) == "dependencies"; - - // `dependencies: [...]` is a flow sequence on this line, so no block - // items follow. - if in_dependencies && !rest.trim().is_empty() { - names.extend(parse_flow_sequence(rest.trim(), line_number)?); - in_dependencies = false; - } - } - - finish_entry(entry.take(), &mut names)?; - - Ok(names) +#[derive(Deserialize, Default)] +struct Manifest { + #[serde(default)] + dependencies: Vec, } -/// One in-progress `dependencies` entry. An entry spans its `-` line plus any -/// deeper continuation lines, so its name is only known once the next item, -/// the next top-level key, or the end of the file arrives. -struct Entry { +#[derive(Deserialize)] +struct Dependency { name: Option, - line: usize, -} - -impl Entry { - fn new(line: usize) -> Self { - Entry { name: None, line } - } - - /// Reads one `key: value` pair of the entry's mapping, keeping only - /// `name` — Ruby looks at nothing else. - fn read_pair(&mut self, body: &str, line: usize) -> Result<(), String> { - let Some((key, value)) = body.split_once(':') else { - return Err(format!( - "line {line}: expected `- name: `, found {body:?}" - )); - }; - if unquote(key.trim()) != "name" { - return Ok(()); - } - - let value = unquote(value.trim()); - if value.is_empty() { - return Err(format!("line {line}: empty dependency name")); - } - if self.name.is_none() { - self.name = Some(value.to_string()); - } - Ok(()) - } -} - -fn finish_entry(entry: Option, names: &mut Vec) -> Result<(), String> { - let Some(entry) = entry else { - return Ok(()); - }; - match entry.name { - Some(name) => { - names.push(name); - Ok(()) - } - None => Err(format!("line {}: entry has no `name` key", entry.line)), - } -} - -/// Cuts a trailing comment, applying YAML's rule that `#` starts one only at -/// the start of a line or after whitespace, and never inside quotes. -fn strip_comment(line: &str) -> &str { - let bytes = line.as_bytes(); - let mut quote: Option = None; - - for (index, &byte) in bytes.iter().enumerate() { - match quote { - Some(open) if byte == open => quote = None, - Some(_) => {} - None => match byte { - b'\'' | b'"' => quote = Some(byte), - b'#' if index == 0 || bytes[index - 1].is_ascii_whitespace() => { - return &line[..index]; - } - _ => {} - }, - } - } - - line -} - -/// Reads `[{name: a}, {name: b}]` and `[]`. Only single-line flow sequences -/// are supported; anything else is an error rather than a silent skip. -fn parse_flow_sequence(rest: &str, line: usize) -> Result, String> { - let Some(inner) = rest.strip_prefix('[') else { - return Err(format!( - "line {line}: `dependencies:` must be a sequence of `name:` entries, found {rest:?}" - )); - }; - let Some(inner) = inner.strip_suffix(']') else { - return Err(format!( - "line {line}: unterminated flow sequence; multi-line flow style is not supported" - )); - }; - - let mut names = Vec::new(); - let mut rest = inner.trim(); - - while !rest.is_empty() { - let item = rest - .strip_prefix('{') - .ok_or_else(|| format!("line {line}: expected `{{name: }}`, found {rest:?}"))?; - let (body, tail) = item - .split_once('}') - .ok_or_else(|| format!("line {line}: unterminated flow mapping"))?; - - names.push(flow_mapping_name(body, line)?); - - rest = tail.trim_start(); - match rest.strip_prefix(',') { - Some(next) => rest = next.trim_start(), - None if rest.is_empty() => {} - None => { - return Err(format!( - "line {line}: expected `,` between entries, found {rest:?}" - )); - } - } - } - - Ok(names) -} - -/// Picks the `name` value out of a flow mapping body (`name: uri, kind: x`), -/// mirroring Ruby reading only `dep['name']`. -fn flow_mapping_name(body: &str, line: usize) -> Result { - for pair in body.split(',') { - let Some((key, value)) = pair.split_once(':') else { - return Err(format!( - "line {line}: expected `key: value`, found {pair:?}" - )); - }; - if unquote(key.trim()) == "name" { - let name = unquote(value.trim()); - if name.is_empty() { - return Err(format!("line {line}: empty dependency name")); - } - return Ok(name.to_string()); - } - } - - Err(format!("line {line}: entry has no `name` key: {body:?}")) } -/// Strips one pair of matching quotes. Escape sequences are not supported; -/// no signature manifest uses them. -fn unquote(value: &str) -> &str { - for quote in ['\'', '"'] { - if let Some(inner) = value - .strip_prefix(quote) - .and_then(|v| v.strip_suffix(quote)) - { - return inner; - } - } - value +fn parse(content: &str) -> Result, String> { + // An empty or all-comment file has no document; Ruby's YAML.safe_load + // returns nil for it, which yields no dependencies. + if content.trim().is_empty() { + return Ok(Vec::new()); + } + + let manifest: Manifest = serde_yaml::from_str(content).map_err(|error| error.to_string())?; + + manifest + .dependencies + .into_iter() + .enumerate() + .map(|(index, dependency)| { + dependency + .name + .filter(|name| !name.is_empty()) + .ok_or_else(|| format!("dependencies[{index}] has no `name` key")) + }) + .collect() } #[cfg(test)] @@ -329,12 +122,11 @@ mod tests { } #[test] - fn multi_line_flow_style_is_an_error() { + fn multi_line_flow_style_is_accepted() { + // The hand-rolled parser this replaced rejected multi-line flow + // style; Ruby's YAML.safe_load accepts it. let dir = manifest("dependencies: [\n {name: uri}\n]\n"); - assert!(matches!( - dependencies(dir.path()), - Err(LoadError::Manifest { .. }) - )); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); } #[test] @@ -417,4 +209,22 @@ mod tests { let dir = manifest("dependencies:\n - name: uri"); assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); } + + #[test] + fn a_nested_block_sequence_inside_an_entry_is_accepted() { + let dir = manifest("dependencies:\n - name: uri\n platforms:\n - mri\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); + } + + #[test] + fn a_flow_mapping_as_a_block_sequence_item_is_accepted() { + let dir = manifest("dependencies:\n - {name: uri}\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); + } + + #[test] + fn a_column_zero_sequence_under_an_unrelated_key_is_accepted() { + let dir = manifest("authors:\n- John Doe\ndependencies:\n - name: uri\n"); + assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); + } } From e5c8bbe540fa379725145f92b0b05230736b44a6 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:14:42 +0900 Subject: [PATCH 07/24] Register stdlib_root into EnvironmentLoader's loading repository Without this, the natural default configuration (core_root + stdlib_root, no extra wiring) always failed with UnknownLibrary("stringio"), since the loading repository stayed empty unlike Ruby's auto-registered stdlib. Co-Authored-By: Claude Fable 5 --- rust/ruby-rbs/src/loader/mod.rs | 61 ++++++++++++++++++++++----------- rust/ruby-rbs/tests/loader.rs | 24 ++++++++----- 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index 51a9d6ae07..64150c57c9 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -103,33 +103,31 @@ pub struct LoadedFile { pub struct EnvironmentLoader { core_root: Option, stdlib_root: Option, - repository: Repository, + repository_dirs: Vec, libs: Vec, dirs: Vec, } impl EnvironmentLoader { /// `core_root` is `None` to skip core; `stdlib_root` is the stdlib - /// signature directory used for dependency expansion, mirroring - /// `Collection::Sources::Stdlib` (which Ruby pins to its bundled - /// `stdlib/`) — `None` disables manifest-based dependency expansion. + /// signature directory used for dependency expansion and for resolving + /// requested libraries — `None` disables both. /// Both are required arguments, not builder defaults, so callers decide /// them explicitly instead of silently getting `None`. - /// - /// `stdlib_root` is independent of [`EnvironmentLoader::repository`]: - /// add the same directory there to make the expanded libraries loadable. pub fn new(core_root: Option, stdlib_root: Option) -> Self { EnvironmentLoader { core_root, stdlib_root, - repository: Repository::new(), + repository_dirs: Vec::new(), libs: Vec::new(), dirs: Vec::new(), } } - pub fn repository(mut self, repository: Repository) -> Self { - self.repository = repository; + /// Adds a repository root for resolving libraries, searched after + /// `stdlib_root`; a later root wins for the same gem and version. + pub fn add_repository_dir(mut self, path: PathBuf) -> Self { + self.repository_dirs.push(path); self } @@ -157,11 +155,12 @@ impl EnvironmentLoader { /// as the Ruby implementation adding sources as it walks the directories. pub fn load(&self, env: &mut Environment) -> Result, LoadError> { let stdlib = self.stdlib_repository()?; + let repository = self.loading_repository()?; let mut loaded = Vec::new(); let mut seen_files: HashSet = HashSet::new(); - for (kind, dir, skip_hidden) in self.each_dir(&stdlib)? { + for (kind, dir, skip_hidden) in self.each_dir(&stdlib, &repository)? { let files = file_finder::each_file(&dir, skip_hidden).map_err(|source| LoadError::Io { path: dir.clone(), @@ -189,7 +188,11 @@ impl EnvironmentLoader { /// Resolves directories in the Ruby implementation's order: /// core, then libraries (with dependencies), then explicit directories. - fn each_dir(&self, stdlib: &Repository) -> Result, LoadError> { + fn each_dir( + &self, + stdlib: &Repository, + repository: &Repository, + ) -> Result, LoadError> { let mut result = Vec::new(); if let Some(core) = &self.core_root { @@ -197,9 +200,8 @@ impl EnvironmentLoader { } for library in self.resolved_libraries(stdlib)? { - let dir = self - .library_dir(&library) - .ok_or_else(|| LoadError::UnknownLibrary { + let dir = + library_dir(repository, &library).ok_or_else(|| LoadError::UnknownLibrary { name: library.name.clone(), version: library.version.clone(), })?; @@ -231,6 +233,25 @@ impl EnvironmentLoader { Ok(repository) } + /// `stdlib_root` is registered first, standing in for Ruby's + /// `Repository.new` auto-registering `DEFAULT_STDLIB_ROOT`. + fn loading_repository(&self) -> Result { + let mut repository = Repository::new(); + if let Some(root) = &self.stdlib_root { + repository.add(root).map_err(|source| LoadError::Io { + path: root.clone(), + source, + })?; + } + for dir in &self.repository_dirs { + repository.add(dir).map_err(|source| LoadError::Io { + path: dir.clone(), + source, + })?; + } + Ok(repository) + } + /// Expands manifest dependencies depth-first in request order, matching /// the Ruby implementation's insertion-ordered `Set` of libraries. fn resolved_libraries(&self, stdlib: &Repository) -> Result, LoadError> { @@ -300,12 +321,12 @@ impl EnvironmentLoader { Ok(()) } +} - fn library_dir(&self, library: &Library) -> Option { - self.repository - .lookup(&library.name, library.version.as_deref()) - .map(Path::to_path_buf) - } +fn library_dir(repository: &Repository, library: &Library) -> Option { + repository + .lookup(&library.name, library.version.as_deref()) + .map(Path::to_path_buf) } /// Reads, parses, and converts a single signature file into an owned diff --git a/rust/ruby-rbs/tests/loader.rs b/rust/ruby-rbs/tests/loader.rs index b61f04371e..3103d55f66 100644 --- a/rust/ruby-rbs/tests/loader.rs +++ b/rust/ruby-rbs/tests/loader.rs @@ -44,7 +44,6 @@ fn loads_core_stdlib_and_dependencies_in_ruby_order() { Some(repo_root().join("core")), Some(repo_root().join("stdlib")), ) - .repository(stdlib_repository()) .add_library("bigdecimal-math", None); let mut env = Environment::new(); @@ -72,8 +71,7 @@ fn from_loader_is_the_primary_entry_point() { let loader = EnvironmentLoader::new( Some(repo_root().join("core")), Some(repo_root().join("stdlib")), - ) - .repository(stdlib_repository()); + ); let env = Environment::from_loader(&loader).unwrap(); @@ -139,6 +137,19 @@ fn parse_errors_carry_the_file_path() { )); } +#[test] +fn default_configuration_loads_without_extra_repository_wiring() { + let loader = EnvironmentLoader::new( + Some(repo_root().join("core")), + Some(repo_root().join("stdlib")), + ); + + let mut env = Environment::new(); + let loaded = loader.load(&mut env).unwrap(); + + assert!(loaded.iter().any(|f| f.kind == lib("stringio"))); +} + #[test] fn core_requires_resolvable_stringio() { let loader = EnvironmentLoader::new(Some(repo_root().join("core")), None); @@ -157,7 +168,7 @@ fn custom_repository_manifests_do_not_expand_dependencies() { // Without stdlib_root the bigdecimal-math manifest is ignored even // though the loading repository resolves the library itself. let loader = EnvironmentLoader::new(None, None) - .repository(stdlib_repository()) + .add_repository_dir(repo_root().join("stdlib")) .add_library("bigdecimal-math", None); let mut env = Environment::new(); @@ -205,11 +216,8 @@ fn library_dirs_skip_underscore_directories() { ("gem1/1.2.3/_private/b.rbs", "class Person::Internal\nend\n"), ]); - let mut repository = Repository::new(); - repository.add(dir.path()).unwrap(); - let loader = EnvironmentLoader::new(None, None) - .repository(repository) + .add_repository_dir(dir.path().to_path_buf()) .add_library("gem1", Some("1.2.3")); let mut env = Environment::new(); From 4515bc9a2bcee706981831f56386eb0ffbcb19fd Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Wed, 12 Aug 2026 21:52:42 +0900 Subject: [PATCH 08/24] Remove redundant comments Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/src/buffer.rs | 15 --------------- rust/ruby-rbs/src/environment/mod.rs | 9 --------- rust/ruby-rbs/src/environment/source.rs | 16 ++++++++-------- rust/ruby-rbs/src/gem_version.rs | 15 ++------------- rust/ruby-rbs/src/interners.rs | 4 ---- rust/ruby-rbs/src/loader/manifest.rs | 16 ---------------- rust/ruby-rbs/src/loader/mod.rs | 23 ++--------------------- rust/ruby-rbs/src/repository.rs | 13 ------------- rust/ruby-rbs/tests/loader.rs | 10 +--------- 9 files changed, 13 insertions(+), 108 deletions(-) diff --git a/rust/ruby-rbs/src/buffer.rs b/rust/ruby-rbs/src/buffer.rs index c046924575..84168c8fa6 100644 --- a/rust/ruby-rbs/src/buffer.rs +++ b/rust/ruby-rbs/src/buffer.rs @@ -8,10 +8,6 @@ use std::path::{Path, PathBuf}; /// Positions and columns are byte offsets, unlike Ruby's character offsets; /// they agree on ASCII-only content. Use the parser's `RBSLocationRange` when /// character offsets are needed. -/// -/// The line table is built on first use, not in [`Buffer::new`]: a normal -/// load never converts a position, and eagerly scanning every file would -/// cost more than the parse it accompanies. #[derive(Debug, Clone)] pub struct Buffer { name: PathBuf, @@ -51,24 +47,17 @@ impl Buffer { Some(&self.content[range.clone()]) } - /// Converts a byte position into `(line, column)`. - /// /// A position past the end of the content maps to `(line_count + 1, 0)`, /// same as the Ruby implementation. pub fn pos_to_loc(&self, pos: usize) -> (usize, usize) { let ranges = self.line_ranges(); let index = ranges.partition_point(|range| range.end < pos); match ranges.get(index) { - // saturating_sub: positions inside a line terminator do not occur - // for parser-produced token boundaries. Some(range) => (index + 1, pos.saturating_sub(range.start)), None => (ranges.len() + 1, 0), } } - /// Converts `(line, column)` into a byte position. Line 0 maps into the - /// last line, mirroring the Ruby implementation's `Array#fetch(-1)` - /// negative indexing; lines past the end map to `last_position`. pub fn loc_to_pos(&self, line: usize, column: usize) -> usize { let ranges = self.line_ranges(); let range = match line.checked_sub(1) { @@ -91,8 +80,6 @@ fn compute_line_ranges(content: &str) -> Vec> { let mut offset = 0; for line in content.split_inclusive('\n') { - // Like Ruby's String#chomp: strip "\r\n" or "\n", and a bare - // trailing "\r" (which can only occur on the final line). let without_terminator = match line.strip_suffix('\n') { Some(l) => l.strip_suffix('\r').unwrap_or(l), None => line.strip_suffix('\r').unwrap_or(line), @@ -101,8 +88,6 @@ fn compute_line_ranges(content: &str) -> Vec> { offset += line.len(); } - // Empty content is one empty line, and content ending in a newline has an - // empty line after it; both are the same trailing empty range. if content.is_empty() || content.ends_with('\n') { ranges.push(offset..offset); } diff --git a/rust/ruby-rbs/src/environment/mod.rs b/rust/ruby-rbs/src/environment/mod.rs index 2da5c52657..aa73d7d9f4 100644 --- a/rust/ruby-rbs/src/environment/mod.rs +++ b/rust/ruby-rbs/src/environment/mod.rs @@ -5,8 +5,6 @@ pub use source::{Source, SourceKind}; use crate::interners::Interners; use crate::loader::{EnvironmentLoader, LoadError}; -/// Owns the interners and the loaded sources. -/// /// Owning the interners here gives a single `Environment` value the same /// role as the Ruby implementation's global name pool: names interned while /// loading stay resolvable and displayable for the environment's lifetime. @@ -31,8 +29,6 @@ impl Environment { &self.sources } - /// Crate-private: only a caller that interned the source's names into - /// *this* environment can safely add it. pub(crate) fn interners_mut(&mut self) -> &mut Interners { &mut self.interners } @@ -41,11 +37,6 @@ impl Environment { self.sources.push(source); } - /// Loads every signature file the loader resolves, in load order - /// (equivalent to Ruby's `Environment.from_loader(loader)`). - /// - /// On `Err` the environment is dropped, so the partially loaded state - /// `load` leaves behind is not observable here. pub fn from_loader(loader: &EnvironmentLoader) -> Result { let mut env = Environment::new(); loader.load(&mut env)?; diff --git a/rust/ruby-rbs/src/environment/source.rs b/rust/ruby-rbs/src/environment/source.rs index eb05893801..cac63c5057 100644 --- a/rust/ruby-rbs/src/environment/source.rs +++ b/rust/ruby-rbs/src/environment/source.rs @@ -7,20 +7,20 @@ use crate::buffer::Buffer; /// values yielded by `RBS::EnvironmentLoader#each_dir`. #[derive(Debug, Clone, PartialEq, Eq)] pub enum SourceKind { - /// Core library signatures (`:core` in Ruby). Core, - /// A library resolved through the repository. - /// /// `path` is the resolved version directory /// ([`Repository::lookup`](crate::repository::Repository::lookup)), which /// holds the library's RBS files directly. - Library { name: String, path: PathBuf }, - /// An explicitly added signature directory. - Dir { path: PathBuf }, + Library { + name: String, + path: PathBuf, + }, + Dir { + path: PathBuf, + }, } -/// A parsed signature file: its buffer, `use` directives, and declarations -/// (`RBS::Source::RBS` equivalent). +/// `RBS::Source::RBS` equivalent. #[derive(Debug)] pub struct Source { pub buffer: Buffer, diff --git a/rust/ruby-rbs/src/gem_version.rs b/rust/ruby-rbs/src/gem_version.rs index 77d374e2c9..8b398a27dd 100644 --- a/rust/ruby-rbs/src/gem_version.rs +++ b/rust/ruby-rbs/src/gem_version.rs @@ -30,15 +30,14 @@ impl Segment { } impl GemVersion { - /// Parses a version string. Returns `None` when the string is not a valid - /// `Gem::Version` (i.e. `Gem::Version.correct?` would return false). + /// Returns `None` when the string is not a valid `Gem::Version` + /// (`Gem::Version.correct?`). pub fn parse(source: &str) -> Option { let trimmed = source.trim(); if !is_correct(trimmed) { return None; } - // Same normalization as Gem::Version#initialize. let version = if trimmed.is_empty() { "0".to_string() } else { @@ -59,8 +58,6 @@ impl GemVersion { self.prerelease } - /// Returns the release version, dropping prerelease segments - /// (`1.2.0.a` -> `1.2.0`), same as `Gem::Version#release`. pub fn release(&self) -> GemVersion { if !self.prerelease { return self.clone(); @@ -88,9 +85,6 @@ impl GemVersion { } } - /// Canonical segments for comparison: the numeric prefix and the part - /// starting at the first string segment, each with trailing zeros removed - /// (`Gem::Version#canonical_segments`). fn canonical_segments(&self) -> Vec<&Segment> { let split = self .segments @@ -159,8 +153,6 @@ impl Ord for GemVersion { } } -/// `Gem::Version.correct?`: optional `digits ('.' alnum+)* ('-' pre)?`, -/// surrounded by optional whitespace. fn is_correct(trimmed: &str) -> bool { if trimmed.is_empty() { return true; @@ -193,7 +185,6 @@ fn is_correct(trimmed: &str) -> bool { true } -/// Splits into numeric and alphabetic runs, like `@version.scan(/[0-9]+|[a-z]+/i)`. fn scan_segments(version: &str) -> Vec { let bytes = version.as_bytes(); let mut segments = Vec::new(); @@ -307,10 +298,8 @@ mod tests { #[test] fn numeric_segments_compare_with_arbitrary_precision() { - // Beyond u64::MAX; RubyGems compares these as bignums. assert!(v("18446744073709551616") > v("18446744073709551615")); assert!(v("1.99999999999999999999999999") < v("1.100000000000000000000000000")); - // Leading zeros are normalized like Ruby's `"007".to_i`. assert_eq!(v("1.007"), v("1.7.0")); assert_eq!(v("1.007.a").release(), v("1.7")); } diff --git a/rust/ruby-rbs/src/interners.rs b/rust/ruby-rbs/src/interners.rs index b9061c8490..a290c5ea1f 100644 --- a/rust/ruby-rbs/src/interners.rs +++ b/rust/ruby-rbs/src/interners.rs @@ -2,9 +2,6 @@ use crate::interner::StringInterner; use crate::type_name::TypeNameInterner; /// The pair of interners that an owned AST's ids refer to. -/// -/// An [`crate::environment::Environment`] owns one of these, giving a single -/// value the same role as the Ruby implementation's global name pool. #[derive(Default)] pub struct Interners { pub strings: StringInterner, @@ -17,7 +14,6 @@ impl Interners { Self::default() } - /// Absorbs every entry of `other`, keeping the ones already present. pub fn merge(&mut self, other: Interners) { self.strings.merge(other.strings); self.type_names.merge(other.type_names); diff --git a/rust/ruby-rbs/src/loader/manifest.rs b/rust/ruby-rbs/src/loader/manifest.rs index 44030e6160..feaaf45743 100644 --- a/rust/ruby-rbs/src/loader/manifest.rs +++ b/rust/ruby-rbs/src/loader/manifest.rs @@ -4,10 +4,6 @@ use serde::Deserialize; use super::LoadError; -/// Reads dependency library names from `manifest.yaml` directly under `dir`, -/// as used by stdlib and gem signature directories. Returns an empty list -/// when the file does not exist. -/// /// Reads exactly what Ruby reads — `manifest['dependencies'][*]['name']` — /// and nothing else about the notation, so any YAML that `YAML.safe_load` /// accepts is accepted here too. @@ -40,8 +36,6 @@ struct Dependency { } fn parse(content: &str) -> Result, String> { - // An empty or all-comment file has no document; Ruby's YAML.safe_load - // returns nil for it, which yields no dependencies. if content.trim().is_empty() { return Ok(Vec::new()); } @@ -116,15 +110,12 @@ mod tests { #[test] fn flow_style_dependencies_are_accepted() { - // Rejecting this would break a gem that loads fine under Ruby. let dir = manifest("dependencies: [{name: uri}, {name: 'csv'}]\n"); assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri", "csv"]); } #[test] fn multi_line_flow_style_is_accepted() { - // The hand-rolled parser this replaced rejected multi-line flow - // style; Ruby's YAML.safe_load accepts it. let dir = manifest("dependencies: [\n {name: uri}\n]\n"); assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); } @@ -138,21 +129,14 @@ mod tests { )); } - // The tests below all cover input `YAML.safe_load` reads without - // complaint, verified against Ruby. Rejecting or silently dropping any of - // it would make a gem that loads under the Ruby implementation fail here. - #[test] fn sequence_at_the_keys_own_indent_is_accepted() { - // Reading `- name: uri` as the next top-level key silently returned - // no dependencies at all. let dir = manifest("dependencies:\n- name: uri\n- name: csv\n"); assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri", "csv"]); } #[test] fn document_and_directive_markers_are_accepted() { - // `YAML.dump` always emits `---`. let dir = manifest("%YAML 1.1\n---\ndependencies:\n - name: uri\n...\n"); assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); } diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index 64150c57c9..376439c4be 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -14,13 +14,9 @@ use crate::interners::Interners; use crate::node; use crate::repository::Repository; -/// Errors raised while resolving and loading signature files, -/// mirroring `RBS::EnvironmentLoader::UnknownLibraryError` and friends. #[derive(Debug)] #[non_exhaustive] pub enum LoadError { - /// No signature directory was found for the library - /// (`RBS::EnvironmentLoader::UnknownLibraryError`). UnknownLibrary { name: String, version: Option, @@ -82,15 +78,13 @@ impl std::error::Error for LoadError { } } -/// A library requested by name and optional version -/// (`RBS::EnvironmentLoader::Library` equivalent). +/// `RBS::EnvironmentLoader::Library` equivalent. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Library { pub name: String, pub version: Option, } -/// A file loaded by [`EnvironmentLoader::load`], in load order. #[derive(Debug, Clone, PartialEq, Eq)] pub struct LoadedFile { pub path: PathBuf, @@ -112,8 +106,6 @@ impl EnvironmentLoader { /// `core_root` is `None` to skip core; `stdlib_root` is the stdlib /// signature directory used for dependency expansion and for resolving /// requested libraries — `None` disables both. - /// Both are required arguments, not builder defaults, so callers decide - /// them explicitly instead of silently getting `None`. pub fn new(core_root: Option, stdlib_root: Option) -> Self { EnvironmentLoader { core_root, @@ -131,7 +123,7 @@ impl EnvironmentLoader { self } - /// Requests a library. Its `manifest.yaml` dependencies are resolved + /// A requested library's `manifest.yaml` dependencies are resolved /// transitively at load time. pub fn add_library(mut self, name: &str, version: Option<&str>) -> Self { self.libs.push(Library { @@ -148,9 +140,6 @@ impl EnvironmentLoader { self } - /// Loads every signature file into `env` and returns the loaded files in - /// load order (`RBS::EnvironmentLoader#load` equivalent). - /// /// On `Err` the sources read before the failure are already in `env`, same /// as the Ruby implementation adding sources as it walks the directories. pub fn load(&self, env: &mut Environment) -> Result, LoadError> { @@ -171,9 +160,6 @@ impl EnvironmentLoader { if !seen_files.insert(path.clone()) { continue; } - // `parse_one` is a free function so this loop can later be - // spread over worker threads with worker-local `Interners`; - // `add_source` stays in file order. let source = parse_one(&path, &kind, env.interners_mut())?; env.add_source(source); loaded.push(LoadedFile { @@ -186,8 +172,6 @@ impl EnvironmentLoader { Ok(loaded) } - /// Resolves directories in the Ruby implementation's order: - /// core, then libraries (with dependencies), then explicit directories. fn each_dir( &self, stdlib: &Repository, @@ -329,9 +313,6 @@ fn library_dir(repository: &Repository, library: &Library) -> Option { .map(Path::to_path_buf) } -/// Reads, parses, and converts a single signature file into an owned -/// [`Source`]. -/// /// Deliberately a free function taking only the interners, so the load loop /// can later be parallelised by handing each worker its own [`Interners`]. /// The parser's `SignatureNode` holds raw pointers and is not `Send`, so it diff --git a/rust/ruby-rbs/src/repository.rs b/rust/ruby-rbs/src/repository.rs index 664f133747..f824b85024 100644 --- a/rust/ruby-rbs/src/repository.rs +++ b/rust/ruby-rbs/src/repository.rs @@ -33,14 +33,10 @@ impl Repository { Self::default() } - /// Directories registered via [`Repository::add`], in insertion order. pub fn dirs(&self) -> &[PathBuf] { &self.dirs } - /// Indexes `{dir}/{gem}/{version}/` entries. Non-version and prerelease - /// directory names are ignored. Later additions overwrite earlier ones - /// for the same gem and version. pub fn add(&mut self, dir: &Path) -> io::Result<()> { self.dirs.push(dir.to_path_buf()); @@ -94,8 +90,6 @@ impl Repository { let requested = GemVersion::parse(version)?; find_best_version(&gem.versions, &requested.release()) } - // Ruby's `latest_version` sorts by the version *string* - // (`sort_by(&:version)`): the string-wise greatest key wins. None => gem .versions .last_key_value() @@ -150,7 +144,6 @@ mod tests { lookup_name(&repository, "uri", None), Some("1.0".to_string()) ); - // prerelease directories are ignored assert_eq!( lookup_name(&repository, "csv", None), Some("2.0".to_string()) @@ -176,17 +169,14 @@ mod tests { lookup_name(&repository, "uri", Some("3.0")), Some("2.0".to_string()) ); - // older than every candidate: fall back to the oldest assert_eq!( lookup_name(&repository, "uri", Some("0.0.1")), Some("0".to_string()) ); - // prerelease requests match on their release version assert_eq!( lookup_name(&repository, "uri", Some("1.0.pre")), Some("1.0".to_string()) ); - // an invalid version string finds nothing rather than the latest assert_eq!(repository.lookup("uri", Some("junk")), None); } @@ -221,12 +211,10 @@ mod tests { let mut repository = Repository::new(); repository.add(dir.path()).unwrap(); - // Ruby's latest_version sorts by version string: "1.11" < "1.2". assert_eq!( lookup_name(&repository, "uri", None), Some("1.2".to_string()) ); - // A requested version searches semantically instead. assert_eq!( lookup_name(&repository, "uri", Some("99")), Some("1.11".to_string()) @@ -239,7 +227,6 @@ mod tests { let mut repository = Repository::new(); repository.add(dir.path()).unwrap(); - // Deterministic tie-break: the string-wise last spelling wins. assert_eq!( lookup_name(&repository, "uri", Some("1.0")), Some("1.0.0".to_string()) diff --git a/rust/ruby-rbs/tests/loader.rs b/rust/ruby-rbs/tests/loader.rs index 3103d55f66..cd40ccfc1a 100644 --- a/rust/ruby-rbs/tests/loader.rs +++ b/rust/ruby-rbs/tests/loader.rs @@ -26,8 +26,6 @@ fn lib(name: &str) -> SourceKind { } } -/// Writes `files` (relative path to content) under a fresh temporary -/// directory. fn tree(files: &[(&str, &str)]) -> tempfile::TempDir { let dir = tempfile::tempdir().unwrap(); for (path, content) in files { @@ -50,7 +48,6 @@ fn loads_core_stdlib_and_dependencies_in_ruby_order() { let loaded = loader.load(&mut env).unwrap(); assert_eq!(loaded.first().unwrap().kind, SourceKind::Core); - // bigdecimal-math pulls bigdecimal via manifest.yaml, after itself let first_math = loaded .iter() .position(|f| f.kind == lib("bigdecimal-math")) @@ -60,7 +57,6 @@ fn loads_core_stdlib_and_dependencies_in_ruby_order() { .position(|f| f.kind == lib("bigdecimal")) .unwrap(); assert!(first_math < first_dep); - // loading core implies stringio (stdlib migration compatibility) assert!(loaded.iter().any(|f| f.kind == lib("stringio"))); assert_eq!(env.sources().len(), loaded.len()); assert!(!env.interners().strings.is_empty()); @@ -165,8 +161,6 @@ fn core_requires_resolvable_stringio() { #[test] fn custom_repository_manifests_do_not_expand_dependencies() { - // Without stdlib_root the bigdecimal-math manifest is ignored even - // though the loading repository resolves the library itself. let loader = EnvironmentLoader::new(None, None) .add_repository_dir(repo_root().join("stdlib")) .add_library("bigdecimal-math", None); @@ -198,8 +192,7 @@ fn loaded_sources_carry_converted_declarations_and_directives() { source.declarations ); }; - // Names stay as written; Ruby absolutises them in `insert_rbs_decl`, - // which comes with declaration indexing later. + // Names stay as written; Ruby absolutises them in `insert_rbs_decl`. let interners = env.interners(); assert_eq!( interners.type_names.display(class.name, &interners.strings), @@ -210,7 +203,6 @@ fn loaded_sources_carry_converted_declarations_and_directives() { #[test] fn library_dirs_skip_underscore_directories() { - // Ruby passes skip_hidden: true for libraries, unlike explicit dirs. let dir = tree(&[ ("gem1/1.2.3/a.rbs", "class Person\nend\n"), ("gem1/1.2.3/_private/b.rbs", "class Person::Internal\nend\n"), From a4b3d65c3f7c7fcb3ed54cad4651a33ee22e2dd3 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Wed, 12 Aug 2026 22:25:39 +0900 Subject: [PATCH 09/24] Take a parsed GemVersion in Repository::lookup An invalid version string was indistinguishable from a missing gem at the lookup boundary, so every caller had to guard against it separately. Parsing once in the loader and carrying the result alongside its Library makes that mismatch unrepresentable. Co-Authored-By: Claude Opus 5 --- rust/ruby-rbs/src/loader/mod.rs | 63 +++++++++++++++++++-------------- rust/ruby-rbs/src/repository.rs | 18 +++++----- 2 files changed, 44 insertions(+), 37 deletions(-) diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index 376439c4be..45ae92d078 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -85,6 +85,14 @@ pub struct Library { pub version: Option, } +/// A [`Library`] paired with its parsed `version`, keeping the two from +/// drifting apart as they travel through dependency expansion and the final +/// `Repository::lookup`. +struct ResolvedLibrary { + library: Library, + version: Option, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct LoadedFile { pub path: PathBuf, @@ -183,14 +191,16 @@ impl EnvironmentLoader { result.push((SourceKind::Core, core.clone(), true)); } - for library in self.resolved_libraries(stdlib)? { - let dir = - library_dir(repository, &library).ok_or_else(|| LoadError::UnknownLibrary { - name: library.name.clone(), - version: library.version.clone(), + for resolved in self.resolved_libraries(stdlib)? { + let dir = repository + .lookup(&resolved.library.name, resolved.version.as_ref()) + .map(Path::to_path_buf) + .ok_or_else(|| LoadError::UnknownLibrary { + name: resolved.library.name.clone(), + version: resolved.library.version.clone(), })?; let kind = SourceKind::Library { - name: library.name, + name: resolved.library.name, path: dir.clone(), }; result.push((kind, dir, true)); @@ -238,7 +248,7 @@ impl EnvironmentLoader { /// Expands manifest dependencies depth-first in request order, matching /// the Ruby implementation's insertion-ordered `Set` of libraries. - fn resolved_libraries(&self, stdlib: &Repository) -> Result, LoadError> { + fn resolved_libraries(&self, stdlib: &Repository) -> Result, LoadError> { let mut seen: HashSet = HashSet::new(); let mut result = Vec::new(); @@ -264,31 +274,36 @@ impl EnvironmentLoader { library: Library, stdlib: &Repository, seen: &mut HashSet, - result: &mut Vec, + result: &mut Vec, ) -> Result<(), LoadError> { - if let Some(version) = &library.version { - // Ruby raises ArgumentError from Gem::Version/Gem::Requirement - // while resolving; fail before any lookup can misinterpret it. - if GemVersion::parse(version).is_none() { - return Err(LoadError::InvalidVersion { - name: library.name.clone(), - version: version.clone(), - }); - } - } - if !seen.insert(library.clone()) { return Ok(()); } - result.push(library.clone()); + + // Ruby raises ArgumentError from Gem::Version/Gem::Requirement while + // resolving; parse here, once, so an invalid version can never be + // misread by `Repository::lookup` as "not found". + let version = library + .version + .as_deref() + .map(|version| { + GemVersion::parse(version).ok_or_else(|| LoadError::InvalidVersion { + name: library.name.clone(), + version: version.to_string(), + }) + }) + .transpose()?; // Mirrors Ruby's resolve_dependencies: the stdlib repository is // consulted, not the loading repository, so a library that only // exists in a custom repository loads without dependency expansion, // same as Ruby. let dependency_dir = stdlib - .lookup(&library.name, library.version.as_deref()) + .lookup(&library.name, version.as_ref()) .map(Path::to_path_buf); + + result.push(ResolvedLibrary { library, version }); + if let Some(dir) = dependency_dir { for name in manifest::dependencies(&dir)? { self.add_library_recursive( @@ -307,12 +322,6 @@ impl EnvironmentLoader { } } -fn library_dir(repository: &Repository, library: &Library) -> Option { - repository - .lookup(&library.name, library.version.as_deref()) - .map(Path::to_path_buf) -} - /// Deliberately a free function taking only the interners, so the load loop /// can later be parallelised by handing each worker its own [`Interners`]. /// The parser's `SignatureNode` holds raw pointers and is not `Send`, so it diff --git a/rust/ruby-rbs/src/repository.rs b/rust/ruby-rbs/src/repository.rs index f824b85024..1b0a327ea9 100644 --- a/rust/ruby-rbs/src/repository.rs +++ b/rust/ruby-rbs/src/repository.rs @@ -76,20 +76,18 @@ impl Repository { /// string* sorts last — Ruby's `latest_version` sorts by the version /// string, so `1.2` beats `1.11`. /// - /// Returns `None` when `version` is not a valid `Gem::Version`; the - /// loader reports this as `InvalidVersion` instead of silently using the - /// latest version. - pub fn lookup(&self, gem: &str, version: Option<&str>) -> Option<&Path> { + /// Takes an already-parsed `GemVersion` rather than a raw string: an + /// invalid version string is a caller error (Ruby raises `ArgumentError` + /// resolving it), not a "not found" result, so it must be rejected + /// before it can reach `lookup` at all. + pub fn lookup(&self, gem: &str, version: Option<&GemVersion>) -> Option<&Path> { let gem = self.gems.get(gem)?; if gem.versions.is_empty() { return None; } match version { - Some(version) => { - let requested = GemVersion::parse(version)?; - find_best_version(&gem.versions, &requested.release()) - } + Some(requested) => find_best_version(&gem.versions, &requested.release()), None => gem .versions .last_key_value() @@ -129,8 +127,9 @@ mod tests { } fn lookup_name(repository: &Repository, gem: &str, version: Option<&str>) -> Option { + let version = version.map(|v| GemVersion::parse(v).unwrap()); repository - .lookup(gem, version) + .lookup(gem, version.as_ref()) .map(|path| path.file_name().unwrap().to_string_lossy().into_owned()) } @@ -177,7 +176,6 @@ mod tests { lookup_name(&repository, "uri", Some("1.0.pre")), Some("1.0".to_string()) ); - assert_eq!(repository.lookup("uri", Some("junk")), None); } #[test] From 43408ac55692ceef5a91090cb570ba8bf06bcc39 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Wed, 12 Aug 2026 22:58:18 +0900 Subject: [PATCH 10/24] Skip unreadable subdirectories instead of aborting the whole scan collect() propagated every io::Error, so one EACCES subdirectory failed the entire load. Ruby's Dir.glob silently skips such directories, so only skip PermissionDenied and NotFound to match it, while still surfacing genuine I/O failures. Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/src/file_finder.rs | 101 +++++++++++++++++++++++++------ 1 file changed, 82 insertions(+), 19 deletions(-) diff --git a/rust/ruby-rbs/src/file_finder.rs b/rust/ruby-rbs/src/file_finder.rs index 05ba46e42c..3a64457fa2 100644 --- a/rust/ruby-rbs/src/file_finder.rs +++ b/rust/ruby-rbs/src/file_finder.rs @@ -1,19 +1,12 @@ use std::io; use std::path::{Path, PathBuf}; -/// Enumerates `.rbs` entries under `path` in a deterministic (path-sorted) -/// order, mirroring `RBS::FileFinder.each_file` and its `Dir.glob` -/// semantics: entries whose name starts with `.` are neither listed nor -/// descended into, symlinked directories are not descended into, and any -/// entry named `*.rbs` is listed regardless of its file type. +/// Enumerates `.rbs` entries under `path` in path-sorted order, mirroring +/// `RBS::FileFinder.each_file` and its `Dir.glob` semantics. /// -/// When `path` is a file it is returned as-is. When `skip_hidden` is true, -/// entries under a directory whose name starts with `_` are skipped; the -/// entry's own name is not checked, same as the Ruby implementation. -/// -/// One divergence: Ruby sorts the `/`-joined strings `Dir.glob` returns, while -/// the sort here compares the platform separator. The two agree except on -/// Windows for names containing a character below `/`. +/// Divergences: the sort compares the platform separator rather than +/// `/`-joined strings, and only `PermissionDenied`/`NotFound` are skipped +/// where Ruby skips every open/stat failure. pub fn each_file(path: &Path, skip_hidden: bool) -> io::Result> { let mut files = Vec::new(); @@ -27,9 +20,26 @@ pub fn each_file(path: &Path, skip_hidden: bool) -> io::Result> { Ok(files) } +fn is_skippable(error: &io::Error) -> bool { + matches!( + error.kind(), + io::ErrorKind::PermissionDenied | io::ErrorKind::NotFound + ) +} + fn collect(dir: &Path, skip_hidden: bool, files: &mut Vec) -> io::Result<()> { - for entry in std::fs::read_dir(dir)? { - let entry = entry?; + let entries = match std::fs::read_dir(dir) { + Ok(entries) => entries, + Err(error) if is_skippable(&error) => return Ok(()), + Err(error) => return Err(error), + }; + + for entry in entries { + let entry = match entry { + Ok(entry) => entry, + Err(error) if is_skippable(&error) => continue, + Err(error) => return Err(error), + }; let path = entry.path(); if entry.file_name().to_string_lossy().starts_with('.') { @@ -40,9 +50,13 @@ fn collect(dir: &Path, skip_hidden: bool, files: &mut Vec) -> io::Resul files.push(path.clone()); } - // `file_type()` does not follow symlinks, so this naturally skips - // symlinked directories. - if entry.file_type()?.is_dir() { + // `file_type()` does not follow symlinks, which skips symlinked dirs. + let file_type = match entry.file_type() { + Ok(file_type) => file_type, + Err(error) if is_skippable(&error) => continue, + Err(error) => return Err(error), + }; + if file_type.is_dir() { let hidden = path .file_name() .is_some_and(|name| name.to_string_lossy().starts_with('_')); @@ -70,8 +84,6 @@ mod tests { dir } - /// Paths relative to `root`, always `/`-joined so the expectations below - /// read the same on Windows. fn relative(paths: Vec, root: &Path) -> Vec { paths .iter() @@ -152,6 +164,57 @@ mod tests { assert_eq!(relative(found, dir.path()), vec!["normal/ok.rbs"]); } + #[cfg(unix)] + #[test] + fn unreadable_directories_are_skipped() { + use std::os::unix::fs::PermissionsExt; + + let dir = fixture(&["a.rbs", "readable/b.rbs", "locked/c.rbs"]); + let locked = dir.path().join("locked"); + fs::set_permissions(&locked, fs::Permissions::from_mode(0o000)).unwrap(); + + // Permission bits do not deny root (common in CI containers). + if fs::read_dir(&locked).is_ok() { + fs::set_permissions(&locked, fs::Permissions::from_mode(0o755)).unwrap(); + return; + } + + let found = each_file(dir.path(), false); + + fs::set_permissions(&locked, fs::Permissions::from_mode(0o755)).unwrap(); + + assert_eq!( + relative(found.unwrap(), dir.path()), + vec!["a.rbs", "readable/b.rbs"] + ); + } + + #[cfg(unix)] + #[test] + fn unstattable_entries_are_skipped() { + use std::os::unix::fs::PermissionsExt; + + let dir = fixture(&["a.rbs", "listable/b.rbs", "listable/nested/c.rbs"]); + let listable = dir.path().join("listable"); + fs::set_permissions(&listable, fs::Permissions::from_mode(0o444)).unwrap(); + + // Permission bits do not deny root (common in CI containers). + if fs::metadata(listable.join("b.rbs")).is_ok() { + fs::set_permissions(&listable, fs::Permissions::from_mode(0o755)).unwrap(); + return; + } + + let found = each_file(dir.path(), false); + + fs::set_permissions(&listable, fs::Permissions::from_mode(0o755)).unwrap(); + + // `b.rbs` is listed by name; `nested` cannot be identified as a dir. + assert_eq!( + relative(found.unwrap(), dir.path()), + vec!["a.rbs", "listable/b.rbs"] + ); + } + #[test] fn entries_named_rbs_match_regardless_of_file_type() { let dir = fixture(&["dir.rbs/inner.rbs"]); From cd1faf7009a6224ec436fde3572b08ce033dac66 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Wed, 12 Aug 2026 23:08:55 +0900 Subject: [PATCH 11/24] Remove unused position-conversion machinery from Buffer line_ranges, line_count, line, pos_to_loc, loc_to_pos, and last_position had no callers outside their own tests, and their byte-offset semantics diverge from Ruby's character-offset RBS::Buffer anyway. Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/src/buffer.rs | 166 +----------------------------------- 1 file changed, 2 insertions(+), 164 deletions(-) diff --git a/rust/ruby-rbs/src/buffer.rs b/rust/ruby-rbs/src/buffer.rs index 84168c8fa6..527ff1cfb3 100644 --- a/rust/ruby-rbs/src/buffer.rs +++ b/rust/ruby-rbs/src/buffer.rs @@ -1,27 +1,15 @@ -use std::cell::OnceCell; -use std::ops::Range; use std::path::{Path, PathBuf}; -/// A signature file's content with byte position <-> line/column conversion, -/// mirroring `RBS::Buffer`. -/// -/// Positions and columns are byte offsets, unlike Ruby's character offsets; -/// they agree on ASCII-only content. Use the parser's `RBSLocationRange` when -/// character offsets are needed. +/// A signature file's content, mirroring `RBS::Buffer`. #[derive(Debug, Clone)] pub struct Buffer { name: PathBuf, content: String, - line_ranges: OnceCell>>, } impl Buffer { pub fn new(name: PathBuf, content: String) -> Self { - Self { - name, - content, - line_ranges: OnceCell::new(), - } + Self { name, content } } pub fn name(&self) -> &Path { @@ -31,154 +19,4 @@ impl Buffer { pub fn content(&self) -> &str { &self.content } - - fn line_ranges(&self) -> &[Range] { - self.line_ranges - .get_or_init(|| compute_line_ranges(&self.content)) - } - - pub fn line_count(&self) -> usize { - self.line_ranges().len() - } - - /// Returns the 1-origin `line` without its line terminator. - pub fn line(&self, line: usize) -> Option<&str> { - let range = self.line_ranges().get(line.checked_sub(1)?)?; - Some(&self.content[range.clone()]) - } - - /// A position past the end of the content maps to `(line_count + 1, 0)`, - /// same as the Ruby implementation. - pub fn pos_to_loc(&self, pos: usize) -> (usize, usize) { - let ranges = self.line_ranges(); - let index = ranges.partition_point(|range| range.end < pos); - match ranges.get(index) { - Some(range) => (index + 1, pos.saturating_sub(range.start)), - None => (ranges.len() + 1, 0), - } - } - - pub fn loc_to_pos(&self, line: usize, column: usize) -> usize { - let ranges = self.line_ranges(); - let range = match line.checked_sub(1) { - None => ranges.last(), - Some(index) => ranges.get(index), - }; - match range { - Some(range) => range.start + column, - None => self.last_position(), - } - } - - pub fn last_position(&self) -> usize { - self.line_ranges().last().map_or(0, |range| range.end) - } -} - -fn compute_line_ranges(content: &str) -> Vec> { - let mut ranges = Vec::new(); - let mut offset = 0; - - for line in content.split_inclusive('\n') { - let without_terminator = match line.strip_suffix('\n') { - Some(l) => l.strip_suffix('\r').unwrap_or(l), - None => line.strip_suffix('\r').unwrap_or(line), - }; - ranges.push(offset..offset + without_terminator.len()); - offset += line.len(); - } - - if content.is_empty() || content.ends_with('\n') { - ranges.push(offset..offset); - } - - ranges -} - -#[cfg(test)] -mod tests { - use super::*; - - fn buffer(content: &str) -> Buffer { - Buffer::new(PathBuf::from("a.rbs"), content.to_string()) - } - - #[test] - fn lines_of_content_with_trailing_newline() { - let buffer = buffer("123\nabc\n"); - assert_eq!(buffer.line_count(), 3); - assert_eq!(buffer.line(1), Some("123")); - assert_eq!(buffer.line(2), Some("abc")); - assert_eq!(buffer.line(3), Some("")); - assert_eq!(buffer.line(4), None); - assert_eq!(buffer.line(0), None); - } - - #[test] - fn lines_of_content_without_trailing_newline() { - let buffer = buffer("123\nabc"); - assert_eq!(buffer.line_count(), 2); - assert_eq!(buffer.line(2), Some("abc")); - assert_eq!(buffer.last_position(), 7); - } - - #[test] - fn empty_content_has_one_empty_line() { - let buffer = buffer(""); - assert_eq!(buffer.line_count(), 1); - assert_eq!(buffer.line(1), Some("")); - assert_eq!(buffer.last_position(), 0); - assert_eq!(buffer.pos_to_loc(0), (1, 0)); - } - - #[test] - fn crlf_is_excluded_from_line_ranges() { - let buffer = buffer("abc\r\ndef\r\n"); - assert_eq!(buffer.line(1), Some("abc")); - assert_eq!(buffer.line(2), Some("def")); - assert_eq!(buffer.pos_to_loc(5), (2, 0)); - } - - #[test] - fn pos_to_loc_matches_ruby_buffer_for_ascii() { - let buffer = buffer("123\nabc\n"); - assert_eq!(buffer.pos_to_loc(0), (1, 0)); - assert_eq!(buffer.pos_to_loc(3), (1, 3)); - assert_eq!(buffer.pos_to_loc(4), (2, 0)); - assert_eq!(buffer.pos_to_loc(7), (2, 3)); - assert_eq!(buffer.pos_to_loc(8), (3, 0)); - assert_eq!(buffer.pos_to_loc(9), (4, 0)); - } - - #[test] - fn loc_to_pos_matches_ruby_buffer_for_ascii() { - let buffer = buffer("123\nabc\n"); - assert_eq!(buffer.loc_to_pos(1, 0), 0); - assert_eq!(buffer.loc_to_pos(2, 3), 7); - assert_eq!(buffer.loc_to_pos(3, 0), 8); - assert_eq!(buffer.loc_to_pos(10, 5), 8); - assert_eq!(buffer.last_position(), 8); - } - - #[test] - fn trailing_carriage_return_without_newline_is_chomped() { - let buffer = buffer("abc\ndef\r"); - assert_eq!(buffer.line(2), Some("def")); - assert_eq!(buffer.last_position(), 7); - } - - #[test] - fn line_zero_maps_to_the_last_line() { - let buffer = buffer("123\nabc"); - assert_eq!(buffer.loc_to_pos(0, 2), 6); - } - - #[test] - fn the_line_table_is_built_only_on_demand() { - let buffer = buffer("123\nabc\n"); - assert!(buffer.line_ranges.get().is_none()); - - buffer.pos_to_loc(4); - assert!(buffer.line_ranges.get().is_some()); - } } From e088d876bb240f3f6382564b41d68cc79f682b09 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Wed, 12 Aug 2026 23:21:08 +0900 Subject: [PATCH 12/24] Remove duplicate ModuleSelfAnnotation match arm after merge Merging upstream/master reintroduced this arm earlier in the match (added independently on master), leaving the branch's copy at the end unreachable. With RUSTFLAGS=-D warnings this failed cargo test and cargo clippy in CI. Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/src/ast/convert.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rust/ruby-rbs/src/ast/convert.rs b/rust/ruby-rbs/src/ast/convert.rs index b0877e5643..163f4b7004 100644 --- a/rust/ruby-rbs/src/ast/convert.rs +++ b/rust/ruby-rbs/src/ast/convert.rs @@ -1252,6 +1252,5 @@ fn node_kind(node: &Node<'_>) -> &'static str { Node::UnionType(_) => "UnionType", Node::UntypedFunctionType(_) => "UntypedFunctionType", Node::VariableType(_) => "VariableType", - Node::ModuleSelfAnnotation(_) => "ModuleSelfAnnotation", } } From fcf2f0f2f202840d9891697f044fe2f7e96c33b3 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Thu, 13 Aug 2026 22:46:27 +0900 Subject: [PATCH 13/24] Add Environment::add_rbs and route EnvironmentLoader through it Steep-style callers need to add parsed sources to an Environment without going through EnvironmentLoader's directory scan. Parsing always uses the environment's own Interners, so a source added this way can never carry ids from a different environment. load() now calls add_rbs_file too, so parsing lives in one place. Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/src/environment/mod.rs | 79 +++++++++++++++++++++++----- rust/ruby-rbs/src/loader/mod.rs | 55 +------------------ 2 files changed, 69 insertions(+), 65 deletions(-) diff --git a/rust/ruby-rbs/src/environment/mod.rs b/rust/ruby-rbs/src/environment/mod.rs index aa73d7d9f4..3167c1f7d9 100644 --- a/rust/ruby-rbs/src/environment/mod.rs +++ b/rust/ruby-rbs/src/environment/mod.rs @@ -2,8 +2,13 @@ pub mod source; pub use source::{Source, SourceKind}; +use std::path::{Path, PathBuf}; + +use crate::ast::AstConverter; +use crate::buffer::Buffer; use crate::interners::Interners; use crate::loader::{EnvironmentLoader, LoadError}; +use crate::node; /// Owning the interners here gives a single `Environment` value the same /// role as the Ruby implementation's global name pool: names interned while @@ -29,12 +34,56 @@ impl Environment { &self.sources } - pub(crate) fn interners_mut(&mut self) -> &mut Interners { - &mut self.interners + /// Parses `content` with this environment's own [`Interners`] and adds it + /// as a source, without going through an [`EnvironmentLoader`] — the + /// entry point for editor / in-memory callers (`RBS::Environment#add_source` + /// equivalent, e.g. Steep). Parsing always uses this environment's + /// `Interners`, so a `Source` added this way can never carry ids from a + /// different environment. + pub fn add_rbs( + &mut self, + path: PathBuf, + content: String, + kind: SourceKind, + ) -> Result<(), LoadError> { + let signature = node::parse(&content).map_err(|message| LoadError::Parse { + path: path.clone(), + message, + })?; + + let mut converter = + AstConverter::new(&mut self.interners.strings, &mut self.interners.type_names); + let directives = signature + .directives() + .iter() + .map(|node| converter.convert_directive(&node)) + .collect(); + let declarations = signature + .declarations() + .iter() + .map(|node| converter.convert_declaration(&node)) + .collect(); + // SignatureNode borrows `content` and has a Drop impl; drop it + // explicitly before moving `content` into the Buffer. + drop(signature); + + self.sources.push(Source { + buffer: Buffer::new(path, content), + directives, + declarations, + kind, + }); + Ok(()) } - pub(crate) fn add_source(&mut self, source: Source) { - self.sources.push(source); + /// Reads `path` and adds it as a source; a thin wrapper around + /// [`Environment::add_rbs`]. + pub fn add_rbs_file(&mut self, path: &Path, kind: SourceKind) -> Result<(), LoadError> { + let content = std::fs::read_to_string(path).map_err(|source| LoadError::Io { + path: path.to_path_buf(), + source, + })?; + self.add_rbs(path.to_path_buf(), content, kind) } pub fn from_loader(loader: &EnvironmentLoader) -> Result { @@ -53,21 +102,27 @@ impl Default for Environment { #[cfg(test)] mod tests { use super::*; + use crate::ast::Declaration; #[test] fn environment_owns_interners() { let mut env = Environment::new(); + env.add_rbs( + PathBuf::from("test.rbs"), + "class Foo\nend\n".to_string(), + SourceKind::Dir { + path: PathBuf::from("."), + }, + ) + .unwrap(); - let interners = env.interners_mut(); - let symbol = interners.strings.intern("Foo"); - let root = interners.type_names.absolute_root(); - let name = interners.type_names.append(root, symbol); - + let [Declaration::Class(class)] = env.sources()[0].declarations.as_slice() else { + panic!("expected one class declaration"); + }; let interners = env.interners(); assert_eq!( - interners.type_names.display(name, &interners.strings), - "::Foo" + interners.type_names.display(class.name, &interners.strings), + "Foo" ); - assert!(env.sources().is_empty()); } } diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index 45ae92d078..475fea2ec0 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -5,13 +5,9 @@ use std::fmt; use std::io; use std::path::{Path, PathBuf}; -use crate::ast::AstConverter; -use crate::buffer::Buffer; -use crate::environment::{Environment, Source, SourceKind}; +use crate::environment::{Environment, SourceKind}; use crate::file_finder; use crate::gem_version::GemVersion; -use crate::interners::Interners; -use crate::node; use crate::repository::Repository; #[derive(Debug)] @@ -168,8 +164,7 @@ impl EnvironmentLoader { if !seen_files.insert(path.clone()) { continue; } - let source = parse_one(&path, &kind, env.interners_mut())?; - env.add_source(source); + env.add_rbs_file(&path, kind.clone())?; loaded.push(LoadedFile { path, kind: kind.clone(), @@ -321,49 +316,3 @@ impl EnvironmentLoader { Ok(()) } } - -/// Deliberately a free function taking only the interners, so the load loop -/// can later be parallelised by handing each worker its own [`Interners`]. -/// The parser's `SignatureNode` holds raw pointers and is not `Send`, so it -/// must not escape this function — only the owned `Source` does. -/// -/// Crate-private: a caller outside the crate has no way to merge its -/// worker-local [`Interners`] into the environment, so the `Source` it -/// produced would carry ids that environment cannot resolve. -pub(crate) fn parse_one( - path: &Path, - kind: &SourceKind, - interners: &mut Interners, -) -> Result { - let content = std::fs::read_to_string(path).map_err(|source| LoadError::Io { - path: path.to_path_buf(), - source, - })?; - - let signature = node::parse(&content).map_err(|message| LoadError::Parse { - path: path.to_path_buf(), - message, - })?; - - let mut converter = AstConverter::new(&mut interners.strings, &mut interners.type_names); - let directives = signature - .directives() - .iter() - .map(|node| converter.convert_directive(&node)) - .collect(); - let declarations = signature - .declarations() - .iter() - .map(|node| converter.convert_declaration(&node)) - .collect(); - // SignatureNode borrows `content` and has a Drop impl; drop it - // explicitly before moving `content` into the Buffer. - drop(signature); - - Ok(Source { - buffer: Buffer::new(path.to_path_buf(), content), - directives, - declarations, - kind: kind.clone(), - }) -} From 7e98ba9a64699fd68b215371ca335ad464989386 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Thu, 13 Aug 2026 22:46:49 +0900 Subject: [PATCH 14/24] Take a resolved path in EnvironmentLoader::add_library Ruby now resolves gem name/version to a directory before calling Rust, so add_library just takes the path. Drop the loader's own resolution path with it: dependency expansion, the stdlib/loading repositories, and the version-related LoadError variants. Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/src/environment/source.rs | 14 +- rust/ruby-rbs/src/loader/mod.rs | 257 ++++-------------------- rust/ruby-rbs/tests/loader.rs | 106 ++-------- 3 files changed, 63 insertions(+), 314 deletions(-) diff --git a/rust/ruby-rbs/src/environment/source.rs b/rust/ruby-rbs/src/environment/source.rs index cac63c5057..237d2cbe89 100644 --- a/rust/ruby-rbs/src/environment/source.rs +++ b/rust/ruby-rbs/src/environment/source.rs @@ -8,9 +8,9 @@ use crate::buffer::Buffer; #[derive(Debug, Clone, PartialEq, Eq)] pub enum SourceKind { Core, - /// `path` is the resolved version directory - /// ([`Repository::lookup`](crate::repository::Repository::lookup)), which - /// holds the library's RBS files directly. + /// `path` is the library's version directory, already resolved by the + /// caller (Ruby's `Repository#lookup` / `gem_sig_path`), holding the + /// library's RBS files directly. Library { name: String, path: PathBuf, @@ -20,6 +20,14 @@ pub enum SourceKind { }, } +impl SourceKind { + /// Whether `_`-prefixed subdirectories are skipped while scanning. + /// Only a user-specified [`SourceKind::Dir`] does not skip them. + pub fn skips_hidden(&self) -> bool { + !matches!(self, SourceKind::Dir { .. }) + } +} + /// `RBS::Source::RBS` equivalent. #[derive(Debug)] pub struct Source { diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index 475fea2ec0..4d228934f3 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -1,66 +1,27 @@ -pub mod manifest; - use std::collections::HashSet; use std::fmt; use std::io; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use crate::environment::{Environment, SourceKind}; use crate::file_finder; -use crate::gem_version::GemVersion; -use crate::repository::Repository; #[derive(Debug)] #[non_exhaustive] pub enum LoadError { - UnknownLibrary { - name: String, - version: Option, - }, - /// A requested library version is not a valid `Gem::Version`, where the - /// Ruby implementation raises `ArgumentError`. - InvalidVersion { - name: String, - version: String, - }, - Io { - path: PathBuf, - source: io::Error, - }, - Parse { - path: PathBuf, - message: String, - }, - Manifest { - path: PathBuf, - message: String, - }, + Io { path: PathBuf, source: io::Error }, + Parse { path: PathBuf, message: String }, } impl fmt::Display for LoadError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - LoadError::UnknownLibrary { name, version } => write!( - f, - "Cannot find type definitions for library: {} ({})", - name, - version.as_deref().unwrap_or("[nil]") - ), - LoadError::InvalidVersion { name, version } => { - write!( - f, - "Malformed version number string {version} for library {name}" - ) - } LoadError::Io { path, source } => { write!(f, "IO error on {}: {}", path.display(), source) } LoadError::Parse { path, message } => { write!(f, "Syntax error in {}: {}", path.display(), message) } - LoadError::Manifest { path, message } => { - write!(f, "Invalid manifest {}: {}", path.display(), message) - } } } } @@ -74,66 +35,41 @@ impl std::error::Error for LoadError { } } -/// `RBS::EnvironmentLoader::Library` equivalent. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct Library { - pub name: String, - pub version: Option, -} - -/// A [`Library`] paired with its parsed `version`, keeping the two from -/// drifting apart as they travel through dependency expansion and the final -/// `Repository::lookup`. -struct ResolvedLibrary { - library: Library, - version: Option, -} - #[derive(Debug, Clone, PartialEq, Eq)] pub struct LoadedFile { pub path: PathBuf, pub kind: SourceKind, } -/// Resolves core, library, and explicit signature directories, parses every -/// `.rbs` file exactly once, and feeds the results into an [`Environment`], -/// mirroring `RBS::EnvironmentLoader`. +/// Enumerates core, library, and explicit signature directories, parses +/// every `.rbs` file exactly once, and feeds the results into an +/// [`Environment`], mirroring `RBS::EnvironmentLoader`. +/// +/// Unlike the Ruby implementation, this does not resolve gem names or +/// versions to directories, and does not expand `manifest.yaml` +/// dependencies: callers (Ruby's `Repository#lookup` / `gem_sig_path`) pass +/// already-resolved paths to [`EnvironmentLoader::add_library`]. Loading +/// core does not implicitly add `stringio` either — that dependency is the +/// caller's responsibility, same as any other library. pub struct EnvironmentLoader { core_root: Option, - stdlib_root: Option, - repository_dirs: Vec, - libs: Vec, + libs: Vec<(String, PathBuf)>, dirs: Vec, } impl EnvironmentLoader { - /// `core_root` is `None` to skip core; `stdlib_root` is the stdlib - /// signature directory used for dependency expansion and for resolving - /// requested libraries — `None` disables both. - pub fn new(core_root: Option, stdlib_root: Option) -> Self { + /// `core_root` is `None` to skip core. + pub fn new(core_root: Option) -> Self { EnvironmentLoader { core_root, - stdlib_root, - repository_dirs: Vec::new(), libs: Vec::new(), dirs: Vec::new(), } } - /// Adds a repository root for resolving libraries, searched after - /// `stdlib_root`; a later root wins for the same gem and version. - pub fn add_repository_dir(mut self, path: PathBuf) -> Self { - self.repository_dirs.push(path); - self - } - - /// A requested library's `manifest.yaml` dependencies are resolved - /// transitively at load time. - pub fn add_library(mut self, name: &str, version: Option<&str>) -> Self { - self.libs.push(Library { - name: name.to_string(), - version: version.map(str::to_string), - }); + /// Adds a library by name and its already-resolved signature directory. + pub fn add_library(mut self, name: &str, path: PathBuf) -> Self { + self.libs.push((name.to_string(), path)); self } @@ -147,18 +83,16 @@ impl EnvironmentLoader { /// On `Err` the sources read before the failure are already in `env`, same /// as the Ruby implementation adding sources as it walks the directories. pub fn load(&self, env: &mut Environment) -> Result, LoadError> { - let stdlib = self.stdlib_repository()?; - let repository = self.loading_repository()?; - let mut loaded = Vec::new(); let mut seen_files: HashSet = HashSet::new(); - for (kind, dir, skip_hidden) in self.each_dir(&stdlib, &repository)? { - let files = - file_finder::each_file(&dir, skip_hidden).map_err(|source| LoadError::Io { + for (kind, dir) in self.each_dir() { + let files = file_finder::each_file(&dir, kind.skips_hidden()).map_err(|source| { + LoadError::Io { path: dir.clone(), source, - })?; + } + })?; for path in files { if !seen_files.insert(path.clone()) { @@ -175,144 +109,29 @@ impl EnvironmentLoader { Ok(loaded) } - fn each_dir( - &self, - stdlib: &Repository, - repository: &Repository, - ) -> Result, LoadError> { + /// Groups directories core → libs → dirs, matching + /// `RBS::EnvironmentLoader#each_dir`'s enumeration order. + fn each_dir(&self) -> Vec<(SourceKind, PathBuf)> { let mut result = Vec::new(); if let Some(core) = &self.core_root { - result.push((SourceKind::Core, core.clone(), true)); + result.push((SourceKind::Core, core.clone())); } - for resolved in self.resolved_libraries(stdlib)? { - let dir = repository - .lookup(&resolved.library.name, resolved.version.as_ref()) - .map(Path::to_path_buf) - .ok_or_else(|| LoadError::UnknownLibrary { - name: resolved.library.name.clone(), - version: resolved.library.version.clone(), - })?; - let kind = SourceKind::Library { - name: resolved.library.name, - path: dir.clone(), - }; - result.push((kind, dir, true)); + for (name, path) in &self.libs { + result.push(( + SourceKind::Library { + name: name.clone(), + path: path.clone(), + }, + path.clone(), + )); } for dir in &self.dirs { - result.push((SourceKind::Dir { path: dir.clone() }, dir.clone(), false)); - } - - Ok(result) - } - - /// The repository backing dependency expansion, mirroring - /// `Collection::Sources::Stdlib`'s dedicated repository. Built from - /// `stdlib_root` on each load; independent of the loading repository. - fn stdlib_repository(&self) -> Result { - let mut repository = Repository::new(); - if let Some(root) = &self.stdlib_root { - repository.add(root).map_err(|source| LoadError::Io { - path: root.clone(), - source, - })?; - } - Ok(repository) - } - - /// `stdlib_root` is registered first, standing in for Ruby's - /// `Repository.new` auto-registering `DEFAULT_STDLIB_ROOT`. - fn loading_repository(&self) -> Result { - let mut repository = Repository::new(); - if let Some(root) = &self.stdlib_root { - repository.add(root).map_err(|source| LoadError::Io { - path: root.clone(), - source, - })?; - } - for dir in &self.repository_dirs { - repository.add(dir).map_err(|source| LoadError::Io { - path: dir.clone(), - source, - })?; - } - Ok(repository) - } - - /// Expands manifest dependencies depth-first in request order, matching - /// the Ruby implementation's insertion-ordered `Set` of libraries. - fn resolved_libraries(&self, stdlib: &Repository) -> Result, LoadError> { - let mut seen: HashSet = HashSet::new(); - let mut result = Vec::new(); - - for library in &self.libs { - self.add_library_recursive(library.clone(), stdlib, &mut seen, &mut result)?; - } - - // Loading core implies stringio (stdlib migration), unconditionally, - // same as Ruby; an unresolvable stringio is an UnknownLibrary error. - if self.core_root.is_some() && !seen.iter().any(|library| library.name == "stringio") { - let stringio = Library { - name: "stringio".to_string(), - version: None, - }; - self.add_library_recursive(stringio, stdlib, &mut seen, &mut result)?; - } - - Ok(result) - } - - fn add_library_recursive( - &self, - library: Library, - stdlib: &Repository, - seen: &mut HashSet, - result: &mut Vec, - ) -> Result<(), LoadError> { - if !seen.insert(library.clone()) { - return Ok(()); - } - - // Ruby raises ArgumentError from Gem::Version/Gem::Requirement while - // resolving; parse here, once, so an invalid version can never be - // misread by `Repository::lookup` as "not found". - let version = library - .version - .as_deref() - .map(|version| { - GemVersion::parse(version).ok_or_else(|| LoadError::InvalidVersion { - name: library.name.clone(), - version: version.to_string(), - }) - }) - .transpose()?; - - // Mirrors Ruby's resolve_dependencies: the stdlib repository is - // consulted, not the loading repository, so a library that only - // exists in a custom repository loads without dependency expansion, - // same as Ruby. - let dependency_dir = stdlib - .lookup(&library.name, version.as_ref()) - .map(Path::to_path_buf); - - result.push(ResolvedLibrary { library, version }); - - if let Some(dir) = dependency_dir { - for name in manifest::dependencies(&dir)? { - self.add_library_recursive( - Library { - name, - version: None, - }, - stdlib, - seen, - result, - )?; - } + result.push((SourceKind::Dir { path: dir.clone() }, dir.clone())); } - Ok(()) + result } } diff --git a/rust/ruby-rbs/tests/loader.rs b/rust/ruby-rbs/tests/loader.rs index cd40ccfc1a..4ebdfbc8da 100644 --- a/rust/ruby-rbs/tests/loader.rs +++ b/rust/ruby-rbs/tests/loader.rs @@ -4,25 +4,19 @@ use std::path::{Path, PathBuf}; use ruby_rbs::ast::{Declaration, Directive}; use ruby_rbs::environment::{Environment, SourceKind}; use ruby_rbs::loader::{EnvironmentLoader, LoadError}; -use ruby_rbs::repository::Repository; fn repo_root() -> PathBuf { Path::new(env!("CARGO_MANIFEST_DIR")).join("../..") } -fn stdlib_repository() -> Repository { - let mut repository = Repository::new(); - repository.add(&repo_root().join("stdlib")).unwrap(); - repository +fn stdlib_dir(name: &str) -> PathBuf { + repo_root().join("stdlib").join(name).join("0") } fn lib(name: &str) -> SourceKind { SourceKind::Library { name: name.to_string(), - path: stdlib_repository() - .lookup(name, None) - .expect("test library must resolve in the stdlib repository") - .to_path_buf(), + path: stdlib_dir(name), } } @@ -37,12 +31,10 @@ fn tree(files: &[(&str, &str)]) -> tempfile::TempDir { } #[test] -fn loads_core_stdlib_and_dependencies_in_ruby_order() { - let loader = EnvironmentLoader::new( - Some(repo_root().join("core")), - Some(repo_root().join("stdlib")), - ) - .add_library("bigdecimal-math", None); +fn loads_registered_sources_in_registration_order() { + let loader = EnvironmentLoader::new(Some(repo_root().join("core"))) + .add_library("bigdecimal-math", stdlib_dir("bigdecimal-math")) + .add_library("bigdecimal", stdlib_dir("bigdecimal")); let mut env = Environment::new(); let loaded = loader.load(&mut env).unwrap(); @@ -57,17 +49,14 @@ fn loads_core_stdlib_and_dependencies_in_ruby_order() { .position(|f| f.kind == lib("bigdecimal")) .unwrap(); assert!(first_math < first_dep); - assert!(loaded.iter().any(|f| f.kind == lib("stringio"))); assert_eq!(env.sources().len(), loaded.len()); assert!(!env.interners().strings.is_empty()); } #[test] fn from_loader_is_the_primary_entry_point() { - let loader = EnvironmentLoader::new( - Some(repo_root().join("core")), - Some(repo_root().join("stdlib")), - ); + let loader = EnvironmentLoader::new(Some(repo_root().join("core"))) + .add_library("stringio", stdlib_dir("stringio")); let env = Environment::from_loader(&loader).unwrap(); @@ -79,7 +68,7 @@ fn files_are_loaded_once_first_wins() { let dir = tempfile::tempdir().unwrap(); fs::write(dir.path().join("a.rbs"), "class Foo\nend\n").unwrap(); - let loader = EnvironmentLoader::new(None, None) + let loader = EnvironmentLoader::new(None) .add_dir(dir.path().to_path_buf()) .add_dir(dir.path().to_path_buf()); @@ -96,7 +85,7 @@ fn explicit_dirs_do_not_skip_underscore_directories() { fs::create_dir(dir.path().join("_private")).unwrap(); fs::write(dir.path().join("_private/a.rbs"), "class Foo\nend\n").unwrap(); - let loader = EnvironmentLoader::new(None, None).add_dir(dir.path().to_path_buf()); + let loader = EnvironmentLoader::new(None).add_dir(dir.path().to_path_buf()); let mut env = Environment::new(); let loaded = loader.load(&mut env).unwrap(); @@ -104,25 +93,12 @@ fn explicit_dirs_do_not_skip_underscore_directories() { assert_eq!(loaded.len(), 1); } -#[test] -fn unknown_library_is_an_error() { - let loader = EnvironmentLoader::new(None, None).add_library("no_such_gem", None); - - let mut env = Environment::new(); - let error = loader.load(&mut env).unwrap_err(); - - assert!(matches!( - error, - LoadError::UnknownLibrary { ref name, version: None } if name == "no_such_gem" - )); -} - #[test] fn parse_errors_carry_the_file_path() { let dir = tempfile::tempdir().unwrap(); fs::write(dir.path().join("broken.rbs"), "class\n").unwrap(); - let loader = EnvironmentLoader::new(None, None).add_dir(dir.path().to_path_buf()); + let loader = EnvironmentLoader::new(None).add_dir(dir.path().to_path_buf()); let mut env = Environment::new(); let error = loader.load(&mut env).unwrap_err(); @@ -133,45 +109,6 @@ fn parse_errors_carry_the_file_path() { )); } -#[test] -fn default_configuration_loads_without_extra_repository_wiring() { - let loader = EnvironmentLoader::new( - Some(repo_root().join("core")), - Some(repo_root().join("stdlib")), - ); - - let mut env = Environment::new(); - let loaded = loader.load(&mut env).unwrap(); - - assert!(loaded.iter().any(|f| f.kind == lib("stringio"))); -} - -#[test] -fn core_requires_resolvable_stringio() { - let loader = EnvironmentLoader::new(Some(repo_root().join("core")), None); - - let mut env = Environment::new(); - let error = loader.load(&mut env).unwrap_err(); - - assert!(matches!( - error, - LoadError::UnknownLibrary { ref name, version: None } if name == "stringio" - )); -} - -#[test] -fn custom_repository_manifests_do_not_expand_dependencies() { - let loader = EnvironmentLoader::new(None, None) - .add_repository_dir(repo_root().join("stdlib")) - .add_library("bigdecimal-math", None); - - let mut env = Environment::new(); - let loaded = loader.load(&mut env).unwrap(); - - assert!(!loaded.is_empty()); - assert!(loaded.iter().all(|f| f.kind == lib("bigdecimal-math"))); -} - #[test] fn loaded_sources_carry_converted_declarations_and_directives() { let dir = tree(&[( @@ -179,7 +116,7 @@ fn loaded_sources_carry_converted_declarations_and_directives() { "use Foo::Bar\n\nclass Person\n def name: () -> String\nend\n", )]); - let loader = EnvironmentLoader::new(None, None).add_dir(dir.path().to_path_buf()); + let loader = EnvironmentLoader::new(None).add_dir(dir.path().to_path_buf()); let env = Environment::from_loader(&loader).unwrap(); let source = &env.sources()[0]; @@ -208,9 +145,7 @@ fn library_dirs_skip_underscore_directories() { ("gem1/1.2.3/_private/b.rbs", "class Person::Internal\nend\n"), ]); - let loader = EnvironmentLoader::new(None, None) - .add_repository_dir(dir.path().to_path_buf()) - .add_library("gem1", Some("1.2.3")); + let loader = EnvironmentLoader::new(None).add_library("gem1", dir.path().join("gem1/1.2.3")); let mut env = Environment::new(); let loaded = loader.load(&mut env).unwrap(); @@ -225,16 +160,3 @@ fn library_dirs_skip_underscore_directories() { } ); } - -#[test] -fn invalid_library_version_is_reported_distinctly() { - let loader = EnvironmentLoader::new(None, None).add_library("uri", Some("junk")); - - let mut env = Environment::new(); - let error = loader.load(&mut env).unwrap_err(); - - assert!(matches!( - error, - LoadError::InvalidVersion { ref name, ref version } if name == "uri" && version == "junk" - )); -} From 8df6b4d6a9a219e3b1e4f24ba8e58d568fe0acb1 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Thu, 13 Aug 2026 22:47:12 +0900 Subject: [PATCH 15/24] Remove Repository, GemVersion, and manifest parsing Unused now that add_library takes an already-resolved path instead of a name/version pair. Recoverable from git history if Rust ever needs its own version comparison again. Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/Cargo.toml | 2 - rust/ruby-rbs/src/gem_version.rs | 306 --------------------------- rust/ruby-rbs/src/lib.rs | 4 +- rust/ruby-rbs/src/loader/manifest.rs | 214 ------------------- rust/ruby-rbs/src/repository.rs | 237 --------------------- 5 files changed, 1 insertion(+), 762 deletions(-) delete mode 100644 rust/ruby-rbs/src/gem_version.rs delete mode 100644 rust/ruby-rbs/src/loader/manifest.rs delete mode 100644 rust/ruby-rbs/src/repository.rs diff --git a/rust/ruby-rbs/Cargo.toml b/rust/ruby-rbs/Cargo.toml index c6d3ede5a0..973162288b 100644 --- a/rust/ruby-rbs/Cargo.toml +++ b/rust/ruby-rbs/Cargo.toml @@ -17,8 +17,6 @@ include = [ [dependencies] ruby-rbs-sys = { version = "0.3", path = "../ruby-rbs-sys" } xxhash-rust = { version = "0.8", features = ["xxh3"] } -serde = { version = "1.0", features = ["derive"] } -serde_yaml = "0.9" [build-dependencies] serde = { version = "1.0", features = ["derive"] } diff --git a/rust/ruby-rbs/src/gem_version.rs b/rust/ruby-rbs/src/gem_version.rs deleted file mode 100644 index 8b398a27dd..0000000000 --- a/rust/ruby-rbs/src/gem_version.rs +++ /dev/null @@ -1,306 +0,0 @@ -use std::cmp::Ordering; -use std::fmt; - -/// A version number compatible with RubyGems' `Gem::Version`. -/// -/// Comparison follows `Gem::Version#<=>`: trailing zero segments are ignored -/// (`1.0 == 1.0.0`) and prerelease versions sort before their release -/// (`1.0.b1 < 1.0`). Numeric segments are kept as normalized digit strings -/// and compared by length then lexicographically, equivalent to RubyGems' -/// arbitrary-precision comparison. -#[derive(Debug, Clone)] -pub struct GemVersion { - segments: Vec, - prerelease: bool, - version: String, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -enum Segment { - /// Decimal digits with leading zeros stripped (`"0"` for zero), matching - /// Ruby's `"007".to_i`. Compared by length, then lexicographically. - Number(String), - Str(String), -} - -impl Segment { - fn is_zero(&self) -> bool { - matches!(self, Segment::Number(digits) if digits == "0") - } -} - -impl GemVersion { - /// Returns `None` when the string is not a valid `Gem::Version` - /// (`Gem::Version.correct?`). - pub fn parse(source: &str) -> Option { - let trimmed = source.trim(); - if !is_correct(trimmed) { - return None; - } - - let version = if trimmed.is_empty() { - "0".to_string() - } else { - trimmed.replace('-', ".pre.") - }; - - let prerelease = version.bytes().any(|b| b.is_ascii_alphabetic()); - let segments = scan_segments(&version); - - Some(GemVersion { - segments, - prerelease, - version, - }) - } - - pub fn is_prerelease(&self) -> bool { - self.prerelease - } - - pub fn release(&self) -> GemVersion { - if !self.prerelease { - return self.clone(); - } - - let end = self - .segments - .iter() - .position(|segment| matches!(segment, Segment::Str(_))) - .unwrap_or(self.segments.len()); - let segments: Vec = self.segments[..end].to_vec(); - let version = segments - .iter() - .map(|segment| match segment { - Segment::Number(digits) => digits.clone(), - Segment::Str(s) => s.clone(), - }) - .collect::>() - .join("."); - - GemVersion { - segments, - prerelease: false, - version, - } - } - - fn canonical_segments(&self) -> Vec<&Segment> { - let split = self - .segments - .iter() - .position(|segment| matches!(segment, Segment::Str(_))) - .unwrap_or(self.segments.len()); - - let mut result = Vec::new(); - for part in [&self.segments[..split], &self.segments[split..]] { - let end = part - .iter() - .rposition(|segment| !segment.is_zero()) - .map_or(0, |index| index + 1); - result.extend(&part[..end]); - } - result - } -} - -impl fmt::Display for GemVersion { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(&self.version) - } -} - -impl PartialEq for GemVersion { - fn eq(&self, other: &Self) -> bool { - self.cmp(other) == Ordering::Equal - } -} - -impl Eq for GemVersion {} - -impl PartialOrd for GemVersion { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for GemVersion { - fn cmp(&self, other: &Self) -> Ordering { - let zero = Segment::Number("0".to_string()); - - let lhs = self.canonical_segments(); - let rhs = other.canonical_segments(); - - for i in 0..lhs.len().max(rhs.len()) { - let l = lhs.get(i).copied().unwrap_or(&zero); - let r = rhs.get(i).copied().unwrap_or(&zero); - - let ordering = match (l, r) { - // Digit strings are leading-zero free: longer means larger. - (Segment::Number(a), Segment::Number(b)) => { - (a.len(), a.as_str()).cmp(&(b.len(), b.as_str())) - } - (Segment::Str(a), Segment::Str(b)) => a.cmp(b), - (Segment::Str(_), Segment::Number(_)) => Ordering::Less, - (Segment::Number(_), Segment::Str(_)) => Ordering::Greater, - }; - if ordering != Ordering::Equal { - return ordering; - } - } - - Ordering::Equal - } -} - -fn is_correct(trimmed: &str) -> bool { - if trimmed.is_empty() { - return true; - } - - let (main, pre) = match trimmed.split_once('-') { - Some((main, pre)) => (main, Some(pre)), - None => (trimmed, None), - }; - - let mut parts = main.split('.'); - let first = parts.next().unwrap_or(""); - if first.is_empty() || !first.bytes().all(|b| b.is_ascii_digit()) { - return false; - } - for part in parts { - if part.is_empty() || !part.bytes().all(|b| b.is_ascii_alphanumeric()) { - return false; - } - } - - if let Some(pre) = pre { - for part in pre.split('.') { - if part.is_empty() || !part.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-') { - return false; - } - } - } - - true -} - -fn scan_segments(version: &str) -> Vec { - let bytes = version.as_bytes(); - let mut segments = Vec::new(); - let mut i = 0; - - while i < bytes.len() { - if bytes[i].is_ascii_digit() { - let start = i; - while i < bytes.len() && bytes[i].is_ascii_digit() { - i += 1; - } - let digits = version[start..i].trim_start_matches('0'); - let digits = if digits.is_empty() { "0" } else { digits }; - segments.push(Segment::Number(digits.to_string())); - } else if bytes[i].is_ascii_alphabetic() { - let start = i; - while i < bytes.len() && bytes[i].is_ascii_alphabetic() { - i += 1; - } - segments.push(Segment::Str(version[start..i].to_string())); - } else { - i += 1; - } - } - - segments -} - -#[cfg(test)] -mod tests { - use super::*; - - fn v(source: &str) -> GemVersion { - GemVersion::parse(source).unwrap() - } - - #[test] - fn parse_rejects_malformed_versions() { - let bad = [ - "junk", - "1.0\n2.0", - "1..2", - "1.2 3.4", - "1.2.3+build", - "2.3422222.222.222222222.22222.ads0as.dasd0.ddd2222.2.qd3e.", - ]; - for source in bad { - assert!( - GemVersion::parse(source).is_none(), - "{source:?} should be invalid" - ); - } - - let good = [ - "", - "0", - "1.0", - "1.0.0-beta", - "5.2.4", - "1.0.0.a.1", - " 1.0 ", - "1.8.2.a10", - ]; - for source in good { - assert!( - GemVersion::parse(source).is_some(), - "{source:?} should be valid" - ); - } - } - - #[test] - fn equality_ignores_trailing_zero_segments() { - assert_eq!(v("1.0"), v("1.0.0")); - assert_eq!(v("1.0"), v("1.0.0.0")); - assert_eq!(v(""), v("0")); - assert_ne!(v("1.0"), v("1.1")); - assert_ne!(v("1.0"), v("1.0.b1")); - } - - #[test] - fn ordering_matches_gem_version() { - assert!(v("1.0") < v("1.1")); - assert!(v("1.0.b1") < v("1.0")); - assert!(v("1.0.a.2") < v("1.0.b1")); - assert!(v("1.0.beta.1") < v("1.0.beta.2")); - assert!(v("1.0.beta.2") < v("1.0.rc.1")); - assert!(v("1.9.3") < v("1.10.0")); - assert!(v("0.9") < v("1.0.a.2")); - assert!(v("1.0.a.2") < v("1.0.0")); - assert_eq!(v("1.0").cmp(&v("1.0.0")), Ordering::Equal); - } - - #[test] - fn prerelease_detection() { - assert!(v("1.2.0.a").is_prerelease()); - assert!(v("1.0.0-beta").is_prerelease()); - assert!(v("2.9.b").is_prerelease()); - assert!(!v("1.2.0").is_prerelease()); - assert!(!v("0").is_prerelease()); - } - - #[test] - fn release_drops_prerelease_segments() { - assert_eq!(v("1.2.0.a").release(), v("1.2.0")); - assert_eq!(v("1.1.rc10").release(), v("1.1")); - assert_eq!(v("1.0.0-beta").release(), v("1.0.0")); - assert_eq!(v("1.9.3").release(), v("1.9.3")); - assert!(!v("1.2.0.a").release().is_prerelease()); - } - - #[test] - fn numeric_segments_compare_with_arbitrary_precision() { - assert!(v("18446744073709551616") > v("18446744073709551615")); - assert!(v("1.99999999999999999999999999") < v("1.100000000000000000000000000")); - assert_eq!(v("1.007"), v("1.7.0")); - assert_eq!(v("1.007.a").release(), v("1.7")); - } -} diff --git a/rust/ruby-rbs/src/lib.rs b/rust/ruby-rbs/src/lib.rs index 985f507f6b..e89428ac6e 100644 --- a/rust/ruby-rbs/src/lib.rs +++ b/rust/ruby-rbs/src/lib.rs @@ -1,12 +1,10 @@ pub mod ast; pub mod buffer; pub mod environment; -pub mod file_finder; -pub mod gem_version; +pub(crate) mod file_finder; pub mod ids; pub mod interner; pub mod interners; pub mod loader; pub mod node; -pub mod repository; pub mod type_name; diff --git a/rust/ruby-rbs/src/loader/manifest.rs b/rust/ruby-rbs/src/loader/manifest.rs deleted file mode 100644 index feaaf45743..0000000000 --- a/rust/ruby-rbs/src/loader/manifest.rs +++ /dev/null @@ -1,214 +0,0 @@ -use std::path::Path; - -use serde::Deserialize; - -use super::LoadError; - -/// Reads exactly what Ruby reads — `manifest['dependencies'][*]['name']` — -/// and nothing else about the notation, so any YAML that `YAML.safe_load` -/// accepts is accepted here too. -/// -/// Anything outside that is a [`LoadError::Manifest`] rather than silently -/// ignored: a gem's `sig/manifest.yaml` is written by a third party, and a -/// dropped dependency would otherwise surface much later as a confusing -/// missing-type error. -pub fn dependencies(dir: &Path) -> Result, LoadError> { - let path = dir.join("manifest.yaml"); - - let content = match std::fs::read_to_string(&path) { - Ok(content) => content, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), - Err(source) => return Err(LoadError::Io { path, source }), - }; - - parse(&content).map_err(|message| LoadError::Manifest { path, message }) -} - -#[derive(Deserialize, Default)] -struct Manifest { - #[serde(default)] - dependencies: Vec, -} - -#[derive(Deserialize)] -struct Dependency { - name: Option, -} - -fn parse(content: &str) -> Result, String> { - if content.trim().is_empty() { - return Ok(Vec::new()); - } - - let manifest: Manifest = serde_yaml::from_str(content).map_err(|error| error.to_string())?; - - manifest - .dependencies - .into_iter() - .enumerate() - .map(|(index, dependency)| { - dependency - .name - .filter(|name| !name.is_empty()) - .ok_or_else(|| format!("dependencies[{index}] has no `name` key")) - }) - .collect() -} - -#[cfg(test)] -mod tests { - use super::*; - use std::fs; - - fn manifest(content: &str) -> tempfile::TempDir { - let dir = tempfile::tempdir().unwrap(); - fs::write(dir.path().join("manifest.yaml"), content).unwrap(); - dir - } - - #[test] - fn reads_dependency_names() { - let dir = manifest("dependencies:\n - name: bigdecimal\n - name: singleton\n"); - assert_eq!( - dependencies(dir.path()).unwrap(), - vec!["bigdecimal", "singleton"] - ); - } - - #[test] - fn missing_manifest_means_no_dependencies() { - let dir = tempfile::tempdir().unwrap(); - assert_eq!(dependencies(dir.path()).unwrap(), Vec::::new()); - } - - #[test] - fn comments_blank_lines_and_quotes_are_accepted() { - let dir = manifest("# a comment\n\ndependencies:\n - name: 'uri'\n - name: \"csv\"\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri", "csv"]); - } - - #[test] - fn unrelated_top_level_keys_are_ignored() { - let dir = manifest("type: stdlib\ndependencies:\n - name: uri\nother:\n - name: nope\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); - } - - #[test] - fn empty_flow_sequence_means_no_dependencies() { - let dir = manifest("dependencies: []\n"); - assert_eq!(dependencies(dir.path()).unwrap(), Vec::::new()); - } - - #[test] - fn non_sequence_dependencies_is_an_error() { - let dir = manifest("dependencies: 42\n"); - assert!(matches!( - dependencies(dir.path()), - Err(LoadError::Manifest { .. }) - )); - } - - #[test] - fn flow_style_dependencies_are_accepted() { - let dir = manifest("dependencies: [{name: uri}, {name: 'csv'}]\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri", "csv"]); - } - - #[test] - fn multi_line_flow_style_is_accepted() { - let dir = manifest("dependencies: [\n {name: uri}\n]\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); - } - - #[test] - fn unexpected_entry_shape_is_an_error() { - let dir = manifest("dependencies:\n - uri\n"); - assert!(matches!( - dependencies(dir.path()), - Err(LoadError::Manifest { .. }) - )); - } - - #[test] - fn sequence_at_the_keys_own_indent_is_accepted() { - let dir = manifest("dependencies:\n- name: uri\n- name: csv\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri", "csv"]); - } - - #[test] - fn document_and_directive_markers_are_accepted() { - let dir = manifest("%YAML 1.1\n---\ndependencies:\n - name: uri\n...\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); - } - - #[test] - fn entry_keys_other_than_name_are_ignored() { - let dir = - manifest("dependencies:\n - name: uri\n kind: gem\n - kind: gem\n name: csv\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri", "csv"]); - } - - #[test] - fn an_entrys_mapping_may_start_on_the_next_line() { - let dir = manifest("dependencies:\n -\n name: uri\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); - } - - #[test] - fn trailing_comments_are_stripped() { - let dir = manifest("dependencies:\n - name: uri # needed by Foo\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); - - let dir = manifest("dependencies: [{name: uri}] # needed by Foo\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); - } - - #[test] - fn a_hash_inside_quotes_is_not_a_comment() { - let dir = manifest("dependencies:\n - name: \"x # y\"\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["x # y"]); - } - - #[test] - fn an_entry_without_a_name_is_an_error() { - // Ruby yields a nil name here, which fails later and confusingly. - let dir = manifest("dependencies:\n - kind: gem\n"); - assert!(matches!( - dependencies(dir.path()), - Err(LoadError::Manifest { .. }) - )); - } - - #[test] - fn a_mapping_instead_of_a_sequence_is_an_error() { - let dir = manifest("dependencies:\n foo: bar\n"); - assert!(matches!( - dependencies(dir.path()), - Err(LoadError::Manifest { .. }) - )); - } - - #[test] - fn a_trailing_entry_is_not_lost() { - let dir = manifest("dependencies:\n - name: uri"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); - } - - #[test] - fn a_nested_block_sequence_inside_an_entry_is_accepted() { - let dir = manifest("dependencies:\n - name: uri\n platforms:\n - mri\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); - } - - #[test] - fn a_flow_mapping_as_a_block_sequence_item_is_accepted() { - let dir = manifest("dependencies:\n - {name: uri}\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); - } - - #[test] - fn a_column_zero_sequence_under_an_unrelated_key_is_accepted() { - let dir = manifest("authors:\n- John Doe\ndependencies:\n - name: uri\n"); - assert_eq!(dependencies(dir.path()).unwrap(), vec!["uri"]); - } -} diff --git a/rust/ruby-rbs/src/repository.rs b/rust/ruby-rbs/src/repository.rs deleted file mode 100644 index 1b0a327ea9..0000000000 --- a/rust/ruby-rbs/src/repository.rs +++ /dev/null @@ -1,237 +0,0 @@ -use std::collections::{BTreeMap, HashMap}; -use std::io; -use std::path::{Path, PathBuf}; - -use crate::gem_version::GemVersion; - -/// An index of versioned RBS directories laid out as `{dir}/{gem}/{version}/`, -/// mirroring `RBS::Repository`. -/// -/// Unlike the Ruby implementation, no default stdlib directory is registered; -/// callers add repository roots explicitly via [`Repository::add`]. -#[derive(Debug, Default)] -pub struct Repository { - dirs: Vec, - gems: HashMap, -} - -#[derive(Debug, Default)] -struct GemRbs { - /// Keyed by the raw version directory name: Ruby's versions hash uses - /// `Gem::Version` string equality, keeping `1.0` and `1.0.0` distinct. - versions: BTreeMap, -} - -#[derive(Debug)] -struct VersionPath { - version: GemVersion, - path: PathBuf, -} - -impl Repository { - pub fn new() -> Self { - Self::default() - } - - pub fn dirs(&self) -> &[PathBuf] { - &self.dirs - } - - pub fn add(&mut self, dir: &Path) -> io::Result<()> { - self.dirs.push(dir.to_path_buf()); - - for gem_entry in std::fs::read_dir(dir)? { - let gem_entry = gem_entry?; - if !gem_entry.path().is_dir() { - continue; - } - - let gem_name = gem_entry.file_name().to_string_lossy().into_owned(); - let gem = self.gems.entry(gem_name).or_default(); - - for version_entry in std::fs::read_dir(gem_entry.path())? { - let version_entry = version_entry?; - let name = version_entry.file_name().to_string_lossy().into_owned(); - let Some(version) = GemVersion::parse(&name) else { - continue; - }; - if version.is_prerelease() { - continue; - } - gem.versions.insert( - name, - VersionPath { - version, - path: version_entry.path(), - }, - ); - } - } - - Ok(()) - } - - /// Returns the signature directory for `gem`, choosing the newest version - /// `<= version`, or, when `version` is `None`, the version whose *name - /// string* sorts last — Ruby's `latest_version` sorts by the version - /// string, so `1.2` beats `1.11`. - /// - /// Takes an already-parsed `GemVersion` rather than a raw string: an - /// invalid version string is a caller error (Ruby raises `ArgumentError` - /// resolving it), not a "not found" result, so it must be rejected - /// before it can reach `lookup` at all. - pub fn lookup(&self, gem: &str, version: Option<&GemVersion>) -> Option<&Path> { - let gem = self.gems.get(gem)?; - if gem.versions.is_empty() { - return None; - } - - match version { - Some(requested) => find_best_version(&gem.versions, &requested.release()), - None => gem - .versions - .last_key_value() - .map(|(_, vp)| vp.path.as_path()), - } - } -} - -/// Returns the semantically newest version that is `<= requested`, or the -/// semantically oldest one when every candidate is newer -/// (`RBS::Repository.find_best_version` compatible). Versions that compare -/// equal (`1.0` vs `1.0.0`) tie-break deterministically on the string-wise -/// last key; Ruby's unstable sort leaves that case unspecified. -fn find_best_version<'a>( - versions: &'a BTreeMap, - requested: &GemVersion, -) -> Option<&'a Path> { - versions - .values() - .filter(|vp| vp.version <= *requested) - .max_by(|a, b| a.version.cmp(&b.version)) - .or_else(|| versions.values().min_by(|a, b| a.version.cmp(&b.version))) - .map(|vp| vp.path.as_path()) -} - -#[cfg(test)] -mod tests { - use super::*; - use std::fs; - - fn fixture(entries: &[&str]) -> tempfile::TempDir { - let dir = tempfile::tempdir().unwrap(); - for entry in entries { - fs::create_dir_all(dir.path().join(entry)).unwrap(); - } - dir - } - - fn lookup_name(repository: &Repository, gem: &str, version: Option<&str>) -> Option { - let version = version.map(|v| GemVersion::parse(v).unwrap()); - repository - .lookup(gem, version.as_ref()) - .map(|path| path.file_name().unwrap().to_string_lossy().into_owned()) - } - - #[test] - fn lookup_returns_latest_version_without_request() { - let dir = fixture(&["uri/0", "uri/1.0", "csv/2.0", "csv/3.0.pre"]); - let mut repository = Repository::new(); - repository.add(dir.path()).unwrap(); - - assert_eq!( - lookup_name(&repository, "uri", None), - Some("1.0".to_string()) - ); - assert_eq!( - lookup_name(&repository, "csv", None), - Some("2.0".to_string()) - ); - assert_eq!(repository.lookup("no_such_gem", None), None); - } - - #[test] - fn lookup_finds_best_version() { - let dir = fixture(&["uri/0", "uri/1.0", "uri/2.0"]); - let mut repository = Repository::new(); - repository.add(dir.path()).unwrap(); - - assert_eq!( - lookup_name(&repository, "uri", Some("1.5")), - Some("1.0".to_string()) - ); - assert_eq!( - lookup_name(&repository, "uri", Some("1.0")), - Some("1.0".to_string()) - ); - assert_eq!( - lookup_name(&repository, "uri", Some("3.0")), - Some("2.0".to_string()) - ); - assert_eq!( - lookup_name(&repository, "uri", Some("0.0.1")), - Some("0".to_string()) - ); - assert_eq!( - lookup_name(&repository, "uri", Some("1.0.pre")), - Some("1.0".to_string()) - ); - } - - #[test] - fn non_version_directories_are_ignored() { - let dir = fixture(&["uri/not_a_version", "uri/1.0"]); - let mut repository = Repository::new(); - repository.add(dir.path()).unwrap(); - - assert_eq!( - lookup_name(&repository, "uri", None), - Some("1.0".to_string()) - ); - } - - #[test] - fn later_add_overwrites_same_version() { - let dir1 = fixture(&["uri/1.0"]); - let dir2 = fixture(&["uri/1.0"]); - let mut repository = Repository::new(); - repository.add(dir1.path()).unwrap(); - repository.add(dir2.path()).unwrap(); - - let path = repository.lookup("uri", None).unwrap(); - assert!(path.starts_with(dir2.path())); - assert_eq!(repository.dirs().len(), 2); - } - - #[test] - fn lookup_without_version_uses_string_order() { - let dir = fixture(&["uri/1.2", "uri/1.11"]); - let mut repository = Repository::new(); - repository.add(dir.path()).unwrap(); - - assert_eq!( - lookup_name(&repository, "uri", None), - Some("1.2".to_string()) - ); - assert_eq!( - lookup_name(&repository, "uri", Some("99")), - Some("1.11".to_string()) - ); - } - - #[test] - fn equal_versions_with_different_spellings_stay_distinct() { - let dir = fixture(&["uri/1.0", "uri/1.0.0"]); - let mut repository = Repository::new(); - repository.add(dir.path()).unwrap(); - - assert_eq!( - lookup_name(&repository, "uri", Some("1.0")), - Some("1.0.0".to_string()) - ); - assert_eq!( - lookup_name(&repository, "uri", None), - Some("1.0.0".to_string()) - ); - } -} From 0b70e05f60f5b9999ea8a8207a8fc8660d7629d3 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Thu, 13 Aug 2026 22:59:42 +0900 Subject: [PATCH 16/24] Revert "Add Environment::add_rbs and route EnvironmentLoader through it" This reverts commit fcf2f0f2f202840d9891697f044fe2f7e96c33b3. --- rust/ruby-rbs/src/environment/mod.rs | 79 +++++----------------------- rust/ruby-rbs/src/loader/mod.rs | 57 ++++++++++++++++++-- 2 files changed, 66 insertions(+), 70 deletions(-) diff --git a/rust/ruby-rbs/src/environment/mod.rs b/rust/ruby-rbs/src/environment/mod.rs index 3167c1f7d9..aa73d7d9f4 100644 --- a/rust/ruby-rbs/src/environment/mod.rs +++ b/rust/ruby-rbs/src/environment/mod.rs @@ -2,13 +2,8 @@ pub mod source; pub use source::{Source, SourceKind}; -use std::path::{Path, PathBuf}; - -use crate::ast::AstConverter; -use crate::buffer::Buffer; use crate::interners::Interners; use crate::loader::{EnvironmentLoader, LoadError}; -use crate::node; /// Owning the interners here gives a single `Environment` value the same /// role as the Ruby implementation's global name pool: names interned while @@ -34,56 +29,12 @@ impl Environment { &self.sources } - /// Parses `content` with this environment's own [`Interners`] and adds it - /// as a source, without going through an [`EnvironmentLoader`] — the - /// entry point for editor / in-memory callers (`RBS::Environment#add_source` - /// equivalent, e.g. Steep). Parsing always uses this environment's - /// `Interners`, so a `Source` added this way can never carry ids from a - /// different environment. - pub fn add_rbs( - &mut self, - path: PathBuf, - content: String, - kind: SourceKind, - ) -> Result<(), LoadError> { - let signature = node::parse(&content).map_err(|message| LoadError::Parse { - path: path.clone(), - message, - })?; - - let mut converter = - AstConverter::new(&mut self.interners.strings, &mut self.interners.type_names); - let directives = signature - .directives() - .iter() - .map(|node| converter.convert_directive(&node)) - .collect(); - let declarations = signature - .declarations() - .iter() - .map(|node| converter.convert_declaration(&node)) - .collect(); - // SignatureNode borrows `content` and has a Drop impl; drop it - // explicitly before moving `content` into the Buffer. - drop(signature); - - self.sources.push(Source { - buffer: Buffer::new(path, content), - directives, - declarations, - kind, - }); - Ok(()) + pub(crate) fn interners_mut(&mut self) -> &mut Interners { + &mut self.interners } - /// Reads `path` and adds it as a source; a thin wrapper around - /// [`Environment::add_rbs`]. - pub fn add_rbs_file(&mut self, path: &Path, kind: SourceKind) -> Result<(), LoadError> { - let content = std::fs::read_to_string(path).map_err(|source| LoadError::Io { - path: path.to_path_buf(), - source, - })?; - self.add_rbs(path.to_path_buf(), content, kind) + pub(crate) fn add_source(&mut self, source: Source) { + self.sources.push(source); } pub fn from_loader(loader: &EnvironmentLoader) -> Result { @@ -102,27 +53,21 @@ impl Default for Environment { #[cfg(test)] mod tests { use super::*; - use crate::ast::Declaration; #[test] fn environment_owns_interners() { let mut env = Environment::new(); - env.add_rbs( - PathBuf::from("test.rbs"), - "class Foo\nend\n".to_string(), - SourceKind::Dir { - path: PathBuf::from("."), - }, - ) - .unwrap(); - let [Declaration::Class(class)] = env.sources()[0].declarations.as_slice() else { - panic!("expected one class declaration"); - }; + let interners = env.interners_mut(); + let symbol = interners.strings.intern("Foo"); + let root = interners.type_names.absolute_root(); + let name = interners.type_names.append(root, symbol); + let interners = env.interners(); assert_eq!( - interners.type_names.display(class.name, &interners.strings), - "Foo" + interners.type_names.display(name, &interners.strings), + "::Foo" ); + assert!(env.sources().is_empty()); } } diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index 4d228934f3..aaf2a012cd 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -1,10 +1,14 @@ use std::collections::HashSet; use std::fmt; use std::io; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; -use crate::environment::{Environment, SourceKind}; +use crate::ast::AstConverter; +use crate::buffer::Buffer; +use crate::environment::{Environment, Source, SourceKind}; use crate::file_finder; +use crate::interners::Interners; +use crate::node; #[derive(Debug)] #[non_exhaustive] @@ -98,7 +102,8 @@ impl EnvironmentLoader { if !seen_files.insert(path.clone()) { continue; } - env.add_rbs_file(&path, kind.clone())?; + let source = parse_one(&path, &kind, env.interners_mut())?; + env.add_source(source); loaded.push(LoadedFile { path, kind: kind.clone(), @@ -135,3 +140,49 @@ impl EnvironmentLoader { result } } + +/// Deliberately a free function taking only the interners, so the load loop +/// can later be parallelised by handing each worker its own [`Interners`]. +/// The parser's `SignatureNode` holds raw pointers and is not `Send`, so it +/// must not escape this function — only the owned `Source` does. +/// +/// Crate-private: a caller outside the crate has no way to merge its +/// worker-local [`Interners`] into the environment, so the `Source` it +/// produced would carry ids that environment cannot resolve. +pub(crate) fn parse_one( + path: &Path, + kind: &SourceKind, + interners: &mut Interners, +) -> Result { + let content = std::fs::read_to_string(path).map_err(|source| LoadError::Io { + path: path.to_path_buf(), + source, + })?; + + let signature = node::parse(&content).map_err(|message| LoadError::Parse { + path: path.to_path_buf(), + message, + })?; + + let mut converter = AstConverter::new(&mut interners.strings, &mut interners.type_names); + let directives = signature + .directives() + .iter() + .map(|node| converter.convert_directive(&node)) + .collect(); + let declarations = signature + .declarations() + .iter() + .map(|node| converter.convert_declaration(&node)) + .collect(); + // SignatureNode borrows `content` and has a Drop impl; drop it + // explicitly before moving `content` into the Buffer. + drop(signature); + + Ok(Source { + buffer: Buffer::new(path.to_path_buf(), content), + directives, + declarations, + kind: kind.clone(), + }) +} From 2b3df5a413e0f1022c7a7ece6ed8bc6bfde70a4f Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Thu, 13 Aug 2026 23:24:06 +0900 Subject: [PATCH 17/24] Remove Buffer; store path instead of source text on Source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ruby's RBS::Buffer exists because RBS::Location is lazy: it holds a reference to the buffer and slices out text on demand for #source and pos_to_loc. The Rust port mirrored that shape, but the architecture underneath it isn't lazy — AstConverter eagerly interns or copies every semantically relevant string (names, comments, annotations, literals) at conversion time, and LocationRange is a plain 4x u32 offset Copy type with no reference back into the source. There is no deferred slicing step left that would ever need the buffer, so nothing in the crate calls Buffer::content() after parse_one() returns — it's a write-only store. Keeping it anyway means holding the full text of every loaded file for the Environment's lifetime for no benefit: ~4.3MB across core + stdlib alone, more with gems, growing for as long as the Environment lives. Source.buffer: Buffer is replaced with Source.path: PathBuf, keeping just enough to identify the file for error reporting. parse_one() no longer moves `content` into a Buffer, so the explicit drop(signature) that used to be needed to end its borrow first is gone too. Co-Authored-By: Claude Sonnet 5 --- rust/ruby-rbs/src/buffer.rs | 22 ---------------------- rust/ruby-rbs/src/environment/source.rs | 3 +-- rust/ruby-rbs/src/lib.rs | 1 - rust/ruby-rbs/src/loader/mod.rs | 6 +----- rust/ruby-rbs/tests/loader.rs | 2 +- 5 files changed, 3 insertions(+), 31 deletions(-) delete mode 100644 rust/ruby-rbs/src/buffer.rs diff --git a/rust/ruby-rbs/src/buffer.rs b/rust/ruby-rbs/src/buffer.rs deleted file mode 100644 index 527ff1cfb3..0000000000 --- a/rust/ruby-rbs/src/buffer.rs +++ /dev/null @@ -1,22 +0,0 @@ -use std::path::{Path, PathBuf}; - -/// A signature file's content, mirroring `RBS::Buffer`. -#[derive(Debug, Clone)] -pub struct Buffer { - name: PathBuf, - content: String, -} - -impl Buffer { - pub fn new(name: PathBuf, content: String) -> Self { - Self { name, content } - } - - pub fn name(&self) -> &Path { - &self.name - } - - pub fn content(&self) -> &str { - &self.content - } -} diff --git a/rust/ruby-rbs/src/environment/source.rs b/rust/ruby-rbs/src/environment/source.rs index 237d2cbe89..7a98d7b7da 100644 --- a/rust/ruby-rbs/src/environment/source.rs +++ b/rust/ruby-rbs/src/environment/source.rs @@ -1,7 +1,6 @@ use std::path::PathBuf; use crate::ast::{Declaration, Directive}; -use crate::buffer::Buffer; /// Where a loaded signature file came from, corresponding to the `source` /// values yielded by `RBS::EnvironmentLoader#each_dir`. @@ -31,7 +30,7 @@ impl SourceKind { /// `RBS::Source::RBS` equivalent. #[derive(Debug)] pub struct Source { - pub buffer: Buffer, + pub path: PathBuf, pub directives: Vec, pub declarations: Vec, pub kind: SourceKind, diff --git a/rust/ruby-rbs/src/lib.rs b/rust/ruby-rbs/src/lib.rs index e89428ac6e..1e4cb82f49 100644 --- a/rust/ruby-rbs/src/lib.rs +++ b/rust/ruby-rbs/src/lib.rs @@ -1,5 +1,4 @@ pub mod ast; -pub mod buffer; pub mod environment; pub(crate) mod file_finder; pub mod ids; diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index aaf2a012cd..c65364de1b 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -4,7 +4,6 @@ use std::io; use std::path::{Path, PathBuf}; use crate::ast::AstConverter; -use crate::buffer::Buffer; use crate::environment::{Environment, Source, SourceKind}; use crate::file_finder; use crate::interners::Interners; @@ -175,12 +174,9 @@ pub(crate) fn parse_one( .iter() .map(|node| converter.convert_declaration(&node)) .collect(); - // SignatureNode borrows `content` and has a Drop impl; drop it - // explicitly before moving `content` into the Buffer. - drop(signature); Ok(Source { - buffer: Buffer::new(path.to_path_buf(), content), + path: path.to_path_buf(), directives, declarations, kind: kind.clone(), diff --git a/rust/ruby-rbs/tests/loader.rs b/rust/ruby-rbs/tests/loader.rs index 4ebdfbc8da..33304a84a8 100644 --- a/rust/ruby-rbs/tests/loader.rs +++ b/rust/ruby-rbs/tests/loader.rs @@ -120,7 +120,7 @@ fn loaded_sources_carry_converted_declarations_and_directives() { let env = Environment::from_loader(&loader).unwrap(); let source = &env.sources()[0]; - assert!(source.buffer.name().ends_with("person.rbs")); + assert!(source.path.ends_with("person.rbs")); assert!(matches!(source.directives.as_slice(), [Directive::Use(_)])); let [Declaration::Class(class)] = source.declarations.as_slice() else { From eddc09afea15b5b32cc2b4a4e974d963d5659ea8 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Fri, 14 Aug 2026 09:20:44 +0900 Subject: [PATCH 18/24] Remove doc comments that restate the code Co-Authored-By: Claude Fable 5 --- rust/ruby-rbs/src/environment/source.rs | 7 ++----- rust/ruby-rbs/src/file_finder.rs | 3 +-- rust/ruby-rbs/src/interners.rs | 1 - rust/ruby-rbs/src/loader/mod.rs | 14 +------------- 4 files changed, 4 insertions(+), 21 deletions(-) diff --git a/rust/ruby-rbs/src/environment/source.rs b/rust/ruby-rbs/src/environment/source.rs index 7a98d7b7da..7e83cc9979 100644 --- a/rust/ruby-rbs/src/environment/source.rs +++ b/rust/ruby-rbs/src/environment/source.rs @@ -2,8 +2,8 @@ use std::path::PathBuf; use crate::ast::{Declaration, Directive}; -/// Where a loaded signature file came from, corresponding to the `source` -/// values yielded by `RBS::EnvironmentLoader#each_dir`. +/// Corresponds to the `source` values yielded by +/// `RBS::EnvironmentLoader#each_dir`. #[derive(Debug, Clone, PartialEq, Eq)] pub enum SourceKind { Core, @@ -20,14 +20,11 @@ pub enum SourceKind { } impl SourceKind { - /// Whether `_`-prefixed subdirectories are skipped while scanning. - /// Only a user-specified [`SourceKind::Dir`] does not skip them. pub fn skips_hidden(&self) -> bool { !matches!(self, SourceKind::Dir { .. }) } } -/// `RBS::Source::RBS` equivalent. #[derive(Debug)] pub struct Source { pub path: PathBuf, diff --git a/rust/ruby-rbs/src/file_finder.rs b/rust/ruby-rbs/src/file_finder.rs index 3a64457fa2..dac7dad5ca 100644 --- a/rust/ruby-rbs/src/file_finder.rs +++ b/rust/ruby-rbs/src/file_finder.rs @@ -1,8 +1,7 @@ use std::io; use std::path::{Path, PathBuf}; -/// Enumerates `.rbs` entries under `path` in path-sorted order, mirroring -/// `RBS::FileFinder.each_file` and its `Dir.glob` semantics. +/// Mirrors `RBS::FileFinder.each_file` and its `Dir.glob` semantics. /// /// Divergences: the sort compares the platform separator rather than /// `/`-joined strings, and only `PermissionDenied`/`NotFound` are skipped diff --git a/rust/ruby-rbs/src/interners.rs b/rust/ruby-rbs/src/interners.rs index a290c5ea1f..f8a1860735 100644 --- a/rust/ruby-rbs/src/interners.rs +++ b/rust/ruby-rbs/src/interners.rs @@ -1,7 +1,6 @@ use crate::interner::StringInterner; use crate::type_name::TypeNameInterner; -/// The pair of interners that an owned AST's ids refer to. #[derive(Default)] pub struct Interners { pub strings: StringInterner, diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index c65364de1b..902f6472b7 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -44,9 +44,7 @@ pub struct LoadedFile { pub kind: SourceKind, } -/// Enumerates core, library, and explicit signature directories, parses -/// every `.rbs` file exactly once, and feeds the results into an -/// [`Environment`], mirroring `RBS::EnvironmentLoader`. +/// Mirrors `RBS::EnvironmentLoader`. /// /// Unlike the Ruby implementation, this does not resolve gem names or /// versions to directories, and does not expand `manifest.yaml` @@ -61,7 +59,6 @@ pub struct EnvironmentLoader { } impl EnvironmentLoader { - /// `core_root` is `None` to skip core. pub fn new(core_root: Option) -> Self { EnvironmentLoader { core_root, @@ -70,14 +67,11 @@ impl EnvironmentLoader { } } - /// Adds a library by name and its already-resolved signature directory. pub fn add_library(mut self, name: &str, path: PathBuf) -> Self { self.libs.push((name.to_string(), path)); self } - /// Adds an explicit signature directory. Unlike libraries, `_`-prefixed - /// subdirectories are not skipped. pub fn add_dir(mut self, path: PathBuf) -> Self { self.dirs.push(path); self @@ -113,8 +107,6 @@ impl EnvironmentLoader { Ok(loaded) } - /// Groups directories core → libs → dirs, matching - /// `RBS::EnvironmentLoader#each_dir`'s enumeration order. fn each_dir(&self) -> Vec<(SourceKind, PathBuf)> { let mut result = Vec::new(); @@ -144,10 +136,6 @@ impl EnvironmentLoader { /// can later be parallelised by handing each worker its own [`Interners`]. /// The parser's `SignatureNode` holds raw pointers and is not `Send`, so it /// must not escape this function — only the owned `Source` does. -/// -/// Crate-private: a caller outside the crate has no way to merge its -/// worker-local [`Interners`] into the environment, so the `Source` it -/// produced would carry ids that environment cannot resolve. pub(crate) fn parse_one( path: &Path, kind: &SourceKind, From a7affdeed498031148a411c5a2baf69384616346 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Fri, 14 Aug 2026 10:10:26 +0900 Subject: [PATCH 19/24] Simplify file_finder's error skipping and name handling The "skip this entry" match was written out three times, differing only in whether it returned or continued. Collapse it into one `skippable` adapter returning `io::Result>`, so the policy for which IO errors Ruby swallows lives in one place. The entry name was also derived twice per entry: from `entry.file_name()` for the leading-`.` test, then again from `path.file_name()` for the leading-`_` test. Derive it once from `path`, which drops the `OsString` that `DirEntry::file_name` allocates for every entry walked, matched or not. Co-Authored-By: Claude Opus 5 (1M context) --- rust/ruby-rbs/src/file_finder.rs | 47 +++++++++++++++++--------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/rust/ruby-rbs/src/file_finder.rs b/rust/ruby-rbs/src/file_finder.rs index dac7dad5ca..a539459023 100644 --- a/rust/ruby-rbs/src/file_finder.rs +++ b/rust/ruby-rbs/src/file_finder.rs @@ -19,29 +19,37 @@ pub fn each_file(path: &Path, skip_hidden: bool) -> io::Result> { Ok(files) } -fn is_skippable(error: &io::Error) -> bool { - matches!( - error.kind(), - io::ErrorKind::PermissionDenied | io::ErrorKind::NotFound - ) +/// `None` for the failures Ruby swallows by letting `Dir.glob` skip the entry. +fn skippable(result: io::Result) -> io::Result> { + match result { + Ok(value) => Ok(Some(value)), + Err(error) + if matches!( + error.kind(), + io::ErrorKind::PermissionDenied | io::ErrorKind::NotFound + ) => + { + Ok(None) + } + Err(error) => Err(error), + } } fn collect(dir: &Path, skip_hidden: bool, files: &mut Vec) -> io::Result<()> { - let entries = match std::fs::read_dir(dir) { - Ok(entries) => entries, - Err(error) if is_skippable(&error) => return Ok(()), - Err(error) => return Err(error), + let Some(entries) = skippable(std::fs::read_dir(dir))? else { + return Ok(()); }; for entry in entries { - let entry = match entry { - Ok(entry) => entry, - Err(error) if is_skippable(&error) => continue, - Err(error) => return Err(error), + let Some(entry) = skippable(entry)? else { + continue; }; let path = entry.path(); + let Some(name) = path.file_name().map(|name| name.to_string_lossy()) else { + continue; + }; - if entry.file_name().to_string_lossy().starts_with('.') { + if name.starts_with('.') { continue; } @@ -50,16 +58,11 @@ fn collect(dir: &Path, skip_hidden: bool, files: &mut Vec) -> io::Resul } // `file_type()` does not follow symlinks, which skips symlinked dirs. - let file_type = match entry.file_type() { - Ok(file_type) => file_type, - Err(error) if is_skippable(&error) => continue, - Err(error) => return Err(error), + let Some(file_type) = skippable(entry.file_type())? else { + continue; }; if file_type.is_dir() { - let hidden = path - .file_name() - .is_some_and(|name| name.to_string_lossy().starts_with('_')); - if skip_hidden && hidden { + if skip_hidden && name.starts_with('_') { continue; } collect(&path, skip_hidden, files)?; From b0a2b06f321deefadfa9788dfa07cec01d6fa123 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Fri, 14 Aug 2026 10:10:36 +0900 Subject: [PATCH 20/24] Return a borrowing iterator from EnvironmentLoader::each_dir each_dir built a Vec of owned pairs before the load loop started, cloning every registered path twice: once into the SourceKind and once as the directory to walk. The kinds still have to be built, but the directory to walk can be borrowed from the loader, which outlives the load call. Co-Authored-By: Claude Opus 5 (1M context) --- rust/ruby-rbs/src/loader/mod.rs | 44 +++++++++++++++------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index 902f6472b7..cc01483053 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -84,9 +84,9 @@ impl EnvironmentLoader { let mut seen_files: HashSet = HashSet::new(); for (kind, dir) in self.each_dir() { - let files = file_finder::each_file(&dir, kind.skips_hidden()).map_err(|source| { + let files = file_finder::each_file(dir, kind.skips_hidden()).map_err(|source| { LoadError::Io { - path: dir.clone(), + path: dir.to_path_buf(), source, } })?; @@ -107,28 +107,24 @@ impl EnvironmentLoader { Ok(loaded) } - fn each_dir(&self) -> Vec<(SourceKind, PathBuf)> { - let mut result = Vec::new(); - - if let Some(core) = &self.core_root { - result.push((SourceKind::Core, core.clone())); - } - - for (name, path) in &self.libs { - result.push(( - SourceKind::Library { - name: name.clone(), - path: path.clone(), - }, - path.clone(), - )); - } - - for dir in &self.dirs { - result.push((SourceKind::Dir { path: dir.clone() }, dir.clone())); - } - - result + fn each_dir(&self) -> impl Iterator { + let core = self + .core_root + .iter() + .map(|path| (SourceKind::Core, path.as_path())); + let libs = self.libs.iter().map(|(name, path)| { + let kind = SourceKind::Library { + name: name.clone(), + path: path.clone(), + }; + (kind, path.as_path()) + }); + let dirs = self + .dirs + .iter() + .map(|path| (SourceKind::Dir { path: path.clone() }, path.as_path())); + + core.chain(libs).chain(dirs) } } From c4aaa54a9a8dccd0dbc106509da886eaca0ab19e Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Fri, 14 Aug 2026 10:10:43 +0900 Subject: [PATCH 21/24] Remove unused Interners::merge Nothing calls it. It forwards to StringInterner::merge and TypeNameInterner::merge, which are already covered by tests next to their own definitions, so its test asserted nothing new either. The parallel load loop parse_one is shaped for would need it back, but re-adding a two-line delegation then is cheaper than carrying an untested-by-use API until that lands. Co-Authored-By: Claude Opus 5 (1M context) --- rust/ruby-rbs/src/interners.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/rust/ruby-rbs/src/interners.rs b/rust/ruby-rbs/src/interners.rs index f8a1860735..dc69b46b06 100644 --- a/rust/ruby-rbs/src/interners.rs +++ b/rust/ruby-rbs/src/interners.rs @@ -12,27 +12,4 @@ impl Interners { pub fn new() -> Self { Self::default() } - - pub fn merge(&mut self, other: Interners) { - self.strings.merge(other.strings); - self.type_names.merge(other.type_names); - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn merge_makes_worker_local_ids_resolvable() { - let mut local = Interners::new(); - let foo = local.strings.intern("Foo"); - let root = local.type_names.absolute_root(); - let name = local.type_names.append(root, foo); - - let mut global = Interners::new(); - global.merge(local); - - assert_eq!(global.type_names.display(name, &global.strings), "::Foo"); - } } From 953df39242f104ea2445564750a3e835f1e37b4a Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Fri, 14 Aug 2026 10:10:49 +0900 Subject: [PATCH 22/24] Derive Default for Environment Default delegated to a new() that listed the fields by hand, so adding a field meant remembering to update new(). Reverse the delegation and derive Default, leaving the generated code as the only place that knows the fields. This is also the shape StringInterner and Interners already use. TypeNameInterner keeps its hand-written Default because it has real initialisation to do. Co-Authored-By: Claude Opus 5 (1M context) --- rust/ruby-rbs/src/environment/mod.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/rust/ruby-rbs/src/environment/mod.rs b/rust/ruby-rbs/src/environment/mod.rs index aa73d7d9f4..fa2cedf103 100644 --- a/rust/ruby-rbs/src/environment/mod.rs +++ b/rust/ruby-rbs/src/environment/mod.rs @@ -8,17 +8,16 @@ use crate::loader::{EnvironmentLoader, LoadError}; /// Owning the interners here gives a single `Environment` value the same /// role as the Ruby implementation's global name pool: names interned while /// loading stay resolvable and displayable for the environment's lifetime. +#[derive(Default)] pub struct Environment { interners: Interners, sources: Vec, } impl Environment { + #[must_use] pub fn new() -> Self { - Environment { - interners: Interners::new(), - sources: Vec::new(), - } + Self::default() } pub fn interners(&self) -> &Interners { @@ -44,12 +43,6 @@ impl Environment { } } -impl Default for Environment { - fn default() -> Self { - Self::new() - } -} - #[cfg(test)] mod tests { use super::*; From 0b5c931ba0ffb6bd2eead918b3e4d6b27e9f8d00 Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Fri, 14 Aug 2026 10:10:56 +0900 Subject: [PATCH 23/24] Narrow SourceKind::skips_hidden to pub(crate) The bool it returns is only accepted by file_finder::each_file, which is pub(crate), so a caller outside the crate could obtain the answer but had nowhere to pass it. The variants themselves stay public, so no information is withdrawn from consumers of Source.kind. Also record on Library why it carries the resolved path, since dropping version for path was a deliberate choice and the field reads like duplication next to each_dir without it. Co-Authored-By: Claude Opus 5 (1M context) --- rust/ruby-rbs/src/environment/source.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rust/ruby-rbs/src/environment/source.rs b/rust/ruby-rbs/src/environment/source.rs index 7e83cc9979..0ae87760c5 100644 --- a/rust/ruby-rbs/src/environment/source.rs +++ b/rust/ruby-rbs/src/environment/source.rs @@ -9,7 +9,8 @@ pub enum SourceKind { Core, /// `path` is the library's version directory, already resolved by the /// caller (Ruby's `Repository#lookup` / `gem_sig_path`), holding the - /// library's RBS files directly. + /// library's RBS files directly. Version alone could not recover where a + /// library's signatures came from; the path can. Library { name: String, path: PathBuf, @@ -20,7 +21,7 @@ pub enum SourceKind { } impl SourceKind { - pub fn skips_hidden(&self) -> bool { + pub(crate) fn skips_hidden(&self) -> bool { !matches!(self, SourceKind::Dir { .. }) } } From 86f3acf05b00f37ff037ddcac06ce39d3817321e Mon Sep 17 00:00:00 2001 From: dak2 <32436625+dak2@users.noreply.github.com> Date: Fri, 14 Aug 2026 10:11:05 +0900 Subject: [PATCH 24/24] Cover why load returns the files it read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing pinned down the return value: every assertion against it was answerable from env.sources(), and one of them compared the two lengths against each other. Add load_reports_only_what_this_call_added, which loads twice into one Environment and checks the second call reports only its own file — the case env.sources() cannot answer, and the reason the return value is not just a projection of the environment. Say as much on load itself, and note on LoadedFile that it corresponds to an entry of what RBS::EnvironmentLoader#load returns, at file rather than declaration granularity. The remaining tests reuse the tree helper this file already had instead of hand-rolling tempdir plus write, and from_loader_is_the_primary_entry_point no longer reads stdlib to assert that something was loaded. Co-Authored-By: Claude Opus 5 (1M context) --- rust/ruby-rbs/src/loader/mod.rs | 8 ++++-- rust/ruby-rbs/tests/loader.rs | 46 +++++++++++++++++++++++---------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/rust/ruby-rbs/src/loader/mod.rs b/rust/ruby-rbs/src/loader/mod.rs index cc01483053..c436f8e5cd 100644 --- a/rust/ruby-rbs/src/loader/mod.rs +++ b/rust/ruby-rbs/src/loader/mod.rs @@ -38,6 +38,8 @@ impl std::error::Error for LoadError { } } +/// Corresponds to one entry of the array `RBS::EnvironmentLoader#load` +/// returns, at file rather than declaration granularity. #[derive(Debug, Clone, PartialEq, Eq)] pub struct LoadedFile { pub path: PathBuf, @@ -77,8 +79,10 @@ impl EnvironmentLoader { self } - /// On `Err` the sources read before the failure are already in `env`, same - /// as the Ruby implementation adding sources as it walks the directories. + /// Returns what this call read, in the order it read it, so a caller + /// appending to a non-empty `env` still learns what it added. On `Err` the + /// sources read before the failure are already in `env`, same as the Ruby + /// implementation adding sources as it walks the directories. pub fn load(&self, env: &mut Environment) -> Result, LoadError> { let mut loaded = Vec::new(); let mut seen_files: HashSet = HashSet::new(); diff --git a/rust/ruby-rbs/tests/loader.rs b/rust/ruby-rbs/tests/loader.rs index 33304a84a8..3246dbec11 100644 --- a/rust/ruby-rbs/tests/loader.rs +++ b/rust/ruby-rbs/tests/loader.rs @@ -55,18 +55,17 @@ fn loads_registered_sources_in_registration_order() { #[test] fn from_loader_is_the_primary_entry_point() { - let loader = EnvironmentLoader::new(Some(repo_root().join("core"))) - .add_library("stringio", stdlib_dir("stringio")); + let dir = tree(&[("a.rbs", "class Foo\nend\n")]); + let loader = EnvironmentLoader::new(None).add_dir(dir.path().to_path_buf()); let env = Environment::from_loader(&loader).unwrap(); - assert!(!env.sources().is_empty()); + assert_eq!(env.sources().len(), 1); } #[test] fn files_are_loaded_once_first_wins() { - let dir = tempfile::tempdir().unwrap(); - fs::write(dir.path().join("a.rbs"), "class Foo\nend\n").unwrap(); + let dir = tree(&[("a.rbs", "class Foo\nend\n")]); let loader = EnvironmentLoader::new(None) .add_dir(dir.path().to_path_buf()) @@ -79,11 +78,31 @@ fn files_are_loaded_once_first_wins() { assert_eq!(env.sources().len(), 1); } +#[test] +fn load_reports_only_what_this_call_added() { + let first = tree(&[("a.rbs", "class Foo\nend\n")]); + let second = tree(&[("b.rbs", "class Bar\nend\n")]); + + let mut env = Environment::new(); + EnvironmentLoader::new(None) + .add_dir(first.path().to_path_buf()) + .load(&mut env) + .unwrap(); + let loaded = EnvironmentLoader::new(None) + .add_dir(second.path().to_path_buf()) + .load(&mut env) + .unwrap(); + + assert_eq!(env.sources().len(), 2); + let [only] = loaded.as_slice() else { + panic!("expected one loaded file, got {loaded:?}"); + }; + assert!(only.path.ends_with("b.rbs")); +} + #[test] fn explicit_dirs_do_not_skip_underscore_directories() { - let dir = tempfile::tempdir().unwrap(); - fs::create_dir(dir.path().join("_private")).unwrap(); - fs::write(dir.path().join("_private/a.rbs"), "class Foo\nend\n").unwrap(); + let dir = tree(&[("_private/a.rbs", "class Foo\nend\n")]); let loader = EnvironmentLoader::new(None).add_dir(dir.path().to_path_buf()); @@ -95,8 +114,7 @@ fn explicit_dirs_do_not_skip_underscore_directories() { #[test] fn parse_errors_carry_the_file_path() { - let dir = tempfile::tempdir().unwrap(); - fs::write(dir.path().join("broken.rbs"), "class\n").unwrap(); + let dir = tree(&[("broken.rbs", "class\n")]); let loader = EnvironmentLoader::new(None).add_dir(dir.path().to_path_buf()); @@ -150,10 +168,12 @@ fn library_dirs_skip_underscore_directories() { let mut env = Environment::new(); let loaded = loader.load(&mut env).unwrap(); - assert_eq!(loaded.len(), 1); - assert!(loaded[0].path.ends_with("a.rbs")); + let [only] = loaded.as_slice() else { + panic!("expected one loaded file, got {loaded:?}"); + }; + assert!(only.path.ends_with("a.rbs")); assert_eq!( - loaded[0].kind, + only.kind, SourceKind::Library { name: "gem1".to_string(), path: dir.path().join("gem1/1.2.3"),