aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Gattozzi <mgattozzi@gmail.com>2019-12-09 17:42:08 -0500
committerMichael Gattozzi <mgattozzi@gmail.com>2019-12-10 13:17:08 -0500
commit1dec32c37ac2f563e164021c9f21052f11fd1d20 (patch)
tree8d5508b09b742e63e72fbbf1571b5667b3ec0e1e
parentfcd4ccbec30493872feba137342347541b6a4e28 (diff)
Make the pre-commit script pedantic and fix errors
This commit really ups the level and quality of the Rust code by setting clippy to pedantic mode. It also fixes an issue where bash continued to run scripts even if something failed with a non-zero exit status. We also deny all warnings so as to actually fail the builds and the commit hooks. This should make sure code quality stays at a high level.
-rw-r--r--.cargo/config61
-rwxr-xr-x.dev-suite/hooked/pre-commit4
-rw-r--r--hooked/src/bin/hooked-commit-msg.rs2
-rw-r--r--hooked/src/main.rs14
-rw-r--r--hooked/tests/init.rs4
-rw-r--r--shared/src/lib.rs5
-rw-r--r--src/main.rs9
-rw-r--r--ticket/src/actions.rs17
-rw-r--r--ticket/src/main.rs47
-rw-r--r--ticket/src/tui.rs16
10 files changed, 137 insertions, 42 deletions
diff --git a/.cargo/config b/.cargo/config
new file mode 100644
index 0000000..76fe2f0
--- /dev/null
+++ b/.cargo/config
@@ -0,0 +1,61 @@
+[build]
+rustflags = [
+ "-D",
+ "warnings",
+ "-D",
+ "missing-debug-implementations",
+ "-D",
+ "missing-docs",
+ "-D",
+ "trivial-casts",
+ "-D",
+ "trivial-numeric-casts",
+ "-D",
+ "unused-extern-crates",
+ "-D",
+ "unused-import-braces",
+ "-D",
+ "unused-qualifications",
+ "-D",
+ "unused-results",
+ "-D",
+ "bad-style",
+ "-D",
+ "const-err",
+ "-D",
+ "dead-code",
+ "-D",
+ "improper-ctypes",
+ "-D",
+ "legacy-directory-ownership",
+ "-D",
+ "non-shorthand-field-patterns",
+ "-D",
+ "no-mangle-generic-items",
+ "-D",
+ "overflowing-literals",
+ "-D",
+ "path_statements",
+ "-D",
+ "patterns-in-fns-without-body",
+ "-D",
+ "plugin-as-library",
+ "-D",
+ "private-in-public",
+ "-D",
+ "safe-extern-statics",
+ "-D",
+ "unconditional-recursion",
+ "-D",
+ "unions-with-drop-fields",
+ "-D",
+ "unused",
+ "-D",
+ "unused-allocation",
+ "-D",
+ "unused-comparisons",
+ "-D",
+ "unused-parens",
+ "-D",
+ "while-true"
+]
diff --git a/.dev-suite/hooked/pre-commit b/.dev-suite/hooked/pre-commit
index 9ae83cf..931d7ac 100755
--- a/.dev-suite/hooked/pre-commit
+++ b/.dev-suite/hooked/pre-commit
@@ -1,5 +1,7 @@
#! /bin/bash
+set -eu -o pipefail
+
cargo build --all
cargo test --all
cargo +nightly fmt --all -- --check
-cargo clippy --all --all-targets
+cargo clippy --all --all-targets -- -W clippy::pedantic
diff --git a/hooked/src/bin/hooked-commit-msg.rs b/hooked/src/bin/hooked-commit-msg.rs
index 7f2a996..b7de022 100644
--- a/hooked/src/bin/hooked-commit-msg.rs
+++ b/hooked/src/bin/hooked-commit-msg.rs
@@ -1,3 +1,5 @@
+//! Linting tool for commit messages
+
use shared::find_root;
use std::{
env::args,
diff --git a/hooked/src/main.rs b/hooked/src/main.rs
index a3d8199..7e33499 100644
--- a/hooked/src/main.rs
+++ b/hooked/src/main.rs
@@ -1,3 +1,5 @@
+//! git hook manager tool
+
#[cfg(windows)]
use anyhow::bail;
use anyhow::Result;
@@ -45,9 +47,9 @@ enum Args {
#[paw::main]
fn main(args: Args) {
- env::var("RUST_LOG").map(drop).unwrap_or_else(|_| {
- env::set_var("RUST_LOG", "info");
- });
+ env::var("RUST_LOG")
+ .ok()
+ .map_or_else(|| env::set_var("RUST_LOG", "info"), drop);
pretty_env_logger::init();
if let Err(e) = match args {
Args::Init => init(),
@@ -74,7 +76,9 @@ fn init() -> Result<()> {
let git_hook = &git_hooks.join(hook);
debug!("git_hook path: {}", git_hook.display());
- if !path.exists() {
+ if path.exists() {
+ debug!("git hook {} already exists. Skipping creation.", hook);
+ } else {
debug!("Creating dev-suite hook.");
let mut file = fs::File::create(&path)?;
trace!("File created.");
@@ -86,8 +90,6 @@ fn init() -> Result<()> {
file.write_all(b"#! /bin/bash")?;
debug!("Writing data to file.");
debug!("Created git hook {}.", hook);
- } else {
- debug!("git hook {} already exists. Skipping creation.", hook);
}
let path = path.canonicalize()?;
diff --git a/hooked/tests/init.rs b/hooked/tests/init.rs
index 2596d5b..65c4aab 100644
--- a/hooked/tests/init.rs
+++ b/hooked/tests/init.rs
@@ -33,10 +33,10 @@ const HOOKS: [&str; 18] = [
#[test]
fn init() -> Result<(), Box<dyn Error>> {
let dir = tempdir()?;
- Repository::init(&dir)?;
+ let _ = Repository::init(&dir)?;
let mut cmd = Command::cargo_bin("hooked")?;
env::set_current_dir(&dir)?;
- cmd.arg("init").assert().success();
+ let _ = cmd.arg("init").assert().success();
let git = &dir.path().join(".git").join("hooks");
let dev = &dir.path().join(".dev-suite").join("hooked");
diff --git a/shared/src/lib.rs b/shared/src/lib.rs
index 138e8d0..db28822 100644
--- a/shared/src/lib.rs
+++ b/shared/src/lib.rs
@@ -1,3 +1,5 @@
+//! Common functionality needed in many of the tools
+
use anyhow::{
bail,
Result,
@@ -7,6 +9,7 @@ use std::{
path::PathBuf,
};
+/// Finds the top level folder of the repo and returns it's canonicalized path
pub fn find_root() -> Result<PathBuf> {
let mut location = env::current_dir()?;
let mut found_root = false;
@@ -14,7 +17,7 @@ pub fn find_root() -> Result<PathBuf> {
for loc in location.ancestors() {
let mut loc = loc.join(".git");
if loc.exists() {
- loc.pop();
+ let _ = loc.pop();
found_root = true;
location = loc.canonicalize()?;
break;
diff --git a/src/main.rs b/src/main.rs
index 7869f4d..a1fac85 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,3 +1,4 @@
+//! dev-suite cli tool to install and update devsuite and it's tooling
use anyhow::{
format_err,
Result,
@@ -32,9 +33,10 @@ fn main(args: Args) {
}
}
+/// Initialize a git repo with all the tools wanted for it
fn init() -> Result<()> {
// Make sure we're in a valid git repo
- find_root()?;
+ let _ = find_root()?;
let checkboxes = &["hooked - Managed git hooks", "ticket - In repo tickets"];
let defaults = &[true, true];
let selections = Checkboxes::with_theme(&ColorfulTheme::default())
@@ -61,7 +63,7 @@ fn init() -> Result<()> {
.map_err(|_| format_err!(
"It looks like hooked is not on your $PATH. Did you run 'ds install'?"
))?;
- Command::new("hooked").arg("init").spawn()?.wait()?;
+ let _ = Command::new("hooked").arg("init").spawn()?.wait()?;
}
Tools::Ticket => {
which("ticket")
@@ -69,7 +71,7 @@ fn init() -> Result<()> {
.map_err(|_| format_err!(
"It looks like ticket is not on your $PATH. Did you run 'ds install'?"
))?;
- Command::new("ticket").arg("init").spawn()?.wait()?;
+ let _ = Command::new("ticket").arg("init").spawn()?.wait()?;
}
}
}
@@ -77,6 +79,7 @@ fn init() -> Result<()> {
Ok(())
}
+/// Tools available for installation or usage by dev-suite
enum Tools {
Hooked,
Ticket,
diff --git a/ticket/src/actions.rs b/ticket/src/actions.rs
index 3e237ff..0d371b3 100644
--- a/ticket/src/actions.rs
+++ b/ticket/src/actions.rs
@@ -10,18 +10,21 @@ use log::*;
use shared::find_root;
use std::{
fs,
- path::PathBuf,
+ path::{
+ Path,
+ PathBuf,
+ },
};
pub fn get_open_tickets() -> Result<Vec<Ticket>> {
- get_tickets(open_tickets()?)
+ get_tickets(&open_tickets()?)
}
pub fn get_closed_tickets() -> Result<Vec<Ticket>> {
- get_tickets(closed_tickets()?)
+ get_tickets(&closed_tickets()?)
}
-fn get_tickets(path: PathBuf) -> Result<Vec<Ticket>> {
+fn get_tickets(path: &Path) -> Result<Vec<Ticket>> {
let mut out = Vec::new();
debug!("Looking for ticket.");
for entry in fs::read_dir(&path)? {
@@ -63,14 +66,14 @@ pub fn get_all_ticketsv0() -> Result<Vec<TicketV0>> {
Ok(tickets)
}
pub fn get_open_ticketsv0() -> Result<Vec<TicketV0>> {
- get_ticketsv0(open_tickets()?)
+ get_ticketsv0(&open_tickets()?)
}
pub fn get_closed_ticketsv0() -> Result<Vec<TicketV0>> {
- get_ticketsv0(closed_tickets()?)
+ get_ticketsv0(&closed_tickets()?)
}
-fn get_ticketsv0(path: PathBuf) -> Result<Vec<TicketV0>> {
+fn get_ticketsv0(path: &Path) -> Result<Vec<TicketV0>> {
let mut out = Vec::new();
debug!("Looking for ticket.");
for entry in fs::read_dir(&path)? {
diff --git a/ticket/src/main.rs b/ticket/src/main.rs
index 11e27e8..74a5699 100644
--- a/ticket/src/main.rs
+++ b/ticket/src/main.rs
@@ -1,3 +1,6 @@
+//! ticket is a cli tool to create, delete, and manage tickets as part of
+//! repository, rather than a separate service outside the history of the
+//! code.
mod actions;
mod tui;
@@ -18,6 +21,7 @@ use serde::{
Serialize,
};
use std::{
+ convert::TryInto,
env,
fs,
process,
@@ -55,9 +59,9 @@ enum Cmd {
#[paw::main]
fn main(args: Args) {
- env::var("RUST_LOG").map(drop).unwrap_or_else(|_| {
- env::set_var("RUST_LOG", "info");
- });
+ env::var("RUST_LOG")
+ .ok()
+ .map_or_else(|| env::set_var("RUST_LOG", "info"), drop);
pretty_env_logger::init();
if let Some(cmd) = args.cmd {
@@ -116,8 +120,8 @@ fn new() -> Result<()> {
"Create buffer file for the description at {}.",
description.display()
);
- fs::File::create(&description)?;
- Command::new(&env::var("EDITOR").unwrap_or_else(|_| "vi".into()))
+ let _ = fs::File::create(&description)?;
+ let _ = Command::new(&env::var("EDITOR").unwrap_or_else(|_| "vi".into()))
.arg(&description)
.spawn()?
.wait()?;
@@ -131,7 +135,11 @@ fn new() -> Result<()> {
title,
status: Status::Open,
id: Uuid::new_v1(
- Timestamp::from_unix(Context::new(1), Utc::now().timestamp() as u64, 0),
+ Timestamp::from_unix(
+ Context::new(1),
+ Utc::now().timestamp().try_into()?,
+ 0,
+ ),
&[0, 5, 2, 4, 9, 3],
)?,
assignees: Vec::new(),
@@ -254,15 +262,15 @@ fn migrate() -> Result<()> {
let open_tickets_path = open_tickets()?;
let closed_tickets_path = closed_tickets()?;
- for t in tickets.into_iter() {
+ for t in tickets {
let ticket = Ticket {
title: t.title,
status: t.status,
id: Uuid::new_v1(
- Timestamp::from_unix(&ctx, Utc::now().timestamp() as u64, 0),
+ Timestamp::from_unix(&ctx, Utc::now().timestamp().try_into()?, 0),
&[0, 5, 2, 4, 9, 3],
)?,
- assignees: t.assignee.map(|a| vec![a]).unwrap_or_else(Vec::new),
+ assignees: t.assignee.map_or_else(Vec::new, |a| vec![a]),
description: t.description,
comments: Vec::new(),
version: Version::V1,
@@ -289,7 +297,9 @@ fn migrate() -> Result<()> {
Ok(())
}
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
+/// The fundamental type this tool revolves around. The ticket represents
+/// everything about an issue or future plan for the code base.
pub struct Ticket {
title: String,
status: Status,
@@ -300,15 +310,21 @@ pub struct Ticket {
version: Version,
}
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
+/// Enum representing what version of the ticket it is and the assumptions that
+/// can be made about it
pub enum Version {
+ /// The first version
V1,
}
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
+/// Newtype to represent a User or maintainer
pub struct User(String);
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
+/// Original version of the tickets on disk. This exists for historical reasons
+/// but is deprecated and likely to be removed.
pub struct TicketV0 {
title: String,
status: Status,
@@ -317,8 +333,11 @@ pub struct TicketV0 {
description: String,
}
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
+/// What is the current state of a ticket
pub enum Status {
+ /// The ticket has been opened but the issue has not been resolved
Open,
+ /// The ticket has a corresponding fix and has been closed
Closed,
}
diff --git a/ticket/src/tui.rs b/ticket/src/tui.rs
index e1aa666..01f00bf 100644
--- a/ticket/src/tui.rs
+++ b/ticket/src/tui.rs
@@ -130,8 +130,8 @@ pub struct Config {
}
impl Default for Config {
- fn default() -> Config {
- Config {
+ fn default() -> Self {
+ Self {
exit_key: Key::Char('q'),
tick_rate: Duration::from_millis(250),
}
@@ -139,11 +139,11 @@ impl Default for Config {
}
impl Events {
- pub fn new() -> Events {
- Events::with_config(Config::default())
+ pub fn new() -> Self {
+ Self::with_config(Config::default())
}
- pub fn with_config(config: Config) -> Events {
+ pub fn with_config(config: Config) -> Self {
let (tx, rx) = mpsc::channel();
let input_handle = {
let tx = tx.clone();
@@ -171,7 +171,7 @@ impl Events {
}
})
};
- Events {
+ Self {
rx,
input_handle,
tick_handle,
@@ -198,8 +198,8 @@ pub fn run() -> Result<()> {
tabs: TabsState::new(vec!["Open", "Closed"]),
tickets: {
let mut map = BTreeMap::new();
- map.insert("Open".into(), get_open_tickets()?);
- map.insert("Closed".into(), get_closed_tickets()?);
+ let _ = map.insert("Open".into(), get_open_tickets()?);
+ let _ = map.insert("Closed".into(), get_closed_tickets()?);
TicketState::new(map)
},
};