Make treatment of string instantiation more explicit

CHANGELOG.md

@@ -24,6 +24,7 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - Breaking: Move `ua::VariantValue` and `ua::ScalarValue` to top-level export outside `ua`
 - Breaking: Remove `ua::ArrayValue` for now (until we have a better interface).
 - Breaking: Return output arguments directly from `AsyncClient::call_method()`, without `Option`.
+- Breaking: Remove misleading `FromStr` trait implementation and offer `ua::String::new()` instead.
 
 ### Fixed
 

src/ua/data_types/application_description.rs

@@ -1,5 +1,3 @@
-use std::str::FromStr;
-
 use crate::{ua, DataType as _};
 
 crate::data_type!(ApplicationDescription);
@@ -15,7 +13,7 @@ impl ApplicationDescription {
     /// The string must not contain any NUL bytes.
     #[must_use]
     pub fn with_application_uri(mut self, application_uri: &str) -> Self {
-        ua::String::from_str(application_uri)
+        ua::String::new(application_uri)
             .unwrap()
             .move_into_raw(&mut self.0.applicationUri);
         self
@@ -28,7 +26,7 @@ impl ApplicationDescription {
     /// The string must not contain any NUL bytes.
     #[must_use]
     pub fn with_product_uri(mut self, product_uri: &str) -> Self {
-        ua::String::from_str(product_uri)
+        ua::String::new(product_uri)
             .unwrap()
             .move_into_raw(&mut self.0.productUri);
         self
@@ -41,7 +39,7 @@ impl ApplicationDescription {
     /// The strings must not contain any NUL bytes.
     #[must_use]
     pub fn with_application_name(mut self, locale: &str, application_name: &str) -> Self {
-        ua::LocalizedText::try_from((locale, application_name))
+        ua::LocalizedText::new(locale, application_name)
             .unwrap()
             .move_into_raw(&mut self.0.applicationName);
         self
@@ -62,7 +60,7 @@ impl ApplicationDescription {
     /// The string must not contain any NUL bytes.
     #[must_use]
     pub fn with_gateway_server_uri(mut self, gateway_server_uri: &str) -> Self {
-        ua::String::from_str(gateway_server_uri)
+        ua::String::new(gateway_server_uri)
             .unwrap()
             .move_into_raw(&mut self.0.gatewayServerUri);
         self
@@ -75,7 +73,7 @@ impl ApplicationDescription {
     /// The string must not contain any NUL bytes.
     #[must_use]
     pub fn with_discovery_profile_uri(mut self, discovery_profile_uri: &str) -> Self {
-        ua::String::from_str(discovery_profile_uri)
+        ua::String::new(discovery_profile_uri)
             .unwrap()
             .move_into_raw(&mut self.0.discoveryProfileUri);
         self
@@ -90,7 +88,7 @@ impl ApplicationDescription {
     pub fn with_discovery_urls(mut self, discovery_urls: &[&str]) -> Self {
         let discovery_urls = discovery_urls
             .iter()
-            .map(|discovery_url| ua::String::from_str(discovery_url).unwrap());
+            .map(|discovery_url| ua::String::new(discovery_url).unwrap());
         ua::Array::from_iter(discovery_urls)
             .move_into_raw(&mut self.0.discoveryUrlsSize, &mut self.0.discoveryUrls);
         self

src/ua/data_types/localized_text.rs

@@ -1,23 +1,30 @@
-use std::str::FromStr;
-
 use crate::{ua, DataType as _, Error};
 
 crate::data_type!(LocalizedText);
 
 impl LocalizedText {
+    /// Creates localized text from locale and text.
+    ///
+    /// # Errors
+    ///
+    /// The strings must not contain any NUL bytes.
+    pub fn new(locale: &str, text: &str) -> Result<Self, Error> {
+        Self::init().with_locale(locale)?.with_text(text)
+    }
+
     /// # Errors
     ///
     /// The string must not contain any NUL bytes.
     pub fn with_locale(mut self, locale: &str) -> Result<Self, Error> {
-        ua::String::from_str(locale)?.move_into_raw(&mut self.0.locale);
+        ua::String::new(locale)?.move_into_raw(&mut self.0.locale);
         Ok(self)
     }
 
     /// # Errors
     ///
     /// The string must not contain any NUL bytes.
     pub fn with_text(mut self, text: &str) -> Result<Self, Error> {
-        ua::String::from_str(text)?.move_into_raw(&mut self.0.text);
+        ua::String::new(text)?.move_into_raw(&mut self.0.text);
         Ok(self)
     }
 
@@ -31,11 +38,3 @@ impl LocalizedText {
         ua::String::raw_ref(&self.0.text)
     }
 }
-
-impl TryFrom<(&str, &str)> for LocalizedText {
-    type Error = Error;
-
-    fn try_from((locale, text): (&str, &str)) -> Result<Self, Self::Error> {
-        Self::init().with_locale(locale)?.with_text(text)
-    }
-}

src/ua/data_types/node_id.rs

@@ -123,7 +123,7 @@ impl str::FromStr for NodeId {
         let mut node_id = NodeId::init();
 
         let status_code = ua::StatusCode::new({
-            let str: ua::String = s.parse()?;
+            let str = ua::String::new(s)?;
             // SAFETY: `UA_NodeId_parse()` expects the string passed by value but does not take
             // ownership.
             let str = unsafe { ua::String::to_raw_copy(&str) };
@@ -216,7 +216,8 @@ mod tests {
 
     #[test]
     fn string_representation() {
-        // We explicitly derive `FromStr` and `ToString`. This is part of the public interface.
+        // We explicitly derive `FromStr` and `ToString`. This is part of the public interface. This
+        // is the reason why the explicit turbofish syntax is used below.
         //
         let node_id =
             <ua::NodeId as str::FromStr>::from_str("ns=0;i=2258").expect("should be valid node ID");

src/ua/data_types/string.rs

@@ -1,4 +1,4 @@
-use std::{ffi::CString, fmt, slice, str};
+use std::{ffi::CString, fmt, ptr, slice, str};
 
 use open62541_sys::UA_String_fromChars;
 
@@ -10,6 +10,34 @@ crate::data_type!(String);
 // strings of `length` 0. It may also be `ptr::null()` for "invalid" strings. This is similar to how
 // OPC UA treats arrays (which also distinguishes between empty and invalid instances).
 impl String {
+    /// Creates string from string slice.
+    ///
+    /// # Errors
+    ///
+    /// The string must not contain any NUL bytes.
+    pub fn new(s: &str) -> Result<Self, Error> {
+        // We do not know for sure if `open62541` handles strings with contained NUL bytes correctly
+        // in all situations. We avoid this entirely (at least for now). We may revisit this later.
+        let src =
+            CString::new(s).map_err(|_| Error::internal("string should not contain NUL bytes"))?;
+        let str = unsafe { UA_String_fromChars(src.as_ptr()) };
+        Ok(Self(str))
+    }
+
+    /// Creates invalid string (as defined by OPC UA).
+    // TODO: The OPC UA specification calls invalid strings "null". Consider changing this to match.
+    #[allow(dead_code)] // This is unused for now.
+    pub(crate) fn invalid() -> Self {
+        let str = unsafe { UA_String_fromChars(ptr::null()) };
+        Self(str)
+    }
+
+    /// Creates empty string.
+    #[allow(dead_code)] // This is unused for now.
+    pub(crate) fn empty() -> Self {
+        Self::new("").unwrap()
+    }
+
     /// Checks if string is invalid.
     ///
     /// The invalid state is defined by OPC UA. It is a third state which is distinct from empty and
@@ -83,32 +111,6 @@ impl fmt::Display for String {
     }
 }
 
-impl str::FromStr for String {
-    type Err = Error;
-
-    /// Creates [`String`] from string slice.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// use open62541::ua;
-    ///
-    /// let node_id: ua::String = "Lorem Ipsum".parse().unwrap();
-    /// ```
-    ///
-    /// # Errors
-    ///
-    /// The string slice must not contain any NUL bytes.
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        // We do not know for sure if `open62541` handles strings with contained NUL bytes correctly
-        // in all situations. We avoid this entirely (at least for now). We may revisit this later.
-        let src =
-            CString::new(s).map_err(|_| Error::internal("string should not contain NUL bytes"))?;
-        let str = unsafe { UA_String_fromChars(src.as_ptr()) };
-        Ok(Self(str))
-    }
-}
-
 #[cfg(feature = "serde")]
 impl serde::Serialize for String {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
@@ -127,7 +129,7 @@ mod tests {
 
     #[test]
     fn valid_string() {
-        let str: ua::String = "lorem ipsum".parse().expect("should parse string");
+        let str = ua::String::new("lorem ipsum").expect("should parse string");
         assert_eq!(str.as_str().expect("should display string"), "lorem ipsum");
         assert_eq!(str.to_string(), "lorem ipsum");
     }
@@ -136,7 +138,7 @@ mod tests {
     fn empty_string() {
         // Empty strings may have an internal representation in `UA_String` that contains invalid or
         // null pointers. This must not cause any problems.
-        let str: ua::String = "".parse().expect("should parse empty string");
+        let str = ua::String::new("").expect("should parse empty string");
         assert_eq!(str.as_str().expect("should display empty string"), "");
         assert_eq!(str.to_string(), "");
     }

src/ua/data_types/variant.rs

@@ -244,8 +244,6 @@ mod tests {
 
     #[cfg(feature = "serde")]
     mod serde {
-        use std::str::FromStr as _;
-
         use crate::{ua, DataType as _};
 
         #[test]
@@ -308,19 +306,19 @@ mod tests {
         #[test]
         fn serialize_string() {
             // Empty string
-            let ua_string = ua::String::from_str("").unwrap();
+            let ua_string = ua::String::new("").unwrap();
             let ua_variant = ua::Variant::init().with_scalar(&ua_string);
             let json = serde_json::to_string(&ua_variant).unwrap();
             assert_eq!(r#""""#, json);
 
             // Short string
-            let ua_string = ua::String::from_str("lorem ipsum").unwrap();
+            let ua_string = ua::String::new("lorem ipsum").unwrap();
             let ua_variant = ua::Variant::init().with_scalar(&ua_string);
             let json = serde_json::to_string(&ua_variant).unwrap();
             assert_eq!(r#""lorem ipsum""#, json);
 
             // Special characters
-            let ua_string = ua::String::from_str(r#"a'b"c{dẞe"#).unwrap();
+            let ua_string = ua::String::new(r#"a'b"c{dẞe"#).unwrap();
             let ua_variant = ua::Variant::init().with_scalar(&ua_string);
             let json = serde_json::to_string(&ua_variant).unwrap();
             assert_eq!(r#""a'b\"c{dẞe""#, json);