Enable additional rustc and Clippy lints (#618)

Enables some off-by-default lints that we already use in the `libcnb.rs` repository. https://doc.rust-lang.org/rustc/lints/listing/allowed-by-default.html#unreachable-pub https://doc.rust-lang.org/rustc/lints/listing/allowed-by-default.html#unsafe-code https://rust-lang.github.io/rust-clippy/master/index.html#/panic_in_result_fn https://rust-lang.github.io/rust-clippy/master/index.html#/unwrap_used GUS-W-14523834.
heroku · Nov 20, 2023 · ce4f929 · ce4f929
1 parent 642d6a3
commit ce4f929
Show file tree

Hide file tree

Showing 19 changed files with 52 additions and 47 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -18,10 +18,14 @@ 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"
 module_name_repetitions = "allow"

diff --git a/buildpacks/gradle/src/gradle_command/dependency_report.rs b/buildpacks/gradle/src/gradle_command/dependency_report.rs
@@ -51,10 +51,7 @@ impl GradleDependencyReport {
             })
     }
 
-    pub(crate) fn flattened_dependencies(
-        &self,
-        configuration_name: &str,
-    ) -> Option<Vec<Dependency>> {
+    fn flattened_dependencies(&self, configuration_name: &str) -> Option<Vec<Dependency>> {
         self.entries.get(configuration_name).map(|dependencies| {
             let mut acc = vec![];
 
@@ -105,7 +102,7 @@ pub(crate) enum Suffix {
 }
 
 #[derive(Debug)]
-pub(crate) enum ParseError {}
+enum ParseError {}
 
 mod parser {
     use super::{Dependency, GradleDependencyReport, Suffix};

diff --git a/buildpacks/gradle/src/gradle_command/mod.rs b/buildpacks/gradle/src/gradle_command/mod.rs
@@ -41,10 +41,7 @@ impl<P> GradleCommandError<P> {
     }
 }
 
-pub(crate) fn run_gradle_command<T, F, P>(
-    command: &mut Command,
-    parser: F,
-) -> Result<T, GradleCommandError<P>>
+fn run_gradle_command<T, F, P>(command: &mut Command, parser: F) -> Result<T, GradleCommandError<P>>
 where
     F: FnOnce(&str, &str) -> Result<T, P>,
 {

diff --git a/buildpacks/gradle/src/gradle_command/tasks.rs b/buildpacks/gradle/src/gradle_command/tasks.rs
@@ -29,7 +29,7 @@ pub(crate) struct Tasks {
 }
 
 impl Tasks {
-    pub(crate) fn names(&self) -> Vec<String> {
+    fn names(&self) -> Vec<String> {
         self.groups
             .iter()
             .flat_map(|task_group| &task_group.tasks)

diff --git a/buildpacks/gradle/src/main.rs b/buildpacks/gradle/src/main.rs
@@ -29,10 +29,10 @@ mod framework;
 mod gradle_command;
 mod layers;
 
-pub(crate) struct GradleBuildpack;
+struct GradleBuildpack;
 
 #[derive(Debug)]
-pub(crate) enum GradleBuildpackError {
+enum GradleBuildpackError {
     GradleWrapperNotFound,
     DetectError(std::io::Error),
     GradleBuildIoError(std::io::Error),
@@ -47,7 +47,7 @@ pub(crate) enum GradleBuildpackError {
 }
 
 #[derive(Debug, Deserialize)]
-pub(crate) struct GradleBuildpackMetadata {}
+struct GradleBuildpackMetadata {}
 
 impl Buildpack for GradleBuildpack {
     type Platform = GenericPlatform;
@@ -147,4 +147,4 @@ impl From<GradleBuildpackError> for libcnb::Error<GradleBuildpackError> {
     }
 }
 
-pub(crate) const GRADLE_TASK_NAME_HEROKU_START_DAEMON: &str = "heroku_buildpack_start_daemon";
+const GRADLE_TASK_NAME_HEROKU_START_DAEMON: &str = "heroku_buildpack_start_daemon";
diff --git a/buildpacks/gradle/tests/integration/main.rs b/buildpacks/gradle/tests/integration/main.rs
@@ -13,7 +13,7 @@ use libcnb_test::BuildpackReference;
 mod smoke;
 mod ux;
 
-pub(crate) fn default_buildpacks() -> Vec<BuildpackReference> {
+fn default_buildpacks() -> Vec<BuildpackReference> {
     vec![
         BuildpackReference::Other(String::from("heroku/jvm")),
         BuildpackReference::CurrentCrate,

diff --git a/buildpacks/jvm-function-invoker/src/layers/bundle.rs b/buildpacks/jvm-function-invoker/src/layers/bundle.rs
@@ -126,4 +126,4 @@ pub(crate) enum BundleLayerError {
     CouldNotReadFunctionBundleToml(TomlFileError),
 }
 
-pub(crate) const FUNCTION_BUNDLE_DIR_ENV_VAR_NAME: &str = "JVM_FUNCTION_BUNDLE_DIR";
+const FUNCTION_BUNDLE_DIR_ENV_VAR_NAME: &str = "JVM_FUNCTION_BUNDLE_DIR";
diff --git a/buildpacks/jvm-function-invoker/src/main.rs b/buildpacks/jvm-function-invoker/src/main.rs
@@ -29,15 +29,15 @@ mod common;
 mod error;
 mod layers;
 
-pub(crate) struct JvmFunctionInvokerBuildpack;
+struct JvmFunctionInvokerBuildpack;
 
 #[derive(Deserialize, Debug)]
-pub(crate) struct JvmFunctionInvokerBuildpackMetadata {
+struct JvmFunctionInvokerBuildpackMetadata {
     runtime: JvmFunctionInvokerBuildpackRuntimeMetadata,
 }
 
 #[derive(Deserialize, Debug)]
-pub(crate) struct JvmFunctionInvokerBuildpackRuntimeMetadata {
+struct JvmFunctionInvokerBuildpackRuntimeMetadata {
     url: String,
     sha256: String,
 }

diff --git a/buildpacks/jvm/src/bin/heroku_database_env_var_rewrite.rs b/buildpacks/jvm/src/bin/heroku_database_env_var_rewrite.rs
@@ -6,7 +6,9 @@ use libcnb::exec_d::write_exec_d_program_output;
 use std::collections::HashMap;
 use url::Url;
 
-pub(crate) fn main() {
+fn main() {
+    // TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-jvm/issues/616
+    #[allow(clippy::unwrap_used)]
     write_exec_d_program_output(
         jvm_env_vars_for_env(&std::env::vars().collect())
             .into_iter()
@@ -68,6 +70,8 @@ fn jvm_env_vars_for_env(input: &HashMap<String, String>) -> HashMap<String, Stri
         .iter()
         .filter(|(name, _)| name.starts_with("HEROKU_POSTGRESQL_") && name.ends_with("_URL"))
     {
+        // TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-jvm/issues/616
+        #[allow(clippy::unwrap_used)]
         result.extend(env_vars_for_database_url(
             value,
             format!("{}_JDBC", name.strip_suffix("_URL").unwrap()),
@@ -90,6 +94,8 @@ fn env_vars_for_database_url(
     url_string: impl AsRef<str>,
     env_var_prefix: impl AsRef<str>,
 ) -> HashMap<String, String> {
+    // TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-jvm/issues/616
+    #[allow(clippy::unwrap_used)]
     let mut url = Url::parse(url_string.as_ref()).unwrap();
 
     // Previous versions of this script only set the environment variables when a username and
@@ -104,14 +110,19 @@ fn env_vars_for_database_url(
         Some(password) => String::from(password),
     };
 
+    // TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-jvm/issues/616
+    #[allow(clippy::unwrap_used)]
     url.set_username("").unwrap();
+    #[allow(clippy::unwrap_used)]
     url.set_password(None).unwrap();
 
     url.query_pairs_mut()
         .append_pair("user", &original_username)
         .append_pair("password", &original_password);
 
     if url.scheme() == "postgres" {
+        // TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-jvm/issues/616
+        #[allow(clippy::unwrap_used)]
         url.set_scheme("postgresql").unwrap();
         url.query_pairs_mut().append_pair("sslmode", "require");
     };

diff --git a/buildpacks/jvm/src/bin/heroku_metrics_agent_setup.rs b/buildpacks/jvm/src/bin/heroku_metrics_agent_setup.rs
@@ -7,7 +7,7 @@ use libcnb::exec_d::write_exec_d_program_output;
 use libcnb::Env;
 use std::collections::HashMap;
 
-pub(crate) fn main() {
+fn main() {
     write_exec_d_program_output(output_from_env(&Env::from_current()));
 }
 

diff --git a/buildpacks/jvm/src/main.rs b/buildpacks/jvm/src/main.rs
@@ -31,10 +31,10 @@ use buildpacks_jvm_shared_test as _;
 #[cfg(test)]
 use libcnb_test as _;
 
-pub(crate) struct OpenJdkBuildpack;
+struct OpenJdkBuildpack;
 
 #[derive(Debug)]
-pub(crate) enum OpenJdkBuildpackError {
+enum OpenJdkBuildpackError {
     OpenJdkDownloadError(DownloadError),
     MetricsAgentDownloadError(DownloadError),
     MetricsAgentSha256ValidationError(ValidateSha256Error),
@@ -125,13 +125,13 @@ impl Buildpack for OpenJdkBuildpack {
 }
 
 #[derive(Deserialize, Debug)]
-pub(crate) struct OpenJdkBuildpackMetadata {
+struct OpenJdkBuildpackMetadata {
     #[serde(rename = "heroku-metrics-agent")]
     heroku_metrics_agent: HerokuMetricsAgentMetadata,
 }
 
 #[derive(Serialize, Deserialize, Clone, Debug, Eq, PartialEq)]
-pub(crate) struct HerokuMetricsAgentMetadata {
+struct HerokuMetricsAgentMetadata {
     url: String,
     sha256: String,
 }

diff --git a/buildpacks/maven/src/layer/maven.rs b/buildpacks/maven/src/layer/maven.rs
@@ -35,6 +35,8 @@ impl Layer for MavenLayer {
         _context: &BuildContext<Self::Buildpack>,
         layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
+        // TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-jvm/issues/616
+        #[allow(clippy::unwrap_used)]
         let temp_dir = tempfile::tempdir().unwrap();
         let temp_file_path = temp_dir.path().join("maven.tar.gz");
 
@@ -54,6 +56,8 @@ impl Layer for MavenLayer {
                 }
             })?;
 
+        // TODO: Remove usage of unwrap(): https://github.com/heroku/buildpacks-jvm/issues/616
+        #[allow(clippy::unwrap_used)]
         extract_tarball(&mut File::open(&temp_file_path).unwrap(), layer_path, 1)
             .map_err(MavenBuildpackError::MavenTarballDecompressError)?;
 

diff --git a/buildpacks/maven/src/main.rs b/buildpacks/maven/src/main.rs
@@ -39,10 +39,10 @@ mod settings;
 mod util;
 mod warnings;
 
-pub(crate) struct MavenBuildpack;
+struct MavenBuildpack;
 
 #[derive(Debug)]
-pub(crate) enum MavenBuildpackError {
+enum MavenBuildpackError {
     UnsupportedMavenVersion(String),
     MavenTarballDownloadError(DownloadError),
     MavenTarballSha256IoError(std::io::Error),
@@ -62,14 +62,14 @@ pub(crate) enum MavenBuildpackError {
 }
 
 #[derive(Debug, Deserialize)]
-pub(crate) struct MavenBuildpackMetadata {
+struct MavenBuildpackMetadata {
     #[serde(rename = "default-version")]
     default_version: String,
     tarballs: HashMap<String, Tarball>,
 }
 
 #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
-pub(crate) struct Tarball {
+struct Tarball {
     url: String,
     sha256: String,
 }
@@ -284,7 +284,7 @@ impl From<MavenBuildpackError> for libcnb::Error<MavenBuildpackError> {
     }
 }
 
-pub(crate) fn app_dependency_list_path<P: AsRef<Path>>(app_dir: P) -> PathBuf {
+fn app_dependency_list_path<P: AsRef<Path>>(app_dir: P) -> PathBuf {
     app_dir.as_ref().join("target/mvn-dependency-list.log")
 }
 

diff --git a/buildpacks/maven/tests/integration/customization.rs b/buildpacks/maven/tests/integration/customization.rs
@@ -4,7 +4,7 @@ use libcnb_test::{assert_contains, TestRunner};
 
 #[test]
 #[ignore = "integration test"]
-pub(crate) fn maven_custom_goals() {
+fn maven_custom_goals() {
     TestRunner::default().build(default_config().env("MAVEN_CUSTOM_GOALS", "site"), |context| {
         // Assert only the goals in MAVEN_CUSTOM_GOALS are executed
         assert_contains!(context.pack_stdout, "./mvnw -DskipTests site");
@@ -41,7 +41,7 @@ pub(crate) fn maven_custom_goals() {
 
 #[test]
 #[ignore = "integration test"]
-pub(crate) fn maven_custom_opts() {
+fn maven_custom_opts() {
     TestRunner::default().build(default_config().env("MAVEN_CUSTOM_OPTS", "-X"), |context| {
         // Assert only the options in MAVEN_CUSTOM_GOALS are used
         assert_contains!(context.pack_stdout, "./mvnw -X clean install");

diff --git a/buildpacks/maven/tests/integration/main.rs b/buildpacks/maven/tests/integration/main.rs
@@ -7,6 +7,8 @@
 
 // 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 buildpacks_jvm_shared_test::DEFAULT_INTEGRATION_TEST_BUILDER;
 use libcnb_test::{BuildConfig, BuildpackReference};
@@ -20,7 +22,7 @@ mod settings_xml;
 mod smoke;
 mod versions;
 
-pub(crate) fn default_config() -> BuildConfig {
+fn default_config() -> BuildConfig {
     BuildConfig::new(
         DEFAULT_INTEGRATION_TEST_BUILDER,
         "test-apps/simple-http-service",
@@ -29,7 +31,7 @@ pub(crate) fn default_config() -> BuildConfig {
     .clone()
 }
 
-pub(crate) fn default_buildpacks() -> Vec<BuildpackReference> {
+fn default_buildpacks() -> Vec<BuildpackReference> {
     vec![
         BuildpackReference::Other(String::from("heroku/jvm")),
         BuildpackReference::CurrentCrate,

diff --git a/buildpacks/sbt/src/main.rs b/buildpacks/sbt/src/main.rs
@@ -34,7 +34,7 @@ use tempfile as _;
 #[cfg(test)]
 use ureq as _;
 
-pub(crate) struct SbtBuildpack;
+struct SbtBuildpack;
 
 impl Buildpack for SbtBuildpack {
     type Platform = GenericPlatform;

diff --git a/buildpacks/sbt/tests/integration/main.rs b/buildpacks/sbt/tests/integration/main.rs
@@ -15,7 +15,7 @@ mod sbt_at_launch;
 mod smoke;
 mod ux;
 
-pub(crate) fn default_buildpacks() -> Vec<BuildpackReference> {
+fn default_buildpacks() -> Vec<BuildpackReference> {
     vec![
         BuildpackReference::Other(String::from("heroku/jvm")),
         BuildpackReference::CurrentCrate,

diff --git a/clippy.toml b/clippy.toml
@@ -0,0 +1 @@
+allow-unwrap-in-tests = true
diff --git a/shared/src/fs.rs b/shared/src/fs.rs
@@ -21,17 +21,6 @@ where
         .map(|dir_entries| dir_entries.into_iter().map(|dir_entry| dir_entry.path()))
 }
 
-#[cfg(unix)]
-pub fn is_executable<P: AsRef<Path>>(path: P) -> bool {
-    use std::os::unix::fs::PermissionsExt;
-
-    path.as_ref()
-        .metadata()
-        .map(|metadata| metadata.is_file() && metadata.permissions().mode() & 0o111 != 0)
-        .ok()
-        .unwrap_or_default()
-}
-
 #[cfg(unix)]
 pub fn set_executable<P: AsRef<Path>>(path: P) -> std::io::Result<()> {
     use std::os::unix::fs::PermissionsExt;