Skip to content

Fix clippy warnings and add clippy to CI #696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Apr 3, 2020
Merged
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ jobs:
- name: Build docs.rs
run: cargo build --locked

- name: Run clippy
run: cargo clippy -- -D warnings

- name: Prepare the test environment
run: |
prefix=$(pwd)/ignored/prefix
Expand Down
3 changes: 2 additions & 1 deletion build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ fn main() {


fn write_git_version() {
let git_hash = get_git_hash().unwrap_or("???????".to_owned());
let maybe_hash = get_git_hash();
let git_hash = maybe_hash.as_deref().unwrap_or("???????");
let build_date = time::strftime("%Y-%m-%d", &time::now_utc()).unwrap();
let dest_path = Path::new(&env::var("OUT_DIR").unwrap()).join("git_version");
let mut file = File::create(&dest_path).unwrap();
Expand Down
24 changes: 12 additions & 12 deletions src/bin/cratesfyi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use cratesfyi::Server;
use cratesfyi::db::{add_path_into_database, connect_db};


#[allow(clippy::cognitive_complexity)]
pub fn main() {
logger_init();

Expand Down Expand Up @@ -169,7 +170,7 @@ pub fn main() {

let mut docbuilder = DocBuilder::new(docbuilder_opts);

if let Some(_) = matches.subcommand_matches("world") {
if matches.subcommand_matches("world").is_some() {
docbuilder.load_cache().expect("Failed to load cache");
let mut builder = RustwideBuilder::init().unwrap();
builder.build_world(&mut docbuilder).expect("Failed to build world");
Expand Down Expand Up @@ -197,11 +198,11 @@ pub fn main() {
} else if matches.subcommand_matches("add-essential-files").is_some() {
let mut builder = RustwideBuilder::init().unwrap();
builder.add_essential_files().expect("failed to add essential files");
} else if let Some(_) = matches.subcommand_matches("lock") {
} else if matches.subcommand_matches("lock").is_some() {
docbuilder.lock().expect("Failed to lock");
} else if let Some(_) = matches.subcommand_matches("unlock") {
} else if matches.subcommand_matches("unlock").is_some() {
docbuilder.unlock().expect("Failed to unlock");
} else if let Some(_) = matches.subcommand_matches("print-options") {
} else if matches.subcommand_matches("print-options").is_some() {
println!("{:?}", docbuilder.options());
}

Expand All @@ -211,20 +212,20 @@ pub fn main() {
.expect("Version should be an integer"));
db::migrate(version, &connect_db().expect("failed to connect to the database"))
.expect("Failed to run database migrations");
} else if let Some(_) = matches.subcommand_matches("update-github-fields") {
} else if matches.subcommand_matches("update-github-fields").is_some() {
cratesfyi::utils::github_updater().expect("Failed to update github fields");
} else if let Some(matches) = matches.subcommand_matches("add-directory") {
add_path_into_database(&db::connect_db().unwrap(),
matches.value_of("PREFIX").unwrap_or(""),
matches.value_of("DIRECTORY").unwrap())
.expect("Failed to add directory into database");
} else if let Some(_) = matches.subcommand_matches("update-release-activity") {
} else if matches.subcommand_matches("update-release-activity").is_some() {
// FIXME: This is actually util command not database
cratesfyi::utils::update_release_activity().expect("Failed to update release activity");
} else if let Some(_) = matches.subcommand_matches("update-search-index") {
} else if matches.subcommand_matches("update-search-index").is_some() {
let conn = db::connect_db().unwrap();
db::update_search_index(&conn).expect("Failed to update search index");
} else if let Some(_) = matches.subcommand_matches("move-to-s3") {
} else if matches.subcommand_matches("move-to-s3").is_some() {
let conn = db::connect_db().unwrap();
let mut count = 1;
let mut total = 0;
Expand All @@ -243,7 +244,7 @@ pub fn main() {
} else if let Some(matches) = matches.subcommand_matches("blacklist") {
let conn = db::connect_db().expect("failed to connect to the database");

if let Some(_) = matches.subcommand_matches("list") {
if matches.subcommand_matches("list").is_some() {
let crates = db::blacklist::list_crates(&conn).expect("failed to list crates on blacklist");
println!("{}", crates.join("\n"));

Expand All @@ -260,7 +261,7 @@ pub fn main() {
} else if let Some(matches) = matches.subcommand_matches("start-web-server") {
Server::start(Some(matches.value_of("SOCKET_ADDR").unwrap_or("0.0.0.0:3000")));

} else if let Some(_) = matches.subcommand_matches("daemon") {
} else if matches.subcommand_matches("daemon").is_some() {
let foreground = matches.subcommand_matches("daemon")
.map_or(false, |opts| opts.is_present("FOREGROUND"));
cratesfyi::utils::start_daemon(!foreground);
Expand Down Expand Up @@ -292,8 +293,7 @@ fn logger_init() {
record.target(),
record.args())
});
builder.parse_filters(&env::var("RUST_LOG")
.unwrap_or("cratesfyi=info".to_owned()));
builder.parse_filters(env::var("RUST_LOG").ok().as_deref().unwrap_or("cratesfyi=info"));

rustwide::logging::init_with(builder.build());
}
47 changes: 24 additions & 23 deletions src/db/add_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use failure::err_msg;
///
/// NOTE: `source_files` refers to the files originally in the crate,
/// not the files generated by rustdoc.
#[allow(clippy::too_many_arguments)]
pub(crate) fn add_package_into_database(conn: &Connection,
metadata_pkg: &MetadataPackage,
source_dir: &Path,
Expand All @@ -45,9 +46,9 @@ pub(crate) fn add_package_into_database(conn: &Connection,

let release_id: i32 = {
let rows = conn.query("SELECT id FROM releases WHERE crate_id = $1 AND version = $2",
&[&crate_id, &format!("{}", metadata_pkg.version)])?;
&[&crate_id, &metadata_pkg.version.to_string()])?;

if rows.len() == 0 {
if rows.is_empty() {
let rows = conn.query("INSERT INTO releases (
crate_id, version, release_time,
dependencies, target_name, yanked, build_status,
Expand Down Expand Up @@ -117,7 +118,7 @@ pub(crate) fn add_package_into_database(conn: &Connection,
default_target = $25
WHERE crate_id = $1 AND version = $2",
&[&crate_id,
&format!("{}", metadata_pkg.version),
&metadata_pkg.version.to_string(),
&cratesio_data.release_time,
&dependencies.to_json(),
&metadata_pkg.package_name(),
Expand Down Expand Up @@ -146,9 +147,9 @@ pub(crate) fn add_package_into_database(conn: &Connection,
};


add_keywords_into_database(&conn, &metadata_pkg, &release_id)?;
add_authors_into_database(&conn, &metadata_pkg, &release_id)?;
add_owners_into_database(&conn, &cratesio_data.owners, &crate_id)?;
add_keywords_into_database(&conn, &metadata_pkg, release_id)?;
add_authors_into_database(&conn, &metadata_pkg, release_id)?;
add_owners_into_database(&conn, &cratesio_data.owners, crate_id)?;


// Update versions
Expand All @@ -167,7 +168,7 @@ pub(crate) fn add_package_into_database(conn: &Connection,
}
}
if !found {
versions_array.push(format!("{}", &metadata_pkg.version).to_json());
versions_array.push(metadata_pkg.version.to_string().to_json());
}
}
let _ = conn.query("UPDATE crates SET versions = $1 WHERE id = $2",
Expand All @@ -180,7 +181,7 @@ pub(crate) fn add_package_into_database(conn: &Connection,

/// Adds a build into database
pub(crate) fn add_build_into_database(conn: &Connection,
release_id: &i32,
release_id: i32,
res: &BuildResult)
-> Result<i32> {
debug!("Adding build into database");
Expand All @@ -189,7 +190,7 @@ pub(crate) fn add_build_into_database(conn: &Connection,
build_status, output)
VALUES ($1, $2, $3, $4, $5)
RETURNING id",
&[release_id,
&[&release_id,
&res.rustc_version,
&res.docsrs_version,
&res.successful,
Expand All @@ -201,7 +202,7 @@ pub(crate) fn add_build_into_database(conn: &Connection,
fn initialize_package_in_database(conn: &Connection, pkg: &MetadataPackage) -> Result<i32> {
let mut rows = conn.query("SELECT id FROM crates WHERE name = $1", &[&pkg.name])?;
// insert crate into database if it is not exists
if rows.len() == 0 {
if rows.is_empty() {
rows = conn.query("INSERT INTO crates (name) VALUES ($1) RETURNING id",
&[&pkg.name])?;
}
Expand All @@ -225,13 +226,13 @@ fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)>

/// Reads readme if there is any read defined in Cargo.toml of a Package
fn get_readme(pkg: &MetadataPackage, source_dir: &Path) -> Result<Option<String>> {
let readme_path = source_dir.join(pkg.readme.clone().unwrap_or("README.md".to_owned()));
let readme_path = source_dir.join(pkg.readme.as_deref().unwrap_or("README.md"));

if !readme_path.exists() {
return Ok(None);
}

let mut reader = fs::File::open(readme_path).map(|f| BufReader::new(f))?;
let mut reader = fs::File::open(readme_path).map(BufReader::new)?;
let mut readme = String::new();
reader.read_to_string(&mut readme)?;

Expand Down Expand Up @@ -262,7 +263,7 @@ fn get_rustdoc(pkg: &MetadataPackage, source_dir: &Path) -> Result<Option<String

/// Reads rustdoc from library
fn read_rust_doc(file_path: &Path) -> Result<Option<String>> {
let reader = fs::File::open(file_path).map(|f| BufReader::new(f))?;
let reader = fs::File::open(file_path).map(BufReader::new)?;
let mut rustdoc = String::new();

for line in reader.lines() {
Expand Down Expand Up @@ -356,17 +357,17 @@ fn get_release_time_yanked_downloads(
}
}

Ok((release_time.unwrap_or(time::get_time()), yanked.unwrap_or(false), downloads.unwrap_or(0)))
Ok((release_time.unwrap_or_else(time::get_time), yanked.unwrap_or(false), downloads.unwrap_or(0)))
}


/// Adds keywords into database
fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: &i32) -> Result<()> {
fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: i32) -> Result<()> {
for keyword in &pkg.keywords {
let slug = slugify(&keyword);
let keyword_id: i32 = {
let rows = conn.query("SELECT id FROM keywords WHERE slug = $1", &[&slug])?;
if rows.len() > 0 {
if !rows.is_empty() {
rows.get(0).get(0)
} else {
conn.query("INSERT INTO keywords (name, slug) VALUES ($1, $2) RETURNING id",
Expand All @@ -377,7 +378,7 @@ fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_
};
// add releationship
let _ = conn.query("INSERT INTO keyword_rels (rid, kid) VALUES ($1, $2)",
&[release_id, &keyword_id]);
&[&release_id, &keyword_id]);
}

Ok(())
Expand All @@ -386,7 +387,7 @@ fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_


/// Adds authors into database
fn add_authors_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: &i32) -> Result<()> {
fn add_authors_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: i32) -> Result<()> {

let author_capture_re = Regex::new("^([^><]+)<*(.*?)>*$").unwrap();
for author in &pkg.authors {
Expand All @@ -397,7 +398,7 @@ fn add_authors_into_database(conn: &Connection, pkg: &MetadataPackage, release_i

let author_id: i32 = {
let rows = conn.query("SELECT id FROM authors WHERE slug = $1", &[&slug])?;
if rows.len() > 0 {
if !rows.is_empty() {
rows.get(0).get(0)
} else {
conn.query("INSERT INTO authors (name, email, slug) VALUES ($1, $2, $3)
Expand All @@ -410,7 +411,7 @@ fn add_authors_into_database(conn: &Connection, pkg: &MetadataPackage, release_i

// add relationship
let _ = conn.query("INSERT INTO author_rels (rid, aid) VALUES ($1, $2)",
&[release_id, &author_id]);
&[&release_id, &author_id]);
}
}

Expand Down Expand Up @@ -479,11 +480,11 @@ fn get_owners(pkg: &MetadataPackage) -> Result<Vec<CrateOwner>> {
}

/// Adds owners into database
fn add_owners_into_database(conn: &Connection, owners: &[CrateOwner], crate_id: &i32) -> Result<()> {
fn add_owners_into_database(conn: &Connection, owners: &[CrateOwner], crate_id: i32) -> Result<()> {
for owner in owners {
let owner_id: i32 = {
let rows = conn.query("SELECT id FROM owners WHERE login = $1", &[&owner.login])?;
if rows.len() > 0 {
if !rows.is_empty() {
rows.get(0).get(0)
} else {
conn.query("INSERT INTO owners (login, avatar, name, email)
Expand All @@ -497,7 +498,7 @@ fn add_owners_into_database(conn: &Connection, owners: &[CrateOwner], crate_id:

// add relationship
let _ = conn.query("INSERT INTO owner_rels (cid, oid) VALUES ($1, $2)",
&[crate_id, &owner_id]);
&[&crate_id, &owner_id]);
}
Ok(())
}
2 changes: 1 addition & 1 deletion src/db/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub fn get_path(conn: &Connection, path: &str) -> Option<Blob> {
FROM files
WHERE path = $1", &[&path]).unwrap();

if rows.len() == 0 {
if rows.is_empty() {
None
} else {
let row = rows.get(0);
Expand Down
4 changes: 2 additions & 2 deletions src/docbuilder/crates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn crates_from_file<F>(path: &PathBuf, func: &mut F) -> Result<()>
where F: FnMut(&str, &str) -> ()
{

let reader = fs::File::open(path).map(|f| BufReader::new(f))?;
let reader = fs::File::open(path).map(BufReader::new)?;

let mut name = String::new();
let mut versions = Vec::new();
Expand Down Expand Up @@ -43,7 +43,7 @@ fn crates_from_file<F>(path: &PathBuf, func: &mut F) -> Result<()>

name.clear();
name.push_str(crate_name);
versions.push(format!("{}", vers));
versions.push(vers.to_string());
}

if !name.is_empty() {
Expand Down
2 changes: 1 addition & 1 deletion src/docbuilder/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ fn scale(value: usize, interval: usize, labels: &[&str]) -> String {
for label in &labels[1..] {
if value / interval >= 1.0 {
chosen_label = label;
value = value / interval;
value /= interval;
} else {
break;
}
Expand Down
6 changes: 3 additions & 3 deletions src/docbuilder/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ pub(super) struct BuildTargets<'a> {

impl Metadata {
pub(crate) fn from_source_dir(source_dir: &Path) -> Result<Metadata> {
for c in ["Cargo.toml.orig", "Cargo.toml"].iter() {
let manifest_path = source_dir.clone().join(c);
for &c in &["Cargo.toml.orig", "Cargo.toml"] {
let manifest_path = source_dir.join(c);
if manifest_path.exists() {
return Ok(Metadata::from_manifest(manifest_path));
}
Expand All @@ -94,7 +94,7 @@ impl Metadata {
Err(_) => return Metadata::default(),
};
let mut s = String::new();
if let Err(_) = f.read_to_string(&mut s) {
if f.read_to_string(&mut s).is_err() {
return Metadata::default();
}
Metadata::from_str(&s)
Expand Down
4 changes: 2 additions & 2 deletions src/docbuilder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub struct DocBuilder {
impl DocBuilder {
pub fn new(options: DocBuilderOptions) -> DocBuilder {
DocBuilder {
options: options,
options,
cache: BTreeSet::new(),
db_cache: BTreeSet::new(),
}
Expand All @@ -43,7 +43,7 @@ impl DocBuilder {
pub fn load_cache(&mut self) -> Result<()> {
debug!("Loading cache");
let path = PathBuf::from(&self.options.prefix).join("cache");
let reader = fs::File::open(path).map(|f| BufReader::new(f));
let reader = fs::File::open(path).map(BufReader::new);

if let Ok(reader) = reader {
for line in reader.lines() {
Expand Down
9 changes: 4 additions & 5 deletions src/docbuilder/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ impl Default for DocBuilderOptions {
generate_paths(cwd);

DocBuilderOptions {
prefix: prefix,
crates_io_index_path: crates_io_index_path,
prefix,
crates_io_index_path,

keep_build_directory: false,
skip_if_exists: false,
Expand Down Expand Up @@ -63,9 +63,8 @@ impl DocBuilderOptions {
let (prefix, crates_io_index_path) =
generate_paths(prefix);
DocBuilderOptions {
prefix: prefix,
crates_io_index_path: crates_io_index_path,

prefix,
crates_io_index_path,
..Default::default()
}
}
Expand Down
Loading