From 9070a5806801c382cae719fdba216900f7589d99 Mon Sep 17 00:00:00 2001
From: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Date: Sun, 24 Mar 2024 18:25:37 +0100
Subject: [PATCH] fixup! plugin: deny unwrap call

---
 plugin/src/commands/builtin.rs | 11 ++++++++++-
 plugin/src/commands/mod.rs     |  2 +-
 plugin/src/io.rs               |  1 +
 plugin/src/plugin.rs           | 15 +++++++++++++++
 plugin_macros/src/plugin.rs    |  5 +----
 tests/src/lib.rs               |  4 ++--
 6 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/plugin/src/commands/builtin.rs b/plugin/src/commands/builtin.rs
index c1eebe9..7dd2366 100644
--- a/plugin/src/commands/builtin.rs
+++ b/plugin/src/commands/builtin.rs
@@ -51,14 +51,21 @@ impl<T: Clone> RPCCommand<T> for ManifestRPC {
 #[derive(Clone)]
 /// Type to define the init method and its attributes, used in plugin
 pub struct InitRPC<T: 'static + Clone> {
+    #[allow(clippy::type_complexity)]
     pub(crate) on_init: Option<Arc<dyn Fn(&mut Plugin<T>) -> Value>>,
 }
 
 impl<T: Clone> InitRPC<T> {
     fn parse_option(&self, plugin: &mut Plugin<T>, options: &HashMap<String, serde_json::Value>) {
         for option_name in options.keys() {
+            // SAFETY: We are iterating over the key this never None
+            #[allow(clippy::unwrap_used)]
             let option = options.get(option_name).unwrap();
-            plugin.option.get_mut(option_name).unwrap().value = Some(option.to_owned());
+            // SAFETY: we put them into it so it is safe to unwrap.
+            // If we panic this mean that there is a bug
+            #[allow(clippy::unwrap_used)]
+            let opt = plugin.option.get_mut(option_name).unwrap();
+            opt.value = Some(option.to_owned());
         }
     }
 }
@@ -66,6 +73,8 @@ impl<T: Clone> InitRPC<T> {
 impl<T: Clone> RPCCommand<T> for InitRPC<T> {
     fn call<'c>(&self, plugin: &mut Plugin<T>, request: Value) -> Result<Value, PluginError> {
         let mut response = init_payload();
+        // SAFETY: Shouwl be valid json so should be safe to unwrap
+        #[allow(clippy::unwrap_used)]
         let init: InitConf = serde_json::from_value(request.to_owned()).unwrap();
         plugin.configuration = Some(init.configuration);
         self.parse_option(plugin, &init.options);
diff --git a/plugin/src/commands/mod.rs b/plugin/src/commands/mod.rs
index d85123f..4caee2a 100644
--- a/plugin/src/commands/mod.rs
+++ b/plugin/src/commands/mod.rs
@@ -15,7 +15,7 @@ use crate::errors::PluginError;
 /// in contrast, it is more complex but the plugin_macros package will help to simplify the API.
 pub trait RPCCommand<T: Clone>: RPCCommandClone<T> {
     /// call is a generic method that it is used to simulate the callback.
-    fn call<'c>(
+    fn call(
         &self,
         _: &mut Plugin<T>,
         _: serde_json::Value,
diff --git a/plugin/src/io.rs b/plugin/src/io.rs
index 9a2bcfc..74f57df 100644
--- a/plugin/src/io.rs
+++ b/plugin/src/io.rs
@@ -30,6 +30,7 @@ impl AsyncIO {
         Ok(())
     }
 
+    #[allow(clippy::wrong_self_convention)]
     pub fn into_loop<F>(&mut self, mut async_callback: F) -> io::Result<()>
     where
         F: FnMut(String) -> Option<String>,
diff --git a/plugin/src/plugin.rs b/plugin/src/plugin.rs
index 95be857..ab6522f 100644
--- a/plugin/src/plugin.rs
+++ b/plugin/src/plugin.rs
@@ -126,6 +126,8 @@ impl<'a, T: 'a + Clone> Plugin<T> {
             params: payload,
         };
         crate::poll_loop!({
+            // SAFETY: it is valid json and if we panic there is a buf somewhere
+            #[allow(clippy::unwrap_used)]
             writer.write_all(serde_json::to_string(&request).unwrap().as_bytes())
         });
         crate::poll_loop!({ writer.flush() });
@@ -142,8 +144,12 @@ impl<'a, T: 'a + Clone> Plugin<T> {
     ) -> &mut Self {
         let def_val = match opt_type {
             "flag" | "bool" => {
+                // FIXME: remove unwrap and return the error
+                #[allow(clippy::unwrap_used)]
                 def_val.map(|val| serde_json::json!(val.parse::<bool>().unwrap()))
             }
+            // FIXME: remove unwrap and return the error
+            #[allow(clippy::unwrap_used)]
             "int" => def_val.map(|val| serde_json::json!(val.parse::<i64>().unwrap())),
             "string" => def_val.map(|val| serde_json::json!(val)),
             _ => unreachable!("{opt_type} not supported"),
@@ -212,6 +218,9 @@ impl<'a, T: 'a + Clone> Plugin<T> {
     }
 
     fn handle_notification(&'a mut self, name: &str, params: serde_json::Value) {
+        // SAFETY: we register the notification and if we do not have inside the map
+        // this is a bug.
+        #[allow(clippy::unwrap_used)]
         let notification = self.rpc_notification.get(name).unwrap().clone();
         notification.call_void(self, &params);
     }
@@ -253,6 +262,8 @@ impl<'a, T: 'a + Clone> Plugin<T> {
         match result {
             Ok(json_resp) => response["result"] = json_resp.to_owned(),
             Err(json_err) => {
+                // SAFETY: should be valud json
+                #[allow(clippy::unwrap_used)]
                 let err_resp = serde_json::to_value(json_err).unwrap();
                 response["error"] = err_resp;
             }
@@ -281,6 +292,8 @@ impl<'a, T: 'a + Clone> Plugin<T> {
         asyncio.into_loop(|buffer| {
             #[cfg(feature = "log")]
             log::info!("looping around the string: {buffer}");
+            // SAFETY: should be valud json
+            #[allow(clippy::unwrap_used)]
             let request: Request<serde_json::Value> = serde_json::from_str(&buffer).unwrap();
             if let Some(id) = request.id {
                 // when the id is specified this is a RPC or Hook, so we need to return a response
@@ -293,6 +306,8 @@ impl<'a, T: 'a + Clone> Plugin<T> {
                     request.method,
                     rpc_response
                 );
+                // SAFETY: should be valud json
+                #[allow(clippy::unwrap_used)]
                 Some(serde_json::to_string(&rpc_response).unwrap())
             } else {
                 // in case of the id is None, we are receiving the notification, so the server is not
diff --git a/plugin_macros/src/plugin.rs b/plugin_macros/src/plugin.rs
index 7bf1ef8..f8d89d1 100644
--- a/plugin_macros/src/plugin.rs
+++ b/plugin_macros/src/plugin.rs
@@ -5,8 +5,7 @@ use kproc_parser::kproc_macros::KTokenStream;
 use kproc_parser::proc_macro::{TokenStream, TokenTree};
 use kproc_parser::{build_error, check, trace};
 
-#[derive(Debug)]
-#[derive(Default)]
+#[derive(Debug, Default)]
 pub struct PluginDeclaration {
     pub state: Option<String>,
     pub dynamic: Option<TokenTree>,
@@ -59,8 +58,6 @@ impl std::fmt::Display for PluginDeclaration {
     }
 }
 
-
-
 /// proc macro syntax is something like this
 ///
 /// ```ignore
diff --git a/tests/src/lib.rs b/tests/src/lib.rs
index d3a3e8d..b0a2fa3 100644
--- a/tests/src/lib.rs
+++ b/tests/src/lib.rs
@@ -36,14 +36,14 @@ pub mod fixtures {
     pub fn lightningd() -> cln::Node {
         init();
         let pwd = std::env::var("PWD").unwrap();
-        
+
         async_run!(cln::Node::with_params(&format!("--developer --plugin={pwd}/target/debug/examples/foo_plugin --plugin={pwd}/target/debug/examples/macros_ex"), "regtest")).unwrap()
     }
 
     #[fixture]
     pub fn lightningd_second() -> cln::Node {
         init();
-        
+
         async_run!(cln::Node::with_params("--developer", "regtest")).unwrap()
     }
 }