Skip to content

Commit

Permalink
Address Clippy lints and update CI (#315)
Browse files Browse the repository at this point in the history
* apply clippy lints

* update CI to address MSRV
  • Loading branch information
jxs authored Mar 1, 2024
1 parent d4e9449 commit ce0a0d8
Show file tree
Hide file tree
Showing 14 changed files with 90 additions and 54 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cargo-publish.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
uses: actions/checkout@v3

- name: Setup toolchain
- uses: actions-rs/toolchain@v1
uses: actions-rs/toolchain@v1
with:
toolchain: stable

Expand Down
78 changes: 54 additions & 24 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ jobs:
name: CI is green
runs-on: ubuntu-latest
needs:
- cargo-fmt-clippy-and-test-macros-and-cli
- cargo-fmt-clippy
- test-macros-and-cli
- test-sqlite
- test-postgres
- test-tokio-postgres
Expand All @@ -17,35 +18,59 @@ jobs:
steps:
- run: exit 0

cargo-fmt-clippy-and-test-macros-and-cli:
name: Cargo fmt/clippy/test-macros-and-cli
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [windows-latest, ubuntu-latest]
rust: [stable, nightly, 1.56.0]
set-rust-versions:
runs-on: ubuntu-latest
outputs:
versions: ${{ steps.set-versions.outputs.versions }}
steps:
- name: checkout repo
uses: actions/checkout@v2
- id: set-versions
run: |
MSRV=$(grep -oP 'rust-version\s*=\s*"\K[^"]+' ./refinery/Cargo.toml)
echo "versions=['stable', 'nightly', '$MSRV']" >> $GITHUB_OUTPUT
cargo-fmt-clippy:
name: Cargo fmt and clippy
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
toolchain: ${{ matrix.rust }}
toolchain: beta
- run: rustup self update
- run: rustup component add clippy
- run: rustup component add rustfmt
- run: cargo fmt --all -- --check
- run: cargo clippy --all-targets --all-features

test-macros-and-cli:
name: test-macros-and-cli
needs: set-rust-versions
strategy:
matrix:
rust: ${{ fromJson(needs.set-rust-versions.outputs.versions) }}
os: [windows-latest, ubuntu-latest]

runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
toolchain: ${{ matrix.rust }}
- run: rustup self update
- run: cd refinery_core && cargo test --all-features -- --test-threads 1
- run: cd refinery && cargo build --all-features
- run: cd refinery_macros && cargo clippy
- run: cd refinery_cli && cargo clippy
- run: cd refinery_macros && cargo test
- run: cd refinery_cli && cargo test

test-sqlite:
name: Test Sqlite
needs: set-rust-versions
runs-on: ubuntu-latest
strategy:
matrix:
rust: [stable, nightly, 1.65.0]
matrix:
rust: ${{ fromJson(needs.set-rust-versions.outputs.versions) }}
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
Expand All @@ -56,10 +81,11 @@ jobs:

test-postgres:
name: Test postgres
needs: set-rust-versions
runs-on: ubuntu-latest
strategy:
matrix:
rust: [stable, nightly, 1.56.0]
matrix:
rust: ${{ fromJson(needs.set-rust-versions.outputs.versions) }}
services:
postgres:
image: postgres:9.6.13-alpine
Expand All @@ -75,10 +101,11 @@ jobs:

test-tokio-postgres:
name: Test tokio-postgres
runs-on: ubuntu-latest
needs: set-rust-versions
strategy:
matrix:
rust: [stable, nightly, 1.56.0]
matrix:
rust: ${{ fromJson(needs.set-rust-versions.outputs.versions) }}
runs-on: ubuntu-latest
services:
postgres:
image: postgres:9.6.13-alpine
Expand All @@ -93,10 +120,11 @@ jobs:

test-mysql:
name: Test mysql
needs: set-rust-versions
runs-on: ubuntu-latest
strategy:
matrix:
rust: [stable, nightly, 1.56.0]
matrix:
rust: ${{ fromJson(needs.set-rust-versions.outputs.versions) }}
services:
postgres:
image: mysql:latest
Expand All @@ -117,10 +145,11 @@ jobs:

test-mysql-async:
name: Test mysql-async
needs: set-rust-versions
runs-on: ubuntu-latest
strategy:
matrix:
rust: [stable, nightly, 1.56.0]
matrix:
rust: ${{ fromJson(needs.set-rust-versions.outputs.versions) }}
services:
postgres:
image: mysql:latest
Expand All @@ -140,10 +169,11 @@ jobs:

test-tiberius:
name: Test tiberius
needs: set-rust-versions
runs-on: ubuntu-latest
strategy:
matrix:
rust: [stable, nightly, 1.56.0]
matrix:
rust: ${{ fromJson(needs.set-rust-versions.outputs.versions) }}
services:
mssql:
image: mcr.microsoft.com/mssql/server:2017-latest
Expand Down
2 changes: 1 addition & 1 deletion refinery/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "refinery"
version = "0.8.12"
rust-version = "1.56"
rust-version = "1.75"
authors = ["Katharina Fey <kookie@spacekookie.de>", "João Oliveira <hello@jxs.pt>"]
license = "MIT"
description = "Powerful SQL migration toolkit for Rust"
Expand Down
4 changes: 2 additions & 2 deletions refinery/tests/mysql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ mod mysql {
where
T: FnOnce() + std::panic::UnwindSafe,
{
let result = std::panic::catch_unwind(|| test());
let result = std::panic::catch_unwind(test);

clean_database();

Expand Down Expand Up @@ -767,7 +767,7 @@ mod mysql {
// cli only finds .sql migration files
run_test(|| {
Command::new("refinery")
.args(&[
.args([
"migrate",
"-c",
"tests/mysql_refinery.toml",
Expand Down
4 changes: 2 additions & 2 deletions refinery/tests/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ mod postgres {
where
T: FnOnce() + std::panic::UnwindSafe,
{
let result = std::panic::catch_unwind(|| test());
let result = std::panic::catch_unwind(test);

clean_database();

Expand Down Expand Up @@ -748,7 +748,7 @@ mod postgres {
fn migrates_from_cli() {
run_test(|| {
Command::new("refinery")
.args(&[
.args([
"migrate",
"-c",
"tests/postgres_refinery.toml",
Expand Down
10 changes: 5 additions & 5 deletions refinery/tests/rusqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ mod rusqlite {
let filepath = "tests/db.sql";
File::create(filepath).unwrap();

let result = std::panic::catch_unwind(|| test());
let result = std::panic::catch_unwind(test);

fs::remove_file(filepath).unwrap();

Expand Down Expand Up @@ -199,7 +199,7 @@ mod rusqlite {

conn.execute(
"INSERT INTO persons (name, city) VALUES (?, ?)",
&["John Legend", "New York"],
["John Legend", "New York"],
)
.unwrap();
let (name, city): (String, String) = conn
Expand All @@ -222,7 +222,7 @@ mod rusqlite {

conn.execute(
"INSERT INTO persons (name, city) VALUES (?, ?)",
&["John Legend", "New York"],
["John Legend", "New York"],
)
.unwrap();
let (name, city): (String, String) = conn
Expand All @@ -245,7 +245,7 @@ mod rusqlite {

conn.execute(
"INSERT INTO persons (name, city) VALUES (?, ?)",
&["John Legend", "New York"],
["John Legend", "New York"],
)
.unwrap();
let (name, city): (String, String) = conn
Expand Down Expand Up @@ -831,7 +831,7 @@ mod rusqlite {
fn migrates_from_cli() {
run_test(|| {
Command::new("refinery")
.args(&[
.args([
"migrate",
"-c",
"tests/sqlite_refinery.toml",
Expand Down
6 changes: 3 additions & 3 deletions refinery/tests/tiberius.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use barrel::backend::MsSql as Sql;

#[cfg(all(feature = "tiberius-config"))]
#[cfg(feature = "tiberius-config")]
mod tiberius {
use assert_cmd::prelude::*;
use futures::FutureExt;
Expand All @@ -16,7 +16,7 @@ mod tiberius {
use time::OffsetDateTime;
use tokio_util::compat::TokioAsyncWriteCompatExt;

const CONFIG: &'static str = "mssql://SA:Passw0rd@localhost:1433/refinery_test?trust_cert=true";
const CONFIG: &str = "mssql://SA:Passw0rd@localhost:1433/refinery_test?trust_cert=true";
const DEFAULT_TABLE_NAME: &str = "refinery_schema_history";

fn get_migrations() -> Vec<Migration> {
Expand Down Expand Up @@ -1076,7 +1076,7 @@ mod tiberius {
async fn migrates_from_cli() {
run_test(async {
Command::new("refinery")
.args(&[
.args([
"migrate",
"-c",
"tests/tiberius_refinery.toml",
Expand Down
1 change: 1 addition & 0 deletions refinery_cli/src/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub fn handle_migration_command(args: MigrateArgs) -> anyhow::Result<()> {
Ok(())
}

#[allow(clippy::too_many_arguments)]
fn run_migrations(
config_location: &Path,
grouped: bool,
Expand Down
4 changes: 2 additions & 2 deletions refinery_cli/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ mod cli {
fn cli_version() {
Command::cargo_bin("refinery")
.unwrap()
.args(&["-V"])
.args(["-V"])
.assert()
.stdout(contains(env!("CARGO_PKG_VERSION")));
}
Expand All @@ -23,7 +23,7 @@ mod cli {
fn migrate_no_args() {
Command::cargo_bin("refinery")
.unwrap()
.args(&["migrate"])
.args(["migrate"])
.assert()
.failure();
}
Expand Down
11 changes: 5 additions & 6 deletions refinery_core/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::error::Kind;
use crate::Error;
use std::convert::TryFrom;
use std::fs;
use std::path::{Path, PathBuf};
use std::path::PathBuf;
use std::str::FromStr;
use url::Url;

Expand Down Expand Up @@ -54,7 +53,7 @@ impl Config {

/// create a new Config instance from a config file located on the file system
#[cfg(feature = "toml")]
pub fn from_file_location<T: AsRef<Path>>(location: T) -> Result<Config, Error> {
pub fn from_file_location<T: AsRef<std::path::Path>>(location: T) -> Result<Config, Error> {
let file = std::fs::read_to_string(&location).map_err(|err| {
Error::new(
Kind::ConfigError(format!("could not open config file, {}", err)),
Expand Down Expand Up @@ -86,7 +85,7 @@ impl Config {
.unwrap_or(&std::env::current_dir().unwrap())
.to_path_buf();

config_db_dir = fs::canonicalize(config_db_dir).unwrap();
config_db_dir = std::fs::canonicalize(config_db_dir).unwrap();
config_db_path = config_db_dir.join(&config_db_path)
}

Expand All @@ -105,7 +104,7 @@ impl Config {

cfg_if::cfg_if! {
if #[cfg(feature = "rusqlite")] {
pub(crate) fn db_path(&self) -> Option<&Path> {
pub(crate) fn db_path(&self) -> Option<&std::path::Path> {
self.main.db_path.as_deref()
}

Expand Down Expand Up @@ -338,7 +337,7 @@ cfg_if::cfg_if! {
if config.main.trust_cert {
tconfig.trust_cert();
}
tconfig.authentication(AuthMethod::sql_server(&user, &pass));
tconfig.authentication(AuthMethod::sql_server(user, pass));

Ok(tconfig)
}
Expand Down
3 changes: 3 additions & 0 deletions refinery_core/src/drivers/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ impl AsyncQuery<Vec<Migration>> for Config {
}
// this is written as macro so that we don't have to deal with type signatures
#[cfg(any(feature = "mysql", feature = "postgres", feature = "rusqlite"))]
#[allow(clippy::redundant_closure_call)]
macro_rules! with_connection {
($config:ident, $op: expr) => {
#[allow(clippy::redundant_closure_call)]
match $config.db_type() {
ConfigDbType::Mysql => {
cfg_if::cfg_if! {
Expand Down Expand Up @@ -101,6 +103,7 @@ macro_rules! with_connection {
))]
macro_rules! with_connection_async {
($config: ident, $op: expr) => {
#[allow(clippy::redundant_closure_call)]
match $config.db_type() {
ConfigDbType::Mysql => {
cfg_if::cfg_if! {
Expand Down
2 changes: 1 addition & 1 deletion refinery_core/src/drivers/tokio_postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl AsyncTransaction for Client {
let transaction = self.transaction().await?;
let mut count = 0;
for query in queries {
transaction.batch_execute(*query).await?;
transaction.batch_execute(query).await?;
count += 1;
}
transaction.commit().await?;
Expand Down
Loading

0 comments on commit ce0a0d8

Please sign in to comment.