Merge pull request #1255 from PowerShell/copilot/replace-file-paths-with-pathbuf

Replace String/&str with PathBuf/&Path for file path handling

Author: SteveL-MSFT

dsc/src/subcommand.rs

@@ -263,7 +263,7 @@ fn initialize_config_root(path: Option<&String>) -> Option<String> {
     } else {
         let current_directory = std::env::current_dir().unwrap_or_default();
         debug!("DSC_CONFIG_ROOT = {} '{current_directory:?}'", t!("subcommand.currentDirectory"));
-        set_dscconfigroot(&current_directory.to_string_lossy());
+        set_dscconfigroot(current_directory.to_str().unwrap_or_default());
     }
 
     // if the path is "-", we need to return it so later processing can handle it correctly

dsc/src/util.rs

@@ -469,8 +469,9 @@ pub fn get_input(input: Option<&String>, file: Option<&String>) -> String {
         } else {
             // see if an extension should handle this file
             let mut discovery = Discovery::new();
+            let path_buf = Path::new(path);
             for extension in discovery.get_extensions(&Capability::Import) {
-                if let Ok(content) = extension.import(path) {
+                if let Ok(content) = extension.import(path_buf) {
                     return content;
                 }
             }

lib/dsc-lib/src/discovery/command_discovery.rs

@@ -751,8 +751,8 @@ fn load_resource_manifest(path: &Path, manifest: &ResourceManifest) -> Result<Ds
         description: manifest.description.clone(),
         version: manifest.version.clone(),
         capabilities,
-        path: path.to_str().unwrap().to_string(),
-        directory: path.parent().unwrap().to_str().unwrap().to_string(),
+        path: path.to_path_buf(),
+        directory: path.parent().unwrap().to_path_buf(),
         manifest: Some(serde_json::to_value(manifest)?),
         ..Default::default()
     };
@@ -793,8 +793,8 @@ fn load_extension_manifest(path: &Path, manifest: &ExtensionManifest) -> Result<
         version: manifest.version.clone(),
         capabilities,
         import_file_extensions: import_extensions,
-        path: path.to_str().unwrap().to_string(),
-        directory: path.parent().unwrap().to_str().unwrap().to_string(),
+        path: path.to_path_buf(),
+        directory: path.parent().unwrap().to_path_buf(),
         manifest: serde_json::to_value(manifest)?,
         ..Default::default()
     };
@@ -803,7 +803,7 @@ fn load_extension_manifest(path: &Path, manifest: &ExtensionManifest) -> Result<
 }
 
 fn verify_executable(resource: &str, operation: &str, executable: &str, directory: &Path) {
-    if canonicalize_which(executable, Some(directory.to_string_lossy().as_ref())).is_err() {
+    if canonicalize_which(executable, Some(directory)).is_err() {
         info!("{}", t!("discovery.commandDiscovery.executableNotFound", resource = resource, operation = operation, executable = executable));
     }
 }

lib/dsc-lib/src/dscresources/command_resource.rs

@@ -6,7 +6,7 @@ use jsonschema::Validator;
 use rust_i18n::t;
 use serde::Deserialize;
 use serde_json::{Map, Value};
-use std::{collections::HashMap, env, process::Stdio};
+use std::{collections::HashMap, env, path::Path, process::Stdio};
 use crate::{configure::{config_doc::ExecutionKind, config_result::{ResourceGetResult, ResourceTestResult}}, util::canonicalize_which};
 use crate::dscerror::DscError;
 use super::{dscresource::{get_diff, redact}, invoke_result::{ExportResult, GetResult, ResolveResult, SetResult, TestResult, ValidateResult, ResourceGetResponse, ResourceSetResponse, ResourceTestResponse, get_in_desired_state}, resource_manifest::{ArgKind, InputKind, Kind, ResourceManifest, ReturnKind, SchemaKind}};
@@ -25,7 +25,7 @@ pub const EXIT_PROCESS_TERMINATED: i32 = 0x102;
 /// # Errors
 ///
 /// Error returned if the resource does not successfully get the current state
-pub fn invoke_get(resource: &ResourceManifest, cwd: &str, filter: &str, target_resource: Option<&str>) -> Result<GetResult, DscError> {
+pub fn invoke_get(resource: &ResourceManifest, cwd: &Path, filter: &str, target_resource: Option<&str>) -> Result<GetResult, DscError> {
     debug!("{}", t!("dscresources.commandResource.invokeGet", resource = &resource.resource_type));
     let mut command_input = CommandInput { env: None, stdin: None };
     let Some(get) = &resource.get else {
@@ -78,7 +78,7 @@ pub fn invoke_get(resource: &ResourceManifest, cwd: &str, filter: &str, target_r
 ///
 /// Error returned if the resource does not successfully set the desired state
 #[allow(clippy::too_many_lines)]
-pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_test: bool, execution_type: &ExecutionKind, target_resource: Option<&str>) -> Result<SetResult, DscError> {
+pub fn invoke_set(resource: &ResourceManifest, cwd: &Path, desired: &str, skip_test: bool, execution_type: &ExecutionKind, target_resource: Option<&str>) -> Result<SetResult, DscError> {
     debug!("{}", t!("dscresources.commandResource.invokeSet", resource = &resource.resource_type));
     let operation_type: String;
     let mut is_synthetic_what_if = false;
@@ -271,7 +271,7 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te
 /// # Errors
 ///
 /// Error is returned if the underlying command returns a non-zero exit code.
-pub fn invoke_test(resource: &ResourceManifest, cwd: &str, expected: &str, target_resource: Option<&str>) -> Result<TestResult, DscError> {
+pub fn invoke_test(resource: &ResourceManifest, cwd: &Path, expected: &str, target_resource: Option<&str>) -> Result<TestResult, DscError> {
     debug!("{}", t!("dscresources.commandResource.invokeTest", resource = &resource.resource_type));
     let Some(test) = &resource.test else {
         info!("{}", t!("dscresources.commandResource.testSyntheticTest", resource = &resource.resource_type));
@@ -380,7 +380,7 @@ fn get_desired_state(actual: &Value) -> Result<Option<bool>, DscError> {
     Ok(in_desired_state)
 }
 
-fn invoke_synthetic_test(resource: &ResourceManifest, cwd: &str, expected: &str, target_resource: Option<&str>) -> Result<TestResult, DscError> {
+fn invoke_synthetic_test(resource: &ResourceManifest, cwd: &Path, expected: &str, target_resource: Option<&str>) -> Result<TestResult, DscError> {
     let get_result = invoke_get(resource, cwd, expected, target_resource)?;
     let actual_state = match get_result {
         GetResult::Group(results) => {
@@ -415,7 +415,7 @@ fn invoke_synthetic_test(resource: &ResourceManifest, cwd: &str, expected: &str,
 /// # Errors
 ///
 /// Error is returned if the underlying command returns a non-zero exit code.
-pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str, target_resource: Option<&str>) -> Result<(), DscError> {
+pub fn invoke_delete(resource: &ResourceManifest, cwd: &Path, filter: &str, target_resource: Option<&str>) -> Result<(), DscError> {
     let Some(delete) = &resource.delete else {
         return Err(DscError::NotImplemented("delete".to_string()));
     };
@@ -450,7 +450,7 @@ pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str, targe
 /// # Errors
 ///
 /// Error is returned if the underlying command returns a non-zero exit code.
-pub fn invoke_validate(resource: &ResourceManifest, cwd: &str, config: &str, target_resource: Option<&str>) -> Result<ValidateResult, DscError> {
+pub fn invoke_validate(resource: &ResourceManifest, cwd: &Path, config: &str, target_resource: Option<&str>) -> Result<ValidateResult, DscError> {
     trace!("{}", t!("dscresources.commandResource.invokeValidateConfig", resource = &resource.resource_type, config = &config));
     // TODO: use schema to validate config if validate is not implemented
     let Some(validate) = resource.validate.as_ref() else {
@@ -479,7 +479,7 @@ pub fn invoke_validate(resource: &ResourceManifest, cwd: &str, config: &str, tar
 /// # Errors
 ///
 /// Error if schema is not available or if there is an error getting the schema
-pub fn get_schema(resource: &ResourceManifest, cwd: &str) -> Result<String, DscError> {
+pub fn get_schema(resource: &ResourceManifest, cwd: &Path) -> Result<String, DscError> {
     let Some(schema_kind) = resource.schema.as_ref() else {
         return Err(DscError::SchemaNotAvailable(resource.resource_type.clone()));
     };
@@ -511,7 +511,7 @@ pub fn get_schema(resource: &ResourceManifest, cwd: &str) -> Result<String, DscE
 /// # Errors
 ///
 /// Error returned if the resource does not successfully export the current state
-pub fn invoke_export(resource: &ResourceManifest, cwd: &str, input: Option<&str>, target_resource: Option<&str>) -> Result<ExportResult, DscError> {
+pub fn invoke_export(resource: &ResourceManifest, cwd: &Path, input: Option<&str>, target_resource: Option<&str>) -> Result<ExportResult, DscError> {
     let Some(export) = resource.export.as_ref() else {
         // see if get is supported and use that instead
         if resource.get.is_some() {
@@ -591,7 +591,7 @@ pub fn invoke_export(resource: &ResourceManifest, cwd: &str, input: Option<&str>
 /// # Errors
 ///
 /// Error returned if the resource does not successfully resolve the input
-pub fn invoke_resolve(resource: &ResourceManifest, cwd: &str, input: &str) -> Result<ResolveResult, DscError> {
+pub fn invoke_resolve(resource: &ResourceManifest, cwd: &Path, input: &str) -> Result<ResolveResult, DscError> {
     let Some(resolve) = &resource.resolve else {
         return Err(DscError::Operation(t!("dscresources.commandResource.resolveNotSupported", resource = &resource.resource_type).to_string()));
     };
@@ -620,7 +620,7 @@ pub fn invoke_resolve(resource: &ResourceManifest, cwd: &str, input: &str) -> Re
 ///
 /// Error is returned if the command fails to execute or stdin/stdout/stderr cannot be opened.
 ///
-async fn run_process_async(executable: &str, args: Option<Vec<String>>, input: Option<&str>, cwd: Option<&str>, env: Option<HashMap<String, String>>, exit_codes: Option<&HashMap<i32, String>>) -> Result<(i32, String, String), DscError> {
+async fn run_process_async(executable: &str, args: Option<Vec<String>>, input: Option<&str>, cwd: Option<&Path>, env: Option<HashMap<String, String>>, exit_codes: Option<&HashMap<i32, String>>) -> Result<(i32, String, String), DscError> {
 
     // use somewhat large initial buffer to avoid early string reallocations;
     // the value is based on list result of largest of built-in adapters - WMI adapter ~500KB
@@ -761,15 +761,15 @@ fn convert_hashmap_string_keys_to_i32(input: Option<&HashMap<String, String>>) -
 /// Will panic if tokio runtime can't be created.
 ///
 #[allow(clippy::implicit_hasher)]
-pub fn invoke_command(executable: &str, args: Option<Vec<String>>, input: Option<&str>, cwd: Option<&str>, env: Option<HashMap<String, String>>, exit_codes: Option<&HashMap<String, String>>) -> Result<(i32, String, String), DscError> {
+pub fn invoke_command(executable: &str, args: Option<Vec<String>>, input: Option<&str>, cwd: Option<&Path>, env: Option<HashMap<String, String>>, exit_codes: Option<&HashMap<String, String>>) -> Result<(i32, String, String), DscError> {
     let exit_codes = convert_hashmap_string_keys_to_i32(exit_codes)?;
     let executable = canonicalize_which(executable, cwd)?;
 
     tokio::runtime::Builder::new_multi_thread().enable_all().build().unwrap().block_on(
         async {
             trace!("{}", t!("dscresources.commandResource.commandInvoke", executable = executable, args = args : {:?}));
             if let Some(cwd) = cwd {
-                trace!("{}", t!("dscresources.commandResource.commandCwd", cwd = cwd));
+                trace!("{}", t!("dscresources.commandResource.commandCwd", cwd = cwd.display()));
             }
 
             match run_process_async(&executable, args, input, cwd, env, exit_codes.as_ref()).await {
@@ -854,7 +854,7 @@ fn get_command_input(input_kind: Option<&InputKind>, input: &str) -> Result<Comm
     })
 }
 
-fn verify_json(resource: &ResourceManifest, cwd: &str, json: &str) -> Result<(), DscError> {
+fn verify_json(resource: &ResourceManifest, cwd: &Path, json: &str) -> Result<(), DscError> {
 
     debug!("{}", t!("dscresources.commandResource.verifyJson", resource = resource.resource_type));
 

lib/dsc-lib/src/dscresources/dscresource.rs

@@ -11,6 +11,7 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use serde_json::{Map, Value};
 use std::collections::HashMap;
+use std::path::PathBuf;
 use tracing::{debug, info, trace};
 
 use super::{
@@ -37,11 +38,11 @@ pub struct DscResource {
     /// The capabilities of the resource.
     pub capabilities: Vec<Capability>,
     /// The file path to the resource.
-    pub path: String,
+    pub path: PathBuf,
     /// The description of the resource.
     pub description: Option<String>,
     // The directory path to the resource.
-    pub directory: String,
+    pub directory: PathBuf,
     /// The implementation of the resource.
     #[serde(rename="implementedAs")]
     pub implemented_as: ImplementedAs,
@@ -98,8 +99,8 @@ impl DscResource {
             version: String::new(),
             capabilities: Vec::new(),
             description: None,
-            path: String::new(),
-            directory: String::new(),
+            path: PathBuf::new(),
+            directory: PathBuf::new(),
             implemented_as: ImplementedAs::Command,
             author: None,
             properties: Vec::new(),

lib/dsc-lib/src/dscresources/include.rs

@@ -3,11 +3,12 @@
 
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
+use std::path::PathBuf;
 
 #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
 pub struct Include {
     /// The path to the file to include.  Relative paths are relative to the file containing the include
     /// and not allowed to reference parent directories.
     #[serde(rename = "configurationFile")]
-    pub configuration_file: String,
+    pub configuration_file: PathBuf,
 }

lib/dsc-lib/src/extensions/discover.rs

@@ -24,7 +24,7 @@ use crate::{
 use rust_i18n::t;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use std::path::Path;
+use std::path::PathBuf;
 use tracing::{info, trace};
 
 #[derive(Debug, Default, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
@@ -39,7 +39,7 @@ pub struct DiscoverMethod {
 pub struct DiscoverResult {
     /// The path to the resource manifest, must be absolute.
     #[serde(rename = "manifestPath")]
-    pub manifest_path: String,
+    pub manifest_path: PathBuf,
 }
 
 impl DscExtension {
@@ -70,7 +70,7 @@ impl DscExtension {
                 &discover.executable,
                 args,
                 None,
-                Some(self.directory.as_str()),
+                Some(&self.directory),
                 None,
                 extension.exit_codes.as_ref(),
             )?;
@@ -85,12 +85,11 @@ impl DscExtension {
                             return Err(DscError::Json(err));
                         }
                     };
-                    if !Path::new(&discover_result.manifest_path).is_absolute() {
-                        return Err(DscError::Extension(t!("extensions.dscextension.discoverNotAbsolutePath", extension = self.type_name.clone(), path = discover_result.manifest_path.clone()).to_string()));
+                    if !discover_result.manifest_path.is_absolute() {
+                        return Err(DscError::Extension(t!("extensions.dscextension.discoverNotAbsolutePath", extension = self.type_name.clone(), path = discover_result.manifest_path.display()).to_string()));
                     }
-                    let manifest_path = Path::new(&discover_result.manifest_path);
                     // Currently we don't support extensions discovering other extensions
-                    for imported_manifest in load_manifest(manifest_path)? {
+                    for imported_manifest in load_manifest(&discover_result.manifest_path)? {
                         if let ImportedManifest::Resource(resource) = imported_manifest {
                             resources.push(resource);
                         }

lib/dsc-lib/src/extensions/dscextension.rs

@@ -6,6 +6,7 @@ use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use schemars::JsonSchema;
 use std::fmt::Display;
+use std::path::PathBuf;
 
 #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
 #[serde(deny_unknown_fields)]
@@ -21,11 +22,11 @@ pub struct DscExtension {
     #[serde(rename = "importFileExtensions")]
     pub import_file_extensions: Option<Vec<String>>,
     /// The file path to the resource.
-    pub path: String,
+    pub path: PathBuf,
     /// The description of the resource.
     pub description: Option<String>,
     // The directory path to the resource.
-    pub directory: String,
+    pub directory: PathBuf,
     /// The author of the resource.
     pub author: Option<String>,
     /// The manifest of the resource.
@@ -63,8 +64,8 @@ impl DscExtension {
             capabilities: Vec::new(),
             import_file_extensions: None,
             description: None,
-            path: String::new(),
-            directory: String::new(),
+            path: PathBuf::new(),
+            directory: PathBuf::new(),
             author: None,
             manifest: Value::Null,
         }

lib/dsc-lib/src/extensions/import.rs

@@ -61,16 +61,15 @@ impl DscExtension {
     /// # Errors
     ///
     /// This function will return an error if the import fails or if the extension does not support the import capability.
-    pub fn import(&self, file: &str) -> Result<String, DscError> {
+    pub fn import(&self, file: &Path) -> Result<String, DscError> {
         if self.capabilities.contains(&Capability::Import) {
-            let file_path = Path::new(file);
-            let file_extension = file_path.extension().and_then(|s| s.to_str()).unwrap_or_default().to_string();
+            let file_extension = file.extension().and_then(|s| s.to_str()).unwrap_or_default().to_string();
             if self.import_file_extensions.as_ref().is_some_and(|exts| exts.contains(&file_extension)) {
-                debug!("{}", t!("extensions.dscextension.importingFile", file = file, extension = self.type_name));
+                debug!("{}", t!("extensions.dscextension.importingFile", file = file.display(), extension = self.type_name));
             } else {
-                debug!("{}", t!("extensions.dscextension.importNotSupported", file = file, extension = self.type_name));
+                debug!("{}", t!("extensions.dscextension.importNotSupported", file = file.display(), extension = self.type_name));
                 return Err(DscError::NotSupported(
-                    t!("extensions.dscextension.importNotSupported", file = file, extension = self.type_name).to_string(),
+                    t!("extensions.dscextension.importNotSupported", file = file.display(), extension = self.type_name).to_string(),
                 ));
             }
 
@@ -88,7 +87,7 @@ impl DscExtension {
                 &import.executable,
                 args,
                 None,
-                Some(self.directory.as_str()),
+                Some(&self.directory),
                 None,
                 extension.exit_codes.as_ref(),
             )?;
@@ -105,16 +104,15 @@ impl DscExtension {
     }    
 }
 
-fn process_import_args(args: Option<&Vec<ImportArgKind>>, file: &str) -> Result<Option<Vec<String>>, DscError> {
+fn process_import_args(args: Option<&Vec<ImportArgKind>>, file: &Path) -> Result<Option<Vec<String>>, DscError> {
     let Some(arg_values) = args else {
         debug!("{}", t!("dscresources.commandResource.noArgs"));
         return Ok(None);
     };
 
     // make path absolute
-    let path = Path::new(file);
-    let Ok(full_path) = path.absolutize() else {
-        return Err(DscError::Extension(t!("util.failedToAbsolutizePath", path = path : {:?}).to_string()));
+    let Ok(full_path) = file.absolutize() else {
+        return Err(DscError::Extension(t!("util.failedToAbsolutizePath", path = file.display()).to_string()));
     };
 
     let mut processed_args = Vec::<String>::new();

lib/dsc-lib/src/extensions/secret.rs

@@ -78,7 +78,7 @@ impl DscExtension {
                 &secret.executable,
                 args,
                 vault,
-                Some(self.directory.as_str()),
+                Some(&self.directory),
                 None,
                 extension.exit_codes.as_ref(),
             )?;