Skip to content

Commit

Permalink
Enable additional rustc and Clippy lints
Browse files Browse the repository at this point in the history
  • Loading branch information
edmorley committed Nov 20, 2023
1 parent f75eb07 commit f7a201c
Show file tree
Hide file tree
Showing 16 changed files with 55 additions and 33 deletions.
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@ edition = "2021"
rust-version = "1.74"

[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"
module_name_repetitions = "allow"
9 changes: 6 additions & 3 deletions buildpacks/ruby/src/gem_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use std::process::Command;
///
/// Requires `ruby` and `bundle` to be installed and on the PATH
#[derive(Debug)]
pub struct GemList {
pub gems: HashMap<String, GemVersion>,
pub(crate) struct GemList {
pub(crate) gems: HashMap<String, GemVersion>,
}

/// Converts the output of `$ gem list` into a data structure that can be inspected and compared
Expand Down Expand Up @@ -59,7 +59,10 @@ impl GemList {
/// # Errors
///
/// Errors if the command `bundle list` is unsuccessful.
pub fn from_bundle_list<T, K, V>(envs: T, _logger: &dyn SectionLogger) -> Result<Self, CmdError>
pub(crate) fn from_bundle_list<T, K, V>(
envs: T,
_logger: &dyn SectionLogger,
) -> Result<Self, CmdError>
where
T: IntoIterator<Item = (K, V)>,
K: AsRef<OsStr>,
Expand Down
8 changes: 4 additions & 4 deletions buildpacks/ruby/src/layers/bundle_download_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::process::Command;

#[derive(Deserialize, Serialize, Debug, Clone)]
pub(crate) struct BundleDownloadLayerMetadata {
pub version: ResolvedBundlerVersion,
pub(crate) version: ResolvedBundlerVersion,
}

/// # Install the bundler gem
Expand All @@ -28,9 +28,9 @@ pub(crate) struct BundleDownloadLayerMetadata {
/// Installs a copy of `bundler` to the `<layer-dir>` with a bundler executable in
/// `<layer-dir>/bin`. Must run before [`crate.steps.bundle_install`].
pub(crate) struct BundleDownloadLayer<'a> {
pub env: Env,
pub metadata: BundleDownloadLayerMetadata,
pub _section_logger: &'a dyn SectionLogger,
pub(crate) env: Env,
pub(crate) metadata: BundleDownloadLayerMetadata,
pub(crate) _section_logger: &'a dyn SectionLogger,
}

impl<'a> Layer for BundleDownloadLayer<'a> {
Expand Down
16 changes: 8 additions & 8 deletions buildpacks/ruby/src/layers/bundle_install_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,17 @@ pub(crate) const FORCE_BUNDLE_INSTALL_CACHE_KEY: &str = "v1";
/// `BundleInstallLayer::create` are the same.
#[derive(Debug)]
pub(crate) struct BundleInstallLayer<'a> {
pub env: Env,
pub without: BundleWithout,
pub _section_log: &'a dyn SectionLogger,
pub metadata: BundleInstallLayerMetadata,
pub(crate) env: Env,
pub(crate) without: BundleWithout,
pub(crate) _section_log: &'a dyn SectionLogger,
pub(crate) metadata: BundleInstallLayerMetadata,
}

#[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)]
pub(crate) struct BundleInstallLayerMetadata {
pub stack: StackId,
pub ruby_version: ResolvedRubyVersion,
pub force_bundle_install_key: String,
pub(crate) stack: StackId,
pub(crate) ruby_version: ResolvedRubyVersion,
pub(crate) force_bundle_install_key: String,

/// A struct that holds the cryptographic hash of components that can
/// affect the result of `bundle install`. When these values do not
Expand All @@ -54,7 +54,7 @@ pub(crate) struct BundleInstallLayerMetadata {
/// This value is cached with metadata, so changing the struct
/// may cause metadata to be invalidated (and the cache cleared).
///
pub digest: MetadataDigest, // Must be last for serde to be happy https://github.com/toml-rs/toml-rs/issues/142
pub(crate) digest: MetadataDigest, // Must be last for serde to be happy https://github.com/toml-rs/toml-rs/issues/142
}

impl<'a> BundleInstallLayer<'a> {
Expand Down
2 changes: 1 addition & 1 deletion buildpacks/ruby/src/layers/metrics_agent_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const DOWNLOAD_SHA: &str = "f9bf9f33c949e15ffed77046ca38f8dae9307b6a0181c6af29a2

#[derive(Debug)]
pub(crate) struct MetricsAgentInstall<'a> {
pub _in_section: &'a dyn SectionLogger, // force the layer to be called within a Section logging context, not necessary but it's safer
pub(crate) _in_section: &'a dyn SectionLogger, // force the layer to be called within a Section logging context, not necessary but it's safer
}

#[derive(Deserialize, Serialize, Debug, Clone)]
Expand Down
8 changes: 4 additions & 4 deletions buildpacks/ruby/src/layers/ruby_install_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ use url::Url;
/// When the Ruby version changes, invalidate and re-run.
///
pub(crate) struct RubyInstallLayer<'a> {
pub _in_section: &'a dyn SectionLogger, // force the layer to be called within a Section logging context, not necessary but it's safer
pub metadata: RubyInstallLayerMetadata,
pub(crate) _in_section: &'a dyn SectionLogger, // force the layer to be called within a Section logging context, not necessary but it's safer
pub(crate) metadata: RubyInstallLayerMetadata,
}

#[derive(Deserialize, Serialize, Debug, Clone)]
pub(crate) struct RubyInstallLayerMetadata {
pub stack: StackId,
pub version: ResolvedRubyVersion,
pub(crate) stack: StackId,
pub(crate) version: ResolvedRubyVersion,
}

impl<'a> Layer for RubyInstallLayer<'a> {
Expand Down
6 changes: 3 additions & 3 deletions buildpacks/ruby/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use libcnb_test as _;

use clap as _;

pub(crate) struct RubyBuildpack;
struct RubyBuildpack;

impl Buildpack for RubyBuildpack {
type Platform = GenericPlatform;
Expand Down Expand Up @@ -238,7 +238,7 @@ fn needs_java(gemfile_lock: &str) -> bool {
}

#[derive(Debug)]
pub(crate) enum RubyBuildpackError {
enum RubyBuildpackError {
RakeDetectError(CmdError),
GemListGetError(CmdError),
RubyInstallError(RubyInstallError),
Expand All @@ -260,7 +260,7 @@ impl From<RubyBuildpackError> for libcnb::Error<RubyBuildpackError> {
buildpack_main!(RubyBuildpack);

#[derive(serde::Deserialize, serde::Serialize, Debug, Clone, PartialEq, Eq)]
pub(crate) struct BundleWithout(String);
struct BundleWithout(String);

impl BundleWithout {
fn new(without: impl AsRef<str>) -> Self {
Expand Down
4 changes: 2 additions & 2 deletions buildpacks/ruby/src/rake_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::gem_list::GemList;
use std::path::{Path, PathBuf};

/// Determine if an application is ready to run a rake task or not
pub fn check_rake_ready(
pub(crate) fn check_rake_ready(
app_path: &Path,
gem_list: &GemList,
globs: impl IntoIterator<Item = impl AsRef<str>>,
Expand All @@ -15,7 +15,7 @@ pub fn check_rake_ready(
}

#[derive(Debug, Eq, PartialEq)]
pub enum RakeStatus {
pub(crate) enum RakeStatus {
Ready(PathBuf),
MissingRakeGem,
MissingRakefile,
Expand Down
10 changes: 7 additions & 3 deletions buildpacks/ruby/src/rake_task_detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,19 @@ use std::{ffi::OsStr, process::Command};
/// assert!(!rake_detect.has_task("assets:precompile"));
/// ```
#[derive(Default)]
pub struct RakeDetect {
pub(crate) struct RakeDetect {
output: String,
}

impl RakeDetect {
/// # Errors
///
/// Will return `Err` if `bundle exec rake -p` command cannot be invoked by the operating system.
pub fn from_rake_command<T: IntoIterator<Item = (K, V)>, K: AsRef<OsStr>, V: AsRef<OsStr>>(
pub(crate) fn from_rake_command<
T: IntoIterator<Item = (K, V)>,
K: AsRef<OsStr>,
V: AsRef<OsStr>,
>(
_logger: &dyn SectionLogger,
envs: T,
error_on_failure: bool,
Expand Down Expand Up @@ -53,7 +57,7 @@ impl RakeDetect {
}

#[must_use]
pub fn has_task(&self, string: &str) -> bool {
pub(crate) fn has_task(&self, string: &str) -> bool {
let task_re = regex::Regex::new(&format!("\\s{string}")).expect("clippy");
task_re.is_match(&self.output)
}
Expand Down
2 changes: 2 additions & 0 deletions buildpacks/ruby/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_empty, BuildConfig, BuildpackReference, ContainerConfig,
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
6 changes: 6 additions & 0 deletions commons/bin/print_style_guide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ fn main() {
.step("Output can be streamed. Mostly from commands. Example:")
.step_timed_stream(&format!("Running {}", fmt::command(command.name())));

// TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-ruby/issues/238
#[allow(clippy::unwrap_used)]
command.stream_output(stream.io(), stream.io()).unwrap();
log = stream.finish_timed_stream().end_section();
drop(log);
Expand Down Expand Up @@ -100,6 +102,8 @@ fn main() {
// do work here
});
log_step_stream("log_step_stream()", |stream| {
// TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-ruby/issues/238
#[allow(clippy::unwrap_used)]
Command::new("bash")
.args(["-c", "ps aux | grep cargo"])
.stream_output(stream.io(), stream.io())
Expand All @@ -113,6 +117,8 @@ fn main() {
}

{
// TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-ruby/issues/238
#[allow(clippy::unwrap_used)]
let cmd_error = Command::new("iDoNotExist").named_output().err().unwrap();

let mut log = BuildLog::new(stdout()).buildpack_name("Error and warnings");
Expand Down
4 changes: 2 additions & 2 deletions commons/src/cache/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ mod tests {
use super::*;

#[test]
pub fn test_grabs_files() {
fn test_grabs_files() {
// FWIW This action must be done on two lines as the tmpdir gets cleaned
// as soon as the variable goes out of scope and a path
// reference does not retain it's caller
Expand All @@ -146,7 +146,7 @@ mod tests {

use byte_unit::n_mib_bytes;

pub fn touch_file(path: &PathBuf, f: impl FnOnce(&PathBuf)) {
fn touch_file(path: &PathBuf, f: impl FnOnce(&PathBuf)) {
if let Some(parent) = path.parent() {
if !parent.exists() {
fs_err::create_dir_all(parent).unwrap();
Expand Down
4 changes: 2 additions & 2 deletions commons/src/cache/in_app_dir_cache_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use std::path::PathBuf;
/// asset.
#[derive(Deserialize, Serialize, Debug, Clone)]
pub(crate) struct InAppDirCacheLayer<B> {
pub app_dir_path: PathBuf,
pub(crate) app_dir_path: PathBuf,
buildpack: PhantomData<B>,
}

Expand All @@ -33,7 +33,7 @@ pub(crate) struct InAppDirCacheLayerMetadata {
}

impl<B> InAppDirCacheLayer<B> {
pub fn new(app_dir_path: PathBuf) -> Self {
pub(crate) fn new(app_dir_path: PathBuf) -> Self {
Self {
app_dir_path,
buildpack: PhantomData,
Expand Down
2 changes: 2 additions & 0 deletions commons/src/output/background_timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ where

let arc_io = arc_io.clone();
let handle = std::thread::spawn(move || {
// TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-ruby/issues/238
#[allow(clippy::unwrap_used)]
let mut io = arc_io.lock().unwrap();
write!(&mut io, "{start}").expect("Internal error");
io.flush().expect("Internal error");
Expand Down
2 changes: 1 addition & 1 deletion commons/src/output/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ pub(crate) struct LinesWithEndings<'a> {
}

impl<'a> LinesWithEndings<'a> {
pub fn from(input: &'a str) -> LinesWithEndings<'a> {
pub(crate) fn from(input: &'a str) -> LinesWithEndings<'a> {
LinesWithEndings { input }
}
}
Expand Down

0 comments on commit f7a201c

Please sign in to comment.