Skip to content

Commit

Permalink
Enable additional rustc and Clippy lints (#731)
Browse files Browse the repository at this point in the history
  • Loading branch information
edmorley authored Nov 20, 2023
1 parent cf5e184 commit 0c30930
Show file tree
Hide file tree
Showing 28 changed files with 68 additions and 60 deletions.
5 changes: 5 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@ edition = "2021"
publish = false

[workspace.lints.rust]
unreachable_pub = "warn"
unsafe_code = "warn"
unused_crate_dependencies = "warn"

[workspace.lints.clippy]
panic_in_result_fn = "warn"
pedantic = "warn"
unwrap_used = "warn"
enum_variant_names = "allow"
missing_errors_doc = "allow"
missing_panics_doc = "allow"
module_name_repetitions = "allow"
Expand Down
4 changes: 2 additions & 2 deletions buildpacks/nodejs-corepack/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ mod cmd;
mod errors;
mod layers;

pub(crate) struct CorepackBuildpack;
struct CorepackBuildpack;

impl Buildpack for CorepackBuildpack {
type Platform = GenericPlatform;
Expand Down Expand Up @@ -113,7 +113,7 @@ impl Buildpack for CorepackBuildpack {
}

#[derive(Debug)]
pub(crate) enum CorepackBuildpackError {
enum CorepackBuildpackError {
PackageManagerMissing,
PackageJson(PackageJsonError),
ShimLayer(std::io::Error),
Expand Down
2 changes: 1 addition & 1 deletion buildpacks/nodejs-engine/src/bin/web_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const MAX_AVAILABLE_MEMORY_MB: usize = 14336;
const DEFAULT_AVAILABLE_MEMORY_MB: usize = 512;
const DEFAULT_WEB_MEMORY_MB: usize = 512;

pub fn main() {
fn main() {
write_exec_d_program_output(web_env(read_env("WEB_CONCURRENCY"), read_env("WEB_MEMORY")));
}

Expand Down
8 changes: 4 additions & 4 deletions buildpacks/nodejs-engine/src/layers/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@ use tempfile::NamedTempFile;
use thiserror::Error;

/// A layer that downloads the Node.js distribution artifacts
pub struct DistLayer {
pub release: Release,
pub(crate) struct DistLayer {
pub(crate) release: Release,
}

#[derive(Deserialize, Serialize, Clone, PartialEq, Eq)]
pub struct DistLayerMetadata {
pub(crate) struct DistLayerMetadata {
layer_version: String,
nodejs_version: String,
stack_id: StackId,
}

#[derive(Error, Debug)]
pub enum DistLayerError {
pub(crate) enum DistLayerError {
#[error("Couldn't create tempfile for Node.js distribution: {0}")]
TempFile(std::io::Error),
#[error("Couldn't download Node.js distribution: {0}")]
Expand Down
4 changes: 2 additions & 2 deletions buildpacks/nodejs-engine/src/layers/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
mod dist;
mod web_env;

pub use dist::*;
pub use web_env::*;
pub(crate) use dist::*;
pub(crate) use web_env::*;
2 changes: 1 addition & 1 deletion buildpacks/nodejs-engine/src/layers/web_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use libcnb::generic::GenericMetadata;
use libcnb::layer::{Layer, LayerResult, LayerResultBuilder};

/// A layer that sets `WEB_MEMORY` and `WEB_CONCURRENCY` via exec.d
pub struct WebEnvLayer;
pub(crate) struct WebEnvLayer;

impl Layer for WebEnvLayer {
type Buildpack = NodeJsEngineBuildpack;
Expand Down
4 changes: 2 additions & 2 deletions buildpacks/nodejs-engine/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const INVENTORY: &str = include_str!("../inventory.toml");

const LTS_VERSION: &str = "20.x";

pub struct NodeJsEngineBuildpack;
struct NodeJsEngineBuildpack;

impl Buildpack for NodeJsEngineBuildpack {
type Platform = GenericPlatform;
Expand Down Expand Up @@ -153,7 +153,7 @@ impl Buildpack for NodeJsEngineBuildpack {
}

#[derive(Error, Debug)]
pub enum NodeJsEngineBuildpackError {
enum NodeJsEngineBuildpackError {
#[error("Couldn't parse Node.js inventory: {0}")]
InventoryParseError(toml::de::Error),
#[error("Couldn't parse package.json: {0}")]
Expand Down
10 changes: 5 additions & 5 deletions buildpacks/nodejs-function-invoker/src/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use libherokubuildpack::toml::toml_select_value;
use std::path::PathBuf;
use thiserror::Error;

pub fn is_function<P>(d: P) -> bool
pub(crate) fn is_function<P>(d: P) -> bool
where
P: Into<PathBuf>,
{
Expand All @@ -21,7 +21,7 @@ where
}
}

pub fn get_main<P>(d: P) -> Result<PathBuf, MainError>
pub(crate) fn get_main<P>(d: P) -> Result<PathBuf, MainError>
where
P: Into<PathBuf>,
{
Expand All @@ -33,7 +33,7 @@ where
.and_then(|path| path.exists().then_some(path).ok_or(MainError::MissingFile))
}

pub fn get_declared_runtime_package_version<P>(
pub(crate) fn get_declared_runtime_package_version<P>(
app_dir: P,
package_name: &String,
) -> Result<Option<String>, ExplicitRuntimeDependencyError>
Expand All @@ -57,7 +57,7 @@ where
}

#[derive(Error, Debug)]
pub enum MainError {
pub(crate) enum MainError {
#[error("Could not determine function file location from package.json. {0}")]
PackageJson(#[from] PackageJsonError),
#[error(
Expand All @@ -69,7 +69,7 @@ pub enum MainError {
}

#[derive(Error, Debug)]
pub enum ExplicitRuntimeDependencyError {
pub(crate) enum ExplicitRuntimeDependencyError {
#[error("Failure while attempting to read from package.json. {0}")]
PackageJson(#[from] PackageJsonError),
#[error("The '{package_name}' package must be declared in the 'dependencies' field of your package.json but was found in 'devDependencies'.")]
Expand Down
8 changes: 4 additions & 4 deletions buildpacks/nodejs-function-invoker/src/layers/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,19 @@ use std::process::Command;
use thiserror::Error;

/// A layer that installs the Node.js Invoker/Runtime package
pub struct RuntimeLayer {
pub package: String,
pub(crate) struct RuntimeLayer {
pub(crate) package: String,
}

#[derive(Deserialize, Serialize, Clone, PartialEq, Eq)]
pub struct RuntimeLayerMetadata {
pub(crate) struct RuntimeLayerMetadata {
layer_version: String,
package: String,
stack_id: StackId,
}

#[derive(Error, Debug)]
pub enum RuntimeLayerError {
pub(crate) enum RuntimeLayerError {
#[error("Couldn't run `npm install` command: {0}")]
NpmCommandError(std::io::Error),
#[error("Couldn't install invoker runtime with `npm install`: #{0}")]
Expand Down
2 changes: 1 addition & 1 deletion buildpacks/nodejs-function-invoker/src/layers/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl Layer for ScriptLayer {
}

#[derive(thiserror::Error, Debug)]
pub enum ScriptLayerError {
pub(crate) enum ScriptLayerError {
#[error("Could not write runtime script to layer: {0}")]
CouldNotWriteRuntimeScript(std::io::Error),
#[error("Could not set executable bit on runtime script: {0}")]
Expand Down
14 changes: 7 additions & 7 deletions buildpacks/nodejs-function-invoker/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,17 @@ use ureq as _;
mod function;
mod layers;

pub struct NodeJsInvokerBuildpack;
struct NodeJsInvokerBuildpack;

#[derive(Deserialize, Debug)]
pub struct NodeJsInvokerBuildpackMetadata {
pub runtime: NodeJsInvokerBuildpackRuntimeMetadata,
struct NodeJsInvokerBuildpackMetadata {
runtime: NodeJsInvokerBuildpackRuntimeMetadata,
}

#[derive(Deserialize, Debug)]
pub struct NodeJsInvokerBuildpackRuntimeMetadata {
pub package_name: String,
pub package_version: String,
struct NodeJsInvokerBuildpackRuntimeMetadata {
package_name: String,
package_version: String,
}

impl Buildpack for NodeJsInvokerBuildpack {
Expand Down Expand Up @@ -159,7 +159,7 @@ impl Buildpack for NodeJsInvokerBuildpack {
}

#[derive(Error, Debug)]
pub enum NodeJsInvokerBuildpackError {
enum NodeJsInvokerBuildpackError {
#[error("{0}")]
MainFunction(#[from] MainError),
#[error("{0}")]
Expand Down
2 changes: 2 additions & 0 deletions buildpacks/nodejs-function-invoker/tests/integration_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Required due to: https://github.com/rust-lang/rust/issues/95513
#![allow(unused_crate_dependencies)]
// Required due to: https://github.com/rust-lang/rust-clippy/issues/11119
#![allow(clippy::unwrap_used)]

use base64::Engine;
use libcnb_test::{assert_contains, assert_not_contains, TestContext};
Expand Down
4 changes: 2 additions & 2 deletions buildpacks/nodejs-npm-engine/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ use std::process::Command;
#[cfg(test)]
use test_support as _;

pub(crate) const BUILDPACK_NAME: &str = "Heroku Node.js npm Engine Buildpack";
const BUILDPACK_NAME: &str = "Heroku Node.js npm Engine Buildpack";

const INVENTORY: &str = include_str!("../inventory.toml");

pub(crate) struct NpmEngineBuildpack;
struct NpmEngineBuildpack;

impl Buildpack for NpmEngineBuildpack {
type Platform = GenericPlatform;
Expand Down
2 changes: 2 additions & 0 deletions buildpacks/nodejs-npm-engine/tests/integration_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Required due to: https://github.com/rust-lang/rust/issues/95513
#![allow(unused_crate_dependencies)]
// Required due to: https://github.com/rust-lang/rust-clippy/issues/11119
#![allow(clippy::unwrap_used)]

use libcnb_test::assert_contains;
use std::fs;
Expand Down
2 changes: 1 addition & 1 deletion buildpacks/nodejs-npm-install/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub(crate) fn on_error(error: libcnb::Error<NpmInstallBuildpackError>) {
}
}

pub(crate) fn on_buildpack_error(error: NpmInstallBuildpackError, logger: Box<dyn StartedLogger>) {
fn on_buildpack_error(error: NpmInstallBuildpackError, logger: Box<dyn StartedLogger>) {
match error {
NpmInstallBuildpackError::Application(e) => on_application_error(&e, logger),
NpmInstallBuildpackError::BuildScript(e) => on_build_script_error(&e, logger),
Expand Down
4 changes: 2 additions & 2 deletions buildpacks/nodejs-npm-install/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ use std::process::Command;
#[cfg(test)]
use test_support as _;

pub(crate) const BUILDPACK_NAME: &str = "Heroku Node.js npm Install Buildpack";
const BUILDPACK_NAME: &str = "Heroku Node.js npm Install Buildpack";

pub(crate) struct NpmInstallBuildpack;
struct NpmInstallBuildpack;

impl Buildpack for NpmInstallBuildpack {
type Platform = GenericPlatform;
Expand Down
2 changes: 2 additions & 0 deletions buildpacks/nodejs-npm-install/tests/integration_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Required due to: https://github.com/rust-lang/rust/issues/95513
#![allow(unused_crate_dependencies)]
// Required due to: https://github.com/rust-lang/rust-clippy/issues/11119
#![allow(clippy::unwrap_used)]

use libcnb_test::{assert_contains, assert_not_contains, PackResult};
use serde_json::json;
Expand Down
4 changes: 2 additions & 2 deletions buildpacks/nodejs-pnpm-install/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ mod errors;
mod layers;
mod store;

pub(crate) struct PnpmInstallBuildpack;
struct PnpmInstallBuildpack;

impl Buildpack for PnpmInstallBuildpack {
type Platform = GenericPlatform;
Expand Down Expand Up @@ -110,7 +110,7 @@ impl Buildpack for PnpmInstallBuildpack {
}

#[derive(Debug)]
pub(crate) enum PnpmInstallBuildpackError {
enum PnpmInstallBuildpackError {
BuildScript(cmd::Error),
PackageJson(PackageJsonError),
PnpmDir(cmd::Error),
Expand Down
2 changes: 1 addition & 1 deletion buildpacks/nodejs-yarn/src/layers/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use thiserror::Error;

/// A layer that downloads and installs the yarn cli
pub(crate) struct CliLayer {
pub release: Release,
pub(crate) release: Release,
}

#[derive(Deserialize, Serialize, Clone, PartialEq, Eq)]
Expand Down
4 changes: 2 additions & 2 deletions buildpacks/nodejs-yarn/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ mod yarn;
const INVENTORY: &str = include_str!("../inventory.toml");
const DEFAULT_YARN_REQUIREMENT: &str = "1.22.x";

pub(crate) struct YarnBuildpack;
struct YarnBuildpack;

impl Buildpack for YarnBuildpack {
type Platform = GenericPlatform;
Expand Down Expand Up @@ -205,7 +205,7 @@ impl Buildpack for YarnBuildpack {
}

#[derive(Error, Debug)]
pub(crate) enum YarnBuildpackError {
enum YarnBuildpackError {
#[error("Couldn't run build script: {0}")]
BuildScript(cmd::Error),
#[error("{0}")]
Expand Down
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-unwrap-in-tests = true
4 changes: 1 addition & 3 deletions common/nodejs-utils/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,14 @@ use std::fmt::{Display, Formatter};
use std::io::Stdout;
use std::path::Path;

pub type Result<T> = std::result::Result<T, Error>;

/// Checks for npm, Yarn, pnpm, and shrink-wrap lockfiles and raises an error if multiple are detected.
///
/// # Errors
///
/// Will return an `Err` when:
/// - More than one lockfile exists in the `app_dir`.
/// - No lockfile exists in the `app_dir`.
pub fn check_for_singular_lockfile(app_dir: &Path) -> Result<()> {
pub fn check_for_singular_lockfile(app_dir: &Path) -> Result<(), Error> {
let detected_lockfiles = [
PackageManager::Npm,
PackageManager::Pnpm,
Expand Down
2 changes: 1 addition & 1 deletion common/nodejs-utils/src/distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::{fmt, str::FromStr};
/// Heroku nodebin AWS S3 Bucket name
pub const DEFAULT_BUCKET: &str = "heroku-nodebin";
/// Heroku nodebin AWS S3 Region
pub const DEFAULT_REGION: &str = "us-east-1";
pub(crate) const DEFAULT_REGION: &str = "us-east-1";

#[derive(Debug, Clone, Copy)]
pub enum Distribution {
Expand Down
8 changes: 4 additions & 4 deletions common/nodejs-utils/src/inv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ use thiserror::Error;

/// Default/assumed operating system for node release lookups
#[cfg(target_os = "macos")]
pub const OS: &str = "darwin";
const OS: &str = "darwin";
#[cfg(target_os = "linux")]
pub const OS: &str = "linux";
const OS: &str = "linux";

/// Default/assumed architecture for node release lookups
pub const ARCH: &str = "x64";
const ARCH: &str = "x64";

/// Represents a software inventory with releases.
#[derive(Debug, Deserialize, Serialize)]
Expand Down Expand Up @@ -47,7 +47,7 @@ impl Inventory {
}

#[must_use]
pub fn resolve_other(
fn resolve_other(
&self,
version_requirements: &Requirement,
platform: &str,
Expand Down
2 changes: 1 addition & 1 deletion common/nodejs-utils/src/package_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl PackageManager {
}
}

pub fn iterator() -> impl Iterator<Item = PackageManager> {
pub(crate) fn iterator() -> impl Iterator<Item = PackageManager> {
Self::VALUES.iter().copied()
}
}
Expand Down
2 changes: 1 addition & 1 deletion common/nodejs-utils/src/s3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(crate) struct Content {
#[derive(Debug, Deserialize)]
#[serde(rename_all = "PascalCase")]
#[allow(dead_code)]
pub(crate) struct ListBucketResult {
struct ListBucketResult {
name: String,
pub(crate) prefix: String,
max_keys: usize,
Expand Down
Loading

0 comments on commit 0c30930

Please sign in to comment.