Rust API: Additional Cleanup

16 files changed, 99 insertions, 119 deletions

diff --git a/rust/binaryninjacore-sys/src/lib.rs b/rust/binaryninjacore-sys/src/lib.rs
index effc9340..d39a7c00 100644
--- a/rust/binaryninjacore-sys/src/lib.rs
+++ b/rust/binaryninjacore-sys/src/lib.rs
@@ -2,7 +2,6 @@
 #![allow(non_camel_case_types)]
 #![allow(non_snake_case)]
 #![allow(unused)]
-
 #![allow(clippy::type_complexity)]
 
 include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
diff --git a/rust/src/callingconvention.rs b/rust/src/callingconvention.rs
index 99b6c654..52d1407c 100644
--- a/rust/src/callingconvention.rs
+++ b/rust/src/callingconvention.rs
@@ -449,7 +449,7 @@ impl<A: Architecture> CallingConvention<A> {
     pub fn variables_for_parameters<S: Clone + BnStrCompatible>(
         &self,
         params: &[FunctionParameter<S>],
-        int_arg_registers: Option<Vec<A::Register>>,
+        int_arg_registers: Option<&[A::Register]>,
     ) -> Vec<Variable> {
         let mut bn_params: Vec<BNFunctionParameter> = vec![];
         let mut name_strings = vec![];
diff --git a/rust/src/custombinaryview.rs b/rust/src/custombinaryview.rs
index aa4e9e2f..90673fb7 100644
--- a/rust/src/custombinaryview.rs
+++ b/rust/src/custombinaryview.rs
@@ -441,10 +441,10 @@ impl<'a, T: CustomBinaryViewType> CustomViewBuilder<'a, T> {
 
                 if context.initialized {
                     mem::forget(context.args); // already consumed
-                    // mem::drop(context.view); // cb_init was called
+                                               // mem::drop(context.view); // cb_init was called
                 } else {
                     mem::drop(context.args); // never consumed
-                    // mem::forget(context.view); // cb_init was not called, is uninit
+                                             // mem::forget(context.view); // cb_init was not called, is uninit
 
                     if context.raw_handle.is_null() {
                         // being called here is essentially a guarantee that BNCreateBinaryViewOfType
diff --git a/rust/src/debuginfo.rs b/rust/src/debuginfo.rs
index 37335255..3ec0691c 100644
--- a/rust/src/debuginfo.rs
+++ b/rust/src/debuginfo.rs
@@ -575,15 +575,16 @@ impl DebugInfo {
         let names_and_types: &[*mut BNNameAndType] =
             unsafe { slice::from_raw_parts(raw_names_and_types as *mut _, count) };
 
-        let mut result = Vec::with_capacity(count);
-        for name_and_type in names_and_types.iter().take(count) {
-            unsafe {
-                result.push((
-                    raw_to_string((**name_and_type).name).unwrap(),
-                    Type::ref_from_raw(BNNewTypeReference((**name_and_type).type_)),
-                ))
-            };
-        }
+        let result = names_and_types
+            .iter()
+            .take(count)
+            .map(|&name_and_type| unsafe {
+                (
+                    raw_to_string((*name_and_type).name).unwrap(),
+                    Type::ref_from_raw(BNNewTypeReference((*name_and_type).type_)),
+                )
+            })
+            .collect();
 
         unsafe { BNFreeNameAndTypeList(raw_names_and_types, count) };
         result
@@ -604,16 +605,17 @@ impl DebugInfo {
         let variables_and_names: &[*mut BNDataVariableAndName] =
             unsafe { slice::from_raw_parts(raw_variables_and_names as *mut _, count) };
 
-        let mut result = Vec::with_capacity(count);
-        for variable_and_name in variables_and_names.iter().take(count) {
-            unsafe {
-                result.push((
-                    raw_to_string((**variable_and_name).name).unwrap(),
-                    (**variable_and_name).address,
-                    Type::ref_from_raw(BNNewTypeReference((**variable_and_name).type_)),
-                ))
-            };
-        }
+        let result = variables_and_names
+            .iter()
+            .take(count)
+            .map(|&variable_and_name| unsafe {
+                (
+                    raw_to_string((*variable_and_name).name).unwrap(),
+                    (*variable_and_name).address,
+                    Type::ref_from_raw(BNNewTypeReference((*variable_and_name).type_)),
+                )
+            })
+            .collect();
 
         unsafe { BNFreeDataVariablesAndName(raw_variables_and_names, count) };
         result
@@ -628,16 +630,17 @@ impl DebugInfo {
         let variables_and_names: &[*mut BNDataVariableAndNameAndDebugParser] =
             unsafe { slice::from_raw_parts(raw_variables_and_names as *mut _, count) };
 
-        let mut result = Vec::with_capacity(count);
-        for variable_and_name in variables_and_names.iter().take(count) {
-            unsafe {
-                result.push((
-                    raw_to_string((**variable_and_name).parser).unwrap(),
-                    raw_to_string((**variable_and_name).name).unwrap(),
-                    Type::ref_from_raw(BNNewTypeReference((**variable_and_name).type_)),
-                ))
-            };
-        }
+        let result = variables_and_names
+            .iter()
+            .take(count)
+            .map(|&variable_and_name| unsafe {
+                (
+                    raw_to_string((*variable_and_name).parser).unwrap(),
+                    raw_to_string((*variable_and_name).name).unwrap(),
+                    Type::ref_from_raw(BNNewTypeReference((*variable_and_name).type_)),
+                )
+            })
+            .collect();
 
         unsafe { BNFreeDataVariableAndNameAndDebugParserList(raw_variables_and_names, count) };
         result
diff --git a/rust/src/demangle.rs b/rust/src/demangle.rs
index e9948cb6..0b01a231 100644
--- a/rust/src/demangle.rs
+++ b/rust/src/demangle.rs
@@ -15,8 +15,8 @@
 //! Interfaces for demangling and simplifying mangled names in binaries.
 
 use binaryninjacore_sys::*;
-use std::{ffi::CStr, result};
 use std::os::raw::c_char;
+use std::{ffi::CStr, result};
 
 use crate::architecture::CoreArchitecture;
 use crate::string::{BnStrCompatible, BnString};
diff --git a/rust/src/disassembly.rs b/rust/src/disassembly.rs
index ff33e3dd..950335a3 100644
--- a/rust/src/disassembly.rs
+++ b/rust/src/disassembly.rs
@@ -289,7 +289,7 @@ impl DisassemblyTextLine {
 impl std::fmt::Display for DisassemblyTextLine {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         for token in self.tokens() {
-            write!(f, "{}", token.text())?
+            write!(f, "{}", token.text())?;
         }
 
         Ok(())
diff --git a/rust/src/downloadprovider.rs b/rust/src/downloadprovider.rs
index dfc8b5e2..7de8704c 100644
--- a/rust/src/downloadprovider.rs
+++ b/rust/src/downloadprovider.rs
@@ -38,9 +38,8 @@ impl DownloadProvider {
         Ok(unsafe { Array::new(list, count, ()) })
     }
 
-    /// TODO : Clippy isn't happy...says we should `impl Default`....excessive error checking might be preventing us from doing so
-    #[allow(clippy::should_implement_trait)]
-    pub fn default() -> Result<DownloadProvider, ()> {
+    /// TODO : We may want to `impl Default`....excessive error checking might be preventing us from doing so
+    pub fn try_default() -> Result<DownloadProvider, ()> {
         let s = Settings::new("");
         let dp_name = s.get_string("network.downloadProviderName", None, None);
         Self::get(dp_name).ok_or(())
diff --git a/rust/src/flowgraph.rs b/rust/src/flowgraph.rs
index 92b098cd..a0e6caaa 100644
--- a/rust/src/flowgraph.rs
+++ b/rust/src/flowgraph.rs
@@ -89,14 +89,7 @@ impl<'a> FlowGraphNode<'a> {
         target: &'a FlowGraphNode,
         edge_style: &'a EdgeStyle,
     ) {
-        unsafe {
-            BNAddFlowGraphNodeOutgoingEdge(
-                self.handle,
-                type_,
-                target.handle,
-                edge_style.0,
-            )
-        }
+        unsafe { BNAddFlowGraphNodeOutgoingEdge(self.handle, type_, target.handle, edge_style.0) }
     }
 }
 
diff --git a/rust/src/function.rs b/rust/src/function.rs
index f56f0d64..aaefe5cd 100644
--- a/rust/src/function.rs
+++ b/rust/src/function.rs
@@ -35,10 +35,7 @@ pub struct Location {
 
 impl From<u64> for Location {
     fn from(addr: u64) -> Self {
-        Location {
-            arch: None,
-            addr,
-        }
+        Location { arch: None, addr }
     }
 }
 
diff --git a/rust/src/lib.rs b/rust/src/lib.rs
index db550b49..18f73401 100644
--- a/rust/src/lib.rs
+++ b/rust/src/lib.rs
@@ -12,12 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// TODO : If I commit this I've fucked up
+// TODO : These clippy-allow are bad and needs to be removed
 #![allow(clippy::missing_safety_doc)]
-#![allow(clippy::type_complexity)]
-#![allow(clippy::from_over_into)]
-// TODO : This is bad and needs to be removed
 #![allow(clippy::result_unit_err)]
+#![allow(clippy::type_complexity)]
 #![doc(html_no_source)]
 #![doc(html_favicon_url = "/favicon.ico")]
 #![doc(html_logo_url = "/logo.png")]
@@ -348,7 +346,8 @@ pub fn open_view<F: AsRef<Path>>(filename: F) -> Result<rc::Ref<binaryview::Bina
         let mut file = File::open(filename).or(Err("Could not open file".to_string()))?;
 
         let mut buf = [0; 15];
-        file.read_exact(&mut buf).map_err(|_| "Not a valid BNDB (too small)".to_string())?;
+        file.read_exact(&mut buf)
+            .map_err(|_| "Not a valid BNDB (too small)".to_string())?;
         let sqlite_string = "SQLite format 3";
         if buf != sqlite_string.as_bytes() {
             return Err("Not a valid BNDB (invalid magic)".to_string());
diff --git a/rust/src/llil/lifting.rs b/rust/src/llil/lifting.rs
index bd87b9bc..1f019c85 100644
--- a/rust/src/llil/lifting.rs
+++ b/rust/src/llil/lifting.rs
@@ -1403,6 +1403,6 @@ impl Label {
 
 impl Default for Label {
     fn default() -> Self {
-        Label::new()
+        Self::new()
     }
 }
diff --git a/rust/src/metadata.rs b/rust/src/metadata.rs
index b17ba41c..1ccb9ae4 100644
--- a/rust/src/metadata.rs
+++ b/rust/src/metadata.rs
@@ -511,9 +511,9 @@ impl<S: BnStrCompatible> From<Vec<S>> for Ref<Metadata> {
     }
 }
 
-impl PartialEq<Self> for Ref<Metadata> {
+impl PartialEq for Metadata {
     fn eq(&self, other: &Self) -> bool {
-        unsafe { BNMetadataIsEqual(self.as_ref().handle, other.as_ref().handle) }
+        unsafe { BNMetadataIsEqual(self.handle, other.handle) }
     }
 }
 
diff --git a/rust/src/rc.rs b/rust/src/rc.rs
index 57deddc3..4ffecb3f 100644
--- a/rust/src/rc.rs
+++ b/rust/src/rc.rs
@@ -16,6 +16,7 @@
 
 use std::borrow::Borrow;
 use std::fmt::{Debug, Display, Formatter};
+use std::hash::{Hash, Hasher};
 use std::marker::PhantomData;
 use std::mem;
 use std::ops::{Deref, DerefMut};
@@ -118,7 +119,13 @@ impl<T: RefCountable + PartialEq> PartialEq for Ref<T> {
     }
 }
 
-impl<T: RefCountable + PartialEq> Eq for Ref<T> {}
+impl<T: RefCountable + Eq> Eq for Ref<T> {}
+
+impl<T: RefCountable + Hash> Hash for Ref<T> {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.contents.hash(state);
+    }
+}
 
 // Guard provides access to a core-allocated resource whose
 // reference is held indirectly (e.g. a core-allocated array
@@ -147,7 +154,7 @@ impl<'a, T> Guard<'a, T>
 where
     T: RefCountable,
 {
-    #[allow(clippy::should_implement_trait)]  // This _is_ out own (lite) version of that trait
+    #[allow(clippy::should_implement_trait)] // This _is_ out own (lite) version of that trait
     pub fn clone(&self) -> Ref<T> {
         unsafe { <T as RefCountable>::inc_ref(&self.contents) }
     }
diff --git a/rust/src/section.rs b/rust/src/section.rs
index b73809c0..49642b4d 100644
--- a/rust/src/section.rs
+++ b/rust/src/section.rs
@@ -46,11 +46,11 @@ impl From<BNSectionSemantics> for Semantics {
     }
 }
 
-impl Into<BNSectionSemantics> for Semantics {
-    fn into(self) -> BNSectionSemantics {
+impl From<Semantics> for BNSectionSemantics {
+    fn from(semantics: Semantics) -> Self {
         use self::BNSectionSemantics::*;
 
-        match self {
+        match semantics {
             Semantics::DefaultSection => DefaultSectionSemantics,
             Semantics::ReadOnlyCode => ReadOnlyCodeSectionSemantics,
             Semantics::ReadOnlyData => ReadOnlyDataSectionSemantics,
diff --git a/rust/src/symbol.rs b/rust/src/symbol.rs
index 0b3ef224..6b04582b 100644
--- a/rust/src/symbol.rs
+++ b/rust/src/symbol.rs
@@ -50,11 +50,11 @@ impl From<BNSymbolType> for SymbolType {
     }
 }
 
-impl Into<BNSymbolType> for SymbolType {
-    fn into(self) -> BNSymbolType {
+impl From<SymbolType> for BNSymbolType {
+    fn from(symbol_type: SymbolType) -> Self {
         use self::BNSymbolType::*;
 
-        match self {
+        match symbol_type {
             SymbolType::Function => FunctionSymbol,
             SymbolType::LibraryFunction => LibraryFunctionSymbol,
             SymbolType::ImportAddress => ImportAddressSymbol,
@@ -87,11 +87,11 @@ impl From<BNSymbolBinding> for Binding {
     }
 }
 
-impl Into<BNSymbolBinding> for Binding {
-    fn into(self) -> BNSymbolBinding {
+impl From<Binding> for BNSymbolBinding {
+    fn from(binding: Binding) -> Self {
         use self::BNSymbolBinding::*;
 
-        match self {
+        match binding {
             Binding::None => NoBinding,
             Binding::Local => LocalBinding,
             Binding::Global => GlobalBinding,
@@ -299,8 +299,4 @@ impl PartialEq for Symbol {
     }
 }
 
-impl Hash for Ref<Symbol> {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        (**self).hash(state);
-    }
-}
+impl Eq for Symbol {}
diff --git a/rust/src/types.rs b/rust/src/types.rs
index d9b28b4c..75475b6e 100644
--- a/rust/src/types.rs
+++ b/rust/src/types.rs
@@ -244,47 +244,38 @@ impl From<BNOffsetWithConfidence> for Conf<i64> {
     }
 }
 
-impl Into<BNTypeWithConfidence> for Conf<&Type> {
-    fn into(self) -> BNTypeWithConfidence {
-        BNTypeWithConfidence {
-            type_: self.contents.handle,
-            confidence: self.confidence,
-        }
-    }
-}
-
-impl Into<BNTypeWithConfidence> for &Conf<&Type> {
-    fn into(self) -> BNTypeWithConfidence {
-        BNTypeWithConfidence {
-            type_: self.contents.handle,
-            confidence: self.confidence,
+impl From<Conf<&Type>> for BNTypeWithConfidence {
+    fn from(conf: Conf<&Type>) -> Self {
+        Self {
+            type_: conf.contents.handle,
+            confidence: conf.confidence,
         }
     }
 }
 
-impl Into<BNBoolWithConfidence> for Conf<bool> {
-    fn into(self) -> BNBoolWithConfidence {
-        BNBoolWithConfidence {
-            value: self.contents,
-            confidence: self.confidence,
+impl From<Conf<bool>> for BNBoolWithConfidence {
+    fn from(conf: Conf<bool>) -> Self {
+        Self {
+            value: conf.contents,
+            confidence: conf.confidence,
         }
     }
 }
 
-impl<A: Architecture> Into<BNCallingConventionWithConfidence> for Conf<&CallingConvention<A>> {
-    fn into(self) -> BNCallingConventionWithConfidence {
-        BNCallingConventionWithConfidence {
-            convention: self.contents.handle,
-            confidence: self.confidence,
+impl<A: Architecture> From<Conf<&CallingConvention<A>>> for BNCallingConventionWithConfidence {
+    fn from(conf: Conf<&CallingConvention<A>>) -> Self {
+        Self {
+            convention: conf.contents.handle,
+            confidence: conf.confidence,
         }
     }
 }
 
-impl Into<BNOffsetWithConfidence> for Conf<i64> {
-    fn into(self) -> BNOffsetWithConfidence {
-        BNOffsetWithConfidence {
-            value: self.contents,
-            confidence: self.confidence,
+impl From<Conf<i64>> for BNOffsetWithConfidence {
+    fn from(conf: Conf<i64>) -> Self {
+        Self {
+            value: conf.contents,
+            confidence: conf.confidence,
         }
     }
 }
@@ -664,7 +655,6 @@ impl Drop for TypeBuilder {
 //////////
 // Type
 
-#[derive(Eq)]
 pub struct Type {
     pub(crate) handle: *mut BNType,
 }
@@ -1245,6 +1235,8 @@ impl PartialEq for Type {
     }
 }
 
+impl Eq for Type {}
+
 impl Hash for Type {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.handle.hash(state);
@@ -1417,12 +1409,7 @@ impl EnumerationBuilder {
         self
     }
 
-    pub fn replace<S: BnStrCompatible>(
-        &mut self,
-        id: usize,
-        name: S,
-        value: u64,
-    ) -> &mut Self {
+    pub fn replace<S: BnStrCompatible>(&self, id: usize, name: S, value: u64) -> &Self {
         let name = name.into_bytes_with_nul();
         unsafe {
             BNReplaceEnumerationBuilderMember(self.handle, id, name.as_ref().as_ptr() as _, value);
@@ -1622,7 +1609,7 @@ impl StructureBuilder {
         self
     }
 
-    pub fn append<'a, 'b, S: BnStrCompatible, T: Into<Conf<&'b Type>>>(
+    pub fn append<'a, S: BnStrCompatible, T: Into<Conf<&'a Type>>>(
         &'a self,
         t: T,
         name: S,
@@ -1643,11 +1630,11 @@ impl StructureBuilder {
         self
     }
 
-    pub fn insert_member<'a, 'b>(
-        &'a mut self,
-        member: &'b StructureMember,
+    pub fn insert_member(
+        &mut self,
+        member: &StructureMember,
         overwrite_existing: bool,
-    ) -> &'a mut Self {
+    ) -> &mut Self {
         let ty = member.ty.clone();
         self.insert(
             ty.as_ref(),
@@ -1660,15 +1647,15 @@ impl StructureBuilder {
         self
     }
 
-    pub fn insert<'a, 'b, S: BnStrCompatible, T: Into<Conf<&'b Type>>>(
-        &'a mut self,
+    pub fn insert<'a, S: BnStrCompatible, T: Into<Conf<&'a Type>>>(
+        &mut self,
         t: T,
         name: S,
         offset: u64,
         overwrite_existing: bool,
         access: MemberAccess,
         scope: MemberScope,
-    ) -> &'a mut Self {
+    ) -> &mut Self {
         let name = name.into_bytes_with_nul();
         unsafe {
             BNAddStructureBuilderMemberAtOffset(
@@ -1967,7 +1954,7 @@ impl QualifiedName {
 
     pub fn join(&self) -> Cow<str> {
         let join: *mut c_char = self.0.join;
-        unsafe { CStr::from_ptr(join).to_string_lossy() }
+        unsafe { CStr::from_ptr(join) }.to_string_lossy()
     }
 
     pub fn strings(&self) -> Vec<Cow<str>> {