From 2c30f64e52b507f0a8c899a60eb54b984712d55f Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 27 Feb 2023 10:40:51 +0000 Subject: [PATCH] chore: restrict function visibility where overly broad (#920) * chore: throw warning on unreachable pub items * chore: make restricted visibility explicit --- crates/arena/src/lib.rs | 2 ++ crates/fm/src/file_reader.rs | 4 ++-- crates/fm/src/lib.rs | 2 ++ crates/iter-extended/src/lib.rs | 2 ++ crates/nargo/src/backends.rs | 6 +++--- crates/nargo/src/cli/fs/mod.rs | 16 ++++++++-------- crates/nargo/src/cli/mod.rs | 2 +- crates/nargo/src/git.rs | 4 ++-- crates/nargo/src/lib.rs | 2 ++ crates/nargo/src/resolver.rs | 4 ++-- crates/nargo/src/toml.rs | 22 ++++++++++++---------- crates/noirc_abi/src/lib.rs | 2 ++ crates/noirc_abi/src/serialization.rs | 4 ++-- crates/noirc_driver/src/lib.rs | 2 ++ crates/noirc_errors/src/lib.rs | 2 ++ crates/noirc_errors/src/reporter.rs | 6 +++--- crates/wasm/src/lib.rs | 2 ++ 17 files changed, 51 insertions(+), 33 deletions(-) diff --git a/crates/arena/src/lib.rs b/crates/arena/src/lib.rs index bbb04a26d84..b31e33a8898 100644 --- a/crates/arena/src/lib.rs +++ b/crates/arena/src/lib.rs @@ -1,3 +1,5 @@ #![forbid(unsafe_code)] +#![warn(unreachable_pub)] + // For now we use a wrapper around generational-arena pub use generational_arena::{Arena, Index}; diff --git a/crates/fm/src/file_reader.rs b/crates/fm/src/file_reader.rs index edc37f2364d..28c1c148fdc 100644 --- a/crates/fm/src/file_reader.rs +++ b/crates/fm/src/file_reader.rs @@ -13,7 +13,7 @@ cfg_if::cfg_if! { fn read_file(path: &str) -> Result; } - pub fn read_file_to_string(path_to_file: &Path) -> Result { + pub(crate) fn read_file_to_string(path_to_file: &Path) -> Result { use std::io::ErrorKind; let path_str = path_to_file.as_os_str().to_str().unwrap(); match read_file(path_str) { @@ -22,7 +22,7 @@ cfg_if::cfg_if! { } } } else { - pub fn read_file_to_string(path_to_file: &Path) -> Result { + pub(crate) fn read_file_to_string(path_to_file: &Path) -> Result { std::fs::read_to_string(path_to_file) } } diff --git a/crates/fm/src/lib.rs b/crates/fm/src/lib.rs index 4679f6f7d9c..9b50cc62d59 100644 --- a/crates/fm/src/lib.rs +++ b/crates/fm/src/lib.rs @@ -1,4 +1,6 @@ #![forbid(unsafe_code)] +#![warn(unreachable_pub)] + mod file_map; mod file_reader; diff --git a/crates/iter-extended/src/lib.rs b/crates/iter-extended/src/lib.rs index b5729c9a895..4c2d847a585 100644 --- a/crates/iter-extended/src/lib.rs +++ b/crates/iter-extended/src/lib.rs @@ -1,4 +1,6 @@ #![forbid(unsafe_code)] +#![warn(unreachable_pub)] + use std::collections::BTreeMap; /// Equivalent to .into_iter().map(f).collect::>() diff --git a/crates/nargo/src/backends.rs b/crates/nargo/src/backends.rs index 85e94edda82..ff64931a0d9 100644 --- a/crates/nargo/src/backends.rs +++ b/crates/nargo/src/backends.rs @@ -1,11 +1,11 @@ cfg_if::cfg_if! { if #[cfg(feature = "plonk_bn254")] { - pub use aztec_backend::Plonk as ConcreteBackend; + pub(crate) use aztec_backend::Plonk as ConcreteBackend; } else if #[cfg(feature = "plonk_bn254_wasm")] { - pub use aztec_wasm_backend::Plonk as ConcreteBackend; + pub(crate) use aztec_wasm_backend::Plonk as ConcreteBackend; } else if #[cfg(feature = "marlin")] { // R1CS_MARLIN_ARKWORKS - pub use marlin_arkworks_backend::Marlin as ConcreteBackend; + pub(crate) use marlin_arkworks_backend::Marlin as ConcreteBackend; } else { compile_error!("please specify a backend to compile with"); } diff --git a/crates/nargo/src/cli/fs/mod.rs b/crates/nargo/src/cli/fs/mod.rs index a8c7e96388c..e8bed13d7a8 100644 --- a/crates/nargo/src/cli/fs/mod.rs +++ b/crates/nargo/src/cli/fs/mod.rs @@ -6,11 +6,11 @@ use std::{ use crate::errors::CliError; -pub mod acir; -pub mod inputs; -pub mod keys; -pub mod proof; -pub mod witness; +pub(super) mod acir; +pub(super) mod inputs; +pub(super) mod keys; +pub(super) mod proof; +pub(super) mod witness; fn create_dir>(dir_path: P) -> Result { let mut dir = std::path::PathBuf::new(); @@ -19,11 +19,11 @@ fn create_dir>(dir_path: P) -> Result { Ok(dir) } -pub(crate) fn create_named_dir(named_dir: &Path, name: &str) -> PathBuf { +pub(super) fn create_named_dir(named_dir: &Path, name: &str) -> PathBuf { create_dir(named_dir).unwrap_or_else(|_| panic!("could not create the `{name}` directory")) } -pub(crate) fn write_to_file(bytes: &[u8], path: &Path) -> String { +pub(super) fn write_to_file(bytes: &[u8], path: &Path) -> String { let display = path.display(); let mut file = match File::create(path) { @@ -37,7 +37,7 @@ pub(crate) fn write_to_file(bytes: &[u8], path: &Path) -> String { } } -pub(crate) fn load_hex_data>(path: P) -> Result, CliError> { +pub(super) fn load_hex_data>(path: P) -> Result, CliError> { let hex_data: Vec<_> = std::fs::read(&path).map_err(|_| CliError::PathNotValid(path.as_ref().to_path_buf()))?; diff --git a/crates/nargo/src/cli/mod.rs b/crates/nargo/src/cli/mod.rs index 1f4901a692d..805a3a238a2 100644 --- a/crates/nargo/src/cli/mod.rs +++ b/crates/nargo/src/cli/mod.rs @@ -120,7 +120,7 @@ mod tests { /// Compiles a file and returns true if compilation was successful /// /// This is used for tests. - pub fn file_compiles>(root_file: P) -> bool { + fn file_compiles>(root_file: P) -> bool { let mut driver = Driver::new(&acvm::Language::R1CS); driver.create_local_crate(&root_file, CrateType::Binary); super::add_std_lib(&mut driver); diff --git a/crates/nargo/src/git.rs b/crates/nargo/src/git.rs index e6273a903ac..7f103e21b38 100644 --- a/crates/nargo/src/git.rs +++ b/crates/nargo/src/git.rs @@ -1,6 +1,6 @@ use std::path::PathBuf; -pub fn git_dep_location(base: &url::Url, tag: &str) -> PathBuf { +pub(crate) fn git_dep_location(base: &url::Url, tag: &str) -> PathBuf { let folder_name = super::resolver::resolve_folder_name(base, tag); super::nargo_crates().join(folder_name) @@ -12,7 +12,7 @@ pub fn git_dep_location(base: &url::Url, tag: &str) -> PathBuf { /// github-rs looks promising, however it seems to require an API token /// /// One advantage of using "git clone" is that there is effectively no rate limit -pub fn clone_git_repo(url: &str, tag: &str) -> Result { +pub(crate) fn clone_git_repo(url: &str, tag: &str) -> Result { use std::process::Command; let base = match url::Url::parse(url) { diff --git a/crates/nargo/src/lib.rs b/crates/nargo/src/lib.rs index fd001dd3e98..fe198df7815 100644 --- a/crates/nargo/src/lib.rs +++ b/crates/nargo/src/lib.rs @@ -1,4 +1,6 @@ #![forbid(unsafe_code)] +#![warn(unreachable_pub)] + use noirc_frontend::graph::CrateType; use std::path::{Path, PathBuf}; diff --git a/crates/nargo/src/resolver.rs b/crates/nargo/src/resolver.rs index 3f21f698fd7..553f5ec19a8 100644 --- a/crates/nargo/src/resolver.rs +++ b/crates/nargo/src/resolver.rs @@ -49,7 +49,7 @@ impl<'a> Resolver<'a> { /// Note that the backend is ignored in the dependencies. /// Since Noir is backend agnostic, this is okay to do. /// XXX: Need to handle when a local package changes! - pub fn resolve_root_config( + pub(crate) fn resolve_root_config( dir_path: &std::path::Path, np_language: Language, ) -> Result { @@ -134,7 +134,7 @@ impl<'a> Resolver<'a> { } } - pub fn resolve_git_dep(url: &str, tag: &str) -> Result { + fn resolve_git_dep(url: &str, tag: &str) -> Result { match super::git::clone_git_repo(url, tag) { Ok(path) => Ok(path), Err(msg) => Err(CliError::Generic(msg)), diff --git a/crates/nargo/src/toml.rs b/crates/nargo/src/toml.rs index 04df9dd0c39..562eea49617 100644 --- a/crates/nargo/src/toml.rs +++ b/crates/nargo/src/toml.rs @@ -5,15 +5,16 @@ use std::path::Path; use crate::errors::CliError; #[derive(Debug, Deserialize, Clone)] -pub struct Config { - pub package: Package, - pub dependencies: BTreeMap, +pub(crate) struct Config { + #[allow(dead_code)] + pub(crate) package: Package, + pub(crate) dependencies: BTreeMap, } impl Config { // Local paths are usually relative and are discouraged when sharing libraries // It is better to separate these into different packages. - pub fn has_local_path(&self) -> bool { + pub(crate) fn has_local_path(&self) -> bool { let mut has_local_path = false; for dep in self.dependencies.values() { if let Dependency::Path { .. } = dep { @@ -25,25 +26,26 @@ impl Config { } } +#[allow(dead_code)] #[derive(Debug, Deserialize, Clone)] -pub struct Package { +pub(crate) struct Package { // Note: a package name is not needed unless there is a registry - pub authors: Vec, + authors: Vec, // If not compiler version is supplied, the latest is used // For now, we state that all packages must be compiled under the same // compiler version. // We also state that ACIR and the compiler will upgrade in lockstep. // so you will not need to supply an ACIR and compiler version - pub compiler_version: Option, - pub backend: Option, - pub license: Option, + compiler_version: Option, + backend: Option, + license: Option, } #[derive(Debug, Deserialize, Clone)] #[serde(untagged)] /// Enum representing the different types of ways to /// supply a source for the dependency -pub enum Dependency { +pub(crate) enum Dependency { Github { git: String, tag: String }, Path { path: String }, } diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 8c627c8fc28..913a4491b05 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -1,4 +1,6 @@ #![forbid(unsafe_code)] +#![warn(unreachable_pub)] + use std::{collections::BTreeMap, str}; use acvm::{acir::native_types::Witness, FieldElement}; diff --git a/crates/noirc_abi/src/serialization.rs b/crates/noirc_abi/src/serialization.rs index f4bbdee4661..1156ad4473f 100644 --- a/crates/noirc_abi/src/serialization.rs +++ b/crates/noirc_abi/src/serialization.rs @@ -18,7 +18,7 @@ struct StructField { typ: AbiType, } -pub fn serialize_struct_fields( +pub(crate) fn serialize_struct_fields( fields: &BTreeMap, s: S, ) -> Result @@ -32,7 +32,7 @@ where fields_vector.serialize(s) } -pub fn deserialize_struct_fields<'de, D>( +pub(crate) fn deserialize_struct_fields<'de, D>( deserializer: D, ) -> Result, D::Error> where diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 2ba135abfab..1d54630f586 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -1,4 +1,6 @@ #![forbid(unsafe_code)] +#![warn(unreachable_pub)] + use acvm::acir::circuit::Circuit; use acvm::Language; diff --git a/crates/noirc_errors/src/lib.rs b/crates/noirc_errors/src/lib.rs index 99dd6554dbb..3118e9e5e26 100644 --- a/crates/noirc_errors/src/lib.rs +++ b/crates/noirc_errors/src/lib.rs @@ -1,4 +1,6 @@ #![forbid(unsafe_code)] +#![warn(unreachable_pub)] + mod position; pub mod reporter; pub use position::{Location, Position, Span, Spanned}; diff --git a/crates/noirc_errors/src/reporter.rs b/crates/noirc_errors/src/reporter.rs index dec215295ad..1ec5fa3dbb5 100644 --- a/crates/noirc_errors/src/reporter.rs +++ b/crates/noirc_errors/src/reporter.rs @@ -91,12 +91,12 @@ impl std::fmt::Display for CustomDiagnostic { #[derive(Debug, PartialEq, Eq)] struct CustomLabel { - pub message: String, - pub span: Span, + message: String, + span: Span, } impl CustomLabel { - pub fn new(message: String, span: Span) -> CustomLabel { + fn new(message: String, span: Span) -> CustomLabel { CustomLabel { message, span } } } diff --git a/crates/wasm/src/lib.rs b/crates/wasm/src/lib.rs index 36fb0d60850..157a17c321d 100644 --- a/crates/wasm/src/lib.rs +++ b/crates/wasm/src/lib.rs @@ -1,4 +1,6 @@ #![forbid(unsafe_code)] +#![warn(unreachable_pub)] + use acvm::acir::circuit::Circuit; use gloo_utils::format::JsValueSerdeExt; use serde::{Deserialize, Serialize};