From 0e2d9413f9ef816143d1fe48d3ba5cde1893de86 Mon Sep 17 00:00:00 2001 From: waffle Date: Fri, 29 Aug 2025 01:15:27 -0400 Subject: [PATCH 1/4] Integrate wally-package-types This change integrates the core behavior of https://github.com/JohnnyMorganz/wally-package-types into wally so that after install, package thunks are automatically mutated to re-export Luau types. No Rojo sourcemap.json is needed; no separate tool or CLI flag is required. New modules, adapted/copied from wally-package-types: src/require_parser.rs src/link_mutator.rs (kept for AST helpers/tests; not used by the main pass) src/type_reexports.rs (adapted from wally-package-types command.rs to run with filesystem traversal instead of a sourcemap) The install flow now performs a best-effort post-pass to mutate thunks and add Luau type re-exports. See Rust.InstallationContext::install(). The pass is invoked automatically and logs warnings rather than failing installation if it encounters an error. TLS stabilization (Windows): Switched to rustls-only reqwest in Cargo.toml and wally-registry-backend/Cargo.toml to avoid schannel/native-tls instability. src/package_source/registry.rs constructs a rustls client explicitly. --- Cargo.lock | 117 +++++++++-- Cargo.toml | 3 +- README.md | 41 ++++ src/commands/install.rs | 6 +- src/commands/update.rs | 2 +- src/installation.rs | 16 +- src/lib.rs | 3 + src/link_mutator.rs | 288 +++++++++++++++++++++++++++ src/package_source/registry.rs | 6 +- src/require_parser.rs | 105 ++++++++++ src/type_reexports.rs | 310 ++++++++++++++++++++++++++++++ tests/integration/util.rs | 4 +- wally-registry-backend/Cargo.toml | 2 +- 13 files changed, 871 insertions(+), 32 deletions(-) create mode 100644 src/link_mutator.rs create mode 100644 src/require_parser.rs create mode 100644 src/type_reexports.rs diff --git a/Cargo.lock b/Cargo.lock index fa8ea0db..f2f840a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "adler32" @@ -98,7 +98,7 @@ checksum = "16e62a023e7c117e27523144c5d2459f4397fcc3cab0085af8e2224f643a0193" dependencies = [ "proc-macro2", "quote", - "syn 2.0.43", + "syn 2.0.87", ] [[package]] @@ -218,6 +218,15 @@ dependencies = [ "generic-array", ] +[[package]] +name = "borsh" +version = "1.5.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ad8646f98db542e39fc66e68a20b2144f6a732636df7c2354e74645faaa433ce" +dependencies = [ + "cfg_aliases", +] + [[package]] name = "bstr" version = "0.2.15" @@ -332,6 +341,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "cfg_aliases" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" + [[package]] name = "chrono" version = "0.4.19" @@ -658,6 +673,27 @@ dependencies = [ "powerfmt", ] +[[package]] +name = "derive_more" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4a9b99b9cbbe49445b21764dc0625032a89b145a2642e67603e1c936f5458d05" +dependencies = [ + "derive_more-impl", +] + +[[package]] +name = "derive_more-impl" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb7330aeadfbe296029522e6c40f315320aba36fc43a5b3632f3795348f3bd22" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.87", + "unicode-xid", +] + [[package]] name = "devise" version = "0.4.1" @@ -688,7 +724,7 @@ dependencies = [ "proc-macro2", "proc-macro2-diagnostics", "quote", - "syn 2.0.43", + "syn 2.0.87", ] [[package]] @@ -923,6 +959,33 @@ dependencies = [ "winapi", ] +[[package]] +name = "full_moon" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a02c056a5966f6db8e663ee48093280f098d68d97763a94490ee63178d54776" +dependencies = [ + "bytecount", + "cfg-if 1.0.0", + "derive_more", + "full_moon_derive", + "paste", + "serde", + "smol_str", +] + +[[package]] +name = "full_moon_derive" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "99b4bd12ce56927d1dc5478d21528ea8c4b93ca85ff8f8043b6a5351a2a3c6f7" +dependencies = [ + "indexmap 1.9.3", + "proc-macro2", + "quote", + "syn 1.0.67", +] + [[package]] name = "futures" version = "0.3.17" @@ -1934,6 +1997,12 @@ dependencies = [ "windows-sys 0.45.0", ] +[[package]] +name = "paste" +version = "1.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" + [[package]] name = "pear" version = "0.2.4" @@ -1954,7 +2023,7 @@ dependencies = [ "proc-macro2", "proc-macro2-diagnostics", "quote", - "syn 2.0.43", + "syn 2.0.87", ] [[package]] @@ -2072,7 +2141,7 @@ checksum = "606c4ba35817e2922a308af55ad51bab3645b59eae5c570d4a6cf07e36bd493b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.43", + "syn 2.0.87", "version_check", "yansi", ] @@ -2106,9 +2175,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.28" +version = "1.0.40" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b9ab9c7eadfd8df19006f1cf1a4aed13540ed5cbc047010ece5826e10825488" +checksum = "1885c039570dc00dcb4ff087a89e185fd56bae234ddc7f056a945bf36467248d" dependencies = [ "proc-macro2", ] @@ -2357,12 +2426,10 @@ dependencies = [ "http-body", "hyper", "hyper-rustls", - "hyper-tls", "ipnet", "js-sys", "log", "mime", - "native-tls", "once_cell", "percent-encoding", "pin-project-lite", @@ -2372,7 +2439,6 @@ dependencies = [ "serde_json", "serde_urlencoded", "tokio", - "tokio-native-tls", "tokio-rustls", "tokio-util", "tower-service", @@ -2449,7 +2515,7 @@ dependencies = [ "proc-macro2", "quote", "rocket_http", - "syn 2.0.43", + "syn 2.0.87", "unicode-xid", ] @@ -2777,7 +2843,7 @@ checksum = "43576ca501357b9b071ac53cdc7da8ef0cbd9493d8df094cd821777ea6e894d3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.43", + "syn 2.0.87", ] [[package]] @@ -2870,7 +2936,7 @@ checksum = "91d129178576168c589c9ec973feedf7d3126c01ac2bf08795109aa35b69fb8f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.43", + "syn 2.0.87", ] [[package]] @@ -2975,6 +3041,16 @@ version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a507befe795404456341dfab10cef66ead4c041f62b8b11bbb92bffe5d0953e0" +[[package]] +name = "smol_str" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9676b89cd56310a87b93dec47b11af744f34d5fc9f367b829474eec0a891350d" +dependencies = [ + "borsh", + "serde", +] + [[package]] name = "socket2" version = "0.4.9" @@ -3082,9 +3158,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.43" +version = "2.0.87" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee659fb5f3d355364e1f3e5bc10fb82068efbf824a1e9d1c9504244a6469ad53" +checksum = "25aa4ce346d03a6dcd68dd8b4010bcb74e54e62c90c573f394c46eae99aba32d" dependencies = [ "proc-macro2", "quote", @@ -3335,7 +3411,7 @@ checksum = "630bdcf245f78637c13ec01ffae6187cca34625e8c63150d424b59e55af2675e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.43", + "syn 2.0.87", ] [[package]] @@ -3463,7 +3539,7 @@ checksum = "0f57e3ca2a01450b1a921183a9c9cbfda207fd822cef4ccb00a65402cbba7a74" dependencies = [ "proc-macro2", "quote", - "syn 2.0.43", + "syn 2.0.87", ] [[package]] @@ -3523,7 +3599,7 @@ version = "1.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1f559b464de2e2bdabcac6a210d12e9b5a5973c251e102c44c585c71d51bd78e" dependencies = [ - "cfg-if 0.1.10", + "cfg-if 1.0.0", "static_assertions", ] @@ -3605,9 +3681,9 @@ checksum = "9337591893a19b88d8d87f2cec1e73fad5cdfd10e5a6f349f498ad6ea2ffb1e3" [[package]] name = "unicode-xid" -version = "0.2.1" +version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7fe0bb3479651439c9112f72b6c505038574c9fbb575ed1bf3b797fa39dd564" +checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" [[package]] name = "unindent" @@ -3719,6 +3795,7 @@ dependencies = [ "dirs", "env_logger", "fs-err", + "full_moon", "git2", "globset", "hex", diff --git a/Cargo.toml b/Cargo.toml index 6deccd31..da84f0af 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,7 @@ indoc = "1.0.3" log = "0.4.11" once_cell = "1.5.2" opener = "0.5.0" -reqwest = { version = "0.11.18", features = ["blocking", "json"] } +reqwest = { version = "0.11.18", default-features = false, features = ["blocking", "json", "rustls-tls"] } rpassword = "5.0.1" semver = { version = "0.11.0", features = ["serde"] } serde = { version = "1.0.116", features = ["derive"] } @@ -59,6 +59,7 @@ indicatif = "0.17.4" tokio = "1.28.2" serial_test = "2.0.0" time = "=0.3.35" +full_moon = { version = "1.2.0", features = ["luau"] } [dev-dependencies] insta = { version = "1.1.0" } diff --git a/README.md b/README.md index e71128c1..443743f0 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,7 @@ * [Installation](#installation) * [Commands](#commands) +* [Type Re-export Functionality](#type-re-export-functionality) * [Prior Art](#prior-art) * [Manifest Format](#manifest-format) * [Lockfile Format](#lockfile-format) @@ -189,6 +190,13 @@ exclude = ["node_modules"] # Packages can be marked as private to prevent them from being published. private = true +[place] +# Where the shared packages folder is located in the Roblox Datamodel +shared-packages = "game.ReplicatedStorage.Packages" + +# Where the server packages folder is located in the Roblox Datamodel +server-packages = "game.ServerScriptStorage.Packages" + [dependencies] # Most dependencies will look like this. # @@ -211,6 +219,39 @@ Promise = "evaera/promise@2.0.1" TestEZ = "roblox/testez@0.4.1" ``` +## Type Re-export Functionality + +Wally automatically generates type re-exports to improve Luau type checking support in Roblox projects. This runs as a post-pass immediately after installation. + +### How it works + +- After installation, Wally traverses `Packages`, `ServerPackages`, and `DevPackages` and inspects generated link thunks. +- It resolves the `require(...)` target using the filesystem relative to the thunk location: + - Supports `script.Parent` chains and `game.*.Packages` paths. + - Probes module files in this order: exact path, `{name}.lua` / `{name}.luau`, `init.lua` / `init.luau`, `src/init.lua` / `src/init.luau`. +- It scans the target module textually for lines beginning with `export type` and emits re-exports into the thunk: + - Keeps the original generic parameter list on the left-hand side. + - Uses only generic names (not defaults) on the right-hand side. + - Includes variadic generic packs (e.g., `T...`). + +### Example + +A simple package thunk like: +```lua +return require(script.Parent.Parent["scope/package"]["package"]) +``` + +Becomes a type-aware thunk like: +```lua +local REQUIRED_MODULE = require(script.Parent.Parent["scope/package"]["package"]) + +export type MyExportedType = REQUIRED_MODULE.MyExportedType + +return REQUIRED_MODULE +``` + +This enables proper type checking when using Wally packages in your Roblox projects. + ## Lockfile Format The lockfile contains the exact versions of each dependency that a project depends on. They're a critical feature that ensures that everyone who works on a game is getting the exact same version of every package. diff --git a/src/commands/install.rs b/src/commands/install.rs index 3ef98de6..d19cd1b8 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -12,7 +12,6 @@ use structopt::StructOpt; use crate::installation::InstallationContext; use crate::lockfile::Lockfile; use crate::manifest::Manifest; -use crate::package_id::PackageId; use crate::package_source::{PackageSource, PackageSourceMap, Registry, TestRegistry}; use crate::resolution::resolve; @@ -136,12 +135,13 @@ impl InstallSubcommand { SetForegroundColor(Color::DarkGreen), SetForegroundColor(Color::Reset) )); - let root_package_id = PackageId::new(manifest.package.name, manifest.package.version); + + let root_package_id = manifest.package_id(); let installation = InstallationContext::new( &self.project_path, manifest.place.shared_packages, manifest.place.server_packages, - ); + )?; installation.clean()?; progress.println(format!( diff --git a/src/commands/update.rs b/src/commands/update.rs index 36c4cd62..2211b503 100644 --- a/src/commands/update.rs +++ b/src/commands/update.rs @@ -120,7 +120,7 @@ impl UpdateSubcommand { &self.project_path, manifest.place.shared_packages, manifest.place.server_packages, - ); + )?; progress.set_message(format!( "{} Cleaning {}package destination...", diff --git a/src/installation.rs b/src/installation.rs index c9fd4f9d..0868a948 100644 --- a/src/installation.rs +++ b/src/installation.rs @@ -37,7 +37,7 @@ impl InstallationContext { project_path: &Path, shared_path: Option, server_path: Option, - ) -> Self { + ) -> anyhow::Result { let shared_dir = project_path.join("Packages"); let server_dir = project_path.join("ServerPackages"); let dev_dir = project_path.join("DevPackages"); @@ -46,7 +46,7 @@ impl InstallationContext { let server_index_dir = server_dir.join("_Index"); let dev_index_dir = dev_dir.join("_Index"); - Self { + Ok(Self { shared_dir, shared_index_dir, shared_path, @@ -55,7 +55,7 @@ impl InstallationContext { server_path, dev_dir, dev_index_dir, - } + }) } /// Delete the existing index, if it exists. @@ -171,6 +171,16 @@ impl InstallationContext { } bar.finish_and_clear(); + + // Post-pass: filesystem-based traversal/mutation on installed links + for dir in [&self.shared_dir, &self.server_dir, &self.dev_dir] { + if dir.exists() { + if let Err(err) = crate::type_reexports::run_on_packages_fs(dir) { + log::warn!("Type re-export pass failed for {}: {}", dir.display(), err); + } + } + } + log::info!("Downloaded {} packages!", num_packages); Ok(()) diff --git a/src/lib.rs b/src/lib.rs index 1d734aff..cab640cd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,5 +12,8 @@ pub mod package_req; pub mod package_source; pub mod resolution; pub mod test_package; +pub mod require_parser; +pub mod link_mutator; +pub mod type_reexports; pub use commands::*; diff --git a/src/link_mutator.rs b/src/link_mutator.rs new file mode 100644 index 00000000..f6650a20 --- /dev/null +++ b/src/link_mutator.rs @@ -0,0 +1,288 @@ +use anyhow::{bail, Result}; +use full_moon::{ + ast::{ + luau::{ + ExportedTypeDeclaration, GenericDeclaration, GenericDeclarationParameter, + GenericParameterInfo, IndexedTypeInfo, TypeInfo, + }, + punctuated::{Pair, Punctuated}, + span::ContainedSpan, + Ast, Expression, LastStmt, LocalAssignment, Return, Stmt, + }, + tokenizer::{Token, TokenReference, TokenType}, +}; + +/// Finds all exported type declarations from a give source file +pub fn type_declarations_from_source(code: &str) -> Result> { + let parsed_module = match full_moon::parse(code) { + Ok(parsed_code) => parsed_code, + Err(errors) => bail!(errors + .iter() + .map(|err| err.to_string()) + .collect::>() + .join("\n")), + }; + + Ok(parsed_module + .nodes() + .stmts() + .filter_map(|stmt| match stmt { + Stmt::ExportedTypeDeclaration(stmt) => Some(stmt.clone()), + _ => None, + }) + .collect()) +} + +fn should_keep_default_type(type_info: &TypeInfo, resolved_types: &[String]) -> bool { + // TODO: we could be more clever here, but for now we keep it simple + match type_info { + TypeInfo::Basic(name) => resolved_types.contains(&name.token().to_string()), + TypeInfo::Boolean(_) => true, + _ => false, + } +} + +fn strip_unknown_default_generics( + generics: &GenericDeclaration, + resolved_types: &[String], +) -> Punctuated { + generics + .generics() + .pairs() + .map(|pair| { + pair.clone().map(|decl| match decl.default_type() { + Some(type_info) if should_keep_default_type(type_info, resolved_types) => decl, + _ => decl.with_default(None), + }) + }) + .collect::>() +} + +pub fn create_new_type_declaration(stmt: &ExportedTypeDeclaration) -> ExportedTypeDeclaration { + let type_info = match stmt.type_declaration().generics() { + Some(generics) => IndexedTypeInfo::Generic { + base: stmt.type_declaration().type_name().clone(), + arrows: ContainedSpan::new( + TokenReference::symbol("<").unwrap(), + TokenReference::symbol(">").unwrap(), + ), + generics: generics + .generics() + .pairs() + .map(|pair| { + pair.clone().map(|decl| match decl.parameter() { + GenericParameterInfo::Name(token) => TypeInfo::Basic(token.clone()), + GenericParameterInfo::Variadic { name, ellipsis } => { + TypeInfo::GenericPack { + name: name.clone(), + ellipsis: ellipsis.clone(), + } + } + _ => unreachable!(), + }) + }) + .collect::>(), + }, + None => IndexedTypeInfo::Basic(stmt.type_declaration().type_name().clone()), + }; + + // Modify the original type declaration to remove the default generics, if they are not resolvable + let mut resolved_types = stmt + .type_declaration() + .generics() + .map_or(vec![], |generics| { + generics + .generics() + .iter() + .map(|generic| match generic.parameter() { + GenericParameterInfo::Name(name) => name.token().to_string(), + GenericParameterInfo::Variadic { name, .. } => name.token().to_string(), + other => unreachable!("unknown node: {:?}", other), + }) + .collect() + }); + resolved_types.extend( + [ + "any", "boolean", "buffer", "never", "number", "string", "thread", "unknown", + ] + .iter() + .map(|s| s.to_string()), + ); + + let original_type_declaration = match stmt.type_declaration().generics() { + Some(generics) => stmt.type_declaration().clone().with_generics(Some( + generics + .clone() + .with_generics(strip_unknown_default_generics(generics, &resolved_types)), + )), + None => stmt.type_declaration().clone(), + }; + + // Can't use TypeDeclaration::new(), since it always panics + let type_declaration = original_type_declaration.with_type_definition(TypeInfo::Module { + module: TokenReference::new( + vec![], + Token::new(TokenType::Identifier { + identifier: "REQUIRED_MODULE".into(), + }), + vec![], + ), + punctuation: TokenReference::symbol(".").unwrap(), + type_info: Box::new(type_info), + }); + + ExportedTypeDeclaration::new(type_declaration) +} + +// Creates a list of re-exported type declarations from the type declarations found in the source file +fn re_export_type_declarations( + stmts: Vec, +) -> Vec<(Stmt, Option)> { + stmts + .iter() + .map(|stmt| { + ( + Stmt::ExportedTypeDeclaration(create_new_type_declaration(stmt)), + Some(TokenReference::new( + vec![], + Token::new(TokenType::Whitespace { + characters: "\n".into(), + }), + vec![], + )), + ) + }) + .collect() +} + +/// Extracts a require expression out into a local variable of form `local REQUIRED_MODULE = ...` +fn extract_require_into_local_stmt( + return_expressions: Punctuated, +) -> (Stmt, Option) { + ( + Stmt::LocalAssignment( + LocalAssignment::new( + std::iter::once(Pair::End(TokenReference::new( + vec![], + Token::new(TokenType::Identifier { + identifier: "REQUIRED_MODULE".into(), + }), + vec![], + ))) + .collect(), + ) + .with_equal_token(Some(TokenReference::symbol(" = ").unwrap())) + .with_expressions(return_expressions), + ), + None, + ) +} + +/// Creates a `return REQUIRED_MODULE` node +fn create_return_require_variable() -> (LastStmt, Option) { + ( + LastStmt::Return( + Return::new().with_returns( + std::iter::once(Pair::End(Expression::Symbol(TokenReference::new( + vec![], + Token::new(TokenType::Identifier { + identifier: "REQUIRED_MODULE".into(), + }), + vec![Token::new(TokenType::Whitespace { + characters: "\n".into(), + })], + )))) + .collect(), + ), + ), + None, + ) +} + +pub enum MutateLinkResult { + Changed(Ast), + Unchanged, +} + +/// Given an old link and the contents of the file it points to, creates a new link source +pub fn mutate_link( + parsed_code: Ast, + return_expressions: Punctuated, + contents: &str, +) -> Result { + let type_declarations = type_declarations_from_source(contents)?; + + if type_declarations.is_empty() { + return Ok(MutateLinkResult::Unchanged); + } + + let new_nodes = parsed_code + .nodes() + .clone() + .with_stmts( + std::iter::once(extract_require_into_local_stmt(return_expressions)) + .chain(re_export_type_declarations(type_declarations)) + .collect(), + ) + .with_last_stmt(Some(create_return_require_variable())); + Ok(MutateLinkResult::Changed(parsed_code.with_nodes(new_nodes))) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn re_exports_generic_defaults_if_they_are_part_of_the_type() { + let code = r" + export type Value = Types.Value + "; + + let type_declarations = type_declarations_from_source(code).unwrap(); + assert_eq!(type_declarations.len(), 1); + + let reexported_type_declarations = re_export_type_declarations(type_declarations); + assert_eq!(reexported_type_declarations.len(), 1); + + assert_eq!( + reexported_type_declarations[0].0.to_string(), + "export type Value = REQUIRED_MODULE.Value" + ); + } + + #[test] + fn does_not_re_export_unknown_default_generics() { + let code = r" + export type Value = Types.Value + "; + + let type_declarations = type_declarations_from_source(code).unwrap(); + assert_eq!(type_declarations.len(), 1); + + let reexported_type_declarations = re_export_type_declarations(type_declarations); + assert_eq!(reexported_type_declarations.len(), 1); + + assert_eq!( + reexported_type_declarations[0].0.to_string(), + "export type Value = REQUIRED_MODULE.Value" + ); + } + + #[test] + fn re_exports_generic_defaults_if_they_are_builtin_types() { + let code = r" + export type Value = Types.Value + "; + + let type_declarations = type_declarations_from_source(code).unwrap(); + assert_eq!(type_declarations.len(), 1); + + let reexported_type_declarations = re_export_type_declarations(type_declarations); + assert_eq!(reexported_type_declarations.len(), 1); + + assert_eq!( + reexported_type_declarations[0].0.to_string(), + "export type Value = REQUIRED_MODULE.Value" + ); + } +} \ No newline at end of file diff --git a/src/package_source/registry.rs b/src/package_source/registry.rs index bd678fdf..ab76dea3 100644 --- a/src/package_source/registry.rs +++ b/src/package_source/registry.rs @@ -30,12 +30,14 @@ impl Registry { /// `registry` field of a package manifest. pub fn from_registry_spec(spec: &str) -> anyhow::Result { let index_url = Url::parse(spec)?; - + // Force Rustls to avoid Windows schannel/native-tls issues + let client = Client::builder().use_rustls_tls().build()?; + Ok(Self { index_url, auth_token: OnceCell::new(), index: OnceCell::new(), - client: Client::new(), + client, }) } diff --git a/src/require_parser.rs b/src/require_parser.rs new file mode 100644 index 00000000..b87c7a23 --- /dev/null +++ b/src/require_parser.rs @@ -0,0 +1,105 @@ +use anyhow::{bail, Result}; +use full_moon::{ + ast::{Call, Expression, FunctionArgs, Index, Suffix, Var}, + tokenizer::TokenType, +}; + +/// Decomposes a VarExpression into a list of string components +pub fn expression_to_components(expression: &Expression) -> Result> { + let mut components = Vec::new(); + + let Expression::Var(Var::Expression(var_expression)) = expression else { + bail!("require expression not supported: expression must contain components of form `.value` or `['value']`") + }; + + components.push(var_expression.prefix().to_string().trim().to_string()); + + for suffix in var_expression.suffixes() { + let Suffix::Index(index) = suffix else { + bail!("require expression not supported: expression must contain components of form `.value` or `['value']`") + }; + + match index { + Index::Dot { name, .. } => { + components.push(name.to_string().trim().to_string()); + } + Index::Brackets { expression, .. } => { + let Expression::String(name) = expression else { + bail!("require expression not supported: expression contains brackets component not of the form ['value']") + }; + let TokenType::StringLiteral { literal, .. } = name.token_type() else { + bail!("require expression not supported: expression contains brackets component not of the form ['value']") + }; + components.push(literal.trim().to_string()); + } + _ => unreachable!(), + } + } + + Ok(components) +} + +/// Parse a require() expression and extract its path components +pub fn match_require(expression: &Expression) -> Result> { + let Expression::FunctionCall(call) = expression else { + bail!("'{}' is not a function call", expression.to_string().trim()); + }; + + if call.prefix().to_string().trim() == "require" && call.suffixes().count() == 1 { + if let Suffix::Call(Call::AnonymousCall(FunctionArgs::Parentheses { arguments, .. })) = + call.suffixes().next().unwrap() + { + if arguments.len() == 1 { + return expression_to_components(arguments.iter().next().unwrap()); + } + } + } else { + bail!("unknown require expression '{}'", call.to_string().trim()); + } + + bail!( + "'{}' is not a require function call", + call.to_string().trim() + ) +} + +#[cfg(test)] +mod tests { + use full_moon::ast::Stmt; + + use super::*; + + fn require_expression(code: &str) -> Expression { + let parsed_ast = full_moon::parse(code).unwrap(); + let stmt = parsed_ast.nodes().stmts().next().unwrap(); + let Stmt::FunctionCall(expression) = stmt else { + unreachable!() + }; + Expression::FunctionCall(expression.clone()) + } + + fn expression_into_components(code: &str, components: Vec<&str>) -> bool { + match_require(&require_expression(code)).unwrap() == components + } + + #[test] + fn simple_require() { + assert!(expression_into_components( + "require(script.Parent.Example)", + vec!["script", "Parent", "Example"] + )) + } + + #[test] + fn require_with_brackets() { + assert!(expression_into_components( + "require(script.Parent['Example'])", + vec!["script", "Parent", "Example"] + )) + } + + #[test] + fn unhandled_require() { + assert!(match_require(&require_expression("require('string')")).is_err()) + } +} \ No newline at end of file diff --git a/src/type_reexports.rs b/src/type_reexports.rs new file mode 100644 index 00000000..4b4432ce --- /dev/null +++ b/src/type_reexports.rs @@ -0,0 +1,310 @@ +use anyhow::{bail, Context, Result}; +use full_moon::ast::LastStmt; +use log::{error, info, warn}; +use std::path::{Path, PathBuf}; + +use crate::require_parser::*; + + +// Filesystem-based resolver +fn resolve_components_fs(link_path: &Path, packages_root: &Path, path_components: &[String]) -> Option { + if path_components.is_empty() { + return None; + } + + let first = &path_components[0]; + let mut cursor: PathBuf; + + if first == "script" { + // Start from the link file path (`script`), allow Parent to walk up to the link's directory + cursor = link_path.to_path_buf(); + for comp in path_components.iter().skip(1) { + if comp == "Parent" { + cursor = cursor.parent()?.to_path_buf(); + } else { + cursor.push(comp); + } + } + } else if first == "game" { + // Map game.<...>.Packages.* to the on-disk Packages root for this pass + cursor = packages_root.to_path_buf(); + + let mut started = false; + for comp in path_components.iter().skip(1) { + if !started { + // Skip components until we hit "Packages" + if comp == "Packages" { + started = true; + } + continue; + } + + if comp == "Parent" { + cursor = cursor.parent()?.to_path_buf(); + } else { + cursor.push(comp); + } + } + + // Fallback: if "Packages" wasn't found, try pushing everything after 'game' + if !started { + for comp in path_components.iter().skip(1) { + if comp == "Parent" { + cursor = cursor.parent()?.to_path_buf(); + } else { + cursor.push(comp); + } + } + } + } else { + return None; + } + + // If direct file exists, use it. + if cursor.is_file() { + return Some(cursor); + } + + // Try {name}.lua / {name}.luau + let candidate_lua = cursor.with_extension("lua"); + if candidate_lua.is_file() { + return Some(candidate_lua); + } + let candidate_luau = cursor.with_extension("luau"); + if candidate_luau.is_file() { + return Some(candidate_luau); + } + + // Try init.lua / init.luau inside a directory + let init_lua = cursor.join("init.lua"); + if init_lua.is_file() { + return Some(init_lua); + } + let init_luau = cursor.join("init.luau"); + if init_luau.is_file() { + return Some(init_luau); + } + + // Also try src/init.lua / src/init.luau (common layout) + let src_init_lua = cursor.join("src").join("init.lua"); + if src_init_lua.is_file() { + return Some(src_init_lua); + } + let src_init_luau = cursor.join("src").join("init.luau"); + if src_init_luau.is_file() { + return Some(src_init_luau); + } + + None +} + +/// Mutate a single thunk using filesystem resolution only (no sourcemap) +fn mutate_thunk_fs(path: &Path, packages_root: &Path) -> Result<()> { + info!("Found link file '{}'", path.display()); + + // Skip already-mutated thunks + let original_src = std::fs::read_to_string(path)?; + if original_src.contains("local REQUIRED_MODULE") || original_src.contains("return REQUIRED_MODULE") { + info!("Link already mutated, leaving unchanged"); + return Ok(()); + } + + // Parse to extract the require() expression + let parsed_code = match full_moon::parse(&original_src) { + Ok(parsed_code) => parsed_code, + Err(errors) => bail!(errors + .iter() + .map(|err| err.to_string()) + .collect::>() + .join("\n")), + }; + + if let Some(LastStmt::Return(r#return)) = parsed_code.nodes().last_stmt() { + let returned_expression = r#return.returns().iter().next().unwrap(); + + let path_components = match match_require(returned_expression) { + Ok(components) => components, + Err(err) => { + warn!("Malformed link file, could not parse return expression, skipping. Run `wally install` to regenerate link files"); + error!("{:#}", err); + return Ok(()); + } + }; + + info!( + "Require expression converted to path: '{}'", + path_components.join("/") + ); + + let file_path = resolve_components_fs(path, packages_root, &path_components) + .context("Could not resolve require expression to file path via filesystem")?; + let pass_through_contents = + std::fs::read_to_string(&file_path).context("Failed to read linked file")?; + + // Build textual re-exports by scanning the target module for "export type" declarations. + // We keep the original generic declaration on the LHS and emit RHS with generic names only, + // including variadic generic packs (e.g., T...). + let mut reexports = Vec::new(); + for line in pass_through_contents.lines() { + let trimmed = line.trim_start(); + if !trimmed.starts_with("export type ") { + continue; + } + + // Parse "export type Name<...> = ..." + let rest = &trimmed["export type ".len()..]; + + // Extract type name and optional generics by locating the first '<' ... '>' pair. + let (type_name, generics_opt) = if let Some(lt_rel) = rest.find('<') { + let name = rest[..lt_rel].trim(); + // find the closing '>' after lt_rel + let gt_rel = match rest[lt_rel + 1..].find('>') { + Some(i) => lt_rel + 1 + i, + None => continue, + }; + let generics = &rest[lt_rel + 1..gt_rel]; + (name, Some(generics.trim())) + } else { + // No generics + // Stop at the '=' if present, otherwise take the whole remainder + let name_end = rest.find('=').unwrap_or(rest.len()); + (rest[..name_end].trim(), None) + }; + + // Derive list of generic parameter identifiers without defaults. + // Keep variadic packs by preserving the trailing "...". + let mut rhs_generics: Vec = Vec::new(); + if let Some(generics) = generics_opt { + for raw in generics.split(',') { + // Strip default, e.g., "S = T" -> "S" and preserve "..." + let name_only = raw.splitn(2, '=').next().unwrap().trim(); + if !name_only.is_empty() { + rhs_generics.push(name_only.to_string()); + } + } + } + + // Rebuild generics for LHS (keep original) and RHS (names only) + let lhs_suffix = if let Some(generics) = generics_opt { + format!("<{}>", generics) + } else { + String::new() + }; + let rhs_suffix = if rhs_generics.is_empty() { + String::new() + } else { + format!("<{}>", rhs_generics.join(", ")) + }; + + reexports.push(format!( + "export type {}{} = REQUIRED_MODULE.{}{}", + type_name, lhs_suffix, type_name, rhs_suffix + )); + } + + if reexports.is_empty() { + info!("No exported types, leaving unchanged"); + return Ok(()); + } + + // Extract the original `require(...)` expression from the link source to preserve formatting. + let mut require_expr_opt: Option = None; + for line in original_src.lines() { + let t = line.trim_start(); + if t.starts_with("return require") { + // Strip leading "return " + let expr = t.strip_prefix("return ").unwrap_or(t).trim().to_string(); + require_expr_opt = Some(expr); + break; + } + } + // Build canonical one if missing + let require_expr = require_expr_opt.unwrap_or_else(|| { + let mut expr = String::from("require("); + let mut first = true; + for comp in &path_components { + if first { + expr.push_str(comp); + first = false; + } else if comp == "Parent" { + expr.push_str(".Parent"); + } else if comp == "_Index" { + expr.push_str("._Index"); + } else { + expr.push_str(&format!("[\"{}\"]", comp)); + } + } + expr.push(')'); + expr + }); + + let mut new_source = String::new(); + new_source.push_str(&format!("local REQUIRED_MODULE = {}\n", require_expr)); + for line in reexports { + new_source.push_str(&line); + new_source.push('\n'); + } + new_source.push_str("return REQUIRED_MODULE\n"); + + info!("Exported types found, writing new linker file"); + std::fs::write(path, new_source)?; + } else { + warn!("Malformed link file, no return statement found, skipping. Run `wally install` to regenerate link files"); + return Ok(()); + } + + Ok(()) +} + +fn handle_index_directory_fs(index_dir: &Path, packages_root: &Path) -> Result { + let mut success = true; + + for package_entry in std::fs::read_dir(index_dir)?.flatten() { + for thunk in std::fs::read_dir(package_entry.path())?.flatten() { + if thunk.file_type().map(|t| t.is_file()).unwrap_or(false) { + if let Err(err) = mutate_thunk_fs(&thunk.path(), packages_root) { + error!("{:#}", err); + success = false; + } + } + } + } + + Ok(success) +} + +/// Public entry point: filesystem-based traversal (no sourcemap needed) +pub fn run_on_packages_fs(packages_root: &Path) -> Result<()> { + let mut success = true; + + // Iterate top-level under Packages + for entry in std::fs::read_dir(packages_root) + .with_context(|| format!("Failed to read packages folder {}", packages_root.display()))? + .flatten() + { + if entry.file_name() == "_Index" { + match handle_index_directory_fs(&entry.path(), packages_root) { + Ok(ok) => success &= ok, + Err(err) => { + error!("{:#}", err); + success = false; + } + } + continue; + } + + // Only mutate link thunks (files) + if entry.file_type().map(|t| t.is_file()).unwrap_or(false) { + if let Err(err) = mutate_thunk_fs(&entry.path(), packages_root) { + error!("{:#}", err); + success = false; + } + } + } + + if success { + Ok(()) + } else { + bail!("Mutation did not complete successfully") + } +} \ No newline at end of file diff --git a/tests/integration/util.rs b/tests/integration/util.rs index c307dffc..108a1102 100644 --- a/tests/integration/util.rs +++ b/tests/integration/util.rs @@ -35,6 +35,8 @@ pub fn read_path(path: &Path) -> anyhow::Result { Ok(Entry::Dir(children)) } else { let contents = fs_err::read_to_string(path)?; - Ok(Entry::File(contents)) + // Normalize line endings for stable cross-platform snapshots + let normalized = contents.replace("\r\n", "\n"); + Ok(Entry::File(normalized)) } } diff --git a/wally-registry-backend/Cargo.toml b/wally-registry-backend/Cargo.toml index 4a11ae82..1fb85900 100644 --- a/wally-registry-backend/Cargo.toml +++ b/wally-registry-backend/Cargo.toml @@ -24,7 +24,7 @@ figment = "0.10.9" fs-err = "2.5.0" futures = "0.3.13" git2 = "0.16.1" -reqwest = { version = "0.11.0", features = ["blocking", "json"] } +reqwest = { version = "0.11.18", default-features = false, features = ["blocking", "json", "rustls-tls"] } rocket = { git = "https://github.com/SergioBenitez/Rocket", rev = "91f6288ea4aeb3d5a502b2f18b2b9677a85463ea", features = ["json"] } rusoto_core = { version = "0.48.0", optional = true } rusoto_s3 = { version = "0.48.0", optional = true } From 20b7180b82c6fc27e3dcb5d97866c801d00df2f8 Mon Sep 17 00:00:00 2001 From: waffle Date: Fri, 29 Aug 2025 13:01:58 -0400 Subject: [PATCH 2/4] satisfy cargo clippy many minor changes to satisfy `cargo fmt -- --check` and `cargo clippy` to pass the build --- src/auth.rs | 4 ++-- src/commands/mod.rs | 13 +------------ src/commands/package.rs | 2 +- src/git_util.rs | 18 ++++++++---------- src/lib.rs | 4 ++-- src/link_mutator.rs | 8 +++++--- src/lockfile.rs | 16 ++++++++-------- src/manifest.rs | 15 ++------------- src/package_contents.rs | 9 +++------ src/package_index.rs | 4 ++-- src/package_name.rs | 4 ++-- src/package_req.rs | 2 +- src/package_source/in_memory.rs | 10 +++++++++- src/package_source/registry.rs | 4 ++-- src/require_parser.rs | 2 +- src/test_package.rs | 3 +-- src/type_reexports.rs | 25 +++++++++++++++---------- wally-registry-backend/src/main.rs | 4 ++-- wally-registry-backend/src/search.rs | 2 +- 19 files changed, 68 insertions(+), 81 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 72b30308..9704b3a1 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -70,13 +70,13 @@ impl AuthStore { } fn contents(path: &Path) -> anyhow::Result { - match fs_err::read_to_string(&path) { + match fs_err::read_to_string(path) { Ok(contents) => Ok(contents), Err(err) => { if err.kind() == io::ErrorKind::NotFound { Ok(DEFAULT_AUTH_TOML.to_owned()) } else { - return Err(err.into()); + Err(err.into()) } } } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 1af65781..1f14c5a3 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -48,7 +48,7 @@ impl Args { } /// Options that apply to all subcommands for the CLI. -#[derive(Debug, StructOpt)] +#[derive(Debug, StructOpt, Default)] pub struct GlobalOptions { /// Enable more verbose logging. Can be specified multiple times to increase /// verbosity further. @@ -68,17 +68,6 @@ pub struct GlobalOptions { pub check_token: Option, } -impl Default for GlobalOptions { - fn default() -> Self { - Self { - verbosity: 0, - test_registry: false, - use_temp_index: false, - check_token: None, - } - } -} - #[derive(Debug, StructOpt)] pub enum Subcommand { Init(InitSubcommand), diff --git a/src/commands/package.rs b/src/commands/package.rs index 6386c088..4eead2fd 100644 --- a/src/commands/package.rs +++ b/src/commands/package.rs @@ -32,7 +32,7 @@ impl PackageSubcommand { } } else { let contents = PackageContents::pack_from_path(&self.project_path)?; - fs_err::write(&self.output_path.unwrap(), contents.data())?; + fs_err::write(self.output_path.unwrap(), contents.data())?; } Ok(()) diff --git a/src/git_util.rs b/src/git_util.rs index f0ab3ae4..87a7f615 100644 --- a/src/git_util.rs +++ b/src/git_util.rs @@ -31,13 +31,11 @@ fn make_credentials_callback( if let Some(token) = &access_token { if !token_tried { token_tried = true; - return Cred::userpass_plaintext(&token, ""); - } - } else { - if !cred_helper_tried { - cred_helper_tried = true; - return Cred::credential_helper(config, url, username); + return Cred::userpass_plaintext(token, ""); } + } else if !cred_helper_tried { + cred_helper_tried = true; + return Cred::credential_helper(config, url, username); } } @@ -142,7 +140,7 @@ pub fn commit_and_push( let git_config = git2::Config::open_default()?; // libgit2 only accepts a relative path - let relative_path = modified_file.strip_prefix(&index_path).with_context(|| { + let relative_path = modified_file.strip_prefix(index_path).with_context(|| { format!( "Path {} was not relative to package path {}", modified_file.display(), @@ -161,7 +159,7 @@ pub fn commit_and_push( let head = repository.head()?; let parent = repository.find_commit(head.target().unwrap())?; let sig = git2::Signature::now("PackageUser", "PackageUser@localhost")?; - repository.commit(Some("HEAD"), &sig, &sig, &message, &tree, &[&parent])?; + repository.commit(Some("HEAD"), &sig, &sig, message, &tree, &[&parent])?; // git push let mut ref_status = Ok(()); @@ -202,7 +200,7 @@ pub fn update_index(access_token: Option, repository: &Repository) -> an repository .find_remote("origin")? .fetch(&["main"], Some(&mut fetch_options), None) - .with_context(|| format!("could not fetch Git repository"))?; + .with_context(|| "could not fetch Git repository".to_string())?; let mut options = git2::build::CheckoutBuilder::new(); options.force(); @@ -215,7 +213,7 @@ pub fn update_index(access_token: Option, repository: &Repository) -> an git2::ResetType::Hard, Some(&mut options), ) - .with_context(|| format!("could not reset git repo to fetch_head"))?; + .with_context(|| "could not reset git repo to fetch_head".to_string())?; Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index cab640cd..cae5074d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ pub mod auth; pub mod commands; pub mod git_util; pub mod installation; +pub mod link_mutator; pub mod lockfile; pub mod manifest; pub mod package_contents; @@ -10,10 +11,9 @@ pub mod package_index; pub mod package_name; pub mod package_req; pub mod package_source; +pub mod require_parser; pub mod resolution; pub mod test_package; -pub mod require_parser; -pub mod link_mutator; pub mod type_reexports; pub use commands::*; diff --git a/src/link_mutator.rs b/src/link_mutator.rs index f6650a20..5e66aeaa 100644 --- a/src/link_mutator.rs +++ b/src/link_mutator.rs @@ -200,7 +200,7 @@ fn create_return_require_variable() -> (LastStmt, Option) { } pub enum MutateLinkResult { - Changed(Ast), + Changed(Box), Unchanged, } @@ -225,7 +225,9 @@ pub fn mutate_link( .collect(), ) .with_last_stmt(Some(create_return_require_variable())); - Ok(MutateLinkResult::Changed(parsed_code.with_nodes(new_nodes))) + Ok(MutateLinkResult::Changed(Box::new( + parsed_code.with_nodes(new_nodes), + ))) } #[cfg(test)] @@ -285,4 +287,4 @@ mod tests { "export type Value = REQUIRED_MODULE.Value" ); } -} \ No newline at end of file +} diff --git a/src/lockfile.rs b/src/lockfile.rs index cff9ed2a..be81f5cc 100644 --- a/src/lockfile.rs +++ b/src/lockfile.rs @@ -39,7 +39,7 @@ fn grab_dependencies( .map(|(key, value)| (key.clone(), value.clone())) .collect() }) - .unwrap_or_else(Vec::new) + .unwrap_or_default() } impl Lockfile { @@ -55,9 +55,9 @@ impl Lockfile { for package_id in &resolve.activated { let dependencies = [ - grab_dependencies(&package_id, &resolve.shared_dependencies), - grab_dependencies(&package_id, &resolve.server_dependencies), - grab_dependencies(&package_id, &resolve.dev_dependencies), + grab_dependencies(package_id, &resolve.shared_dependencies), + grab_dependencies(package_id, &resolve.server_dependencies), + grab_dependencies(package_id, &resolve.dev_dependencies), ] .concat(); @@ -97,7 +97,7 @@ impl Lockfile { writeln!(file, "# This file is automatically @generated by Wally.")?; writeln!(file, "# It is not intended for manual editing.")?; writeln!(file, "registry = \"{}\"", self.registry)?; - writeln!(file, "")?; + writeln!(file)?; for lock_package in self.packages.iter() { writeln!(file, "[[package]]")?; @@ -111,7 +111,7 @@ impl Lockfile { writeln!(file, "checksum = \"{}\"", checksum)?; } - if registry_lock_package.dependencies.len() == 0 { + if registry_lock_package.dependencies.is_empty() { writeln!(file, "dependencies = []")?; } else { writeln!(file, "dependencies = [")?; @@ -126,7 +126,7 @@ impl Lockfile { writeln!(file, "rev = \"{}\"", git_lock_package.rev)?; writeln!(file, "commit = \"{}\"", git_lock_package.commit)?; - if git_lock_package.dependencies.len() == 0 { + if git_lock_package.dependencies.is_empty() { writeln!(file, "dependencies = []")?; } else { writeln!(file, "dependencies = [")?; @@ -138,7 +138,7 @@ impl Lockfile { } } - writeln!(file, "")?; + writeln!(file)?; } file.flush()?; diff --git a/src/manifest.rs b/src/manifest.rs index dc25085a..48a80390 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -43,9 +43,7 @@ impl Manifest { } pub fn from_slice(slice: &[u8]) -> anyhow::Result { - let manifest: Manifest = - toml::from_slice(slice).with_context(|| format!("failed to parse manifest"))?; - + let manifest: Manifest = toml::from_slice(slice).context("failed to parse manifest")?; Ok(manifest) } @@ -142,7 +140,7 @@ pub struct Package { // Metadata we require when this manifest will be used to generate package folders // This information can be present in any package but is only used in the root package -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] #[serde(rename_all = "kebab-case")] pub struct PlaceInfo { /// Where the shared packages folder is located in the Roblox Datamodel @@ -158,15 +156,6 @@ pub struct PlaceInfo { pub server_packages: Option, } -impl Default for PlaceInfo { - fn default() -> Self { - Self { - shared_packages: None, - server_packages: None, - } - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum Realm { diff --git a/src/package_contents.rs b/src/package_contents.rs index da2250d0..f3c60f0a 100644 --- a/src/package_contents.rs +++ b/src/package_contents.rs @@ -101,12 +101,9 @@ impl PackageContents { if includes.is_empty() && Path::new(".gitignore").exists() { let gitignore = File::open(Path::new(".gitignore"))?; - BufReader::new(gitignore) - .lines() - .flatten() - .for_each(|pattern| { - excludes.push(pattern); - }); + for pattern in BufReader::new(gitignore).lines().map_while(Result::ok) { + excludes.push(pattern); + } } EXCLUDED_GLOBS diff --git a/src/package_index.rs b/src/package_index.rs index 12f49e71..142ec196 100644 --- a/src/package_index.rs +++ b/src/package_index.rs @@ -102,7 +102,7 @@ impl PackageIndex { repository.find_remote("origin")?.url().unwrap() ); git_util::update_index(self.access_token.clone(), &repository) - .with_context(|| format!("could not update package index"))?; + .context("could not update package index")?; Ok(()) } @@ -230,7 +230,7 @@ impl PackageIndex { path.push("owners.json"); { - let mut owners = self.get_scope_owners(&scope)?; + let mut owners = self.get_scope_owners(scope)?; let mut file = OpenOptions::new().write(true).create(true).open(&path)?; owners.push(*owner_id); diff --git a/src/package_name.rs b/src/package_name.rs index 33d674dd..3bbe3e83 100644 --- a/src/package_name.rs +++ b/src/package_name.rs @@ -57,7 +57,7 @@ fn validate_scope(scope: &str) -> anyhow::Result<()> { '-')", scope ); - ensure!(scope.len() > 0, "package scopes cannot be empty"); + ensure!(!scope.is_empty(), "package scopes cannot be empty"); ensure!( scope.len() <= 64, "package scopes cannot exceed 64 characters in length" @@ -77,7 +77,7 @@ fn validate_name(name: &str) -> anyhow::Result<()> { '-')", name ); - ensure!(name.len() > 0, "package names cannot be empty"); + ensure!(!name.is_empty(), "package names cannot be empty"); ensure!( name.len() <= 64, "package names cannot exceed 64 characters in length" diff --git a/src/package_req.rs b/src/package_req.rs index 234d5465..31ceac5a 100644 --- a/src/package_req.rs +++ b/src/package_req.rs @@ -70,7 +70,7 @@ impl FromStr for PackageReq { // so let's check for that here. // // https://github.com/steveklabnik/semver-parser/issues/51 - if version_req_source.len() == 0 || version_req_source.chars().all(char::is_whitespace) { + if version_req_source.is_empty() || version_req_source.chars().all(char::is_whitespace) { bail!(BAD_FORMAT_MSG); } diff --git a/src/package_source/in_memory.rs b/src/package_source/in_memory.rs index e031503e..801c6942 100644 --- a/src/package_source/in_memory.rs +++ b/src/package_source/in_memory.rs @@ -54,6 +54,12 @@ impl InMemoryRegistry { } } +impl Default for InMemoryRegistry { + fn default() -> Self { + Self::new() + } +} + /// Returned by `InMemoryRegistry::source` and can be passed to package /// resolution code in order to tell it to use this package registry. #[derive(Clone)] @@ -120,7 +126,9 @@ struct PackageEntry { contents: PackageContents, } +type RegistryMap = HashMap>>; + #[derive(Clone, Default)] struct Storage { - contents: Arc>>>>, + contents: Arc>, } diff --git a/src/package_source/registry.rs b/src/package_source/registry.rs index ab76dea3..cd8f192a 100644 --- a/src/package_source/registry.rs +++ b/src/package_source/registry.rs @@ -32,7 +32,7 @@ impl Registry { let index_url = Url::parse(spec)?; // Force Rustls to avoid Windows schannel/native-tls issues let client = Client::builder().use_rustls_tls().build()?; - + Ok(Self { index_url, auth_token: OnceCell::new(), @@ -47,7 +47,7 @@ impl Registry { Some(token) => Ok(Some(Arc::from(token.as_str()))), None => Ok(None), }) - .map(|token| token.clone()) + .cloned() } fn index(&self) -> anyhow::Result<&Arc> { diff --git a/src/require_parser.rs b/src/require_parser.rs index b87c7a23..d7c21c62 100644 --- a/src/require_parser.rs +++ b/src/require_parser.rs @@ -102,4 +102,4 @@ mod tests { fn unhandled_require() { assert!(match_require(&require_expression("require('string')")).is_err()) } -} \ No newline at end of file +} diff --git a/src/test_package.rs b/src/test_package.rs index d7284d69..41d6d32f 100644 --- a/src/test_package.rs +++ b/src/test_package.rs @@ -116,8 +116,7 @@ impl PackageBuilder { archive.finish().unwrap(); drop(archive); - let contents = PackageContents::from_buffer(buffer); - contents + PackageContents::from_buffer(buffer) } pub fn package(self) -> (Manifest, PackageContents) { diff --git a/src/type_reexports.rs b/src/type_reexports.rs index 4b4432ce..2e04ba4e 100644 --- a/src/type_reexports.rs +++ b/src/type_reexports.rs @@ -5,9 +5,12 @@ use std::path::{Path, PathBuf}; use crate::require_parser::*; - // Filesystem-based resolver -fn resolve_components_fs(link_path: &Path, packages_root: &Path, path_components: &[String]) -> Option { +fn resolve_components_fs( + link_path: &Path, + packages_root: &Path, + path_components: &[String], +) -> Option { if path_components.is_empty() { return None; } @@ -104,7 +107,9 @@ fn mutate_thunk_fs(path: &Path, packages_root: &Path) -> Result<()> { // Skip already-mutated thunks let original_src = std::fs::read_to_string(path)?; - if original_src.contains("local REQUIRED_MODULE") || original_src.contains("return REQUIRED_MODULE") { + if original_src.contains("local REQUIRED_MODULE") + || original_src.contains("return REQUIRED_MODULE") + { info!("Link already mutated, leaving unchanged"); return Ok(()); } @@ -150,10 +155,10 @@ fn mutate_thunk_fs(path: &Path, packages_root: &Path) -> Result<()> { if !trimmed.starts_with("export type ") { continue; } - + // Parse "export type Name<...> = ..." let rest = &trimmed["export type ".len()..]; - + // Extract type name and optional generics by locating the first '<' ... '>' pair. let (type_name, generics_opt) = if let Some(lt_rel) = rest.find('<') { let name = rest[..lt_rel].trim(); @@ -170,20 +175,20 @@ fn mutate_thunk_fs(path: &Path, packages_root: &Path) -> Result<()> { let name_end = rest.find('=').unwrap_or(rest.len()); (rest[..name_end].trim(), None) }; - + // Derive list of generic parameter identifiers without defaults. // Keep variadic packs by preserving the trailing "...". let mut rhs_generics: Vec = Vec::new(); if let Some(generics) = generics_opt { for raw in generics.split(',') { // Strip default, e.g., "S = T" -> "S" and preserve "..." - let name_only = raw.splitn(2, '=').next().unwrap().trim(); + let name_only = raw.split('=').next().unwrap().trim(); if !name_only.is_empty() { rhs_generics.push(name_only.to_string()); } } } - + // Rebuild generics for LHS (keep original) and RHS (names only) let lhs_suffix = if let Some(generics) = generics_opt { format!("<{}>", generics) @@ -195,7 +200,7 @@ fn mutate_thunk_fs(path: &Path, packages_root: &Path) -> Result<()> { } else { format!("<{}>", rhs_generics.join(", ")) }; - + reexports.push(format!( "export type {}{} = REQUIRED_MODULE.{}{}", type_name, lhs_suffix, type_name, rhs_suffix @@ -307,4 +312,4 @@ pub fn run_on_packages_fs(packages_root: &Path) -> Result<()> { } else { bail!("Mutation did not complete successfully") } -} \ No newline at end of file +} diff --git a/wally-registry-backend/src/main.rs b/wally-registry-backend/src/main.rs index 07ca2c6b..f29db9c8 100644 --- a/wally-registry-backend/src/main.rs +++ b/wally-registry-backend/src/main.rs @@ -155,7 +155,7 @@ async fn publish( let manifest = get_manifest(&mut archive).status(Status::BadRequest)?; let package_id = manifest.package_id(); - if !authorization.can_write_package(&package_id, &index)? { + if !authorization.can_write_package(&package_id, index)? { return Err(format_err!( "you do not have permission to write in scope {}", package_id.name().scope() @@ -195,7 +195,7 @@ async fn publish( if let Ok(mut search_backend) = search_backend.try_write() { // TODO: Recrawling the whole index for each publish is very wasteful! // Eventually this will get too expensive and we should only add the new package. - search_backend.crawl_packages(&index)?; + search_backend.crawl_packages(index)?; } Ok(Json(json!({ diff --git a/wally-registry-backend/src/search.rs b/wally-registry-backend/src/search.rs index 02917c25..4d68482c 100644 --- a/wally-registry-backend/src/search.rs +++ b/wally-registry-backend/src/search.rs @@ -106,7 +106,7 @@ impl SearchBackend { let mut doc = Document::default(); - for manifest in &(*metadata).versions { + for manifest in &metadata.versions { doc.add_text(versions, manifest.package.version.to_string()); if !manifest.package.version.is_prerelease() { From c94bf7439b76fbcb78b14cc837bf6057aef3771a Mon Sep 17 00:00:00 2001 From: waffle Date: Thu, 18 Sep 2025 01:52:45 -0400 Subject: [PATCH 3/4] Revert "satisfy cargo clippy" This reverts commit 20b7180b82c6fc27e3dcb5d97866c801d00df2f8. --- src/auth.rs | 4 ++-- src/commands/mod.rs | 13 ++++++++++++- src/commands/package.rs | 2 +- src/git_util.rs | 18 ++++++++++-------- src/lib.rs | 4 ++-- src/link_mutator.rs | 8 +++----- src/lockfile.rs | 16 ++++++++-------- src/manifest.rs | 15 +++++++++++++-- src/package_contents.rs | 9 ++++++--- src/package_index.rs | 4 ++-- src/package_name.rs | 4 ++-- src/package_req.rs | 2 +- src/package_source/in_memory.rs | 10 +--------- src/package_source/registry.rs | 4 ++-- src/require_parser.rs | 2 +- src/test_package.rs | 3 ++- src/type_reexports.rs | 25 ++++++++++--------------- wally-registry-backend/src/main.rs | 4 ++-- wally-registry-backend/src/search.rs | 2 +- 19 files changed, 81 insertions(+), 68 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 9704b3a1..72b30308 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -70,13 +70,13 @@ impl AuthStore { } fn contents(path: &Path) -> anyhow::Result { - match fs_err::read_to_string(path) { + match fs_err::read_to_string(&path) { Ok(contents) => Ok(contents), Err(err) => { if err.kind() == io::ErrorKind::NotFound { Ok(DEFAULT_AUTH_TOML.to_owned()) } else { - Err(err.into()) + return Err(err.into()); } } } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 1f14c5a3..1af65781 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -48,7 +48,7 @@ impl Args { } /// Options that apply to all subcommands for the CLI. -#[derive(Debug, StructOpt, Default)] +#[derive(Debug, StructOpt)] pub struct GlobalOptions { /// Enable more verbose logging. Can be specified multiple times to increase /// verbosity further. @@ -68,6 +68,17 @@ pub struct GlobalOptions { pub check_token: Option, } +impl Default for GlobalOptions { + fn default() -> Self { + Self { + verbosity: 0, + test_registry: false, + use_temp_index: false, + check_token: None, + } + } +} + #[derive(Debug, StructOpt)] pub enum Subcommand { Init(InitSubcommand), diff --git a/src/commands/package.rs b/src/commands/package.rs index 4eead2fd..6386c088 100644 --- a/src/commands/package.rs +++ b/src/commands/package.rs @@ -32,7 +32,7 @@ impl PackageSubcommand { } } else { let contents = PackageContents::pack_from_path(&self.project_path)?; - fs_err::write(self.output_path.unwrap(), contents.data())?; + fs_err::write(&self.output_path.unwrap(), contents.data())?; } Ok(()) diff --git a/src/git_util.rs b/src/git_util.rs index 87a7f615..f0ab3ae4 100644 --- a/src/git_util.rs +++ b/src/git_util.rs @@ -31,11 +31,13 @@ fn make_credentials_callback( if let Some(token) = &access_token { if !token_tried { token_tried = true; - return Cred::userpass_plaintext(token, ""); + return Cred::userpass_plaintext(&token, ""); + } + } else { + if !cred_helper_tried { + cred_helper_tried = true; + return Cred::credential_helper(config, url, username); } - } else if !cred_helper_tried { - cred_helper_tried = true; - return Cred::credential_helper(config, url, username); } } @@ -140,7 +142,7 @@ pub fn commit_and_push( let git_config = git2::Config::open_default()?; // libgit2 only accepts a relative path - let relative_path = modified_file.strip_prefix(index_path).with_context(|| { + let relative_path = modified_file.strip_prefix(&index_path).with_context(|| { format!( "Path {} was not relative to package path {}", modified_file.display(), @@ -159,7 +161,7 @@ pub fn commit_and_push( let head = repository.head()?; let parent = repository.find_commit(head.target().unwrap())?; let sig = git2::Signature::now("PackageUser", "PackageUser@localhost")?; - repository.commit(Some("HEAD"), &sig, &sig, message, &tree, &[&parent])?; + repository.commit(Some("HEAD"), &sig, &sig, &message, &tree, &[&parent])?; // git push let mut ref_status = Ok(()); @@ -200,7 +202,7 @@ pub fn update_index(access_token: Option, repository: &Repository) -> an repository .find_remote("origin")? .fetch(&["main"], Some(&mut fetch_options), None) - .with_context(|| "could not fetch Git repository".to_string())?; + .with_context(|| format!("could not fetch Git repository"))?; let mut options = git2::build::CheckoutBuilder::new(); options.force(); @@ -213,7 +215,7 @@ pub fn update_index(access_token: Option, repository: &Repository) -> an git2::ResetType::Hard, Some(&mut options), ) - .with_context(|| "could not reset git repo to fetch_head".to_string())?; + .with_context(|| format!("could not reset git repo to fetch_head"))?; Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index cae5074d..cab640cd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,7 +2,6 @@ pub mod auth; pub mod commands; pub mod git_util; pub mod installation; -pub mod link_mutator; pub mod lockfile; pub mod manifest; pub mod package_contents; @@ -11,9 +10,10 @@ pub mod package_index; pub mod package_name; pub mod package_req; pub mod package_source; -pub mod require_parser; pub mod resolution; pub mod test_package; +pub mod require_parser; +pub mod link_mutator; pub mod type_reexports; pub use commands::*; diff --git a/src/link_mutator.rs b/src/link_mutator.rs index 5e66aeaa..f6650a20 100644 --- a/src/link_mutator.rs +++ b/src/link_mutator.rs @@ -200,7 +200,7 @@ fn create_return_require_variable() -> (LastStmt, Option) { } pub enum MutateLinkResult { - Changed(Box), + Changed(Ast), Unchanged, } @@ -225,9 +225,7 @@ pub fn mutate_link( .collect(), ) .with_last_stmt(Some(create_return_require_variable())); - Ok(MutateLinkResult::Changed(Box::new( - parsed_code.with_nodes(new_nodes), - ))) + Ok(MutateLinkResult::Changed(parsed_code.with_nodes(new_nodes))) } #[cfg(test)] @@ -287,4 +285,4 @@ mod tests { "export type Value = REQUIRED_MODULE.Value" ); } -} +} \ No newline at end of file diff --git a/src/lockfile.rs b/src/lockfile.rs index be81f5cc..cff9ed2a 100644 --- a/src/lockfile.rs +++ b/src/lockfile.rs @@ -39,7 +39,7 @@ fn grab_dependencies( .map(|(key, value)| (key.clone(), value.clone())) .collect() }) - .unwrap_or_default() + .unwrap_or_else(Vec::new) } impl Lockfile { @@ -55,9 +55,9 @@ impl Lockfile { for package_id in &resolve.activated { let dependencies = [ - grab_dependencies(package_id, &resolve.shared_dependencies), - grab_dependencies(package_id, &resolve.server_dependencies), - grab_dependencies(package_id, &resolve.dev_dependencies), + grab_dependencies(&package_id, &resolve.shared_dependencies), + grab_dependencies(&package_id, &resolve.server_dependencies), + grab_dependencies(&package_id, &resolve.dev_dependencies), ] .concat(); @@ -97,7 +97,7 @@ impl Lockfile { writeln!(file, "# This file is automatically @generated by Wally.")?; writeln!(file, "# It is not intended for manual editing.")?; writeln!(file, "registry = \"{}\"", self.registry)?; - writeln!(file)?; + writeln!(file, "")?; for lock_package in self.packages.iter() { writeln!(file, "[[package]]")?; @@ -111,7 +111,7 @@ impl Lockfile { writeln!(file, "checksum = \"{}\"", checksum)?; } - if registry_lock_package.dependencies.is_empty() { + if registry_lock_package.dependencies.len() == 0 { writeln!(file, "dependencies = []")?; } else { writeln!(file, "dependencies = [")?; @@ -126,7 +126,7 @@ impl Lockfile { writeln!(file, "rev = \"{}\"", git_lock_package.rev)?; writeln!(file, "commit = \"{}\"", git_lock_package.commit)?; - if git_lock_package.dependencies.is_empty() { + if git_lock_package.dependencies.len() == 0 { writeln!(file, "dependencies = []")?; } else { writeln!(file, "dependencies = [")?; @@ -138,7 +138,7 @@ impl Lockfile { } } - writeln!(file)?; + writeln!(file, "")?; } file.flush()?; diff --git a/src/manifest.rs b/src/manifest.rs index 48a80390..dc25085a 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -43,7 +43,9 @@ impl Manifest { } pub fn from_slice(slice: &[u8]) -> anyhow::Result { - let manifest: Manifest = toml::from_slice(slice).context("failed to parse manifest")?; + let manifest: Manifest = + toml::from_slice(slice).with_context(|| format!("failed to parse manifest"))?; + Ok(manifest) } @@ -140,7 +142,7 @@ pub struct Package { // Metadata we require when this manifest will be used to generate package folders // This information can be present in any package but is only used in the root package -#[derive(Debug, Clone, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct PlaceInfo { /// Where the shared packages folder is located in the Roblox Datamodel @@ -156,6 +158,15 @@ pub struct PlaceInfo { pub server_packages: Option, } +impl Default for PlaceInfo { + fn default() -> Self { + Self { + shared_packages: None, + server_packages: None, + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum Realm { diff --git a/src/package_contents.rs b/src/package_contents.rs index f3c60f0a..da2250d0 100644 --- a/src/package_contents.rs +++ b/src/package_contents.rs @@ -101,9 +101,12 @@ impl PackageContents { if includes.is_empty() && Path::new(".gitignore").exists() { let gitignore = File::open(Path::new(".gitignore"))?; - for pattern in BufReader::new(gitignore).lines().map_while(Result::ok) { - excludes.push(pattern); - } + BufReader::new(gitignore) + .lines() + .flatten() + .for_each(|pattern| { + excludes.push(pattern); + }); } EXCLUDED_GLOBS diff --git a/src/package_index.rs b/src/package_index.rs index 142ec196..12f49e71 100644 --- a/src/package_index.rs +++ b/src/package_index.rs @@ -102,7 +102,7 @@ impl PackageIndex { repository.find_remote("origin")?.url().unwrap() ); git_util::update_index(self.access_token.clone(), &repository) - .context("could not update package index")?; + .with_context(|| format!("could not update package index"))?; Ok(()) } @@ -230,7 +230,7 @@ impl PackageIndex { path.push("owners.json"); { - let mut owners = self.get_scope_owners(scope)?; + let mut owners = self.get_scope_owners(&scope)?; let mut file = OpenOptions::new().write(true).create(true).open(&path)?; owners.push(*owner_id); diff --git a/src/package_name.rs b/src/package_name.rs index 3bbe3e83..33d674dd 100644 --- a/src/package_name.rs +++ b/src/package_name.rs @@ -57,7 +57,7 @@ fn validate_scope(scope: &str) -> anyhow::Result<()> { '-')", scope ); - ensure!(!scope.is_empty(), "package scopes cannot be empty"); + ensure!(scope.len() > 0, "package scopes cannot be empty"); ensure!( scope.len() <= 64, "package scopes cannot exceed 64 characters in length" @@ -77,7 +77,7 @@ fn validate_name(name: &str) -> anyhow::Result<()> { '-')", name ); - ensure!(!name.is_empty(), "package names cannot be empty"); + ensure!(name.len() > 0, "package names cannot be empty"); ensure!( name.len() <= 64, "package names cannot exceed 64 characters in length" diff --git a/src/package_req.rs b/src/package_req.rs index 31ceac5a..234d5465 100644 --- a/src/package_req.rs +++ b/src/package_req.rs @@ -70,7 +70,7 @@ impl FromStr for PackageReq { // so let's check for that here. // // https://github.com/steveklabnik/semver-parser/issues/51 - if version_req_source.is_empty() || version_req_source.chars().all(char::is_whitespace) { + if version_req_source.len() == 0 || version_req_source.chars().all(char::is_whitespace) { bail!(BAD_FORMAT_MSG); } diff --git a/src/package_source/in_memory.rs b/src/package_source/in_memory.rs index 801c6942..e031503e 100644 --- a/src/package_source/in_memory.rs +++ b/src/package_source/in_memory.rs @@ -54,12 +54,6 @@ impl InMemoryRegistry { } } -impl Default for InMemoryRegistry { - fn default() -> Self { - Self::new() - } -} - /// Returned by `InMemoryRegistry::source` and can be passed to package /// resolution code in order to tell it to use this package registry. #[derive(Clone)] @@ -126,9 +120,7 @@ struct PackageEntry { contents: PackageContents, } -type RegistryMap = HashMap>>; - #[derive(Clone, Default)] struct Storage { - contents: Arc>, + contents: Arc>>>>, } diff --git a/src/package_source/registry.rs b/src/package_source/registry.rs index cd8f192a..ab76dea3 100644 --- a/src/package_source/registry.rs +++ b/src/package_source/registry.rs @@ -32,7 +32,7 @@ impl Registry { let index_url = Url::parse(spec)?; // Force Rustls to avoid Windows schannel/native-tls issues let client = Client::builder().use_rustls_tls().build()?; - + Ok(Self { index_url, auth_token: OnceCell::new(), @@ -47,7 +47,7 @@ impl Registry { Some(token) => Ok(Some(Arc::from(token.as_str()))), None => Ok(None), }) - .cloned() + .map(|token| token.clone()) } fn index(&self) -> anyhow::Result<&Arc> { diff --git a/src/require_parser.rs b/src/require_parser.rs index d7c21c62..b87c7a23 100644 --- a/src/require_parser.rs +++ b/src/require_parser.rs @@ -102,4 +102,4 @@ mod tests { fn unhandled_require() { assert!(match_require(&require_expression("require('string')")).is_err()) } -} +} \ No newline at end of file diff --git a/src/test_package.rs b/src/test_package.rs index 41d6d32f..d7284d69 100644 --- a/src/test_package.rs +++ b/src/test_package.rs @@ -116,7 +116,8 @@ impl PackageBuilder { archive.finish().unwrap(); drop(archive); - PackageContents::from_buffer(buffer) + let contents = PackageContents::from_buffer(buffer); + contents } pub fn package(self) -> (Manifest, PackageContents) { diff --git a/src/type_reexports.rs b/src/type_reexports.rs index 2e04ba4e..4b4432ce 100644 --- a/src/type_reexports.rs +++ b/src/type_reexports.rs @@ -5,12 +5,9 @@ use std::path::{Path, PathBuf}; use crate::require_parser::*; + // Filesystem-based resolver -fn resolve_components_fs( - link_path: &Path, - packages_root: &Path, - path_components: &[String], -) -> Option { +fn resolve_components_fs(link_path: &Path, packages_root: &Path, path_components: &[String]) -> Option { if path_components.is_empty() { return None; } @@ -107,9 +104,7 @@ fn mutate_thunk_fs(path: &Path, packages_root: &Path) -> Result<()> { // Skip already-mutated thunks let original_src = std::fs::read_to_string(path)?; - if original_src.contains("local REQUIRED_MODULE") - || original_src.contains("return REQUIRED_MODULE") - { + if original_src.contains("local REQUIRED_MODULE") || original_src.contains("return REQUIRED_MODULE") { info!("Link already mutated, leaving unchanged"); return Ok(()); } @@ -155,10 +150,10 @@ fn mutate_thunk_fs(path: &Path, packages_root: &Path) -> Result<()> { if !trimmed.starts_with("export type ") { continue; } - + // Parse "export type Name<...> = ..." let rest = &trimmed["export type ".len()..]; - + // Extract type name and optional generics by locating the first '<' ... '>' pair. let (type_name, generics_opt) = if let Some(lt_rel) = rest.find('<') { let name = rest[..lt_rel].trim(); @@ -175,20 +170,20 @@ fn mutate_thunk_fs(path: &Path, packages_root: &Path) -> Result<()> { let name_end = rest.find('=').unwrap_or(rest.len()); (rest[..name_end].trim(), None) }; - + // Derive list of generic parameter identifiers without defaults. // Keep variadic packs by preserving the trailing "...". let mut rhs_generics: Vec = Vec::new(); if let Some(generics) = generics_opt { for raw in generics.split(',') { // Strip default, e.g., "S = T" -> "S" and preserve "..." - let name_only = raw.split('=').next().unwrap().trim(); + let name_only = raw.splitn(2, '=').next().unwrap().trim(); if !name_only.is_empty() { rhs_generics.push(name_only.to_string()); } } } - + // Rebuild generics for LHS (keep original) and RHS (names only) let lhs_suffix = if let Some(generics) = generics_opt { format!("<{}>", generics) @@ -200,7 +195,7 @@ fn mutate_thunk_fs(path: &Path, packages_root: &Path) -> Result<()> { } else { format!("<{}>", rhs_generics.join(", ")) }; - + reexports.push(format!( "export type {}{} = REQUIRED_MODULE.{}{}", type_name, lhs_suffix, type_name, rhs_suffix @@ -312,4 +307,4 @@ pub fn run_on_packages_fs(packages_root: &Path) -> Result<()> { } else { bail!("Mutation did not complete successfully") } -} +} \ No newline at end of file diff --git a/wally-registry-backend/src/main.rs b/wally-registry-backend/src/main.rs index f29db9c8..07ca2c6b 100644 --- a/wally-registry-backend/src/main.rs +++ b/wally-registry-backend/src/main.rs @@ -155,7 +155,7 @@ async fn publish( let manifest = get_manifest(&mut archive).status(Status::BadRequest)?; let package_id = manifest.package_id(); - if !authorization.can_write_package(&package_id, index)? { + if !authorization.can_write_package(&package_id, &index)? { return Err(format_err!( "you do not have permission to write in scope {}", package_id.name().scope() @@ -195,7 +195,7 @@ async fn publish( if let Ok(mut search_backend) = search_backend.try_write() { // TODO: Recrawling the whole index for each publish is very wasteful! // Eventually this will get too expensive and we should only add the new package. - search_backend.crawl_packages(index)?; + search_backend.crawl_packages(&index)?; } Ok(Json(json!({ diff --git a/wally-registry-backend/src/search.rs b/wally-registry-backend/src/search.rs index 4d68482c..02917c25 100644 --- a/wally-registry-backend/src/search.rs +++ b/wally-registry-backend/src/search.rs @@ -106,7 +106,7 @@ impl SearchBackend { let mut doc = Document::default(); - for manifest in &metadata.versions { + for manifest in &(*metadata).versions { doc.add_text(versions, manifest.package.version.to_string()); if !manifest.package.version.is_prerelease() { From c100027c60ef2b1b01a258e1fa1e7dca7876f596 Mon Sep 17 00:00:00 2001 From: waffle Date: Thu, 18 Sep 2025 02:00:05 -0400 Subject: [PATCH 4/4] Inline type re-exports at link write remove post-pass traversal and AST plumbing, drop full_moon, add integration test for typed re-exports --- Cargo.lock | 80 --------- Cargo.toml | 1 - src/installation.rs | 187 +++++++++++++++++---- src/lib.rs | 3 - src/link_mutator.rs | 288 -------------------------------- src/require_parser.rs | 105 ------------ src/type_reexports.rs | 310 ----------------------------------- tests/integration/install.rs | 124 ++++++++++++++ 8 files changed, 276 insertions(+), 822 deletions(-) delete mode 100644 src/link_mutator.rs delete mode 100644 src/require_parser.rs delete mode 100644 src/type_reexports.rs diff --git a/Cargo.lock b/Cargo.lock index f2f840a2..a4e5345c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -218,15 +218,6 @@ dependencies = [ "generic-array", ] -[[package]] -name = "borsh" -version = "1.5.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ad8646f98db542e39fc66e68a20b2144f6a732636df7c2354e74645faaa433ce" -dependencies = [ - "cfg_aliases", -] - [[package]] name = "bstr" version = "0.2.15" @@ -341,12 +332,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" -[[package]] -name = "cfg_aliases" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" - [[package]] name = "chrono" version = "0.4.19" @@ -673,27 +658,6 @@ dependencies = [ "powerfmt", ] -[[package]] -name = "derive_more" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4a9b99b9cbbe49445b21764dc0625032a89b145a2642e67603e1c936f5458d05" -dependencies = [ - "derive_more-impl", -] - -[[package]] -name = "derive_more-impl" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb7330aeadfbe296029522e6c40f315320aba36fc43a5b3632f3795348f3bd22" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.87", - "unicode-xid", -] - [[package]] name = "devise" version = "0.4.1" @@ -959,33 +923,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "full_moon" -version = "1.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a02c056a5966f6db8e663ee48093280f098d68d97763a94490ee63178d54776" -dependencies = [ - "bytecount", - "cfg-if 1.0.0", - "derive_more", - "full_moon_derive", - "paste", - "serde", - "smol_str", -] - -[[package]] -name = "full_moon_derive" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "99b4bd12ce56927d1dc5478d21528ea8c4b93ca85ff8f8043b6a5351a2a3c6f7" -dependencies = [ - "indexmap 1.9.3", - "proc-macro2", - "quote", - "syn 1.0.67", -] - [[package]] name = "futures" version = "0.3.17" @@ -1997,12 +1934,6 @@ dependencies = [ "windows-sys 0.45.0", ] -[[package]] -name = "paste" -version = "1.0.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" - [[package]] name = "pear" version = "0.2.4" @@ -3041,16 +2972,6 @@ version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a507befe795404456341dfab10cef66ead4c041f62b8b11bbb92bffe5d0953e0" -[[package]] -name = "smol_str" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9676b89cd56310a87b93dec47b11af744f34d5fc9f367b829474eec0a891350d" -dependencies = [ - "borsh", - "serde", -] - [[package]] name = "socket2" version = "0.4.9" @@ -3795,7 +3716,6 @@ dependencies = [ "dirs", "env_logger", "fs-err", - "full_moon", "git2", "globset", "hex", diff --git a/Cargo.toml b/Cargo.toml index da84f0af..60c5de7a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -59,7 +59,6 @@ indicatif = "0.17.4" tokio = "1.28.2" serial_test = "2.0.0" time = "=0.3.35" -full_moon = { version = "1.2.0", features = ["luau"] } [dev-dependencies] insta = { version = "1.1.0" } diff --git a/src/installation.rs b/src/installation.rs index 0868a948..6eb8d5e5 100644 --- a/src/installation.rs +++ b/src/installation.rs @@ -19,6 +19,59 @@ use crate::{ resolution::Resolve, }; +/// Extract type re-exports from module content +fn extract_type_reexports(content: &str) -> Vec { + let mut reexports = Vec::new(); + for line in content.lines() { + let trimmed = line.trim_start(); + if !trimmed.starts_with("export type ") { + continue; + } + + let rest = &trimmed["export type ".len()..]; + + let (type_name, generics_opt) = if let Some(lt_rel) = rest.find('<') { + let name = rest[..lt_rel].trim(); + let gt_rel = match rest[lt_rel + 1..].find('>') { + Some(i) => lt_rel + 1 + i, + None => continue, + }; + let generics = &rest[lt_rel + 1..gt_rel]; + (name, Some(generics.trim())) + } else { + let name_end = rest.find('=').unwrap_or(rest.len()); + (rest[..name_end].trim(), None) + }; + + let mut rhs_generics: Vec = Vec::new(); + if let Some(generics) = generics_opt { + for raw in generics.split(',') { + let name_only = raw.splitn(2, '=').next().unwrap().trim(); + if !name_only.is_empty() { + rhs_generics.push(name_only.to_string()); + } + } + } + + let lhs_suffix = if let Some(generics) = generics_opt { + format!("<{}>", generics) + } else { + String::new() + }; + let rhs_suffix = if rhs_generics.is_empty() { + String::new() + } else { + format!("<{}>", rhs_generics.join(", ")) + }; + + reexports.push(format!( + "export type {}{} = REQUIRED_MODULE.{}{}", + type_name, lhs_suffix, type_name, rhs_suffix + )); + } + reexports +} + #[derive(Clone)] pub struct InstallationContext { shared_dir: PathBuf, @@ -106,40 +159,15 @@ impl InstallationContext { for package_id in resolved_copy.activated { log::debug!("Installing {}...", package_id); - let shared_deps = resolved.shared_dependencies.get(&package_id); - let server_deps = resolved.server_dependencies.get(&package_id); - let dev_deps = resolved.dev_dependencies.get(&package_id); // We do not need to install the root package, but we should create // package links for its dependencies. if package_id == root_package_id { - if let Some(deps) = shared_deps { - self.write_root_package_links(Realm::Shared, deps, &resolved)?; - } - - if let Some(deps) = server_deps { - self.write_root_package_links(Realm::Server, deps, &resolved)?; - } - - if let Some(deps) = dev_deps { - self.write_root_package_links(Realm::Dev, deps, &resolved)?; - } + // Defer link writing until after all packages are unpacked } else { let metadata = resolved.metadata.get(&package_id).unwrap(); let package_realm = metadata.origin_realm; - if let Some(deps) = shared_deps { - self.write_package_links(&package_id, package_realm, deps, &resolved)?; - } - - if let Some(deps) = server_deps { - self.write_package_links(&package_id, package_realm, deps, &resolved)?; - } - - if let Some(deps) = dev_deps { - self.write_package_links(&package_id, package_realm, deps, &resolved)?; - } - let source_registry = resolved_copy.metadata[&package_id].source_registry.clone(); let source_copy = sources.clone(); let context = self.clone(); @@ -172,12 +200,33 @@ impl InstallationContext { bar.finish_and_clear(); - // Post-pass: filesystem-based traversal/mutation on installed links - for dir in [&self.shared_dir, &self.server_dir, &self.dev_dir] { - if dir.exists() { - if let Err(err) = crate::type_reexports::run_on_packages_fs(dir) { - log::warn!("Type re-export pass failed for {}: {}", dir.display(), err); - } + // After unpacking all package contents, write link thunks with type re-exports. + // Root package links + if let Some(deps) = resolved.shared_dependencies.get(&root_package_id) { + self.write_root_package_links(Realm::Shared, deps, &resolved)?; + } + if let Some(deps) = resolved.server_dependencies.get(&root_package_id) { + self.write_root_package_links(Realm::Server, deps, &resolved)?; + } + if let Some(deps) = resolved.dev_dependencies.get(&root_package_id) { + self.write_root_package_links(Realm::Dev, deps, &resolved)?; + } + + // Links for each non-root package + for (pkg_id, metadata) in &resolved.metadata { + if pkg_id == &root_package_id { + continue; + } + let package_realm = metadata.origin_realm; + + if let Some(deps) = resolved.shared_dependencies.get(pkg_id) { + self.write_package_links(pkg_id, package_realm, deps, &resolved)?; + } + if let Some(deps) = resolved.server_dependencies.get(pkg_id) { + self.write_package_links(pkg_id, package_realm, deps, &resolved)?; + } + if let Some(deps) = resolved.dev_dependencies.get(pkg_id) { + self.write_package_links(pkg_id, package_realm, deps, &resolved)?; } } @@ -239,7 +288,7 @@ impl InstallationContext { A dev dependency is depending on a server dependency. To link these packages correctly you must declare where server packages are placed in the roblox datamodel in your wally.toml. - + This typically looks like: [place] @@ -258,6 +307,68 @@ impl InstallationContext { Ok(contents) } + /// Generate thunk content with type re-exports for a package link + fn generate_thunk_with_types(&self, require_expr: &str, target_file_path: Option<&Path>) -> String { + // Accept either "require(...)" or "return require(...)" for convenience + let mut expr = require_expr.trim(); + if let Some(stripped) = expr.strip_prefix("return ") { + expr = stripped.trim_start(); + } + + // Collect re-exports, if any + let mut reexports: Vec = Vec::new(); + if let Some(target_path) = target_file_path { + if let Ok(target_content) = std::fs::read_to_string(target_path) { + reexports = extract_type_reexports(&target_content); + } + } + + // Preserve legacy one-liner if there are no exported types + if reexports.is_empty() { + // Always include trailing newline to keep snapshot output stable + format!("return {}\n", expr) + } else { + let mut content = format!("local REQUIRED_MODULE = {}\n", expr); + for reexport in reexports { + content.push_str(&reexport); + content.push('\n'); + } + content.push_str("return REQUIRED_MODULE\n"); + content + } + } + + /// Resolve the target entry module file for a package (no sourcemap, minimal checks) + fn resolve_package_file(&self, package_id: &PackageId, realm: Realm) -> Option { + let index_dir = match realm { + Realm::Shared => &self.shared_index_dir, + Realm::Server => &self.server_index_dir, + Realm::Dev => &self.dev_index_dir, + }; + + let package_dir = index_dir + .join(package_id_file_name(package_id)) + .join(package_id.name().name()); + + // Common layouts for Wally packages + let candidates = [ + package_dir.join(format!("{}.lua", package_id.name().name())), + package_dir.join(format!("{}.luau", package_id.name().name())), + package_dir.join("init.lua"), + package_dir.join("init.luau"), + package_dir.join("src").join("init.lua"), + package_dir.join("src").join("init.luau"), + ]; + + for cand in candidates { + if cand.exists() { + return Some(cand); + } + } + + None + } + fn write_root_package_links<'a, K: Display>( &self, root_realm: Realm, @@ -279,7 +390,7 @@ impl InstallationContext { let dependencies_realm = resolved.metadata.get(dep_package_id).unwrap().origin_realm; let path = base_path.join(format!("{}.lua", dep_name)); - let contents = match (root_realm, dependencies_realm) { + let require_expr = match (root_realm, dependencies_realm) { (source, dest) if source == dest => self.link_root_same_index(dep_package_id), (_, Realm::Server) => self.link_server_index(dep_package_id)?, (_, Realm::Shared) => self.link_shared_index(dep_package_id)?, @@ -288,6 +399,9 @@ impl InstallationContext { } }; + let target_file = self.resolve_package_file(dep_package_id, dependencies_realm); + let contents = self.generate_thunk_with_types(&require_expr, target_file.as_deref()); + log::trace!("Writing {}", path.display()); fs::write(path, contents)?; } @@ -319,7 +433,7 @@ impl InstallationContext { let dependencies_realm = resolved.metadata.get(dep_package_id).unwrap().origin_realm; let path = base_path.join(format!("{}.lua", dep_name)); - let contents = match (package_realm, dependencies_realm) { + let require_expr = match (package_realm, dependencies_realm) { (source, dest) if source == dest => self.link_sibling_same_index(dep_package_id), (_, Realm::Server) => self.link_server_index(dep_package_id)?, (_, Realm::Shared) => self.link_shared_index(dep_package_id)?, @@ -328,6 +442,9 @@ impl InstallationContext { } }; + let target_file = self.resolve_package_file(dep_package_id, dependencies_realm); + let contents = self.generate_thunk_with_types(&require_expr, target_file.as_deref()); + log::trace!("Writing {}", path.display()); fs::write(path, contents)?; } diff --git a/src/lib.rs b/src/lib.rs index cab640cd..1d734aff 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,8 +12,5 @@ pub mod package_req; pub mod package_source; pub mod resolution; pub mod test_package; -pub mod require_parser; -pub mod link_mutator; -pub mod type_reexports; pub use commands::*; diff --git a/src/link_mutator.rs b/src/link_mutator.rs deleted file mode 100644 index f6650a20..00000000 --- a/src/link_mutator.rs +++ /dev/null @@ -1,288 +0,0 @@ -use anyhow::{bail, Result}; -use full_moon::{ - ast::{ - luau::{ - ExportedTypeDeclaration, GenericDeclaration, GenericDeclarationParameter, - GenericParameterInfo, IndexedTypeInfo, TypeInfo, - }, - punctuated::{Pair, Punctuated}, - span::ContainedSpan, - Ast, Expression, LastStmt, LocalAssignment, Return, Stmt, - }, - tokenizer::{Token, TokenReference, TokenType}, -}; - -/// Finds all exported type declarations from a give source file -pub fn type_declarations_from_source(code: &str) -> Result> { - let parsed_module = match full_moon::parse(code) { - Ok(parsed_code) => parsed_code, - Err(errors) => bail!(errors - .iter() - .map(|err| err.to_string()) - .collect::>() - .join("\n")), - }; - - Ok(parsed_module - .nodes() - .stmts() - .filter_map(|stmt| match stmt { - Stmt::ExportedTypeDeclaration(stmt) => Some(stmt.clone()), - _ => None, - }) - .collect()) -} - -fn should_keep_default_type(type_info: &TypeInfo, resolved_types: &[String]) -> bool { - // TODO: we could be more clever here, but for now we keep it simple - match type_info { - TypeInfo::Basic(name) => resolved_types.contains(&name.token().to_string()), - TypeInfo::Boolean(_) => true, - _ => false, - } -} - -fn strip_unknown_default_generics( - generics: &GenericDeclaration, - resolved_types: &[String], -) -> Punctuated { - generics - .generics() - .pairs() - .map(|pair| { - pair.clone().map(|decl| match decl.default_type() { - Some(type_info) if should_keep_default_type(type_info, resolved_types) => decl, - _ => decl.with_default(None), - }) - }) - .collect::>() -} - -pub fn create_new_type_declaration(stmt: &ExportedTypeDeclaration) -> ExportedTypeDeclaration { - let type_info = match stmt.type_declaration().generics() { - Some(generics) => IndexedTypeInfo::Generic { - base: stmt.type_declaration().type_name().clone(), - arrows: ContainedSpan::new( - TokenReference::symbol("<").unwrap(), - TokenReference::symbol(">").unwrap(), - ), - generics: generics - .generics() - .pairs() - .map(|pair| { - pair.clone().map(|decl| match decl.parameter() { - GenericParameterInfo::Name(token) => TypeInfo::Basic(token.clone()), - GenericParameterInfo::Variadic { name, ellipsis } => { - TypeInfo::GenericPack { - name: name.clone(), - ellipsis: ellipsis.clone(), - } - } - _ => unreachable!(), - }) - }) - .collect::>(), - }, - None => IndexedTypeInfo::Basic(stmt.type_declaration().type_name().clone()), - }; - - // Modify the original type declaration to remove the default generics, if they are not resolvable - let mut resolved_types = stmt - .type_declaration() - .generics() - .map_or(vec![], |generics| { - generics - .generics() - .iter() - .map(|generic| match generic.parameter() { - GenericParameterInfo::Name(name) => name.token().to_string(), - GenericParameterInfo::Variadic { name, .. } => name.token().to_string(), - other => unreachable!("unknown node: {:?}", other), - }) - .collect() - }); - resolved_types.extend( - [ - "any", "boolean", "buffer", "never", "number", "string", "thread", "unknown", - ] - .iter() - .map(|s| s.to_string()), - ); - - let original_type_declaration = match stmt.type_declaration().generics() { - Some(generics) => stmt.type_declaration().clone().with_generics(Some( - generics - .clone() - .with_generics(strip_unknown_default_generics(generics, &resolved_types)), - )), - None => stmt.type_declaration().clone(), - }; - - // Can't use TypeDeclaration::new(), since it always panics - let type_declaration = original_type_declaration.with_type_definition(TypeInfo::Module { - module: TokenReference::new( - vec![], - Token::new(TokenType::Identifier { - identifier: "REQUIRED_MODULE".into(), - }), - vec![], - ), - punctuation: TokenReference::symbol(".").unwrap(), - type_info: Box::new(type_info), - }); - - ExportedTypeDeclaration::new(type_declaration) -} - -// Creates a list of re-exported type declarations from the type declarations found in the source file -fn re_export_type_declarations( - stmts: Vec, -) -> Vec<(Stmt, Option)> { - stmts - .iter() - .map(|stmt| { - ( - Stmt::ExportedTypeDeclaration(create_new_type_declaration(stmt)), - Some(TokenReference::new( - vec![], - Token::new(TokenType::Whitespace { - characters: "\n".into(), - }), - vec![], - )), - ) - }) - .collect() -} - -/// Extracts a require expression out into a local variable of form `local REQUIRED_MODULE = ...` -fn extract_require_into_local_stmt( - return_expressions: Punctuated, -) -> (Stmt, Option) { - ( - Stmt::LocalAssignment( - LocalAssignment::new( - std::iter::once(Pair::End(TokenReference::new( - vec![], - Token::new(TokenType::Identifier { - identifier: "REQUIRED_MODULE".into(), - }), - vec![], - ))) - .collect(), - ) - .with_equal_token(Some(TokenReference::symbol(" = ").unwrap())) - .with_expressions(return_expressions), - ), - None, - ) -} - -/// Creates a `return REQUIRED_MODULE` node -fn create_return_require_variable() -> (LastStmt, Option) { - ( - LastStmt::Return( - Return::new().with_returns( - std::iter::once(Pair::End(Expression::Symbol(TokenReference::new( - vec![], - Token::new(TokenType::Identifier { - identifier: "REQUIRED_MODULE".into(), - }), - vec![Token::new(TokenType::Whitespace { - characters: "\n".into(), - })], - )))) - .collect(), - ), - ), - None, - ) -} - -pub enum MutateLinkResult { - Changed(Ast), - Unchanged, -} - -/// Given an old link and the contents of the file it points to, creates a new link source -pub fn mutate_link( - parsed_code: Ast, - return_expressions: Punctuated, - contents: &str, -) -> Result { - let type_declarations = type_declarations_from_source(contents)?; - - if type_declarations.is_empty() { - return Ok(MutateLinkResult::Unchanged); - } - - let new_nodes = parsed_code - .nodes() - .clone() - .with_stmts( - std::iter::once(extract_require_into_local_stmt(return_expressions)) - .chain(re_export_type_declarations(type_declarations)) - .collect(), - ) - .with_last_stmt(Some(create_return_require_variable())); - Ok(MutateLinkResult::Changed(parsed_code.with_nodes(new_nodes))) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn re_exports_generic_defaults_if_they_are_part_of_the_type() { - let code = r" - export type Value = Types.Value - "; - - let type_declarations = type_declarations_from_source(code).unwrap(); - assert_eq!(type_declarations.len(), 1); - - let reexported_type_declarations = re_export_type_declarations(type_declarations); - assert_eq!(reexported_type_declarations.len(), 1); - - assert_eq!( - reexported_type_declarations[0].0.to_string(), - "export type Value = REQUIRED_MODULE.Value" - ); - } - - #[test] - fn does_not_re_export_unknown_default_generics() { - let code = r" - export type Value = Types.Value - "; - - let type_declarations = type_declarations_from_source(code).unwrap(); - assert_eq!(type_declarations.len(), 1); - - let reexported_type_declarations = re_export_type_declarations(type_declarations); - assert_eq!(reexported_type_declarations.len(), 1); - - assert_eq!( - reexported_type_declarations[0].0.to_string(), - "export type Value = REQUIRED_MODULE.Value" - ); - } - - #[test] - fn re_exports_generic_defaults_if_they_are_builtin_types() { - let code = r" - export type Value = Types.Value - "; - - let type_declarations = type_declarations_from_source(code).unwrap(); - assert_eq!(type_declarations.len(), 1); - - let reexported_type_declarations = re_export_type_declarations(type_declarations); - assert_eq!(reexported_type_declarations.len(), 1); - - assert_eq!( - reexported_type_declarations[0].0.to_string(), - "export type Value = REQUIRED_MODULE.Value" - ); - } -} \ No newline at end of file diff --git a/src/require_parser.rs b/src/require_parser.rs deleted file mode 100644 index b87c7a23..00000000 --- a/src/require_parser.rs +++ /dev/null @@ -1,105 +0,0 @@ -use anyhow::{bail, Result}; -use full_moon::{ - ast::{Call, Expression, FunctionArgs, Index, Suffix, Var}, - tokenizer::TokenType, -}; - -/// Decomposes a VarExpression into a list of string components -pub fn expression_to_components(expression: &Expression) -> Result> { - let mut components = Vec::new(); - - let Expression::Var(Var::Expression(var_expression)) = expression else { - bail!("require expression not supported: expression must contain components of form `.value` or `['value']`") - }; - - components.push(var_expression.prefix().to_string().trim().to_string()); - - for suffix in var_expression.suffixes() { - let Suffix::Index(index) = suffix else { - bail!("require expression not supported: expression must contain components of form `.value` or `['value']`") - }; - - match index { - Index::Dot { name, .. } => { - components.push(name.to_string().trim().to_string()); - } - Index::Brackets { expression, .. } => { - let Expression::String(name) = expression else { - bail!("require expression not supported: expression contains brackets component not of the form ['value']") - }; - let TokenType::StringLiteral { literal, .. } = name.token_type() else { - bail!("require expression not supported: expression contains brackets component not of the form ['value']") - }; - components.push(literal.trim().to_string()); - } - _ => unreachable!(), - } - } - - Ok(components) -} - -/// Parse a require() expression and extract its path components -pub fn match_require(expression: &Expression) -> Result> { - let Expression::FunctionCall(call) = expression else { - bail!("'{}' is not a function call", expression.to_string().trim()); - }; - - if call.prefix().to_string().trim() == "require" && call.suffixes().count() == 1 { - if let Suffix::Call(Call::AnonymousCall(FunctionArgs::Parentheses { arguments, .. })) = - call.suffixes().next().unwrap() - { - if arguments.len() == 1 { - return expression_to_components(arguments.iter().next().unwrap()); - } - } - } else { - bail!("unknown require expression '{}'", call.to_string().trim()); - } - - bail!( - "'{}' is not a require function call", - call.to_string().trim() - ) -} - -#[cfg(test)] -mod tests { - use full_moon::ast::Stmt; - - use super::*; - - fn require_expression(code: &str) -> Expression { - let parsed_ast = full_moon::parse(code).unwrap(); - let stmt = parsed_ast.nodes().stmts().next().unwrap(); - let Stmt::FunctionCall(expression) = stmt else { - unreachable!() - }; - Expression::FunctionCall(expression.clone()) - } - - fn expression_into_components(code: &str, components: Vec<&str>) -> bool { - match_require(&require_expression(code)).unwrap() == components - } - - #[test] - fn simple_require() { - assert!(expression_into_components( - "require(script.Parent.Example)", - vec!["script", "Parent", "Example"] - )) - } - - #[test] - fn require_with_brackets() { - assert!(expression_into_components( - "require(script.Parent['Example'])", - vec!["script", "Parent", "Example"] - )) - } - - #[test] - fn unhandled_require() { - assert!(match_require(&require_expression("require('string')")).is_err()) - } -} \ No newline at end of file diff --git a/src/type_reexports.rs b/src/type_reexports.rs deleted file mode 100644 index 4b4432ce..00000000 --- a/src/type_reexports.rs +++ /dev/null @@ -1,310 +0,0 @@ -use anyhow::{bail, Context, Result}; -use full_moon::ast::LastStmt; -use log::{error, info, warn}; -use std::path::{Path, PathBuf}; - -use crate::require_parser::*; - - -// Filesystem-based resolver -fn resolve_components_fs(link_path: &Path, packages_root: &Path, path_components: &[String]) -> Option { - if path_components.is_empty() { - return None; - } - - let first = &path_components[0]; - let mut cursor: PathBuf; - - if first == "script" { - // Start from the link file path (`script`), allow Parent to walk up to the link's directory - cursor = link_path.to_path_buf(); - for comp in path_components.iter().skip(1) { - if comp == "Parent" { - cursor = cursor.parent()?.to_path_buf(); - } else { - cursor.push(comp); - } - } - } else if first == "game" { - // Map game.<...>.Packages.* to the on-disk Packages root for this pass - cursor = packages_root.to_path_buf(); - - let mut started = false; - for comp in path_components.iter().skip(1) { - if !started { - // Skip components until we hit "Packages" - if comp == "Packages" { - started = true; - } - continue; - } - - if comp == "Parent" { - cursor = cursor.parent()?.to_path_buf(); - } else { - cursor.push(comp); - } - } - - // Fallback: if "Packages" wasn't found, try pushing everything after 'game' - if !started { - for comp in path_components.iter().skip(1) { - if comp == "Parent" { - cursor = cursor.parent()?.to_path_buf(); - } else { - cursor.push(comp); - } - } - } - } else { - return None; - } - - // If direct file exists, use it. - if cursor.is_file() { - return Some(cursor); - } - - // Try {name}.lua / {name}.luau - let candidate_lua = cursor.with_extension("lua"); - if candidate_lua.is_file() { - return Some(candidate_lua); - } - let candidate_luau = cursor.with_extension("luau"); - if candidate_luau.is_file() { - return Some(candidate_luau); - } - - // Try init.lua / init.luau inside a directory - let init_lua = cursor.join("init.lua"); - if init_lua.is_file() { - return Some(init_lua); - } - let init_luau = cursor.join("init.luau"); - if init_luau.is_file() { - return Some(init_luau); - } - - // Also try src/init.lua / src/init.luau (common layout) - let src_init_lua = cursor.join("src").join("init.lua"); - if src_init_lua.is_file() { - return Some(src_init_lua); - } - let src_init_luau = cursor.join("src").join("init.luau"); - if src_init_luau.is_file() { - return Some(src_init_luau); - } - - None -} - -/// Mutate a single thunk using filesystem resolution only (no sourcemap) -fn mutate_thunk_fs(path: &Path, packages_root: &Path) -> Result<()> { - info!("Found link file '{}'", path.display()); - - // Skip already-mutated thunks - let original_src = std::fs::read_to_string(path)?; - if original_src.contains("local REQUIRED_MODULE") || original_src.contains("return REQUIRED_MODULE") { - info!("Link already mutated, leaving unchanged"); - return Ok(()); - } - - // Parse to extract the require() expression - let parsed_code = match full_moon::parse(&original_src) { - Ok(parsed_code) => parsed_code, - Err(errors) => bail!(errors - .iter() - .map(|err| err.to_string()) - .collect::>() - .join("\n")), - }; - - if let Some(LastStmt::Return(r#return)) = parsed_code.nodes().last_stmt() { - let returned_expression = r#return.returns().iter().next().unwrap(); - - let path_components = match match_require(returned_expression) { - Ok(components) => components, - Err(err) => { - warn!("Malformed link file, could not parse return expression, skipping. Run `wally install` to regenerate link files"); - error!("{:#}", err); - return Ok(()); - } - }; - - info!( - "Require expression converted to path: '{}'", - path_components.join("/") - ); - - let file_path = resolve_components_fs(path, packages_root, &path_components) - .context("Could not resolve require expression to file path via filesystem")?; - let pass_through_contents = - std::fs::read_to_string(&file_path).context("Failed to read linked file")?; - - // Build textual re-exports by scanning the target module for "export type" declarations. - // We keep the original generic declaration on the LHS and emit RHS with generic names only, - // including variadic generic packs (e.g., T...). - let mut reexports = Vec::new(); - for line in pass_through_contents.lines() { - let trimmed = line.trim_start(); - if !trimmed.starts_with("export type ") { - continue; - } - - // Parse "export type Name<...> = ..." - let rest = &trimmed["export type ".len()..]; - - // Extract type name and optional generics by locating the first '<' ... '>' pair. - let (type_name, generics_opt) = if let Some(lt_rel) = rest.find('<') { - let name = rest[..lt_rel].trim(); - // find the closing '>' after lt_rel - let gt_rel = match rest[lt_rel + 1..].find('>') { - Some(i) => lt_rel + 1 + i, - None => continue, - }; - let generics = &rest[lt_rel + 1..gt_rel]; - (name, Some(generics.trim())) - } else { - // No generics - // Stop at the '=' if present, otherwise take the whole remainder - let name_end = rest.find('=').unwrap_or(rest.len()); - (rest[..name_end].trim(), None) - }; - - // Derive list of generic parameter identifiers without defaults. - // Keep variadic packs by preserving the trailing "...". - let mut rhs_generics: Vec = Vec::new(); - if let Some(generics) = generics_opt { - for raw in generics.split(',') { - // Strip default, e.g., "S = T" -> "S" and preserve "..." - let name_only = raw.splitn(2, '=').next().unwrap().trim(); - if !name_only.is_empty() { - rhs_generics.push(name_only.to_string()); - } - } - } - - // Rebuild generics for LHS (keep original) and RHS (names only) - let lhs_suffix = if let Some(generics) = generics_opt { - format!("<{}>", generics) - } else { - String::new() - }; - let rhs_suffix = if rhs_generics.is_empty() { - String::new() - } else { - format!("<{}>", rhs_generics.join(", ")) - }; - - reexports.push(format!( - "export type {}{} = REQUIRED_MODULE.{}{}", - type_name, lhs_suffix, type_name, rhs_suffix - )); - } - - if reexports.is_empty() { - info!("No exported types, leaving unchanged"); - return Ok(()); - } - - // Extract the original `require(...)` expression from the link source to preserve formatting. - let mut require_expr_opt: Option = None; - for line in original_src.lines() { - let t = line.trim_start(); - if t.starts_with("return require") { - // Strip leading "return " - let expr = t.strip_prefix("return ").unwrap_or(t).trim().to_string(); - require_expr_opt = Some(expr); - break; - } - } - // Build canonical one if missing - let require_expr = require_expr_opt.unwrap_or_else(|| { - let mut expr = String::from("require("); - let mut first = true; - for comp in &path_components { - if first { - expr.push_str(comp); - first = false; - } else if comp == "Parent" { - expr.push_str(".Parent"); - } else if comp == "_Index" { - expr.push_str("._Index"); - } else { - expr.push_str(&format!("[\"{}\"]", comp)); - } - } - expr.push(')'); - expr - }); - - let mut new_source = String::new(); - new_source.push_str(&format!("local REQUIRED_MODULE = {}\n", require_expr)); - for line in reexports { - new_source.push_str(&line); - new_source.push('\n'); - } - new_source.push_str("return REQUIRED_MODULE\n"); - - info!("Exported types found, writing new linker file"); - std::fs::write(path, new_source)?; - } else { - warn!("Malformed link file, no return statement found, skipping. Run `wally install` to regenerate link files"); - return Ok(()); - } - - Ok(()) -} - -fn handle_index_directory_fs(index_dir: &Path, packages_root: &Path) -> Result { - let mut success = true; - - for package_entry in std::fs::read_dir(index_dir)?.flatten() { - for thunk in std::fs::read_dir(package_entry.path())?.flatten() { - if thunk.file_type().map(|t| t.is_file()).unwrap_or(false) { - if let Err(err) = mutate_thunk_fs(&thunk.path(), packages_root) { - error!("{:#}", err); - success = false; - } - } - } - } - - Ok(success) -} - -/// Public entry point: filesystem-based traversal (no sourcemap needed) -pub fn run_on_packages_fs(packages_root: &Path) -> Result<()> { - let mut success = true; - - // Iterate top-level under Packages - for entry in std::fs::read_dir(packages_root) - .with_context(|| format!("Failed to read packages folder {}", packages_root.display()))? - .flatten() - { - if entry.file_name() == "_Index" { - match handle_index_directory_fs(&entry.path(), packages_root) { - Ok(ok) => success &= ok, - Err(err) => { - error!("{:#}", err); - success = false; - } - } - continue; - } - - // Only mutate link thunks (files) - if entry.file_type().map(|t| t.is_file()).unwrap_or(false) { - if let Err(err) = mutate_thunk_fs(&entry.path(), packages_root) { - error!("{:#}", err); - success = false; - } - } - } - - if success { - Ok(()) - } else { - bail!("Mutation did not complete successfully") - } -} \ No newline at end of file diff --git a/tests/integration/install.rs b/tests/integration/install.rs index a7bb5415..474156c7 100644 --- a/tests/integration/install.rs +++ b/tests/integration/install.rs @@ -101,3 +101,127 @@ fn run_install_test(name: &str) -> TempProject { assert_dir_snapshot!(project.path()); project } + + +#[test] +fn type_reexports_variadics_and_defaults() { + use fs_err as fs; + use tempfile::tempdir; + use libwally::{Args, GlobalOptions, InstallSubcommand, Subcommand}; + use libwally::test_package::PackageBuilder; + + // 1) Create a temporary test registry on disk + let reg_dir = tempdir().unwrap(); + let reg_path = reg_dir.path(); + + fs::create_dir_all(reg_path.join("index").join("test")).unwrap(); + fs::create_dir_all(reg_path.join("contents").join("test").join("typed-exports")).unwrap(); + // Minimal config for TestRegistry fallback + fs::write( + reg_path.join("index").join("config.json"), + r#"{"api":"http://localhost","fallback_registries":[]}"#, + ) + .unwrap(); + + // 2) Build a package with exported types (including defaults and variadic packs) + let pkg = PackageBuilder::new("test/typed-exports@0.1.0").with_file( + "src/init.luau", + r#"export type Foo = number +export type Bar = { x: T, y: S } +export type Variadic = (T...) +return {} +"#, + ); + + // Add manifest entry to registry index (as JSON lines) + let mut manifest = pkg.manifest().clone(); + manifest.package.registry = reg_path.to_str().unwrap().replace('\\', "/"); + let index_path = reg_path.join("index").join("test").join("typed-exports"); + fs::write( + &index_path, + format!("{}\n", serde_json::to_string(&manifest).unwrap()), + ) + .unwrap(); + + // Add contents zip to registry contents/ + let contents = pkg.contents(); + let contents_path = reg_path + .join("contents") + .join("test") + .join("typed-exports") + .join("0.1.0.zip"); + fs::write(contents_path, contents.data()).unwrap(); + + // 3) Create a temporary project that depends on our typed package + let proj_dir = tempdir().unwrap(); + let project_path = proj_dir.path(); + + fs::create_dir_all(project_path.join("src")).unwrap(); + fs::write(project_path.join("src").join("init.lua"), "return \"ok\"\n").unwrap(); + + // Write wally.toml pointing registry to our temp registry path + let wally_toml = format!( + r#"[package] +name = "example/app" +version = "0.1.0" +license = "MIT" +realm = "shared" +registry = "{registry}" + +[dependencies] +Typed = "test/typed-exports@0.1.0" +"#, + registry = manifest.package.registry + ); + fs::write(project_path.join("wally.toml"), wally_toml).unwrap(); + + // Optional default.project.json + fs::write( + project_path.join("default.project.json"), + "{\n \"name\": \"app\",\n \"tree\": {\"$path\": \"src\"}\n}\n", + ) + .unwrap(); + + // 4) Run `wally install` against our temp registry + let args = Args { + global: GlobalOptions { + test_registry: true, + ..Default::default() + }, + subcommand: Subcommand::Install(InstallSubcommand { + project_path: project_path.to_owned(), + locked: false, + }), + }; + args.run().unwrap(); + + // 5) Verify the generated thunk contains type re-exports + let link = fs::read_to_string(project_path.join("Packages").join("Typed.lua")).unwrap(); + + assert!( + link.contains("local REQUIRED_MODULE = require("), + "expected thunk to define REQUIRED_MODULE, got:\n{}", + link + ); + assert!( + link.contains("export type Foo = REQUIRED_MODULE.Foo"), + "expected Foo re-export, got:\n{}", + link + ); + assert!( + link.contains("export type Bar = REQUIRED_MODULE.Bar"), + "expected Bar re-export with defaults kept on LHS, got:\n{}", + link + ); + assert!( + link.contains("export type Variadic = REQUIRED_MODULE.Variadic"), + "expected variadic generic pack re-export, got:\n{}", + link + ); + assert!( + link.ends_with("return REQUIRED_MODULE\n") + || link.ends_with("return REQUIRED_MODULE\r\n"), + "expected to return REQUIRED_MODULE, got:\n{}", + link + ); +}