From 037c59f37f7a5afd84fb706b82adc8277a34e37c Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sat, 19 Jul 2025 15:40:21 +0200 Subject: [PATCH 01/12] ci: add clippy --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab533045a1..34047e52ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -80,6 +80,7 @@ jobs: OPAM_VERSION: 2.3.0 DUNE_PROFILE: release RUST_BACKTRACE: "1" + RUSTFLAGS: "-Dwarnings" steps: - name: "Windows: Set git config" @@ -131,6 +132,11 @@ jobs: run: | cargo build --manifest-path rewatch/Cargo.toml --target ${{ matrix.rust-target }} --release + - name: Lint rewatch + if: steps.rewatch-build-cache.outputs.cache-hit != 'true' + run: | + cargo clippy --manifest-path rewatch/Cargo.toml --all-targets --all-features + - name: Run rewatch unit tests if: steps.rewatch-build-cache.outputs.cache-hit != 'true' run: | From 6db30a22d6ac0242103b3af564fb51f536af9a42 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sat, 19 Jul 2025 15:47:30 +0200 Subject: [PATCH 02/12] fix: warning to show ci failure --- rewatch/src/watcher.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewatch/src/watcher.rs b/rewatch/src/watcher.rs index 2843a9d110..79829d1c20 100644 --- a/rewatch/src/watcher.rs +++ b/rewatch/src/watcher.rs @@ -324,7 +324,7 @@ pub fn start( ) .await { - println!("{:?}", e) + println!("{e:?}") } }) } From 3abe00285551bae6d4db9b47d465239c647acd32 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sat, 19 Jul 2025 15:51:36 +0200 Subject: [PATCH 03/12] fix: run `cargo clippy --fix` --- rewatch/src/build.rs | 30 ++-- rewatch/src/build/compile.rs | 139 ++++++++---------- rewatch/src/build/compile/dependency_cycle.rs | 4 +- rewatch/src/build/deps.rs | 8 +- rewatch/src/build/namespaces.rs | 4 +- rewatch/src/build/packages.rs | 34 ++--- rewatch/src/build/parse.rs | 8 +- rewatch/src/build/read_compile_state.rs | 4 +- rewatch/src/config.rs | 4 +- rewatch/src/format.rs | 4 +- rewatch/src/helpers.rs | 22 +-- rewatch/src/lock.rs | 13 +- rewatch/src/sourcedirs.rs | 2 +- rewatch/src/watcher.rs | 17 +-- 14 files changed, 137 insertions(+), 156 deletions(-) diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index e72a13b31d..b4a7673540 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -58,7 +58,7 @@ pub struct CompilerArgs { pub fn get_compiler_args(path: &Path) -> Result { let filename = &helpers::get_abs_path(path); let package_root = - helpers::get_abs_path(&helpers::get_nearest_config(&path).expect("Couldn't find package root")); + helpers::get_abs_path(&helpers::get_nearest_config(path).expect("Couldn't find package root")); let workspace_root = get_workspace_root(&package_root).map(|p| helpers::get_abs_path(&p)); let root_rescript_config = packages::read_config(&workspace_root.to_owned().unwrap_or(package_root.to_owned()))?; @@ -77,7 +77,7 @@ pub fn get_compiler_args(path: &Path) -> Result { let (ast_path, parser_args) = parser_args( &rescript_config, &root_rescript_config, - &relative_filename, + relative_filename, &workspace_root, workspace_root.as_ref().unwrap_or(&package_root), &contents, @@ -94,7 +94,7 @@ pub fn get_compiler_args(path: &Path) -> Result { &rescript_config, &root_rescript_config, &ast_path, - &relative_filename, + relative_filename, is_interface, has_interface, &package_root, @@ -216,7 +216,7 @@ pub fn initialize_build( if show_progress { if snapshot_output { - println!("Cleaned {}/{}", diff_cleanup, total_cleanup) + println!("Cleaned {diff_cleanup}/{total_cleanup}") } else { println!( "{}{} {}Cleaned {}/{} {:.2}s", @@ -234,7 +234,7 @@ pub fn initialize_build( } fn format_step(current: usize, total: usize) -> console::StyledObject { - style(format!("[{}/{}]", current, total)).bold().dim() + style(format!("[{current}/{total}]")).bold().dim() } #[derive(Debug, Clone)] @@ -254,23 +254,23 @@ impl fmt::Display for IncrementalBuildError { match &self.kind { IncrementalBuildErrorKind::SourceFileParseError => { if self.snapshot_output { - write!(f, "{} Could not parse Source Files", LINE_CLEAR,) + write!(f, "{LINE_CLEAR} Could not parse Source Files",) } else { - write!(f, "{} {}Could not parse Source Files", LINE_CLEAR, CROSS,) + write!(f, "{LINE_CLEAR} {CROSS}Could not parse Source Files",) } } IncrementalBuildErrorKind::CompileError(Some(e)) => { if self.snapshot_output { - write!(f, "{} Failed to Compile. Error: {e}", LINE_CLEAR,) + write!(f, "{LINE_CLEAR} Failed to Compile. Error: {e}",) } else { - write!(f, "{} {}Failed to Compile. Error: {e}", LINE_CLEAR, CROSS,) + write!(f, "{LINE_CLEAR} {CROSS}Failed to Compile. Error: {e}",) } } IncrementalBuildErrorKind::CompileError(None) => { if self.snapshot_output { - write!(f, "{} Failed to Compile. See Errors Above", LINE_CLEAR,) + write!(f, "{LINE_CLEAR} Failed to Compile. See Errors Above",) } else { - write!(f, "{} {}Failed to Compile. See Errors Above", LINE_CLEAR, CROSS,) + write!(f, "{LINE_CLEAR} {CROSS}Failed to Compile. See Errors Above",) } } } @@ -312,7 +312,7 @@ pub fn incremental_build( Ok(_ast) => { if show_progress { if snapshot_output { - println!("Parsed {} source files", num_dirty_modules) + println!("Parsed {num_dirty_modules} source files") } else { println!( "{}{} {}Parsed {} source files in {:.2}s", @@ -370,7 +370,7 @@ pub fn incremental_build( if log_enabled!(log::Level::Trace) { for (module_name, module) in build_state.modules.iter() { if module.compile_dirty { - println!("compile dirty: {}", module_name); + println!("compile dirty: {module_name}"); } } }; @@ -411,7 +411,7 @@ pub fn incremental_build( if !compile_errors.is_empty() { if show_progress { if snapshot_output { - println!("Compiled {} modules", num_compiled_modules) + println!("Compiled {num_compiled_modules} modules") } else { println!( "{}{} {}Compiled {} modules in {:.2}s", @@ -439,7 +439,7 @@ pub fn incremental_build( } else { if show_progress { if snapshot_output { - println!("Compiled {} modules", num_compiled_modules) + println!("Compiled {num_compiled_modules} modules") } else { println!( "{}{} {}Compiled {} modules in {:.2}s", diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs index a3e78383fb..81044de5ba 100644 --- a/rewatch/src/build/compile.rs +++ b/rewatch/src/build/compile.rs @@ -417,8 +417,8 @@ pub fn compiler_args( specs .iter() - .map(|spec| { - return vec![ + .flat_map(|spec| { + vec![ "-bs-package-output".to_string(), format!( "{}:{}:{}", @@ -437,9 +437,8 @@ pub fn compiler_args( }, root_config.get_suffix(spec), ), - ]; + ] }) - .flatten() .collect() }; @@ -614,8 +613,7 @@ fn compile_file( Err(stderr.to_string() + &stdout) } Err(e) => Err(format!( - "Could not compile file. Error: {}. Path to AST: {:?}", - e, ast_path + "Could not compile file. Error: {e}. Path to AST: {ast_path:?}" )), Ok(x) => { let err = std::str::from_utf8(&x.stderr) @@ -633,15 +631,15 @@ fn compile_file( // because editor tooling doesn't support namespace entries yet // we just remove the @ for now. This makes sure the editor support // doesn't break - .join(format!("{}.cmi", module_name)), - ocaml_build_path_abs.join(format!("{}.cmi", module_name)), + .join(format!("{module_name}.cmi")), + ocaml_build_path_abs.join(format!("{module_name}.cmi")), ); let _ = std::fs::copy( package .get_build_path() .join(dir) - .join(format!("{}.cmj", module_name)), - ocaml_build_path_abs.join(format!("{}.cmj", module_name)), + .join(format!("{module_name}.cmj")), + ocaml_build_path_abs.join(format!("{module_name}.cmj")), ); let _ = std::fs::copy( package @@ -650,98 +648,89 @@ fn compile_file( // because editor tooling doesn't support namespace entries yet // we just remove the @ for now. This makes sure the editor support // doesn't break - .join(format!("{}.cmt", module_name)), - ocaml_build_path_abs.join(format!("{}.cmt", module_name)), + .join(format!("{module_name}.cmt")), + ocaml_build_path_abs.join(format!("{module_name}.cmt")), ); } else { let _ = std::fs::copy( package .get_build_path() .join(dir) - .join(format!("{}.cmti", module_name)), - ocaml_build_path_abs.join(format!("{}.cmti", module_name)), + .join(format!("{module_name}.cmti")), + ocaml_build_path_abs.join(format!("{module_name}.cmti")), ); let _ = std::fs::copy( package .get_build_path() .join(dir) - .join(format!("{}.cmi", module_name)), - ocaml_build_path_abs.join(format!("{}.cmi", module_name)), + .join(format!("{module_name}.cmi")), + ocaml_build_path_abs.join(format!("{module_name}.cmi")), ); } - match &module.source_type { - SourceType::SourceFile(SourceFile { + if let SourceType::SourceFile(SourceFile { interface: Some(Interface { path, .. }), .. - }) => { - // we need to copy the source file to the build directory. - // editor tools expects the source file in lib/bs for finding the current package - // and in lib/ocaml when referencing modules in other packages - let _ = std::fs::copy( - Path::new(&package.path).join(path), - package.get_build_path().join(path), - ) - .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"); - } - _ => (), + }) = &module.source_type { + // we need to copy the source file to the build directory. + // editor tools expects the source file in lib/bs for finding the current package + // and in lib/ocaml when referencing modules in other packages + let _ = std::fs::copy( + Path::new(&package.path).join(path), + package.get_build_path().join(path), + ) + .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"); } - match &module.source_type { - SourceType::SourceFile(SourceFile { + if let SourceType::SourceFile(SourceFile { implementation: Implementation { path, .. }, .. - }) => { - // we need to copy the source file to the build directory. - // editor tools expects the source file in lib/bs for finding the current package - // and in lib/ocaml when referencing modules in other packages - let _ = std::fs::copy( - Path::new(&package.path).join(path), - package.get_build_path().join(path), - ) - .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"); - } - _ => (), + }) = &module.source_type { + // we need to copy the source file to the build directory. + // editor tools expects the source file in lib/bs for finding the current package + // and in lib/ocaml when referencing modules in other packages + let _ = std::fs::copy( + Path::new(&package.path).join(path), + package.get_build_path().join(path), + ) + .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"); } // copy js file root_package.config.get_package_specs().iter().for_each(|spec| { if spec.in_source { - match &module.source_type { - SourceType::SourceFile(SourceFile { + if let SourceType::SourceFile(SourceFile { implementation: Implementation { path, .. }, .. - }) => { - let source = helpers::get_source_file_from_rescript_file( - &Path::new(&package.path).join(path), - &root_package.config.get_suffix(spec), - ); - let destination = helpers::get_source_file_from_rescript_file( - &package.get_build_path().join(path), - &root_package.config.get_suffix(spec), - ); - - if source.exists() { - let _ = - std::fs::copy(&source, &destination).expect("copying source file failed"); - } + }) = &module.source_type { + let source = helpers::get_source_file_from_rescript_file( + &Path::new(&package.path).join(path), + &root_package.config.get_suffix(spec), + ); + let destination = helpers::get_source_file_from_rescript_file( + &package.get_build_path().join(path), + &root_package.config.get_suffix(spec), + ); + + if source.exists() { + let _ = + std::fs::copy(&source, &destination).expect("copying source file failed"); } - _ => (), } } }); diff --git a/rewatch/src/build/compile/dependency_cycle.rs b/rewatch/src/build/compile/dependency_cycle.rs index b96ea8eb8d..a048d0ad8d 100644 --- a/rewatch/src/build/compile/dependency_cycle.rs +++ b/rewatch/src/build/compile/dependency_cycle.rs @@ -60,12 +60,12 @@ fn find_shortest_cycle(modules: &Vec<(&String, &Module)>) -> Vec { } // Skip nodes with no incoming edges - if in_degrees.get(&start_node).map_or(true, |&d| d == 0) { + if in_degrees.get(&start_node).is_none_or(|&d| d == 0) { no_cycle_cache.insert(start_node.clone()); continue; } - if let Some(cycle) = find_cycle_bfs(&start_node, &graph, current_shortest_length) { + if let Some(cycle) = find_cycle_bfs(start_node, &graph, current_shortest_length) { if shortest_cycle.is_empty() || cycle.len() < shortest_cycle.len() { shortest_cycle = cycle.clone(); current_shortest_length = cycle.len(); diff --git a/rewatch/src/build/deps.rs b/rewatch/src/build/deps.rs index 947915c385..6bf8c1cd0a 100644 --- a/rewatch/src/build/deps.rs +++ b/rewatch/src/build/deps.rs @@ -45,7 +45,7 @@ fn get_dep_modules( .cloned() .collect(); - return deps + deps .iter() .map(|dep| { let dep_first = dep.split('.').next().unwrap(); @@ -94,7 +94,7 @@ fn get_dep_modules( true }) - .collect::>(); + .collect::>() } pub fn get_deps(build_state: &mut BuildState, deleted_modules: &AHashSet) { @@ -115,7 +115,7 @@ pub fn get_deps(build_state: &mut BuildState, deleted_modules: &AHashSet package.namespace.to_suffix(), package.modules.as_ref().unwrap(), all_mod, - &package, + package, build_state, ); @@ -127,7 +127,7 @@ pub fn get_deps(build_state: &mut BuildState, deleted_modules: &AHashSet package.namespace.to_suffix(), package.modules.as_ref().unwrap(), all_mod, - &package, + package, build_state, )) } diff --git a/rewatch/src/build/namespaces.rs b/rewatch/src/build/namespaces.rs index c5d8355e28..4bd63501b5 100644 --- a/rewatch/src/build/namespaces.rs +++ b/rewatch/src/build/namespaces.rs @@ -31,7 +31,7 @@ pub fn gen_mlmap( // recompile in a different way but we need to put it in the file for it to // be readable. - let path = build_path_abs.join(format!("{}.mlmap", namespace)); + let path = build_path_abs.join(format!("{namespace}.mlmap")); let mut file = File::create(&path).expect("Unable to create mlmap"); file.write_all(b"randjbuildsystem\n") @@ -53,7 +53,7 @@ pub fn gen_mlmap( pub fn compile_mlmap(package: &packages::Package, namespace: &str, bsc_path: &Path) { let build_path_abs = package.get_build_path(); - let mlmap_name = format!("{}.mlmap", namespace); + let mlmap_name = format!("{namespace}.mlmap"); let args = vec!["-w", "-49", "-color", "always", "-no-alias-deps", &mlmap_name]; let _ = Command::new(bsc_path) diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 2b6a8c1d16..86529b942d 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -103,7 +103,7 @@ impl Package { .namespace .to_suffix() .expect("namespace should be set for mlmap module"); - self.get_build_path().join(format!("{}.mlmap", suffix)) + self.get_build_path().join(format!("{suffix}.mlmap")) } pub fn get_mlmap_compile_path(&self) -> PathBuf { @@ -111,7 +111,7 @@ impl Package { .namespace .to_suffix() .expect("namespace should be set for mlmap module"); - self.get_build_path().join(format!("{}.cmi", suffix)) + self.get_build_path().join(format!("{suffix}.cmi")) } pub fn is_source_file_type_dev(&self, path: &Path) -> bool { @@ -171,7 +171,7 @@ pub fn read_folders( if metadata.file_type().is_dir() && recurse { match read_folders(filter, package_dir, &new_path, recurse, is_type_dev) { Ok(s) => map.extend(s), - Err(e) => log::error!("Could not read directory: {}", e), + Err(e) => log::error!("Could not read directory: {e}"), } } @@ -189,8 +189,8 @@ pub fn read_folders( ); } - Ok(_) => log::info!("Filtered: {:?}", name), - Err(ref e) => log::error!("Could not read directory: {}", e), + Ok(_) => log::info!("Filtered: {name:?}"), + Err(ref e) => log::error!("Could not read directory: {e}"), }, _ => (), } @@ -255,11 +255,11 @@ pub fn read_dependency( project_root: &Path, workspace_root: &Option, ) -> Result { - let path_from_parent = PathBuf::from(helpers::package_path(parent_path, package_name)); - let path_from_project_root = PathBuf::from(helpers::package_path(project_root, package_name)); + let path_from_parent = helpers::package_path(parent_path, package_name); + let path_from_project_root = helpers::package_path(project_root, package_name); let maybe_path_from_workspace_root = workspace_root .as_ref() - .map(|workspace_root| PathBuf::from(helpers::package_path(workspace_root, package_name))); + .map(|workspace_root| helpers::package_path(workspace_root, package_name)); let path = match ( path_from_parent, @@ -272,8 +272,7 @@ pub fn read_dependency( Ok(path_from_workspace_root) } _ => Err(format!( - "The package \"{}\" is not found (are node_modules up-to-date?)...", - package_name + "The package \"{package_name}\" is not found (are node_modules up-to-date?)..." )), }?; @@ -332,7 +331,7 @@ fn read_dependencies( .par_iter() .map(|package_name| { let (config, canonical_path) = - match read_dependency(package_name, parent_path, project_root, &workspace_root) { + match read_dependency(package_name, parent_path, project_root, workspace_root) { Err(error) => { if show_progress { println!( @@ -537,8 +536,7 @@ pub fn get_source_files( let is_type_dev = type_ .as_ref() .map(|t| t.as_str() == "dev") - .unwrap_or(false) - .clone(); + .unwrap_or(false); match (build_dev_deps, type_) { (false, Some(type_)) if type_ == "dev" => (), _ => match read_folders(filter, package_dir, path_dir, recurse, is_type_dev) { @@ -635,7 +633,7 @@ pub fn parse_packages(build_state: &mut BuildState) { .clone() .iter() .for_each(|(package_name, package)| { - debug!("Parsing package: {}", package_name); + debug!("Parsing package: {package_name}"); if let Some(package_modules) = package.modules.to_owned() { build_state.module_names.extend(package_modules) } @@ -668,7 +666,7 @@ pub fn parse_packages(build_state: &mut BuildState) { helpers::create_path(&package.get_js_path()); relative_dirs.iter().for_each(|path_buf| { helpers::create_path_for_path(&Path::join( - &PathBuf::from(package.get_js_path()), + &package.get_js_path(), path_buf, )) }) @@ -676,7 +674,7 @@ pub fn parse_packages(build_state: &mut BuildState) { helpers::create_path(&package.get_es6_path()); relative_dirs.iter().for_each(|path_buf| { helpers::create_path_for_path(&Path::join( - &PathBuf::from(package.get_es6_path()), + &package.get_es6_path(), path_buf, )) }) @@ -757,7 +755,7 @@ pub fn parse_packages(build_state: &mut BuildState) { 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_name = helpers::file_path_to_module_name(file, &namespace); if helpers::is_implementation_file(extension) { build_state @@ -970,7 +968,7 @@ pub fn validate_packages_dependencies(packages: &AHashMap) -> b } }); } - let has_any_unallowed_dependent = detected_unallowed_dependencies.len() > 0; + let has_any_unallowed_dependent = !detected_unallowed_dependencies.is_empty(); if has_any_unallowed_dependent { log::error!( diff --git a/rewatch/src/build/parse.rs b/rewatch/src/build/parse.rs index 4298f75f71..7412137ff0 100644 --- a/rewatch/src/build/parse.rs +++ b/rewatch/src/build/parse.rs @@ -22,7 +22,7 @@ pub fn generate_asts( .modules .par_iter() .map(|(module_name, module)| { - debug!("Generating AST for module: {}", module_name); + debug!("Generating AST for module: {module_name}"); let package = build_state .get_package(&module.package_name) .expect("Package not found"); @@ -199,9 +199,9 @@ pub fn generate_asts( // probably better to do this in a different function // specific to compiling mlmaps let compile_path = package.get_mlmap_compile_path(); - let mlmap_hash = helpers::compute_file_hash(&Path::new(&compile_path)); + let mlmap_hash = helpers::compute_file_hash(Path::new(&compile_path)); namespaces::compile_mlmap(package, module_name, &build_state.bsc_path); - let mlmap_hash_after = helpers::compute_file_hash(&Path::new(&compile_path)); + let mlmap_hash_after = helpers::compute_file_hash(Path::new(&compile_path)); let suffix = package .namespace @@ -348,7 +348,7 @@ fn generate_ast( }; if let Ok((ast_path, _)) = &result { let _ = std::fs::copy( - Path::new(&build_path_abs).join(&ast_path), + Path::new(&build_path_abs).join(ast_path), package.get_ocaml_build_path().join(ast_path.file_name().unwrap()), ); } diff --git a/rewatch/src/build/read_compile_state.rs b/rewatch/src/build/read_compile_state.rs index c6a05e445d..0f516fc6d0 100644 --- a/rewatch/src/build/read_compile_state.rs +++ b/rewatch/src/build/read_compile_state.rs @@ -45,7 +45,7 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { .packages .par_iter() .map(|(_, package)| { - let read_dir = fs::read_dir(&package.get_ocaml_build_path()).unwrap(); + let read_dir = fs::read_dir(package.get_ocaml_build_path()).unwrap(); read_dir .filter_map(|entry| match entry { Ok(entry) => { @@ -83,7 +83,7 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { .packages .get(&build_state.root_config_name) .expect("Could not find root package"); - if let Some(res_file_path_buf) = get_res_path_from_ast(&path) { + if let Some(res_file_path_buf) = get_res_path_from_ast(path) { let _ = ast_modules.insert( res_file_path_buf.clone(), AstModule { diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index 2c62a2c900..dc63912e26 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -434,7 +434,7 @@ impl Config { Some(version) if version == 4 => { vec!["-bs-jsx".to_string(), version.to_string()] } - Some(version) => panic!("JSX version {} is unsupported", version), + Some(version) => panic!("JSX version {version} is unsupported"), None => vec![], }, None => vec![], @@ -843,7 +843,7 @@ pub mod tests { let config = serde_json::from_str::(json).unwrap(); assert_eq!( - config.get_suffix(&config.get_package_specs().first().unwrap()), + config.get_suffix(config.get_package_specs().first().unwrap()), ".mjs" ); } diff --git a/rewatch/src/format.rs b/rewatch/src/format.rs index 2c64247907..bbcb0b46d9 100644 --- a/rewatch/src/format.rs +++ b/rewatch/src/format.rs @@ -101,7 +101,7 @@ fn format_files(bsc_exe: &Path, files: Vec, check: bool) -> Result<()> { let original_content = fs::read_to_string(file)?; let formatted_content = String::from_utf8_lossy(&output.stdout); if original_content != formatted_content { - eprintln!("[format check] {}", file); + eprintln!("[format check] {file}"); incorrectly_formatted_files.fetch_add(1, Ordering::SeqCst); } } @@ -118,7 +118,7 @@ fn format_files(bsc_exe: &Path, files: Vec, check: bool) -> Result<()> { if count == 1 { eprintln!("The file listed above needs formatting"); } else { - eprintln!("The {} files listed above need formatting", count); + eprintln!("The {count} files listed above need formatting"); } bail!("Formatting check failed"); } diff --git a/rewatch/src/helpers.rs b/rewatch/src/helpers.rs index e673135a1d..eb58726770 100644 --- a/rewatch/src/helpers.rs +++ b/rewatch/src/helpers.rs @@ -105,18 +105,18 @@ pub fn package_path(root: &Path, package_name: &str) -> PathBuf { pub fn get_abs_path(path: &Path) -> PathBuf { let abs_path_buf = PathBuf::from(path); - return abs_path_buf + abs_path_buf .to_lexical_absolute() - .expect("Could not canonicalize"); + .expect("Could not canonicalize") } pub fn get_basename(path: &Path) -> String { - return path + path .file_stem() .expect("Could not get basename") .to_str() .expect("Could not get basename 2") - .to_string(); + .to_string() } /// Capitalizes the first character in s. @@ -234,7 +234,7 @@ pub fn get_ast_path(source_file: &Path) -> PathBuf { source_path .parent() .unwrap() - .join(format!("{}{}", basename, extension)) + .join(format!("{basename}{extension}")) } pub fn get_compiler_asset( @@ -250,14 +250,14 @@ pub fn get_compiler_asset( let basename = file_path_to_compiler_asset_basename(source_file, namespace); package .get_ocaml_build_path() - .join(format!("{}.{}", basename, extension)) + .join(format!("{basename}.{extension}")) } pub fn canonicalize_string_path(path: &str) -> Option { - return Path::new(path) + Path::new(path) .canonicalize() .map(StrippedVerbatimPath::to_stripped_verbatim_path) - .ok(); + .ok() } pub fn get_bs_compiler_asset( @@ -277,7 +277,7 @@ pub fn get_bs_compiler_asset( package .get_build_path() .join(dir) - .join(format!("{}{}", basename, extension)) + .join(format!("{basename}{extension}")) .to_str() .unwrap() .to_owned() @@ -327,12 +327,12 @@ pub fn is_non_exotic_module_name(module_name: &str) -> bool { } pub fn get_extension(path: &Path) -> String { - return path + path .extension() .expect("Could not get extension") .to_str() .expect("Could not get extension 2") - .to_string(); + .to_string() } pub fn format_namespaced_module_name(module_name: &str) -> String { diff --git a/rewatch/src/lock.rs b/rewatch/src/lock.rs index d81e54c1c4..e5427cc8f1 100644 --- a/rewatch/src/lock.rs +++ b/rewatch/src/lock.rs @@ -22,20 +22,17 @@ impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { let msg = match self { Error::Locked(pid) => format!( - "A ReScript build is already running. The process ID (PID) is {}", - pid + "A ReScript build is already running. The process ID (PID) is {pid}" ), Error::ParsingLockfile(e) => format!( - "Could not parse lockfile: \n {} \n (try removing it and running the command again)", - e + "Could not parse lockfile: \n {e} \n (try removing it and running the command again)" ), Error::ReadingLockfile(e) => format!( - "Could not read lockfile: \n {} \n (try removing it and running the command again)", - e + "Could not read lockfile: \n {e} \n (try removing it and running the command again)" ), - Error::WritingLockfile(e) => format!("Could not write lockfile: \n {}", e), + Error::WritingLockfile(e) => format!("Could not write lockfile: \n {e}"), }; - write!(f, "{}", msg) + write!(f, "{msg}") } } diff --git a/rewatch/src/sourcedirs.rs b/rewatch/src/sourcedirs.rs index 411607759b..f15b275a2b 100644 --- a/rewatch/src/sourcedirs.rs +++ b/rewatch/src/sourcedirs.rs @@ -21,7 +21,7 @@ pub struct SourceDirs<'a> { } fn package_to_dirs(package: &Package, root_package_path: &Path) -> AHashSet { - let relative_path = package.path.strip_prefix(&root_package_path).unwrap(); + let relative_path = package.path.strip_prefix(root_package_path).unwrap(); package .dirs diff --git a/rewatch/src/watcher.rs b/rewatch/src/watcher.rs index 79829d1c20..1a4f5746cf 100644 --- a/rewatch/src/watcher.rs +++ b/rewatch/src/watcher.rs @@ -103,16 +103,13 @@ async fn async_watch( for event in events { // if there is a file named rescript.lock in the events path, we can quit the watcher - if let Some(_) = event.paths.iter().find(|path| path.ends_with(LOCKFILE)) { - match event.kind { - EventKind::Remove(_) => { - if show_progress { - println!("\nExiting... (lockfile removed)"); - } - clean::cleanup_after_build(&build_state); - return Ok(()); + if event.paths.iter().any(|path| path.ends_with(LOCKFILE)) { + if let EventKind::Remove(_) = event.kind { + if show_progress { + println!("\nExiting... (lockfile removed)"); } - _ => (), + clean::cleanup_after_build(&build_state); + return Ok(()); } } @@ -228,7 +225,7 @@ async fn async_watch( if show_progress { let compilation_type = if initial_build { "initial" } else { "incremental" }; if snapshot_output { - println!("Finished {} compilation", compilation_type) + println!("Finished {compilation_type} compilation") } else { println!( "\n{}{}Finished {} compilation in {:.2}s\n", From 91822a1f89eee5bd6b46b29788e4685504174302 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sat, 19 Jul 2025 15:54:00 +0200 Subject: [PATCH 04/12] fix: rogue comment --- rewatch/src/build/packages.rs | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 86529b942d..d5016c1a29 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -292,8 +292,6 @@ pub fn read_dependency( Ok(canonical_path) } -/// # Make Package - /// Given a config, recursively finds all dependencies. /// 1. It starts with registering dependencies and /// prevents the operation for the ones which are already @@ -533,10 +531,7 @@ pub fn get_source_files( }; let path_dir = Path::new(&source.dir); - let is_type_dev = type_ - .as_ref() - .map(|t| t.as_str() == "dev") - .unwrap_or(false); + let is_type_dev = type_.as_ref().map(|t| t.as_str() == "dev").unwrap_or(false); match (build_dev_deps, type_) { (false, Some(type_)) if type_ == "dev" => (), _ => match read_folders(filter, package_dir, path_dir, recurse, is_type_dev) { @@ -665,18 +660,12 @@ pub fn parse_packages(build_state: &mut BuildState) { if spec.is_common_js() { helpers::create_path(&package.get_js_path()); relative_dirs.iter().for_each(|path_buf| { - helpers::create_path_for_path(&Path::join( - &package.get_js_path(), - path_buf, - )) + helpers::create_path_for_path(&Path::join(&package.get_js_path(), path_buf)) }) } else { helpers::create_path(&package.get_es6_path()); relative_dirs.iter().for_each(|path_buf| { - helpers::create_path_for_path(&Path::join( - &package.get_es6_path(), - path_buf, - )) + helpers::create_path_for_path(&Path::join(&package.get_es6_path(), path_buf)) }) } } From 976aad18a8a2aa605b18347ce9ef65b18d1abf79 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sat, 19 Jul 2025 15:58:26 +0200 Subject: [PATCH 05/12] fix: redundant match --- rewatch/src/config.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index dc63912e26..dbdcbcb2a3 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -159,10 +159,7 @@ impl PackageSpec { } pub fn is_common_js(&self) -> bool { - match self.module.as_str() { - "commonjs" => true, - _ => false, - } + self.module.as_str() == "commonjs" } pub fn get_suffix(&self) -> Option { From 2a540e571b136cab98eeb9b83a5e520572be8621 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sat, 19 Jul 2025 16:04:06 +0200 Subject: [PATCH 06/12] fix: too many args by introducing a wrapper struct --- rewatch/src/watcher.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/rewatch/src/watcher.rs b/rewatch/src/watcher.rs index 1a4f5746cf..b0cb187c97 100644 --- a/rewatch/src/watcher.rs +++ b/rewatch/src/watcher.rs @@ -55,15 +55,28 @@ fn matches_filter(path_buf: &Path, filter: &Option) -> bool { filter.as_ref().map(|re| !re.is_match(&name)).unwrap_or(true) } -async fn async_watch( +struct AsyncWatchArgs<'a> { q: Arc>>, - path: &Path, + path: &'a Path, show_progress: bool, - filter: &Option, + filter: &'a Option, after_build: Option, create_sourcedirs: bool, build_dev_deps: bool, snapshot_output: bool, +} + +async fn async_watch( + AsyncWatchArgs { + q, + path, + show_progress, + filter, + after_build, + create_sourcedirs, + build_dev_deps, + snapshot_output, + }: AsyncWatchArgs<'_>, ) -> notify::Result<()> { let mut build_state = build::initialize_build(None, filter, show_progress, path, build_dev_deps, snapshot_output) @@ -309,8 +322,8 @@ pub fn start( let path = Path::new(folder); - if let Err(e) = async_watch( - consumer, + if let Err(e) = async_watch(AsyncWatchArgs { + q: consumer, path, show_progress, filter, @@ -318,7 +331,7 @@ pub fn start( create_sourcedirs, build_dev_deps, snapshot_output, - ) + }) .await { println!("{e:?}") From ff9cdaef55db2324239cb31f1e1c39bacb70c1a6 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sat, 19 Jul 2025 16:16:58 +0200 Subject: [PATCH 07/12] chore: run cargo fmt --- rewatch/src/build/compile.rs | 24 +++++++++++++----------- rewatch/src/build/deps.rs | 3 +-- rewatch/src/helpers.rs | 6 ++---- rewatch/src/lock.rs | 12 ++++++------ 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs index 81044de5ba..fdf7c35de3 100644 --- a/rewatch/src/build/compile.rs +++ b/rewatch/src/build/compile.rs @@ -669,9 +669,10 @@ fn compile_file( } if let SourceType::SourceFile(SourceFile { - interface: Some(Interface { path, .. }), - .. - }) = &module.source_type { + interface: Some(Interface { path, .. }), + .. + }) = &module.source_type + { // we need to copy the source file to the build directory. // editor tools expects the source file in lib/bs for finding the current package // and in lib/ocaml when referencing modules in other packages @@ -690,9 +691,10 @@ fn compile_file( .expect("copying source file failed"); } if let SourceType::SourceFile(SourceFile { - implementation: Implementation { path, .. }, - .. - }) = &module.source_type { + implementation: Implementation { path, .. }, + .. + }) = &module.source_type + { // we need to copy the source file to the build directory. // editor tools expects the source file in lib/bs for finding the current package // and in lib/ocaml when referencing modules in other packages @@ -715,9 +717,10 @@ fn compile_file( root_package.config.get_package_specs().iter().for_each(|spec| { if spec.in_source { if let SourceType::SourceFile(SourceFile { - implementation: Implementation { path, .. }, - .. - }) = &module.source_type { + implementation: Implementation { path, .. }, + .. + }) = &module.source_type + { let source = helpers::get_source_file_from_rescript_file( &Path::new(&package.path).join(path), &root_package.config.get_suffix(spec), @@ -728,8 +731,7 @@ fn compile_file( ); if source.exists() { - let _ = - std::fs::copy(&source, &destination).expect("copying source file failed"); + let _ = std::fs::copy(&source, &destination).expect("copying source file failed"); } } } diff --git a/rewatch/src/build/deps.rs b/rewatch/src/build/deps.rs index 6bf8c1cd0a..8159cdc5f3 100644 --- a/rewatch/src/build/deps.rs +++ b/rewatch/src/build/deps.rs @@ -45,8 +45,7 @@ fn get_dep_modules( .cloned() .collect(); - deps - .iter() + deps.iter() .map(|dep| { let dep_first = dep.split('.').next().unwrap(); let dep_second = dep.split('.').nth(1); diff --git a/rewatch/src/helpers.rs b/rewatch/src/helpers.rs index eb58726770..04427a8df1 100644 --- a/rewatch/src/helpers.rs +++ b/rewatch/src/helpers.rs @@ -111,8 +111,7 @@ pub fn get_abs_path(path: &Path) -> PathBuf { } pub fn get_basename(path: &Path) -> String { - path - .file_stem() + path.file_stem() .expect("Could not get basename") .to_str() .expect("Could not get basename 2") @@ -327,8 +326,7 @@ pub fn is_non_exotic_module_name(module_name: &str) -> bool { } pub fn get_extension(path: &Path) -> String { - path - .extension() + path.extension() .expect("Could not get extension") .to_str() .expect("Could not get extension 2") diff --git a/rewatch/src/lock.rs b/rewatch/src/lock.rs index e5427cc8f1..e12306e96f 100644 --- a/rewatch/src/lock.rs +++ b/rewatch/src/lock.rs @@ -21,15 +21,15 @@ pub enum Error { impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { let msg = match self { - Error::Locked(pid) => format!( - "A ReScript build is already running. The process ID (PID) is {pid}" - ), + Error::Locked(pid) => { + format!("A ReScript build is already running. The process ID (PID) is {pid}") + } Error::ParsingLockfile(e) => format!( "Could not parse lockfile: \n {e} \n (try removing it and running the command again)" ), - Error::ReadingLockfile(e) => format!( - "Could not read lockfile: \n {e} \n (try removing it and running the command again)" - ), + Error::ReadingLockfile(e) => { + format!("Could not read lockfile: \n {e} \n (try removing it and running the command again)") + } Error::WritingLockfile(e) => format!("Could not write lockfile: \n {e}"), }; write!(f, "{msg}") From 5ff6bace06c3c1001bef5abc639f3199fc4a970f Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sat, 19 Jul 2025 16:30:53 +0200 Subject: [PATCH 08/12] refactor: get_source_files split match --- rewatch/src/build/packages.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index d5016c1a29..f4b41bac4c 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -521,14 +521,14 @@ pub fn get_source_files( ) -> AHashMap { let mut map: AHashMap = AHashMap::new(); - let (recurse, type_) = match source { + let recurse = match source { config::PackageSource { subdirs: Some(config::Subdirs::Recurse(subdirs)), - type_, .. - } => (subdirs.to_owned(), type_), - config::PackageSource { type_, .. } => (false, type_), + } => *subdirs, + _ => false, }; + let type_ = source.type_.clone(); let path_dir = Path::new(&source.dir); let is_type_dev = type_.as_ref().map(|t| t.as_str() == "dev").unwrap_or(false); From 118335e88c0adf8a337ba315debd46949601ebb2 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sat, 19 Jul 2025 16:39:15 +0200 Subject: [PATCH 09/12] refactor: get_source_files use `PackageSource.is_type_dev()` --- rewatch/src/build/packages.rs | 7 +++---- rewatch/src/config.rs | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index f4b41bac4c..7fd85fcb0d 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -528,12 +528,11 @@ pub fn get_source_files( } => *subdirs, _ => false, }; - let type_ = source.type_.clone(); let path_dir = Path::new(&source.dir); - let is_type_dev = type_.as_ref().map(|t| t.as_str() == "dev").unwrap_or(false); - match (build_dev_deps, type_) { - (false, Some(type_)) if type_ == "dev" => (), + let is_type_dev = source.is_type_dev(); + match build_dev_deps { + false if is_type_dev => (), _ => match read_folders(filter, package_dir, path_dir, recurse, is_type_dev) { Ok(files) => map.extend(files), diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index dbdcbcb2a3..8fd41c98a7 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -30,7 +30,7 @@ pub struct PackageSource { } impl PackageSource { - fn is_type_dev(&self) -> bool { + pub fn is_type_dev(&self) -> bool { match &self.type_ { Some(type_) => type_ == "dev", None => false, From fb89e8c8a32c623c8adcb8379c7ade1992afbf63 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sat, 19 Jul 2025 16:43:37 +0200 Subject: [PATCH 10/12] refactor: simplify get_source_files --- rewatch/src/build/packages.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 7fd85fcb0d..5b3d56f7a7 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -531,18 +531,20 @@ pub fn get_source_files( let path_dir = Path::new(&source.dir); let is_type_dev = source.is_type_dev(); - match build_dev_deps { - false if is_type_dev => (), - _ => match read_folders(filter, package_dir, path_dir, recurse, is_type_dev) { - Ok(files) => map.extend(files), - - Err(_e) => log::error!( - "Could not read folder: {:?}. Specified in dependency: {}, located {:?}...", - path_dir.to_path_buf().into_os_string(), - package_name, - package_dir - ), - }, + + if !build_dev_deps && is_type_dev { + return map; + } + + match read_folders(filter, package_dir, path_dir, recurse, is_type_dev) { + Ok(files) => map.extend(files), + + Err(_e) => log::error!( + "Could not read folder: {:?}. Specified in dependency: {}, located {:?}...", + path_dir.to_path_buf().into_os_string(), + package_name, + package_dir + ), }; map From 0754018c46980d9c48e07506cbf95ff550a25818 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sun, 20 Jul 2025 12:47:32 +0200 Subject: [PATCH 11/12] chore: CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fff4a2eeb2..76d7127496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ - Configuration fields `bs-dependencies`, `bs-dev-dependencies` and `bsc-flags` are now deprecated in favor of `dependencies`, `dev-dependencies` and `compiler-flags`. https://github.com/rescript-lang/rescript/pull/7658 +#### :house: Internal + +- Add rust linting to CI with `clippy`. https://github.com/rescript-lang/rescript/pull/7675 + # 12.0.0-beta.2 #### :boom: Breaking Change From dffc928da1df4bb777891641a66e6d072896f039 Mon Sep 17 00:00:00 2001 From: Bushuo Date: Sun, 20 Jul 2025 12:53:44 +0200 Subject: [PATCH 12/12] fix: warnings after rebase --- rewatch/src/build.rs | 5 ++--- rewatch/src/config.rs | 8 ++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index b4a7673540..2f44a4f88e 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -485,9 +485,8 @@ fn log_deprecations(build_state: &BuildState) { fn log_deprecated_config_field(package_name: &str, field_name: &str, new_field_name: &str) { let warning = format!( - "The field '{}' found in the package config of '{}' is deprecated and will be removed in a future version.\n\ - Use '{}' instead.", - field_name, package_name, new_field_name + "The field '{field_name}' found in the package config of '{package_name}' is deprecated and will be removed in a future version.\n\ + Use '{new_field_name}' instead." ); println!("\n{}", style(warning).yellow()); } diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index 8fd41c98a7..5e0b51f866 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -892,7 +892,7 @@ pub mod tests { let config = Config::new_from_json_string(json).expect("a valid json string"); assert_eq!(config.dependencies, Some(vec!["@testrepo/main".to_string()])); - assert_eq!(config.get_deprecations().is_empty(), true); + assert!(config.get_deprecations().is_empty()); } #[test] @@ -942,7 +942,7 @@ pub mod tests { let config = Config::new_from_json_string(json).expect("a valid json string"); assert_eq!(config.dev_dependencies, Some(vec!["@testrepo/main".to_string()])); - assert_eq!(config.get_deprecations().is_empty(), true); + assert!(config.get_deprecations().is_empty()); } #[test] @@ -967,7 +967,7 @@ pub mod tests { let config = Config::new_from_json_string(json).expect("a valid json string"); if let Some(flags) = &config.compiler_flags { - if let Some(OneOrMore::Single(flag)) = flags.get(0) { + if let Some(OneOrMore::Single(flag)) = flags.first() { assert_eq!(flag.as_str(), "-open ABC"); } else { dbg!(config.compiler_flags); @@ -977,7 +977,7 @@ pub mod tests { dbg!(config.compiler_flags); unreachable!("Expected compiler flags to be Some"); } - assert_eq!(config.get_deprecations().is_empty(), true); + assert!(config.get_deprecations().is_empty()); } #[test]