Enable more lints and document exceptions

Cargo.toml

@@ -50,32 +50,71 @@ time = ["dep:time"]
 tokio = ["dep:tokio"]
 
 [lints.rust]
-future_incompatible = "warn"
-let_underscore = "warn"
-nonstandard_style = "warn"
-rust_2018_compatibility = "warn"
-rust_2018_idioms = "warn"
-rust_2021_compatibility = "warn"
+future_incompatible = { level = "warn", priority = -1 }
+keyword_idents = { level = "warn", priority = -1 }
+let_underscore = { level = "warn", priority = -1 }
+missing_debug_implementations = "warn"
+nonstandard_style = { level = "warn", priority = -1 }
+refining_impl_trait = { level = "warn", priority = -1 }
+rust_2018_compatibility = { level = "warn", priority = -1 }
+rust_2018_idioms = { level = "warn", priority = -1 }
+rust_2021_compatibility = { level = "warn", priority = -1 }
+rust_2024_compatibility = { level = "warn", priority = -1 }
 trivial_casts = "warn"
 trivial_numeric_casts = "warn"
+unreachable_pub = "warn"
+# Writing unsafe code is a necessity for FFI wrappers.
+unsafe_code = "allow"
 unsafe_op_in_unsafe_fn = "warn"
-unused = "warn"
+unused = { level = "warn", priority = -1 }
 warnings = "warn"
 
 [lints.clippy]
+# We use absolute paths where we require some item only once. Most often, these
+# share a name with another item in scope and we don't want to import an alias.
+absolute_paths = "allow"
+allow_attributes = "warn"
+allow_attributes_without_reason = "warn"
 as_conversions = "warn"
 as_ptr_cast_mut = "warn"
 as_underscore = "warn"
+cast_possible_truncation = "warn"
 clone_on_ref_ptr = "warn"
+default_trait_access = "warn"
+enum_variant_names = "warn"
+error_impl_error = "warn"
+# We panic in certain less likely situations. In each case, this is documented.
+expect_used = "allow"
+fallible_impl_from = "warn"
+format_push_string = "warn"
+get_unwrap = "warn"
+index_refutable_slice = "warn"
+indexing_slicing = "warn"
+manual_assert = "warn"
+match_on_vec_items = "warn"
+match_wild_err_arm = "warn"
+# TODO: Add assert messages.
+missing_assert_message = "allow"
 missing_const_for_fn = "warn"
+missing_errors_doc = "warn"
 mod_module_files = "warn"
+# We export most types from the top module, allow prefixes to distinguish them.
 module_name_repetitions = "allow"
-pedantic = "warn"
-
-# This warns even when lint group and lint have the same level (`warn`). This is
-# very misleading and results in lots of false positives. See
-# https://github.com/rust-lang/rust-clippy/issues/12270
-lint_groups_priority = "allow"
+# We panic in certain less likely situations. In each case, this is documented.
+panic = "allow"
+panic_in_result_fn = "warn"
+pedantic = { level = "warn", priority = -1 }
+should_panic_without_expect = "warn"
+string_slice = "warn"
+unimplemented = "warn"
+unnecessary_self_imports = "warn"
+# We panic in certain less likely situations. In each case, this is documented.
+unreachable = "allow"
+# We panic in certain less likely situations. In each case, this is documented.
+unwrap_in_result = "allow"
+# TODO: Use `expect()` instead.
+unwrap_used = "allow"
+verbose_file_reads = "warn"
 
 [[example]]
 name = "async_browse"

examples/async_browse.rs

@@ -110,6 +110,7 @@ async fn browse_hierarchy(
 ///
 /// This consumes any continuation points that might be returned from browsing, ensuring that all
 /// references are returned eventually.
+#[allow(clippy::indexing_slicing)] // Allow panicking index for simplicity.
 async fn browse_many_contd(
     client: &AsyncClient,
     node_ids: &[ua::NodeId],

src/async_client.rs

@@ -16,8 +16,8 @@ use open62541_sys::{
 use tokio::{sync::oneshot, task, time::Instant};
 
 use crate::{
-    ua, AsyncSubscription, Attribute, CallbackOnce, DataType, DataValue, Error, Result,
-    ServiceRequest, ServiceResponse,
+    ua, AsyncSubscription, Attribute, BrowseResult, CallbackOnce, DataType, DataValue, Error,
+    Result, ServiceRequest, ServiceResponse,
 };
 
 /// Timeout for `UA_Client_run_iterate()`.
@@ -39,6 +39,7 @@ const RUN_ITERATE_TIMEOUT: Duration = Duration::from_millis(200);
 /// while being dropped. In most cases, this is not the desired behavior.
 ///
 /// See [Client](crate::Client) for more details.
+#[derive(Debug)]
 pub struct AsyncClient {
     client: Arc<ua::Client>,
     background_canceled: Arc<AtomicBool>,
@@ -616,9 +617,6 @@ async fn service_request<R: ServiceRequest>(
         .unwrap_or(Err(Error::internal("callback should send result")))
 }
 
-/// Result type for browsing.
-pub type BrowseResult = Result<(Vec<ua::ReferenceDescription>, Option<ua::ContinuationPoint>)>;
-
 /// Converts [`ua::BrowseResult`] to our public result type.
 fn to_browse_result(result: &ua::BrowseResult, node_id: Option<&ua::NodeId>) -> BrowseResult {
     // Make sure to verify the inner status code inside `BrowseResult`. The service request finishes

src/async_monitored_item.rs

@@ -19,6 +19,7 @@ use tokio::sync::mpsc;
 use crate::{ua, CallbackOnce, CallbackStream, DataType as _, Error, Result};
 
 /// Monitored item (with asynchronous API).
+#[derive(Debug)]
 pub struct AsyncMonitoredItem {
     client: Weak<ua::Client>,
     subscription_id: ua::SubscriptionId,

src/async_subscription.rs

@@ -13,6 +13,7 @@ use open62541_sys::{
 use crate::{ua, AsyncMonitoredItem, CallbackOnce, DataType as _, Error, Result};
 
 /// Subscription (with asynchronous API).
+#[derive(Debug)]
 pub struct AsyncSubscription {
     client: Weak<ua::Client>,
     subscription_id: ua::SubscriptionId,

src/browse_result.rs

@@ -0,0 +1,4 @@
+use crate::{ua, Result};
+
+/// Result type for browsing.
+pub type BrowseResult = Result<(Vec<ua::ReferenceDescription>, Option<ua::ContinuationPoint>)>;

src/callback.rs

@@ -38,6 +38,7 @@ use crate::Userdata;
 /// // Value has been received.
 /// assert_eq!(cell.get(), 123);
 /// ```
+#[derive(Debug)]
 pub struct CallbackOnce<T>(PhantomData<T>);
 
 // TODO: Use inherent associated type to define this directly on `CallbackOnce`. At the moment, this
@@ -112,6 +113,7 @@ impl<T> CallbackOnce<T> {
 /// assert_eq!(block_on(rx.recv()), Some(3));
 /// assert_eq!(block_on(rx.recv()), None);
 /// ```
+#[derive(Debug)]
 pub struct CallbackStream<T>(PhantomData<T>);
 
 // TODO: Use inherent associated type to define this directly on `CallbackOnce`. At the moment, this

src/client.rs

@@ -24,7 +24,7 @@ use crate::{ua, DataType as _, Error, Result};
 /// # Ok(())
 /// # }
 /// ```
-#[derive(Default)]
+#[derive(Debug, Default)]
 pub struct ClientBuilder(ua::ClientConfig);
 
 impl ClientBuilder {
@@ -150,6 +150,7 @@ impl ClientBuilder {
 ///
 /// To disconnect, prefer method [`disconnect()`](Self::disconnect) over simply dropping the client:
 /// disconnection involves server communication and might take a short amount of time.
+#[derive(Debug)]
 pub struct Client(
     #[allow(dead_code)] // --no-default-features
     ua::Client,

src/error.rs

@@ -12,6 +12,7 @@ pub type Result<T> = std::result::Result<T, Error>;
 ///
 /// [`is_good()`]: crate::ua::StatusCode::is_good
 #[derive(Debug, Error)]
+#[allow(clippy::error_impl_error)] // The main error type of our crate may be named `Error`.
 pub enum Error {
     /// Error from server.
     #[error("{0}")]

src/lib.rs

@@ -222,6 +222,7 @@ mod async_monitored_item;
 #[cfg(feature = "tokio")]
 mod async_subscription;
 mod attributes;
+mod browse_result;
 #[cfg(feature = "tokio")]
 mod callback;
 mod data_value;
@@ -231,6 +232,7 @@ mod userdata;
 mod value;
 
 pub use self::{
+    browse_result::BrowseResult,
     client::{Client, ClientBuilder},
     data_type::DataType,
     data_value::DataValue,

src/server.rs

@@ -20,7 +20,7 @@ use open62541_sys::{
     __UA_Server_addNode, __UA_Server_write, UA_STATUSCODE_BADNOTFOUND,
 };
 
-use crate::{ua, Attribute, Attributes, DataType, DataValue, Error, Result};
+use crate::{ua, Attribute, Attributes, BrowseResult, DataType, DataValue, Error, Result};
 
 pub(crate) use self::node_context::NodeContext;
 pub use self::{
@@ -53,7 +53,7 @@ pub use self::{
 /// # Ok(())
 /// # }
 /// ```
-#[derive(Default)]
+#[derive(Debug, Default)]
 pub struct ServerBuilder(ua::ServerConfig);
 
 impl ServerBuilder {
@@ -145,7 +145,7 @@ impl ServerBuilder {
 ///
 /// Note: The server must be started with [`ServerRunner::run()`] before it can accept connections
 /// from clients.
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct Server(Arc<ua::Server>);
 
 impl Server {
@@ -1296,6 +1296,7 @@ impl Server {
     }
 }
 
+#[derive(Debug)]
 pub struct ServerRunner(Arc<ua::Server>);
 
 impl ServerRunner {
@@ -1321,9 +1322,6 @@ impl ServerRunner {
     }
 }
 
-/// Result type for browsing.
-pub type BrowseResult = Result<(Vec<ua::ReferenceDescription>, Option<ua::ContinuationPoint>)>;
-
 /// Converts [`ua::BrowseResult`] to our public result type.
 fn to_browse_result(result: &ua::BrowseResult) -> BrowseResult {
     // Make sure to verify the inner status code inside `BrowseResult`. The service request finishes

src/server/data_source.rs

@@ -88,6 +88,7 @@ pub trait DataSource {
 }
 
 /// Context when [`DataSource`] is being read from.
+#[derive(Debug)]
 pub struct DataSourceReadContext {
     /// Outgoing value to be read.
     ///
@@ -131,6 +132,7 @@ impl DataSourceReadContext {
 }
 
 /// Context when [`DataSource`] is being written to.
+#[derive(Debug)]
 pub struct DataSourceWriteContext {
     /// Incoming value to be written.
     ///

src/server/method_callback.rs

@@ -68,6 +68,7 @@ pub trait MethodCallback {
 }
 
 /// Context when [`MethodCallback`] is being called.
+#[derive(Debug)]
 pub struct MethodCallbackContext {
     object_id: NonNull<UA_NodeId>,
     input_size: usize,

src/server/node_types.rs

@@ -1,3 +1,5 @@
+use std::fmt;
+
 use crate::{ua, Attributes, DataType};
 
 use crate::server::NodeContext;
@@ -87,6 +89,29 @@ impl<T: Attributes> Node<T> {
     }
 }
 
+impl<T: Attributes> fmt::Debug for Node<T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let Self {
+            requested_new_node_id,
+            parent_node_id,
+            reference_type_id,
+            browse_name,
+            type_definition,
+            attributes,
+            context: _,
+        } = self;
+
+        f.debug_struct("Node")
+            .field("requested_new_node_id", requested_new_node_id)
+            .field("parent_node_id", parent_node_id)
+            .field("reference_type_id", reference_type_id)
+            .field("browse_name", browse_name)
+            .field("type_definition", type_definition)
+            .field("attributes", attributes)
+            .finish_non_exhaustive()
+    }
+}
+
 #[derive(Debug, Clone)]
 pub struct ObjectNode {
     pub requested_new_node_id: Option<ua::NodeId>,
@@ -107,6 +132,7 @@ pub struct VariableNode {
     pub attributes: ua::VariableAttributes,
 }
 
+#[derive(Debug, Clone)]
 pub struct MethodNode {
     pub requested_new_node_id: Option<ua::NodeId>,
     pub parent_node_id: ua::NodeId,

src/ua/array.rs

@@ -430,12 +430,12 @@ impl<T: DataType> Drop for Array<T> {
     }
 }
 
-// TODO
-// impl<T: DataType> Clone for Array<T> {
-//     fn clone(&self) -> Self {
-//         todo!()
-//     }
-// }
+impl<T: DataType> Clone for Array<T> {
+    fn clone(&self) -> Self {
+        // TODO: Use more efficient implementation that uses `UA_Array_copy()` directly.
+        Self::from_slice(self.as_slice())
+    }
+}
 
 impl<T: DataType> fmt::Debug for Array<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -446,12 +446,14 @@ impl<T: DataType> fmt::Debug for Array<T> {
 impl<T: DataType> ops::Index<usize> for Array<T> {
     type Output = T;
 
+    #[allow(clippy::indexing_slicing)] // We forward the underlying panic.
     fn index(&self, index: usize) -> &Self::Output {
         &self.as_slice()[index]
     }
 }
 
 impl<T: DataType> ops::IndexMut<usize> for Array<T> {
+    #[allow(clippy::indexing_slicing)] // We forward the underlying panic.
     fn index_mut(&mut self, index: usize) -> &mut Self::Output {
         &mut self.as_slice_mut()[index]
     }

src/ua/browse_result_mask.rs

@@ -1,6 +1,7 @@
 // We do not expose the inner enum. We want to use a proper `u32` for bit operations on the mask and
 // we want to be clear about what is an initial (const, enum-like) value and what is a derived mask;
 // specifically, the bitmask type is _not_ an enum even though declared so in `open62541-sys`.
+#[allow(unreachable_pub)]
 mod inner {
     crate::data_type!(BrowseResultMask);
 

src/ua/client.rs

@@ -10,6 +10,7 @@ use crate::{ua, DataType as _, Error};
 /// Combined state for [`Client`] and [`AsyncClient`].
 ///
 /// [`AsyncClient`]: crate::AsyncClient
+#[derive(Debug)]
 pub struct ClientState {
     pub channel_state: ua::SecureChannelState,
     pub session_state: ua::SessionState,
@@ -23,6 +24,7 @@ pub struct ClientState {
 ///
 /// This owns the wrapped data type. When the wrapper is dropped, its inner value is cleaned up with
 /// [`UA_Client_delete()`].
+#[derive(Debug)]
 pub struct Client(NonNull<UA_Client>);
 
 // SAFETY: We know that the underlying `UA_Client` allows access from different threads, i.e. it may

src/ua/client_config.rs

@@ -1,4 +1,4 @@
-use std::mem::MaybeUninit;
+use std::{fmt, mem::MaybeUninit};
 
 use open62541_sys::{UA_ClientConfig, UA_ClientConfig_clear, UA_ClientConfig_setDefault};
 
@@ -79,6 +79,12 @@ impl Drop for ClientConfig {
     }
 }
 
+impl fmt::Debug for ClientConfig {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("ClientConfig").finish_non_exhaustive()
+    }
+}
+
 impl Default for ClientConfig {
     fn default() -> Self {
         let mut config = Self::init();

src/ua/secure_channel_state.rs

@@ -1,6 +1,7 @@
 use open62541_sys::UA_SecureChannelState;
 
 /// Wrapper for [`UA_SecureChannelState`] from [`open62541_sys`].
+#[derive(Debug)]
 pub struct SecureChannelState(UA_SecureChannelState);
 
 impl SecureChannelState {