From f9345536f3b602e4b147e3821bdb1e1eec08c904 Mon Sep 17 00:00:00 2001 From: Roland Peelen Date: Fri, 12 Apr 2024 12:06:50 +0200 Subject: [PATCH] :rotating_light: - Fix Clippy Warnings --- src/bsconfig.rs | 21 ++-- src/build.rs | 10 +- src/build/build_types.rs | 8 +- src/build/clean.rs | 92 +++++++--------- src/build/compile.rs | 58 +++++----- src/build/compile/dependency_cycle.rs | 6 +- src/build/deps.rs | 29 +++-- src/build/logs.rs | 6 +- src/build/packages.rs | 80 +++++++------- src/build/parse.rs | 146 ++++++++++---------------- src/build/read_compile_state.rs | 13 +-- src/helpers.rs | 36 +++---- src/lock.rs | 22 ++-- src/main.rs | 6 +- src/watcher.rs | 50 ++++----- 15 files changed, 249 insertions(+), 334 deletions(-) diff --git a/src/bsconfig.rs b/src/bsconfig.rs index 600de81..fb76739 100644 --- a/src/bsconfig.rs +++ b/src/bsconfig.rs @@ -172,15 +172,13 @@ pub fn flatten_flags(flags: &Option>>) -> Vec { None => vec![], Some(xs) => xs .iter() - .map(|x| match x { + .flat_map(|x| match x { OneOrMore::Single(y) => vec![y.to_owned()], OneOrMore::Multiple(ys) => ys.to_owned(), }) - .flatten() .collect::>() .iter() - .map(|str| str.split(" ")) - .flatten() + .flat_map(|str| str.split(' ')) .map(|str| str.to_string()) .collect::>(), } @@ -197,9 +195,9 @@ pub fn flatten_ppx_flags( None => vec![], Some(xs) => xs .iter() - .map(|x| match x { + .flat_map(|x| match x { OneOrMore::Single(y) => { - let first_character = y.chars().nth(0); + let first_character = y.chars().next(); match first_character { Some('.') => { vec![ @@ -210,9 +208,9 @@ pub fn flatten_ppx_flags( _ => vec!["-ppx".to_string(), node_modules_dir.to_owned() + "/" + y], } } - OneOrMore::Multiple(ys) if ys.len() == 0 => vec![], + OneOrMore::Multiple(ys) if ys.is_empty() => vec![], OneOrMore::Multiple(ys) => { - let first_character = ys[0].chars().nth(0); + let first_character = ys[0].chars().next(); let ppx = match first_character { Some('.') => node_modules_dir.to_owned() + "/" + package_name + "/" + &ys[0], _ => node_modules_dir.to_owned() + "/" + &ys[0], @@ -227,7 +225,6 @@ pub fn flatten_ppx_flags( ] } }) - .flatten() .collect::>(), } } @@ -243,14 +240,14 @@ pub fn read(path: String) -> Config { } fn check_if_rescript11_or_higher(version: &str) -> bool { - version.split(".").nth(0).unwrap().parse::().unwrap() >= 11 + version.split('.').next().unwrap().parse::().unwrap() >= 11 } fn namespace_from_package_name(package_name: &str) -> String { package_name .to_owned() - .replace("@", "") - .replace("/", "_") + .replace('@', "") + .replace('/', "_") .to_case(Case::Pascal) } diff --git a/src/build.rs b/src/build.rs index 0cd1eb5..18e2be4 100644 --- a/src/build.rs +++ b/src/build.rs @@ -79,7 +79,7 @@ pub fn get_compiler_args(path: &str, rescript_version: Option) -> String &workspace_root, workspace_root.as_ref().unwrap_or(&package_root), ); - let is_interface = filename.ends_with("i"); + let is_interface = filename.ends_with('i'); let has_interface = if is_interface { true } else { @@ -106,7 +106,7 @@ pub fn get_compiler_args(path: &str, rescript_version: Option) -> String .unwrap() } -pub fn initialize_build<'a>( +pub fn initialize_build( default_timing: Option, filter: &Option, path: &str, @@ -120,7 +120,7 @@ pub fn initialize_build<'a>( print!("{}{}Building package tree...", style("[1/7]").bold().dim(), TREE); let _ = stdout().flush(); let timing_package_tree = Instant::now(); - let packages = packages::make(&filter, &project_root, &workspace_root); + let packages = packages::make(filter, &project_root, &workspace_root); let timing_package_tree_elapsed = timing_package_tree.elapsed(); println!( @@ -307,7 +307,7 @@ pub fn incremental_build( logs::finalize(&build_state.packages); pb.finish(); - if compile_errors.len() > 0 { + if !compile_errors.is_empty() { if helpers::contains_ascii_characters(&compile_warnings) { println!("{}", &compile_warnings); } @@ -326,7 +326,7 @@ pub fn incremental_build( module.compile_dirty = true; } } - return Err(()); + Err(()) } else { println!( "{}{} {}Compiled {} modules in {:.2}s", diff --git a/src/build/build_types.rs b/src/build/build_types.rs index 37250cc..5ada3c2 100644 --- a/src/build/build_types.rs +++ b/src/build/build_types.rs @@ -65,12 +65,10 @@ pub struct Module { impl Module { pub fn is_mlmap(&self) -> bool { - match self.source_type { - SourceType::MlMap(_) => true, - _ => false, - } + matches!(self.source_type, SourceType::MlMap(_)) } - pub fn get_interface<'a>(&'a self) -> &'a Option { + + pub fn get_interface(&self) -> &Option { match &self.source_type { SourceType::SourceFile(source_file) => &source_file.interface, _ => &None, diff --git a/src/build/clean.rs b/src/build/clean.rs index 361829c..210a825 100644 --- a/src/build/clean.rs +++ b/src/build/clean.rs @@ -72,7 +72,7 @@ pub fn clean_mjs_files(build_state: &BuildState) { .expect("Could not find root package"); Some(( std::path::PathBuf::from(package.path.to_string()) - .join(source_file.implementation.path.to_string()) + .join(&source_file.implementation.path) .to_string_lossy() .to_string(), root_package @@ -88,7 +88,7 @@ pub fn clean_mjs_files(build_state: &BuildState) { rescript_file_locations .par_iter() - .for_each(|(rescript_file_location, suffix)| remove_mjs_file(&rescript_file_location, &suffix)); + .for_each(|(rescript_file_location, suffix)| remove_mjs_file(rescript_file_location, suffix)); } // TODO: change to scan_previous_build => CompileAssetsState @@ -131,7 +131,7 @@ pub fn cleanup_previous_build( .expect("Could not find package"); remove_compile_assets(package, res_file_location); remove_mjs_file( - &res_file_location, + res_file_location, &suffix .to_owned() .unwrap_or(String::from(bsconfig::DEFAULT_SUFFIX)), @@ -152,7 +152,6 @@ pub fn cleanup_previous_build( compile_assets_state .ast_rescript_file_locations .intersection(&compile_assets_state.rescript_file_locations) - .into_iter() .for_each(|res_file_location| { let AstModule { module_name, @@ -173,7 +172,7 @@ pub fn cleanup_previous_build( let last_modified = Some(ast_last_modified); if let Some(last_modified) = last_modified { - if compile_dirty > &last_modified && !deleted_interfaces.contains(module_name) { + if compile_dirty > last_modified && !deleted_interfaces.contains(module_name) { module.compile_dirty = false; } } @@ -209,18 +208,18 @@ pub fn cleanup_previous_build( .cmi_modules .iter() .for_each(|(module_name, last_modified)| { - build_state.modules.get_mut(module_name).map(|module| { + if let Some(module) = build_state.modules.get_mut(module_name) { module.last_compiled_cmi = Some(*last_modified); - }); + } }); compile_assets_state .cmt_modules .iter() .for_each(|(module_name, last_modified)| { - build_state.modules.get_mut(module_name).map(|module| { + if let Some(module) = build_state.modules.get_mut(module_name) { module.last_compiled_cmt = Some(*last_modified); - }); + } }); let ast_module_names = compile_assets_state @@ -241,11 +240,7 @@ pub fn cleanup_previous_build( ) .collect::>(); - let all_module_names = build_state - .modules - .keys() - .map(|module_name| module_name) - .collect::>(); + let all_module_names = build_state.modules.keys().collect::>(); let deleted_module_names = ast_module_names .difference(&all_module_names) @@ -254,7 +249,7 @@ pub fn cleanup_previous_build( if let Some(namespace) = helpers::get_namespace_from_module_name(module_name) { return namespace; } - return module_name.to_string(); + module_name.to_string() }) .collect::>(); @@ -264,59 +259,50 @@ pub fn cleanup_previous_build( } fn has_parse_warnings(module: &Module) -> bool { - match &module.source_type { + matches!( + &module.source_type, SourceType::SourceFile(SourceFile { - implementation: - Implementation { - parse_state: ParseState::Warning, - .. - }, + implementation: Implementation { + parse_state: ParseState::Warning, + .. + }, .. - }) => true, - SourceType::SourceFile(SourceFile { - interface: - Some(Interface { - parse_state: ParseState::Warning, - .. - }), + }) | SourceType::SourceFile(SourceFile { + interface: Some(Interface { + parse_state: ParseState::Warning, + .. + }), .. - }) => true, - _ => false, - } + }) + ) } fn has_compile_warnings(module: &Module) -> bool { - match &module.source_type { + matches!( + &module.source_type, SourceType::SourceFile(SourceFile { - implementation: - Implementation { - compile_state: CompileState::Warning, - .. - }, + implementation: Implementation { + compile_state: CompileState::Warning, + .. + }, .. - }) => true, - SourceType::SourceFile(SourceFile { - interface: - Some(Interface { - compile_state: CompileState::Warning, - .. - }), + }) | SourceType::SourceFile(SourceFile { + interface: Some(Interface { + compile_state: CompileState::Warning, + .. + }), .. - }) => true, - _ => false, - } + }) + ) } pub fn cleanup_after_build(build_state: &BuildState) { build_state.modules.par_iter().for_each(|(_module_name, module)| { let package = build_state.get_package(&module.package_name).unwrap(); if has_parse_warnings(module) { - match &module.source_type { - SourceType::SourceFile(source_file) => { - remove_iast(package, &source_file.implementation.path); - remove_ast(package, &source_file.implementation.path); - } - _ => (), + if let SourceType::SourceFile(source_file) = &module.source_type { + remove_iast(package, &source_file.implementation.path); + remove_ast(package, &source_file.implementation.path); } } if has_compile_warnings(module) { diff --git a/src/build/compile.rs b/src/build/compile.rs index d1f95c4..f081b86 100644 --- a/src/build/compile.rs +++ b/src/build/compile.rs @@ -1,3 +1,5 @@ +#![allow(clippy::too_many_arguments)] + mod dependency_cycle; use super::build_types::*; @@ -16,8 +18,8 @@ use std::time::SystemTime; pub fn compile( build_state: &mut BuildState, - inc: impl Fn() -> () + std::marker::Sync, - set_length: impl Fn(u64) -> (), + inc: impl Fn() + std::marker::Sync, + set_length: impl Fn(u64), ) -> (String, String, usize) { let mut compiled_modules = AHashSet::::new(); let dirty_modules = build_state @@ -86,7 +88,7 @@ pub fn compile( let mut in_progress_modules = compile_universe .iter() .filter(|module_name| { - let module = build_state.get_module(*module_name).unwrap(); + let module = build_state.get_module(module_name).unwrap(); module.deps.intersection(&compile_universe).count() == 0 }) .map(|module_name| module_name.to_string()) @@ -156,8 +158,8 @@ pub fn compile( let interface_result = match source_file.interface.to_owned() { Some(Interface { path, .. }) => { let result = compile_file( - &package, - &root_package, + package, + root_package, &package.get_iast_path(&path), module, &build_state.rescript_version, @@ -172,8 +174,8 @@ pub fn compile( _ => None, }; let result = compile_file( - &package, - &root_package, + package, + root_package, &package.get_ast_path(&source_file.implementation.path), module, &build_state.rescript_version, @@ -280,24 +282,24 @@ pub fn compile( match result { Ok(Some(err)) => { source_file.implementation.compile_state = CompileState::Warning; - logs::append(package, &err); - compile_warnings.push_str(&err); + logs::append(package, err); + compile_warnings.push_str(err); } Ok(None) => { source_file.implementation.compile_state = CompileState::Success; } Err(err) => { source_file.implementation.compile_state = CompileState::Error; - logs::append(package, &err); - compile_errors.push_str(&err); + logs::append(package, err); + compile_errors.push_str(err); } }; match interface_result { Some(Ok(Some(err))) => { source_file.interface.as_mut().unwrap().compile_state = CompileState::Warning; - logs::append(package, &err); - compile_warnings.push_str(&err); + logs::append(package, err); + compile_warnings.push_str(err); } Some(Ok(None)) => { if let Some(interface) = source_file.interface.as_mut() { @@ -308,8 +310,8 @@ pub fn compile( Some(Err(err)) => { source_file.interface.as_mut().unwrap().compile_state = CompileState::Error; - logs::append(package, &err); - compile_errors.push_str(&err); + logs::append(package, err); + compile_errors.push_str(err); } _ => (), }; @@ -353,7 +355,7 @@ pub fn compile( dependency_cycle::format(&cycle) )) } - if compile_errors.len() > 0 { + if !compile_errors.is_empty() { break; }; } @@ -492,7 +494,7 @@ pub fn compiler_args( ] }; - let to_mjs_args = vec![ + vec![ namespace_args, read_cmi_args, vec!["-I".to_string(), ".".to_string()], @@ -515,9 +517,7 @@ pub fn compiler_args( // ], vec![ast_path.to_string()], ] - .concat(); - - to_mjs_args + .concat() } fn compile_file( @@ -544,7 +544,7 @@ fn compile_file( &root_package.bsconfig, ast_path, version, - &implementation_file_path, + implementation_file_path, is_interface, has_interface, project_root, @@ -580,7 +580,7 @@ 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(module_name.to_owned().replace("@", "") + ".cmi"), + .join(module_name.to_owned().replace('@', "") + ".cmi"), ); let _ = std::fs::copy( build_path_abs.to_string() + "/" + &module_name + ".cmj", @@ -595,7 +595,7 @@ 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(module_name.to_owned().replace("@", "") + ".cmt"), + .join(module_name.to_owned().replace('@', "") + ".cmt"), ); } else { let _ = std::fs::copy( @@ -725,13 +725,13 @@ pub fn mark_modules_with_expired_deps_dirty(build_state: &mut BuildState) { SourceType::MlMap(_) => { for dependent_of_namespace in dependent_module.dependents.iter() { let dependent_module = build_state.modules.get(dependent_of_namespace).unwrap(); - match (dependent_module.last_compiled_cmt, module.last_compiled_cmi) { - (Some(last_compiled_dependent), Some(last_compiled)) => { - if last_compiled_dependent < last_compiled { - modules_with_expired_deps.insert(dependent.to_string()); - } + + if let (Some(last_compiled_dependent), Some(last_compiled)) = + (dependent_module.last_compiled_cmt, module.last_compiled_cmi) + { + if last_compiled_dependent < last_compiled { + modules_with_expired_deps.insert(dependent.to_string()); } - _ => (), } } } diff --git a/src/build/compile/dependency_cycle.rs b/src/build/compile/dependency_cycle.rs index 5495ac0..072ca06 100644 --- a/src/build/compile/dependency_cycle.rs +++ b/src/build/compile/dependency_cycle.rs @@ -13,9 +13,9 @@ pub fn find(modules: &Vec<(&String, &Module)>) -> Vec { .map(|(name, _)| name.to_string()) .collect::>(); - module_names.sort_by(|a, b| a.cmp(b)); + module_names.sort(); for module_name in module_names { - if find_dependency_cycle_helper(&module_name, &modules, &mut visited, &mut stack) { + if find_dependency_cycle_helper(&module_name, modules, &mut visited, &mut stack) { return stack; } visited.clear(); @@ -62,7 +62,7 @@ fn find_dependency_cycle_helper( false } -pub fn format(cycle: &Vec) -> String { +pub fn format(cycle: &[String]) -> String { cycle .iter() .map(|s| helpers::format_namespaced_module_name(s)) diff --git a/src/build/deps.rs b/src/build/deps.rs index 8573346..dad3131 100644 --- a/src/build/deps.rs +++ b/src/build/deps.rs @@ -17,17 +17,12 @@ fn get_dep_modules( // the following lines in the AST are the dependency modules // we stop when we hit a line that starts with a "/", this is the path of the file. // this is the point where the dependencies end and the actual AST starts - for line in lines.skip(1) { - match line { - Ok(line) => { - let line = line.trim().to_string(); - if line.starts_with('/') { - break; - } else if !line.is_empty() { - deps.insert(line); - } - } - Err(_) => (), + for line in lines.skip(1).flatten() { + let line = line.trim().to_string(); + if line.starts_with('/') { + break; + } else if !line.is_empty() { + deps.insert(line); } } } else { @@ -48,12 +43,12 @@ fn get_dep_modules( Some(dep_second) if dep_first == namespace => dep_second, _ => dep_first, }; - let namespaced_name = dep.to_owned() + "-" + &namespace; + let namespaced_name = dep.to_owned() + "-" + namespace; if package_modules.contains(&namespaced_name) { - return namespaced_name; + namespaced_name } else { - return dep.to_string(); - }; + dep.to_string() + } } None => dep_first.to_string(), } @@ -84,7 +79,7 @@ pub fn get_deps(build_state: &mut BuildState, deleted_modules: &AHashSet let mut deps = get_dep_modules( &ast_path, package.namespace.to_suffix(), - &package.modules.as_ref().unwrap(), + package.modules.as_ref().unwrap(), all_mod, ); @@ -95,7 +90,7 @@ pub fn get_deps(build_state: &mut BuildState, deleted_modules: &AHashSet deps.extend(get_dep_modules( &iast_path, package.namespace.to_suffix(), - &package.modules.as_ref().unwrap(), + package.modules.as_ref().unwrap(), all_mod, )) } diff --git a/src/build/logs.rs b/src/build/logs.rs index 7a2fa9c..8f79afd 100644 --- a/src/build/logs.rs +++ b/src/build/logs.rs @@ -48,8 +48,8 @@ fn write_to_log_file(mut file: File, package_name: &str, content: &str) { pub fn initialize(packages: &AHashMap) { packages.par_iter().for_each(|(name, package)| { - let _ = File::create(get_log_file_path(package, Location::Bs)) - .map(|file| write_to_log_file(file, &name, &format!("#Start({})\n", helpers::get_system_time()))) + File::create(get_log_file_path(package, Location::Bs)) + .map(|file| write_to_log_file(file, name, &format!("#Start({})\n", helpers::get_system_time()))) .expect(&("Cannot create compiler log for package ".to_owned() + name)); }) } @@ -67,7 +67,7 @@ pub fn finalize(packages: &AHashMap) { let _ = File::options() .append(true) .open(get_log_file_path(package, Location::Bs)) - .map(|file| write_to_log_file(file, &name, &format!("#Done({})\n", helpers::get_system_time()))); + .map(|file| write_to_log_file(file, name, &format!("#Done({})\n", helpers::get_system_time()))); let _ = std::fs::copy( get_log_file_path(package, Location::Bs), diff --git a/src/build/packages.rs b/src/build/packages.rs index a12c349..171bc0c 100644 --- a/src/build/packages.rs +++ b/src/build/packages.rs @@ -111,7 +111,7 @@ impl PartialEq for Package { impl Eq for Package {} impl Hash for Package { fn hash(&self, _state: &mut H) { - blake3::hash(&self.name.as_bytes()); + blake3::hash(self.name.as_bytes()); } } @@ -130,7 +130,7 @@ pub fn read_folders( ) -> Result, Box> { let mut map: AHashMap = AHashMap::new(); let path_buf = PathBuf::from(path); - let meta = fs::metadata(package_dir.join(&path)); + let meta = fs::metadata(package_dir.join(path)); let path_with_meta = meta.map(|meta| { ( path.to_owned(), @@ -148,7 +148,7 @@ pub fn read_folders( let path_ext = entry_path_buf.extension().and_then(|x| x.to_str()); let new_path = path_buf.join(&name); if metadata.file_type().is_dir() && recurse { - match read_folders(&filter, package_dir, &new_path, recurse) { + match read_folders(filter, package_dir, &new_path, recurse) { Ok(s) => map.extend(s), Err(e) => println!("Error reading directory: {}", e), } @@ -215,7 +215,7 @@ fn get_source_dirs(source: bsconfig::Source, sub_path: Option) -> AHash } pub fn read_bsconfig(package_dir: &str) -> bsconfig::Config { - let prefix = if package_dir == "" { + let prefix = if package_dir.is_empty() { "".to_string() } else { package_dir.to_string() + "/" @@ -241,7 +241,7 @@ pub fn read_dependency( let path_from_project_root = PathBuf::from(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| PathBuf::from(helpers::package_path(workspace_root, package_name))); let path = match ( path_from_parent, @@ -266,7 +266,7 @@ pub fn read_dependency( "Failed canonicalizing the package \"{}\" path \"{}\" (are node_modules up-to-date?)...\nMore details: {}", package_name, path.to_string_lossy(), - e.to_string() + e )) } }?; @@ -282,8 +282,8 @@ pub fn read_dependency( /// registerd for the parent packages. Especially relevant for peerDependencies. /// 2. In parallel performs IO to read the dependencies bsconfig and /// recursively continues operation for their dependencies as well. -fn read_dependencies<'a>( - registered_dependencies_set: &'a mut AHashSet, +fn read_dependencies( + registered_dependencies_set: &mut AHashSet, parent_bsconfig: &bsconfig::Config, parent_path: &str, project_root: &str, @@ -420,7 +420,7 @@ fn read_packages(project_root: &str, workspace_root: Option) -> AHashMap } }); - return map; + map } /// `get_source_files` is essentially a wrapper around `read_structure`, which read a @@ -450,7 +450,7 @@ pub fn get_source_files( let path_dir = Path::new(&source.dir); // don't include dev sources for now if type_ != &Some("dev".to_string()) { - match read_folders(&filter, package_dir, path_dir, recurse) { + match read_folders(filter, package_dir, path_dir, recurse) { Ok(files) => map.extend(files), Err(_e) if type_ == &Some("dev".to_string()) => { println!( @@ -476,7 +476,7 @@ fn extend_with_children( value .source_folders .par_iter() - .map(|source| get_source_files(Path::new(&value.path), &filter, source)) + .map(|source| get_source_files(Path::new(&value.path), filter, source)) .collect::>>() .into_iter() .for_each(|source| map.extend(source)); @@ -521,21 +521,18 @@ pub fn make( /* Once we have the deduplicated packages, we can add the source files for each - to minimize * the IO */ - let result = extend_with_children(&filter, map); - result - .values() - .into_iter() - .for_each(|package| match &package.dirs { - Some(dirs) => dirs.iter().for_each(|dir| { - let _ = std::fs::create_dir_all(std::path::Path::new(&package.get_bs_build_path()).join(dir)); - }), - None => (), - }); + let result = extend_with_children(filter, map); + result.values().for_each(|package| match &package.dirs { + Some(dirs) => dirs.iter().for_each(|dir| { + let _ = std::fs::create_dir_all(std::path::Path::new(&package.get_bs_build_path()).join(dir)); + }), + None => (), + }); result } pub fn get_package_name(path: &str) -> String { - let bsconfig = read_bsconfig(&path); + let bsconfig = read_bsconfig(path); bsconfig.name } @@ -546,9 +543,8 @@ pub fn parse_packages(build_state: &mut BuildState) { .iter() .for_each(|(package_name, package)| { debug!("Parsing package: {}", package_name); - match package.modules.to_owned() { - Some(package_modules) => build_state.module_names.extend(package_modules), - None => (), + if let Some(package_modules) = package.modules.to_owned() { + build_state.module_names.extend(package_modules) } let build_path_abs = package.get_build_path(); let bs_build_path = package.get_bs_build_path(); @@ -571,7 +567,7 @@ pub fn parse_packages(build_state: &mut BuildState) { let depending_modules = source_files .iter() - .map(|path| helpers::file_path_to_module_name(&path, &packages::Namespace::NoNamespace)) + .map(|path| helpers::file_path_to_module_name(path, &packages::Namespace::NoNamespace)) .filter(|module_name| { if let Some(entry) = entry { module_name != entry @@ -582,7 +578,7 @@ pub fn parse_packages(build_state: &mut BuildState) { .filter(|module_name| helpers::is_non_exotic_module_name(module_name)) .collect::>(); - let mlmap = namespaces::gen_mlmap(&package, namespace, &depending_modules); + let mlmap = namespaces::gen_mlmap(package, namespace, &depending_modules); // mlmap will be compiled in the AST generation step // compile_mlmap(&package, namespace, &project_root); @@ -590,11 +586,11 @@ pub fn parse_packages(build_state: &mut BuildState) { .iter() .filter(|path| { helpers::is_non_exotic_module_name(&helpers::file_path_to_module_name( - &path, + path, &packages::Namespace::NoNamespace, )) }) - .map(|path| helpers::file_path_to_module_name(&path, &package.namespace)) + .map(|path| helpers::file_path_to_module_name(path, &package.namespace)) .filter(|module_name| { if let Some(entry) = entry { module_name != entry @@ -632,8 +628,8 @@ pub fn parse_packages(build_state: &mut BuildState) { build_state .modules .entry(module_name.to_string()) - .and_modify(|module| match module.source_type { - SourceType::SourceFile(ref mut source_file) => { + .and_modify(|module| { + if let SourceType::SourceFile(ref mut source_file) = module.source_type { if &source_file.implementation.path != file { error!("Duplicate files found for module: {}", &module_name); error!("file 1: {}", &source_file.implementation.path); @@ -645,7 +641,6 @@ pub fn parse_packages(build_state: &mut BuildState) { source_file.implementation.last_modified = metadata.modified; source_file.implementation.parse_dirty = true; } - _ => (), }) .or_insert(Module { source_type: SourceType::SourceFile(SourceFile { @@ -680,8 +675,10 @@ pub fn parse_packages(build_state: &mut BuildState) { build_state .modules .entry(module_name.to_string()) - .and_modify(|module| match module.source_type { - SourceType::SourceFile(ref mut source_file) => { + .and_modify(|module| { + if let SourceType::SourceFile(ref mut source_file) = + module.source_type + { source_file.interface = Some(Interface { path: file.to_owned(), parse_state: ParseState::Pending, @@ -690,7 +687,6 @@ pub fn parse_packages(build_state: &mut BuildState) { parse_dirty: true, }); } - _ => (), }) .or_insert(Module { source_type: SourceType::SourceFile(SourceFile { @@ -758,7 +754,7 @@ fn get_unallowed_dependents( } } } - return None; + None } #[derive(Debug, Clone)] struct UnallowedDependency { @@ -793,10 +789,10 @@ pub fn validate_packages_dependencies(packages: &AHashMap) -> b let unallowed_dependency = detected_unallowed_dependencies.entry(String::from(package_name)); let value = unallowed_dependency.or_insert_with(|| empty_unallowed_deps); - match dependency_type { - &"bs-dependencies" => value.bs_deps.push(String::from(unallowed_dependency_name)), - &"pinned-dependencies" => value.pinned_deps.push(String::from(unallowed_dependency_name)), - &"bs-dev-dependencies" => value.bs_dev_deps.push(String::from(unallowed_dependency_name)), + match *dependency_type { + "bs-dependencies" => value.bs_deps.push(unallowed_dependency_name), + "pinned-dependencies" => value.pinned_deps.push(unallowed_dependency_name), + "bs-dev-dependencies" => value.bs_dev_deps.push(unallowed_dependency_name), _ => (), } } @@ -816,7 +812,7 @@ pub fn validate_packages_dependencies(packages: &AHashMap) -> b ] .iter() .for_each(|(deps_type, map)| { - if map.len() > 0 { + if !map.is_empty() { println!( "{} dependencies: {}", console::style(deps_type).bold().dim(), @@ -834,7 +830,7 @@ pub fn validate_packages_dependencies(packages: &AHashMap) -> b console::style("bsconfig.json").bold().dim() ) } - return !has_any_unallowed_dependent; + !has_any_unallowed_dependent } #[cfg(test)] diff --git a/src/build/parse.rs b/src/build/parse.rs index c52bacd..ca72180 100644 --- a/src/build/parse.rs +++ b/src/build/parse.rs @@ -13,12 +13,12 @@ use std::process::Command; pub fn generate_asts( build_state: &mut BuildState, - inc: impl Fn() -> () + std::marker::Sync, + inc: impl Fn() + std::marker::Sync, ) -> Result { let mut has_failure = false; let mut stderr = "".to_string(); - let results = build_state + build_state .modules .par_iter() .map(|(module_name, module)| { @@ -61,7 +61,7 @@ pub fn generate_asts( &build_state.bsc_path, &build_state.workspace_root, ) - .map(|result| Some(result)), + .map(Some), _ => Ok(None), }; @@ -89,100 +89,67 @@ pub fn generate_asts( Result<(String, Option), String>, Result)>, String>, bool, - )>>(); - - results + )>>() .into_iter() .for_each(|(module_name, ast_path, iast_path, is_dirty)| { if let Some(module) = build_state.modules.get_mut(&module_name) { + module.compile_dirty = is_dirty; let package = build_state .packages .get(&module.package_name) .expect("Package not found"); - if is_dirty { - module.compile_dirty = true - } - match ast_path { - Ok((_path, err)) => { - match module.source_type { - SourceType::SourceFile(ref mut source_file) => { - source_file.implementation.parse_dirty = false; - source_file - .interface - .as_mut() - .map(|interface| interface.parse_dirty = false); - } - _ => (), - } - // supress warnings in non-pinned deps - match module.source_type { - SourceType::SourceFile(ref mut source_file) => { - source_file.implementation.parse_state = ParseState::Success; - } - _ => (), - } - - if package.is_pinned_dep { - if let Some(err) = err { - match module.source_type { - SourceType::SourceFile(ref mut source_file) => { - source_file.implementation.parse_state = ParseState::Warning; - source_file.implementation.parse_dirty = true; - } - _ => (), - } - logs::append(package, &err); - stderr.push_str(&err); - } + match (ast_path, module.source_type.to_owned()) { + // supress warnings in non-pinned deps + (Ok((_path, Some(err))), SourceType::SourceFile(ref mut source_file)) + if package.is_pinned_dep => + { + source_file.implementation.parse_state = ParseState::Warning; + source_file.implementation.parse_dirty = true; + if let Some(interface) = source_file.interface.as_mut() { + interface.parse_dirty = false; } + logs::append(package, &err); + stderr.push_str(&err); } - Err(err) => { - match module.source_type { - SourceType::SourceFile(ref mut source_file) => { - source_file.implementation.parse_state = ParseState::ParseError; - source_file.implementation.parse_dirty = true; - } - _ => (), + (Ok((_path, None)), SourceType::SourceFile(ref mut source_file)) => { + source_file.implementation.parse_state = ParseState::Success; + source_file.implementation.parse_dirty = false; + if let Some(interface) = source_file.interface.as_mut() { + interface.parse_dirty = false; } + } + (Err(err), SourceType::SourceFile(ref mut source_file)) => { + source_file.implementation.parse_state = ParseState::ParseError; + source_file.implementation.parse_dirty = true; logs::append(package, &err); has_failure = true; stderr.push_str(&err); } + _ => (), }; - match iast_path { - Ok(Some((_path, err))) => { + + match (iast_path, module.source_type.to_owned()) { + (Ok(Some((_path, Some(err)))), SourceType::SourceFile(ref mut source_file)) + if package.is_pinned_dep => + { // supress warnings in non-pinned deps - if package.is_pinned_dep { - if let Some(err) = err { - match module.source_type { - SourceType::SourceFile(ref mut source_file) => { - source_file.interface.as_mut().map(|interface| { - interface.parse_state = ParseState::ParseError; - interface.parse_dirty = true; - }); - } - _ => (), - } - logs::append(package, &err); - stderr.push_str(&err); - } + if let Some(interface) = source_file.interface.as_mut() { + interface.parse_state = ParseState::Warning; + interface.parse_dirty = true; } + logs::append(package, &err); + stderr.push_str(&err); } - Ok(None) => (), - Err(err) => { - match module.source_type { - SourceType::SourceFile(ref mut source_file) => { - source_file.interface.as_mut().map(|interface| { - interface.parse_state = ParseState::ParseError; - interface.parse_dirty = true; - }); - } - _ => (), + (Err(err), SourceType::SourceFile(ref mut source_file)) => { + if let Some(interface) = source_file.interface.as_mut() { + interface.parse_state = ParseState::ParseError; + interface.parse_dirty = true; } logs::append(package, &err); has_failure = true; stderr.push_str(&err); } + _ => (), }; } }); @@ -208,7 +175,7 @@ pub fn generate_asts( // specific to compiling mlmaps let compile_path = package.get_mlmap_compile_path(); let mlmap_hash = helpers::compute_file_hash(&compile_path); - namespaces::compile_mlmap(&package, module_name, &build_state.bsc_path); + namespaces::compile_mlmap(package, module_name, &build_state.bsc_path); let mlmap_hash_after = helpers::compute_file_hash(&compile_path); match (mlmap_hash, mlmap_hash_after) { @@ -314,7 +281,7 @@ fn generate_ast( if res_to_ast.status.success() { Ok((ast_path, Some(stderr.to_string()))) } else { - println!("err: {}", stderr.to_string()); + println!("err: {}", stderr); Err(stderr.to_string()) } } else { @@ -344,20 +311,15 @@ fn filter_ppx_flags(ppx_flags: &Option>>) -> Option None, Err(_) => Some("bisect"), }; - match ppx_flags { - Some(flags) => Some( - flags - .iter() - .filter(|flag| match (flag, filter) { - (bsconfig::OneOrMore::Single(str), Some(filter)) => !str.contains(filter), - (bsconfig::OneOrMore::Multiple(str), Some(filter)) => { - !str.first().unwrap().contains(filter) - } - _ => true, - }) - .map(|x| x.to_owned()) - .collect::>>(), - ), - None => None, - } + ppx_flags.as_ref().map(|flags| { + flags + .iter() + .filter(|flag| match (flag, filter) { + (bsconfig::OneOrMore::Single(str), Some(filter)) => !str.contains(filter), + (bsconfig::OneOrMore::Multiple(str), Some(filter)) => !str.first().unwrap().contains(filter), + _ => true, + }) + .map(|x| x.to_owned()) + .collect::>>() + }) } diff --git a/src/build/read_compile_state.rs b/src/build/read_compile_state.rs index f4cf7a0..fc08853 100644 --- a/src/build/read_compile_state.rs +++ b/src/build/read_compile_state.rs @@ -22,7 +22,7 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { Some( PathBuf::from(&package.path) - .join(source_file.implementation.path.to_owned()) + .join(&source_file.implementation.path) .to_string_lossy() .to_string(), ) @@ -39,7 +39,7 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { let package = build_state.packages.get(&module.package_name).unwrap(); module.get_interface().as_ref().map(|interface| { PathBuf::from(&package.path) - .join(interface.path.to_owned()) + .join(&interface.path) .to_string_lossy() .to_string() }) @@ -85,7 +85,7 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { match extension.as_str() { "iast" | "ast" => { let module_name = - helpers::file_path_to_module_name(path.to_str().unwrap(), &package_namespace); + helpers::file_path_to_module_name(path.to_str().unwrap(), package_namespace); let ast_file_path = path.to_str().unwrap().to_owned(); let res_file_path = get_res_path_from_ast(&ast_file_path); @@ -93,8 +93,7 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { .packages .get(&build_state.root_config_name) .expect("Could not find root package"); - match res_file_path { - Some(res_file_path) => { + if let Some(res_file_path) = res_file_path { let _ = ast_modules.insert( res_file_path.to_owned(), AstModule { @@ -108,8 +107,6 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { }, ); let _ = ast_rescript_file_locations.insert(res_file_path); - } - None => (), } } "cmi" => { @@ -159,5 +156,5 @@ fn get_res_path_from_ast(ast_file: &str) -> Option { } } } - return None; + None } diff --git a/src/helpers.rs b/src/helpers.rs index a71e0e8..fe2434e 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -39,7 +39,7 @@ impl LexicalAbsolute for Path { Component::ParentDir => { absolute.pop(); } - component @ _ => absolute.push(component.as_os_str()), + component => absolute.push(component.as_os_str()), } } Ok(absolute) @@ -122,7 +122,7 @@ pub fn contains_ascii_characters(str: &str) -> bool { return true; } } - return false; + false } pub fn create_build_path(build_path: &str) { @@ -163,7 +163,7 @@ pub fn get_bsc(root_path: &str, workspace_root: Option) -> String { .to_string() } -pub fn string_ends_with_any(s: &PathBuf, suffixes: &[&str]) -> bool { +pub fn string_ends_with_any(s: &Path, suffixes: &[&str]) -> bool { suffixes .iter() .any(|&suffix| s.extension().unwrap_or(&OsString::new()).to_str().unwrap_or("") == suffix) @@ -215,7 +215,7 @@ pub fn get_bs_compiler_asset( } pub fn get_namespace_from_module_name(module_name: &str) -> Option { - let mut split = module_name.split("-"); + let mut split = module_name.split('-'); let _ = split.next(); split.next().map(|s| s.to_string()) } @@ -236,17 +236,11 @@ pub fn get_system_time() -> u128 { } pub fn is_interface_file(extension: &str) -> bool { - match extension { - "resi" | "mli" | "rei" => true, - _ => false, - } + matches!(extension, "resi" | "mli" | "rei") } pub fn is_implementation_file(extension: &str) -> bool { - match extension { - "res" | "ml" | "re" => true, - _ => false, - } + matches!(extension, "res" | "ml" | "re") } pub fn is_source_file(extension: &str) -> bool { @@ -258,7 +252,7 @@ pub fn is_non_exotic_module_name(module_name: &str) -> bool { if chars.next().unwrap().is_ascii_uppercase() && chars.all(|c| c.is_ascii_alphanumeric() || c == '_') { return true; } - return false; + false } pub fn get_extension(path: &str) -> String { @@ -274,14 +268,14 @@ pub fn get_extension(path: &str) -> String { pub fn format_namespaced_module_name(module_name: &str) -> String { // from ModuleName-Namespace to Namespace.ModuleName // also format ModuleName-@Namespace to Namespace.ModuleName - let mut split = module_name.split("-"); + let mut split = module_name.split('-'); let module_name = split.next().unwrap(); let namespace = split.next(); - let namespace = namespace.map(|ns| ns.trim_start_matches("@")); - return match namespace { + let namespace = namespace.map(|ns| ns.trim_start_matches('@')); + match namespace { None => module_name.to_string(), Some(ns) => ns.to_string() + "." + module_name, - }; + } } pub fn compute_file_hash(path: &str) -> Option { @@ -291,18 +285,18 @@ pub fn compute_file_hash(path: &str) -> Option { } } -fn has_rescript_config(path: &PathBuf) -> bool { +fn has_rescript_config(path: &Path) -> bool { path.join("bsconfig.json").exists() || path.join("rescript.json").exists() } pub fn get_workspace_root(package_root: &str) -> Option { std::path::PathBuf::from(&package_root) .parent() - .and_then(|parent_dir| get_nearest_bsconfig(&parent_dir.to_path_buf())) + .and_then(get_nearest_bsconfig) } // traverse up the directory tree until we find a bsconfig.json, if not return None -pub fn get_nearest_bsconfig(path_buf: &PathBuf) -> Option { +pub fn get_nearest_bsconfig(path_buf: &Path) -> Option { let mut current_dir = path_buf.to_owned(); loop { if has_rescript_config(¤t_dir) { @@ -323,6 +317,6 @@ pub fn get_rescript_version(bsc_path: &str) -> String { std::str::from_utf8(&version_cmd.stdout) .expect("Could not read version from rescript") - .replace("\n", "") + .replace('\n', "") .replace("ReScript ", "") } diff --git a/src/lock.rs b/src/lock.rs index f9b00b8..98ab217 100644 --- a/src/lock.rs +++ b/src/lock.rs @@ -1,6 +1,6 @@ use std::fs; use std::fs::File; -use std::io::prelude::*; +use std::io::Write; use std::path::Path; use std::process; use sysinfo::{PidExt, System, SystemExt}; @@ -22,9 +22,9 @@ 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!("Rewatch is already running with PID {}", pid), - Error::ParsingLockfile(e) => format!("Could not parse lockfile: \n {}", e.to_string()), - Error::ReadingLockfile(e) => format!("Could not read lockfile: \n {}", e.to_string()), - Error::WritingLockfile(e) => format!("Could not write lockfile: \n {}", e.to_string()), + Error::ParsingLockfile(e) => format!("Could not parse lockfile: \n {}", e), + Error::ReadingLockfile(e) => format!("Could not read lockfile: \n {}", e), + Error::WritingLockfile(e) => format!("Could not write lockfile: \n {}", e), }; write!(f, "{}", msg) } @@ -38,18 +38,14 @@ pub enum Lock { fn exists(to_check_pid: u32) -> bool { System::new_all() .processes() - .into_iter() + .iter() .any(|(pid, _process)| pid.as_u32() == to_check_pid) } fn create(lockfile_location: &Path, pid: u32) -> Lock { // Create /lib if not exists - match lockfile_location - .parent() - .map(|folder| fs::create_dir_all(folder)) - { - Some(Err(e)) => return Lock::Error(Error::WritingLockfile(e)), - _ => (), + if let Some(Err(e)) = lockfile_location.parent().map(fs::create_dir_all) { + return Lock::Error(Error::WritingLockfile(e)); }; File::create(lockfile_location) @@ -63,10 +59,10 @@ pub fn get(folder: &str) -> Lock { let pid = process::id(); match fs::read_to_string(&location) { - Err(e) if (e.kind() == std::io::ErrorKind::NotFound) => create(&path, pid), + Err(e) if (e.kind() == std::io::ErrorKind::NotFound) => create(path, pid), Err(e) => Lock::Error(Error::ReadingLockfile(e)), Ok(s) => match s.parse::() { - Ok(parsed_pid) if !exists(parsed_pid) => create(&path, pid), + Ok(parsed_pid) if !exists(parsed_pid) => create(path, pid), Ok(parsed_pid) => Lock::Error(Error::Locked(parsed_pid)), Err(e) => Lock::Error(Error::ParsingLockfile(e)), }, diff --git a/src/main.rs b/src/main.rs index 3d6f8e2..8bb6aef 100644 --- a/src/main.rs +++ b/src/main.rs @@ -73,7 +73,7 @@ fn main() { match lock::get(&folder) { lock::Lock::Error(ref e) => { - eprintln!("Error while trying to get lock: {}", e.to_string()); + eprintln!("Error while trying to get lock: {e}"); std::process::exit(1) } lock::Lock::Aquired(_) => match command { @@ -82,7 +82,9 @@ fn main() { match build::build(&filter, &folder, args.no_timing.unwrap_or(false)) { Err(()) => std::process::exit(1), Ok(_) => { - args.after_build.map(|command| cmd::run(command)); + if let Some(args_after_build) = args.after_build { + cmd::run(args_after_build) + } std::process::exit(0) } }; diff --git a/src/watcher.rs b/src/watcher.rs index 0728c22..674d5f6 100644 --- a/src/watcher.rs +++ b/src/watcher.rs @@ -9,7 +9,7 @@ use crate::queue::*; use futures_timer::Delay; use notify::event::ModifyKind; use notify::{Config, Error, Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher}; -use std::path::PathBuf; +use std::path::Path; use std::sync::Arc; use std::sync::Mutex; use std::time::{Duration, Instant}; @@ -21,24 +21,24 @@ enum CompileType { None, } -fn is_rescript_file(path_buf: &PathBuf) -> bool { +fn is_rescript_file(path_buf: &Path) -> bool { let extension = path_buf.extension().and_then(|ext| ext.to_str()); if let Some(extension) = extension { - helpers::is_implementation_file(&extension) || helpers::is_interface_file(&extension) + helpers::is_implementation_file(extension) || helpers::is_interface_file(extension) } else { false } } -fn is_in_build_path(path_buf: &PathBuf) -> bool { +fn is_in_build_path(path_buf: &Path) -> bool { path_buf .to_str() .map(|x| x.contains("/lib/bs/") || x.contains("/lib/ocaml/")) .unwrap_or(false) } -fn matches_filter(path_buf: &PathBuf, filter: &Option) -> bool { +fn matches_filter(path_buf: &Path, filter: &Option) -> bool { let name = path_buf .file_name() .map(|x| x.to_string_lossy().to_string()) @@ -79,9 +79,8 @@ async fn async_watch( Delay::new(Duration::from_millis(50)).await; } while !q.is_empty() { - match q.pop() { - Ok(event) => events.push(event), - Err(_) => (), + if let Ok(event) = q.pop() { + events.push(event) } } @@ -127,7 +126,7 @@ async fn async_watch( .expect("Package not found"); let canonicalized_implementation_file = std::path::PathBuf::from(package.path.to_string()) - .join(source_file.implementation.path.to_string()); + .join(&source_file.implementation.path); if canonicalized_path_buf == canonicalized_implementation_file { if let Ok(modified) = canonicalized_path_buf.metadata().and_then(|x| x.modified()) @@ -142,7 +141,7 @@ async fn async_watch( if let Some(ref mut interface) = source_file.interface { let canonicalized_interface_file = std::path::PathBuf::from(package.path.to_string()) - .join(interface.path.to_string()); + .join(&interface.path); if canonicalized_path_buf == canonicalized_interface_file { if let Ok(modified) = canonicalized_path_buf .metadata() @@ -179,24 +178,17 @@ async fn async_watch( match needs_compile_type { CompileType::Incremental => { let timing_total = Instant::now(); - match build::incremental_build( - &mut build_state, - None, - initial_build, - if initial_build { false } else { true }, - ) { - Ok(_) => { - after_build.clone().map(|command| cmd::run(command)); - let timing_total_elapsed = timing_total.elapsed(); - println!( - "\n{}{}Finished {} compilation in {:.2}s\n", - LINE_CLEAR, - SPARKLES, - if initial_build { "initial" } else { "incremental" }, - timing_total_elapsed.as_secs_f64() - ); - } - Err(_) => (), + if build::incremental_build(&mut build_state, None, initial_build, !initial_build).is_ok() + { + if let Some(a) = after_build.clone() { cmd::run(a) } + let timing_total_elapsed = timing_total.elapsed(); + println!( + "\n{}{}Finished {} compilation in {:.2}s\n", + LINE_CLEAR, + SPARKLES, + if initial_build { "initial" } else { "incremental" }, + timing_total_elapsed.as_secs_f64() + ); } needs_compile_type = CompileType::None; initial_build = false; @@ -205,7 +197,7 @@ async fn async_watch( let timing_total = Instant::now(); build_state = build::initialize_build(None, filter, path).expect("Can't initialize build"); let _ = build::incremental_build(&mut build_state, None, initial_build, false); - after_build.clone().map(|command| cmd::run(command)); + if let Some(a) = after_build.clone() { cmd::run(a) } let timing_total_elapsed = timing_total.elapsed(); println!( "\n{}{}Finished compilation in {:.2}s\n",