Merge pull request #5 from openlawlibrary/tombh/rustism-first-review

Rustisms: part 1

.github/workflows/ci.yml

@@ -29,7 +29,7 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.STELE_GITHUB_TOKEN }}
 
-      - name: Run just test
+      - name: Run tests
         run: just test
 
   lints:

Cargo.toml

@@ -8,6 +8,7 @@ license = "AGPL-3.0"
 keywords = ["authentication", "laws", "preservation"]
 categories = ["authentication", "web-programming::http-server"]
 repository = "https://github.com/openlawlibrary/stele"
+rust-version = "1.66"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
@@ -19,6 +20,10 @@ git2 = "0.15"
 lazy_static = "1.4.0"
 regex = "1"
 serde = "1.0"
+tracing = "0.1.37"
+tracing-subscriber = "0.3.16"
+tracing-actix-web = "0.6.2"
+derive_more = "0.99.17"
 
 [dev-dependencies]
 criterion = "0.3"

README.md

@@ -18,7 +18,6 @@ Stele is a system for distributing, preserving, and authenticating laws.
 6. We recommend using [VSCode](https://code.visualstudio.com/Download) (default settings provided in repo), but you can use any editor you like.
 
 ### Development
-
 - Lints must pass before merging into master
 - All code must have tests. Tests should conform to our testing guidelines.
 - Run `just` from within the repository to list all available just commands. Currently:
@@ -31,6 +30,11 @@ Stele is a system for distributing, preserving, and authenticating laws.
     - `test`: Run all tests
 - On windows, especially, you may wish to run just through the nu shell, which can be done by calling all commands with the `--shell` command, e.g. `just --shell nu lint`.
 
+## Logging
+
+The ENV variable `RUST_LOG` can be set with one of `trace`, `debug`, `info`, `warn`, `error`. Filters can be set based on the `target` components seen in the logs lines, for example: to use `trace` but turn down the noise from the Actix dispatcher: `RUST_LOG="trace,actix_http::h1::dispatcher=warn"`
+
+See [tracing-subscriber docs](https://docs.rs/tracing-subscriber/latest/tracing_subscriber/fmt/index.html#filtering-events-with-environment-variables) and [env_logger syntax](https://docs.rs/env_logger/latest/env_logger/#enabling-logging]).
 
 ## Q&A
 - Why do we suggest NuShell?

benches/git_benchmark.rs

@@ -1,34 +1,20 @@
 //! benchmark for git utils
 #![allow(clippy::self_named_module_files)]
-#![allow(clippy::std_instead_of_alloc)]
 #![allow(clippy::implicit_return)]
-#![allow(clippy::multiple_crate_versions)]
 #![allow(clippy::expect_used)]
 #![allow(missing_docs)]
 
 use criterion::{criterion_group, criterion_main, Criterion};
-use std::env::current_exe;
 use std::fs::create_dir_all;
 use std::path::PathBuf;
 use std::sync::Once;
 use stele::utils::git::Repo;
 
-/// get the path to the test library at `$REPO_ROOT/test/library`.
+/// get the path to the test library at `$REPO_ROOT/tests/fixtures/library`.
 fn get_test_library_path() -> PathBuf {
-    let mut library_path = current_exe()
-        .expect("Something went wrong getting the library path")
-        .parent()
-        .expect("Something went wrong getting the library path")
-        .parent()
-        .expect("Something went wrong getting the library path")
-        .parent()
-        .expect("Something went wrong getting the library path")
-        .parent()
-        .expect("Something went wrong getting the library path")
-        .to_owned();
-    library_path.push("test");
-    library_path.push("library");
-    library_path
+    let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    path.push("tests/fixtures/library");
+    path
 }
 
 /// ensure `initialize` function, below, is only called once

justfile

@@ -9,7 +9,7 @@ lint: format clippy
 
 # Format code
 format:
-	cargo fmt --all -- --check
+  cargo fmt --all -- --check
 
 # Run all tests
 test:
@@ -27,4 +27,4 @@ ci: lint test bench
 
 # Run all benchmarks
 bench:
-  cargo bench
+  cargo bench

src/lib.rs

@@ -9,11 +9,79 @@
 //! publish the law. The Code of Hammurabi, one of the earliest preserved
 //! written laws, was published on a Stele in ~1750 BCE and is still readable
 //! nearly four millennia later.
-#![allow(clippy::self_named_module_files)]
-#![allow(clippy::std_instead_of_alloc)]
-#![allow(clippy::implicit_return)]
-#![allow(clippy::multiple_crate_versions)]
-#![allow(clippy::std_instead_of_core)]
+
+// =========================================================================
+//                  Canonical lints for whole crate
+// =========================================================================
+// Official docs:
+//   https://doc.rust-lang.org/nightly/clippy/lints.html
+// Useful app to lookup full details of individual lints:
+//   https://rust-lang.github.io/rust-clippy/master/index.html
+//
+// We set base lints to give the fullest, most pedantic feedback possible.
+// Though we prefer that they are just warnings during development so that build-denial
+// is only enforced in CI.
+//
+#![warn(
+    // `clippy::all` is already on by default. It implies the following:
+    //   clippy::correctness code that is outright wrong or useless
+    //   clippy::suspicious code that is most likely wrong or useless
+    //   clippy::complexity code that does something simple but in a complex way
+    //   clippy::perf code that can be written to run faster
+    //   clippy::style code that should be written in a more idiomatic way
+    clippy::all,
+
+    // It's always good to write as much documentation as possible
+    missing_docs,
+
+    // > clippy::pedantic lints which are rather strict or might have false positives
+    clippy::pedantic,
+
+    // > new lints that are still under development"
+    // (so "nursery" doesn't mean "Rust newbies")
+    clippy::nursery,
+
+    // > The clippy::cargo group gives you suggestions on how to improve your Cargo.toml file.
+    // > This might be especially interesting if you want to publish your crate and are not sure
+    // > if you have all useful information in your Cargo.toml.
+    clippy::cargo
+)]
+// > The clippy::restriction group will restrict you in some way.
+// > If you enable a restriction lint for your crate it is recommended to also fix code that
+// > this lint triggers on. However, those lints are really strict by design and you might want
+// > to #[allow] them in some special cases, with a comment justifying that.
+#![allow(clippy::blanket_clippy_restriction_lints)]
+#![warn(clippy::restriction)]
+//
+//
+// =========================================================================
+//   Individually blanket-allow single lints relevant to this whole crate
+// =========================================================================
+#![allow(
+    // This is idiomatic Rust
+    clippy::implicit_return,
+
+    // Multiple deps are currently pinning `hermit-abi` — December 2022
+    clippy::multiple_crate_versions,
+
+    // We're not interested in becoming no-std compatible
+    clippy::std_instead_of_alloc,
+    clippy::std_instead_of_core,
+
+    // TODO: But I think the mod.rs is more conventional — @tombh
+    clippy::mod_module_files,
+
+    // Although performance is of course important for this application, it is not currently
+    // such that it would benefit from explicit inline suggestions. Besides, not specifying
+    // `#[inline]` doesn't mean that a function won't be inlined. And if performance does start
+    // to become a problem, there are other avenues to explore before deciding on which functions
+    // would benefit from explicit inlining.
+    clippy::missing_inline_in_public_items,
+
+    // I think marking `#[non_exhaustive]` is more for structs/enums that are imported into other crates
+    clippy::exhaustive_structs,
+    clippy::exhaustive_enums
+)]
 
 pub mod server;
 pub mod utils;