(MAINT) Refactor type name parsing for reuse

michaeltlombardi · michaeltlombardi · commit 82b65e2aef02 · 2026-03-31T16:48:45.000-05:00
Prior to this change, the `parse()` method for both
`FullyQualifiedTypeName` and `WildcardTypeName` contained common logic
for parsing the input string into segments and validating them.

This change adds the `FullyQualifiedTypeName::parse_segments()` method
that encapsulates the common parsing logic for both types. The new
`WildcardTypeName::parse_segments()` method is a thin wrapper around
the `FullyQualifiedTypeName` helper method to pass the correct regex
and convert the errors into the appropriate `WildcardTypeNameError`
variants.

The helper methods only validate owner and namespaces - validating the
owner segment is left to the calling `parse()` method, since the rules
for the name segment vary between the types.

This change also updates the tests to include a case for an empty
namespace segment at the end of the namespaces, which wasn't previously
covered in the integration tests for either type.
diff --git a/lib/dsc-lib/locales/en-us.toml b/lib/dsc-lib/locales/en-us.toml
@@ -875,6 +875,7 @@ invalidWildcardTypeNameFilter = "invalid wildcard type name filter: %{err}"
 emptyNamespaceSegment = "namespace segment %{index} is empty"
 emptyOwnerSegment = "owner segment is empty"
 emptyTypeName = "wildcard type name cannot be an empty string"
+invalidErrorConversion = "cannot convert `FullyQualifiedTypeName` parsing error into a `WildcardTypeName` parsing error: %{err}"
 invalidNameSegment = "name segment '%{name}' contains invalid characters"
 invalidNamespaceSegment = "namespace segment '%{namespace}' contains invalid characters"
 invalidOwnerSegment = "owner segment '%{owner}' contains invalid characters"
diff --git a/lib/dsc-lib/src/types/fully_qualified_type_name.rs b/lib/dsc-lib/src/types/fully_qualified_type_name.rs
@@ -402,17 +402,78 @@ impl FullyQualifiedTypeName {
     ///   );
     ///   ```
     pub fn parse(text: &str) -> Result<Self, FullyQualifiedTypeNameError> {
+        let errors = &mut Vec::<FullyQualifiedTypeNameError>::new();
+        let validating_segment_regex = Self::init_validating_segment_regex();
+        let (_owner, _namespaces, name) = Self::parse_segments(
+            text,
+            &validating_segment_regex,
+            errors
+        );
+        if errors.len() == 1 && errors[0] == FullyQualifiedTypeNameError::EmptyTypeName {
+            // If the only error encountered was that the input text is empty, return that error directly
+            // rather than wrapping it in an InvalidTypeName error, since an empty string is a common
+            // and specific case that can be handled directly without needing to check the list of
+            // errors for that condition.
+            return Err(errors[0].clone())
+        }
+
+        if name.is_empty() {
+            errors.push(FullyQualifiedTypeNameError::MissingNameSegment);
+        } else if !validating_segment_regex.is_match(&name) {
+            errors.push(FullyQualifiedTypeNameError::InvalidNameSegment {
+                segment_text: name.clone()
+            });
+        }
+
+        if errors.is_empty() {
+            Ok(Self(text.to_string()))
+        } else {
+            Err(FullyQualifiedTypeNameError::InvalidTypeName {
+                text: text.to_string(),
+                errors: errors.clone(),
+            })
+        }
+    }
+
+    /// Private helper to parse the input into segments and collect validation errors for owner
+    /// and namespace segments.
+    /// 
+    /// This is used by the public `parse()` method to handle the common parsing and validation
+    /// logic that is shared by [`FullyQualifiedTypeName`] and [`WildcardTypeName`] since they
+    /// share the same overall structure.
+    /// 
+    /// # Arguments
+    /// 
+    /// - `text`: The input string to parse into type name segments.
+    /// - `validating_segment_regex`: The regex to use for validating each segment of the type name.
+    /// - `errors`: A mutable reference to a vector for collecting any validation errors encountered
+    ///   while parsing the segments. This allows the method to accumulate multiple errors for
+    ///   different segments of the type name.
+    /// 
+    /// # Returns
+    /// 
+    /// The method returns the parsed segments (`owner`, `namespaces`, and `name`) as a tuple. Any
+    /// validation errors encountered during parsing are added to the provided `errors` vector for
+    /// the caller to handle.
+    pub(crate) fn parse_segments(
+        text: &str,
+        validating_segment_regex: &Regex,
+        errors: &mut Vec<FullyQualifiedTypeNameError>
+    ) -> (String, Vec<String>, String) {
         // If the input text is empty, return an error immediately to avoid unnecessary processing.
         if text.is_empty() {
-            return Err(FullyQualifiedTypeNameError::EmptyTypeName);
+            errors.push(FullyQualifiedTypeNameError::EmptyTypeName);
+            return (String::new(), Vec::new(), String::new());
         }
 
-        let errors = &mut Vec::<FullyQualifiedTypeNameError>::new();
+        // Initialize the variables to hold the parsed segments
         let owner: String;
         let namespaces: Vec<String>;
         let name: String;
-        let validating_segment_regex = Self::init_validating_segment_regex();
 
+        // Split the input text into segments based on the presence of the '/' character, which
+        // separates the name segment from the owner and namespace segments. Then split the owner
+        // and namespace portion based on '.' characters to separate the owner from the namespaces.
         if let Some((owner_and_namespaces, name_segment)) = text.rsplit_once('/') {
             name = name_segment.to_string();
             let mut segments = owner_and_namespaces
@@ -435,6 +496,7 @@ impl FullyQualifiedTypeName {
             name = String::new();
         }
 
+        // Validate the owner segment, which *must* be defined.
         if owner.is_empty() {
             errors.push(FullyQualifiedTypeNameError::EmptyOwnerSegment);
         } else if !validating_segment_regex.is_match(&owner) {
@@ -443,7 +505,8 @@ impl FullyQualifiedTypeName {
             });
         }
 
-        for (index, namespace) in namespaces.into_iter().enumerate() {
+        // Validate each namespace segment, if any are defined.
+        for (index, namespace) in namespaces.clone().into_iter().enumerate() {
             if namespace.is_empty() {
                 errors.push(FullyQualifiedTypeNameError::EmptyNamespaceSegment {
                     // Insert the index as 1-based for more user-friendly error messages
@@ -456,22 +519,7 @@ impl FullyQualifiedTypeName {
             }
         }
 
-        if name.is_empty() {
-            errors.push(FullyQualifiedTypeNameError::MissingNameSegment);
-        } else if !validating_segment_regex.is_match(&name) {
-            errors.push(FullyQualifiedTypeNameError::InvalidNameSegment {
-                segment_text: name.clone()
-            });
-        }
-
-        if errors.is_empty() {
-            Ok(Self(text.to_string()))
-        } else {
-            Err(FullyQualifiedTypeNameError::InvalidTypeName {
-                text: text.to_string(),
-                errors: errors.clone(),
-            })
-        }
+       (owner, namespaces, name)
     }
 
     /// Defines the regular expression for validating a string as a fully qualified type name.
diff --git a/lib/dsc-lib/src/types/wildcard_type_name.rs b/lib/dsc-lib/src/types/wildcard_type_name.rs
@@ -12,7 +12,7 @@ use rust_i18n::t;
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
-use crate::types::FullyQualifiedTypeName;
+use crate::types::{FullyQualifiedTypeName, FullyQualifiedTypeNameError};
 use crate::util::convert_wildcard_to_regex;
 
 /// Defines the wildcard type name for filtering DSC resources or extensions by name.
@@ -224,6 +224,16 @@ pub enum WildcardTypeNameError {
         #[source]
         err: regex::Error,
     },
+    #[error("{t}", t = t!(
+        "types.wildcard_type_name.invalidErrorConversion",
+        "err" => err
+    ))]
+    InvalidErrorConversion {
+        /// The specific error that was encountered when attempting to convert the wildcard type
+        /// name into a regex pattern for matching, but which is not a regex::Error and thus
+        /// can't be included in the `InvalidRegex` error variant.
+        err: FullyQualifiedTypeNameError,
+    }
 }
 
 /// This static lazily defines the validating regex for [`WildcardTypeName`]. It enables the
@@ -409,73 +419,17 @@ impl WildcardTypeName {
             });
         }
 
-        // We need to validate each segment of the type name separately to provide better error
-        // messages about which specific segment(s) contain invalid characters. We also need to
-        // allow for the presence of wildcard characters in any segment, which means we can't rely
-        // on the same regex pattern used for fully qualified type names and instead need a simpler
-        // pattern that just checks for valid characters within each segment.
-        //
-        // We also want to collect all segment errors rather than failing on the first invalid
-        // segment, to provide more comprehensive feedback to the user.
         let errors = &mut Vec::<WildcardTypeNameError>::new();
-        let owner: String;
-        let namespaces: Vec<String>;
-        let require_name: bool;
-        let name: String;
-        let regex: Regex;
-        let validating_segment_regex = Self::init_validating_segment_regex();
-
-        // Split the input into owner/namespaces and name segments
-        if let Some((owner_and_namespaces, name_segment)) = text.rsplit_once('/') {
-            name = name_segment.to_string();
-            let mut segments = owner_and_namespaces
-                .split('.')
-                .map(|s| s.to_string())
-                .collect::<Vec<String>>();
-            owner = segments.remove(0);
-            namespaces = segments;
-        } else if text.contains('.') {
-            let mut segments = text
-                .split('.')
-                .map(|s| s.to_string())
-                .collect::<Vec<String>>();
-            owner = segments.remove(0);
-            namespaces = segments;
-            name = String::new();
-        } else {
-            owner = text.to_string();
-            namespaces = Vec::new();
-            name = String::new();
-        }
-
-        // Validate the owner segment, which _must_ be defined.
-        if owner.is_empty() {
-            errors.push(WildcardTypeNameError::EmptyOwnerSegment);
-        } else if !validating_segment_regex.is_match(&owner) {
-            errors.push(WildcardTypeNameError::InvalidOwnerSegment {
-                segment_text: owner.clone(),
-            });
-        }
-
-        // Validate every defined namespace segment
-        for (index, namespace) in (&namespaces).into_iter().enumerate() {
-            if namespace.is_empty() {
-                errors.push(WildcardTypeNameError::EmptyNamespaceSegment {
-                    // Insert the index as 1-based for more user-friendly error messages
-                    index: index + 1
-                });
-            } else if !validating_segment_regex.is_match(&namespace) {
-                errors.push(WildcardTypeNameError::InvalidNamespaceSegment {
-                    segment_text: namespace.clone(),
-                });
-            }
-        }
+        let (owner, namespaces, name) = Self::parse_segments(
+            text,
+            errors
+        );
 
         // The name segment is required if no wildcard is defined that can match the name
         // segment. For example, `Contoso.*.Example` would require a name segment because it
         // defines a wildcard in a prior namespace segment with a following literal namespace
         // segment.
-        require_name = if namespaces.is_empty() {
+        let require_name = if namespaces.is_empty() {
             !owner.contains('*')
         } else {
             !namespaces.last().unwrap().contains('*')
@@ -485,6 +439,7 @@ impl WildcardTypeName {
         }
 
         // Validate the name segment for invalid characters if it's defined.
+        let validating_segment_regex = Self::init_validating_segment_regex();
         if !name.is_empty() && !validating_segment_regex.is_match(&name) {
             errors.push(WildcardTypeNameError::InvalidNameSegment {
                 segment_text: name.clone(),
@@ -493,7 +448,7 @@ impl WildcardTypeName {
 
         // Construct the regular expression.
         let pattern = convert_wildcard_to_regex(text);
-        regex = match RegexBuilder::new(&pattern).case_insensitive(true).build() {
+        let regex = match RegexBuilder::new(&pattern).case_insensitive(true).build() {
             Ok(r) => r,
             Err(err) => {
                 errors.push(WildcardTypeNameError::InvalidRegex {
@@ -521,6 +476,49 @@ impl WildcardTypeName {
         }
     }
 
+    /// Private helper to parse the input into segments and collect validation errors for owner
+    /// and namespace segments.
+    /// 
+    /// This is used by the public `parse()` method to perform the initial parsing of the input
+    /// string into its constituent segments. It reuses the implementation from
+    /// [`FullyQualifiedTypeName::parse_segments()`], but converts the errors into the appropriate
+    /// [`WildcardTypeNameError`] variants and collects them in the provided `errors` vector for
+    /// bubbling up in the main error handling logic of the `parse()` method.
+    /// 
+    /// # Arguments
+    /// 
+    /// - `text` - The input string to parse into segments.
+    /// - `errors`: A mutable reference to a vector for collecting any validation errors encountered
+    ///   while parsing the segments. This allows the method to accumulate multiple errors for
+    ///   different segments of the type name.
+    /// 
+    /// # Returns
+    /// 
+    /// The method returns the parsed segments (`owner`, `namespaces`, and `name`) as a tuple. Any
+    /// validation errors encountered during parsing are added to the provided `errors` vector for
+    /// the caller to handle.
+    pub(crate) fn parse_segments(text: &str, errors: &mut Vec<WildcardTypeNameError>) -> (String, Vec<String>, String) {
+        // Reuse the segment parsing logic from FullyQualifiedTypeName since the overall structure
+        // is the same, but we need to provide a different validating regex that allows for
+        // wildcard characters and the name segment may be optional depending on the presence of
+        // wildcards in prior segments.
+        let fqtn_errors = &mut Vec::<FullyQualifiedTypeNameError>::new();
+        let validating_segment_regex = Self::init_validating_segment_regex();
+        let parsed = FullyQualifiedTypeName::parse_segments(
+            text,
+            validating_segment_regex,
+            fqtn_errors
+        );
+        // Convert the FQTN errors and add into the WCTN error collection
+        errors.extend(fqtn_errors.into_iter().map(|e| {
+            match TryInto::<WildcardTypeNameError>::try_into(e.clone()) {
+                Ok(wtne) => wtne,
+                Err(e) => e,
+            }
+        }));
+        // Return the parsed segments.
+        parsed
+    }
     /// Returns `true` if the wildcard type name has a length of zero and otherwise `false`.
     pub fn is_empty(&self) -> bool {
         self.text.is_empty()
@@ -714,3 +712,27 @@ impl Hash for WildcardTypeName {
         self.text.to_lowercase().hash(state);
     }
 }
+
+impl TryFrom<FullyQualifiedTypeNameError> for WildcardTypeNameError {
+    type Error = WildcardTypeNameError;
+    fn try_from(value: FullyQualifiedTypeNameError) -> Result<Self, Self::Error> {
+        match value {
+            FullyQualifiedTypeNameError::InvalidOwnerSegment {
+                segment_text
+            } => Ok(WildcardTypeNameError::InvalidOwnerSegment { segment_text }),
+            FullyQualifiedTypeNameError::InvalidNamespaceSegment {
+                segment_text
+            } => Ok(WildcardTypeNameError::InvalidNamespaceSegment { segment_text }),
+            FullyQualifiedTypeNameError::EmptyNamespaceSegment {
+                index
+            } => Ok(WildcardTypeNameError::EmptyNamespaceSegment { index }),
+            FullyQualifiedTypeNameError::InvalidNameSegment {
+                segment_text
+            } => Ok(WildcardTypeNameError::InvalidNameSegment { segment_text }),
+            FullyQualifiedTypeNameError::EmptyOwnerSegment => Ok(WildcardTypeNameError::EmptyOwnerSegment),
+            FullyQualifiedTypeNameError::MissingNameSegment => Ok(WildcardTypeNameError::MissingNameSegment),
+            // No other errors should be converted
+            _ => Err(WildcardTypeNameError::InvalidErrorConversion { err: value })
+        }
+    }
+}
diff --git a/lib/dsc-lib/tests/integration/types/fully_qualified_type_name.rs b/lib/dsc-lib/tests/integration/types/fully_qualified_type_name.rs
@@ -81,6 +81,16 @@ mod methods {
                 ]
             }; "empty namespace segment"
         )]
+        #[test_case("Owner.Empty.Last.Namespace./Name" =>
+            FullyQualifiedTypeNameError::InvalidTypeName{
+                text: "Owner.Empty.Last.Namespace./Name".to_string(),
+                errors: vec![
+                    FullyQualifiedTypeNameError::EmptyNamespaceSegment {
+                        index: 4
+                    }
+                ]
+            }; "empty namespace segment at end of namespaces"
+        )]
         #[test_case("Invalid&Owner/Name" =>
             FullyQualifiedTypeNameError::InvalidTypeName{
                 text: "Invalid&Owner/Name".to_string(),
diff --git a/lib/dsc-lib/tests/integration/types/wildcard_type_name.rs b/lib/dsc-lib/tests/integration/types/wildcard_type_name.rs
@@ -70,6 +70,16 @@ mod methods {
                 ]
             }; "empty namespace segment"
         )]
+        #[test_case("Owner.Empty.Last.Namespace./*" =>
+            WildcardTypeNameError::InvalidTypeName{
+                text: "Owner.Empty.Last.Namespace./*".to_string(),
+                errors: vec![
+                    WildcardTypeNameError::EmptyNamespaceSegment {
+                        index: 4
+                    }
+                ]
+            }; "empty namespace segment at end of namespaces"
+        )]
         #[test_case("Owner.*/Invalid&CharactersInName" =>
             WildcardTypeNameError::InvalidTypeName {
                 text: "Owner.*/Invalid&CharactersInName".to_string(),
