Merge pull request #121 from mohammadfawaz/mohammadfawaz/chores

Running Clippy + `rustfmt` and add them to CI

.github/workflows/build_and_test.yml

@@ -2,6 +2,7 @@ name: tests
 
 env:
   version: 8.0.3
+  RUST_VERSION: 1.74.0
 
 on:
   push:
@@ -105,6 +106,31 @@ jobs:
           cargo build
           cargo test
 
+  cargo-fmt-check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Install toolchain
+        uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ env.RUST_VERSION }}
+          components: rustfmt
+      - name: Check Formatting
+        run: cargo fmt --all -- --check
+
+  cargo-clippy:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Install toolchain
+        uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ env.RUST_VERSION }}
+          components: clippy
+      - uses: Swatinem/rust-cache@v2
+      - name: Check Clippy Linter
+        run: cargo clippy --all-features --all-targets -- -D warnings
+
 # TODO: move to scip-sys repo
 #  MacOS-test:
 #    runs-on: macos-latest

src/branchrule.rs

@@ -1,7 +1,7 @@
 use crate::ffi;
 use crate::variable::Variable;
-use std::rc::Rc;
 use scip_sys::SCIP_Result;
+use std::rc::Rc;
 
 /// A trait for defining custom branching rules.
 pub trait BranchRule {

src/heuristic.rs

@@ -1,5 +1,5 @@
-use std::ops::{BitOr, BitOrAssign};
 use scip_sys::SCIP_Result;
+use std::ops::{BitOr, BitOrAssign};
 
 use crate::ffi;
 

src/model.rs

@@ -242,6 +242,7 @@ impl Model<ProblemCreated> {
     /// # Returns
     ///
     /// This function returns the `Model` instance for which the heuristic was included. This allows for method chaining.
+    #[allow(clippy::too_many_arguments)]
     pub fn include_heur(
         self,
         name: &str,
@@ -334,14 +335,13 @@ impl Model<ProblemCreated> {
         self.scip
             .solve()
             .expect("Failed to solve problem in state ProblemCreated");
-        let new_model = Model {
+        Model {
             scip: self.scip,
             state: Solved {
                 vars: self.state.vars,
                 conss: self.state.conss,
             },
-        };
-        new_model
+        }
     }
 }
 
@@ -574,6 +574,7 @@ pub trait ProblemOrSolving {
     /// # Panics
     ///
     /// This method panics if the constraint cannot be created in the current state.
+    #[allow(clippy::too_many_arguments)]
     fn add_cons_quadratic(
         &mut self,
         lin_vars: Vec<Rc<Variable>>,
@@ -1114,7 +1115,6 @@ impl<T> Model<T> {
         self
     }
 
-
     /// Includes all default plugins in the SCIP instance and returns a new `Model` instance with a `PluginsIncluded` state.
     pub fn include_default_plugins(mut self) -> Model<PluginsIncluded> {
         self.scip
@@ -1525,11 +1525,11 @@ mod tests {
 
         // Indicator constraint: `b == 1` implies `x1 - x2 <= -1`
         model.add_cons_indicator(
-            b.clone(), 
-            vec![x1.clone(), x2.clone()], 
-            &mut [1., -1.], 
+            b.clone(),
+            vec![x1.clone(), x2.clone()],
+            &mut [1., -1.],
             -1.,
-            "indicator"
+            "indicator",
         );
 
         // Force `b` to be exactly 1 and later make sure that the constraint `x1 - x2 <= -1` is
@@ -1715,13 +1715,17 @@ mod tests {
     #[test]
     fn set_param_all_states() {
         Model::new()
-            .set_int_param("display/verblevel", 0).unwrap()
+            .set_int_param("display/verblevel", 0)
+            .unwrap()
             .include_default_plugins()
-            .set_int_param("display/verblevel", 0).unwrap()
+            .set_int_param("display/verblevel", 0)
+            .unwrap()
             .read_prob("data/test/simple.lp")
             .unwrap()
-            .set_int_param("display/verblevel", 0).unwrap()
+            .set_int_param("display/verblevel", 0)
+            .unwrap()
             .solve()
-            .set_int_param("display/verblevel", 0).unwrap();
+            .set_int_param("display/verblevel", 0)
+            .unwrap();
     }
 }

src/prelude.rs

@@ -1,9 +1,9 @@
 pub use crate::model::Model;
 pub use crate::model::ModelWithProblem;
+pub use crate::model::ObjSense;
 pub use crate::model::ProblemOrSolving;
 pub use crate::model::WithSolutions;
 pub use crate::model::WithSolvingStats;
-pub use crate::model::ObjSense;
+pub use crate::retcode::Retcode;
 pub use crate::status::Status;
 pub use crate::variable::VarType;
-pub use crate::retcode::Retcode;

src/pricer.rs

@@ -1,5 +1,5 @@
-use scip_sys::SCIP_Result;
 use crate::ffi;
+use scip_sys::SCIP_Result;
 
 /// A trait for SCIP pricers.
 pub trait Pricer {
@@ -49,7 +49,8 @@ mod tests {
     use crate::{
         model::{Model, ModelWithProblem},
         status::Status,
-        variable::VarType, Solving, ProblemOrSolving,
+        variable::VarType,
+        ProblemOrSolving, Solving,
     };
 
     struct PanickingPricer;

src/scip.rs

@@ -16,7 +16,7 @@ use std::rc::Rc;
 pub(crate) struct ScipPtr {
     pub(crate) raw: *mut ffi::SCIP,
     consumed: bool,
-    vars_added_in_solving: Vec<*mut ffi::SCIP_VAR>
+    vars_added_in_solving: Vec<*mut ffi::SCIP_VAR>,
 }
 
 impl ScipPtr {
@@ -348,6 +348,7 @@ impl ScipPtr {
         Ok(Constraint { raw: scip_cons })
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub(crate) fn create_cons_quadratic(
         &mut self,
         lin_vars: Vec<Rc<Variable>>,
@@ -810,6 +811,7 @@ impl ScipPtr {
         Ok(())
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub(crate) fn include_heur(
         &self,
         name: &str,
@@ -975,7 +977,11 @@ impl ScipPtr {
 
     pub(crate) fn add_sol(&self, mut sol: Solution) -> Result<bool, Retcode> {
         let mut stored = MaybeUninit::uninit();
-        scip_call!(ffi::SCIPaddSolFree(self.raw, &mut sol.raw, stored.as_mut_ptr()));
+        scip_call!(ffi::SCIPaddSolFree(
+            self.raw,
+            &mut sol.raw,
+            stored.as_mut_ptr()
+        ));
         let stored = unsafe { stored.assume_init() };
         Ok(stored != 0)
     }

src/solution.rs

@@ -87,4 +87,4 @@ mod tests {
 
         assert_eq!(sol.obj_val(), model.obj_val());
     }
-}
+}

src/status.rs

@@ -1,5 +1,5 @@
-use scip_sys::SCIP_Status;
 use crate::ffi;
+use scip_sys::SCIP_Status;
 
 /// An enum representing the status of a SCIP optimization run.
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
@@ -63,17 +63,17 @@ impl From<SCIP_Status> for Status {
     }
 }
 
-
 #[cfg(test)]
-mod tests{
+mod tests {
     use super::*;
     use crate::Model;
 
     #[test]
     fn time_limit() {
         let model = Model::new()
             .hide_output()
-            .set_real_param("limits/time", 0.).unwrap()
+            .set_real_param("limits/time", 0.)
+            .unwrap()
             .include_default_plugins()
             .read_prob("data/test/simple.lp")
             .unwrap()
@@ -82,12 +82,12 @@ mod tests{
         assert_eq!(model.status(), Status::TimeLimit);
     }
 
-
     #[test]
     fn memory_limit() {
         let model = Model::new()
             .hide_output()
-            .set_real_param("limits/memory", 0.).unwrap()
+            .set_real_param("limits/memory", 0.)
+            .unwrap()
             .include_default_plugins()
             .read_prob("data/test/gen-ip054.mps")
             .unwrap()
@@ -100,7 +100,8 @@ mod tests{
     fn gap_limit() {
         let model = Model::new()
             .hide_output()
-            .set_real_param("limits/gap", 100000.).unwrap()
+            .set_real_param("limits/gap", 100000.)
+            .unwrap()
             .include_default_plugins()
             .read_prob("data/test/gen-ip054.mps")
             .unwrap()
@@ -113,7 +114,8 @@ mod tests{
     fn solution_limit() {
         let model = Model::new()
             .hide_output()
-            .set_int_param("limits/solutions", 0).unwrap()
+            .set_int_param("limits/solutions", 0)
+            .unwrap()
             .include_default_plugins()
             .read_prob("data/test/simple.lp")
             .unwrap()
@@ -126,7 +128,8 @@ mod tests{
     fn total_node_limit() {
         let model = Model::new()
             .hide_output()
-            .set_longint_param("limits/totalnodes", 0).unwrap()
+            .set_longint_param("limits/totalnodes", 0)
+            .unwrap()
             .include_default_plugins()
             .read_prob("data/test/simple.lp")
             .unwrap()
@@ -135,12 +138,12 @@ mod tests{
         assert_eq!(model.status(), Status::TotalNodeLimit);
     }
 
-
     #[test]
     fn stall_node_limit() {
         let model = Model::new()
             .hide_output()
-            .set_longint_param("limits/stallnodes", 0).unwrap()
+            .set_longint_param("limits/stallnodes", 0)
+            .unwrap()
             .include_default_plugins()
             .read_prob("data/test/simple.lp")
             .unwrap()
@@ -153,7 +156,8 @@ mod tests{
     fn best_solution_limit() {
         let model = Model::new()
             .hide_output()
-            .set_int_param("limits/bestsol", 0).unwrap()
+            .set_int_param("limits/bestsol", 0)
+            .unwrap()
             .include_default_plugins()
             .read_prob("data/test/simple.lp")
             .unwrap()