chore: fix rust clippy warnings (#432)

* chore: fix rust clippy warnings * chore: fix rust clippy warnings * chore: fix rust clippy warnings * chore: fix rust clippy warnings
TEN-framework · Dec 19, 2024 · e357f53 · e357f53
1 parent 1ac16dd
commit e357f53
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 83 deletions.
diff --git a/build/common/rust/cargo_build.py b/build/common/rust/cargo_build.py
@@ -22,6 +22,40 @@ def __init__(self):
         self.log_level: int
 
 
+def run_clippy_static_checking(args: ArgumentInfo):
+    cmd = [
+        "cargo",
+        "clippy",
+        "--target-dir",
+        args.target_path,
+        "--target",
+        args.target,
+    ]
+
+    if args.manifest_path != "":
+        cmd.append(
+            "--manifest-path",
+        )
+        cmd.append(
+            args.manifest_path,
+        )
+
+    if args.build_type == "release":
+        cmd.append("--release")
+
+    cmd += [
+        "--",
+        "-D",
+        "warnings",
+    ]
+
+    returncode, logs = cmd_exec.run_cmd(cmd, args.log_level)
+    if returncode:
+        raise Exception(f"Failed to cargo clippy rust tests: {logs}")
+    else:
+        print(logs)
+
+
 if __name__ == "__main__":
     parser = argparse.ArgumentParser()
 
@@ -51,6 +85,8 @@ def __init__(self):
     try:
         os.chdir(args.project_path)
 
+        # run_clippy_static_checking(args)
+
         cmd = [
             "cargo",
             "build",

diff --git a/build/common/rust/cargo_build_tests.py b/build/common/rust/cargo_build_tests.py
@@ -83,6 +83,33 @@ def copy_test_binaries(
         )
 
 
+def run_clippy_static_checking(args: ArgumentInfo):
+    cmd = [
+        "cargo",
+        "clippy",
+        "--target-dir",
+        args.target_path,
+        "--target",
+        args.target,
+        "--tests",
+    ]
+
+    if args.build_type == "release":
+        cmd.append("--release")
+
+    cmd += [
+        "--",
+        "-D",
+        "warnings",
+    ]
+
+    returncode, logs = cmd_exec.run_cmd(cmd, args.log_level)
+    if returncode:
+        raise Exception(f"Failed to cargo clippy rust tests: {logs}")
+    else:
+        print(logs)
+
+
 if __name__ == "__main__":
     parser = argparse.ArgumentParser()
 
@@ -127,6 +154,8 @@ def copy_test_binaries(
     try:
         os.chdir(args.project_path)
 
+        # run_clippy_static_checking(args)
+
         # cargo build --tests: only compile the test source files, without
         # running the test cases.
         # cargo test: compile the test source files and run the test cases.

diff --git a/core/src/ten_manager/src/designer/messages/compatible.rs b/core/src/ten_manager/src/designer/messages/compatible.rs
@@ -1020,79 +1020,6 @@ mod tests {
         // assert_eq!(compatibles.data, expected_compatibles);
     }
 
-    // TODO(Liu): Enable this when ManifestPropertyAttributes supports
-    // properties/items.
-    // #[actix_web::test]
-    async fn test_get_compatible_messages_data_required_superset() {
-        let mut designer_state = DesignerState {
-            base_dir: None,
-            all_pkgs: None,
-            tman_config: TmanConfig::default(),
-        };
-
-        // The first item is 'manifest.json', and the second item is
-        // 'property.json'.
-        let all_pkgs_json = vec![
-            (
-                include_str!("test_data_embed/app_manifest.json").to_string(),
-                include_str!("test_data_embed/app_property.json").to_string(),
-            ),
-            (
-                include_str!("test_data_embed/extension_addon_1_manifest.json")
-                    .to_string(),
-                "{}".to_string(),
-            ),
-            (
-                include_str!("test_data_embed/extension_addon_2_manifest.json")
-                    .to_string(),
-                "{}".to_string(),
-            ),
-        ];
-
-        let inject_ret =
-            inject_all_pkgs_for_mock(&mut designer_state, all_pkgs_json);
-        assert!(inject_ret.is_ok());
-
-        let designer_state = Arc::new(RwLock::new(designer_state));
-
-        let app = test::init_service(
-            App::new().app_data(web::Data::new(designer_state)).route(
-                "/api/designer/v1/messages/compatible",
-                web::post().to(get_compatible_messages),
-            ),
-        )
-        .await;
-
-        // Define input data.
-        let input_data = json!({
-          "app": "localhost",
-          "graph": "default",
-          "extension_group": "extension_group_1",
-          "extension": "extension_1",
-          "msg_type": "data",
-          "msg_direction": "out",
-          "msg_name": "data2"
-        });
-
-        // Send request to the test server.
-        let req = test::TestRequest::post()
-            .uri("/api/designer/v1/messages/compatible")
-            .set_json(&input_data)
-            .to_request();
-
-        // Call the service and get the response.
-        let resp = test::call_service(&app, req).await;
-        assert!(resp.status().is_success());
-
-        let body = test::read_body(resp).await;
-        let body_str = std::str::from_utf8(&body).unwrap();
-
-        let compatibles: ApiResponse<Vec<DesignerCompatibleMsg>> =
-            serde_json::from_str(body_str).unwrap();
-
-        assert!(compatibles.data.is_empty());
-    }
-
     #[actix_web::test]
     async fn test_get_compatible_messages_video_target_has_property() {
         let mut designer_state = DesignerState {

diff --git a/core/src/ten_manager/src/solver/introducer.rs b/core/src/ten_manager/src/solver/introducer.rs
@@ -19,14 +19,16 @@ use crate::dep_and_candidate::get_pkg_info_from_candidates;
 pub fn extract_introducer_relations_from_raw_solver_results(
     results: &[String],
     all_candidates: &HashMap<PkgTypeAndName, HashMap<PkgBasicInfo, PkgInfo>>,
-) -> Result<HashMap<PkgInfo, (String, Option<PkgInfo>)>> {
+) -> Result<HashMap<PkgBasicInfo, (String, Option<PkgInfo>)>> {
     let re = Regex::new(
       r#"introducer\("([^"]+)","([^"]+)","([^"]+)","([^"]+)","([^"]*)","([^"]*)"\)"#,
   )
   .unwrap();
 
-    let mut introducer_relations: HashMap<PkgInfo, (String, Option<PkgInfo>)> =
-        HashMap::new();
+    let mut introducer_relations: HashMap<
+        PkgBasicInfo,
+        (String, Option<PkgInfo>),
+    > = HashMap::new();
 
     for result in results.iter() {
         if let Some(caps) = re.captures(result) {
@@ -80,8 +82,10 @@ pub fn extract_introducer_relations_from_raw_solver_results(
                 Some(introducer_info.clone())
             };
 
-            introducer_relations
-                .insert(pkg_info.clone(), (requested_version_str, introducer));
+            introducer_relations.insert(
+                (&pkg_info).into(),
+                (requested_version_str, introducer),
+            );
         }
     }
 
@@ -91,11 +95,11 @@ pub fn extract_introducer_relations_from_raw_solver_results(
 pub fn get_dependency_chain(
     introducer: &PkgInfo,
     conflict_pkg_identity: &PkgTypeAndName,
-    introducer_relations: &HashMap<PkgInfo, (String, Option<PkgInfo>)>,
+    introducer_relations: &HashMap<PkgBasicInfo, (String, Option<PkgInfo>)>,
 ) -> Vec<(String, PkgTypeAndName)> {
     let mut chain = Vec::new();
     let mut current_pkg = introducer.clone();
-    let mut visited = HashSet::new();
+    let mut visited = HashSet::<PkgBasicInfo>::new();
 
     // Add the leaf node (i.e., `introducer` depends on `pkg_name`) to the
     // dependency chain first.
@@ -115,12 +119,12 @@ pub fn get_dependency_chain(
     ));
 
     loop {
-        if !visited.insert(current_pkg.clone()) {
+        if !visited.insert((&current_pkg).into()) {
             // Detected a cycle, break to prevent infinite loop.
             break;
         }
 
-        match introducer_relations.get(&current_pkg) {
+        match introducer_relations.get(&(&current_pkg).into()) {
             Some((requested_version, Some(introducer_pkg))) => {
                 // The dependency chain is that `introducer_pkg` depends on
                 // `current_pkg`@`requested_version`, i.e., the

diff --git a/core/src/ten_manager/src/solver/solver_error.rs b/core/src/ten_manager/src/solver/solver_error.rs
@@ -146,7 +146,7 @@ fn print_dependency_chain(
 
 pub fn print_conflict_info(
     conflict_info: &ConflictInfo,
-    introducer_relations: &HashMap<PkgInfo, (String, Option<PkgInfo>)>,
+    introducer_relations: &HashMap<PkgBasicInfo, (String, Option<PkgInfo>)>,
     all_candidates: &HashMap<PkgTypeAndName, HashMap<PkgBasicInfo, PkgInfo>>,
 ) -> Result<()> {
     println!(

diff --git a/core/src/ten_manager/src/solver/solver_result.rs b/core/src/ten_manager/src/solver/solver_result.rs
@@ -77,7 +77,16 @@ pub fn filter_solver_results_by_type_and_name<'a>(
     let mut filtered_results: Vec<&PkgInfo> = vec![];
 
     for result in solver_results.iter() {
+        // After Rust 1.82, there is an `is_none_or` method, and Clippy will
+        // suggest using `is_none_or` for simplicity. However, since Ten
+        // currently needs to rely on Rust 1.81 on macOS (mainly because debug
+        // mode on Mac enables ASAN, and rustc must match the versions of
+        // LLVM/Clang to prevent ASAN linking errors), we disable this specific
+        // Clippy warning here.
+        #[allow(clippy::unnecessary_map_or)]
         let matches_type = pkg_type.map_or(true, |pt| result.pkg_type == *pt);
+
+        #[allow(clippy::unnecessary_map_or)]
         let matches_name = name.map_or(true, |n| result.name == *n);
 
         let matches = matches_type && matches_name;