diff --git a/compiler/bsc/rescript_compiler_main.ml b/compiler/bsc/rescript_compiler_main.ml index 67fbc0043de..44f14e658d3 100644 --- a/compiler/bsc/rescript_compiler_main.ml +++ b/compiler/bsc/rescript_compiler_main.ml @@ -298,6 +298,8 @@ let command_line_flags : (string * Bsc_args.spec * string) array = set Clflags.transparent_modules, "*internal*Do not record dependencies for module aliases" ); ("-bs-gentype", set Clflags.bs_gentype, "*internal* Pass gentype command"); +<<<<<<< HEAD +<<<<<<< HEAD ( "-bs-gentype-module", string_call (fun s -> GenTypeConfig.module_flag := GenTypeConfig.module_of_string s), @@ -341,6 +343,9 @@ let command_line_flags : (string * Bsc_args.spec * string) array = string_call (fun s -> GenTypeConfig.bsb_project_root := s), "*internal* Set gentype bsb project root (workspace root containing \ .sourcedirs.json)" ); + ( "-bs-multi-entry", + set Js_config.multi_entry, + "*internal* Preserve lowercase file module names" ); (******************************************************************************) ( "-unboxed-types", set Clflags.unboxed_types, diff --git a/compiler/common/js_config.ml b/compiler/common/js_config.ml index 9e2b6f598ca..8b113797026 100644 --- a/compiler/common/js_config.ml +++ b/compiler/common/js_config.ml @@ -54,6 +54,7 @@ let jsx_module = ref React let jsx_preserve = ref false let js_stdout = ref true let all_module_aliases = ref false +let multi_entry = ref false let no_stdlib = ref false let no_export = ref false let int_of_jsx_version = function diff --git a/compiler/common/js_config.mli b/compiler/common/js_config.mli index f19b3c6126c..5c508588045 100644 --- a/compiler/common/js_config.mli +++ b/compiler/common/js_config.mli @@ -88,6 +88,8 @@ val js_stdout : bool ref val all_module_aliases : bool ref +val multi_entry : bool ref + val no_stdlib : bool ref val no_export : bool ref diff --git a/compiler/core/js_implementation.ml b/compiler/core/js_implementation.ml index 5f4e4e6c765..f6760dbf2ae 100644 --- a/compiler/core/js_implementation.ml +++ b/compiler/core/js_implementation.ml @@ -20,7 +20,7 @@ let module_of_filename outputprefix = String.sub basename 0 pos with Not_found -> basename in - String.capitalize_ascii name + if !Js_config.multi_entry then name else String.capitalize_ascii name let fprintf = Format.fprintf @@ -129,7 +129,10 @@ let after_parsing_impl ppf outputprefix (ast : Parsetree.structure) = output_value stdout ast); if !Js_config.syntax_only then Warnings.check_fatal () else - let modulename = Ext_filename.module_name outputprefix in + let modulename = + Ext_filename.module_name ~preserve_case:!Js_config.multi_entry + outputprefix + in Lam_compile_env.reset (); let env = Res_compmisc.initial_env ~modulename () in Env.set_unit_name modulename; @@ -192,7 +195,9 @@ let implementation_map ppf sourcefile = seek_in ichan (Ext_digest.length + 1); let list_of_modules = Ext_io.rev_lines_of_chann ichan in close_in ichan; - let ns = Ext_filename.module_name sourcefile in + let ns = + Ext_filename.module_name ~preserve_case:!Js_config.multi_entry sourcefile + in let ml_ast = Ext_list.fold_left list_of_modules [] (fun acc line -> if Ext_string.is_empty line then acc diff --git a/compiler/ext/ext_filename.ml b/compiler/ext/ext_filename.ml index fd7c64c4267..e476ce16eba 100644 --- a/compiler/ext/ext_filename.ml +++ b/compiler/ext/ext_filename.ml @@ -76,10 +76,13 @@ let new_extension name (ext : string) = we can not tell the difference between "x.cpp.ml" and "x.ml" *) -let module_name name = +let module_name ?(preserve_case = false) name = let rec search_dot i name = - if i < 0 then Ext_string.capitalize_ascii name - else if String.unsafe_get name i = '.' then Ext_string.capitalize_sub name i + if i < 0 then + if preserve_case then name else Ext_string.capitalize_ascii name + else if String.unsafe_get name i = '.' then + if preserve_case then String.sub name 0 i + else Ext_string.capitalize_sub name i else search_dot (i - 1) name in let name = Filename.basename name in diff --git a/compiler/ext/ext_filename.mli b/compiler/ext/ext_filename.mli index 5c678310b9e..8d5b9bee24d 100644 --- a/compiler/ext/ext_filename.mli +++ b/compiler/ext/ext_filename.mli @@ -43,7 +43,7 @@ val new_extension : string -> string -> string val chop_all_extensions_maybe : string -> string (* OCaml specific abstraction*) -val module_name : string -> string +val module_name : ?preserve_case:bool -> string -> string type module_info = {module_name: string; case: bool} diff --git a/docs/docson/build-schema.json b/docs/docson/build-schema.json index af78908ea8c..f78bd9513d9 100644 --- a/docs/docson/build-schema.json +++ b/docs/docson/build-schema.json @@ -364,6 +364,10 @@ "$ref": "#/definitions/namespace-spec", "description": "can be true/false or a customized name" }, + "multi-entry": { + "type": "boolean", + "description": "Default: false. When true, lowercase source files are compiled as private file-level modules without capitalizing their file module name. They cannot be referenced from ReScript code, are not exported through namespaces, and duplicate lowercase file stems are allowed across source folders. Pascal-case file modules keep the normal globally unique module behavior." + }, "sources": { "$ref": "#/definitions/sources", "description": "Source code location" diff --git a/package.json b/package.json index b7085579392..7667f1692ec 100644 --- a/package.json +++ b/package.json @@ -76,15 +76,13 @@ "#cli/*": "./cli/common/*.js", "#dev/*": "./lib_dev/*.js" }, - "dependencies": { - "@rescript/runtime": "workspace:packages/@rescript/runtime" - }, "optionalDependencies": { - "@rescript/darwin-arm64": "workspace:packages/@rescript/darwin-arm64", - "@rescript/darwin-x64": "workspace:packages/@rescript/darwin-x64", - "@rescript/linux-arm64": "workspace:packages/@rescript/linux-arm64", - "@rescript/linux-x64": "workspace:packages/@rescript/linux-x64", - "@rescript/win32-x64": "workspace:packages/@rescript/win32-x64" + "@rescript/darwin-arm64": "workspace:*", + "@rescript/darwin-x64": "workspace:*", + "@rescript/linux-arm64": "workspace:*", + "@rescript/linux-x64": "workspace:*", + "@rescript/runtime": "workspace:*", + "@rescript/win32-x64": "workspace:*" }, "devDependencies": { "@biomejs/biome": "2.4.10", @@ -97,8 +95,7 @@ "typescript": "6.0.3" }, "workspaces": [ - "packages/playground", - "packages/@rescript/*", + "packages/**", "tests/dependencies/**", "tests/analysis_tests/**", "tests/docstring_tests", diff --git a/rewatch/CompilerConfigurationSpec.md b/rewatch/CompilerConfigurationSpec.md index 49a14b01f5c..77a0ec54db9 100644 --- a/rewatch/CompilerConfigurationSpec.md +++ b/rewatch/CompilerConfigurationSpec.md @@ -7,6 +7,7 @@ This document contains a list of all config parameters with remarks, and whether | name | string | | [x] | | namespace | boolean | | [x] | | namespace | string | | [x] | +| multi-entry | boolean | Lowercase file modules stay private and duplicate lowercase file stems are allowed | [x] | | sources | string | | [x] | | sources | array of string | | [x] | | sources | Source | | [x] | diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 73e77987dd7..5a2a4aad798 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -14,22 +14,23 @@ use std::io::Write; use std::path::{Path, PathBuf}; use std::time::Instant; +fn remove_ast_cache(package: &packages::Package, source_file: &Path, extension: &str) { + let ast_cache_path = helpers::get_ocaml_ast_cache_path_for_extension(package, source_file, extension); + let _ = std::fs::remove_file(&ast_cache_path); + + let legacy_ast_cache_path = + helpers::get_compiler_asset(package, &packages::Namespace::NoNamespace, source_file, extension); + if legacy_ast_cache_path != ast_cache_path { + let _ = std::fs::remove_file(legacy_ast_cache_path); + } +} + fn remove_ast(package: &packages::Package, source_file: &Path) { - let _ = std::fs::remove_file(helpers::get_compiler_asset( - package, - &packages::Namespace::NoNamespace, - source_file, - "ast", - )); + remove_ast_cache(package, source_file, "ast"); } fn remove_iast(package: &packages::Package, source_file: &Path) { - let _ = std::fs::remove_file(helpers::get_compiler_asset( - package, - &packages::Namespace::NoNamespace, - source_file, - "iast", - )); + remove_ast_cache(package, source_file, "iast"); } fn remove_mjs_file(source_file: &Path, suffix: &str) { @@ -250,7 +251,9 @@ pub fn cleanup_previous_build( .difference(&all_module_names) .flat_map(|module_name| { // if the module is a namespace, we need to mark the whole namespace as dirty when a module has been deleted - if let Some(namespace) = helpers::get_namespace_from_module_name(module_name) { + if !module_name.contains(':') + && let Some(namespace) = helpers::get_namespace_from_module_name(module_name) + { return vec![namespace, module_name.to_string()]; } vec![module_name.to_string()] diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs index c646309ef7b..a85f33d2643 100644 --- a/rewatch/src/build/compile.rs +++ b/rewatch/src/build/compile.rs @@ -526,10 +526,14 @@ pub fn compiler_args( ) -> Result> { let bsc_flags = config::flatten_flags(&config.compiler_flags); let dependency_paths = get_dependency_paths(config, project_context, packages, is_type_dev); - let module_name = helpers::file_path_to_module_name(file_path, &config.get_namespace()); + let multi_entry = config.is_multi_entry_enabled(); + let module_name = + helpers::file_path_to_constructible_module_name(file_path, &config.get_namespace(), multi_entry); let namespace_args = match &config.get_namespace() { - packages::Namespace::NamespaceWithEntry { namespace: _, entry } if &module_name == entry => { + packages::Namespace::NamespaceWithEntry { namespace: _, entry } + if module_name.as_ref() == Some(entry) => + { // if the module is the entry we just want to open the namespace vec![ "-open".to_string(), @@ -586,11 +590,23 @@ pub fn compiler_args( let package_name_arg = vec!["-bs-package-name".to_string(), config.name.to_owned()]; let project_root_args = config.get_project_root_args(); + let multi_entry_arg = if multi_entry { + vec!["-bs-multi-entry".to_string()] + } else { + vec![] + }; + let implementation_args = if is_interface { - debug!("Compiling interface file: {}", &module_name); + debug!( + "Compiling interface file: {}", + module_name.as_deref().unwrap_or("") + ); vec![] } else { - debug!("Compiling file: {}", &module_name); + debug!( + "Compiling file: {}", + module_name.as_deref().unwrap_or("") + ); let specs = root_config.get_package_specs(); specs @@ -639,6 +655,7 @@ pub fn compiler_args( warning_args, gentype_arg, experimental_args, + multi_entry_arg, // vec!["-warn-error".to_string(), "A".to_string()], // ^^ this one fails for bisect-ppx // this is the default @@ -770,6 +787,12 @@ fn compile_file( .map_err(|e| anyhow!(e))?; let basename = helpers::file_path_to_compiler_asset_basename(implementation_file_path, &package.namespace); + let is_constructible_module = helpers::file_path_to_constructible_module_name( + implementation_file_path, + &package.namespace, + package.config.is_multi_entry_enabled(), + ) + .is_some(); let has_interface = module.get_interface().is_some(); let is_type_dev = module.is_type_dev; // `gentype_dirs` is populated once during package discovery, so we just @@ -817,7 +840,7 @@ fn compile_file( let dir = Path::new(implementation_file_path).parent().unwrap(); // perhaps we can do this copying somewhere else - if !is_interface { + if !is_interface && is_constructible_module { let _ = std::fs::copy( package .get_build_path() @@ -842,7 +865,7 @@ fn compile_file( .join(format!("{basename}.cmt")), ocaml_build_path_abs.join(format!("{basename}.cmt")), ); - } else { + } else if is_interface && is_constructible_module { let _ = std::fs::copy( package .get_build_path() @@ -870,13 +893,15 @@ fn compile_file( ) .expect("copying source file failed"); - let _ = std::fs::copy( - Path::new(&package.path).join(path), - package - .get_ocaml_build_path() - .join(std::path::Path::new(path).file_name().unwrap()), - ) - .expect("copying source file failed"); + if is_constructible_module { + let _ = std::fs::copy( + Path::new(&package.path).join(path), + package + .get_ocaml_build_path() + .join(std::path::Path::new(path).file_name().unwrap()), + ) + .expect("copying source file failed"); + } } if let SourceType::SourceFile(SourceFile { implementation: Implementation { path, .. }, @@ -892,13 +917,15 @@ fn compile_file( ) .expect("copying source file failed"); - let _ = std::fs::copy( - Path::new(&package.path).join(path), - package - .get_ocaml_build_path() - .join(std::path::Path::new(path).file_name().unwrap()), - ) - .expect("copying source file failed"); + if is_constructible_module { + let _ = std::fs::copy( + Path::new(&package.path).join(path), + package + .get_ocaml_build_path() + .join(std::path::Path::new(path).file_name().unwrap()), + ) + .expect("copying source file failed"); + } } // copy js file diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 2584abf190e..c817fe67bfa 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -676,10 +676,13 @@ fn extend_with_children( .into_iter() .for_each(|source| map.extend(source)); - let mut modules = AHashSet::from_iter( - map.keys() - .map(|key| helpers::file_path_to_module_name(key, &package.namespace)), - ); + let mut modules = AHashSet::from_iter(map.keys().filter_map(|key| { + helpers::file_path_to_constructible_module_name( + key, + &package.namespace, + package.config.is_multi_entry_enabled(), + ) + })); match package.namespace.to_owned() { Namespace::Namespace(namespace) => { let _ = modules.insert(namespace); @@ -779,6 +782,7 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { let packages = build_state.packages.clone(); for (package_name, package) in packages.iter() { debug!("Parsing package: {package_name}"); + let multi_entry = package.config.is_multi_entry_enabled(); if let Some(package_modules) = package.modules.to_owned() { build_state.module_names.extend(package_modules) } @@ -835,7 +839,13 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { let depending_modules = source_files .iter() - .map(|path| helpers::file_path_to_module_name(path, &packages::Namespace::NoNamespace)) + .filter_map(|path| { + helpers::file_path_to_constructible_module_name( + path, + &packages::Namespace::NoNamespace, + multi_entry, + ) + }) .filter(|module_name| { if let Some(entry) = entry { module_name != entry @@ -852,13 +862,18 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { // compile_mlmap(&package, namespace, &project_root); let deps = source_files .iter() - .filter(|path| { - helpers::is_non_exotic_module_name(&helpers::file_path_to_module_name( + .filter_map(|path| { + helpers::file_path_to_constructible_module_name( path, &packages::Namespace::NoNamespace, - )) + multi_entry, + ) + .map(|module_name| (path, module_name)) + }) + .filter(|(_, module_name)| helpers::is_non_exotic_module_name(module_name)) + .filter_map(|(path, _)| { + helpers::file_path_to_constructible_module_name(path, &package.namespace, multi_entry) }) - .map(|path| helpers::file_path_to_module_name(path, &package.namespace)) .filter(|module_name| { if let Some(entry) = entry { module_name != entry @@ -891,12 +906,14 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { let namespace = package.namespace.to_owned(); let extension = file.extension().unwrap().to_str().unwrap(); - let module_name = helpers::file_path_to_module_name(file, &namespace); + let module_key = + helpers::file_path_to_module_key(file, &namespace, &package.name, multi_entry); + let canonical_module_name = helpers::file_path_to_module_name(file, &namespace); if helpers::is_implementation_file(extension) { // Store duplicate paths in an Option so we can build the error after the entry borrow ends. let mut duplicate_paths: Option<(PathBuf, PathBuf)> = None; - match build_state.modules.entry(module_name.to_string()) { + match build_state.modules.entry(module_key.to_string()) { Entry::Occupied(mut entry) => { let module = entry.get_mut(); if let SourceType::SourceFile(ref mut source_file) = module.source_type { @@ -946,7 +963,7 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { std::mem::swap(&mut first, &mut second); } return Err(anyhow!( - "Duplicate module name: {module_name}. Found in {} and {}. Rename one of these files.", + "Duplicate module name: {module_key}. Found in {} and {}. Rename one of these files.", first, second )); @@ -964,7 +981,8 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { if let Some(implementation_path) = source_files.keys().find(|path| { let extension = path.extension().and_then(|ext| ext.to_str()); matches!(extension, Some(ext) if helpers::is_implementation_file(ext)) - && helpers::file_path_to_module_name(path, &namespace) == module_name + && helpers::file_path_to_module_name(path, &namespace) + == canonical_module_name }) { let implementation_display = implementation_path.to_string_lossy().to_string(); @@ -984,7 +1002,7 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { Some(_) => { build_state .modules - .entry(module_name.to_string()) + .entry(module_key.to_string()) .and_modify(|module| { if let SourceType::SourceFile(ref mut source_file) = module.source_type { source_file.interface = Some(Interface { diff --git a/rewatch/src/build/parse.rs b/rewatch/src/build/parse.rs index 223d49d90ea..c2db0447d3d 100644 --- a/rewatch/src/build/parse.rs +++ b/rewatch/src/build/parse.rs @@ -383,10 +383,9 @@ fn generate_ast( } }; if let Ok((ast_path, _)) = &result { - let _ = std::fs::copy( - Path::new(&build_path_abs).join(ast_path), - package.get_ocaml_build_path().join(ast_path.file_name().unwrap()), - ); + let cache_path = helpers::get_ocaml_ast_cache_path(&package, filename); + helpers::create_path_for_path(cache_path.parent().expect("AST cache path should have a parent")); + let _ = std::fs::copy(Path::new(&build_path_abs).join(ast_path), cache_path); } result } diff --git a/rewatch/src/build/read_compile_state.rs b/rewatch/src/build/read_compile_state.rs index 3b766294712..08276eddd51 100644 --- a/rewatch/src/build/read_compile_state.rs +++ b/rewatch/src/build/read_compile_state.rs @@ -7,6 +7,17 @@ use std::fs; use std::path::{Path, PathBuf}; use std::time::SystemTime; +type CompileAsset = ( + PathBuf, + SystemTime, + String, + String, + PathBuf, + bool, + packages::Namespace, + bool, +); + pub fn read(build_state: &mut BuildCommandState) -> anyhow::Result { let mut ast_modules: AHashMap = AHashMap::new(); let mut cmi_modules: AHashMap = AHashMap::new(); @@ -45,43 +56,38 @@ pub fn read(build_state: &mut BuildCommandState) -> anyhow::Result { - let path = entry.path(); - let extension = path.extension().and_then(|e| e.to_str()); - match extension { - Some(ext) => match ext { - "iast" | "ast" | "cmi" | "cmt" => Some(( - path.to_owned(), - entry.metadata().unwrap().modified().unwrap(), - ext.to_owned(), - package.name.to_owned(), - package.namespace.to_owned(), - package.is_root, - )), - _ => None, - }, - None => None, - } - } - Err(_) => None, - }) - .collect::>() + let mut package_assets = Vec::new(); + collect_compile_assets(package, &package.get_ocaml_build_path(), &mut package_assets); + package_assets }) .flatten() - .collect::>(); + .collect::>(); let root_config = build_state.get_root_config(); compile_assets.iter().for_each( - |(path, last_modified, extension, package_name, package_namespace, package_is_root)| { + |( + path, + last_modified, + extension, + package_name, + package_path, + package_multi_entry, + package_namespace, + package_is_root, + )| { match extension.as_str() { "iast" | "ast" => { - let module_name = helpers::file_path_to_module_name(path, package_namespace); - if let Some(res_file_path_buf) = get_res_path_from_ast(path) { + let source_path = res_file_path_buf + .strip_prefix(package_path) + .unwrap_or(&res_file_path_buf); + let module_name = helpers::file_path_to_module_key( + source_path, + package_namespace, + package_name, + *package_multi_entry, + ); let _ = ast_modules.insert( res_file_path_buf.clone(), AstModule { @@ -147,3 +153,40 @@ fn get_res_path_from_ast(ast_file: &Path) -> Option { } None } + +fn collect_compile_assets(package: &packages::Package, dir: &Path, assets: &mut Vec) { + let Ok(read_dir) = fs::read_dir(dir) else { + return; + }; + + for entry in read_dir.flatten() { + let path = entry.path(); + let Ok(metadata) = entry.metadata() else { + continue; + }; + + if metadata.is_dir() { + collect_compile_assets(package, &path, assets); + continue; + } + + let Some(extension) = path.extension().and_then(|extension| extension.to_str()) else { + continue; + }; + let extension = extension.to_owned(); + + match extension.as_str() { + "iast" | "ast" | "cmi" | "cmt" => assets.push(( + path, + metadata.modified().unwrap(), + extension, + package.name.to_owned(), + package.path.to_owned(), + package.config.is_multi_entry_enabled(), + package.namespace.to_owned(), + package.is_root, + )), + _ => {} + } + } +} diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index 300a4a9ba29..7f68b244a40 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -405,6 +405,8 @@ pub struct Config { pub gentype_config: Option, #[serde(rename = "js-post-build")] pub js_post_build: Option, + #[serde(rename = "multi-entry", default)] + pub multi_entry: bool, // Used by the VS Code extension; ignored by rewatch but should not emit warnings. // Payload is not validated here, only in the VS Code extension. pub editor: Option, @@ -611,6 +613,10 @@ impl Config { Ok(()) } + pub fn is_multi_entry_enabled(&self) -> bool { + self.multi_entry + } + pub fn get_namespace(&self) -> packages::Namespace { let namespace_from_package = namespace_from_package_name(&self.name); match (self.namespace.as_ref(), self.namespace_entry.as_ref()) { @@ -1077,6 +1083,7 @@ pub mod tests { jsx: None, gentype_config: None, js_post_build: None, + multi_entry: false, editor: None, reanalyze: None, namespace_entry: None, @@ -1321,6 +1328,35 @@ pub mod tests { assert!(config.get_gentype_args(&[], None, &[]).is_empty()); } + #[test] + fn test_multi_entry_defaults_to_false() { + let json = r#" + { + "name": "my-monorepo", + "sources": "src" + } + "#; + + let config = Config::new_from_json_string(json).unwrap(); + assert!(!config.is_multi_entry_enabled()); + assert!(config.get_unknown_fields().is_empty()); + } + + #[test] + fn test_multi_entry_can_be_enabled() { + let json = r#" + { + "name": "my-monorepo", + "sources": "src", + "multi-entry": true + } + "#; + + let config = Config::new_from_json_string(json).unwrap(); + assert!(config.is_multi_entry_enabled()); + assert!(config.get_unknown_fields().is_empty()); + } + #[test] fn test_other_jsx_module() { let json = r#" diff --git a/rewatch/src/helpers.rs b/rewatch/src/helpers.rs index 437fd05c1d5..a766f99e227 100644 --- a/rewatch/src/helpers.rs +++ b/rewatch/src/helpers.rs @@ -325,8 +325,16 @@ pub fn module_name_with_namespace(module_name: &str, namespace: &packages::Names capitalize(&add_suffix(module_name, namespace)) } -// this doesn't capitalize the module name! if the rescript name of the file is "foo.res" the -// compiler assets are foo-Namespace.cmt and foo-Namespace.cmj, but the module name is Foo +pub fn is_constructible_file_module(path: &Path, multi_entry: bool) -> bool { + !multi_entry + || get_basename(path) + .chars() + .next() + .is_some_and(|chr| chr.is_ascii_uppercase()) +} + +// Compiler asset basenames preserve the source file stem. Constructible module names are a +// separate concept because lowercase file modules intentionally stay private to the file level. pub fn file_path_to_compiler_asset_basename(path: &Path, namespace: &packages::Namespace) -> String { let base = get_basename(path); add_suffix(&base, namespace) @@ -336,6 +344,33 @@ pub fn file_path_to_module_name(path: &Path, namespace: &packages::Namespace) -> capitalize(&file_path_to_compiler_asset_basename(path, namespace)) } +pub fn file_path_to_constructible_module_name( + path: &Path, + namespace: &packages::Namespace, + multi_entry: bool, +) -> Option { + if is_constructible_file_module(path, multi_entry) { + Some(file_path_to_module_name(path, namespace)) + } else { + None + } +} + +pub fn file_path_to_module_key( + path: &Path, + namespace: &packages::Namespace, + package_name: &str, + multi_entry: bool, +) -> String { + match file_path_to_constructible_module_name(path, namespace, multi_entry) { + Some(module_name) => module_name, + None => { + let source_path = path.with_extension(""); + format!("{}:{}", package_name, source_path.to_string_lossy()) + } + } +} + pub fn contains_ascii_characters(str: &str) -> bool { for chr in str.chars() { if chr.is_ascii_alphanumeric() { @@ -395,6 +430,32 @@ pub fn get_ast_path(source_file: &Path) -> PathBuf { .join(format!("{basename}{extension}")) } +pub fn get_ocaml_ast_cache_path(package: &packages::Package, source_file: &Path) -> PathBuf { + let source_file = source_file.strip_prefix(&package.path).unwrap_or(source_file); + let ast_path = get_ast_path(source_file); + get_ocaml_ast_cache_path_from_ast_path(package, &ast_path) +} + +pub fn get_ocaml_ast_cache_path_for_extension( + package: &packages::Package, + source_file: &Path, + extension: &str, +) -> PathBuf { + let source_file = source_file.strip_prefix(&package.path).unwrap_or(source_file); + let ast_path = get_ast_path(source_file).with_extension(extension); + get_ocaml_ast_cache_path_from_ast_path(package, &ast_path) +} + +fn get_ocaml_ast_cache_path_from_ast_path(package: &packages::Package, ast_path: &Path) -> PathBuf { + if package.config.is_multi_entry_enabled() { + package.get_ocaml_build_path().join(ast_path) + } else { + package + .get_ocaml_build_path() + .join(ast_path.file_name().expect("AST path should have a file name")) + } +} + pub fn get_compiler_asset( package: &packages::Package, namespace: &packages::Namespace, @@ -550,3 +611,49 @@ pub fn is_local_package(workspace_path: &Path, canonical_package_path: &Path) -> .components() .any(|c| c.as_os_str() == "node_modules") } + +#[cfg(test)] +mod tests { + use super::*; + use crate::build::packages::Namespace; + + #[test] + fn constructible_module_names_require_uppercase_file_stems_when_multi_entry_enabled() { + assert_eq!( + file_path_to_constructible_module_name(Path::new("src/Foo.res"), &Namespace::NoNamespace, true), + Some("Foo".to_string()) + ); + assert_eq!( + file_path_to_constructible_module_name(Path::new("src/foo.res"), &Namespace::NoNamespace, true), + None + ); + } + + #[test] + fn lowercase_file_module_keys_include_package_and_path_when_multi_entry_enabled() { + assert_eq!( + file_path_to_module_key(Path::new("src/a/foo.res"), &Namespace::NoNamespace, "pkg", true), + "pkg:src/a/foo" + ); + assert_eq!( + file_path_to_module_key(Path::new("src/b/foo.res"), &Namespace::NoNamespace, "pkg", true), + "pkg:src/b/foo" + ); + } + + #[test] + fn lowercase_file_module_keys_use_old_capitalized_name_by_default() { + assert_eq!( + file_path_to_module_key(Path::new("src/foo.res"), &Namespace::NoNamespace, "pkg", false), + "Foo" + ); + } + + #[test] + fn uppercase_file_module_keys_stay_constructible_module_names() { + assert_eq!( + file_path_to_module_key(Path::new("src/Foo.res"), &Namespace::NoNamespace, "pkg", true), + "Foo" + ); + } +} diff --git a/rewatch/tests/compile/10-duplicate-module-name.sh b/rewatch/tests/compile/10-duplicate-module-name.sh index 38da1a24de5..fca5fb7dbf0 100755 --- a/rewatch/tests/compile/10-duplicate-module-name.sh +++ b/rewatch/tests/compile/10-duplicate-module-name.sh @@ -8,6 +8,68 @@ bold "Test: It should show an error for duplicate module names" rewatch clean &> /dev/null rewatch build &> /dev/null +cleanup_multi_entry_config() { + git checkout -- packages/main/rescript.json &> /dev/null || true +} +trap cleanup_multi_entry_config EXIT + +node -e 'const fs = require("fs"); const path = "packages/main/rescript.json"; const config = JSON.parse(fs.readFileSync(path, "utf8")); config["multi-entry"] = true; fs.writeFileSync(path, JSON.stringify(config, null, 2) + "\n");' + +mkdir -p packages/main/src/lower-a packages/main/src/lower-b +echo 'let value = 1' > packages/main/src/lower-a/lowercaseDuplicate.res +echo 'let value = 2' > packages/main/src/lower-b/lowercaseDuplicate.res +if rewatch build &> /dev/null; +then + success "Duplicate lowercase file module names are allowed" +else + error "Duplicate lowercase file module names should be allowed" + rm -rf packages/main/src/lower-a packages/main/src/lower-b + exit 1 +fi + +lower_a_ast=packages/main/lib/ocaml/src/lower-a/lowercaseDuplicate.ast +lower_b_ast=packages/main/lib/ocaml/src/lower-b/lowercaseDuplicate.ast +if [ -f "$lower_a_ast" ] && [ -f "$lower_b_ast" ] && [ ! -f packages/main/lib/ocaml/lowercaseDuplicate.ast ]; +then + success "Duplicate lowercase AST cache paths are unique" +else + error "Duplicate lowercase AST cache paths should preserve source directories" + rm -rf packages/main/src/lower-a packages/main/src/lower-b + exit 1 +fi + +rm -rf packages/main/src/lower-a packages/main/src/lower-b +rewatch build &> /dev/null +if [ -f "$lower_a_ast" ] || [ -f "$lower_b_ast" ]; +then + error "Removed lowercase file AST cache paths should be cleaned" + exit 1 +else + success "Removed lowercase file AST cache paths are cleaned" +fi + +mkdir -p packages/main/src/lower-private +echo 'let value = 1' > packages/main/src/lower-private/lowerPrivate.res +echo 'let value = LowerPrivate.value' > packages/main/src/lower-private/UseLowerPrivate.res +if rewatch build &> ../tests/snapshots/lowercase-file-module-private.txt; +then + error "Lowercase file modules should not be constructible" + rm -rf packages/main/src/lower-private + exit 1 +fi +normalize_paths ../tests/snapshots/lowercase-file-module-private.txt +if grep -q "The module or file LowerPrivate can't be found" ../tests/snapshots/lowercase-file-module-private.txt; +then + success "Lowercase file module names are not constructible" +else + error "Lowercase file module diagnostic changed" + cat ../tests/snapshots/lowercase-file-module-private.txt + rm -rf packages/main/src/lower-private + exit 1 +fi +rm -f ../tests/snapshots/lowercase-file-module-private.txt +rm -rf packages/main/src/lower-private + mkdir -p packages/main/src/dupe-a packages/main/src/dupe-b echo 'let value = 1' > packages/main/src/dupe-a/DuplicateModule.res echo 'let value = 2' > packages/main/src/dupe-b/DuplicateModule.res diff --git a/tests/ounit_tests/ounit_string_tests.ml b/tests/ounit_tests/ounit_string_tests.ml index aba2e4114c4..822038bba4e 100644 --- a/tests/ounit_tests/ounit_string_tests.ml +++ b/tests/ounit_tests/ounit_string_tests.ml @@ -82,6 +82,7 @@ let suites = end; *) ( __LOC__ >:: fun _ -> Ext_filename.module_name "a/hello.ml" =~ "Hello"; + Ext_filename.module_name ~preserve_case:true "a/hello.ml" =~ "hello"; Ext_filename.as_module ~basename:"a.ml" =~ Some {module_name = "A"; case = false}; Ext_filename.as_module ~basename:"Aa.ml" @@ -410,6 +411,9 @@ let suites = string_eq (Ext_filename.module_name "a/b/xc.cppo.mli") "Xc.cppo"; string_eq (Ext_filename.module_name "a/b/xc.cppo.") "Xc.cppo"; string_eq (Ext_filename.module_name "a/b/xc..") "Xc."; + string_eq + (Ext_filename.module_name ~preserve_case:true "a/b/xc.res") + "xc"; string_eq (Ext_filename.module_name "a/b/Xc..") "Xc."; string_eq (Ext_filename.module_name "a/b/.") "" ); ( __LOC__ >:: fun _ -> diff --git a/yarn.lock b/yarn.lock index 9ec62921179..29a5bbd5934 100644 --- a/yarn.lock +++ b/yarn.lock @@ -422,25 +422,25 @@ __metadata: languageName: node linkType: hard -"@rescript/darwin-arm64@workspace:packages/@rescript/darwin-arm64": +"@rescript/darwin-arm64@workspace:*, @rescript/darwin-arm64@workspace:packages/@rescript/darwin-arm64": version: 0.0.0-use.local resolution: "@rescript/darwin-arm64@workspace:packages/@rescript/darwin-arm64" languageName: unknown linkType: soft -"@rescript/darwin-x64@workspace:packages/@rescript/darwin-x64": +"@rescript/darwin-x64@workspace:*, @rescript/darwin-x64@workspace:packages/@rescript/darwin-x64": version: 0.0.0-use.local resolution: "@rescript/darwin-x64@workspace:packages/@rescript/darwin-x64" languageName: unknown linkType: soft -"@rescript/linux-arm64@workspace:packages/@rescript/linux-arm64": +"@rescript/linux-arm64@workspace:*, @rescript/linux-arm64@workspace:packages/@rescript/linux-arm64": version: 0.0.0-use.local resolution: "@rescript/linux-arm64@workspace:packages/@rescript/linux-arm64" languageName: unknown linkType: soft -"@rescript/linux-x64@workspace:packages/@rescript/linux-x64": +"@rescript/linux-x64@workspace:*, @rescript/linux-x64@workspace:packages/@rescript/linux-x64": version: 0.0.0-use.local resolution: "@rescript/linux-x64@workspace:packages/@rescript/linux-x64" languageName: unknown @@ -492,7 +492,7 @@ __metadata: languageName: unknown linkType: soft -"@rescript/runtime@workspace:packages/@rescript/runtime": +"@rescript/runtime@workspace:*, @rescript/runtime@workspace:packages/@rescript/runtime": version: 0.0.0-use.local resolution: "@rescript/runtime@workspace:packages/@rescript/runtime" dependencies: @@ -500,7 +500,7 @@ __metadata: languageName: unknown linkType: soft -"@rescript/win32-x64@workspace:packages/@rescript/win32-x64": +"@rescript/win32-x64@workspace:*, @rescript/win32-x64@workspace:packages/@rescript/win32-x64": version: 0.0.0-use.local resolution: "@rescript/win32-x64@workspace:packages/@rescript/win32-x64" languageName: unknown @@ -2431,12 +2431,12 @@ __metadata: resolution: "rescript@workspace:." dependencies: "@biomejs/biome": "npm:2.4.10" - "@rescript/darwin-arm64": "workspace:packages/@rescript/darwin-arm64" - "@rescript/darwin-x64": "workspace:packages/@rescript/darwin-x64" - "@rescript/linux-arm64": "workspace:packages/@rescript/linux-arm64" - "@rescript/linux-x64": "workspace:packages/@rescript/linux-x64" - "@rescript/runtime": "workspace:packages/@rescript/runtime" - "@rescript/win32-x64": "workspace:packages/@rescript/win32-x64" + "@rescript/darwin-arm64": "workspace:*" + "@rescript/darwin-x64": "workspace:*" + "@rescript/linux-arm64": "workspace:*" + "@rescript/linux-x64": "workspace:*" + "@rescript/runtime": "workspace:*" + "@rescript/win32-x64": "workspace:*" "@types/node": "npm:^25.0.3" "@types/semver": "npm:^7.7.0" "@yarnpkg/types": "npm:^4.0.1" @@ -2453,6 +2453,8 @@ __metadata: optional: true "@rescript/linux-x64": optional: true + "@rescript/runtime": + optional: true "@rescript/win32-x64": optional: true bin: