diff --git a/.taskcluster.yml b/.taskcluster.yml index 5f7883c74..9cdef3214 100644 --- a/.taskcluster.yml +++ b/.taskcluster.yml @@ -37,13 +37,38 @@ tasks: cd repo && git config advice.detachedHead false && git checkout ${ref} && - cargo test && - cargo fmt -- --check + cargo test metadata: name: taskchampion-tests description: Run tests for taskchampion owner: dustin@v.igoro.us source: ${repo_url} + - $if: run + then: + provisionerId: 'proj-misc' + workerType: 'ci' + deadline: {$fromNow: '1 hour'} + expires: {$fromNow: '1 day'} + payload: + maxRunTime: 3600 + image: rust:latest + command: + - /bin/bash + - '-c' + - >- + rustup component add rustfmt && + git clone ${repo_url} repo && + cd repo && + git config advice.detachedHead false && + git checkout ${ref} && + rustup component add clippy-preview && + cargo clippy && + cargo fmt -- --check + metadata: + name: taskchampion-clippy + description: Run clippy and rustfmt for taskchampion + owner: dustin@v.igoro.us + source: ${repo_url} - $if: run then: provisionerId: 'proj-misc' diff --git a/cli/src/cmd/pending.rs b/cli/src/cmd/pending.rs index 8fbca52d9..b55c044d0 100644 --- a/cli/src/cmd/pending.rs +++ b/cli/src/cmd/pending.rs @@ -29,8 +29,8 @@ subcommand_invocation! { let mut t = Table::new(); t.set_format(table::format()); t.set_titles(row![b->"id", b->"description"]); - for i in 1..working_set.len() { - if let Some(ref task) = working_set[i] { + for (i, item) in working_set.iter().enumerate() { + if let Some(ref task) = item { t.add_row(row![i, task.get_description()]); } } diff --git a/cli/src/cmd/shared.rs b/cli/src/cmd/shared.rs index a7ef4ff58..4e24bfcc9 100644 --- a/cli/src/cmd/shared.rs +++ b/cli/src/cmd/shared.rs @@ -12,22 +12,16 @@ pub(super) fn get_task>(replica: &mut Replica, task_arg: S) -> Fal let task_arg = task_arg.as_ref(); // first try treating task as a working-set reference - match task_arg.parse::() { - Ok(i) => { - if let Some(task) = replica.get_working_set_task(i)? { - return Ok(task); - } + if let Ok(i) = task_arg.parse::() { + if let Some(task) = replica.get_working_set_task(i)? { + return Ok(task); } - Err(_) => {} } - match Uuid::parse_str(task_arg) { - Ok(uuid) => { - if let Some(task) = replica.get_task(&uuid)? { - return Ok(task); - } + if let Ok(uuid) = Uuid::parse_str(task_arg) { + if let Some(task) = replica.get_task(&uuid)? { + return Ok(task); } - Err(_) => {} } Err(format_err!("Cannot interpret {:?} as a task", task_arg)) diff --git a/sync-server/src/lib.rs b/sync-server/src/lib.rs index 4a50d77a2..e85125ed0 100644 --- a/sync-server/src/lib.rs +++ b/sync-server/src/lib.rs @@ -1,3 +1,5 @@ +#![allow(clippy::new_without_default)] + use std::collections::HashMap; type Blob = Vec; @@ -32,10 +34,7 @@ impl User { if last_version == since_version as usize { return vec![]; } - self.versions[since_version as usize..last_version] - .iter() - .map(|r| r.clone()) - .collect::>() + self.versions[since_version as usize..last_version].to_vec() } fn add_version(&mut self, version: u64, blob: Blob) -> VersionAdd { @@ -72,7 +71,7 @@ impl Server { self.users .get(username) .map(|user| user.get_versions(since_version)) - .unwrap_or_else(|| vec![]) + .unwrap_or_default() } /// Add a new version. If the given version number is incorrect, this responds with the diff --git a/taskchampion/src/lib.rs b/taskchampion/src/lib.rs index 471437a4f..cfc127a93 100644 --- a/taskchampion/src/lib.rs +++ b/taskchampion/src/lib.rs @@ -22,6 +22,7 @@ See the [TaskChampion Book](https://github.com/djmitche/taskchampion/blob/main/d for more information about the design and usage of the tool. */ + mod errors; mod replica; pub mod server; diff --git a/taskchampion/src/replica.rs b/taskchampion/src/replica.rs index 1485a9e0b..0ca2e67f6 100644 --- a/taskchampion/src/replica.rs +++ b/taskchampion/src/replica.rs @@ -16,9 +16,9 @@ pub struct Replica { impl Replica { pub fn new(storage: Box) -> Replica { - return Replica { + Replica { taskdb: TaskDB::new(storage), - }; + } } #[cfg(test)] @@ -56,7 +56,7 @@ impl Replica { pub fn all_tasks(&mut self) -> Fallible> { let mut res = HashMap::new(); for (uuid, tm) in self.taskdb.all_tasks()?.drain(..) { - res.insert(uuid.clone(), Task::new(uuid.clone(), tm)); + res.insert(uuid, Task::new(uuid, tm)); } Ok(res) } @@ -71,10 +71,10 @@ impl Replica { pub fn working_set(&mut self) -> Fallible>> { let working_set = self.taskdb.working_set()?; let mut res = Vec::with_capacity(working_set.len()); - for i in 0..working_set.len() { - res.push(match working_set[i] { + for item in working_set.iter() { + res.push(match item { Some(u) => match self.taskdb.get_task(&u)? { - Some(tm) => Some(Task::new(u, tm)), + Some(tm) => Some(Task::new(*u, tm)), None => None, }, None => None, @@ -88,7 +88,7 @@ impl Replica { Ok(self .taskdb .get_task(uuid)? - .map(move |tm| Task::new(uuid.clone(), tm))) + .map(move |tm| Task::new(*uuid, tm))) } /// Get an existing task by its working set index @@ -102,7 +102,7 @@ impl Replica { .map(move |tm| Task::new(uuid, tm))); } } - return Ok(None); + Ok(None) } /// Get the working set index for the given task uuid @@ -126,8 +126,7 @@ impl Replica { if self.taskdb.get_task(&uuid)?.is_some() { return Err(Error::DBError(format!("Task {} already exists", uuid)).into()); } - self.taskdb - .apply(Operation::Create { uuid: uuid.clone() })?; + self.taskdb.apply(Operation::Create { uuid })?; let mut task = Task::new(uuid, TaskMap::new()).into_mut(self); task.set_description(description)?; task.set_status(status)?; @@ -172,7 +171,7 @@ mod tests { let uuid = Uuid::new_v4(); let t = rep - .new_task(uuid.clone(), Status::Pending, "a task".into()) + .new_task(uuid, Status::Pending, "a task".into()) .unwrap(); assert_eq!(t.get_description(), String::from("a task")); assert_eq!(t.get_status(), Status::Pending); @@ -185,7 +184,7 @@ mod tests { let uuid = Uuid::new_v4(); let t = rep - .new_task(uuid.clone(), Status::Pending, "a task".into()) + .new_task(uuid, Status::Pending, "a task".into()) .unwrap(); let mut t = t.into_mut(&mut rep); @@ -211,10 +210,10 @@ mod tests { let mut rep = Replica::new_inmemory(); let uuid = Uuid::new_v4(); - rep.new_task(uuid.clone(), Status::Pending, "a task".into()) + rep.new_task(uuid, Status::Pending, "a task".into()) .unwrap(); - rep.delete_task(uuid.clone()).unwrap(); + rep.delete_task(uuid).unwrap(); assert_eq!(rep.get_task(&uuid).unwrap(), None); } @@ -223,7 +222,7 @@ mod tests { let mut rep = Replica::new_inmemory(); let uuid = Uuid::new_v4(); - rep.new_task(uuid.clone(), Status::Pending, "another task".into()) + rep.new_task(uuid, Status::Pending, "another task".into()) .unwrap(); let t = rep.get_task(&uuid).unwrap().unwrap(); @@ -247,7 +246,7 @@ mod tests { let mut rep = Replica::new_inmemory(); let uuid = Uuid::new_v4(); - rep.new_task(uuid.clone(), Status::Pending, "to-be-pending".into()) + rep.new_task(uuid, Status::Pending, "to-be-pending".into()) .unwrap(); let t = rep.get_working_set_task(1).unwrap().unwrap(); diff --git a/taskchampion/src/task.rs b/taskchampion/src/task.rs index 69d2a6f56..332eb57b1 100644 --- a/taskchampion/src/task.rs +++ b/taskchampion/src/task.rs @@ -123,7 +123,7 @@ impl Task { pub fn into_mut(self, replica: &mut Replica) -> TaskMut { TaskMut { task: self, - replica: replica, + replica, updated_modified: false, } } @@ -170,7 +170,7 @@ impl<'r> TaskMut<'r> { /// new status puts it in that set. pub fn set_status(&mut self, status: Status) -> Fallible<()> { if status == Status::Pending { - let uuid = self.uuid.clone(); + let uuid = self.uuid; self.replica.add_to_working_set(&uuid)?; } self.set_string("status", Some(String::from(status.to_taskmap()))) @@ -190,7 +190,7 @@ impl<'r> TaskMut<'r> { if !self.updated_modified { let now = format!("{}", Utc::now().timestamp()); self.replica - .update_task(self.task.uuid.clone(), "modified", Some(now.clone()))?; + .update_task(self.task.uuid, "modified", Some(now.clone()))?; self.task.taskmap.insert(String::from("modified"), now); self.updated_modified = true; } @@ -200,7 +200,7 @@ impl<'r> TaskMut<'r> { fn set_string(&mut self, property: &str, value: Option) -> Fallible<()> { self.lastmod()?; self.replica - .update_task(self.task.uuid.clone(), property, value.as_ref())?; + .update_task(self.task.uuid, property, value.as_ref())?; if let Some(v) = value { self.task.taskmap.insert(property.to_string(), v); @@ -213,7 +213,7 @@ impl<'r> TaskMut<'r> { fn set_timestamp(&mut self, property: &str, value: Option>) -> Fallible<()> { self.lastmod()?; self.replica.update_task( - self.task.uuid.clone(), + self.task.uuid, property, value.map(|v| format!("{}", v.timestamp())), ) diff --git a/taskchampion/src/taskdb.rs b/taskchampion/src/taskdb.rs index a24ffff0b..f705f3d5a 100644 --- a/taskchampion/src/taskdb.rs +++ b/taskchampion/src/taskdb.rs @@ -44,32 +44,30 @@ impl TaskDB { fn apply_op(txn: &mut dyn TaskStorageTxn, op: &Operation) -> Fallible<()> { match op { - &Operation::Create { uuid } => { + Operation::Create { uuid } => { // insert if the task does not already exist - if !txn.create_task(uuid)? { + if !txn.create_task(*uuid)? { return Err(Error::DBError(format!("Task {} already exists", uuid)).into()); } } - &Operation::Delete { ref uuid } => { + Operation::Delete { ref uuid } => { if !txn.delete_task(uuid)? { return Err(Error::DBError(format!("Task {} does not exist", uuid)).into()); } } - &Operation::Update { + Operation::Update { ref uuid, ref property, ref value, timestamp: _, } => { // update if this task exists, otherwise ignore - if let Some(task) = txn.get_task(uuid)? { - let mut task = task.clone(); - // TODO: update working_set if this is changing state to or from pending + if let Some(mut task) = txn.get_task(uuid)? { match value { Some(ref val) => task.insert(property.to_string(), val.clone()), None => task.remove(property), }; - txn.set_task(uuid.clone(), task)?; + txn.set_task(*uuid, task)?; } else { return Err(Error::DBError(format!("Task {} does not exist", uuid)).into()); } @@ -80,19 +78,19 @@ impl TaskDB { } /// Get all tasks. - pub fn all_tasks<'a>(&'a mut self) -> Fallible> { + pub fn all_tasks(&mut self) -> Fallible> { let mut txn = self.storage.txn()?; txn.all_tasks() } /// Get the UUIDs of all tasks - pub fn all_task_uuids<'a>(&'a mut self) -> Fallible> { + pub fn all_task_uuids(&mut self) -> Fallible> { let mut txn = self.storage.txn()?; txn.all_task_uuids() } /// Get the working set - pub fn working_set<'a>(&'a mut self) -> Fallible>> { + pub fn working_set(&mut self) -> Fallible>> { let mut txn = self.storage.txn()?; txn.get_working_set() } @@ -124,7 +122,7 @@ impl TaskDB { if let Some(uuid) = elt { if let Some(task) = txn.get_task(&uuid)? { if in_working_set(&task) { - new_ws.push(uuid.clone()); + new_ws.push(uuid); seen.insert(uuid); } } @@ -134,10 +132,8 @@ impl TaskDB { // Now go hunting for tasks that should be in this list but are not, adding them at the // end of the list. for (uuid, task) in txn.all_tasks()? { - if !seen.contains(&uuid) { - if in_working_set(&task) { - new_ws.push(uuid.clone()); - } + if !seen.contains(&uuid) && in_working_set(&task) { + new_ws.push(uuid); } } @@ -186,8 +182,8 @@ impl TaskDB { TaskDB::apply_version(txn.as_mut(), version)?; } - let operations: Vec = txn.operations()?.iter().map(|o| o.clone()).collect(); - if operations.len() == 0 { + let operations: Vec = txn.operations()?.to_vec(); + if operations.is_empty() { // nothing to sync back to the server.. break; } @@ -195,7 +191,7 @@ impl TaskDB { // now make a version of our local changes and push those let new_version = Version { version: txn.base_version()? + 1, - operations: operations, + operations, }; let new_version_str = serde_json::to_string(&new_version).unwrap(); println!("sending version {:?} to server", new_version.version); @@ -457,7 +453,7 @@ mod tests { // add everything to the TaskDB for uuid in &uuids { - db.apply(Operation::Create { uuid: uuid.clone() })?; + db.apply(Operation::Create { uuid: *uuid })?; } for i in &[0usize, 1, 4] { db.apply(Operation::Update { diff --git a/taskchampion/src/taskstorage/inmemory.rs b/taskchampion/src/taskstorage/inmemory.rs index c438b548e..dbc1b43f7 100644 --- a/taskchampion/src/taskstorage/inmemory.rs +++ b/taskchampion/src/taskstorage/inmemory.rs @@ -1,3 +1,5 @@ +#![allow(clippy::new_without_default)] + use crate::taskstorage::{Operation, TaskMap, TaskStorage, TaskStorageTxn}; use failure::{format_err, Fallible}; use std::collections::hash_map::Entry; @@ -48,7 +50,7 @@ impl<'t> TaskStorageTxn for Txn<'t> { fn create_task(&mut self, uuid: Uuid) -> Fallible { if let ent @ Entry::Vacant(_) = self.mut_data_ref().tasks.entry(uuid) { - ent.or_insert(TaskMap::new()); + ent.or_insert_with(TaskMap::new); Ok(true) } else { Ok(false) @@ -61,11 +63,7 @@ impl<'t> TaskStorageTxn for Txn<'t> { } fn delete_task(&mut self, uuid: &Uuid) -> Fallible { - if let Some(_) = self.mut_data_ref().tasks.remove(uuid) { - Ok(true) - } else { - Ok(false) - } + Ok(self.mut_data_ref().tasks.remove(uuid).is_some()) } fn all_tasks<'a>(&mut self) -> Fallible> { @@ -73,12 +71,12 @@ impl<'t> TaskStorageTxn for Txn<'t> { .data_ref() .tasks .iter() - .map(|(u, t)| (u.clone(), t.clone())) + .map(|(u, t)| (*u, t.clone())) .collect()) } fn all_task_uuids<'a>(&mut self) -> Fallible> { - Ok(self.data_ref().tasks.keys().map(|u| u.clone()).collect()) + Ok(self.data_ref().tasks.keys().copied().collect()) } fn base_version(&mut self) -> Fallible { @@ -110,7 +108,7 @@ impl<'t> TaskStorageTxn for Txn<'t> { fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible { let working_set = &mut self.mut_data_ref().working_set; - working_set.push(Some(uuid.clone())); + working_set.push(Some(*uuid)); Ok(working_set.len()) } diff --git a/taskchampion/src/taskstorage/kv.rs b/taskchampion/src/taskstorage/kv.rs index f7cd0cc4b..6a4e8179a 100644 --- a/taskchampion/src/taskstorage/kv.rs +++ b/taskchampion/src/taskstorage/kv.rs @@ -13,21 +13,20 @@ struct Key(uuid::Bytes); impl From<&[u8]> for Key { fn from(bytes: &[u8]) -> Key { - let key = Key(bytes.try_into().unwrap()); - key + Key(bytes.try_into().unwrap()) } } impl From<&Uuid> for Key { fn from(uuid: &Uuid) -> Key { - let key = Key(uuid.as_bytes().clone()); + let key = Key(*uuid.as_bytes()); key } } impl From for Key { fn from(uuid: Uuid) -> Key { - let key = Key(uuid.as_bytes().clone()); + let key = Key(*uuid.as_bytes()); key } } @@ -108,7 +107,7 @@ struct Txn<'t> { impl<'t> Txn<'t> { // get the underlying kv Txn - fn kvtxn<'a>(&mut self) -> &mut kv::Txn<'t> { + fn kvtxn(&mut self) -> &mut kv::Txn<'t> { if let Some(ref mut txn) = self.txn { txn } else { @@ -316,7 +315,7 @@ impl<'t> TaskStorageTxn for Txn<'t> { kvtxn.set( working_set_bucket, next_index.into(), - Msgpack::to_value_buf(uuid.clone())?, + Msgpack::to_value_buf(*uuid)?, )?; kvtxn.set( numbers_bucket, @@ -387,7 +386,7 @@ mod test { let uuid = Uuid::new_v4(); { let mut txn = storage.txn()?; - assert!(txn.create_task(uuid.clone())?); + assert!(txn.create_task(uuid)?); txn.commit()?; } { @@ -405,12 +404,12 @@ mod test { let uuid = Uuid::new_v4(); { let mut txn = storage.txn()?; - assert!(txn.create_task(uuid.clone())?); + assert!(txn.create_task(uuid)?); txn.commit()?; } { let mut txn = storage.txn()?; - assert!(!txn.create_task(uuid.clone())?); + assert!(!txn.create_task(uuid)?); txn.commit()?; } Ok(()) @@ -436,10 +435,7 @@ mod test { let uuid = Uuid::new_v4(); { let mut txn = storage.txn()?; - txn.set_task( - uuid.clone(), - taskmap_with(vec![("k".to_string(), "v".to_string())]), - )?; + txn.set_task(uuid, taskmap_with(vec![("k".to_string(), "v".to_string())]))?; txn.commit()?; } { @@ -472,7 +468,7 @@ mod test { let uuid = Uuid::new_v4(); { let mut txn = storage.txn()?; - assert!(txn.create_task(uuid.clone())?); + assert!(txn.create_task(uuid)?); txn.commit()?; } { diff --git a/taskchampion/src/taskstorage/mod.rs b/taskchampion/src/taskstorage/mod.rs index 8c37e934b..90ec0460e 100644 --- a/taskchampion/src/taskstorage/mod.rs +++ b/taskchampion/src/taskstorage/mod.rs @@ -52,10 +52,10 @@ pub trait TaskStorageTxn { fn delete_task(&mut self, uuid: &Uuid) -> Fallible; /// Get the uuids and bodies of all tasks in the storage, in undefined order. - fn all_tasks<'a>(&mut self) -> Fallible>; + fn all_tasks(&mut self) -> Fallible>; /// Get the uuids of all tasks in the storage, in undefined order. - fn all_task_uuids<'a>(&mut self) -> Fallible>; + fn all_task_uuids(&mut self) -> Fallible>; /// Get the current base_version for this storage -- the last version synced from the server. fn base_version(&mut self) -> Fallible; @@ -65,7 +65,7 @@ pub trait TaskStorageTxn { /// Get the current set of outstanding operations (operations that have not been sync'd to the /// server yet) - fn operations<'a>(&mut self) -> Fallible>; + fn operations(&mut self) -> Fallible>; /// Add an operation to the end of the list of operations in the storage. Note that this /// merely *stores* the operation; it is up to the TaskDB to apply it. diff --git a/taskchampion/src/taskstorage/operation.rs b/taskchampion/src/taskstorage/operation.rs index 52bc55d56..665a151ef 100644 --- a/taskchampion/src/taskstorage/operation.rs +++ b/taskchampion/src/taskstorage/operation.rs @@ -109,12 +109,9 @@ impl Operation { } else if timestamp1 < timestamp2 { // prefer the later modification (None, Some(operation2)) - } else if timestamp1 > timestamp2 { - // prefer the later modification - //(Some(operation1), None) - (Some(operation1), None) } else { - // arbitrarily resolve in favor of the first operation + // prefer the later modification or, if the modifications are the same, + // just choose one of them (Some(operation1), None) } }