Skip to content

Commit

Permalink
Run Rustfmt, resolve Clippy lints, use 2018 edition (#31)
Browse files Browse the repository at this point in the history
I do Rust as part of my living, and I wanted to update this codebase to 
match with the standard way Rust repositories are managed. This PR is 
mostly running Rustfmt and fixing Clippy errors. These are both 
invaluable tools I highly recommend you try out if you haven't already. 
Rustfmt is an autoformatter that not only makes it so there's no 
conflicting styles, but also makes it really easy to do big refactors 
without having to care about style too much. VS Code has an option to 
run a formatter when you save, and if you're using a Rust LSP (RLS or 
the newer rust-analyzer), then it helps productivity a lot. Clippy is a 
very good linter that finds issues that the normal rustc doesn't catch. 
For this repo, it found some style issues and a minor performance issue 
that were easily fixed up.

A lot of the code is inside macros, which clippy and Rustfmt can't 
introspect very well, but this covers most of it.

- Runs cargo fmt*
- Fixes error brought up by cargo clippy*
- Upgrades to Rust 2018, which is regularly updated
- Change `const _: () = "bla"` to `compile_error!("bla")`
- Change `use` to be use the idiomatic grouped syntax

* - I highly recommend setting up a GitHub action to do these.
  • Loading branch information
Mothblocks authored Jun 5, 2020
1 parent 43e14ac commit 66ce0fe
Show file tree
Hide file tree
Showing 17 changed files with 186 additions and 140 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[package]
name = "rust-g"
edition = "2018"
version = "0.4.5"
authors = ["Bjorn Neergaard <[email protected]>"]
repository = "https://github.com/tgstation/rust-g"
Expand Down
Empty file added Rustfmt.toml
Empty file.
45 changes: 32 additions & 13 deletions build.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
//! Buildscript which will save a `rust_g.dm` with the DLL's public API.
use std::io::Write;
use std::fs::File;
use std::{fs::File, io::Write};

macro_rules! enabled {
($name:expr) => {
std::env::var(concat!("CARGO_FEATURE_", $name)).is_ok()
}
};
}

fn main() {
let mut f = File::create("target/rust_g.dm").unwrap();

// header
write!(f, r#"// rust_g.dm - DM API for rust_g extension library
write!(
f,
r#"// rust_g.dm - DM API for rust_g extension library
//
// To configure, create a `rust_g.config.dm` and set what you care about from
// the following options:
Expand Down Expand Up @@ -56,7 +57,9 @@ fn main() {
#define RUSTG_JOB_NO_RESULTS_YET "NO RESULTS YET"
#define RUSTG_JOB_NO_SUCH_JOB "NO SUCH JOB"
#define RUSTG_JOB_ERROR "JOB PANICKED"
"#).unwrap();
"#
)
.unwrap();

// module: dmi
if enabled!("DMI") {
Expand All @@ -75,7 +78,9 @@ fn main() {

// module: file
if enabled!("FILE") {
write!(f, r#"
write!(
f,
r#"
#define rustg_file_read(fname) call(RUST_G, "file_read")(fname)
#define rustg_file_write(text, fname) call(RUST_G, "file_write")(text, fname)
#define rustg_file_append(text, fname) call(RUST_G, "file_append")(text, fname)
Expand All @@ -84,15 +89,21 @@ fn main() {
#define file2text(fname) rustg_file_read(fname)
#define text2file(text, fname) rustg_file_append(text, fname)
#endif
"#).unwrap();
"#
)
.unwrap();
}

// module: git
if enabled!("GIT") {
write!(f, r#"
write!(
f,
r#"
#define rustg_git_revparse(rev) call(RUST_G, "rg_git_revparse")(rev)
#define rustg_git_commit_date(rev) call(RUST_G, "rg_git_commit_date")(rev)
"#).unwrap();
"#
)
.unwrap();
}

// module: hash
Expand All @@ -114,23 +125,31 @@ fn main() {

// module: log
if enabled!("LOG") {
write!(f, r#"
write!(
f,
r#"
#define rustg_log_write(fname, text, format) call(RUST_G, "log_write")(fname, text, format)
/proc/rustg_log_close_all() return call(RUST_G, "log_close_all")()
"#).unwrap();
"#
)
.unwrap();
}

// module: url
if enabled!("URL") {
write!(f, r#"
write!(
f,
r#"
#define rustg_url_encode(text) call(RUST_G, "url_encode")(text)
#define rustg_url_decode(text) call(RUST_G, "url_decode")(text)
#ifdef RUSTG_OVERRIDE_BUILTINS
#define url_encode(text) rustg_url_encode(text)
#define url_decode(text) rustg_url_decode(text)
#endif
"#).unwrap();
"#
)
.unwrap();
}

// module: http
Expand Down
27 changes: 16 additions & 11 deletions src/byond.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
use std::borrow::Cow;
use std::cell::RefCell;
use std::ffi::{CStr, CString};
use std::slice;

use std::os::raw::{c_char, c_int};
use std::{
borrow::Cow,
cell::RefCell,
ffi::{CStr, CString},
os::raw::{c_char, c_int},
slice,
};

static EMPTY_STRING: c_char = 0;
thread_local! {
static RETURN_STRING: RefCell<CString> = RefCell::new(CString::default());
}

pub fn parse_args<'a>(argc: c_int, argv: &'a *const *const c_char) -> Vec<Cow<'a, str>> {
pub fn parse_args<'a>(argc: c_int, argv: *const *const c_char) -> Vec<Cow<'a, str>> {
unsafe {
slice::from_raw_parts(*argv, argc as usize)
.iter()
.map(|ptr| CStr::from_ptr(*ptr))
.map(|ptr| CStr::from_ptr(ptr))
.map(|cstr| cstr.to_string_lossy())
.collect()
}
Expand Down Expand Up @@ -45,19 +46,22 @@ pub fn byond_return(value: Option<Vec<u8>>) -> *const c_char {
macro_rules! byond_fn {
($name:ident() $body:block) => {
#[no_mangle]
#[allow(clippy::missing_safety_doc)]
pub unsafe extern "C" fn $name(
_argc: ::std::os::raw::c_int, _argv: *const *const ::std::os::raw::c_char
) -> *const ::std::os::raw::c_char {
$crate::byond::byond_return((|| $body)().map(From::from))
let closure = || ($body);
$crate::byond::byond_return(closure().map(From::from))
}
};

($name:ident($($arg:ident),* $(, ...$rest:ident)?) $body:block) => {
#[no_mangle]
#[allow(clippy::missing_safety_doc)]
pub unsafe extern "C" fn $name(
_argc: ::std::os::raw::c_int, _argv: *const *const ::std::os::raw::c_char
) -> *const ::std::os::raw::c_char {
let __args = $crate::byond::parse_args(_argc, &_argv);
let __args = $crate::byond::parse_args(_argc, _argv);

let mut __argn = 0;
$(
Expand All @@ -68,7 +72,8 @@ macro_rules! byond_fn {
let $rest = __args.get(__argn..).unwrap_or(&[]);
)?

$crate::byond::byond_return((|| $body)().map(From::from))
let closure = || ($body);
$crate::byond::byond_return(closure().map(From::from))
}
};
}
9 changes: 5 additions & 4 deletions src/dmi.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use std::fs::{File, create_dir_all};
use std::path::Path;
use crate::error::{Error, Result};
use png::{Decoder, Encoder, HasParameters, OutputInfo};

use error::{Result, Error};
use std::{
fs::{create_dir_all, File},
path::Path,
};

byond_fn! { dmi_strip_metadata(path) {
strip_metadata(path).err()
Expand Down
30 changes: 16 additions & 14 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use std::io;
use std::result;
use std::str::Utf8Error;
use std::num::{ParseIntError, ParseFloatError};
use std::{
io,
num::{ParseFloatError, ParseIntError},
result,
str::Utf8Error,
};

#[cfg(feature="png")]
#[cfg(feature = "png")]
use png::{DecodingError, EncodingError};

pub type Result<T> = result::Result<T, Error>;
Expand All @@ -20,23 +22,23 @@ pub enum Error {
Io(#[cause] io::Error),
#[fail(display = "Invalid algorithm specified.")]
InvalidAlgorithm,
#[cfg(feature="png")]
#[cfg(feature = "png")]
#[fail(display = "{}", _0)]
ImageDecoding(#[cause] DecodingError),
#[cfg(feature="png")]
#[cfg(feature = "png")]
#[fail(display = "{}", _0)]
ImageEncoding(#[cause] EncodingError),
#[fail(display = "{}", _0)]
ParseIntError(#[cause] ParseIntError),
#[fail(display = "{}", _0)]
ParseFloatError(#[cause] ParseFloatError),
#[cfg(feature="png")]
#[cfg(feature = "png")]
#[fail(display = "Invalid png data.")]
InvalidPngDataError,
#[cfg(feature="http")]
#[cfg(feature = "http")]
#[fail(display = "{}", _0)]
RequestError(#[cause] reqwest::Error),
#[cfg(feature="http")]
#[cfg(feature = "http")]
#[fail(display = "{}", _0)]
SerializationError(#[cause] serde_json::Error),
}
Expand All @@ -53,14 +55,14 @@ impl From<Utf8Error> for Error {
}
}

#[cfg(feature="png")]
#[cfg(feature = "png")]
impl From<DecodingError> for Error {
fn from(error: DecodingError) -> Error {
Error::ImageDecoding(error)
}
}

#[cfg(feature="png")]
#[cfg(feature = "png")]
impl From<EncodingError> for Error {
fn from(error: EncodingError) -> Error {
Error::ImageEncoding(error)
Expand All @@ -78,14 +80,14 @@ impl From<ParseFloatError> for Error {
Error::ParseFloatError(error)
}
}
#[cfg(feature="http")]
#[cfg(feature = "http")]
impl From<reqwest::Error> for Error {
fn from(error: reqwest::Error) -> Error {
Error::RequestError(error)
}
}

#[cfg(feature="http")]
#[cfg(feature = "http")]
impl From<serde_json::Error> for Error {
fn from(error: serde_json::Error) -> Error {
Error::SerializationError(error)
Expand Down
10 changes: 5 additions & 5 deletions src/file.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::fs::File;
use std::fs::OpenOptions;
use std::io::{Read, Write};

use error::Result;
use crate::error::Result;
use std::{
fs::{File, OpenOptions},
io::{Read, Write},
};

byond_fn! { file_read(path) {
read(path).ok()
Expand Down
4 changes: 2 additions & 2 deletions src/git.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use git2::{Repository, Error, ErrorCode};
use chrono::{Utc, TimeZone};
use chrono::{TimeZone, Utc};
use git2::{Error, ErrorCode, Repository};

thread_local! {
static REPOSITORY: Result<Repository, Error> = Repository::open(".");
Expand Down
9 changes: 2 additions & 7 deletions src/hash.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
use std::fs::File;
use std::io;

use crypto_hash;
use crate::error::{Error, Result};
use crypto_hash::{Algorithm, Hasher};
use hex;

use error::{Error, Result};
use std::{fs::File, io};

byond_fn! { hash_string(algorithm, string) {
string_hash(algorithm, string).ok()
Expand Down
35 changes: 22 additions & 13 deletions src/http.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use std::collections::hash_map::{ HashMap };
use std::collections::BTreeMap;

use error::Result;
use jobs;
use crate::{error::Result, jobs};
use std::collections::{BTreeMap, HashMap};

// ----------------------------------------------------------------------------
// Interface
Expand Down Expand Up @@ -62,15 +59,18 @@ const VERSION: &str = env!("CARGO_PKG_VERSION");
const PKG_NAME: &str = env!("CARGO_PKG_NAME");

fn setup_http_client() -> reqwest::Client {
use reqwest::{ Client, header::{ HeaderMap, USER_AGENT } };
use reqwest::{
header::{HeaderMap, USER_AGENT},
Client,
};

let mut headers = HeaderMap::new();
headers.insert(USER_AGENT, format!("{}/{}", PKG_NAME, VERSION).parse().unwrap());
headers.insert(
USER_AGENT,
format!("{}/{}", PKG_NAME, VERSION).parse().unwrap(),
);

Client::builder()
.default_headers(headers)
.build()
.unwrap()
Client::builder().default_headers(headers).build().unwrap()
}

lazy_static! {
Expand All @@ -85,7 +85,13 @@ struct RequestPrep {
output_filename: Option<String>,
}

fn construct_request(method: &str, url: &str, body: &str, headers: &str, options: Option<&str>) -> Result<RequestPrep> {
fn construct_request(
method: &str,
url: &str,
body: &str,
headers: &str,
options: Option<&str>,
) -> Result<RequestPrep> {
let mut req = match method {
"post" => HTTP_CLIENT.post(url),
"put" => HTTP_CLIENT.put(url),
Expand Down Expand Up @@ -115,7 +121,10 @@ fn construct_request(method: &str, url: &str, body: &str, headers: &str, options
}
}

Ok(RequestPrep { req, output_filename })
Ok(RequestPrep {
req,
output_filename,
})
}

fn submit_request(prep: RequestPrep) -> Result<String> {
Expand Down
10 changes: 6 additions & 4 deletions src/jobs.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
//! Job system
use std::sync::mpsc;
use std::thread;
use std::collections::hash_map::{HashMap, Entry};
use std::cell::RefCell;
use std::{
cell::RefCell,
collections::hash_map::{Entry, HashMap},
sync::mpsc,
thread,
};

struct Job {
rx: mpsc::Receiver<Output>,
Expand Down
Loading

0 comments on commit 66ce0fe

Please sign in to comment.