Code Refactoring

* Correct a number of clippy links.
* Make the generator use a BTreeMap to avoid flakey tests from HashMap.
  BTree means it is ordered and so consistent.

Cargo.toml

@@ -1,5 +1,5 @@
 [workspace]
-
+resolver = "2"
 members = ["ni-fpga-interface", "ni-fpga-interface-build", "examples/host_example"]
 
 [workspace.package]

ni-fpga-interface-build/src/address_definitions.rs

@@ -4,7 +4,7 @@
 use lang_c::ast::{Constant, Expression, IntegerBase};
 
 /// The type of register value we have found.
-#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
+#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, PartialOrd, Ord)]
 pub enum AddressKind {
     Control,
     Indicator,
@@ -19,13 +19,13 @@ pub enum AddressKind {
 impl AddressKind {
     /// Returns if the kind is one of the array types.
     pub fn is_array(&self) -> bool {
-        match self {
+        matches!(
+            self,
             AddressKind::ControlArray
-            | AddressKind::IndicatorArray
-            | AddressKind::ControlArraySize
-            | AddressKind::IndicatorArraySize => true,
-            _ => false,
-        }
+                | AddressKind::IndicatorArray
+                | AddressKind::ControlArraySize
+                | AddressKind::IndicatorArraySize
+        )
     }
 
     /// If it is an array type, it will return the size version of it.

ni-fpga-interface-build/src/address_definitions_visitor.rs

@@ -4,18 +4,21 @@ use super::address_definitions::AddressKind;
 use lang_c::ast::*;
 use lang_c::span::{Node, Span};
 use lang_c::visit::Visit;
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 
 /// Defines a register location.
-#[derive(Debug, PartialEq, Eq, Hash, Clone)]
+#[derive(Debug, PartialEq, Eq, Hash, Clone, PartialOrd, Ord)]
 pub struct LocationDefinition {
     pub kind: AddressKind,
     pub name: String,
     pub datatype: String,
 }
 
 /// The set of registers that this visitor will return.
-pub type AddressSet = HashMap<LocationDefinition, u32>;
+///
+/// We use a BTree here so we get deterministic generation
+/// which was failing tests with HashMap.
+pub type AddressSet = BTreeMap<LocationDefinition, u32>;
 
 /// Extracts the register definitions from the AST.
 pub struct AddressDefinitionsVisitor {
@@ -29,12 +32,12 @@ impl AddressDefinitionsVisitor {
     /// e.g. if the file is called `NiFpga_Main.h` then the prefix is Main.
     pub fn new(interface_name: &str) -> Self {
         Self {
-            registers: HashMap::new(),
+            registers: BTreeMap::new(),
             prefix: format!("NiFpga_{interface_name}_"),
         }
     }
 
-    fn process_enum_type<'ast>(&mut self, node: &'ast EnumType, type_name: &str) {
+    fn process_enum_type(&mut self, node: &EnumType, type_name: &str) {
         let enum_name = type_name.strip_prefix(&self.prefix).unwrap();
         let (kind, type_name) = enum_name_to_types(enum_name);
 
@@ -49,7 +52,7 @@ impl AddressDefinitionsVisitor {
             };
 
             let assignment_express = &variant.expression.as_ref().unwrap().node;
-            let value = value_from_discriminant(&assignment_express);
+            let value = value_from_discriminant(assignment_express);
 
             self.registers.insert(definition, value);
         }
@@ -67,7 +70,7 @@ fn enum_name_to_types(name: &str) -> (AddressKind, &str) {
         type_name = type_name.strip_suffix("Size").unwrap();
     }
 
-    return (kind, type_name);
+    (kind, type_name)
 }
 
 /// Run through the options confirming the prefix.
@@ -84,14 +87,9 @@ fn extract_type_from_start(name: &str) -> Option<AddressKind> {
         AddressKind::TargetToHostFifo,
         AddressKind::HostToTargetFifo,
     ];
-
-    for kind in options {
-        if name.starts_with(kind.prefix()) {
-            return Some(kind);
-        }
-    }
-
-    return None;
+    options
+        .into_iter()
+        .find(|&kind| name.starts_with(kind.prefix()))
 }
 
 /// Extract the name of the individual control which is
@@ -103,25 +101,24 @@ fn control_indicator_name_from_full(full_name: &str) -> &str {
 /// Check if the declaration is a typedef.
 fn is_typedef(node: &Declaration) -> bool {
     for specifier in &node.specifiers {
-        match specifier.node {
-            DeclarationSpecifier::StorageClass(Node {
-                node: StorageClassSpecifier::Typedef,
-                ..
-            }) => return true,
-            _ => (),
+        if let DeclarationSpecifier::StorageClass(Node {
+            node: StorageClassSpecifier::Typedef,
+            ..
+        }) = specifier.node
+        {
+            return true;
         }
     }
-    return false;
+    false
 }
 
 /// Extract the name of a typedef declaration.
 ///
 /// If we can find an identifier then we return none.
 fn get_typedef_name(node: &Declaration) -> Option<String> {
     for declarator in node.declarators.iter() {
-        match &declarator.node.declarator.node.kind.node {
-            DeclaratorKind::Identifier(identifier) => return Some(identifier.node.name.clone()),
-            _ => (),
+        if let DeclaratorKind::Identifier(identifier) = &declarator.node.declarator.node.kind.node {
+            return Some(identifier.node.name.clone());
         }
     }
     None
@@ -132,12 +129,12 @@ impl<'ast> Visit<'ast> for AddressDefinitionsVisitor {
         if is_typedef(declaration) {
             if let Some(name) = get_typedef_name(declaration) {
                 for specifier in declaration.specifiers.iter() {
-                    match &specifier.node {
-                        DeclarationSpecifier::TypeSpecifier(Node {
-                            node: TypeSpecifier::Enum(node),
-                            ..
-                        }) => self.process_enum_type(&node.node, &name),
-                        _ => (),
+                    if let DeclarationSpecifier::TypeSpecifier(Node {
+                        node: TypeSpecifier::Enum(node),
+                        ..
+                    }) = &specifier.node
+                    {
+                        self.process_enum_type(&node.node, &name);
                     }
                 }
             }

ni-fpga-interface-build/src/bindings_parser.rs

@@ -53,7 +53,7 @@ impl InterfaceDescription {
             #registers
             #fifos
         };
-        println!("{}", tokens.to_string());
+        println!("{}", tokens);
         let file = syn::parse2(tokens).unwrap();
         prettyplease::unparse(&file)
     }
@@ -118,7 +118,7 @@ typedef struct NiFpga_FxpTypeInfo
 } NiFpga_FxpTypeInfo;
 
 "#;
-    output.write(common_types.as_bytes()).unwrap();
+    output.write_all(common_types.as_bytes()).unwrap();
 
     for line in BufReader::new(input).lines() {
         let line = line.unwrap();

ni-fpga-interface-build/src/custom_type_register_visitor.rs

@@ -197,7 +197,7 @@ struct ConstantType<'a> {
     suffix: &'a str,
 }
 
-fn get_constant_type_from_name<'a, 'b>(prefix: &'a str, name: &'b str) -> Option<ConstantType<'b>> {
+fn get_constant_type_from_name<'b>(prefix: &str, name: &'b str) -> Option<ConstantType<'b>> {
     let short_name = name.strip_prefix(prefix);
 
     if let Some(name) = short_name {

ni-fpga-interface-build/src/lib.rs

@@ -158,7 +158,7 @@ impl FpgaCInterface {
             &PathBuf::from(&self.custom_h),
         );
 
-        std::fs::write(&mod_path, interface_description.generate_rust_output()).unwrap();
+        std::fs::write(mod_path, interface_description.generate_rust_output()).unwrap();
     }
 }
 

ni-fpga-interface-build/src/registers_generator.rs

@@ -422,8 +422,8 @@ mod tests {
             pub mod fifos {
                 use ni_fpga_interface::fifos::{ ReadFifo, WriteFifo };
 
-                pub const to_fpga: WriteFifo<u8> = WriteFifo::new(0x1);
                 pub const from_fpga: ReadFifo<f32> = ReadFifo::new(0x2);
+                pub const to_fpga: WriteFifo<u8> = WriteFifo::new(0x1);
             }
         };