ci: add clippy (#7675)

* ci: add clippy

* fix: warning to show ci failure

* fix: run `cargo clippy --fix`

* fix: rogue comment

* fix: redundant match

* fix: too many args by introducing a wrapper struct

* chore: run cargo fmt

* refactor: get_source_files split match

* refactor: get_source_files use `PackageSource.is_type_dev()`

* refactor: simplify get_source_files

* chore: CHANGELOG

* fix: warnings after rebase

Author: Bushuo

.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: |

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

rewatch/src/build.rs

@@ -58,7 +58,7 @@ pub struct CompilerArgs {
 pub fn get_compiler_args(path: &Path) -> Result<String> {
     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<String> {
     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<String> {
         &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<String> {
-    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",
@@ -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());
 }

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,91 @@ 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 {
-                    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");
-                }
-                _ => (),
+            if let SourceType::SourceFile(SourceFile {
+                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
+                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 {
-                    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");
-                }
-                _ => (),
+            if let SourceType::SourceFile(SourceFile {
+                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
+                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 {
-                            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");
-                            }
+                    if let SourceType::SourceFile(SourceFile {
+                        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),
+                        );
+                        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");
                         }
-                        _ => (),
                     }
                 }
             });

rewatch/src/build/compile/dependency_cycle.rs

@@ -60,12 +60,12 @@ fn find_shortest_cycle(modules: &Vec<(&String, &Module)>) -> Vec<String> {
         }
 
         // 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();