Merge pull request #2289 from fzyzcjy/feat/12562

frb_codegen/src/library/build_web/mod.rs

@@ -9,7 +9,6 @@ use itertools::Itertools;
 use log::debug;
 use std::path::{Path, PathBuf};
 use std::process::{Command, ExitStatus};
-use std::str::FromStr;
 use std::{env, fs};
 
 // We make the core build-web logic in Dart, and Rust is just a wrapper.
@@ -39,7 +38,7 @@ fn execute_dart_command(
     args: &[String],
     dart_coverage: bool,
 ) -> anyhow::Result<()> {
-    let repo = DartRepository::from_str(&path_to_string(dart_root)?)?;
+    let repo = DartRepository::from_path(dart_root)?;
 
     let dart_run_args = {
         let mut ans = vec![

frb_codegen/src/library/codegen/polisher/auto_upgrade.rs

@@ -2,7 +2,6 @@ use crate::codegen::misc::GeneratorProgressBarPack;
 use crate::integration::integrator::pub_add_dependency_frb;
 use crate::library::commands::cargo::cargo_add;
 use crate::utils::dart_repository::dart_repo::{DartDependencyMode, DartRepository};
-use crate::utils::path_utils::path_to_string;
 use anyhow::Result;
 use cargo_metadata::VersionReq;
 use std::path::Path;
@@ -35,7 +34,7 @@ struct DartUpgrader;
 
 impl Upgrader for DartUpgrader {
     fn check(base_dir: &Path) -> Result<bool> {
-        let repo = DartRepository::from_str(&path_to_string(base_dir)?)?;
+        let repo = DartRepository::from_path(base_dir)?;
         Ok(repo
             .has_specified_and_installed(
                 "flutter_rust_bridge",

frb_codegen/src/library/codegen/polisher/mod.rs

@@ -6,15 +6,13 @@ use crate::library::commands::dart_build_runner::dart_build_runner;
 use crate::library::commands::dart_fix::dart_fix;
 use crate::library::commands::dart_format::dart_format;
 use crate::utils::dart_repository::dart_repo::{DartDependencyMode, DartRepository};
-use crate::utils::path_utils::path_to_string;
 use anyhow::Context;
 use cargo_metadata::VersionReq;
 use itertools::Itertools;
 use lazy_static::lazy_static;
 use log::warn;
 use std::fs;
 use std::path::{Path, PathBuf};
-use std::str::FromStr;
 
 pub(crate) mod add_mod_to_lib;
 mod auto_upgrade;
@@ -68,7 +66,7 @@ fn ensure_dependency_freezed(
     }
 
     if needs_freezed {
-        let repo = DartRepository::from_str(&path_to_string(&config.dart_root)?)?;
+        let repo = DartRepository::from_path(&config.dart_root)?;
         repo.has_specified_and_installed("freezed", DartDependencyMode::Dev, &ANY_REQUIREMENT)?;
         repo.has_specified_and_installed(
             "freezed_annotation",

frb_codegen/src/library/commands/dart_build_runner.rs

@@ -1,17 +1,15 @@
 use crate::command_run;
 use crate::commands::command_runner::call_shell;
 use crate::utils::dart_repository::dart_repo::DartRepository;
-use crate::utils::path_utils::path_to_string;
 use anyhow::bail;
 use log::debug;
 use std::collections::HashMap;
 use std::path::Path;
-use std::str::FromStr;
 
 pub fn dart_build_runner(dart_root: &Path) -> anyhow::Result<()> {
     debug!("Running build_runner at dart_root={dart_root:?}");
 
-    let repo = DartRepository::from_str(&path_to_string(dart_root)?).unwrap();
+    let repo = DartRepository::from_path(dart_root).unwrap();
     let out = command_run!(
         call_shell[Some(dart_root), Some(dart_run_extra_env())],
         *repo.toolchain.as_run_command(),

frb_codegen/src/library/commands/ensure_tools_available.rs

@@ -1,10 +1,8 @@
 use crate::utils::dart_repository::dart_repo::{DartDependencyMode, DartRepository};
-use crate::utils::path_utils::path_to_string;
 use anyhow::bail;
 use cargo_metadata::VersionReq;
 use lazy_static::lazy_static;
 use std::path::Path;
-use std::str::FromStr;
 
 lazy_static! {
     pub(crate) static ref FFIGEN_REQUIREMENT: VersionReq = VersionReq::parse(">= 8.0.0").unwrap();
@@ -15,7 +13,7 @@ pub fn ensure_tools_available(
     enable_deps_check: bool,
     needs_ffigen: bool,
 ) -> anyhow::Result<()> {
-    let repo = DartRepository::from_str(&path_to_string(dart_root)?)?;
+    let repo = DartRepository::from_path(dart_root)?;
     if !repo.toolchain_available() {
         // This will stop the whole generator and tell the users, so we do not care about testing it
         // frb-coverage:ignore-start

frb_codegen/src/library/commands/ffigen.rs

@@ -2,7 +2,6 @@ use super::dart_build_runner::dart_run_extra_env;
 use crate::command_run;
 use crate::commands::command_runner::call_shell;
 use crate::utils::dart_repository::dart_repo::DartRepository;
-use crate::utils::path_utils::path_to_string;
 use anyhow::bail;
 use itertools::Itertools;
 use log::{debug, warn};
@@ -11,7 +10,6 @@ use std::collections::HashMap;
 use std::fs;
 use std::io::Write;
 use std::path::{Path, PathBuf};
-use std::str::FromStr;
 
 pub(crate) struct FfigenArgs<'a> {
     pub c_file_content: &'a str,
@@ -78,7 +76,7 @@ pub(crate) fn ffigen_raw(config: &FfigenCommandConfig, dart_root: &Path) -> anyh
     config_file.write_all(config.as_bytes())?;
     debug!("ffigen_raw config={config:?} config_file={config_file:?}");
 
-    let repo = DartRepository::from_str(&path_to_string(dart_root)?).unwrap();
+    let repo = DartRepository::from_path(dart_root).unwrap();
     let res = command_run!(
         call_shell[Some(dart_root), Some(dart_run_extra_env())],
         *repo.toolchain.as_run_command(),

frb_codegen/src/library/utils/dart_repository/dart_repo.rs

@@ -3,44 +3,41 @@ use crate::utils::dart_repository::pubspec::*;
 use anyhow::{anyhow, bail, Context};
 use cargo_metadata::{Version, VersionReq};
 use log::debug;
+use serde::de::DeserializeOwned;
+use std::collections::HashMap;
 use std::convert::TryFrom;
 use std::fmt::Display;
-use std::path::PathBuf;
-use std::str::FromStr;
+use std::hash::Hash;
+use std::path::{Path, PathBuf};
 
 /// represents a dart / flutter repository
 pub(crate) struct DartRepository {
     pub(crate) at: PathBuf,
     pub(crate) toolchain: DartToolchain,
 }
 
-// TODO it is from path, not from str
-impl FromStr for DartRepository {
-    type Err = anyhow::Error;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+impl DartRepository {
+    pub(crate) fn from_path(path: &Path) -> anyhow::Result<Self> {
         debug!("Guessing toolchain the runner is run into");
-        let filename = DartToolchain::lock_filename();
-        let lock_file = read_file(s, filename)?;
-        let lock_file: PubspecLock = serde_yaml::from_str(&lock_file)
-            .map_err(|e| anyhow!("unable to parse {filename} in {s}: {e:#}"))?;
-        if lock_file
-            .packages
-            .contains_key(&DartToolchain::Flutter.to_string())
-        {
-            return Ok(DartRepository {
-                at: PathBuf::from(s),
-                toolchain: DartToolchain::Flutter,
-            });
-        }
+
+        let manifest_file: PubspecYaml =
+            read_file_and_parse_yaml(path, DartToolchain::manifest_filename())?;
+
+        let toolchain = if option_hash_map_contains(
+            &manifest_file.dependencies,
+            &DartToolchain::Flutter.to_string(),
+        ) {
+            DartToolchain::Flutter
+        } else {
+            DartToolchain::Dart
+        };
+
         Ok(DartRepository {
-            at: PathBuf::from(s),
-            toolchain: DartToolchain::Dart,
+            at: path.to_owned(),
+            toolchain,
         })
     }
-}
 
-impl DartRepository {
     /// check whether the toolchain is available from the CLI
     pub(crate) fn toolchain_available(&self) -> bool {
         self.toolchain.available()
@@ -67,18 +64,10 @@ impl DartRepository {
         manager: DartDependencyMode,
         requirement: &VersionReq,
     ) -> anyhow::Result<()> {
-        let at = self.at.to_str().unwrap();
-        debug!("Checking presence of {} in {} at {}", package, manager, at);
-        let manifest_file = read_file(at, DartToolchain::manifest_filename())?;
-        let manifest_file: PubspecYaml = serde_yaml::from_str(&manifest_file).map_err(|e| {
-            // frb-coverage:ignore-start
-            // This will stop the whole generator and tell the users, so we do not care about testing it
-            anyhow!(
-                "unable to parse {} in {at}: {e:#}",
-                DartToolchain::manifest_filename()
-            )
-            // frb-coverage:ignore-end
-        })?;
+        let at = &self.at;
+        debug!("Checking presence of {package} in {manager} at {at:?}");
+        let manifest_file: PubspecYaml =
+            read_file_and_parse_yaml(at, DartToolchain::manifest_filename())?;
         let deps = match manager {
             DartDependencyMode::Main => manifest_file.dependencies.unwrap_or_default(),
             DartDependencyMode::Dev => manifest_file.dev_dependencies.unwrap_or_default(),
@@ -99,18 +88,19 @@ impl DartRepository {
         manager: DartDependencyMode,
         requirement: &VersionReq,
     ) -> anyhow::Result<()> {
-        let at = self.at.to_str().unwrap();
-        debug!("Checking presence of {} in {} at {}", package, manager, at);
-        let lock_file = read_file(at, DartToolchain::lock_filename())?;
-        let lock_file: PubspecLock = serde_yaml::from_str(&lock_file).map_err(|e| {
-            // This will stop the whole generator and tell the users, so we do not care about testing it
-            // frb-coverage:ignore-start
-            anyhow!(
-                "unable to parse {} in {at}: {e:#}",
-                DartToolchain::lock_filename()
-            )
-            // frb-coverage:ignore-end
-        })?;
+        let at = &self.at;
+        let filename = DartToolchain::lock_filename();
+        debug!("Checking presence of {package} in {manager} at {at:?}");
+
+        // We do not care about this branch
+        // frb-coverage:ignore-start
+        if !at.join(filename).exists() {
+            log::warn!("Skip checking presence of {package} in {manager} at {at:?} since {filename} does not exist. Please check manually.");
+            return Ok(());
+        }
+        // frb-coverage:ignore-end
+
+        let lock_file: PubspecLock = read_file_and_parse_yaml(at, filename)?;
         let dependency = lock_file.packages.get(package);
         let version = match dependency {
             Some(dependency) => {
@@ -126,9 +116,7 @@ impl DartRepository {
                     // frb-coverage:ignore-start
                     anyhow::Error::msg(format!(
                         "unable to parse {} version in {}: {:#}",
-                        package,
-                        DartToolchain::lock_filename(),
-                        e
+                        package, filename, e
                     ))
                     // frb-coverage:ignore-end
                 })?
@@ -143,10 +131,9 @@ impl DartRepository {
             DartPackageVersion::Exact(ref v) if requirement.matches(v) => Ok(()),
             // This will stop the whole generator and tell the users, so we do not care about testing it
             // frb-coverage:ignore-start
-            DartPackageVersion::Range(_) => bail!(
-                "unexpected version range for {package} in {}",
-                DartToolchain::lock_filename()
-            ),
+            DartPackageVersion::Range(_) => {
+                bail!("unexpected version range for {package} in {}", filename)
+            }
             _ => Err(error_invalid_dep(package, manager, requirement)),
             // frb-coverage:ignore-end
         }
@@ -222,20 +209,38 @@ impl Display for DartPackageVersion {
     }
 }
 
-#[inline]
-fn read_file(at: &str, filename: &str) -> anyhow::Result<String> {
-    let file = PathBuf::from(at).join(filename);
+fn read_file(at: &Path, filename: &str) -> anyhow::Result<String> {
+    let file = at.join(filename);
     if !file.exists() {
         // This will stop the whole generator and tell the users, so we do not care about testing it
         // frb-coverage:ignore-start
-        bail!("missing {filename} in {at}");
+        bail!("missing {filename} in {at:?}");
         // frb-coverage:ignore-end
     }
     let content = std::fs::read_to_string(file)
-        .with_context(|| format!("unable to read {filename} in {at}"))?;
+        .with_context(|| format!("unable to read {filename} in {at:?}"))?;
     Ok(content)
 }
 
+fn read_file_and_parse_yaml<T: DeserializeOwned>(at: &Path, filename: &str) -> anyhow::Result<T> {
+    let file = read_file(at, filename)?;
+    let file: T = serde_yaml::from_str(&file).map_err(|e| {
+        // frb-coverage:ignore-start
+        // This will stop the whole generator and tell the users, so we do not care about testing it
+        anyhow!("Unable to parse {filename} in {at:?}: {e:#}")
+        // frb-coverage:ignore-end
+    })?;
+    Ok(file)
+}
+
+fn option_hash_map_contains<K: Hash + Eq, V: Eq>(map: &Option<HashMap<K, V>>, key: &K) -> bool {
+    if let Some(map) = map.as_ref() {
+        map.contains_key(key)
+    } else {
+        false
+    }
+}
+
 impl PubspecLockPackage {
     pub(crate) fn installed_in(&self) -> Option<DartDependencyMode> {
         match self.dependency.as_str() {

frb_codegen/src/library/utils/dart_repository/mod.rs

@@ -37,7 +37,6 @@ mod tests {
     use std::{
         collections::HashMap,
         path::{Path, PathBuf},
-        str::FromStr,
     };
 
     lazy_static! {
@@ -49,7 +48,7 @@ mod tests {
     }
 
     fn guess_toolchain_base(path: &Path, expect_toolchain: DartToolchain) {
-        let repo = DartRepository::from_str(&path.to_string_lossy())
+        let repo = DartRepository::from_path(path)
             .unwrap_or_else(|_| panic!("can get toolchain from {}", path.to_string_lossy()));
         assert_eq!(repo.toolchain, expect_toolchain);
     }